Skip to content

Commit 4ccc21f

Browse files
Wyveraldcopybara-github
Wyverald
authored and
copybara-github
committed
Automated rollback of commit 5de967b.
*** Reason for rollback *** Temporarily rolling back until #14852 is fixed. *** Original change description *** 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// *** PiperOrigin-RevId: 454899732 Change-Id: Ief9b90f981d3fdb551c53e56e9ffcfdfb9f61bd4
1 parent 333d579 commit 4ccc21f

8 files changed

Lines changed: 31 additions & 47 deletions

File tree

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,16 @@ public ImmutableMap<String, RepositoryName> getRepositoryMapping(RepositoryName
316316
throw new UnsupportedOperationException("Can only access the external package repository"
317317
+ "mappings from the //external package");
318318
}
319-
return externalPackageRepositoryMappings.getOrDefault(repository, ImmutableMap.of());
319+
320+
// We are passed a repository name as seen from the main repository, not necessarily
321+
// a canonical repository name. So, we first have to find the canonical name for the
322+
// repository in question before we can look up the mapping for it.
323+
RepositoryName actualRepositoryName =
324+
externalPackageRepositoryMappings
325+
.getOrDefault(RepositoryName.MAIN, ImmutableMap.of())
326+
.getOrDefault(repository.getName(), repository);
327+
328+
return externalPackageRepositoryMappings.getOrDefault(actualRepositoryName, ImmutableMap.of());
320329
}
321330

322331
/** Get the repository mapping for this package. */
@@ -864,14 +873,13 @@ public static Builder newExternalPackageBuilder(
864873
PackageSettings helper,
865874
RootedPath workspacePath,
866875
String workspaceName,
867-
RepositoryMapping mainRepoMapping,
868876
StarlarkSemantics starlarkSemantics) {
869877
return new Builder(
870878
helper,
871879
LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
872880
workspaceName,
873881
starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
874-
mainRepoMapping)
882+
RepositoryMapping.ALWAYS_FALLBACK)
875883
.setFilename(workspacePath);
876884
}
877885

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

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

446446
public Package.Builder newExternalPackageBuilder(
447-
RootedPath workspacePath,
448-
String workspaceName,
449-
RepositoryMapping mainRepoMapping,
450-
StarlarkSemantics starlarkSemantics) {
447+
RootedPath workspacePath, String workspaceName, StarlarkSemantics starlarkSemantics) {
451448
return Package.newExternalPackageBuilder(
452-
packageSettings, workspacePath, workspaceName, mainRepoMapping, starlarkSemantics);
449+
packageSettings, workspacePath, workspaceName, starlarkSemantics);
453450
}
454451

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

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

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
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;
3433
import com.google.devtools.build.lib.events.Event;
3534
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
3635
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -239,33 +238,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
239238
// -- start of historical WorkspaceFileFunction --
240239
// TODO(adonovan): reorganize and simplify.
241240

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-
266241
Package.Builder builder =
267242
packageFactory.newExternalPackageBuilder(
268-
workspaceFile, ruleClassProvider.getRunfilesPrefix(), repoMapping, starlarkSemantics);
243+
workspaceFile, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics);
269244

270245
if (chunks.isEmpty()) {
271246
return new WorkspaceFileValue(
@@ -279,13 +254,28 @@ public SkyValue compute(SkyKey skyKey, Environment env)
279254
ImmutableMap.of());
280255
}
281256

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+
282271
List<StarlarkFile> chunk = chunks.get(key.getIndex());
283272

284273
// Parse the labels in the chunk's load statements.
285274
ImmutableList<Pair<String, Location>> programLoads =
286275
BzlLoadFunction.getLoadsFromStarlarkFiles(chunk);
287276
ImmutableList<Label> loadLabels =
288-
BzlLoadFunction.getLoadLabels(env.getListener(), programLoads, rootPackage, repoMapping);
277+
BzlLoadFunction.getLoadLabels(
278+
env.getListener(), programLoads, rootPackage, RepositoryMapping.ALWAYS_FALLBACK);
289279
if (loadLabels == null) {
290280
NoSuchPackageException e =
291281
PackageFunction.PackageFunctionException.builder()

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ 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",
2625
"//src/main/java/com/google/devtools/build/lib/events",
2726
"//src/main/java/com/google/devtools/build/lib/packages",
2827
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
@@ -69,7 +68,6 @@ java_test(
6968
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
7069
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
7170
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
72-
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
7371
"//src/main/java/com/google/devtools/build/lib/events",
7472
"//src/main/java/com/google/devtools/build/lib/packages",
7573
"//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: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
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;
3029
import com.google.devtools.build.lib.events.ExtendedEventHandler;
3130
import com.google.devtools.build.lib.packages.Attribute;
3231
import com.google.devtools.build.lib.packages.Package;
@@ -140,7 +139,6 @@ protected void setUpContextForRule(
140139
DefaultPackageSettings.INSTANCE,
141140
RootedPath.toRootedPath(root, workspaceFile),
142141
"runfiles",
143-
RepositoryMapping.ALWAYS_FALLBACK,
144142
starlarkSemantics);
145143
ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class);
146144
Rule rule =

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

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

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

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

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

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
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;
2423
import com.google.devtools.build.lib.events.Event;
2524
import com.google.devtools.build.lib.packages.Package.Builder.DefaultPackageSettings;
2625
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -36,7 +35,6 @@
3635
/** Parses a WORKSPACE file with the given content. */
3736
// TODO(adonovan): delete this junk class.
3837
final class WorkspaceFactoryTestHelper {
39-
4038
private final Root root;
4139
private Package.Builder builder;
4240
private StarlarkSemantics starlarkSemantics;
@@ -69,7 +67,6 @@ void parse(String... args) {
6967
DefaultPackageSettings.INSTANCE,
7068
RootedPath.toRootedPath(root, workspaceFilePath),
7169
"",
72-
RepositoryMapping.ALWAYS_FALLBACK,
7370
StarlarkSemantics.DEFAULT);
7471
WorkspaceFactory factory =
7572
new WorkspaceFactory(

0 commit comments

Comments
 (0)