#2440 F33 System-Wide Change: Patches in Forge macros - Auto macros - Detached rpm changelogs
Opened a month ago by bcotton. Modified 12 hours ago

redhat-rpm-config will be updated to add patching support to forge macros, a plug-able framework to register macros to execute in specific sections, and rpm changelogs in detached files.


:sob: for no discussion in devel@. Either it means everything is cool or nobody understood the change. :broken_heart:

It seems the change is bundling too many things at once.

  • Patches in forge macros sounds like a backwards-compatible, isn't it? If not, what are the incompatible changes, how many packages are affected, who will do the migration?
  • Auto macros seems like the main change
  • Detached rpm changelogs seems like a thing we've been tossing over devel@ few months ago

I would prefer to go step by step and not do everything at once. I'm -1 on the detached rpm changelogs.


The change page talks a lot about framework for bundling multiple macro like Go, Fonts and other "buildsystems" things. However, it does not explain how one would be adding support for Python, Rust, Nodejs, Ruby and other languages. Whether it is coming up in the next release or some plans. If we are calling it generic framework, I think we need to have clear vision how we are going to implement support for other languages and have change owners working on that and not hoping that somebody else will step up and do the work. Having some generic framework that implements only 2 out of 15 ecosystems does not sound very useful.

I'd also like to hear opinion of @ffesti (upstream RPM developer and maintainer in Fedora/RHEL) on this change.

Also, the contingency plan is not really good. If the change is approved and starts to be used and then we figure out that something is utterly broken, change owners are not going to do anything... Which is wrong.


I'm +-0 now, but I might forget to put -1 and the change could get approved automatically so count me -1 at this moment.enough votes during the week.

The main change in there is complete refactoring of the lua library, to enable current library users (forge, fonts and go macros) to generate a preamble that follows strict ordering conditions, following interactions with rpm maintainers on Tag ordering in spring.

To achieve strict ordering, similar routines, that had grown organically in slightly different forms in the forge, go and fonts macros (sometimes in slightly different forms in different macros of the same macro set) were replaced by a single canonical version with a single ordering behaviour. That part would have probably shrank the total line number if the functional perimeter had stayed the same, and if there had been no backwards compatibility constrains (If you consider the 3 macro packages as a whole, and not in isolation. Obviously, the effect is less strong in redhat-rpm-config since it gets to keep the canonical version of the code).

Things that depended on the packager remembering to call things in a specific order, were replaced by code that enforces the canonical ordering (freeing the packager to care about more interesting things).

Generic canonical code that is driven by %global control variables requires generic naming for those control variables. A lot of the new code deals with reading and setting %global variables and exchanging those variables between subsystems that need to coordinate their ordering (that was already the case, to a lesser extent, of the previous version of the library code).

Porting a macro set to the framework, will generally require renaming variables that do not follow the strict naming assumed by the implementation.

Lots of limit conditions are now handled, and the corresponding bugs now fixed, which tends to increase the codebase size since it now checks things it would have exploded on before.

Everything else in the change is either a consequence of this revamping of the lua library or something that was easy to implement once the revamping was done.

The functional changes people care about, are just the outer visible layer, of something which is mostly low-level code refactoring and ironing out. Some of the functional changes were added first and foremost to check the underlying mechanisms were robust and generic.

The auto macro is the user visible part of those ordering changes, it’s not the main part of this change in code lines. There was no reason to limit its use to the preamble because it works as fine in every part of the spec. The preamble is actually the harder section to orchestrate due to interactions between srpm and subpackage metadata. The auto calls in other sections of the spec file use a simplified version of the pramble orchestrator.

Most of the changes in forge macros are due to porting to this infrastructure, and fixing old bugs identified while porting, and trying to see how far behaviour could be aligned between different spec generation macro sets (turns out, quite far, but not as far as some may wish).

Another big part of the forge changes is a complete compatibility subsystem, that translates packager-provided %global variables that use the old naming, to the new naming, and translates the result of forge processing that uses the new naming, to old naming.

