#968 Backwards-incompatible stealth rpm change making ~1570 Fedora packages insafe
Closed: invalid 2 years ago by ignatenkobrain. Opened 2 years ago by nim.

In the past days, investigation on a broken build identified an rpm backwards incompatible parser change, that makes ~1570 Fedora packages latent time bombs (they seem to work fine, but as soon as you need to add a patch using normal Fedora conventions, or build in environments with non-default sourcedir definitions, the build fails.

upstream is completely unhelpful and refusing both to back out the change, and to investigate the packaging patterns required to adapt to its parsing changes.

https://github.com/rpm-software-management/rpm/issues/1161
https://bugzilla.redhat.com/show_bug.cgi?id=1820349

In their own words “ just leave the specs alone, they're not broken”, packagers “managed just fine for about a year. I'm quite confident they'll manage in the future too.” They can write pages on why they made the change, and nothing on how to deal with the consequences.

I do not agree with upstream that keeping a packaging pattern in the distribution, that only takes now one patch addition to break, is fine.

I do not agree with upstream that packagers should be left alone to deal with that, doing spec-by-spec local workarounding, not knowing they are part of a mass breakage, without any Fedora or rpm help (that seems to be the whole point, leave people in the dark so they can not bother upstream with help requests).

However, I can not change the pattern to something safe with the new rpm parser code. I tried, that hits other rpm quirks/bugs that upstream is also refusing to look at let alone work on).

Therefore, I am forced to acknowledge a complete breakdown in upstream cooperation, and take a step back. Iam escalating to FPC for guidance and help.

(I could, easily, force the issue by triggering the problem in the 1570 packages, forcing upstream to look at it, but that would be as irresponsible as upstream’s stance IMHO).


I'll read it all, but from the first glance this really looks like "used to work by accident" kinda situation. We cannot expect RPM upstream to remain bug-to-bug compatible forever.

I do not agree with upstream that keeping a packaging pattern in the distribution, that only takes now one patch addition to break, is fine.

Said patch addition being https://src.fedoraproject.org/rpms/dejavu-fonts/c/03ba0905119e48fd3e822411f92496c2fb43d8f0?branch=master

I'll let people draw their own conclusions, but I think Michael Schroeder summarized it quite well in https://github.com/rpm-software-management/rpm/issues/1161#issuecomment-610317827 - how is the following any different?

Patch0:         %{foo}
%define foo bar.diff

Also it's worth repeating here that this change took place in rpm 4.15 already - almost a year ago.

I don’t expect rpm upstream to remain bug-for-bug compatible forever. That’s why none of my messages start by requesting a revert (maybe I should have instead of trying to make the new behaviour work). However, now that the bug-for-bug behaviour has changed, I do expect upstream to help making the new bug-to-bug work when it is incompatible with existing specs and no generic spec strategy dan be defined

I think "define things before they are referenced" is a sane strategy, in any programming language :)

I think "define things before they are referenced" is a sane strategy, in any programming language :)

Fine, we all agree on that, now on the practical, not strategic level what does that mean for the two testcase specs? Production specs and packaging guidelines are about practical, not strategic considerations.

What I'm missing here is any idea of what anyone expects FPC to actually do about this. It's entirely true that packaging guidelines are entirely practical: we live with the RPM that we have. If all of this is just about adding a sentence to a relevant place in the guidelines that says "Don't use macros before they are defined" and specifically mentioning that some RPM header info may define macros, then I guess that's fine. It does, however, seem kind of obvious. The dejavu-fonts spec patch linked above seems particularly bizarre to me, in a "why would anyone actually do that?" kind of way. But I guess if it works in some circumstances and breaks in others then it will definitely confuse people and it doesn't hurt to document it.

Of course, maybe I'm just missing some important point here. I'm not even understanding what actually gets broken here, since as far as I've been able to tell from the discussion, the packages still build unless you do some other thing that's not the default in any of the usual build setups.

Anyway, if FPC is being asked to get RPM to undo this behavioral change, or to somehow mediate the discussion with RPM maintainers, then I don't think that's going to happen. I accept that RPM changes may break some of the weirder stuff that's done in some of our packages. I still prefer the current state of RPM where it actually evolves to give us more useful functionality instead of the old state where it changed very slowly. We may occasionally have to scramble to fix things if a new RPM lands in rawhide and causes issues. (This has happened to me not all that long ago.) But it doesn't sound like that's the case here, either, since the relevant change landed some time ago.

