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

Symlink warnings since upgrading to Bazel 6 #17081

Closed
pdeva opened this issue Dec 27, 2022 · 22 comments
Closed

Symlink warnings since upgrading to Bazel 6 #17081

pdeva opened this issue Dec 27, 2022 · 22 comments
Assignees
Labels
awaiting-user-response Awaiting a response from the author P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug

Comments

@pdeva
Copy link

pdeva commented Dec 27, 2022

Description of the bug:

Seeing this message in output since upgrading to Bazel 6 (from Bazel 5):

WARNING: cleared convenience symlink(s) bazel-bin, bazel-testlogs because their destinations would be ambiguous

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

upgrade to bazel 6 from bazel 5 and run build

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

release 6.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

git@github.com:astradot/monorepo.git
55e066072b359d1bc55555c23bdc1476e79042f5
55e066072b359d1bc55555c23bdc1476e79042f5

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@sgowroji
Copy link
Member

Hi @pdeva, Minimal reproduce case is required for the above issue. Thanks!

@simmonmt
Copy link

simmonmt commented Jan 2, 2023

not minimal, but pull https://github.com/simmonmt/advent-of-code and bazel build in the 2022 directory with bazel 6 will reproduce it.

@fmeum
Copy link
Collaborator

fmeum commented Jan 2, 2023

As the reproducer uses the nogo rule, this is caused by bazel-contrib/rules_go#3409. Updating rules_go to commit bazel-contrib/rules_go@de2074e fixes the warning and results in symlinks being created correctly.

@simmonmt
Copy link

simmonmt commented Jan 2, 2023

Thanks Fabian! Can confirm that adding the manual tag to the nogo rule fixed the problem. That's what the commit you referenced does, so it seemed easy to do that rather than updating rules_go to an unreleased commit. Happy New Year!

simmonmt added a commit to simmonmt/advent-of-code that referenced this issue Jan 2, 2023
@sluongng
Copy link
Contributor

This is a troublesome change from Bazel upstream. In a large project with rules_go's nogo, or rules_docker's container_image or any targets with transition applied, it would cause a lot of noise and some cases... cause builds to fail.

@fmeum hinted, to the discussion thread over at rules_go, but did not point out the root cause being this flag flipped in Bazel 6.0 6452024.

What is the long-term solution for this flag flip? Do we discourage folks from configuring top-level targets with transition?

@sgowroji Is it possible to contact the author of this CL for more info?

PiperOrigin-RevId: 454603210
Change-Id: I9bb2f0a46beeff27798b2a6006d5763ae5ff75c7

@sgowroji sgowroji added type: bug team-Configurability platforms, toolchains, cquery, select(), config transitions untriaged and removed more data needed labels Feb 14, 2023
@bolitt
Copy link

bolitt commented Feb 17, 2023

Got the same message after upgrading 6.0.0. I find bazel-bin and bazel-testlogs are automatically gone.

Build label: 6.0.0
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Dec 19 16:07:42 2022 (1671466062)
Build timestamp: 1671466062
Build timestamp as int: 1671466062

I find the doc of output directory layout is still indicating bazel-bin and bazel-testlogs:
https://bazel.build/remote/output-directories#layout-diagram

It is confusing: Are bazel-bin and bazel-testlogs recommended to be used or not after bazel 6?

Update: Assuming that I want to preserve the two folders, I need to set

bazel build --use_top_level_targets_for_symlinks=false //...

Or, add a flag in .bazelrc

build --use_top_level_targets_for_symlinks=false

@aranguyen
Copy link
Contributor

aranguyen commented Mar 13, 2023

Hi all, apologies for the late response.
@fmeum I'm not familiar with the manual tag in your fix, could you please help explain its functionality?

What is the long-term solution for this flag flip?

@sluongng Since this flag is enabled by default, the long term goal is to remove it and the current behavior would stick. You also mentioned that this caused breakages, could you please help me with a repro for a known breakage? We can talk more about the current WARNING being noisy and perhaps consider its removal after I have a better understanding of your breakage.

As a recap for this flag: When it is enabled, for a build with a single top-level target, this will guarantee that blaze-bin points to whatever the output directory of that top-level target. If there are multiple top level configurations making the output directories ambiguous it clears the symlinks.

@bolitt setting --use_top_level_targets_for_symlinks=false would be a temporary workaround if you want to preserve those symlinks. However, since the flag will be eventually removed and the default behavior would stick, this won't be a long term solution. Is your code somehow dependent on blaze-bin?

@fmeum
Copy link
Collaborator

fmeum commented Mar 13, 2023

@aranguyen tags = ["manual"] makes it so that the tagged target is not matched by wildcard patterns.

OSS users tend to do bazel build //... frequently simply because their repos are small enough that this is feasible. But with targets that self-transition, the new algorithm means that they will essentially always use their convenience symlinks with this pattern.