Therefore the forge change should be backwards compatible at the spec user level (obviously, the lua backend changed a lot). And the compatibility layer has a cost in complexity, and is something that we will want to rid of in a few releases.

A little bit adds patch handling (because I wanted to add it for a long time, because it was easy to add once the other changes were done, because I did not want to inflict on my users a change with no functional gains except keeping newer versions of the rpm parser happy, and because I was sick to death of testing changes that gave me no direct benefit).

The detached changelog part is something that I added the last week-end, after I realised that in four months of refactoring and cleaning up I had remade my specs from top to %changelog line, and why the hell did I make everything else nice and clean, just to stop before the changelog wart. In level of complexity and risk it is quite trivial compared to the rest. The risky bumping parts were split in a separate F34 change the day after the week-end.

The change adds a framework, which is robust enough to handle almost two handfuls of spec generation subsystems, with wildly differing functional needs, with bits of generation in almost every existing section of the spec file (I say almost, because none of those subsystems needed pre/post trans, or triggers, or this kind of thing).

Therefore I consider it good enough to support other domain macro sets should anyone else wish to reuse it. Obviously, new users will state the implementation almost does everything they want, but could it be extended in X or Y direction, and someone that cares about X or Y will have to do the extending, and you can always add another level of refactoring, because code is never clean enough.

I do not intend, to write other domain macro sets, anymore than the rpm maintainers intended to write all the generate_buildrequires macros out there once they added the generate_buildrequires infrastructure.

If, someone decides to declare the change is utterly broken, he can deal with the consequences himself. Declaring gnome 3 was utterly broken did not cause gnome developpers to dump months of work and restart at the gnome 2 point.

I feel completely ill-equipped to make a decision here. I don't fully understand what's going on here and I'm having trouble wrapping my brain around it.

Unfortunately, this means I'm -1 until I understand this.

BTW. This not complex code. However it is also not a 10-liner macro. To understand it you need to look at the copr build logs and take a peek at how control loops are agenced in the code (and going from one to the other is easy because the change includes recording in build logs what is happening).

It’s not your typical silent black box macro.

Metadata Update from @zbyszek:
- Issue tagged with: meeting

22 days ago

After reading the patch, I finally understand what this is about. The changes in the lua code look OK. I'd like to approve this and let the implementation move forward, but there are open process-related questions:

  1. An FPC ticket was opened, but it doesn't say what needs to change in the guidelines. The Change page mentions in passing that some macros were renamed. What are the guidelines changes required? Who will do it?

  2. Backwards compatibility. The Change page says:

the way current forge macros call forge macros will need a little patching once the change lands. For other packagers, there should be no change except a warning in rpm build logs to switch to the new syntax before the compatibility layer is removed.

Is there full backwards compatibility, i.e. will current spec files continue to work? How long will the compatibility layer be maintained? What does "little changes" actually mean?

IIUC, other packagers are supposed to fix their packages. How many packages are impacted? What happens when those packages are not fixed?

  1. The Contingency Plan

    There is no contingency plan because the redhat-rpm-config merge will happen or not.

As @ignatenkobrain said above, this is not a contingency plan. We need to know how to keep packages building if the change is aborted or stopped or stuck at any point in time.

@zbyszek

Thanks a lot for reviewing the actual code. Please don’t hesitate asking questions and pointing changes if you see something wrong. I tried to be clean, but the change suffers from a long gestation without external feedback.

An FPC ticket was opened, but it doesn't say what needs to change in the guidelines. The Change page mentions in passing that some macros were renamed. What are the guidelines changes required? Who will do it?

The concerned guidelines are: forge, fonts and Go macros. I will do the guidelines PRs (though their reviewing falls on FPC of course).

Forge and Fonts are mostly preamble %global variable renamings (%{forgeurl}%{forge_url}) and removal of all the post-preamble peculiarities.

The %auto_foo system removes the need for a lot of per-macro-set documentation after the preamble, which was one of the main reasons I added it. I’m sick of maintaining guidelines for macros sets that behave almost the same but not exactly, now they all use a common framework that only needs documentation once.