It sounds to me like the %description has to be last, so to keep %description right next to Name: and Summary:, Sources: and Patches have been moved to be declared earlier in the SPEC than Name: and Summary:

With the RPM change, to maintain the robustness and still allow using %{name} in the Sources and Patches, you'd have to put Patches: and Sources: after the Name and Summary, but before %description. This makes the SPEC awkward to read, as the %description of the package could be several pages down from the Name and Summary, depending on the number of patches and sources inbetween.

The issue hadn't been hit before because no one tried to use %{name} in a patch or source on a package with its Patches and Sources ordered before Name, to avoid the %description robustness issue.

@tibbs, thanks for the feedback

  1. I do not like the dejavu solution, it’s neither natural (packagers expect to put patches next to sources) nor generalisable (the location used in the dejavu spec is not available in other specs. It can not work as a general solution)

  2. Practically, there are only two locations where you can declare sources+patches in a generic, documentable way for the affected specs: either before general package blocks (name+summary+description) or after those

  3. Fedora had been using the first location, except the rpm change broke it (in a latent way, ie it seems to continue working, till you need a patch or something like that, and then you get into horrible dejavu-like handcrafted workaround land)

  4. the second solution (which would respect the code flow upstream wishes to make mandatory) hits unbounded %description misbehaviour

  5. upstream’s “solution” is to do nothing, neither make the old pattern work again (there are several ways to do it, without going back to pre-change quirk land), nor to make their own target codeflow work (that would require fixing the %description problem).

It sounds to me like the %description has to be last, so to keep %description right next to Name: and Summary:, Sources: and Patches have been moved to be declared earlier in the SPEC than Name: and Summary:

With the RPM change, to maintain the robustness and still allow using %{name} in the Sources and Patches, you'd have to put Patches: and Sources: after the Name and Summary, but before %description. This makes the SPEC awkward to read, as the %description of the package could be several pages down from the Name and Summary, depending on the number of patches and sources inbetween.

In some of the affected spec files, the whole name/summary/description block is generated, so there is literally no point at which Sources+Patches could be inserted. (the specs that do that would still work for now, as a side-effect of rpm expansion, but from a long-term maintenance view making declaration order depend on rpm expansion side effects is crazy)

It‘s easy to check just by taking
https://src.fedoraproject.org/rpms/sil-mondulkiri-extra-fonts/blob/master/f/sil-mondulkiri-extra-fonts.spec
and
https://src.fedoraproject.org/rpms/sil-mondulkiri-fonts/blob/master/f/sil-mondulkiri-fonts.spec

and trying to put the source+patches block in a common location (both before and after rpmspec -P, as had been the case so far)

The issue hadn't been hit before because no one tried to use %{name} in a patch or source on a package with its Patches and Sources ordered before Name, to avoid the %description robustness issue.

More accurately, not since the change, or since the change but without reporting the breakage.

Uh ... those .spec files look like very dark magic and I am surprised that this has ever worked

They also either contain invalid unicode in the %changelog section, or it just doesn't render correctly in pagure ...

I'm wondering why you're not putting the Name and Summary tag at the top with the other Version and Release stuff?

They also either contain invalid unicode in the %changelog section, or it just doesn't render correctly in pagure ...

It’s good unicode, pagure has an UTF-8 rendering bug somewhere. I've reported it some time ago:
https://pagure.io/pagure/issue/4748

You see it in new package imports, where description renders fine in Fedora, new package request bugzilla and rpmlint, and is broken when pagure displays the same text as README.md

I'm wondering why you're not putting the Name and Summary tag at the top with the other Version and Release stuff?

Unfortunately, that is not possible because in some cases srpm name = first subpackage name, and rpm does not permit declaring those separately (that‘s the case for almost all our Go packages because that’s the pattern eclipseo cooked into go2rpm, and you need to deliberately tweak it to change the srpm name, that most packagers do not bother to do).

https://src.fedoraproject.org/rpms/sil-mondulkiri-extra-fonts/blob/master/f/sil-mondulkiri-extra-fonts.spec
and
https://src.fedoraproject.org/rpms/sil-mondulkiri-fonts/blob/master/f/sil-mondulkiri-fonts.spec

