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

Exclude release from filename when using release_file #863

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

kellyma2
Copy link
Contributor

When we're using release_file in lieu of release we're just pointing rpmbuild at the file containing the Release string and we don't have it available to inject into the filename resulting in a strange looking filename of the form Foo-version-.arch.rpm.

This change extracts the RPM name generation to a single helper, _make_rpm_filename and tweaks it s.t. if we're missing the value for release we'll just exclude it from the filename format instead.

When we're using `release_file` in lieu of `release` we're just
pointing rpmbuild at the file containing the `Release` string and we
don't have it available to inject into the filename resulting in a
strange looking filename of the form `Foo-version-.arch.rpm`.

This change extracts the RPM name generation to a single helper,
`_make_rpm_filename` and tweaks it s.t. if we're missing the value for
`release` we'll just exclude it from the filename format instead.
@kellyma2 kellyma2 requested review from aiuto and cgrindel as code owners April 25, 2024 04:34
The test had was using the odd RPM name structure and this change
tweaks it so that the test passes.
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.

LGTM. I will wait for @aiuto to review before merging.

@aiuto aiuto merged commit c8d6a02 into bazelbuild:main Apr 29, 2024
2 checks passed
@kellyma2 kellyma2 deleted the package-name-pr 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