From c3495858f097451e06044778bc806e94e06d6da7 Mon Sep 17 00:00:00 2001 From: dannark Date: Mon, 4 Jun 2018 13:53:22 -0700 Subject: Process 'repo_mapping' attribute from WORKSPACE rules. 'repo_mapping' is a way to remap references to repositories within an external repository by another name. This CL only adds the mappings to the Package object, but it does not actually enable the reassignments. Example usage (in a WORKSPACE file): local_repository( name = ?a?, path = ?../a?, repo_mapping = {?@x? : ?@y?} ) This change also creates a new SkyKey which represents the mappings. This is to prevent all packages from depending on the external package, and instead depending just on the mappings. i.e. a change to the WORKSPACE file that does not touch the mappings shouldn't cause a reload of the package. RELNOTES:None PiperOrigin-RevId: 199187963 --- .../devtools/build/lib/packages/Package.java | 123 +++++++++++++++++++++ .../build/lib/packages/PackageFactory.java | 12 +- .../build/lib/packages/WorkspaceFactory.java | 35 +++++- .../build/lib/skyframe/PackageFunction.java | 35 ++++-- .../lib/skyframe/RepositoryMappingFunction.java | 63 +++++++++++ .../build/lib/skyframe/RepositoryMappingValue.java | 110 ++++++++++++++++++ .../devtools/build/lib/skyframe/SkyFunctions.java | 2 + .../build/lib/skyframe/SkyframeExecutor.java | 1 + .../skyframe/packages/AbstractPackageLoader.java | 2 + 9 files changed, 367 insertions(+), 16 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java (limited to 'src/main/java/com') 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 7e15bd2b42..f0f79d4d7e 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 @@ -25,6 +25,7 @@ import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.collect.ImmutableSortedKeyMap; import com.google.devtools.build.lib.events.Event; @@ -180,6 +181,20 @@ public class Package { private License defaultLicense; private Set defaultDistributionSet; + /** + * The map from each repository to that repository's remappings map. + * This is only used in the //external package, it is an empty map for all other packages. + * For example, an entry of {"@foo" : {"@x", "@y"}} indicates that, within repository foo, + * "@x" should be remapped to "@y". + */ + private ImmutableMap> + externalPackageRepositoryMappings; + + /** + * The map of repository reassignments for BUILD packages. This will be empty for packages + * within the main workspace. + */ + private ImmutableMap repositoryMapping; /** * The names of the package() attributes that declare default values for rule @@ -224,6 +239,49 @@ public class Package { return packageIdentifier; } + /** + * Returns the repository mapping for the requested external repository. + * + * @throws UnsupportedOperationException if called from a package other than + * the //external package + */ + public ImmutableMap getRepositoryMapping( + RepositoryName repository) { + if (!isWorkspace()) { + throw new UnsupportedOperationException("Can only access the external package repository" + + "mappings from the //external package"); + } + return externalPackageRepositoryMappings.getOrDefault(repository, ImmutableMap.of()); + } + + /** + * Returns the workspace mappings for the repository with the given absolute name. + * + * @throws LabelSyntaxException if repository is not a valid {@link RepositoryName} + */ + public ImmutableMap getRepositoryMapping( + String repository) throws LabelSyntaxException { + RepositoryName repositoryName = RepositoryName.create(repository); + return getRepositoryMapping(repositoryName); + } + + /** + * Gets the global name for a repository within an external repository. + * + *

