Skip to content

Commit 5de967b

Browse files
Wyveraldcopybara-github
Wyverald
authored and
copybara-github
committed
Use the proper main repo mapping where appropriate
Use the main repo mapping (instead of ALWAYS_FALLBACK) to a) process .bzl load labels in the WORKSPACE file, and b) to process any labels passed to repo rules in either the WORKSPACE file or WORKSPACE-loaded macros. This change only has a noticeable effect for when the workspace name (specified using the `workspace` directive in the WORKSPACE file) is used in labels as `@workspace_name//some:thing`. Before this change, such a label would actually point to `@workspace_name//some:thing`, which is a different target than `@//some:thing`, even though they're backed by the same source. This is because we insert an implicit `local_repository(name='workspace_name',path='.')` clause into the WORKSPACE file (see also #15657 for a similar issue when Bzlmod is enabled). This quirk can cause many subtle bugs, including toolchains being missing because `@workspace_name//:toolchain_type` and `@//:toolchain_type` are in fact distinct toolchain types! Closes #15666. Fixes #15667. PiperOrigin-RevId: 454644054 Change-Id: Icd3742cfdf85eb5cf05281dd043b02ddc6a4e3c1
1 parent 2454a4c commit 5de967b

9 files changed

Lines changed: 48 additions & 21 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.google.common.collect.ImmutableList;
1818
import com.google.devtools.build.lib.analysis.BlazeDirectories;
1919
import com.google.devtools.build.lib.cmdline.LabelConstants;
20+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
2021
import com.google.devtools.build.lib.events.ExtendedEventHandler;
2122
import com.google.devtools.build.lib.packages.NoSuchPackageException;
2223
import com.google.devtools.build.lib.packages.Package;
@@ -62,6 +63,7 @@ public static Rule createRule(
6263
Root.fromPath(directories.getWorkspace()),
6364
LabelConstants.MODULE_DOT_BAZEL_FILE_NAME),
6465
"dummy_name",
66+
RepositoryMapping.ALWAYS_FALLBACK,
6567
semantics);
6668
BuildLangTypedAttributeValuesMap attributeValues =
6769
new BuildLangTypedAttributeValuesMap(attributes);

‎src/main/java/com/google/devtools/build/lib/packages/Package.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,13 +843,14 @@ public static Builder newExternalPackageBuilder(
843843
PackageSettings helper,
844844
RootedPath workspacePath,
845845
String workspaceName,
846+
RepositoryMapping mainRepoMapping,
846847
StarlarkSemantics starlarkSemantics) {
847848
return new Builder(
848849
helper,
849850
LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
850851
workspaceName,
851852
starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
852-
RepositoryMapping.ALWAYS_FALLBACK)
853+
mainRepoMapping)
853854
.setFilename(workspacePath);
854855
}
855856

‎src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,12 @@ public boolean isImmutable() {
445445

446446
@VisibleForTesting // exposed to WorkspaceFileFunction and BzlmodRepoRuleFunction
447447
public Package.Builder newExternalPackageBuilder(
448-
RootedPath workspacePath, String workspaceName, StarlarkSemantics starlarkSemantics) {
448+
RootedPath workspacePath,
449+
String workspaceName,
450+
RepositoryMapping mainRepoMapping,
451+
StarlarkSemantics starlarkSemantics) {
449452
return Package.newExternalPackageBuilder(
450-
packageSettings, workspacePath, workspaceName, starlarkSemantics);
453+
packageSettings, workspacePath, workspaceName, mainRepoMapping, starlarkSemantics);
451454
}
452455

453456
// This function is public only for the benefit of skyframe.PackageFunction,

‎src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.devtools.build.lib.cmdline.LabelConstants;
3131
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
3232
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
33+
import com.google.devtools.build.lib.cmdline.RepositoryName;
3334
import com.google.devtools.build.lib.events.Event;
3435
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
3536
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -238,9 +239,33 @@ public SkyValue compute(SkyKey skyKey, Environment env)
238239
// -- start of historical WorkspaceFileFunction --
239240
// TODO(adonovan): reorganize and simplify.
240241

242+
// Get the state at the end of the previous chunk.
243+
WorkspaceFileValue prevValue = null;
244+
if (key.getIndex() > 0) {
245+
prevValue =
246+
(WorkspaceFileValue)
247+
env.getValue(WorkspaceFileValue.key(workspaceFile, key.getIndex() - 1));
248+
if (prevValue == null) {
249+
return null;
250+
}
251+
if (prevValue.next() == null) {
252+
return prevValue;
253+
}
254+
}
255+
RepositoryMapping repoMapping;
256+
if (prevValue == null) {
257+
repoMapping = RepositoryMapping.ALWAYS_FALLBACK;
258+
} else {
259+
repoMapping =
260+
RepositoryMapping.createAllowingFallback(
261+
prevValue
262+
.getRepositoryMapping()
263+
.getOrDefault(RepositoryName.MAIN, ImmutableMap.of()));
264+
}
265+
241266
Package.Builder builder =
242267
packageFactory.newExternalPackageBuilder(
243-
workspaceFile, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics);
268+
workspaceFile, ruleClassProvider.getRunfilesPrefix(), repoMapping, starlarkSemantics);
244269

