distro: unify transform/match into a single concept

[mvo5]

This commit simplifies the concept of transform/match
into a single "match" directive. When moving to the
generic distros concept we can not assume that every
distro follows our schema of <name>-<major>.<minor>
and we also need to provide compatibility with older
ways of denoting rhel releases (like "rhel-89" which
means "rhel-8.9"). To accomendate this the "transform_re"
was added. But this causes a weird mix of "match" that
takes glob expression and transform that takes regexp.
Its also cumbersome to have two concepts that do almost
the same.

So this commit makes "match" do both the matching and
(if capture groups are used) the transformation. In
the simple case not much changes, e.g. fedora/rhel-10
that do not need backward compatibility are now written
as:

match: 'fedora-[0-9][0-9]+'
match: 'rhel-10\.[0-9]{1,2}'

and rhel{8,9} are now written as:

match: '(?P<name>rhel)-(?P<major>9)\.?(?P<minor>[0-9]{1,2})'

A match can either have no capture groups or name,major or
name,major,minor. And if a capture group is used the
provided name will be normalized to <name>-<major> or
<name>-<major>.<minor>.

Note that there is a certain overlap with
distroidparser.Standardize(). I think we want a followup
that deals with that, distroidparser was important in the
past but now with everything as a generic distro it seems
we might consider sunsetting it.

Thanks to Ondrej for suggesting this.

[ondrejbudai]

This is awesome, I love it! ❤️

[thozza]

Note that there is a certain overlap with distroidparser.Standardize(). I think we want a followup that deals with that, distroidparser was important in the past but now with everything as a generic distro it seems we might consider sunsetting it.

I'm all for unification.

Just please keep in mind that distroidparser.Standardize() is also used to standardize repo config filenames. Doing a sanity check by testing the integration into osbuild-composer first, and the dot-notation test case passing is a prerequisite from my PoV.

[thozza]

Thanks, this is nice.

I noticed that both the NewDistroYAML() and ParseID() call loadDistros() and do the matching and potential name transformation. I'm not sure about the execution flow after the YAMLification, as it has become more complex now. Would it make sense to perform the loading, matching, and transformation in a single place? I'm hoping the loading, matching, and transformation are not done unnecessarily too often.

Other than that, some regular expressions for matching distros could be made more strict (or accurate), but that's not a big deal (i.e., not allowing leading zeros in the version part).

Any changes can be a follow-up from my PoV.

[thozza]

Nitpick, this should be ideally fedora-[1-9][0-9]+, to not match i.e. fedora-043.

[thozza]

Nitpick: the latest RHEL-7 is 7.9 and there won't be any newer one, so this could be really rhel-7\.[0-9]

[thozza]

I would suggest to also test a case when the regex has a valid number of named match groups, but the names of the match groups are not what the code expects, i.e. (?P<blabla1>rhel)-(?P<blabla2>8)\.?(?P<blabla3>[0-9]{1,2}).

[achilleas-k]

I had the same thought

[achilleas-k]

Couple of minor comments. Otherwise, LGTM (Approved)

[achilleas-k]

This could be dropped, right? Since it's identical to the default case.
I suppose we can keep it so that the 2 case is explicitly covered in the list, since dropping it might make it look like we missed a case, but that could be covered by a comment.

[achilleas-k]

I had the same thought

[mvo5]

[..]

I noticed that both the NewDistroYAML() and ParseID() call loadDistros() and do the matching and potential name transformation. I'm not sure about the execution flow after the YAMLification, as it has become more complex now. Would it make sense to perform the loading, matching, and transformation in a single place? I'm hoping the loading, matching, and transformation are not done unnecessarily too often.
[..]

Yeah, I started to go over pkg/distro/{generic,defs} to cleanup this up a bit, I need to have another go and see if this can be further simplified or optimized. Some is unavoidable as ParseID() really needs to load the YAML now because the matching/normalization can be defined there. But I will double check if we DTRT.