Add and flip --incompatible_locations_prefers_executable
#24690
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
team-Configurability
fmeum
requested review from
brandjon
and removed request for
lberki and
katre
RELNOTES: Extra targets provided to `ctx.expand_location` now expand to their executable (if any) instead of resulting in an error if they provide a number of files different from one. RELNOTES[INC]: The `--incompatible_locations_prefers_executable` flag has been added and enabled, which makes it so that `ctx.expand_location` expands `$(locations :x)` to the executable of an extra target `:x` if it provides one and the number of files provided by it is not one.
fmeum
force-pushed
the
24613-incompatible
branch
from
8667744 to
755b7d3
Compare
|
@brandjon Yun and I agreed that a minor incompatible change would be acceptable even for 8.0.1. Based on that, I personally find this to be the less surprising (and simpler) solution to the linked issues (most importantly the regression #24613) than the fully backwards compatible change that only modifies the behavior of An alternative could be to always prefer the executable, even over a single file, as is the case for location expansion on the hardcoded list of attributes. I could see this being much more breaking, e.g. thinking of a WASM binary that sets its executable to an interpreter that runs the actual artifact. Your opinion and ideas would be very welcome! |
|
Delegating to @comius as this is more analysis-time rule logic than loading phase stuff. |
|
@comius do you think it's a good idea to get this in soon (within the week)? If not, we should take this off the 8.0.1 blocker list. |
comius
approved these changes
Jan 8, 2025
comius
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
fmeum
added a commit
to fmeum/bazel
that referenced
this pull request
Jan 9, 2025
Work towards bazelbuild#11820 Fixes bazelbuild#20038 Fixes bazelbuild#23200 Fixes bazelbuild#24613 RELNOTES: Extra targets provided to `ctx.expand_location` now expand to their executable (if any) instead of resulting in an error if they provide a number of files different from one. RELNOTES[INC]: The `--incompatible_locations_prefers_executable` flag has been added and enabled, which makes it so that `ctx.expand_location` expands `$(locations :x)` to the executable of an extra target `:x` if it provides one and the number of files provided by it is not one. Closes bazelbuild#24690. PiperOrigin-RevId: 713453768 Change-Id: I0d6e052bc70deea029554ab722feb544f9597a23 (cherry picked from commit 457d248)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 9, 2025
…24874) Work towards #11820 Fixes #20038 Fixes #23200 Fixes #24613 RELNOTES: Extra targets provided to `ctx.expand_location` now expand to their executable (if any) instead of resulting in an error if they provide a number of files different from one. RELNOTES[INC]: The `--incompatible_locations_prefers_executable` flag has been added and enabled, which makes it so that `ctx.expand_location` expands `$(locations :x)` to the executable of an extra target `:x` if it provides one and the number of files provided by it is not one. Closes #24690. PiperOrigin-RevId: 713453768 Change-Id: I0d6e052bc70deea029554ab722feb544f9597a23 (cherry picked from commit 457d248) Fixes #24646
|
Thank you @fmeum for the patch, i really wanted to contribute this myself but you beat me to it :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Work towards #11820
Fixes #20038
Fixes #23200
Fixes #24613
RELNOTES: Extra targets provided to
ctx.expand_locationnow expand to their executable (if any) instead of resulting in an error if they provide a number of files different from one.RELNOTES[INC]: The
--incompatible_locations_prefers_executableflag has been added and enabled, which makes it so thatctx.expand_locationexpands$(locations :x)to the executable of an extra target:xif it provides one and the number of files provided by it is not one.