245270
if (chunks.isEmpty()) {
246271
return new WorkspaceFileValue(
@@ -254,28 +279,13 @@ public SkyValue compute(SkyKey skyKey, Environment env)
254279
ImmutableMap.of());
255280
}
256281

257-
// Get the state at the end of the previous chunk.
258-
WorkspaceFileValue prevValue = null;
259-
if (key.getIndex() > 0) {
260-
prevValue =
261-
(WorkspaceFileValue)
262-
env.getValue(WorkspaceFileValue.key(workspaceFile, key.getIndex() - 1));
263-
if (prevValue == null) {
264-
return null;
265-
}
266-
if (prevValue.next() == null) {
267-
return prevValue;
268-
}
269-
}
270-
271282
List<StarlarkFile> chunk = chunks.get(key.getIndex());
272283

273284
// Parse the labels in the chunk's load statements.
274285
ImmutableList<Pair<String, Location>> programLoads =
275286
BzlLoadFunction.getLoadsFromStarlarkFiles(chunk);
276287
ImmutableList<Label> loadLabels =
277-
BzlLoadFunction.getLoadLabels(
278-
env.getListener(), programLoads, rootPackage, RepositoryMapping.ALWAYS_FALLBACK);
288+
BzlLoadFunction.getLoadLabels(env.getListener(), programLoads, rootPackage, repoMapping);
279289
if (loadLabels == null) {
280290
NoSuchPackageException e =
281291
PackageFunction.PackageFunctionException.builder()

‎src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ java_library(
2222
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
2323
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
2424
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
25+
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
2526
"//src/main/java/com/google/devtools/build/lib/events",
2627
"//src/main/java/com/google/devtools/build/lib/packages",
2728
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
@@ -68,6 +69,7 @@ java_test(
6869
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
6970
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
7071
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
72+
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
7173
"//src/main/java/com/google/devtools/build/lib/events",
7274
"//src/main/java/com/google/devtools/build/lib/packages",
7375
"//src/main/java/com/google/devtools/build/lib/packages/semantics",

‎src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.ImmutableSortedMap;
2727
import com.google.common.io.CharStreams;
2828
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
29+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
2930
import com.google.devtools.build.lib.events.ExtendedEventHandler;
3031
import com.google.devtools.build.lib.packages.Attribute;
3132
import com.google.devtools.build.lib.packages.Package;
@@ -139,6 +140,7 @@ protected void setUpContextForRule(
139140
DefaultPackageSettings.INSTANCE,
140141
RootedPath.toRootedPath(root, workspaceFile),
141142
"runfiles",
143+
RepositoryMapping.ALWAYS_FALLBACK,
142144
starlarkSemantics);
143145
ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class);
144146
Rule rule =

‎src/test/java/com/google/devtools/build/lib/packages/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ java_library(
2020
name = "PackageTestsUtil",
2121
srcs = ["WorkspaceFactoryTestHelper.java"],
2222
deps = [
23+
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
2324
"//src/main/java/com/google/devtools/build/lib/events",
2425
"//src/main/java/com/google/devtools/build/lib/packages",
2526
"//src/main/java/com/google/devtools/build/lib/vfs",

‎src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ public void testCreateWorkspaceRule() throws Exception {
123123
Path myPkgPath = scratch.resolve("/workspace/WORKSPACE");
124124
Package.Builder pkgBuilder =
125125
packageFactory.newExternalPackageBuilder(
126-
RootedPath.toRootedPath(root, myPkgPath), "TESTING", StarlarkSemantics.DEFAULT);
126+
RootedPath.toRootedPath(root, myPkgPath),
127+
"TESTING",
128+
RepositoryMapping.ALWAYS_FALLBACK,
129+
StarlarkSemantics.DEFAULT);
127130

128131
Map<String, Object> attributeValues = new HashMap<>();
129132
attributeValues.put("name", "foo");

‎src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.common.base.Joiner;
2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.ImmutableMap;
23+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
2324
import com.google.devtools.build.lib.events.Event;
2425
import com.google.devtools.build.lib.packages.Package.Builder.DefaultPackageSettings;
2526
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -35,6 +36,7 @@
3536
/** Parses a WORKSPACE file with the given content. */
3637
// TODO(adonovan): delete this junk class.
3738
final class WorkspaceFactoryTestHelper {
39+
3840
private final Root root;
3941
private Package.Builder builder;
4042
private StarlarkSemantics starlarkSemantics;
@@ -67,6 +69,7 @@ void parse(String... args) {
6769
DefaultPackageSettings.INSTANCE,
6870
RootedPath.toRootedPath(root, workspaceFilePath),
6971
"",
72+
RepositoryMapping.ALWAYS_FALLBACK,
7073
StarlarkSemantics.DEFAULT);
7174
WorkspaceFactory factory =
7275
new WorkspaceFactory(

0 commit comments

Comments
 (0)