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

Do not set user defined env variables twice for (c)make #860

Merged

Conversation

fmeum
Copy link
Member

@fmeum fmeum commented Jan 20, 2022

These variables are already exported in the build script prelude and thus don't have to be set on individual make commands.

Resolves an undesired side effect of f61ce5d, that however should not have resulted in this kind of breakage.

Workaround for #857, the fix for the underlying issue will be submitted as a separate PR.

@fmeum
Copy link
Member Author

fmeum commented Jan 20, 2022

@garrettkajmowicz-sophos Could you check whether this fixes the issue for you? It does in my tests, which will be part of a follow-up PR.

@garrettkajmowicz-sophos

This does address the issue.

@jsharpe jsharpe enabled auto-merge (squash) January 26, 2022 21:17
auto-merge was automatically disabled February 1, 2022 23:16

Head branch was pushed to by a user without write access

@fmeum fmeum force-pushed the 857-deduplicate-env-vars branch from c8cd180 to 5a18138 Compare February 1, 2022 23:16
@fmeum
Copy link
Member Author

fmeum commented Feb 1, 2022

@jsharpe Sorry, I missed that this became out of date. Rebased onto main.

@jsharpe jsharpe enabled auto-merge (squash) February 1, 2022 23:23
@fmeum
Copy link
Member Author

fmeum commented Feb 8, 2022

@jsharpe This PR's GitHub Action run got hit by the GitHub archive checksum change, see bazel-contrib/SIG-rules-authors#11 (comment) for context. Could you rerun the pipeline so that this PR will be auto-merged?

@jsharpe
Copy link
Member

jsharpe commented Feb 8, 2022

I don't have the permissions to rerun the pipeline in buildkite. You'll need to push a trivial change to retrigger it.

These variables are already exported in the build script prelude and
thus don't have to be set on individual make commands.
auto-merge was automatically disabled February 8, 2022 21:48

Head branch was pushed to by a user without write access

@fmeum fmeum force-pushed the 857-deduplicate-env-vars branch from 5a18138 to 8b8adf5 Compare February 8, 2022 21:48
@fmeum
Copy link
Member Author

fmeum commented Feb 8, 2022

@jsharpe Done

@jsharpe jsharpe enabled auto-merge (squash) February 8, 2022 21:59
@jsharpe jsharpe merged commit f0047ba into bazel-contrib:main Feb 8, 2022
@fmeum fmeum deleted the 857-deduplicate-env-vars branch February 9, 2022 08:08
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