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

--use_top_level_targets_for_symlinks doesn't apply through alias() targets #14602

Closed
keith opened this issue Jan 19, 2022 · 4 comments
Closed
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug

Comments

@keith
Copy link
Member

keith commented Jan 19, 2022

With this BUILD file:

load("@build_bazel_rules_apple//apple:macos.bzl", "macos_command_line_application")

alias(
    name = "alias",
    actual = "cli",
)

cc_library(
    name = "lib",
    srcs = ["main.c"],
)

macos_command_line_application(
    name = "cli",
    minimum_os_version = "10.10",
    deps = ["lib"],
)

Building the CI directly with --use_top_level_targets_for_symlinks correctly results in a top level symlink:

% bazel build cli  --use_top_level_targets_for_symlinks
INFO: Analyzed target //:cli (23 packages loaded, 387 targets configured).
INFO: Found 1 target...
Target //:cli up-to-date:
  bazel-bin/cli
INFO: Elapsed time: 2.727s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

But building the alias target still uses the transition path:

% bazel build alias --use_top_level_targets_for_symlinks
INFO: Analyzed target //:alias (0 packages loaded, 1 target configured).
INFO: Found 1 target...
Target //:cli up-to-date:
  bazel-out/applebin_macos-darwin_x86_64-fastbuild-ST-45701c223259/bin/cli
INFO: Elapsed time: 0.218s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

repro project: aliasrepro.zip

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 5.0.0 (not a regression, also repros on 4.2.2)

@aiuto aiuto added the team-Performance Issues for Performance teams label Jan 20, 2022
@joeleba joeleba added team-Core Skyframe, bazel query, BEP, options parsing, bazelrc and removed team-Performance Issues for Performance teams labels Jan 20, 2022
@joeleba
Copy link
Member

joeleba commented Jan 20, 2022

Reassigning to Janak who added --use_top_level_targets_for_symlinks for triage. Thanks!

@janakdr janakdr added team-Configurability platforms, toolchains, cquery, select(), config transitions and removed team-Core Skyframe, bazel query, BEP, options parsing, bazelrc labels Jan 20, 2022
@janakdr
Copy link
Contributor

janakdr commented Jan 20, 2022

Looks like this was actually added by the configurability team (I just moved the flag around). Passing over.

@gregestren gregestren assigned sdtwigg and unassigned janakdr Jan 26, 2022
@sdtwigg
Copy link
Contributor

sdtwigg commented Jan 31, 2022

This seems to be that the logic in //src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java is not sophisticated enough to consider 'look-through' aliases when doing the filtering.

(Really technically, by default, alias is treated as a rule that forwards all the providers of the aliased rule. In this case, the alias isn't experiencing the self-transition, its target is so the alias has an untransitioned top-level configuration. Consumers, like the logic handling --use_top_level_targets_for_symlinks have to specifically remember to dealias the alias if appropriate.)

@sdtwigg sdtwigg added P2 We'll consider working on this in future. (Assignee optional) untriaged and removed untriaged labels Jan 31, 2022
keith added a commit to keith/bazel that referenced this issue Apr 22, 2022
@keith
Copy link
Member Author

keith commented Apr 22, 2022

I've submitted #15316 which appears to fix this

keith added a commit to keith/bazel that referenced this issue Apr 25, 2022
ckolli5 added a commit that referenced this issue May 10, 2022
Fixes #14602

Closes #15316.

PiperOrigin-RevId: 444970714

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
meteorcloudy pushed a commit that referenced this issue May 10, 2022
Fixes #14602

Closes #15316.

PiperOrigin-RevId: 444970714

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants