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

Repeated invocations with --noallow_analysis_cache_discard don't fail as expected #23491

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

Comments

@lberki
Copy link
Contributor

lberki commented Sep 3, 2024

Description of the bug:

If I run a build with --noallow_analysis_cache_discard and with changed configuration flags, it fails as expected.

However, if I run the same build a second time, it runs, thereby discarding the analysis cache despite being asked not to do so.

Which category does this issue belong to?

Configurability

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

cat > BUILD <<'EOF'
cc_binary(name="cc", srcs=["cc.cc"])
EOF

cat > cc.cc << 'EOF'
int main(void) {
  return 0;
}
EOF

bazel build -c opt :cc
bazel build --noallow_analysis_cache_discard -c dbg :cc  # Fails
bazel build --noallow_analysis_cache_discard -c dbg :cc  # Builds, but it should not (and discards the analysis cache)

Furthermore, switching back to the original configuration fails, although it work:

bazel build -c opt :cc
bazel build --noallow_analysis_cache_discard -c dbg :cc  # Fails
bazel build --noallow_analysis_cache_discard -c opt :cc  # Fails, although analysis cache has -c opt

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

Google-internal version

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

Used the currently released version at Google.

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

No response

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

My guess would be #16805, the initial implementation of this flag although I haven't dug into it.

Have you found anything relevant by searching the web?

No response

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

No response

@lberki lberki added type: bug P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions labels Sep 3, 2024
@gregestren
Copy link
Contributor

@fmeum
Copy link
Collaborator

fmeum commented Sep 17, 2024

@bazel-io fork 7.4.0

lberki added a commit that referenced this issue Sep 18, 2024
…nce.

This didn't work at HEAD because the check there compared the flags in the new configuration with the flags of the old configuration, but overwrote the old configuration with the new one, so running a command that discarded the analysis cache was allowed the second time.

The fix is not as trivial as moving checking the value before setting the new configuration because creating the new configuration entails a Skyframe evaluation, which is a waste if we end up erroring out. So the check is now done by checking the build options of the old configuration and the new build options, which doesn't require creating the new configuration first.

Fixes #23491.

RELNOTES: None.
PiperOrigin-RevId: 675555324
Change-Id: Ib402b699ed9ba9f04542896a40ff1180351471db
@lberki
Copy link
Contributor Author

lberki commented Sep 18, 2024

Manually merged pull request is #23653 .

github-merge-queue bot pushed a commit that referenced this issue Sep 18, 2024
#23653)

…nce.

This didn't work at HEAD because the check there compared the flags in
the new configuration with the flags of the old configuration, but
overwrote the old configuration with the new one, so running a command
that discarded the analysis cache was allowed the second time.

The fix is not as trivial as moving checking the value before setting
the new configuration because creating the new configuration entails a
Skyframe evaluation, which is a waste if we end up erroring out. So the
check is now done by checking the build options of the old configuration
and the new build options, which doesn't require creating the new
configuration first.

Fixes #23491.

RELNOTES: None.
PiperOrigin-RevId: 675555324
Change-Id: Ib402b699ed9ba9f04542896a40ff1180351471db
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.4.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.0rc1. Thanks!

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

No branches or pull requests

4 participants