search

bazelbuild / rules_pkg

Bazel rules for creating packages of many types (zip, tar, deb, rpm, ...)
Apache License 2.0
214 stars 167 forks source link

Add pkg_sub_rpm rule for RPM subpackages #824

Closed kellyma2 closed 4 months ago

kellyma2 commented 4 months ago

This change introduces a pkg_sub_rpm rule that can be used to generate the sub RPMs associated with a given parent RPM. Currently this would be possible to cobble together using multiple pkg_rpm rules, but doing it "correctly" as sub RPMs has a few advantages:

This implementation explicitly only works with the non-specfile based invocations of pkg_rpm so as to avoid having to scrape the specfile in order to determine which subpackages we're generating.

Internally, the rule functions by generating a PackageSubRPMInfo provider that can then be consumed by our top-level pkg_rpm rule to perform the actual RPM generation.

cgrindel commented 4 months ago

This implementation explicitly only works with the non-specfile based invocations of pkg_rpm so as to avoid having to scrape the specfile in order to determine which subpackages we're generating.

I did not notice this being called out in any doc. Is it worth mentioning it somewhere so that folks do not go down a rabbit hole?

kellyma2 commented 4 months ago

@aiuto do you prefer for me to add the doc updates in this PR or should I leave that to be updated separately?

kellyma2 commented 4 months ago

This implementation explicitly only works with the non-specfile based invocations of pkg_rpm so as to avoid having to scrape the specfile in order to determine which subpackages we're generating.

I did not notice this being called out in any doc. Is it worth mentioning it somewhere so that folks do not go down a rabbit hole?

The last commit in the list here adds that clarification to the pkg_rpm rule docstring.

aiuto commented 4 months ago

This implementation explicitly only works with the non-specfile based invocations of pkg_rpm so as to avoid having to scrape the specfile in order to determine which subpackages we're generating.

I did not notice this being called out in any doc. Is it worth mentioning it somewhere so that folks do not go down a rabbit hole?

It sounds like the implementation does not do it, but there is no reason why it could not in some way. Would that mean taking the provided specfile and splicing in new sections for each sub rpm?

kellyma2 commented 4 months ago

Sorry for the slow reply. I started a new role last week and have been swamped.

No worries :)

kellyma2 commented 4 months ago

This implementation explicitly only works with the non-specfile based invocations of pkg_rpm so as to avoid having to scrape the specfile in order to determine which subpackages we're generating.

I did not notice this being called out in any doc. Is it worth mentioning it somewhere so that folks do not go down a rabbit hole?

It sounds like the implementation does not do it, but there is no reason why it could not in some way. Would that mean taking the provided specfile and splicing in new sections for each sub rpm?

Anything is possible in theory, but I expect it'd be a bit messy. I hadn't looked in depth at the legacy/specfile mode as I was assuming this is not the encouraged way to move forward.

That noted, I did add this caveat to the docstrings for the rules to ensure that this is more clear and there's a guard in there to prevent the combination of subrpms and specfile mode.

kellyma2 commented 4 months ago

@aiuto Any updates?

kellyma2 commented 4 months ago

Approval on the grounds that I'm expecting you to fix any bugs people find with this. :-)

Sounds fun :)

aiuto commented 4 months ago

It turns out this breaks on the version of rpmbuild I have. Sigh.

I traced it to the addition of --define buildsubdir . That results in rpmbuild doing a cleanup of rm -rf . /tmp/tmpXXXXXXX/BUILD. And then rm -rf fails because you can't delete . I cc'ed you on the fix PR.

On Fri, Mar 15, 2024 at 12:24 PM Mike Kelly @.***> wrote:

Approval on the grounds that I'm expecting you to fix any bugs people find with this. :-)

Sounds fun :)

— Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_pkg/pull/824#issuecomment-2000014404, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHC7PJQXJBUA4WV7PK3YYMOFHAVCNFSM6AAAAABD6X4TIKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBQGAYTINBQGQ . You are receiving this because you modified the open/close state.Message ID: @.***>

kellyma2 commented 3 months ago

It turns out this breaks on the version of rpmbuild I have. Sigh.

From testing my debuginfo changes, it seems like this is likely more dependant on the underlying system then it is on the rpmbuild version. Specifically, I got very different results from CentOS and Fedora because of how they've defined the RPM macros. Definitely needs more testing. :P