aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
diff options
context:
space:
mode:
authorGravatar Nathan Harmata <nharmata@google.com>2016-05-23 20:57:11 +0000
committerGravatar Yue Gan <yueg@google.com>2016-05-24 11:57:43 +0000
commit87a598065e9574c3d463b1edb5026540df64453f (patch)
treef069d7ce9ccdaff3ed500abf8e2a5fbc93db64ca /src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
parentb4f3769294e110e8054ed05c354f27af4a8effe3 (diff)
Use a non-side-effectful PackageLookupValue#packageExists implementation for //external package lookups, but keep the current (incorrect) semantics for unsuccessful //external package lookups.
Refactor some users of the old WorkspacePackageLookupValue. -- MOS_MIGRATED_REVID=123034174
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java')
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java110
1 files changed, 56 insertions, 54 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 83e992c1a3..2400de7ee0 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
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelValidator;
@@ -57,7 +58,7 @@ public class PackageLookupFunction implements SkyFunction {
if (!packageKey.getRepository().isMain()) {
return computeExternalPackageLookupValue(skyKey, env, packageKey);
} else if (packageKey.equals(Label.EXTERNAL_PACKAGE_IDENTIFIER)) {
- return computeWorkspaceLookupValue(env, packageKey);
+ return computeWorkspacePackageLookupValue(env, pkgLocator.getPathEntries());
}
String packageNameErrorMsg = LabelValidator.validatePackageName(
@@ -84,17 +85,8 @@ public class PackageLookupFunction implements SkyFunction {
}
}
- // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
- // to having restart the SkyFunction after every new dependency. However, if we try to batch
- // the missing value keys, more dependencies than necessary will be declared. This wart can be
- // fixed once we have nicer continuation support [skyframe-loading]
- for (Path packagePathEntry : pkgLocator.getPathEntries()) {
- PackageLookupValue value = getPackageLookupValue(env, packagePathEntry, packageKey);
- if (value == null || value.packageExists()) {
- return value;
- }
- }
- return PackageLookupValue.NO_BUILD_FILE_VALUE;
+ return getPackageLookupValue(env, pkgLocator.getPathEntries(), packageKey,
+ packageKey.getPackageFragment().getChild("BUILD"));
}
@Nullable
@@ -105,10 +97,10 @@ public class PackageLookupFunction implements SkyFunction {
@Nullable
private FileValue getFileValue(
- RootedPath buildFileRootedPath, Environment env, PackageIdentifier packageIdentifier)
+ RootedPath fileRootedPath, Environment env, PackageIdentifier packageIdentifier)
throws PackageLookupFunctionException {
- String basename = buildFileRootedPath.asPath().getBaseName();
- SkyKey fileSkyKey = FileValue.key(buildFileRootedPath);
+ String basename = fileRootedPath.asPath().getBaseName();
+ SkyKey fileSkyKey = FileValue.key(fileRootedPath);
FileValue fileValue = null;
try {
fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class,
@@ -118,12 +110,12 @@ public class PackageLookupFunction implements SkyFunction {
// BuildFileNotFoundException.
throw new PackageLookupFunctionException(new BuildFileNotFoundException(packageIdentifier,
"IO errors while looking for " + basename + " file reading "
- + buildFileRootedPath.asPath() + ": " + e.getMessage(), e),
+ + fileRootedPath.asPath() + ": " + e.getMessage(), e),
Transience.PERSISTENT);
} catch (FileSymlinkException e) {
throw new PackageLookupFunctionException(new BuildFileNotFoundException(packageIdentifier,
"Symlink cycle detected while trying to find " + basename + " file "
- + buildFileRootedPath.asPath()),
+ + fileRootedPath.asPath()),
Transience.PERSISTENT);
} catch (InconsistentFilesystemException e) {
// This error is not transient from the perspective of the PackageLookupFunction.
@@ -132,19 +124,55 @@ public class PackageLookupFunction implements SkyFunction {
return fileValue;
}
- private PackageLookupValue getPackageLookupValue(Environment env, Path packagePathEntry,
- PackageIdentifier packageIdentifier) throws PackageLookupFunctionException {
- PathFragment buildFileFragment = packageIdentifier.getPackageFragment().getChild("BUILD");
- RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry,
- buildFileFragment);
- FileValue fileValue = getFileValue(buildFileRootedPath, env, packageIdentifier);
- if (fileValue == null) {
+ private PackageLookupValue getPackageLookupValue(Environment env,
+ ImmutableList<Path> packagePathEntries, PackageIdentifier packageIdentifier,
+ PathFragment buildFileFragment) throws PackageLookupFunctionException {
+ // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
+ // to having restart the SkyFunction after every new dependency. However, if we try to batch
+ // the missing value keys, more dependencies than necessary will be declared. This wart can be
+ // fixed once we have nicer continuation support [skyframe-loading]
+ for (Path packagePathEntry : packagePathEntries) {
+ RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry,
+ buildFileFragment);
+ FileValue fileValue = getFileValue(buildFileRootedPath, env, packageIdentifier);
+ if (fileValue == null) {
+ return null;
+ }
+ if (fileValue.isFile()) {
+ return PackageLookupValue.success(buildFileRootedPath.getRoot());
+ }
+ }
+ return PackageLookupValue.NO_BUILD_FILE_VALUE;
+ }
+
+ private PackageLookupValue computeWorkspacePackageLookupValue(Environment env,
+ ImmutableList<Path> packagePathEntries)
+ throws PackageLookupFunctionException{
+ PackageLookupValue result = getPackageLookupValue(
+ env, packagePathEntries, Label.EXTERNAL_PACKAGE_IDENTIFIER, new PathFragment("WORKSPACE"));
+ if (result == null) {
return null;
}
- if (fileValue.isFile()) {
- return PackageLookupValue.success(buildFileRootedPath.getRoot());
+ if (result.packageExists()) {
+ return result;
}
- return PackageLookupValue.NO_BUILD_FILE_VALUE;
+ // Fall back on the last package path entry if there were any and nothing else worked.
+ // TODO(kchodorow): get rid of this, the semantics are wrong (successful package lookup should
+ // mean the package exists). a bunch of tests need to be rewritten first though.
+ if (packagePathEntries.isEmpty()) {
+ return PackageLookupValue.NO_BUILD_FILE_VALUE;
+ }
+ Path lastPackagePath = packagePathEntries.get(packagePathEntries.size() - 1);
+ FileValue lastPackagePackagePathFileValue = getFileValue(
+ RootedPath.toRootedPath(lastPackagePath, PathFragment.EMPTY_FRAGMENT),
+ env,
+ Label.EXTERNAL_PACKAGE_IDENTIFIER);
+ if (lastPackagePackagePathFileValue == null) {
+ return null;
+ }
+ return lastPackagePackagePathFileValue.exists()
+ ? PackageLookupValue.success(lastPackagePath)
+ : PackageLookupValue.NO_BUILD_FILE_VALUE;
}
/**
@@ -178,39 +206,13 @@ public class PackageLookupFunction implements SkyFunction {
}
if (fileValue.isFile()) {
- return PackageLookupValue.success(repositoryValue.getPath());
+ return PackageLookupValue.success(repositoryValue.getPath());
}
return PackageLookupValue.NO_BUILD_FILE_VALUE;
}
/**
- * 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, PackageIdentifier packageIdentifier)
- 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, Label.EXTERNAL_PACKAGE_FILE_NAME);
- FileValue value = getFileValue(workspacePath, env, packageIdentifier);
- 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}.
*/