rpm-software-management / rpm

The RPM package manager
http://rpm.org
Other
497 stars 359 forks source link

Properly upstream debuginfo enablement #2204

Closed pmatilai closed 4 months ago

pmatilai commented 1 year ago

While looking at something else, I discovered that the debuginfo machinery is still hooked on to this ugly hack of overloading %install with a macro:

%install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\
%%install\
%{nil}

I'm sure this was initially intended as just a quick hack until a better mechanism is devised, but here we are 20 years later and anybody wanting to use debuginfo packages needs to do this in their distro config. I thought this was taken care of in the great debuginfo revamp a few years ago but apparently not, the above magic is just copied to the test-suite where testing debuginfo is needed.

It's about time we upstream a proper integration for this, really. One that doesn't involve macro overload hacks.

ffesti commented 1 year ago

The issue here is that we actually want some macros in the SPEC file. I wonder if we could just add %__spec_pre and %__spec_post macros that are expanded at the beginning and the end of the spec file. We have something similar for the build scripts already. This would give rpm itself but also distributions a way to inject something into (every) SPEC file.

pmatilai commented 1 year ago

Perhaps, but for debuginfo packages (ie the case of this ticket) we want to move the logic to C. That'll enable making build-time decisions about needing debuginfo packages in the first place, something which is impossible with the current macro based hacks at spec parse time.

ffesti commented 1 year ago

OK, I guess I start with writing down how debuginfo works right now:

There are quite a few macros that control and implement the debuginfo creation.

The macro starting the show is not even in RPM but added by the distributions in their own macros. Z.B. in Fedora. This hack prepends the first piece before %install (creating all kind of problems wrt SPEC order and parsing %prep and %install back to back).

If _enable_debug_packages and buildsubdir are set and this is not an noarch package debuginfos are created. Right there __debug_package is set which is used by the other pieces to activate. Also the main -debuginfo package and - if enabled - -debugsource package is declared there.

Next stop is __spec_install_post which executes find_debuginfo (which is now part of the debugedit package) at the end of %install. This creates the manifest file for the main -debuginfo package and possibly -debugsource package. To confuse developers __spec_install_post is defined both in macros.in and in platform.in

Everything else is handled in build/files. during the assembling of the packages:

generateBuildIDs adds the links to the packages.

Depending on the settings the debuginfo files are processed and relocated if necessary. It does so by finding the previously declared -debuginfo and -debugsource packages now having read in their manifest files. Other debuginfo packages are created by cloning the main debuginfo package in filterDebuginfoPackage

pmatilai commented 1 year ago

FWIW, what I specifically meant by this ticket is the %install override. That has to go. The other bits are far less offensive.

ffesti commented 1 year ago

Yeah, but that is the hard part - especially if we want to keep the debuginfo packages as a macro template. Ofc we could just create those packages in C. But parsing the template macros after build would be closer to the current implementation. With the current parser this is a hard ting to do. But the Dynamic Spec feature has patches to make that work.

In addition the %__spec_install_post part relies on the decision made by the %install part. So removing the later will require a new solution for the former. May be looking at the macros and arch is enough and other decisions can be based on whether debuginfo files were created.

ffesti commented 1 year ago

Well, guess the easiest way to do this is to get #1485 merged and use that. All it takes it looking at the results of the find_debuginfo run and write %{_debuginfo_template} and %{_debugsource_template} into a file. That's kinda what it was designed for...

pmatilai commented 1 year ago

That's one possibility. But we don't need to inject macro templates into the parsed spec to create packages.

ffesti commented 1 year ago

Yes, but creating them from C seems like a step in the wrong direction especially when hard coding the details like Summary and Description.

pmatilai commented 1 year ago

And I didn't say those should or need to be hardcoded in C. Just that it doesn't need the kind of templates we have now, those are pretty rigid too.

pmatilai commented 5 months ago

This beauty blocks %install -a/-p usage so it just went to the head of the priority queue.