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

Explicitly set %{_builddir} macro #792

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Conversation

kellyma2
Copy link
Contributor

rules_pkg has baked in the assumption that the value of %{_builddir} is going to be %{_topdir}/BUILD which is where rpmbuild will cd to when being run. When using the built in system rpmbuild in a situation where the value of %{_builddir} has been overriden with a custom local macro, this assumption may be broken which will result in internal rpmbuild failures.

Because we make this assumption about the value of the macro, we can instead be explicit about what we want the value of %{_builddir} to be so as to avoid this problem altogether.

rules_pkg has baked in the assumption that the value of `%{_builddir}`
is going to be `%{_topdir}/BUILD` which is where rpmbuild will `cd` to
when being run.  When using the built in system rpmbuild in a
situation where the value of `%{_builddir}` has been overriden with a
custom local macro, this assumption may be broken which will result in
internal rpmbuild failures.

Because we make this assumption about the value of the macro, we can
instead be explicit about what we want the value of `%{_builddir}` to
be so as to avoid this problem altogether.
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. The CI failures don't look related to this. However, I have not dug into them.

@aiuto Is there a way to re-run the CI for this PR?

@aiuto
Copy link
Collaborator

aiuto commented Dec 19, 2023

The failures are all stardoc. I"m going to force merge this and then disable the doc building in CI.
I'll do that manually.

@aiuto aiuto merged commit a0d0deb into bazelbuild:main Dec 19, 2023
1 of 2 checks passed
@aiuto
Copy link
Collaborator

aiuto commented Dec 19, 2023

Sigh... It/s not just stardoc. bzlmod at bazel 7.0.0. seems to mess with some things about local_repository.
No time to explain more on that.

@kellyma2 kellyma2 deleted the enforce-macros branch February 17, 2024 01:22
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