Fix WORKSPACE file lookup to look at all package paths
--
MOS_MIGRATED_REVID=89713328
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 775deda..15025b8 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
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
+import com.google.devtools.build.lib.packages.ExternalPackage;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.PackageIdentifier;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -52,6 +53,8 @@
PackageIdentifier packageKey = (PackageIdentifier) skyKey.argument();
if (!packageKey.getRepository().isDefault()) {
return computeExternalPackageLookupValue(skyKey, env);
+ } else if (packageKey.getPackageFragment().equals(new PathFragment(ExternalPackage.NAME))) {
+ return computeWorkspaceLookupValue(env);
}
PathFragment pkg = packageKey.getPackageFragment();
String pkgName = pkg.getPathString();
@@ -84,18 +87,9 @@
return null;
}
- private PackageLookupValue getPackageLookupValue(Environment env, Path packagePathEntry,
- PathFragment pkgFragment) throws PackageLookupFunctionException {
- PathFragment buildFileFragment;
- boolean isWorkspace = false;
- if (pkgFragment.getPathString().equals(PackageFunction.EXTERNAL_PACKAGE_NAME)) {
- buildFileFragment = new PathFragment("WORKSPACE");
- isWorkspace = true;
- } else {
- buildFileFragment = pkgFragment.getChild("BUILD");
- }
- RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry,
- buildFileFragment);
+ @Nullable
+ private FileValue getFileValue(RootedPath buildFileRootedPath, Environment env)
+ throws PackageLookupFunctionException {
String basename = buildFileRootedPath.asPath().getBaseName();
SkyKey fileSkyKey = FileValue.key(buildFileRootedPath);
FileValue fileValue = null;
@@ -103,13 +97,13 @@
fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class,
FileSymlinkCycleException.class, InconsistentFilesystemException.class);
} catch (IOException e) {
- String pkgName = pkgFragment.getPathString();
+ String pkgName = buildFileRootedPath.getRelativePath().getParentDirectory().getPathString();
// TODO(bazel-team): throw an IOException here and let PackageFunction wrap that into a
// BuildFileNotFoundException.
throw new PackageLookupFunctionException(new BuildFileNotFoundException(pkgName,
"IO errors while looking for " + basename + " file reading "
+ buildFileRootedPath.asPath() + ": " + e.getMessage(), e),
- Transience.PERSISTENT);
+ Transience.PERSISTENT);
} catch (FileSymlinkCycleException e) {
String pkgName = buildFileRootedPath.asPath().getPathString();
throw new PackageLookupFunctionException(new BuildFileNotFoundException(pkgName,
@@ -120,10 +114,19 @@
// This error is not transient from the perspective of the PackageLookupFunction.
throw new PackageLookupFunctionException(e, Transience.PERSISTENT);
}
+ return fileValue;
+ }
+
+ private PackageLookupValue getPackageLookupValue(Environment env, Path packagePathEntry,
+ PathFragment pkgFragment) throws PackageLookupFunctionException {
+ PathFragment buildFileFragment = pkgFragment.getChild("BUILD");
+ RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry,
+ buildFileFragment);
+ FileValue fileValue = getFileValue(buildFileRootedPath, env);
if (fileValue == null) {
return null;
}
- if (fileValue.isFile() || isWorkspace) {
+ if (fileValue.isFile()) {
return PackageLookupValue.success(buildFileRootedPath.getRoot());
}
return PackageLookupValue.noBuildFile();
@@ -157,6 +160,31 @@
}
/**
+ * Look for a WORKSPACE file on each package path. If none is found, use the last package path
+ * and pretend it was found there.
+ */
+ private SkyValue computeWorkspaceLookupValue(Environment env)
+ throws PackageLookupFunctionException {
+ PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);
+ Path lastPackagePath = null;
+ for (Path packagePathEntry : pkgLocator.getPathEntries()) {
+ lastPackagePath = packagePathEntry;
+ RootedPath workspacePath = RootedPath.toRootedPath(
+ packagePathEntry, new PathFragment("WORKSPACE"));
+ FileValue value = getFileValue(workspacePath, env);
+ if (value == null) {
+ return null;
+ }
+ if (value.exists()) {
+ return PackageLookupValue.workspace(packagePathEntry);
+ }
+ }
+
+ // Fall back on the last package path entry if there were any and nothing else worked.
+ return PackageLookupValue.workspace(lastPackagePath);
+ }
+
+ /**
* Used to declare all the exception types that can be wrapped in the exception thrown by
* {@link PackageLookupFunction#compute}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
index c877d38..47405ee 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
@@ -25,7 +25,8 @@
*
* <p>Package lookups will always produce a value. On success, the {@code #getRoot} returns the
* package path root under which the package resides and the package's BUILD file is guaranteed to
- * exist; on failure, {@code #getErrorReason} and {@code #getErrorMsg} describe why the package
+ * exist (unless this is looking up a WORKSPACE file, in which case the underlying file may or may
+ * not exist. On failure, {@code #getErrorReason} and {@code #getErrorMsg} describe why the package
* doesn't exist.
*
* <p>Implementation detail: we use inheritance here to optimize for memory usage.
@@ -54,6 +55,10 @@
return new SuccessfulPackageLookupValue(root);
}
+ public static PackageLookupValue workspace(Path root) {
+ return new WorkspacePackageLookupValue(root);
+ }
+
public static PackageLookupValue noBuildFile() {
return NoBuildFilePackageLookupValue.INSTANCE;
}
@@ -70,6 +75,10 @@
return DeletedPackageLookupValue.INSTANCE;
}
+ public boolean isExternalPackage() {
+ return false;
+ }
+
/**
* For a successful package lookup, returns the root (package path entry) that the package
* resides in.
@@ -145,6 +154,27 @@
}
}
+ // TODO(kchodorow): fix these semantics. This class should not exist, WORKSPACE lookup should
+ // just return success/failure like a "normal" package.
+ private static class WorkspacePackageLookupValue extends SuccessfulPackageLookupValue {
+
+ private WorkspacePackageLookupValue(Path root) {
+ super(root);
+ }
+
+ // TODO(kchodorow): get rid of this, the semantics are wrong (successful package lookup should
+ // mean the package exists).
+ @Override
+ public boolean packageExists() {
+ return getRoot().exists();
+ }
+
+ @Override
+ public boolean isExternalPackage() {
+ return true;
+ }
+ }
+
private abstract static class UnsuccessfulPackageLookupValue extends PackageLookupValue {
@Override