i18n (which is a stackeholder fonts-side) already told me they wanted the common redhat-rpm-config core in before working on fonts changes ( @petersen @tagoh @pnemade ) . The copr already contains 90% of the font packages that would be directly affected fonts side, and it emerged this spring that current guidelines are unsafe due to changes in the rpm 4.15 parser. So, current guidelines would need changing with or without this change.

The Go part is a lot more complex because there are interactions with moving Fedora to upstream’s new "module" component system.

Right now my plan is to plug existing Go macros to the new redhat-rpm-config with minimal changes, and propose for F34 a new module-oriented macro set that will fully use the new lua code. So you’ll have for a while two Go macros sets: a legacy one, that will work with un-converted Fedora spec files, and will make little use of this change, and limited to GOPATH subpackages. And a new one, that will produce GOPATH and Go module subpackages, and will use this change extensively (and maybe we’ll get rid of the GOPATH part if by F34 everyone is ready to use Go modules).

Keeping the old Go macros as-is without external changes is key in this strategy. One of the side effects of the new code is forcing rigid regular naming (the new generalized lua routines rely on naming regularity). Turns out, that has the benefit of avoiding naming collisions between old and new macro sets ( @eclipseo @qulogic ).

The new Go macro set is 80% finished.
https://pagure.io/fork/nim/go-rpm-macros/c/e876e2483cbe4443f4d7c18afad6227f4dd7c75c

827 lines added, 2522 lines removed → big simplification of macro-level code, while doing a lot more, thanks to better redhat-rpm-config support, better upstream tooling, and moving go module specific processing to a dedicated binary. The real change footprint will be more, since a lot of the removals are old documentation not replaced with new documentation yet.

A huge amount of the changes in redhat-rpm-config are generalizations of code I wrote initially to support Go modules, once I realized fonts and forge macros where doing the same kind of processing in a more limited and less robust way. fonts and forge did not absolutely need some of the enhancements in there, juggling two deployment formats (and associated subpackages) Go side forced me to invest in those.

So for Go, guidelines changes will be deferred for F34, which is not a problem because I can get existing macros to use the new redhat-rpm-config code without packager-visible changes.

Is there full backwards compatibility, i.e. will current spec files continue to work?

Backwards compatibility testing is green for specs that use current forge macros (I did not even converts the fonts macro package to the new forge syntax, and you can see it building in the copr). Depending on when things happen I will try to do morge forge checking, though IRL constrains mean I won’t do much in August.

I need to check the way old Go macros plug in the code (there are some direct calls to lua functions in there). But not difficult to do and the average packager does not call the lua API directly. Just, do not merge the redhat-rpm-config code without checking with me the Go part is ready :). I did not want to maintain a low-level lua compatibility layer for one user, especially when this sole user is me.

How long will the compatibility layer be maintained?

Al long as people wish. I’d love to remove it by F35, because it’s a lot of code in redhat-rpm-config just to avoid people renaming their macro calls, but if people want it kept longer… The compatibility layer is composed of all the *forge_deprecated* files in
https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/95

As @ignatenkobrain said above, this is not a contingency plan. We need to know how to keep packages building if the change is aborted or stopped or stuck at any point in time.

The Change is one PR with one commit. It can not be "half merged" unless people go out of their way to do weird things.

If there are bugs (this is software after all) they will need fixing. If the forge compatibility layer proves insufficient in some specs, converting those specs to the new syntax will probably be simpler than adding more compat code.

Because the change hides all the post-preamble logic behind generic macro calls (%auto_prep in %prep, %auto_build in %build, etc) it shrinks the macro envelope exposed to packagers. Meaning that once it is adopted in spec files, ulterior code changes won’t require notifying packagers and reworking packaging guidelines.

I’m not 100% satisfied with the naming logic in the preamble, but I’ve been asking rpm maintainers for years to work on something cleaner here, and they did not want to look at the problem. That’s the best I could come up with given current rpm state, and the best is the enemy of good. @ffesti may have an opinion on this part but he has not shared it with me.

