From 04e8e25730017aa742bd7732a63ae361ea0c7147 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 12 Nov 2024 05:38:59 -0800 Subject: [PATCH] Fix Autoloads skyframe injection Before this, changing the value of `--enable_bzlmod` did not invalidate the existing skyframe precomputed value as the instances were considered the same/equal (see `src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java#pruneInjectedValues`) Fixes https://github.com/bazelbuild/rules_python/issues/2387 PiperOrigin-RevId: 695696010 Change-Id: I136f2bec4e2c668d020f6cb1a7d3200f7523e5a0 --- .../build/lib/packages/AutoloadSymbols.java | 9 ++- .../lib/bazel/rules/AutoloadSymbolsTest.java | 57 +++++++++++++++++++ .../devtools/build/lib/bazel/rules/BUILD | 4 ++ 3 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/bazel/rules/AutoloadSymbolsTest.java diff --git a/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java b/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java index fe1d865476f0f6..0a1f562ad40bb5 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java @@ -526,7 +526,11 @@ public final int hashCode() { // These fields are used to generate all other private fields. // Thus, other fields don't need to be included in hash code. return Objects.hash( - autoloadedSymbols, removedSymbols, partiallyRemovedSymbols, reposDisallowingAutoloads); + bzlmodEnabled, + autoloadedSymbols, + removedSymbols, + partiallyRemovedSymbols, + reposDisallowingAutoloads); } @Override @@ -538,7 +542,8 @@ public final boolean equals(Object that) { AutoloadSymbols other = (AutoloadSymbols) that; // These fields are used to generate all other private fields. // Thus, other fields don't need to be included in comparison. - return this.autoloadedSymbols.equals(other.autoloadedSymbols) + return this.bzlmodEnabled == other.bzlmodEnabled + && this.autoloadedSymbols.equals(other.autoloadedSymbols) && this.removedSymbols.equals(other.removedSymbols) && this.partiallyRemovedSymbols.equals(other.partiallyRemovedSymbols) && this.reposDisallowingAutoloads.equals(other.reposDisallowingAutoloads); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/AutoloadSymbolsTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/AutoloadSymbolsTest.java new file mode 100644 index 00000000000000..e5ce0c5f951371 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/AutoloadSymbolsTest.java @@ -0,0 +1,57 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.bazel.rules; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.packages.AutoloadSymbols; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.skyframe.EvaluationContext; +import com.google.devtools.build.skyframe.SkyKey; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class AutoloadSymbolsTest extends BuildViewTestCase { + + private static final SkyKey AUTOLOAD_SYMBOLS_KEY = AutoloadSymbols.AUTOLOAD_SYMBOLS.getKey(); + private static final ImmutableList KEYS_TO_EVALUATE = + ImmutableList.of(AUTOLOAD_SYMBOLS_KEY); + + @Test + public void bzlmodFlagUpdatesAutoloadConfig() throws Exception { + EvaluationContext context = + EvaluationContext.newBuilder().setParallelism(1).setEventHandler(reporter).build(); + + setBuildLanguageOptions("--enable_bzlmod"); + AutoloadSymbols value1 = evaluateAutoloads(context); + setBuildLanguageOptions("--noenable_bzlmod"); + AutoloadSymbols value2 = evaluateAutoloads(context); + + assertThat(value1).isNotSameInstanceAs(value2); + } + + private AutoloadSymbols evaluateAutoloads(EvaluationContext context) throws InterruptedException { + return (AutoloadSymbols) + ((PrecomputedValue) + skyframeExecutor + .getEvaluator() + .evaluate(KEYS_TO_EVALUATE, context) + .get(AUTOLOAD_SYMBOLS_KEY)) + .get(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/rules/BUILD index dfa6aa0882705b..e131932eb8f0da 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/BUILD @@ -32,11 +32,15 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/rules", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols", "//src/main/java/com/google/devtools/build/lib/rules:core_workspace_rules", "//src/main/java/com/google/devtools/build/lib/rules/config", "//src/main/java/com/google/devtools/build/lib/rules/core", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/com/google/devtools/common/options", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//third_party:guava",