are good testcases because they represent those two patterns, while being more or less identical from other points of views. You can diff them, rpmspec -P them, and see where the whole SRPM/RPM name unification bites.

Unfortunately, that is not possible because in some cases srpm name = first subpackage name, and rpm does not permit declaring those separately (that‘s the case for almost all our Go packages because that’s the pattern eclipseo cooked into go2rpm, and you need to deliberately tweak it to change the srpm name, that most packagers do not bother to do).

Would you care to elaborate here please? I do not understand at all what you mean by "srpm name = first subpackage name".

I always thought "SRPM name => main package name" (if a "main" package exists)?

In most of those spec files there is no "main" special package, just a series of generated subpackages, that all use the same rules.

When upstream project name matches whatever Fedora guidelines want created as subpackage, the first subpackage is generated with a starting Name: (and is otherwise identical to the other subpackages, that start with %package -n). That’s the case for:
https://src.fedoraproject.org/rpms/sil-mondulkiri-fonts/blob/master/f/sil-mondulkiri-fonts.spec

When it does not match, the spec includes a manual Name: block just to provide srpm metadata. That’s the case for:
https://src.fedoraproject.org/rpms/sil-mondulkiri-extra-fonts/blob/master/f/sil-mondulkiri-extra-fonts.spec

That’s why there is not place for SRPM-specific Sources+Patches blocks inside the Name/Summary/description blocks, that breaks the symmetry, which is necessary to generate subpackage metadata. Humans do not care much, but code cares about regularity a lot.

Symmetry, without any magic subpackage that declares sources when the others do not, is also a requirement to make the result composable (which is the case right now).

In some sense, I want to say that the deep magic involved in some of these packages might just be the equivalent of flying too close to the sun; relying too much on internal RPM implementation details may have resulted in something which simply is not stable moving forward.

Regardless, I still have the same issue that I originally stated: I do not understand what FPC is being asked to do here.

I personally think the best course of action is to work with the RPM maintainers to find reasonable ways to achieve the needed functionality. If they are unwilling to restore the original behavior and there truly are broken packages in Fedora, then a solution (or at least a workaround) needs to be found.

@tibbs: right now upstream is refusing both to help find a solution with the new state of things for existing packages, and to revert (not my preferred outcome either, but better to leave things broken).

Ok, there is nothing FPC can do. You can go to FESCo and ask to revert RPM 4.16 change, but you'll get my -1 right away.

Your macros (the fonts ones) are uber-complicated and nobody except of you can maintain them. You have relied on things which are not meant to work the way you use them. This has changed in RPM 4.16. Upstream is right that Patch/Source should not expand twice.

You either have to fix macros so that they work with RPM 4.16 or we will have to ask you to revert all that black magic how the macros work.

@ignatenkobrain I though we were on in there together. And, I am not the one who changed things and broke spec files as a result. All the work I contributed has been heavily tested against existing rpm behaviour and documentation.

However, if you’re asking to revert my work so there is no need to think about the consequences of rpm parsing changes, I will revert to the situation that existed before I contributed man-years of work to Fedora. It is not possible do do this kind of work anyway, if rpm can change behaviour at will, without announcement, and go AWOL when downstream tries to make existing specs work with the result.

Once more, "this change" is not new in rpm 4.16, it already changed in 4.15, so it's in all current Fedora versions, been there for almost a year.

That change does not prevent any particular source/patch ordering, that absolutely did NOT change there. What changed is that source/patch declarations are only expanded once at the time they appear in the spec, so you can only use macros that have been already declared. Consider the following:

Name: foo
Version: 1.2
Source: %{name}-%{version}.tar.gz
Source: %{name}-%{version}.tar.gz
Name: foo
Version: 1.2

The latter no longer works in rpm >= 4.15 because of the change, prior to that it worked on source/patch due to an implementation detail, but nothing else. So it's not exactly like ability to use %{name} anywhere at all was a birthright now cruelly taken away.

The reason I don't understand the outcry is that you can still put the sources/patches in all the same places as before, if you stick to the "only defined macros" rule. So if you want to add a Patch: before the Name: then call it by its file name. Use of %{name} for entities outside the spec is bad practise anyhow because the package name can change but changing Name: in spec does not change the sources/patches on disk.