AGREED: Ask @nim to clarify the proposal on those questions, revisit when we have the answers (+6, 1, -0)

From the meeting:
King_InuYasha> I want to know 1) what it does 2) the impact 3) adoption effort required 4) backwards-compatibility guarantees (if any)

Metadata Update from @ignatenkobrain:
- Issue untagged with: meeting
- Issue assigned to nim

13 days ago

Metadata Update from @ignatenkobrain:
- Issue marked as blocking: #2441

13 days ago

We haven't heard from the Change owner and no other comments have been made in ticket, so I think there's no reason to put it on the meeting schedule for tomorrow. Waiting for further input.

I deliberately refrained from answering at once, to keep the conversation cool and calm. Anyway:

  1. what it does: there are already pages of explanations here, in the wiki, and in the commit message. Please ask specific questions about what you do not understand in those pages

  2. the impact: new packaging automation that enables the i18n, fonts and Go SIGs to go forward, automating packaging 1000s of Fedora subpackages, and reducing spec maintenance. See the posted copr for real-world examples.

  3. adoption effort: see the posted copr for real-world examples. Limited to declaring a list of %global control variables and a single shared one-liner entry point in each rpm section that hosts some of this processing (basically, all of them). After the addition of the generic entry points the spec footprint of new processing is limited to the global variables controlling this processing. This is similar to how existing debuginfo subpackages work except the new system is generic and enables coding new processing at any point of the build process without spec changes (within the limitations of rpm lua evaluation).

  4. backwards compatibility:

    • yes for forge macros (see the fonts-rpm-macros build that uses forge compatibility mode),
    • no for fonts macros — affected packages are almost all converted in the example copr, handful of remaining packages to be converted once the patch is merged. Fonts macro have been broken by rpm parser 4.15 backwards-incompatible changes and can not be rescued in a backwards-compatible way.
    • yes for go macro (probably with some lua changes), but those will have to be redone to add support for the new upstream Go module system, and this rework requires this change to go forward
    • technically only the forge macros are included in redhat-rpm-config and part of this change
    • for more compatibility information : read the previous messages as they already discuss the implementation in depth

Alright, at this point...

+1

OK, I think we shouldn't delay this any longer. I don't fully understand all the details of the change, but it seems like it should go reasonably smoothly for F33.

+1

There is a lot going on in this proposal and it is not clear that it is all required at once. I would prefer to see this broken out in to more than one proposal with a distinct workflow change in each so it is easier to understand and builds on the previous change.

Some real examples would make this a lot easier to understand. Simple examples.

Regarding the patches part... what is that even for? Is that for specifying a git commit on an upstream repo and having that apply at build time? Some specific examples here would be helpful. Is patch application order implied by file order or can it be specified?

The title says "Patches in Forge macros - Auto macros - Detached rpm changelogs" but then when it talks about the RPM changelog, it just says it's an external text file. So is there anything here specific to generation of that file? It's not clear.

I am in favor of process improvements and making packaging things from git repos easier, but am -1 on this proposal as written. Needs to be broken in to multiple proposals with clear examples.

@dcantrell

I would prefer to see this broken out

This is not a collection of unrelated code that could be broken into different proposals. It is a trunk of common routines with no direct functional application, and a thin layer of functional applications built over this common trunk. Because the main objective was to reduce the maintenance burden of coding the same things multiple times in slightly different ways in multiple macro sets, the functional layer is as thin as possible.

It was done and is submitted this way for the same reason kernel maintainers refuse to merge drivers that come without some free software userspace: you need a minimum of things that use your plumbing, to check the plumbing is sane.

While the change is new it started as refactoring, generalization and merging of several macro sets that were written over years and added over multiple Fedora releases. That makes it good for maintenance, but hard for reviewers that ignored those previous steps, since multiple refactoring passes result in higher level more abstract code.

The change comes with a copr of simple font packages. They’re as simple as those things can be because most of them do not involve building files with complex project-specific build systems, just copying files in the correct place for fontconfig to index. (The application of the change to Go packages will be a lot more difficult to understand for someone not familiar with the Go toolchain, even if it will reuse the exact same common trunk of macro routines).

