Skip to content

Commit a5b6c30

Browse files
hvadehracopybara-github
authored andcommitted
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 bazel-contrib/rules_python#2387 PiperOrigin-RevId: 695696010 Change-Id: I136f2bec4e2c668d020f6cb1a7d3200f7523e5a0
1 parent 5b25f8c commit a5b6c30

File tree

3 files changed

+68
-2
lines changed

3 files changed

+68
-2
lines changed

src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,11 @@ public final int hashCode() {
541541
// These fields are used to generate all other private fields.
542542
// Thus, other fields don't need to be included in hash code.
543543
return Objects.hash(
544-
autoloadedSymbols, removedSymbols, partiallyRemovedSymbols, reposDisallowingAutoloads);
544+
bzlmodEnabled,
545+
autoloadedSymbols,
546+
removedSymbols,
547+
partiallyRemovedSymbols,
548+
reposDisallowingAutoloads);
545549
}
546550

547551
@Override
@@ -553,7 +557,8 @@ public final boolean equals(Object that) {
553557
AutoloadSymbols other = (AutoloadSymbols) that;
554558
// These fields are used to generate all other private fields.
555559
// Thus, other fields don't need to be included in comparison.
556-
return this.autoloadedSymbols.equals(other.autoloadedSymbols)
560+
return this.bzlmodEnabled == other.bzlmodEnabled
561+
&& this.autoloadedSymbols.equals(other.autoloadedSymbols)
557562
&& this.removedSymbols.equals(other.removedSymbols)
558563
&& this.partiallyRemovedSymbols.equals(other.partiallyRemovedSymbols)
559564
&& this.reposDisallowingAutoloads.equals(other.reposDisallowingAutoloads);
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright 2024 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.bazel.rules;
15+
16+
import static com.google.common.truth.Truth.assertThat;
17+
18+
import com.google.common.collect.ImmutableList;
19+
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
20+
import com.google.devtools.build.lib.packages.AutoloadSymbols;
21+
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
22+
import com.google.devtools.build.skyframe.EvaluationContext;
23+
import com.google.devtools.build.skyframe.SkyKey;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.junit.runners.JUnit4;
27+
28+
@RunWith(JUnit4.class)
29+
public class AutoloadSymbolsTest extends BuildViewTestCase {
30+
31+
private static final SkyKey AUTOLOAD_SYMBOLS_KEY = AutoloadSymbols.AUTOLOAD_SYMBOLS.getKey();
32+
private static final ImmutableList<SkyKey> KEYS_TO_EVALUATE =
33+
ImmutableList.of(AUTOLOAD_SYMBOLS_KEY);
34+
35+
@Test
36+
public void bzlmodFlagUpdatesAutoloadConfig() throws Exception {
37+
EvaluationContext context =
38+
EvaluationContext.newBuilder().setParallelism(1).setEventHandler(reporter).build();
39+
40+
setBuildLanguageOptions("--enable_bzlmod");
41+
AutoloadSymbols value1 = evaluateAutoloads(context);
42+
setBuildLanguageOptions("--noenable_bzlmod");
43+
AutoloadSymbols value2 = evaluateAutoloads(context);
44+
45+
assertThat(value1).isNotSameInstanceAs(value2);
46+
}
47+
48+
private AutoloadSymbols evaluateAutoloads(EvaluationContext context) throws InterruptedException {
49+
return (AutoloadSymbols)
50+
((PrecomputedValue)
51+
skyframeExecutor
52+
.getEvaluator()
53+
.evaluate(KEYS_TO_EVALUATE, context)
54+
.get(AUTOLOAD_SYMBOLS_KEY))
55+
.get();
56+
}
57+
}

src/test/java/com/google/devtools/build/lib/bazel/rules/BUILD

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,15 @@ java_library(
3232
"//src/main/java/com/google/devtools/build/lib/bazel/rules",
3333
"//src/main/java/com/google/devtools/build/lib/cmdline",
3434
"//src/main/java/com/google/devtools/build/lib/packages",
35+
"//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
3536
"//src/main/java/com/google/devtools/build/lib/rules:core_workspace_rules",
3637
"//src/main/java/com/google/devtools/build/lib/rules/config",
3738
"//src/main/java/com/google/devtools/build/lib/rules/core",
39+
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
3840
"//src/main/java/com/google/devtools/build/lib/util:os",
3941
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
42+
"//src/main/java/com/google/devtools/build/skyframe",
43+
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
4044
"//src/main/java/com/google/devtools/common/options",
4145
"//src/test/java/com/google/devtools/build/lib/analysis/util",
4246
"//third_party:guava",

0 commit comments

Comments
 (0)