{@code localName} is a repository name reference found in a BUILD file within a repository + * external to the main workspace. This method returns the main workspace's global remapped name + * for {@code localName}. + */ + public RepositoryName getGlobalName(RepositoryName localName) { + RepositoryName globalname = repositoryMapping.get(localName); + return globalname != null ? globalname : localName; + } + + /** Returns whether we are in the WORKSPACE file or not. */ + public boolean isWorkspace() { + return getPackageIdentifier().equals(Label.EXTERNAL_PACKAGE_IDENTIFIER); + } + /** * Package initialization: part 2 of 3: sets this package's default header * strictness checking. @@ -335,6 +393,20 @@ public class Package { this.posts = ImmutableList.copyOf(builder.posts); this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms); this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains); + this.repositoryMapping = builder.repositoryMapping; + ImmutableMap.Builder> + repositoryMappingsBuilder = ImmutableMap.builder(); + if (!builder.externalPackageRepositoryMappings.isEmpty() && !builder.isWorkspace()) { + // 'repo_mapping' should only be used in the //external package, i.e. should only appear + // in WORKSPACE files. Currently, if someone tries to use 'repo_mapping' in a BUILD rule, they + // will get a "no such attribute" error. This check is to protect against a 'repo_mapping' + // attribute being added to a rule in the future. + throw new IllegalArgumentException( + "'repo_mapping' may only be used in the //external package"); + } + builder.externalPackageRepositoryMappings.forEach((k, v) -> + repositoryMappingsBuilder.put(k, ImmutableMap.copyOf(v))); + this.externalPackageRepositoryMappings = repositoryMappingsBuilder.build(); } /** @@ -729,6 +801,13 @@ public class Package { */ protected Package pkg; + // The map from each repository to that repository's remappings map. + // This is only used in the //external package, it is an empty map for all other packages. + private final HashMap> + externalPackageRepositoryMappings = new HashMap<>(); + // The map of repository reassignments for BUILD packages loaded within external repositories. + // It will be empty for packages within the main workspace. + private ImmutableMap repositoryMapping = ImmutableMap.of(); private Path filename = null; private Label buildFileLabel = null; private InputFile buildFile = null; @@ -801,6 +880,50 @@ public class Package { return pkg.getPackageIdentifier().equals(Label.EXTERNAL_PACKAGE_IDENTIFIER); } + /** + * Updates the externalPackageRepositoryMappings entry for {@code repoWithin}. Adds new + * entry from {@code localName} to {@code mappedName} in {@code repoWithin}'s map. + * + * @param repoWithin the RepositoryName within which the mapping should apply + * @param localName the RepositoryName that actually appears in the WORKSPACE and BUILD files + * in the {@code repoWithin} repository + * @param mappedName the RepositoryName by which localName should be referenced + */ + public Builder addRepositoryMappingEntry( + RepositoryName repoWithin, RepositoryName localName, RepositoryName mappedName) { + HashMap mapping = + externalPackageRepositoryMappings + .computeIfAbsent(repoWithin, (RepositoryName k) -> new HashMap<>()); + mapping.put(localName, mappedName); + return this; + } + + /** Adds all the mappings from a given {@link Package}. */ + public Builder addRepositoryMappings(Package aPackage) { + ImmutableMap> + repositoryMappings = aPackage.externalPackageRepositoryMappings; + for (Map.Entry> repositoryName : + repositoryMappings.entrySet()) { + for (Map.Entry repositoryNameRepositoryNameEntry : + repositoryName.getValue().entrySet()) { + addRepositoryMappingEntry( + repositoryName.getKey(), + repositoryNameRepositoryNameEntry.getKey(), + repositoryNameRepositoryNameEntry.getValue()); + } + } + return this; + } + + /** + * Sets the repository mapping for a regular, BUILD file package (i.e. not the //external + * package) + */ + Builder setRepositoryMapping(ImmutableMap repositoryMapping) { + this.repositoryMapping = repositoryMapping; + return this; + } + /** * Sets the name of this package's BUILD file. */ 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 71bf904666..d9da36f945 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 @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; @@ -1250,6 +1251,7 @@ public final class PackageFactory { new AstParseResult(buildFileAST, localReporterForParsing); return createPackageFromAst( workspaceName, + /*repositoryMapping=*/ ImmutableMap.of(), packageId, buildFile, astParseResult, @@ -1274,6 +1276,7 @@ public final class PackageFactory { public Package.Builder createPackageFromAst( String workspaceName, + ImmutableMap repositoryMapping, PackageIdentifier packageId, Path buildFile, AstParseResult astParseResult, @@ -1297,7 +1300,8 @@ public final class PackageFactory { defaultVisibility, skylarkSemantics, imports, - skylarkFileDependencies); + skylarkFileDependencies, + repositoryMapping); } catch (InterruptedException e) { globber.onInterrupt(); throw e; @@ -1599,7 +1603,8 @@ public final class PackageFactory { RuleVisibility defaultVisibility, SkylarkSemantics skylarkSemantics, Map imports, - ImmutableList

Given the following rule: + * 

{@code
+ * local_repository(
+ *   name = “a”,
+ *   path = “../a”,
+ *   repo_mapping = {“@x” : “@y”}
+ * )
+ * }
+ * + *

The SkyKey would be {@code "@a"} and the SkyValue would be the map {@code {"@x" : "@y"}} + * + *

This is kept as a separate value with trivial change pruning so as to not necessitate a + * dependency from every {@link PackageValue} to the //external {@link PackageValue}, so that + * changes to things in the WORKSPACE other than the mappings (and name) won't require reloading + * all packages. If the mappings are changed then the external packages need to be reloaded. + */ +public class RepositoryMappingValue implements SkyValue { + + private final ImmutableMap repositoryMapping; + + private RepositoryMappingValue(ImmutableMap repositoryMapping) { + this.repositoryMapping = repositoryMapping; + } + + /** Returns the workspace mappings. */ + public ImmutableMap getRepositoryMapping() { + return repositoryMapping; + } + + /** Returns the {@link Key} for {@link RepositoryMappingValue}s. */ + public static Key key(RepositoryName repositoryName) { + return RepositoryMappingValue.Key.create(repositoryName); + } + + /** Returns a {@link RepositoryMappingValue} for a workspace with the given name. */ + public static RepositoryMappingValue withMapping( + ImmutableMap repositoryMapping) { + return new RepositoryMappingValue(Preconditions.checkNotNull(repositoryMapping)); + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof RepositoryMappingValue)) { + return false; + } + RepositoryMappingValue other = (RepositoryMappingValue) o; + return Objects.equals(repositoryMapping, other.repositoryMapping); + } + + @Override + public int hashCode() { + return Objects.hash(repositoryMapping); + } + + /** {@link SkyKey} for {@link RepositoryMappingValue}. */ + @AutoCodec.VisibleForSerialization + @AutoCodec + static class Key extends AbstractSkyKey { + + private static final Interner interner = BlazeInterners.newWeakInterner(); + + private Key(RepositoryName arg) { + super(arg); + } + + @AutoCodec.VisibleForSerialization + @AutoCodec.Instantiator + static Key create(RepositoryName arg) { + return interner.intern(new Key(arg)); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.REPOSITORY_MAPPING; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 4d347e8783..7b9ca6d81f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -114,6 +114,8 @@ public final class SkyFunctions { SkyFunctionName.create("REGISTERED_TOOLCHAINS"); public static final SkyFunctionName TOOLCHAIN_RESOLUTION = SkyFunctionName.create("TOOLCHAIN_RESOLUTION"); + public static final SkyFunctionName REPOSITORY_MAPPING = + SkyFunctionName.create("REPOSITORY_MAPPING"); public static Predicate isSkyFunction(final SkyFunctionName functionName) { return new Predicate() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index c0bb40456d..906924fa9f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -530,6 +530,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { SkyFunctions.REGISTERED_EXECUTION_PLATFORMS, new RegisteredExecutionPlatformsFunction()); map.put(SkyFunctions.REGISTERED_TOOLCHAINS, new RegisteredToolchainsFunction()); map.put(SkyFunctions.TOOLCHAIN_RESOLUTION, new ToolchainResolutionFunction()); + map.put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction()); map.putAll(extraSkyFunctions); return map.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 7ca43d003b..d8f4cb8b83 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -61,6 +61,7 @@ import com.google.devtools.build.lib.skyframe.PackageValue; import com.google.devtools.build.lib.skyframe.PerBuildSyscallCache; import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction; import com.google.devtools.build.lib.skyframe.WorkspaceASTFunction; @@ -415,6 +416,7 @@ public abstract class AbstractPackageLoader implements PackageLoader { SkyFunctions.WORKSPACE_FILE, new WorkspaceFileFunction(ruleClassProvider, pkgFactory, directories)) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()) + .put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction()) .put( SkyFunctions.PACKAGE, new PackageFunction( -- cgit v1.2.3