@gregestren gregestren added P2 We'll consider working on this in future. (Assignee optional) awaiting-user-response Awaiting a response from the author and removed untriaged labels Mar 13, 2023
@sluongng
Copy link
Contributor

@aranguyen Here is one of our latest PR where we have to apply @fmeum 's suggested fix to mitigate the issue buildbuddy-io/buildbuddy#3393. This has an obvious downside: these code paths that are marked with "manual" tag are excluded from our CI and thus, not verified.

Targets with self-transition are very common in the Go ecosystem: engineers often develop on a MacOS machine and deploy on a Linux environment (as a container via rules_docker's go_image), so it's typical for a Bazel + rules_go setup to use transition to help provide cross-compilation between these environments and architectures.

@bolitt
Copy link

bolitt commented Mar 23, 2023

Hi all, apologies for the late response. @fmeum I'm not familiar with the manual tag in your fix, could you please help explain its functionality?

What is the long-term solution for this flag flip?

@sluongng Since this flag is enabled by default, the long term goal is to remove it and the current behavior would stick. You also mentioned that this caused breakages, could you please help me with a repro for a known breakage? We can talk more about the current WARNING being noisy and perhaps consider its removal after I have a better understanding of your breakage.

As a recap for this flag: When it is enabled, for a build with a single top-level target, this will guarantee that blaze-bin points to whatever the output directory of that top-level target. If there are multiple top level configurations making the output directories ambiguous it clears the symlinks.

@bolitt setting --use_top_level_targets_for_symlinks=false would be a temporary workaround if you want to preserve those symlinks. However, since the flag will be eventually removed and the default behavior would stick, this won't be a long term solution. Is your code somehow dependent on blaze-bin?

I have a bunch of scripts to build and run, like

bazel build //a/b:c
./bazel-bin/a/b/c

So my question is: what is the suggested final solution? Only to use the OS-dependent command?

./bazel-out/darwin-opt/bin/a/b/c

@aranguyen
Copy link
Contributor

Hi all, sorry for the late reply but here are updates from our team on this issue:

  • As there is a workaround for this issue, we are going to proceed with the flag removal. However, we also understand your use case and propose to change the behavior of this flag to only clear out convenient symlinks of those that match the top-level configuration. What do you all think of this option?
  • We're also not prioritizing this work at the moment but would be open to provide guidance if anyone would like to contribute for this changes.

@aranguyen aranguyen added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Apr 18, 2023
@sluongng
Copy link
Contributor

I would argue that the current workaround is a UX degradation that needs to be addressed. Not quite sure why flag removal is being pushed forward so aggressively while there are unaddressed problems stated here.

Clearing symlink automatically based on top-level target configurations feels "magical" at best and confusing/anti-user at worst. If a big repo has complicated transitions that are used for purposes other than building a binary (i.e. nogo used as linter), at least offer an optional API to mark such transition as secondary permanently so related binaries are never included in the symlink.

@photex
Copy link

photex commented Apr 18, 2023

I seem to have discovered this as an unfortunate interaction with the CLion plugin.

If I build my target bazel build //plugin:foo then I'll have stable output in bazel-bin, but if I use the CLion command to "Build Project", bazel will remove these symlinks.

This is really confusing and a bit frustrating because there is already a lot to digest with bazels output directory structure, but as a user of a build system who is trying to build a plugin for a host application I really need a way to have a stable output location of the targets I define.

I don't really mind the details of how that's arrived at, but from the user point of view it should work about as easily as bazel build //... creating a folder called bazel-products or bazel-<workspace>/products and finding everything there.

@nikhil-marathe-skydio
Copy link

bazel-testlogs no longer being reliably available means tooling that builds on this no longer works.
For example, Jenkins has a junit function that takes a glob pattern and extracts test failures from the test.xml files Bazel produces.

What is the long term solution for these workflows?

@gregestren
Copy link
Contributor

Thanks for all this feedback. @fmeum , @aranguyen , and I are currently re-reviewing based on your experiences.

@gregestren
Copy link
Contributor

We talked and we'll work on an adjustment from the above feedback. Ball is currently in my court. I'll plan to visit this (ideally with a PR we can review) in June. Feel free to ping if time passes by and you're unsure where things stand.

@SanjayVas
Copy link

Just to be clear on what expectation I believe that was violated: A workspace dependency should not affect the convenience symlinks.

@gregestren
Copy link
Contributor

@SanjayVas can you elaborate more precisely?

@SanjayVas
Copy link

@gregestren My understanding is that the default behavior change means that transitions can result in the convenience symlinks being cleared. Simply depending on rules from workspaces that use transitions (e.g. rules_docker) can therefore trigger this behavior.

@gregestren
Copy link
Contributor

That shouldn't trigger if you're only building the depender. You've noticed otherwise?

@SanjayVas
Copy link

Simple repo that demonstrates this issue: https://github.com/world-federation-of-advertisers/containers

There are no transitions in this code. Any use of transitions is due to use of rules from the rules_docker dependency.

gregestren added a commit to gregestren/bazel that referenced this issue Jul 6, 2023
Old semantics:

New semantics:

Fixes bazelbuild#17081.

PiperOrigin-RevId: 546025529
Change-Id: I095564bb9233eba4b008df7f9e6aedae3e973b23
gregestren added a commit to gregestren/bazel that referenced this issue Jul 7, 2023
Old semantics:

New semantics:

Fixes bazelbuild#17081.

PiperOrigin-RevId: 546025529
Change-Id: I095564bb9233eba4b008df7f9e6aedae3e973b23
@gregestren
Copy link
Contributor

gregestren commented Jul 10, 2023

We're aiming to get ceb9955 into Bazel 6.3.0, which will be cut Wednesday.

Note this doesn't guarantee bazel-bin always exists. Guaranteeing that is a complicated project, described at #15005.

But it should remove the 6.0 regression and (I hope) make 6.3.0 a strict improvement over < 6.0. And it should cleanly address all the rules_go concerns and anyone running tests.

Appreciate anyone's feedback if you can test the change out.

iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jul 10, 2023
Adjust `--top_level_targets_for_symlinks`.

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs, `bazel-bin`, etc. are deleted

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs and at least one of them is the top-level config, `bazel-bin`, etc. point to the top-level config
- If targets in `$ bazel build //...` have mixed configs and none are the top-level config, `bazel-bin`, etc. are deleted

Fixes bazelbuild#17081.

Closes bazelbuild#18854.

PiperOrigin-RevId: 546938509
Change-Id: I75adf0b8c2094522125c5e65d8c450eb2436d392
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jul 10, 2023
Adjust `--top_level_targets_for_symlinks`.

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs, `bazel-bin`, etc. are deleted

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs and at least one of them is the top-level config, `bazel-bin`, etc. point to the top-level config
- If targets in `$ bazel build //...` have mixed configs and none are the top-level config, `bazel-bin`, etc. are deleted

Fixes bazelbuild#17081.

Closes bazelbuild#18854.

PiperOrigin-RevId: 546938509
Change-Id: I75adf0b8c2094522125c5e65d8c450eb2436d392
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jul 11, 2023
Adjust `--top_level_targets_for_symlinks`.

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs, `bazel-bin`, etc. are deleted

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs and at least one of them is the top-level config, `bazel-bin`, etc. point to the top-level config
- If targets in `$ bazel build //...` have mixed configs and none are the top-level config, `bazel-bin`, etc. are deleted

Fixes bazelbuild#17081.

Closes bazelbuild#18854.

PiperOrigin-RevId: 546938509
Change-Id: I75adf0b8c2094522125c5e65d8c450eb2436d392
gregestren added a commit to gregestren/bazel that referenced this issue Jul 12, 2023
Old semantics:

New semantics:

Fixes bazelbuild#17081.

PiperOrigin-RevId: 546025529
Change-Id: I095564bb9233eba4b008df7f9e6aedae3e973b23
gregestren added a commit to gregestren/bazel that referenced this issue Jul 12, 2023
Adjust `--top_level_targets_for_symlinks`.

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs, `bazel-bin`, etc. are deleted

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs and at least one of them is the top-level config, `bazel-bin`, etc. point to the top-level config
- If targets in `$ bazel build //...` have mixed configs and none are the top-level config, `bazel-bin`, etc. are deleted

Fixes bazelbuild#17081.

Closes bazelbuild#18854.

PiperOrigin-RevId: 546938509
Change-Id: I75adf0b8c2094522125c5e65d8c450eb2436d392
iancha1992 pushed a commit that referenced this issue Jul 12, 2023
* Adjust --top_level_targets_for_symlinks

Adjust `--top_level_targets_for_symlinks`.

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs, `bazel-bin`, etc. are deleted

- If all targets in `$ bazel build //...` have the top-level config, `bazel-bin`, etc. point to that config
- If all targets in `$ bazel build //...` have the same transitioned config, `bazel-bin`, etc. point to that config
- If targets in `$ bazel build //...` have mixed configs and at least one of them is the top-level config, `bazel-bin`, etc. point to the top-level config
- If targets in `$ bazel build //...` have mixed configs and none are the top-level config, `bazel-bin`, etc. are deleted

Fixes #17081.

Closes #18854.

PiperOrigin-RevId: 546938509
Change-Id: I75adf0b8c2094522125c5e65d8c450eb2436d392

* Merge with 6.3.0 baseline.

Particularly manually merge relevant signature changes from
1cd3588. Thsi
is different than
1cd3588
itself because that also uses a baseline too new for 6.3.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.