Regarding the patches part... what is that even for?

The patch part is just the integration to patch application to the forge equivalent of autosetup.

It enables attaching a patch list to a forge source. Patches are applied in the order they appear in the list. The exact (not simplified pseudo code) packager-level syntax is given in
https://fedoraproject.org/wiki/Changes/Patches_in_Forge_macros_-_Auto_macros_-_Detached_rpm_changelogs#Forge

Unlike autosetup and other previous attempts patch application is multi-source safe without requiring arcane numerology or long lines of brain-damaging macro arguments. Patch application just works with 1 2 3 or N forge sources, and 1 2 3 or N patches per source

Like autosetup it relies on apply_patch, so all the weird things apply_patch knows about are available to the packager.

It is applied in a real build in
https://copr.fedorainfracloud.org/coprs/nim/refactoring-forge-patches-auto-call-changelog-fonts/build/1517775/

(just grep for patch in the spec file). Incidently, this build also marks the clean resolution of the rpm 4.15 parser change that broke this exact package in spring.

when it talks about the RPM changelog, it just says it's an external text file. So is there anything here specific to generation of that file? It's not clear.

As explained in https://fedoraproject.org/wiki/Changes/Patches_in_Forge_macros_-_Auto_macros_-_Detached_rpm_changelogs#Feedback

How will the changelog be maintained?
The changelog will be maintained any way you wish to maintain it, it’s just a plain text file in package sources.

You won’t find anything about changelog generation because the change does nothing to generate the changelog. It’s not the purpose of a build process to generate changelogs. The follow-up F34 change adds changelog bumping, but again this has nothing to do with generating changelogs, this is just adding the build timestamp to a pre-existing changelog.

The changelog part in this change splits the changelog from the spec, nothing more and nothing less. That a split file is loads easier to manage and generate in higher-level tools is a benefit. But this generation does not belong in rpm macros. I firmly believe that tying rpm to a specific Fedora build infra is a short term optimization that would not end well.

Again, you have almost 100 packages in the copr that show how the detached changelog is integrated and you can trivially check no generation occurs in there.

While forge patches, detached changelogs, etc are different functional applications, they all depend on the auto framework to work, and both this framework and the functional application routines make lavish use of the common macro routines in the lua library.

To be clear, I have no problem with the framework per se, and provided @nim has a plan to fix every single package that breaks with these changes, I'm okay to allow this.

However, I do not endorse the detached changelog idea or implementation and do not wish for it to be used. So I am -1 for this detached changelog functionality.

@ngompa out of curiosity, why?

  • There are enough people that proposed the detached changelog part to show there is strong interest for this function in Fedora.
  • The low level implementation is almost identical to the way others coded it (not because we stole one another’s code, but because how to code it is fairly obvious when you look at the problem).
  • The higher level integration is different, but that’s because the rest of the change provided simple ways to integrate processing at different parts of the spec without requiring packager boilerplate.
  • This change does not include any of the things people do not agree on and fight over (generation and generation policy is out of scope).
  • rpm upstream already refused, or delayed indefinitely implementation it in rpm itself, arguing other distro managed to do it without requiring any rpm effort.

So, why? Are you opposed to detached changelogs in general? If you are not opposed in general, what exactly in this implementation does not sit well with you?

Are you opposed to detached changelogs in general?

I am generally opposed to detached changelogs. My experience with managing them with Debian packaging and SUSE RPM packaging shows that it doesn't really solve the problems people think they would solve, and they add complexity where it isn't worth it. It also eliminates the value of a singular build control file (the spec file) that contains all the necessary information to build a package, for no particularly advantageous reason.

My questions have been addressed in the 05-Aug-2020 FESCo meeting.

I am +1 on this proposal with the exception of detached changelogs. I do not support including the detached changelogs changes in this proposal.

@nim have you tested the backward compatibility of Golang macro changes? If the changes are added to redhat-rpm-config, will my current specs work without any changes?

Login to comment on this ticket.