From a2e5456ddb40377aa94c1bb32640b3e9d80e1917 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 13 Jul 2022 06:09:48 -0700 Subject: [PATCH] Automated rollback of commit 4ccc21f2f089971e5f4032397764a4be3549c40a. *** Reason for rollback *** https://github.com/bazelbuild/bazel/issues/15728 and https://github.com/bazelbuild/bazel/issues/14852 have been fixed *** Original change description *** Automated rollback of commit 5de967bb27ebdfd31bfa193dc2698cbe46600815. *** 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... *** PiperOrigin-RevId: 460696618 Change-Id: I60be60120aff003ebdb0d8d44ef82d412d81af92 --- .../devtools/build/lib/packages/Package.java | 14 ++---- .../build/lib/packages/PackageFactory.java | 7 ++- .../lib/skyframe/WorkspaceFileFunction.java | 44 ++++++++++++------- .../build/lib/bazel/repository/starlark/BUILD | 2 + .../StarlarkRepositoryContextTest.java | 2 + .../google/devtools/build/lib/packages/BUILD | 1 + .../build/lib/packages/RuleFactoryTest.java | 5 ++- .../packages/WorkspaceFactoryTestHelper.java | 3 ++ 8 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 983b44ebe7fc78..9064275fe4fa88 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -317,16 +317,7 @@ public ImmutableMap getRepositoryMapping(RepositoryName throw new UnsupportedOperationException("Can only access the external package repository" + "mappings from the //external package"); } - - // We are passed a repository name as seen from the main repository, not necessarily - // a canonical repository name. So, we first have to find the canonical name for the - // repository in question before we can look up the mapping for it. - RepositoryName actualRepositoryName = - externalPackageRepositoryMappings - .getOrDefault(RepositoryName.MAIN, ImmutableMap.of()) - .getOrDefault(repository.getName(), repository); - - return externalPackageRepositoryMappings.getOrDefault(actualRepositoryName, ImmutableMap.of()); + return externalPackageRepositoryMappings.getOrDefault(repository, ImmutableMap.of()); } /** Get the repository mapping for this package. */ @@ -874,13 +865,14 @@ public static Builder newExternalPackageBuilder( PackageSettings helper, RootedPath workspacePath, String workspaceName, + RepositoryMapping mainRepoMapping, StarlarkSemantics starlarkSemantics) { return new Builder( helper, LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER, workspaceName, starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), - RepositoryMapping.ALWAYS_FALLBACK) + mainRepoMapping) .setFilename(workspacePath); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 1d7df09db538d9..7f650855d7e202 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -449,9 +449,12 @@ public boolean isImmutable() { } public Package.Builder newExternalPackageBuilder( - RootedPath workspacePath, String workspaceName, StarlarkSemantics starlarkSemantics) { + RootedPath workspacePath, + String workspaceName, + RepositoryMapping mainRepoMapping, + StarlarkSemantics starlarkSemantics) { return Package.newExternalPackageBuilder( - packageSettings, workspacePath, workspaceName, starlarkSemantics); + packageSettings, workspacePath, workspaceName, mainRepoMapping, starlarkSemantics); } // This function is public only for the benefit of skyframe.PackageFunction, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index 7d7c3073426d54..dacbf512632f7a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; @@ -240,21 +241,6 @@ public SkyValue compute(SkyKey skyKey, Environment env) // -- start of historical WorkspaceFileFunction -- // TODO(adonovan): reorganize and simplify. - Package.Builder builder = - packageFactory.newExternalPackageBuilder( - workspaceFile, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics); - - if (chunks.isEmpty()) { - return new WorkspaceFileValue( - buildAndReportEvents(builder, env), - /* loadedModules = */ ImmutableMap.of(), - /* loadToChunkMap = */ ImmutableMap.of(), - /* bindings = */ ImmutableMap.of(), - workspaceFile, - /* idx = */ 0, // first fragment - /* hasNext = */ false); - } - // Get the state at the end of the previous chunk. WorkspaceFileValue prevValue = null; if (key.getIndex() > 0) { @@ -268,6 +254,31 @@ public SkyValue compute(SkyKey skyKey, Environment env) return prevValue; } } + RepositoryMapping repoMapping; + if (prevValue == null) { + repoMapping = RepositoryMapping.ALWAYS_FALLBACK; + } else { + repoMapping = + RepositoryMapping.createAllowingFallback( + prevValue + .getRepositoryMapping() + .getOrDefault(RepositoryName.MAIN, ImmutableMap.of())); + } + + Package.Builder builder = + packageFactory.newExternalPackageBuilder( + workspaceFile, ruleClassProvider.getRunfilesPrefix(), repoMapping, starlarkSemantics); + + if (chunks.isEmpty()) { + return new WorkspaceFileValue( + buildAndReportEvents(builder, env), + /* loadedModules = */ ImmutableMap.of(), + /* loadToChunkMap = */ ImmutableMap.of(), + /* bindings = */ ImmutableMap.of(), + workspaceFile, + /* idx = */ 0, // first fragment + /* hasNext = */ false); + } List chunk = chunks.get(key.getIndex()); @@ -275,8 +286,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) ImmutableList> programLoads = BzlLoadFunction.getLoadsFromStarlarkFiles(chunk); ImmutableList