Sub RPM & Debuginfo Support

[kellyma2]

I wanted to add support for two things that appear to be currently missing in the RPM packaging rules: (1) support for building Sub RPMs, (2) support for letting rpmbuild auto-generate debuginfo RPMs. I've hacked out a working PoC for both (the latter depending on the former) but wanted to get some feedback on the approach I've taken before opening a PR.

For (1) the reasoning is that it avoids multiple invocations of rpmbuild and some of the configuration (arch, version, release, etc) doesn't need to be duplicated between rules. Plus, the debuginfo RPM that is generated will cover all of the packages. The rough approach is to add add pkg_sub_rpm rule that has approximately the same form as pkg_rpm but returns a provider that is specific to sub RPMs. From there, pkg_rpm gets an extra attribute that is a list of subpackages to include.

For (2) the reasoning is that the automated rpmbuild debuginfo generation has a lot of magic associated with it and it spans all of the subpackages, so it seems better to leave that magic there instead of asking folks to hack up their own variation with bazel rules. Internally, this requires some tweaks and some explanation (eg: the internal rpmbuild magic expects to find foo.debug for the binaries instead of just foo) but other than fiddling a few knobs just leans on the structures I set up for (1)

Questions:

• does this functionality seem desirable?
• any opinions on the approach?
• anything I'm missing?

[kellyma2]

@aiuto @cgrindel @nacl

[cgrindel]

This sounds reasonable to me. For my clarification, is the pkg_sub_rpm just collecting information that is passed to a parent rule that will then execute the rpmbuild?

[kellyma2]

s/additional/initial/ coffee not yet set in :P

[cgrindel]

I do not have a strong opinion. I would expect the pkg_sub_rpm to build the inner RPM. Either way, each RPM should be built with a separate action. This will help with Bazel caching, and it could help reduce the amount of work if a change only affects a particular sub-RPM.

[kellyma2]

The individual RPMs get captured as separate outputs but the entire set needs to get regenerated whenever rpmbuild is run, unfortunately.

Part of the issue here is debuginfo - specifically, the fact that it is generated implicitly by rpmbuild and it is meant to cover all of the RPMs produced by a given specfile. The process involves internally invoking a substantial (>500 LoC) bash script named find-debuginfo living in the debugedit package that digests the binaries provided as inputs and spits out a Foo-debuginfo.rpm. I'm loath to duplicate this because it would more or less just track debugedit and the surrounding RPM macros anyway. This might be something that can be improved upon though if we want to have better caching here.

tl;dr I'm trying avoid duplicating rpmbuild behaviour instead of just driving it, but it might make the caching worse

[cgrindel]

That sounds reasonable to me.

[aiuto]

I'll presume you are on the right track with the needs, so let's turn to the details.

find-debuginfo living in the debugedit package that digests the binaries provided as inputs and spits out a Foo-debuginfo.rpm.
Without know a lot about the details this feels like that would need all the inputs to the rpm staged again to look for executables.
Is that sufficient or is it worse, like needing to run in the original source tree - thus making remote execution tricky.

tl;dr I'm trying avoid duplicating rpmbuild behaviour instead of just driving it, but it might make the caching worse
Yeah. Tough engineering choice there. It is a pity you can't use providers on the cc_binary targets to gather debug data directly. Perhaps pkg_files could pass them up. You might get better performance, but at the cost of maintaining your own tool.

[kellyma2]

Without know a lot about the details this feels like that would need all the inputs to the rpm staged again to look for executables.
Is that sufficient or is it worse, like needing to run in the original source tree - thus making remote execution tricky.

Can you elaborate on this a bit? I'm definitely unclear on the implications around remote execution in this respect as I've not explored it at all.

Yeah. Tough engineering choice there. It is a pity you can't use providers on the cc_binary targets to gather debug data directly. Perhaps pkg_files could pass them up. You might get better performance, but at the cost of maintaining your own tool.

I'll try to clean up my changes and post a PR next week. I need to do a bit more testing in a "normal" rpmbuild environment to ensure that I haven't gotten anything fundamentally wrong. Further out, it might make sense to distill what's in find-debuginfo into something useful in rules_pkg, but I still expect there'll be some complications (eg: it'll still depend on debugedit which is packaged with find-debuginfo). My main dislike at the moment is that it requires the RPM configuration to be sort of precise, which is a bit fiddly for my tastes.

[kellyma2]

Sorry for the delay in coming back to this, got distracted by other priorities longer than expected. I just pushed the initial PR as #824 . Worth noting that there's not any tests in there yet because I wanted to get agreement as to the form and structure before finalizing that part, although there is an example in there.