Skip to content

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

Merged
Wyverald merged 1 commit intobazelbuild:release-5.0.0rc1from
brentleyjones:bj/remote-merge-target-level-exec_properties-with-remote_default_exec_properties
Nov 3, 2021
Merged

Remote: Merge target-level exec_properties with --remote_default_exec_properties#14212
Wyverald merged 1 commit intobazelbuild:release-5.0.0rc1from
brentleyjones:bj/remote-merge-target-level-exec_properties-with-remote_default_exec_properties

Conversation

@brentleyjones
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

#14193 (comment)

@brentleyjones
Copy link
Copy Markdown
Contributor Author

cc @Wyverald

@coeuvre
Copy link
Copy Markdown
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
Copy Markdown
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