Roll back using labels rather than PathFragments for skylark loads.
RELNOTES:
--
MOS_MIGRATED_REVID=103606693
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
index 1303867..a7cd395 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
@@ -14,8 +14,10 @@
package com.google.devtools.build.lib.skyframe;
-import com.google.devtools.build.lib.cmdline.Label;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Runtime;
@@ -30,95 +32,167 @@
import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/**
- * A SkyFunction for {@link ASTFileLookupValue}s. Given a Label referencing a Skylark file,
- * attempts to locate the file and load it as a syntax tree ({@link BuildFileAST}). If the file
- * (or the package containing it) doesn't exist, the function doesn't fail, but instead returns a
- * specific {@code NO_FILE} {@link ASTFileLookupValue}.
+ * A SkyFunction for {@link ASTFileLookupValue}s. Tries to locate a file and load it as a
+ * syntax tree and cache the resulting {@link BuildFileAST}. If the file doesn't exist
+ * the function doesn't fail but returns a specific NO_FILE ASTLookupValue.
*/
public class ASTFileLookupFunction implements SkyFunction {
+ private abstract static class FileLookupResult {
+ /** Returns whether the file lookup was successful. */
+ public abstract boolean lookupSuccessful();
+
+ /** If {@code lookupSuccessful()}, returns the {@link RootedPath} to the file. */
+ public abstract RootedPath rootedPath();
+
+ /** If {@code lookupSuccessful()}, returns the file's size, in bytes. */
+ public abstract long fileSize();
+
+ static FileLookupResult noFile() {
+ return UnsuccessfulFileResult.INSTANCE;
+ }
+
+ static FileLookupResult file(RootedPath rootedPath, long fileSize) {
+ return new SuccessfulFileResult(rootedPath, fileSize);
+ }
+
+ private static class SuccessfulFileResult extends FileLookupResult {
+ private final RootedPath rootedPath;
+ private final long fileSize;
+
+ private SuccessfulFileResult(RootedPath rootedPath, long fileSize) {
+ this.rootedPath = rootedPath;
+ this.fileSize = fileSize;
+ }
+
+ @Override
+ public boolean lookupSuccessful() {
+ return true;
+ }
+
+ @Override
+ public RootedPath rootedPath() {
+ return rootedPath;
+ }
+
+ @Override
+ public long fileSize() {
+ return fileSize;
+ }
+ }
+
+ private static class UnsuccessfulFileResult extends FileLookupResult {
+ private static final UnsuccessfulFileResult INSTANCE = new UnsuccessfulFileResult();
+ private UnsuccessfulFileResult() {
+ }
+
+ @Override
+ public boolean lookupSuccessful() {
+ return false;
+ }
+
+ @Override
+ public RootedPath rootedPath() {
+ throw new IllegalStateException("unsuccessful lookup");
+ }
+
+ @Override
+ public long fileSize() {
+ throw new IllegalStateException("unsuccessful lookup");
+ }
+ }
+ }
+
+ private final AtomicReference<PathPackageLocator> pkgLocator;
private final RuleClassProvider ruleClassProvider;
- public ASTFileLookupFunction(RuleClassProvider ruleClassProvider) {
+ public ASTFileLookupFunction(AtomicReference<PathPackageLocator> pkgLocator,
+ RuleClassProvider ruleClassProvider) {
+ this.pkgLocator = pkgLocator;
this.ruleClassProvider = ruleClassProvider;
}
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException,
InterruptedException {
- Label fileLabel = (Label) skyKey.argument();
- PathFragment filePathFragment = fileLabel.toPathFragment();
-
- /*
- * Determine whether the package designated by fileLabel exists.
- */
- SkyKey pkgSkyKey = PackageLookupValue.key(fileLabel.getPackageIdentifier());
- PackageLookupValue pkgLookupValue = null;
- pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
- if (pkgLookupValue == null) {
+ PackageIdentifier key = (PackageIdentifier) skyKey.argument();
+ PathFragment astFilePathFragment = key.getPackageFragment();
+ FileLookupResult lookupResult = getASTFile(env, key);
+ if (lookupResult == null) {
return null;
}
- if (!pkgLookupValue.packageExists()) {
- return ASTFileLookupValue.forBadPackage(fileLabel, pkgLookupValue.getErrorMsg());
+ if (!lookupResult.lookupSuccessful()) {
+ return ASTFileLookupValue.noFile();
}
-
- /*
- * Determine whether the file designated by fileLabel exists.
- */
- Path packageRoot = pkgLookupValue.getRoot();
- RootedPath rootedPath = RootedPath.toRootedPath(packageRoot, filePathFragment);
- SkyKey fileSkyKey = FileValue.key(rootedPath);
- FileValue fileValue = null;
- try {
- fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class,
- FileSymlinkException.class, InconsistentFilesystemException.class);
- } catch (IOException | FileSymlinkException e) {
- throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(
- e.getMessage()), Transience.PERSISTENT);
- } catch (InconsistentFilesystemException e) {
- throw new ASTLookupFunctionException(e, Transience.PERSISTENT);
- }
- if (fileValue == null) {
- return null;
- }
- if (!fileValue.isFile()) {
- return ASTFileLookupValue.forBadFile(fileLabel);
- }
-
- /*
- * Both the package and the file exist; load the file and parse it as an AST.
- */
BuildFileAST ast = null;
- Path path = rootedPath.asPath();
- // Skylark files end with bzl
- boolean parseAsSkylark = filePathFragment.getPathString().endsWith(".bzl");
+ Path path = lookupResult.rootedPath().asPath();
+ long fileSize = lookupResult.fileSize();
+ // Skylark files end with bzl.
+ boolean parseAsSkylark = astFilePathFragment.getPathString().endsWith(".bzl");
try {
- long astFileSize = fileValue.getSize();
if (parseAsSkylark) {
try (Mutability mutability = Mutability.create("validate")) {
- ast = BuildFileAST.parseSkylarkFile(path, astFileSize, env.getListener(),
- new ValidationEnvironment(
- ruleClassProvider.createSkylarkRuleClassEnvironment(
- mutability,
- env.getListener(),
- // the two below don't matter for extracting the ValidationEnvironment:
- /*astFileContentHashCode=*/null,
- /*importMap=*/null)
- .setupDynamic(Runtime.PKG_NAME, Runtime.NONE)));
+ ast = BuildFileAST.parseSkylarkFile(path, fileSize, env.getListener(),
+ new ValidationEnvironment(
+ ruleClassProvider.createSkylarkRuleClassEnvironment(
+ mutability,
+ env.getListener(),
+ // the two below don't matter for extracting the ValidationEnvironment:
+ /*astFileContentHashCode=*/null,
+ /*importMap=*/null)
+ .setupDynamic(Runtime.PKG_NAME, Runtime.NONE)));
}
} else {
- ast = BuildFileAST.parseBuildFile(path, astFileSize, env.getListener(), false);
+ ast = BuildFileAST.parseBuildFile(path, fileSize, env.getListener(), false);
}
} catch (IOException e) {
- throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(
- e.getMessage()), Transience.TRANSIENT);
+ throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(
+ e.getMessage()), Transience.TRANSIENT);
+ }
+ return ASTFileLookupValue.withFile(ast);
+ }
+
+ private FileLookupResult getASTFile(Environment env, PackageIdentifier key)
+ throws ASTLookupFunctionException {
+ List<Path> candidateRoots;
+ if (!key.getRepository().isDefault()) {
+ RepositoryValue repository =
+ (RepositoryValue) env.getValue(RepositoryValue.key(key.getRepository()));
+ if (repository == null) {
+ return null;
+ }
+
+ candidateRoots = ImmutableList.of(repository.getPath());
+ } else {
+ candidateRoots = pkgLocator.get().getPathEntries();
}
- return ASTFileLookupValue.withFile(ast);
+ for (Path root : candidateRoots) {
+ RootedPath rootedPath = RootedPath.toRootedPath(root, key.getPackageFragment());
+ FileValue fileValue;
+ try {
+ fileValue = (FileValue) env.getValueOrThrow(FileValue.key(rootedPath),
+ IOException.class, FileSymlinkException.class, InconsistentFilesystemException.class);
+ } catch (IOException | FileSymlinkException e) {
+ throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(
+ e.getMessage()), Transience.PERSISTENT);
+ } catch (InconsistentFilesystemException e) {
+ throw new ASTLookupFunctionException(e, Transience.PERSISTENT);
+ }
+ if (fileValue == null) {
+ return null;
+ }
+ if (fileValue.isFile()) {
+ return FileLookupResult.file(rootedPath, fileValue.getSize());
+ }
+ }
+ return FileLookupResult.noFile();
}
@Nullable
