Properly upstream debuginfo enablement #2204

pmatilai · 2022-09-26T11:51:15Z

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.

The text was updated successfully, but these errors were encountered:

ffesti · 2022-09-27T13:52:57Z

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 · 2022-09-28T05:48:58Z

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 · 2022-09-28T14:39:13Z

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 · 2022-09-29T07:17:36Z

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

ffesti · 2022-09-29T07:36:54Z

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 · 2022-09-29T12:24:52Z

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 · 2022-09-29T13:04:56Z

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

ffesti · 2022-09-29T14:52:13Z

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 · 2022-09-30T06:34:53Z

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 · 2024-04-09T09:09:14Z

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

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Replace the logic parts with actual C code where the logic is more straightforward to follow, taking advantage of dynamic spec generation so debuginfo packages are only ever generated during an actual build. There's a crazy amount of details buried in such a tiny piece, commented in the code now. A noteworthy point here is that the presence of the old %install hack now causes an explicit failure, something like: error: line 4: %package debuginfo: package foo-debuginfo already exists error: parsing failed This explicit break is very much intentional as we want distros to remove those %install hacks from their macro files. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

! This removes the %ifnarch noarch check. We need to find a solution for this before merging (or decise it is just an optimization we don't really need) All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Turn this isto a proper macro that drops the package definition into a .SPECPART file. This way debuginfo can be part of the %install script without messing up the parsing. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Replace the logic parts with actual C code where the logic is more straightforward to follow, taking advantage of dynamic spec generation so debuginfo packages are only ever generated during an actual build. There's a crazy amount of details buried in such a tiny piece, commented in the code now. A noteworthy point here is that the presence of the old %install hack now causes an explicit failure, something like: error: line 4: %package debuginfo: package foo-debuginfo already exists error: parsing failed This explicit break is very much intentional as we want distros to remove those %install hacks from their macro files. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Turn this isto a proper macro in __spec_install_template that drops the package definition into a .SPECPART file. This way debuginfo can be part of the %install script without messing up the parsing. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Co-authored-by: Florian Festi <ffesti@redhat.com> Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. Note that the old %install hack MUST BE REMOVED from distros now. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Co-authored-by: Florian Festi <ffesti@redhat.com> Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. Note that the old %install hack MUST BE REMOVED from distros now. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Co-authored-by: Florian Festi <ffesti@redhat.com> Fixes: #2204 Fixes: #1878

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. Note that the old %install hack MUST BE REMOVED from distros now. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Co-authored-by: Florian Festi <ffesti@redhat.com> Fixes: rpm-software-management#2204 Fixes: rpm-software-management#1878 (cherry picked from commit 8535694)

All these years, enabling debuginfo has required distros to hijack the spec %install section with a macro like this: %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\ %%install\ %{nil} This for a widely used, longtime upstream supported feature is just gross, and also very non-obvious, feeble and whatnot. And totally prevents the new append/prepend options from being used with %install. Take advantage of several newish features to make this happen: we need expressions to properly handle the numeric %_enable_debug_packages value from a macro, and if enabled, output the debuginfo template as a dynamic .specpart. Enable debuginfo on Linux by default in the platform configuration. Notably noarch should not have debuginfo so it's disabled in the platform configuration - since 96467dc we can now actually rely on the platform configuration being valid, so we can drop the "%ifnarch noarch" from the debug package check. Further streamlining should be possible. Note that the old %install hack MUST BE REMOVED from distros now. As a nice bonus, this makes debuginfo work for packages that don't use %setup. Add an explicit test for this in the "rpmbuild %caps" test. specstep.spec needs to be made noarch here, otherwise it'll now try to produce debuginfo and fail. Co-authored-by: Florian Festi <ffesti@redhat.com> Fixes: #2204 Fixes: #1878 (cherry picked from commit 8535694)

The %install override is no longer needed or compatible with 4.20.0 (4.19.91 being the latest pre-release snapshot in Rawhide) now that a proper mechanism is implemented in RPM itself that that takes care of debuginfo enablement, see: - rpm-software-management/rpm#2204 - https://rpm.org/wiki/Releases/4.20.0

The FIPS build runs *_install_post commands early during %install so that the binaries will not be modified after running fipshmac, since those commands are supposed to be no-op if re-run. However, __debug_install_post is only run if __debug_package is defined, which is triggered by the automatic creation of the debuginfo subpackage where appropriate. Previously, a hack in redhat-rpm-config caused this to be enabled by %install, but with RPM 4.20 this is no longer needed, and the hack was removed from redhat-rpm-config for F41. On Fedora builds, %mingw_debug_package triggers this and therefore it still builds, but ELN is build without mingw and therefore there now is nothing to trigger the debuginfo generation during %install. As a result, the binaries would just be stripped without any debuginfo generation during the first run, leaving nothing to detect in the second run, and the build would fail for lack of debug symbols/sources. rpm-software-management/rpm#2204 https://src.fedoraproject.org/rpms/redhat-rpm-config/c/7a1571ee808ba13b129eab7a7ed3869e77740c3e

pmatilai added the RFE label Sep 26, 2022

pmatilai added this to RPM Sep 26, 2022

pmatilai moved this to Backlog in RPM Sep 26, 2022

pmatilai mentioned this issue Sep 26, 2022

The debuginfo machinery breaks if %install follows directly after %prep or preamble #1870

Closed

ffesti moved this from Backlog to In Progress in RPM Oct 4, 2022

ffesti self-assigned this Oct 4, 2022

pmatilai moved this from In Progress to Todo in RPM May 5, 2023

pmatilai added the debuginfo label Sep 14, 2023

ffesti removed their assignment Oct 10, 2023

pmatilai mentioned this issue Apr 9, 2024

%build -a and %install -a overwrite build/installation instructions from %buildsystem_*_* #3024

Closed

pmatilai moved this from Todo to Priority in RPM Apr 9, 2024

pmatilai mentioned this issue Apr 9, 2024

rpmspec does not expect debuginfo rpm for a subpackage #1878

Closed

pmatilai added this to the 4.20.0 milestone Apr 11, 2024

pmatilai moved this from Priority to In Progress in RPM Apr 15, 2024

pmatilai self-assigned this Apr 15, 2024

pmatilai mentioned this issue Apr 15, 2024

Add proper program logic for debuginfo enablement #3036

Closed

pmatilai moved this from In Progress to In Review in RPM Apr 15, 2024

ffesti mentioned this issue Apr 16, 2024

Upstream debuginfo enablement #3040

Closed

pmatilai mentioned this issue Apr 17, 2024

debuginfo generation does not work with --build-in-place #3042

Closed

pmatilai mentioned this issue May 10, 2024

Add proper logic for debuginfo enablement #3085

Merged

ffesti closed this as completed in #3085 May 13, 2024

github-project-automation bot moved this from In Review to Done in RPM May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Properly upstream debuginfo enablement #2204

Properly upstream debuginfo enablement #2204

pmatilai commented Sep 26, 2022

ffesti commented Sep 27, 2022

pmatilai commented Sep 28, 2022 •

edited

Loading

ffesti commented Sep 28, 2022 •

edited

Loading

pmatilai commented Sep 29, 2022

ffesti commented Sep 29, 2022

ffesti commented Sep 29, 2022

pmatilai commented Sep 29, 2022

ffesti commented Sep 29, 2022

pmatilai commented Sep 30, 2022 •

edited

Loading

pmatilai commented Apr 9, 2024 •

edited

Loading

Properly upstream debuginfo enablement #2204

Properly upstream debuginfo enablement #2204

Comments

pmatilai commented Sep 26, 2022

ffesti commented Sep 27, 2022

pmatilai commented Sep 28, 2022 • edited Loading

ffesti commented Sep 28, 2022 • edited Loading

pmatilai commented Sep 29, 2022

ffesti commented Sep 29, 2022

ffesti commented Sep 29, 2022

pmatilai commented Sep 29, 2022

ffesti commented Sep 29, 2022

pmatilai commented Sep 30, 2022 • edited Loading

pmatilai commented Apr 9, 2024 • edited Loading

pmatilai commented Sep 28, 2022 •

edited

Loading

ffesti commented Sep 28, 2022 •

edited

Loading

pmatilai commented Sep 30, 2022 •

edited

Loading

pmatilai commented Apr 9, 2024 •

edited

Loading