Or if you want to use a macro for the project name then declare and use a macro for that, which is free of any rpm spec implied ordering. Which is basically what all these fontpackages do already for many, many things:

Source0:  https://scripts.sil.org/cms/scripts/render_download.php?format=file&media_id=%{archivename}&filename=%{archivename}.zip#/%{archivename}.zip
Source11: 66-%{fontpkgname1}.xml
Version: 2.37
[...]
Source0:  %{forgesource}
Patch0:   %{name}-urn-dtd.patch

Name:     dejavu-fonts

The latter case is the one that is now broken because %{name} is used before Name. There are multiple ways to declare the same thing without relying on special processing (with other unwanted side-effects) for name/source:

Version: 2.37
[...]
Source0:  %{forgesource}
Patch0:   dejavu-fonts-urn-dtd.patch
Name:     dejavu-fonts
Version: 2.37
[...]
Source0:  %{forgesource}
Name:     dejavu-fonts
Patch0:   %{name}-urn-dtd.patch
%global fontname dejavu-fonts
Source0:  %{forgesource}
Patch0:   %{fontname}-urn-dtd.patch
Name:     %{fontname}
Version: 2.37

I also do find the complaint about packagers expecting to put sources/patches together somewhat cheeky in the context of a spec where Name at the bottom of spec preamble, far, far away from Version and Release. Sources/patches don't have together any more than Name/Version/Release do, it's a mere convention.

What I'm missing here is any idea of what anyone expects FPC to actually do about this.

To have something productive come out of this, the FPC could clarify the %{name} macro usage. The %{name} macro in spec refers to the package that will be built, and any relation to source material is packager implied.

In other words: take an arbitrary package, append 123 to its Name: tag and try to build it. If it breaks, the spec is doing it wrong. And yes I'm very aware, most packages are. This is a large part of the reason scl-utils is such an painful exercise to everybody involved.

There might be a case for rpm to add a spec tag to guide people in the right direction eventually, but I don't see that happening as long as it'd be just a mere macro definition. For many purposes self-defined macros are far clearer to the packagers than rpm magic is. Not to mention more compatible.

The reason I don't understand the outcry

And yet, you’ve not proposed any working common solution for the two specs you’ve been pointed at repeatedly. Here, once again
https://src.fedoraproject.org/rpms/sil-mondulkiri-fonts/blob/master/f/sil-mondulkiri-fonts.spec
https://src.fedoraproject.org/rpms/sil-mondulkiri-extra-fonts/blob/master/f/sil-mondulkiri-extra-fonts.spec

Find a common working location for Sources+Patch (with %{name}, before and after rpmspec -P) in both of those and I know the result can be generalised for the 7% of Fedora specs the rpm change broke (I already committed multiple times to generalise it)

The problem created by the rpm change is exhibited in those two specs. Arguing about theory, or other specs, is wasting the time of everyone (yours included).

The problem I see with the .spec files that you keep linking to is that you're probably the only person who understands how they work (or how you want them to work, anyway). Without looking at the definitions of 666 macros I have no idea what's going on, since it doesn't even look like an RPM spec file anymore. There's not even a definition of Name without expanding the macros ...

I'm inclined to say that this issue is entirely self-inflicted, because you can't just assume that undocumented / accidental behavior will keep working as you expect it to. I'd argue that when such changes occur in undocumented behavior, the code that relied on that specific behavior in the first place needs to be adapted. In this case, the font packaging macros will probably need changes, as this probably won't be fixed on the individual .spec level.

Find a common working location for Sources+Patch (with %{name}, before and after rpmspec -P)

No I will not, because you simply cannot expect to use %{name} (or other undefined macros) in any arbitrary location. That's the thing that you need will need to accept and move on.

I've pointed out the simple and clean engineering solutions to that multiple times, including here: https://pagure.io/packaging-committee/issue/968#comment-641830

Those two example packages are building fine as anybody can witness, and will continue to do so as long as the plain, simple rule of define before use is followed. Very early on I asked in the spec whether there's something wrong with the produced binaries as that wouldn't be obvious to me. I haven't seen any such problems pointed out.

