aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar kchodorow <kchodorow@google.com>2017-07-11 22:22:22 +0200
committerGravatar László Csomor <laszlocsomor@google.com>2017-07-12 08:50:25 +0200
commit63570924746274c128e80d3a43558e0d3fad3051 (patch)
tree5e08769586efd9efd87a926d47bc9770e68f2c24 /src
parent3e36d7a88a39d3bb3442c6c2e67c6d83cf0ae0e4 (diff)
Automated rollback of commit 937350211dcd55a4714ec32ebbf33fffcc42cdf2.
*** Reason for rollback *** Broke the go rules (of course) See http://ci.bazel.io/job/rules_go/BAZEL_VERSION=HEAD,PLATFORM_NAME=linux-x86_64/1044/console. *** Original change description *** Resolve references to @main-repo//foo to //foo Bazel was creating an dummy external repository for @main-repo, which doesn't work with package paths and will cause conflicts once @main-repo//foo and //foo refer to the same path. This adds a "soft pull" option to WorkspaceNameFunction: it can either parse the entire WORKSPACE file to find the name or just the first section. That way PackageLookupFunction can find the repository name without causing a circular dependency. This should have no ch... *** PiperOrigin-RevId: 161572272
Diffstat (limited to 'src')
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/Package.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java5
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java20
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java15
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java39
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java43
-rw-r--r--src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java4
-rw-r--r--src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java1
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(