tests/lib: introduce pkgdb helper library

[morphis]

All functions in the pkdb library will abstract different packaging
functions so that we have a common way to install/remove/update
packages in a distribution independent way.

This is required for later changes to add support for other distros
like Fedora or openSUSE.

The new helper functions are not used everywhere yet but follow up
changes will introduce their use everywhere so that everything is
abstracted through pkgdb functions.

[zyga]

Some quick comments

[zyga]

Curious syntax, I wasn't familiar with it. What does this do?

[morphis]

It marks a fall through. See https://stackoverflow.com/questions/12010686/case-statement-fallthrough

[zyga]

Interesting, I never used this in shell :)

[fgimenez]

Nice! Need to remember to use this next time :)

[mvo5]

It seems this is slightly unusual, the syntax that is more familiar to me is:

debian_name_package() {
     case "$1" in
        xdelta3|curl|python3-yaml)
            echo $1

we should also make sure shellcheck is happy, I guess it will complain about the lack of quotes around "$1" :)

[zyga]

A bit of a bike-shead but instead of building a pretend uniform map of packages across distributions (wait until we get to gentoo) how about just having a function like this:

install_package debian:snapd fedora:snapd

Which will use ID and ID_LIKE to install the subset of packages that match what is given on command line. Then we don't need to maintain any central mapping of any kind. The function can be made to fail if no packages match (say we add openSUSE) for better reliability.

[morphis]

The function can be made to fail if no packages match (say we add openSUSE) for better reliability.

That already happens with this PR.

Which will use ID and ID_LIKE to install the subset of packages that match what is given on command line.

What do you mean with ID/ID_LIKE?

A bit of a bike-shead but instead of building a pretend uniform map of packages across distributions (wait until we get to gentoo) how about just having a function like this:

Aren't we doing the same with install_package debian:snapd fedora:snapd? In some cases those lines can get really long. My preference would be to have a generic name for what should be installed and then the logic below picks what is necessary. That way the test code itself can stay less cluttered with distro specifics.

[zyga]

Those can get very long but they are immediately explicit and unambiguous. Anyone working on a given distribution can immediately relate to the outcome and predict what will happen. The special centralized identifiers (that sometimes match packages but not necessarily) feel a bit too magic to me. I specifically am not fond of the set of tables we need to maintain.

The ID and ID_LIKE fields in /etc/os-release can be used to key to filter the part before the colon. A single instruction:

install_package debian:snapd

Will work correctly on Debian and all of its derivatives (through the ID_LIKE property).

[morphis]

Well, the package name here serves really like an "interface" which has different implementations. Having something like install_package debian:snapd means we have to touch a lot of test cases when we add a new distribution and this doesn't even respect different versions of those distributions yet.

Expressing a general name like "netcat" which then is resolved to the correct set of packages needed on a given distributions takes away those extra maintenance steps we have to do for every test case and limits any changes to a single file which can abstract anything by distribution and by their versions.

[fgimenez]

my 2c's about this discussion, +1 for the centralized approach, passing the map-like parameters still might need some inspection to understand what's really going on in the called function.

[mvo5]

@zyga could you elaborate on your concern about gentoo please ? I think @morphis has a point about porting being easier with a centralized place for the package names. If its pkg_install curl in a single place we will not have to each test that uses pkg_install. OTOH I count 12 tests that use apt install or apt-get install right now so the decentralized is not bad. But having only a single place to look at seems easier (unless there is something I miss here (like gentoo) that is so different that this is impractical.

[zyga]

My point that abstract identifiers are more harm/confusion than benefit. Maybe on Debian something is in one package but in another distribution you need two packages (for whatever reason, maybe the base is different). Trying to cram this into a ever-growing table that one must look up each time a new test is written feels wrong to me. I'd much rather implement a "install_package" function that installs packages, as they are spelled out in the argument list, simply ignoring packages for foreign distributions (hence the ${ID}:${package} syntax). The case with gentoo is that packages have widely different, long and path-like names. I really think this is the wrong abstraction.

Let test developers just install what they want. Alternatively my idea can be converted to:

maybe_apt install some packages
maybe_yum install other library-devel too
maybe_zypper ...
maybe_emerge

Where all the maybe_ functions are a no-op unless the given packaging tool exists. (Though I still prefer the thing to be spelled on one line as in my original proposal)

[morphis]

My point that abstract identifiers are more harm/confusion than benefit. Maybe on Debian something is in one package but in another distribution you need two packages (for whatever reason, maybe the base is different). Trying to cram this into a ever-growing table that one must look up each time a new test is written feels wrong to me.

Actually that is what you normally do in programming. You fold those intends into a well defined interface like "I want to print a document". Everything necessary for this isn't handled by your call to the interface but by the implementation below. By putting all these tiny details into the test cases we only make those unnecessarily complex with a lot of distribution specific things.

If there is a much more complex setup required for specific things we could even spell those things out into individual functions later on like distro_install_and_configure_cups. However for what we have today those mapping tables are not that huge and I don't see them getting them real huge in the near future.

[mvo5]

It looks to me like the distro_install cups works on gentoo too, the distro_install might looks a bit ugly inside because it needs to do emerge cups-foo-gentoo cups-bar-with-some-extras but that appears to me not worse than springle the tests with this knowledge. I.e. the complexity/ugliness will appear somewhere, the question is if we show it in the test (which does not sound like a good deal given that the test is not about how cups is called on N distros) or if we show it in a central place. The other benefit is that when porting to a new distro there is just a single place to change, otherwise it will be a git grep distro_install exercise (which is not too bad so probably not a strong argument). Maybe I am missing something still but it seems like both approaches are fine but the centralized is less distracting in the tests itself. Plus I think it is something we can easily change later if it turns out that the approach we take is actually not pratcial.

[morphis]

On IRC we decided to go with the current implementation and rework later if we have to.

[fgimenez]

LGTM

[zyga]

Curious syntax, I wasn't familiar with it. What does this do?

[morphis]

It marks a fall through. See https://stackoverflow.com/questions/12010686/case-statement-fallthrough

[zyga]

Interesting, I never used this in shell :)

[fgimenez]

Nice! Need to remember to use this next time :)

[mvo5]

It seems this is slightly unusual, the syntax that is more familiar to me is:

debian_name_package() {
     case "$1" in
        xdelta3|curl|python3-yaml)
            echo $1

we should also make sure shellcheck is happy, I guess it will complain about the lack of quotes around "$1" :)