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.

Sign up for GitHub

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

Jump to bottom

Improve handling of sub rpms. #833

Merged
merged 1 commit into from
Mar 25, 2024
Merged

Improve handling of sub rpms. #833

merged 1 commit into from
Mar 25, 2024

Conversation

aiuto
Copy link
Collaborator

@aiuto aiuto commented Mar 15, 2024 •
edited
Loading

  • Do not create the subrpms files when we don't have to
  • Do not add buildsubdir . That seems to cause rpmbuild failures with older rpmbuild. It generates a cleanup 
     rm -rf . /tmp/tmpXXXXX/BUILD
    rm chokes on the . and fails.
  • General cleanup: Remove tests against None which is needlessly brittle.
    This is a follow to Add pkg_sub_rpm rule for RPM subpackages #824.
    @kellyma2 : Can you test this against your setup to see if the buildsubdir change breaks you?

@aiuto
Improve handling of sub rpms.
23a4437 
- Do not create the subrpms files when we don't have to
- Do not add `buildsubdir .`  That seems to cause rpmbuild failures with
  older rpmbuild.
- General cleanup: Remove tests against None which is needlessly brittle.
@aiuto aiuto requested a review from cgrindel as a code owner March 15, 2024 15:30
@kellyma2
Copy link
Contributor

Will do.

@cgrindel
Copy link
Collaborator

FYI. I am waiting to hear back about @kellyma2 testing before hitting approve.

@kellyma2
Copy link
Contributor

FYI. I am waiting to hear back about @kellyma2 testing before hitting approve.

Sorry, I think I'm ok to merge this, should have been clearer in my comments. I'm away for the week but have done some clearer tests around buildsubdir and how it needs to be used. I'll have a PR when I'm back that covers this a bit better and also includes debuginfo support

@aiuto aiuto added the P1 An issue that must be resolved. Must have an assignee label Mar 21, 2024
@aiuto aiuto closed this Mar 21, 2024
@aiuto aiuto deleted the zrpm branch March 21, 2024 19:26
@aiuto aiuto restored the zrpm branch March 21, 2024 19:27
@aiuto aiuto reopened this Mar 25, 2024
@aiuto
Copy link
Collaborator Author

aiuto commented Mar 25, 2024

Silly me. Closed without merging. Necromerging now.

@aiuto aiuto merged commit dcce3e6 into bazelbuild:main Mar 25, 2024
2 checks passed
@aiuto aiuto deleted the zrpm branch March 25, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kellyma2 kellyma2 kellyma2 left review comments

@cgrindel cgrindel cgrindel approved these changes

Assignees
No one assigned
Labels
P1 An issue that must be resolved. Must have an assignee
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aiuto @kellyma2 @cgrindel