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

Remote: Merge target-level exec_properties with --remote_default_exec_properties #14212

Conversation

brentleyjones
Copy link
Contributor

Per-target exec_properties was introduced by 0dc53a2 but it didn't take into account of --remote_default_exec_properties which provides default exec properties for the execution platform if it doesn't already set with exec_properties. So the per-target exec_properties overrides --remote_default_exec_properties instead of merging with them which is wrong.

Fixes #10252.

Closes #14193.

PiperOrigin-RevId: 406337649
(cherry picked from commit 3a5b360)

…_properties

Per-target `exec_properties` was introduced by 0dc53a2 but it didn't take into account of `--remote_default_exec_properties` which provides default exec properties for the execution platform if it doesn't already set with `exec_properties`. So the per-target `exec_properties` overrides `--remote_default_exec_properties` instead of merging with them which is wrong.

Fixes bazelbuild#10252.

Closes bazelbuild#14193.

PiperOrigin-RevId: 406337649
(cherry picked from commit 3a5b360)
@google-cla google-cla bot added the cla: yes label Nov 2, 2021
@brentleyjones
Copy link
Contributor Author

#14193 (comment)

@brentleyjones
Copy link
Contributor Author

cc @Wyverald

@coeuvre
Copy link
Member

coeuvre commented Nov 3, 2021

Thanks for the cherrypick! (I was going to collect a bunch of commits for RE and submit in one go.)

@coeuvre coeuvre assigned Wyverald and unassigned coeuvre Nov 3, 2021
@coeuvre coeuvre requested a review from Wyverald November 3, 2021 02:00
@brentleyjones
Copy link
Contributor Author

Thanks for the cherrypick! (I was going to collect a bunch of commits for RE and submit in one go.)

You can still do that and close this out if it makes it easier for you or @Wyverald. I just wasn't sure on the timing of the next release cut, so I wanted to queue this up ASAP.

@Wyverald Wyverald merged commit 713abde into bazelbuild:release-5.0.0rc1 Nov 3, 2021
@brentleyjones brentleyjones deleted the bj/remote-merge-target-level-exec_properties-with-remote_default_exec_properties branch November 3, 2021 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants