ungleich-public/cdist

Fork 10

different ways of updating the apt package index #4

New issue

Open

opened 2021-11-20 11:24:43 +00:00 by ungleich-gitea · 6 comments

ungleich-gitea commented

2021-11-20 11:24:43 +00:00

Owner

It's a bit difficult because each type updates the package to an other point of time:

__package_update_index just updates the index like you would when you execute apt-get update with the addition to not run the update if the index is younger than $x
__apt_update_index updates the index only if an element in /etc/apt/ is newer than the index
__apt_source updates the index if itself changes (correct speaking if the modifing source file changes)
__package_apt updates the index before the package install if it's too old

Moving everything to a single type is a bit difficult to do, but better for maintainability (1 instead of 4 MR's). Having one singleton type isn't a problem if you only depend on it, but get's bigger when the index update depends on something else (like changed sources files). Funny things starts and probably end like __uci_commit ended. It's also the question if we need multiple index updates in one cdist run (What if __apt_key needs to install curl but the index update should happen after the __apt_source run? Assuming we want to update the package index before any package installation because it could be too old.) and so on. But also, we don't want to run the index update a useless second round.

As my first thoughts about this aren't that pretty, this issue should group the whole thread at one place in the hope we find a solution for this code duplication. Maybe with a layer outside the types? A library?

ping @ander and @ssrq

There are multiple types that update the apt package index as I rediscovered them where I wanted to fix them cause of an interactive prompt that the suite changes and aborts when called non-interactive (see #861). Previous it was also noted by Ander at an other fix for `__package_update_index`. It's a bit difficult because each type updates the package to an other point of time: 1. `__package_update_index` just updates the index like you would when you execute `apt-get update` with the addition to not run the update if the index is younger than `$x` 2. `__apt_update_index` updates the index only if an element in `/etc/apt/` is newer than the index 3. `__apt_source` updates the index if itself changes (correct speaking if the modifing source file changes) 4. `__package_apt` updates the index before the package install if it's too old Moving everything to a single type is a bit difficult to do, but better for maintainability (1 instead of 4 MR's). Having one singleton type isn't a problem if you only depend on it, but get's bigger when the index update depends on something else (like changed sources files). Funny things starts and probably end like `__uci_commit` ended. It's also the question if we need multiple index updates in one cdist run (What if `__apt_key` needs to install `curl` but the index update should happen after the `__apt_source` run? _Assuming we want to update the package index before any package installation because it could be too old._) and so on. But also, we don't want to run the index update a useless second round. As my first thoughts about this aren't that pretty, this issue should group the whole thread at one place in the hope we find a solution for this code duplication. Maybe with a layer outside the types? A library? ping @ander and @ssrq

ungleich-gitea commented

2021-11-20 15:24:48 +00:00

Author

Owner

so, perhaps something like this:

https://code.ungleich.ch/ungleich-public/cdist/-/compare/master...ander%2F__package_apt_update_index

so, perhaps something like this: https://code.ungleich.ch/ungleich-public/cdist/-/compare/master...ander%2F__package_apt_update_index

ungleich-gitea commented

2021-11-20 15:24:50 +00:00

Author

Owner

mentioned in merge request !1028

ungleich-gitea commented

2021-11-20 15:24:51 +00:00

Author

Owner

Good cents.

Sure there is: install something, change APT's conf (add keys, sources, whatnot) and then again install something. Then there's this magical question if __apt_key and __apt_source should also automatically update index onchange (sounds familiar? 😄)? I think not, because if you call __package_apt after them, index gets updated, because there have been change in /etc/apt/. So, automatically updating index is only important for __package_apt? If __apt_update_index is singleton, I'm not sure what's going to happen in this kind of situations, when there's actual need to run __apt_update_index multiple times in one configuration run. So, instead, we only need to implement Very Smart Index Updating (tm) for __package_apt and deprecate everything else?
🤷

Good cents. 1. Sure there is: install something, change APT's conf (add keys, sources, whatnot) and then again install something. Then there's this magical question if `__apt_key` and `__apt_source` should also automatically update index onchange (sounds familiar? :smile:)? I think not, because if you call `__package_apt` after them, index gets updated, because there have been change in `/etc/apt/`. So, automatically updating index is only important for `__package_apt`? If `__apt_update_index` is singleton, I'm not sure what's going to happen in this kind of situations, when there's actual need to run `__apt_update_index` multiple times in one configuration run. So, instead, we only need to implement Very Smart Index Updating (tm) for `__package_apt` and deprecate everything else? 2. :shrug:

ungleich-gitea commented

2021-11-20 15:24:52 +00:00

Author

Owner

First of all: yes, the current situation is a mess and it should be solved!

JM2C:

Why can't __*_update_index be singletons? Are there any cases where one would need to update the index multiple times during a single cdist run?
Do we need __package_update_index? What would be the use case?
Are there any other use cases other than __package_update_index --force?

First of all: yes, the current situation is a mess and it should be solved! JM2C: 1. Why can't `__*_update_index` be singletons? Are there any cases where one would need to update the index multiple times during a single cdist run? 2. Do we need `__package_update_index`? What would be the use case? Are there any other use cases other than `__package_update_index --force`?

ungleich-gitea commented

2021-11-20 15:24:54 +00:00

Author

Owner

https://github.com/ungleich/cdist/pull/780

PR in GitHub reminded me this issue.

While sitting in the garden and enjoying nice autumn weather, I did some thinking.

I'll start with following proposal and then try to shoot holes into it:

__package_* types should only deal with installing (and potentially upgrading) packages.
__*_update_index should only update index based on some parameters with sane defaults. For example: always update if something in /etc/apt is newer than /var/lib/apt/lists or if latter is missing. Or update cache if it's older than X hours or cache files are missing.
__package_update_index should behave similarly as __package - calls correct os-specific type according to information given by explorer(s).
__package_* then should call __*_update_index in type manifest. In theory, if first instance of __package_* updates index and type itself is non-parallel, then other instances shouldn't retry it.

Now shooting some holes.

Currently __apt_update_index and __package_update_index are singletons. This must be changed then? Same time calling singleton type multiple times don't raise error...

If __package_* can handle index update anyway, then why splitting this functionality into separate type(s)? Right, do one thing and do it well.

Some users might prefer to explicitly call something like __*_update_index --force or __package_update_index --force on every configuration run before doing anything else.

Is there something I forgot or don't see?

https://github.com/ungleich/cdist/pull/780 PR in GitHub reminded me this issue. While sitting in the garden and enjoying nice autumn weather, I did some thinking. I'll start with following proposal and then try to shoot holes into it: 1. `__package_*` types should only deal with installing (and potentially upgrading) packages. 2. `__*_update_index` should only update index based on some parameters with sane defaults. For example: always update if something in `/etc/apt` is newer than `/var/lib/apt/lists` or if latter is missing. Or update cache if it's older than X hours or cache files are missing. 3. `__package_update_index` should behave similarly as `__package` - calls correct os-specific type according to information given by explorer(s). 4. `__package_*` then should call `__*_update_index` in type manifest. In theory, if first instance of `__package_*` updates index and type itself is non-parallel, then other instances shouldn't retry it. Now shooting some holes. Currently `__apt_update_index` and `__package_update_index` are singletons. This must be changed then? Same time calling singleton type multiple times don't raise error... If `__package_*` can handle index update anyway, then why splitting this functionality into separate type(s)? Right, do one thing and do it well. Some users might prefer to explicitly call something like `__*_update_index --force` or `__package_update_index --force` on every configuration run before doing anything else. Is there something I forgot or don't see?

ungleich-gitea commented

2021-11-20 15:24:55 +00:00

Author

Owner

changed title from diffrent ways of updating the apt package index to diff{+e+}rent ways of updating the apt package index

changed title from **diffrent ways of updating the apt package index** to **diff{+e+}rent ways of updating the apt package index**