@pmatilai

No I will not, because you simply cannot expect to use %{name} (or other undefined macros) in any arbitrary location.

Right now you are refusing to state what non-arbitrary location is possible in those specs. If no good location is possible, the change created an impossible situation, nothing more and nothing less. And no one but you can define what non-arbitrary means, since you’ve been relying on arguments like “well-known unwritten rpm lore” to justify the change. So any location defined by anyone else but you can be invalidated by the same fuzzy rules.

@decathorpe

I'm inclined to say that this issue is entirely self-inflicted, because you can't just assume that undocumented / accidental behavior will keep working as you expect it to.

There is no other way to work on rpm automation because so many things are accidental and undocumented rpm-side.

Any non-trivial rpm automation will sooner or later come to depend on those undocumented accidental behaviours. There are @rh 20-years old redhat-rpm-config macros that depend on rpm variables set as an implementation side effect of other things. Macros you use in every Fedora spec without noticing (for example, you can not use any source without depending variables defined as accidental %setup side effects).

I've pointed out the simple and clean engineering solutions to that multiple times, including here: https://pagure.io/packaging-committee/issue/968#comment-641830

Solutions. The change created such a mess your have no generic solution to propose. Do you really expect us to do one-of-a-kind surgery in more than a thousand and a half specs?

The only thing looking like a generic solution is

%global srpm_name  bar
Name:`%{srpm_name}`

But that’s nothing like simple and clean engineering unless:

  • the variable is official documented rpm variable (not another arbitrary thing that rpm can break at will)
  • it is auto-set by the srpm Name: line
  • rpm uses is by default when it needs the srpm name for something, without requiring an explicit Name: %{srpm_name} redeclaration

Otherwise cut and pasting won’t work between specs, and we’re back in one-of-a-kind land (which is not manageable given the number of affected specs).

Sigh, I give up. For the second time now.

Thus, I’ve posted
https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/83

When it is merged I will apply the corresponding changes to fonts-rpm-macros and go-rpm-macros, and the spec font and go spec files I directly own in devel. That will apply the most excellent help I got here to my spec files. It will also make the resulting spec files backwards incompatible, and probably break a lot of other spec files that were not aware of this most excellent advice.

What I won’t do:
1. change the specs I’m not directly the owner of
2. touch any other branch
3. write or amend any additional document

If anyone blocks the redhat-rpm-config PR or is cheeky enough to complains the result is ugly or not good enough, then I am not up to Fedora packager standards (since apparently Fedora packagers are expected ”manage just fine” with the rpm change). I will therefore apply the other most excellent advice I got here and “revert all that black magic” I contributed to Fedora. I could state how many packages that will break, but that’s “FUD”, so I won’t bother.

Have a nice day.

@nim: Nobody is insinuating that you're not "up to Fedora packager standards", but after spending a couple of hours reviewing everything, I'm a bit disappointed in how argumentative you've been to @pmatilai and @ignatenkobrain. There are much better ways to interact with upstream projects and the way you've approached the whole problem makes people not want to work with you on solving the problem.

@pmatilai: As much as I don't like this, I think this is unfortunately a real quirk we need to have a solution for. It's unfortunate that it wasn't caught before, but having all the Go packages and most of the newer font packages, and everything else using the forge macros break is not good.

@ignatenkobrain: It's not nice to play the blame game. We're all trying our best to support each other in the distribution and make a better platform for everyone. I sympathize with you on the complexity issue. It's somewhat difficult for me to follow a lot of what @nim does. And I agree that the real answer here is to obsolete the complexity by making rpm do better things here, but we have to work with what we have now and work with upstream rpm (@pmatilai and @ffesti) to make things better so the complexity isn't needed.

What we should take away from this is that there's clearly a deficiency in RPM that needs addressing. What exactly that deficiency is should be figured out and planned for fixing. The work @nim is doing is solving very real problems and I'm glad he's working on them. I don't necessarily agree with his approach all the time, but at least he's working on it and making things easier for people.

So please everyone, can we come up with a solution here?

@ngompa, what breakage is that? We've been asking for pointers to failed builds, and the only one we've gotten back is the one on dejavu-fonts where a manual patch used %{name} before it was declared, trivially avoidable.

