Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for generating debuginfo RPMs #842

Merged
merged 13 commits into from
Apr 24, 2024
Merged

Conversation

kellyma2
Copy link
Contributor

This change enables the creation and capture of debuginfo RPMs on
Fedora40 and CentOS7.

See:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/

Fedora 40 expects the RPM contents to be located in a subdirectory
which is specified using the buildsubdir variable. In order to
account for this, we need to tweak some of the location details if
debuginfo is enabled.

CentOS expects buildsubdir to have a value like . instead.

In both cases, we disable debugsource packages by ensuring that we
undefine _debugsource_packages, otherwise we'll try to generate them
alongside the debuginfo packages and will fail.

We only want this method of producing debuginfo to apply when we're
using the system rpmbuild because the underlying behaviour is
controlled by a combination of the rpmbuild version, macro
definitions, find-debuginfo.sh, and debugedit. If we were to expand
this to use a hermetic debuginfo then a different approach might be
desirable.

This change enables the creation and capture of debuginfo RPMs on
Fedora40 and CentOS7.

See:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/

Fedora 40 expects the RPM contents to be located in a subdirectory
which is specified using the `buildsubdir` variable.  In order to
account for this, we need to tweak some of the location details if
debuginfo is enabled.

CentOS expects `buildsubdir` to have a value like `.` instead.

In both cases, we disable debugsource packages by ensuring that we
undefine `_debugsource_packages`, otherwise we'll try to generate them
alongside the debuginfo packages and will fail.

We only want this method of producing debuginfo to apply when we're
using the system `rpmbuild` because the underlying behaviour is
controlled by a combination of the rpmbuild version, macro
definitions, find-debuginfo.sh, and debugedit.  If we were to expand
this to use a hermetic debuginfo then a different approach might be
desirable.
This provides a basic example that generates a debuginfo RPM
configured to run on CentOS7.
rules_python seems to fail us when we're generating debuginfo RPMs
unless we upgrade to a version more recent than 0.24.0.
In lieu of enabling this behaviour by default on the supported
platforms, we add an additional argument to the pkg_rpm() rule that
will allow us to enable it for pkg_rpm() targets.  This prevents us
from enabling it in cases where it's not desired.
This test is modelled on the subrpm test.  In lieu of using a simple
text file as an input it instead generates a binary that includes
debug symbosl from a C source file and includes that in the RPM.

The baseline comparison strips out the `.build-id` paths because the
hashes that are generated may not be stable.x
These values may vary depending on the platform that this is being run
on and we don't really care about them.
Copy link
Collaborator

@cgrindel cgrindel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. I just had a question about the rules_python version bump.

MODULE.bazel Show resolved Hide resolved
@aiuto
Copy link
Collaborator

aiuto commented Mar 26, 2024

I'm going to be slow to respond. Every hour of this week is booked.

@kellyma2
Copy link
Contributor Author

I'm going to be slow to respond. Every hour of this week is booked.

Thanks for the heads up

@kellyma2
Copy link
Contributor Author

kellyma2 commented Apr 3, 2024

@aiuto any cycles to go over this in the near term?

pkg/rpm_pfg.bzl Outdated
doc = """Enable generation of debuginfo RPMs

For supported platforms this will enable the generation of debuginfo RPMs adjacent
to the regular RPMs. Currently this is supported by Fedora 40 and CentOS7
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. at the end of the docstring.

Copy link
Collaborator

@aiuto aiuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an overall question.
I was curious why you passed debuginfo_type via the toolchain, instead of just getting the os type in make_rpm.py. Then I saw you need to change the stanza to add a .. in a path. sigh.

But it leads to the question of the name of debuginfo_type. Is it really distinguishing the format of the debuginfo, or is it really a distinguisher of the target os?
If you could build for fedora from centos, should it be an attribute of the rule rather than picked up as a side effect in the toolchain?

Are there other constraints or information about cross building which should make sense in the docstring?

overall the code looks reasonable.

@kellyma2
Copy link
Contributor Author

But it leads to the question of the name of debuginfo_type. Is it really distinguishing the format of the debuginfo, or is it really a distinguisher of the target os? If you could build for fedora from centos, should it be an attribute of the rule rather than picked up as a side effect in the toolchain?

Are there other constraints or information about cross building which should make sense in the docstring?

tl;dr I don't think I can assert anything about cross building with this at all, unfortunately :(

We're relying on the system rpmbuild for this which means that we're also relying on the various rpm macros and and supporting tools in place that belong to the system that we're building on. debuginfo_type is a bit of a misnomer here, perhaps, but what it's referring to is the OS configuration for the aforementioned version of rpmbuild (incl macros, etc) not the target OS. Initially I had allowed this to be selected in the module extension, but decided I just wanted to make this auto-detect so that we could at least hypothetically have the defined rules be system independant.

Can definitely clarify some of how this works in the docstring, however.

Copy link
Collaborator

@aiuto aiuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just fix that period in the docstring and I'll merge.

@kellyma2
Copy link
Contributor Author

Can you just fix that period in the docstring and I'll merge.

Done

CentOS Stream 9 appears to work more or less the same for debuginfo
generation as CentOS 7.  `os-release` describes it as os == `centos`
and version == `9`.  This change creates an extra token for `centos9`
and sticks it in the places where we currently have controls for
`centos7`.
@aiuto
Copy link
Collaborator

aiuto commented Apr 24, 2024

Sorry for the late merge. Hugely busy weekend.

@aiuto aiuto merged commit 581a86a into bazelbuild:main Apr 24, 2024
2 checks passed
@kellyma2 kellyma2 deleted the debuginfo branch June 28, 2024 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants