diff options
8 files changed, 63 insertions, 75 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 9132208b0e..4ec65dd3b0 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 @@ -207,13 +207,9 @@ public class Package { * @precondition {@code name} must be a suffix of * {@code filename.getParentDirectory())}. */ - protected Package(PackageIdentifier packageId, String workspaceName) { - this.workspaceName = workspaceName; - if (workspaceName.equals(packageId.getRepository().strippedName())) { - this.packageIdentifier = PackageIdentifier.createInMainRepo(packageId.getPackageFragment()); - } else { - this.packageIdentifier = packageId; - } + protected Package(PackageIdentifier packageId, String runfilesPrefix) { + this.packageIdentifier = packageId; + this.workspaceName = runfilesPrefix; this.nameFragment = Canonicalizer.fragments().intern(packageId.getPackageFragment()); this.name = nameFragment.getPathString(); } @@ -304,6 +300,7 @@ public class Package { throw new IllegalArgumentException( "Invalid BUILD file name for package '" + packageIdentifier + "': " + filename); } + this.makeEnv = builder.makeEnv.build(); this.targets = ImmutableSortedKeyMap.copyOf(builder.targets); this.defaultVisibility = builder.defaultVisibility; 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 da4d3e3163..cab4a4f6d6 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 @@ -1667,7 +1667,7 @@ public final class PackageFactory { ImmutableList<Label> skylarkFileDependencies) throws InterruptedException { Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage( - packageId, workspaceName)); + packageId, ruleClassProvider.getRunfilesPrefix())); StoredEventHandler eventHandler = new StoredEventHandler(); try (Mutability mutability = Mutability.create("package %s", packageId)) { @@ -1687,7 +1687,8 @@ public final class PackageFactory { // "defaultVisibility" comes from the command line. Let's give the BUILD file a chance to // set default_visibility once, be reseting the PackageBuilder.defaultVisibilitySet flag. .setDefaultVisibilitySet(false) - .setSkylarkFileDependencies(skylarkFileDependencies); + .setSkylarkFileDependencies(skylarkFileDependencies) + .setWorkspaceName(workspaceName); Event.replayEventsOn(eventHandler, pastEvents); for (Postable post : pastPosts) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index f1c25777f6..ccb81e1201 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension; +import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; @@ -315,6 +316,25 @@ public class WorkspaceFactory { throw new EvalException(ast.getLocation(), errorMessage); } PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name); + Package.Builder builder = PackageFactory.getContext(env, ast).pkgBuilder; + RuleClass localRepositoryRuleClass = ruleFactory.getRuleClass("local_repository"); + RuleClass bindRuleClass = ruleFactory.getRuleClass("bind"); + Map<String, Object> kwargs = ImmutableMap.<String, Object>of( + "name", name, "path", "."); + try { + // This effectively adds a "local_repository(name = "<ws>", path = ".")" + // definition to the WORKSPACE file. + builder + .externalPackageData() + .createAndAddRepositoryRule( + builder, + localRepositoryRuleClass, + bindRuleClass, + kwargs, + ast); + } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) { + throw new EvalException(ast.getLocation(), e.getMessage()); + } return NONE; } }; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java index d264cea205..c0a115ad6c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; 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.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -295,20 +294,8 @@ public class PackageLookupFunction implements SkyFunction { private PackageLookupValue computeExternalPackageLookupValue( SkyKey skyKey, Environment env, PackageIdentifier packageIdentifier) throws PackageLookupFunctionException, InterruptedException { - // Check if this is the main repository. PackageIdentifier id = (PackageIdentifier) skyKey.argument(); - RepositoryName repositoryName = id.getRepository(); - WorkspaceNameValue workspaceNameValue = (WorkspaceNameValue) - env.getValue(WorkspaceNameValue.firstChunk()); - if (workspaceNameValue == null) { - return null; - } - if (repositoryName.strippedName().equals(workspaceNameValue.getName())) { - return (PackageLookupValue) env.getValue(PackageLookupValue.key( - PackageIdentifier.createInMainRepo(id.getPackageFragment()))); - } - // Otherwise look up the repository. - SkyKey repositoryKey = RepositoryValue.key(repositoryName); + SkyKey repositoryKey = RepositoryValue.key(id.getRepository()); RepositoryValue repositoryValue; try { repositoryValue = (RepositoryValue) env.getValueOrThrow( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java index 4582572c4a..9284f5c2e9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java @@ -14,11 +14,9 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -36,47 +34,18 @@ public class WorkspaceNameFunction implements SkyFunction { @Nullable public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException, WorkspaceNameFunctionException { - boolean firstChunk = (boolean) skyKey.argument(); - Package externalPackage = firstChunk ? firstChunk(env) : parseFullPackage(env); - if (externalPackage == null) { + SkyKey externalPackageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); + PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey); + if (externalPackageValue == null) { return null; } + Package externalPackage = externalPackageValue.getPackage(); if (externalPackage.containsErrors()) { - Event.replayEventsOn(env.getListener(), externalPackage.getEvents()); throw new WorkspaceNameFunctionException(); } return WorkspaceNameValue.withName(externalPackage.getWorkspaceName()); } - /** - * This just examines the first chunk of the WORKSPACE file to avoid circular dependencies and - * overeager invalidation during package loading. - */ - private Package firstChunk(Environment env) throws InterruptedException { - SkyKey packageLookupKey = PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); - PackageLookupValue packageLookupValue = (PackageLookupValue) env.getValue(packageLookupKey); - if (packageLookupValue == null) { - return null; - } - RootedPath workspacePath = packageLookupValue.getRootedPath(Label.EXTERNAL_PACKAGE_IDENTIFIER); - - SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath); - WorkspaceFileValue value = (WorkspaceFileValue) env.getValue(workspaceKey); - if (value == null) { - return null; - } - return value.getPackage(); - } - - private Package parseFullPackage(Environment env) throws InterruptedException { - SkyKey externalPackageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); - PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey); - if (externalPackageValue == null) { - return null; - } - return externalPackageValue.getPackage(); - } - @Override @Nullable public String extractTag(SkyKey skyKey) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java index 98fe8b2416..4da943b6ab 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java @@ -28,6 +28,9 @@ import java.util.Objects; * the WORKSPACE file. */ public class WorkspaceNameValue implements SkyValue { + private static final SkyKey KEY = + LegacySkyKey.create(SkyFunctions.WORKSPACE_NAME, DummyArgument.INSTANCE); + private final String workspaceName; private WorkspaceNameValue(String workspaceName) { @@ -41,21 +44,9 @@ public class WorkspaceNameValue implements SkyValue { return workspaceName; } - /** Returns the {@link SkyKey} for {@link WorkspaceNameValue}s. */ + /** Returns the (singleton) {@link SkyKey} for {@link WorkspaceNameValue}s. */ public static SkyKey key() { - return createKey(false); - } - - /** - * Returns the {@link SkyKey} for a less-aggressive {@link WorkspaceNameValue} query, when - * we don't want to parse the whole WORKSPACE file. - */ - public static SkyKey firstChunk() { - return createKey(true); - } - - private static SkyKey createKey(boolean firstChunk) { - return LegacySkyKey.create(SkyFunctions.WORKSPACE_NAME, firstChunk); + return KEY; } /** Returns a {@link WorkspaceNameValue} for a workspace with the given name. */ @@ -81,4 +72,28 @@ public class WorkspaceNameValue implements SkyValue { public String toString() { return String.format("WorkspaceNameValue[name=%s]", workspaceName); } + + /** Singleton class used as the {@link SkyKey#argument} for {@link WorkspaceNameValue#key}. */ + public static final class DummyArgument { + static final int HASHCODE = DummyArgument.class.getCanonicalName().hashCode(); + public static final DummyArgument INSTANCE = new DummyArgument(); + + private DummyArgument() { + } + + @Override + public boolean equals(Object obj) { + return obj instanceof DummyArgument; + } + + @Override + public int hashCode() { + return HASHCODE; + } + + @Override + public String toString() { + return "#"; + } + } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java index 35b08e1444..004987d7b8 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java @@ -297,7 +297,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase { invalidatePackages(); try { - getTarget("@git_repo//:whatever"); + getTarget("@//:git_repo"); fail(); } catch (AssertionError expected) { assertThat(expected) @@ -332,7 +332,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase { fail(); } catch (NoSuchPackageException e) { // This is expected - assertThat(e).hasMessageThat().contains("Package 'external' contains errors"); + assertThat(e).hasMessageThat().contains("Could not load //external package"); } assertContainsEvent("missing value for mandatory attribute 'path' in 'local_repository' rule"); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index f407c247c1..7e770ffe81 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -123,7 +123,6 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase { ruleClassProvider, scratch.getFileSystem()), directories)); - skyFunctions.put(SkyFunctions.WORKSPACE_NAME, new WorkspaceNameFunction()); skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()); skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction()); skyFunctions.put( |