If there's this huge claimed fire then where the in big F is the smoke? I've broken rawhide big time a few times in my career, it tends to light up koji like the veritable x-mas tree. That I've received zero bug reports on this in the last ten months which span two mass-rebuildstells me that the claimed breakage is vastly exaggerated.

So please everyone, can we come up with a solution here?

To recoup what I said two weeks ago in https://pagure.io/packaging-committee/issue/968#comment-641834:

A concrete suggestion as to what FPC can actually do here, is recommend against using %{name} in patches and sources.

The %{name} macro in spec refers to the package that will be built. If you add -compat to the package name, it does not rename the sources and patches, so using %{name} in them is wrong and always was. It's just this weird habit that multiple generations of packagers (yours truly included) have gotten conditioned to from copying what others did. Renaming the package should not break the build.

After tolerating personal level insinuations for several weeks, that's just about the most productive feedback you're going to get from me right now.

On FPC level, we might also say:

Packagers MUST NOT use RPM macros in spec before they are defined. Declarations of Name, Version (...full list here...) define the %name, %version (...full list here...) macros.

On FPC level, we might also say:

Packagers MUST NOT use RPM macros in spec before they are defined. Declarations of Name, Version (...full list here...) define the %name, %version (...full list here...) macros.

You may as well ask to make %{foo} and %{?foo} synonyms in Fedora rpm. The only sensible reason for %{foo} to evaluate as %{foo} and not nothing when foo is not defined yet is to allow referencing variables before they are defined.

Yes, that’s a core rpm macro language feature (not handwaved rpm lore)

"Use" as such is too vague to be accurate in this context. Consider these two as the first line of a spec:

%define foo %{name}
%global foo %{name}

There's nothing wrong with the first definition in itself, but second is not okay really, because it's not what the user intended - if you want a macro that's expanded at the time of use instead of time of declaration then use a %define not %global. Or write it as %global foo %%{name} which is also fine, but IMO just an unnecessary obfuscation.

Rpm is likely start warning on trying to expand undefined macros at some point because confusion surrounding this is a common source of errors in specs.

Sorry, but that’s not how it is used today and %define would not work for other reasons. I can’t stop you from rewriting history to whatever you feel is right today but I can stop trying to workaround your rewriting in Fedora packages.

@nim So, I have a couple of questions here:

  • Can you produce a minimal test case that proves this issue?
  • Can you provide a couple of Go packages that have failed in Fedora because of this change?

It's not that I don't believe you, but I can't personally find a failure case among the packages I'm aware of that use the forge macros and the revised Go macros built on them.

It would go a long way into helping figure out exactly what to do here...

So I just looked at a dozen or so golang packages. Turns out they are already using a scheme much like I suggested:

%global goipath github/some/path
Version: x.y.z

%gometa

Name:           %{goname}
[...]
URL:            %{gourl}
Source0:        %{gosource}

So they never were affected by this in any way, which also explains why we haven't been drowning in bug reports over this over the past 10 months. In addition in the pattern used by the golang-packages could also use %{name} in Source/Patch without any issues because Name is systematically before those.

@ngompa

Sorry there are no simpler test cases than the ones I provided all along.

sil-mondulkiri-fonts is just two font families. You can not get below two without missing the implications of multiple subpackages.

sil-mondulkiri-extra-fonts is arguably more complex, but the other distribution packages where SRPM name ≠ subpackage name are way more complex than this simple package.

You can remove a few font blocks in sil-mondulkiri-extra-fonts and keep only the first two ones if you feel the rest are noise.
https://copr.fedorainfracloud.org/coprs/build/1353498
That will create something with no Fedora history to compare to.

As for go packages.

At the highest most simplified level the go and fonts packages macro sets use the same basic preamble structure:

  1. define domain specific variables (lots of %global)
  2. define sources/patches
  3. write automated and manual package headers (%fontpkg, %gopkg, etc)
%global fontfoo*
…
SourceX:
PatchY:

#OPTIONAL manual headers in addition to the automated ones
%package/Name foo
…

%fontpkg

Step 3 will generate more variables, be it %{name} via Name: or via automated header creation that, in addition to creating Name:, %package etc lines in the preamble will derive missing variables from the ones provided by the packager, using standard derivation rules.

