diff options
author | Kristina Chodorow <kchodorow@google.com> | 2015-03-27 18:41:33 +0000 |
---|---|---|
committer | Ulf Adams <ulfjack@google.com> | 2015-03-30 12:19:33 +0000 |
commit | 3eeb6e6d9db99ed0bf0cc45d05be01a52e2ee4c4 (patch) | |
tree | 9a04f204cf8e40622e0711a055e697a3f179844d /src/main/java/com/google/devtools/build/lib/skyframe | |
parent | 267b0a1d34e73fdfa0a5fb6c5cc8f89a3700708b (diff) |
Fix WORKSPACE file lookup to look at all package paths
--
MOS_MIGRATED_REVID=89713328
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java | 58 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java | 32 |
2 files changed, 74 insertions, 16 deletions
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 775deda07b..15025b8f3d 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.common.collect.ImmutableSet; 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 @@ class PackageLookupFunction implements SkyFunction { 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 @@ class PackageLookupFunction implements SkyFunction { 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 @@ class PackageLookupFunction implements SkyFunction { 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 @@ class PackageLookupFunction implements SkyFunction { // 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 @@ class PackageLookupFunction implements SkyFunction { } /** + * 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 c877d38263..47405ee31a 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 @@ import com.google.devtools.build.skyframe.SkyValue; * * <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 @@ abstract class PackageLookupValue implements SkyValue { 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 @@ abstract class PackageLookupValue implements SkyValue { 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 @@ abstract class PackageLookupValue implements SkyValue { } } + // 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 |