The derivation was never cleanly separated from variable use (especially for the older Go macro codebase), it was more of a just-in-time “duh, I need foo variable to work, the packager did not set it, what is the sane common-sense value that can be derived from the variables the packager did bother to set”.

Derived variables can be used in Source/Patch like the other variables. Just because the packager does not need them today does not mean he won’t need them tomorrow (as happened with dejavu in the original bug).

The rpm change made the whole preamble structure unsafe. To make is safe without rpm changes, it requires refactoring the domain macros and refactoring the Fedora specs preambles into

  1. NEW define the srpm name without Name: use
    • so it can be reused before the start of package headers
  2. define domain specific variables
  3. NEW force set of all derived variables
  4. define source/patches as before but
    • NEW wrapping them into an expand that can be injected after the Name: line so %{name} is once more safe to use in sources/patches
  5. write automated and manual package headers (%fontpkg, %gopkg, etc) as before but
    • NEW forbidding the derivation of new variables
    • NEW making all of those use the wrapper provided in this PR so it checks header name against the NEW variable set in 1., and injects the NEW Source/Patches expand at this point
# Make the SRPM name available early without triggering package header constrains 
%global source_name something

%global fontfoo*
…

# Compute all derived domain variables
%fontmeta

# Wrap Sources/Patches so they can be resolved in the header matching source_name
%global source_files %{expand:
SourceX:
PatchY:
}

# After this point creating or computing variables that may end up in Sources/Patches
# is now unsafe

#OPTIONAL manual headers in addition to the automated ones,
# using the PR new_package wrapper to inject Source/Patches
# at the only location rpm now supports fully
%new_package foo

# Automated headers that also use the PR new_package wrapper
# to coordinate with other automated header macros
# and manual packager declarations
%fontpkg

You can replace the following lines

%global fontfoo*
%fontmeta
%fontpkg

with

%global gofoo*
%gometa
%gopkg

it works exactly the same from the general structure point of view. Except, obviously, the font macro refactoring is now done, and the go refactoring not. I fully expect it to be fiendishly difficult given the cruft in the go macro codebase. The font macro codebase was brand-new and wart free, and benefited a lot from the lessons learn writing the go macros. The Go macros were not due for full restructure before adding support for Go modules.

And, anyone can check by itself than the new preamble structure is more complex than the previous one, even in this extremely simplified and barebones overview.

As stated all along most of the complexity here (past and present) is due solely to the way rpm is unable to close descriptions without starting a new header block or a new section. Otherwise the whole thing could have been replaced, for a decade at least with

%global fontfoo*
…

#OPTIONAL manual headers in addition to the automated ones
%package/Name foo
…

%fontpkg

%close_description

SourceX:
PatchY:

Which is identical to the first structure, except that Source/patches are now cleanly bellow the things that create the variables used in Source/patches, in natural order, and without a single change in existing macros. (in a real-world implementation %fontpkg would write %close_description lines by default and the packager would not need to set them by hand)

So I just looked at a dozen or so golang packages.

It is good that you finally start looking at real packages but a random sampling will not tell you much. Many (most) Go packages are very simple. However the macro logic was designed to scale up to very complex Go projects such as kubernetes or docker. So, most packages won’t exercise all the macro logic, and the packages that would exercise the full logic are still being very carefully unwrapped. They did exercise it during macro design POC-ing, and before we got several upstreams to fix their code.

Besides there is little subpackaging Go side because we don’t support Go modules yet and subpackaging without upstream go module tooling is a PITA. A lot of the logic laid the ground work for the next module phase.

Like many language ecosystems, you have a hard kernel of horrible to bootstrap packages, a huge mass of simple easy to do in bulk packages, and a small number of ultra complex apps at the apex of the packaging pyramid. We have not built cleanly simple apps like prometheus in Fedora yet, let alone the usual monsters.

And we much prefer getting upstream projects fixed, than fixing things downstream using patches and complex macro logic. However, fixing thing upstream is not a given. The downstream safety belt must be kept in working order.

So here is nothing to do here. There is no massive breakage in Fedora due to this. Any further discussion on this topic will be in #982.

Metadata Update from @ignatenkobrain:
- Issue close_status updated to: invalid
- Issue status updated to: Closed (was: Open)

2 years ago

Login to comment on this ticket.

Metadata