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/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index b46fea7..3b7ddd4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.ABSTRACT;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.TEST;
+import com.google.common.base.Preconditions;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -80,7 +81,7 @@
*/
public static class Builder implements RuleDefinitionEnvironment {
private final StringBuilder defaultWorkspaceFile = new StringBuilder();
- private Label preludeLabel;
+ private PathFragment preludePath;
private String runfilesPrefix;
private final List<ConfigurationFragmentFactory> configurationFragments = new ArrayList<>();
private final List<BuildInfoFactory> buildInfoFactories = new ArrayList<>();
@@ -104,14 +105,11 @@
defaultWorkspaceFile.append(contents);
}
- public Builder setPrelude(String preludeLabelString) {
- try {
- this.preludeLabel = Label.parseAbsolute(preludeLabelString);
- } catch (LabelSyntaxException e) {
- String errorMsg =
- String.format("Prelude label '%s' is not syntactically valid", preludeLabelString);
- throw new IllegalArgumentException(errorMsg);
- }
+ public Builder setPrelude(String workspaceRelativePath) {
+ PathFragment preludePathFragment = new PathFragment(workspaceRelativePath);
+ Preconditions.checkArgument(!preludePathFragment.isAbsolute());
+ Preconditions.checkArgument(preludePathFragment.isNormalized());
+ this.preludePath = preludePathFragment;
return this;
}
@@ -228,7 +226,7 @@
}
return new ConfiguredRuleClassProvider(
- preludeLabel,
+ preludePath,
runfilesPrefix,
ImmutableMap.copyOf(ruleClassMap),
ImmutableMap.copyOf(ruleDefinitionMap),
@@ -271,9 +269,9 @@
String defaultWorkspaceFile;
/**
- * Label for the prelude file.
+ * Workspace-relative path to the prelude.
*/
- private final Label preludeLabel;
+ private final PathFragment preludePath;
/**
* The default runfiles prefix.
@@ -317,7 +315,7 @@
private final Environment.Frame globals;
public ConfiguredRuleClassProvider(
- Label preludeLabel,
+ PathFragment preludePath,
String runfilesPrefix,
ImmutableMap<String, RuleClass> ruleClassMap,
ImmutableMap<String, Class<? extends RuleDefinition>> ruleDefinitionMap,
@@ -329,7 +327,7 @@
ConfigurationCollectionFactory configurationCollectionFactory,
PrerequisiteValidator prerequisiteValidator,
ImmutableMap<String, SkylarkType> skylarkAccessibleJavaClasses) {
- this.preludeLabel = preludeLabel;
+ this.preludePath = preludePath;
this.runfilesPrefix = runfilesPrefix;
this.ruleClassMap = ruleClassMap;
this.ruleDefinitionMap = ruleDefinitionMap;
@@ -348,8 +346,8 @@
}
@Override
- public Label getPreludeLabel() {
- return preludeLabel;
+ public PathFragment getPreludePath() {
+ return preludePath;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index df8d9c3..6151ee9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -227,7 +227,7 @@
.addBuildInfoFactory(new CppBuildInfo())
.addBuildInfoFactory(new ObjcBuildInfoFactory())
.setConfigurationCollectionFactory(new BazelConfigurationCollection())
- .setPrelude("//tools/build_rules:prelude_bazel")
+ .setPrelude("tools/build_rules/prelude_bazel")
.setRunfilesPrefix("")
.setPrerequisiteValidator(new BazelPrerequisiteValidator());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index 170180e..e2ce1ba 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.packages;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.Environment.Extension;
@@ -31,9 +30,9 @@
public interface RuleClassProvider {
/**
- * Label referencing the prelude file.
+ * Workspace relative path to the prelude file.
*/
- Label getPreludeLabel();
+ PathFragment getPreludePath();
/**
* The default runfiles prefix (may be overwritten by the WORKSPACE file).
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
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
index 29cc2e8..65fa3f0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
@@ -14,90 +14,48 @@
package com.google.devtools.build.lib.skyframe;
-import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import javax.annotation.Nullable;
+
/**
- * A value that represents an AST file lookup result. There are two subclasses: one for the
- * case where the file is found, and another for the case where the file is missing (but there
- * are no other errors).
+ * A value that represents an AST file lookup result.
*/
-public abstract class ASTFileLookupValue implements SkyValue {
- public abstract boolean lookupSuccessful();
- public abstract BuildFileAST getAST();
- public abstract String getErrorMsg();
-
- /*
- * If the file is found, this class encapsulates the parsed AST.
- */
- private static class ASTLookupWithFile extends ASTFileLookupValue {
- private final BuildFileAST ast;
+public class ASTFileLookupValue implements SkyValue {
- private ASTLookupWithFile(BuildFileAST ast) {
- Preconditions.checkNotNull(ast);
- this.ast = ast;
- }
+ private static final ASTFileLookupValue NO_FILE = new ASTFileLookupValue(null);
- @Override
- public boolean lookupSuccessful() {
- return true;
- }
+ @Nullable private final BuildFileAST ast;
- @Override
- public BuildFileAST getAST() {
- return this.ast;
- }
-
- @Override
- public String getErrorMsg() {
- throw new IllegalStateException("can't retrieve error for successful lookup");
- }
- }
-
- /*
- * If the file isn't found, this class encapsulates a message with the reason.
- */
- private static class ASTLookupNoFile extends ASTFileLookupValue {
- private final String errorMsg;
-
- private ASTLookupNoFile(String errorMsg) {
- this.errorMsg = Preconditions.checkNotNull(errorMsg);
- }
-
- @Override
- public boolean lookupSuccessful() {
- return false;
- }
-
- @Override
- public BuildFileAST getAST() {
- throw new IllegalStateException("can't retrieve AST for unsuccessful lookup");
- }
-
- @Override
- public String getErrorMsg() {
- return this.errorMsg;
- }
+ private ASTFileLookupValue(@Nullable BuildFileAST ast) {
+ this.ast = ast;
}
- static ASTFileLookupValue forBadPackage(Label fileLabel, String reason) {
- return new ASTLookupNoFile(
- String.format("Unable to load package for '%s': %s", fileLabel, reason));
+ public static ASTFileLookupValue noFile() {
+ return NO_FILE;
}
-
- static ASTFileLookupValue forBadFile(Label fileLabel) {
- return new ASTLookupNoFile(
- String.format("Unable to load file '%s': file doesn't exist or isn't a file", fileLabel));
- }
-
+
public static ASTFileLookupValue withFile(BuildFileAST ast) {
- return new ASTLookupWithFile(ast);
+ return new ASTFileLookupValue(ast);
}
- static SkyKey key(Label astFileLabel) {
- return new SkyKey(SkyFunctions.AST_FILE_LOOKUP, astFileLabel);
+ /**
+ * Returns the original AST file.
+ */
+ @Nullable public BuildFileAST getAST() {
+ return ast;
+ }
+
+ static SkyKey key(PackageIdentifier astFileIdentifier) {
+ return new SkyKey(SkyFunctions.AST_FILE_LOOKUP, astFileIdentifier);
+ }
+
+ static final class ASTLookupInputException extends Exception {
+ ASTLookupInputException(String msg) {
+ super(msg);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 10d06c4..5e461fb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -15,7 +15,6 @@
import com.google.common.base.Preconditions;
import com.google.common.cache.Cache;
-import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -23,8 +22,8 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
@@ -43,12 +42,12 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.LoadStatement;
import com.google.devtools.build.lib.syntax.ParserInputSource;
import com.google.devtools.build.lib.syntax.Statement;
import com.google.devtools.build.lib.util.Pair;
@@ -88,7 +87,8 @@
private final AtomicBoolean showLoadingProgress;
private final AtomicInteger numPackagesLoaded;
private final Profiler profiler = Profiler.instance();
- private final Label preludeLabel;
+
+ private final PathFragment preludePath;
static final String DEFAULTS_PACKAGE_NAME = "tools/defaults";
public static final String EXTERNAL_PACKAGE_NAME = "external";
@@ -101,9 +101,9 @@
this.reporter = reporter;
// Can be null in tests.
- this.preludeLabel = packageFactory == null
+ this.preludePath = packageFactory == null
? null
- : packageFactory.getRuleClassProvider().getPreludeLabel();
+ : packageFactory.getRuleClassProvider().getPreludePath();
this.packageFactory = packageFactory;
this.packageLocator = pkgLocator;
this.showLoadingProgress = showLoadingProgress;
@@ -432,21 +432,21 @@
return null;
}
- SkyKey astLookupKey = ASTFileLookupValue.key(preludeLabel);
ASTFileLookupValue astLookupValue = null;
+ SkyKey astLookupKey = ASTFileLookupValue.key(
+ PackageIdentifier.createInDefaultRepo(preludePath));
try {
astLookupValue = (ASTFileLookupValue) env.getValueOrThrow(astLookupKey,
ErrorReadingSkylarkExtensionException.class, InconsistentFilesystemException.class);
} catch (ErrorReadingSkylarkExtensionException | InconsistentFilesystemException e) {
- throw new PackageFunctionException(
- new BadPreludeFileException(packageId, e.getMessage()), Transience.PERSISTENT);
+ throw new PackageFunctionException(new BadPreludeFileException(packageId, e.getMessage()),
+ Transience.PERSISTENT);
}
if (astLookupValue == null) {
return null;
}
- List<Statement> preludeStatements =
- astLookupValue.lookupSuccessful()
- ? astLookupValue.getAST().getStatements() : ImmutableList.<Statement>of();
+ List<Statement> preludeStatements = astLookupValue.getAST() == null
+ ? ImmutableList.<Statement>of() : astLookupValue.getAST().getStatements();
// Load the BUILD file AST and handle Skylark dependencies. This way BUILD files are
// only loaded twice if there are unavailable Skylark or package dependencies or an
@@ -473,6 +473,7 @@
replacementContents,
packageId,
buildFilePath,
+ buildFileFragment,
defaultVisibility,
preludeStatements,
env);
@@ -526,6 +527,7 @@
@Nullable
private SkylarkImportResult discoverSkylarkImports(
Path buildFilePath,
+ PathFragment buildFileFragment,
PackageIdentifier packageId,
Environment env,
ParserInputSource inputSource,
@@ -545,7 +547,8 @@
ImmutableMap.<PathFragment, Extension>of(),
ImmutableList.<Label>of());
} else {
- importResult = fetchImportsFromBuildFile(buildFilePath, packageId, buildFileAST, env);
+ importResult =
+ fetchImportsFromBuildFile(buildFilePath, buildFileFragment, packageId, buildFileAST, env);
}
return importResult;
@@ -558,33 +561,33 @@
@Nullable
private SkylarkImportResult fetchImportsFromBuildFile(
Path buildFilePath,
+ PathFragment buildFileFragment,
PackageIdentifier packageId,
BuildFileAST buildFileAST,
Environment env)
throws PackageFunctionException {
- ImmutableCollection<LoadStatement> imports = buildFileAST.getImports();
+ ImmutableMap<Location, PathFragment> imports = buildFileAST.getImports();
Map<PathFragment, Extension> importMap = new HashMap<>();
ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
- Label buildFileLabel;
try {
- buildFileLabel = Label.create(packageId, "BUILD");
- } catch (LabelSyntaxException e) {
- // Shouldn't happen; the Label is well-formed by construction.
- throw new IllegalStateException(e);
- }
- try {
- for (LoadStatement loadStmt : imports) {
- Label importLabel =
- SkylarkImportLookupFunction.findLabelForLoadStatement(loadStmt, buildFileLabel, env);
- if (importLabel == null) {
- return null;
- }
- SkyKey importsLookupKey = SkylarkImportLookupValue.key(importLabel);
- SkylarkImportLookupValue importLookupValue = (SkylarkImportLookupValue) env.getValueOrThrow(
- importsLookupKey, SkylarkImportFailedException.class,
- InconsistentFilesystemException.class, BuildFileNotFoundException.class);
+ for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) {
+ PathFragment importFile = entry.getValue();
+ // HACK: The prelude sometimes contains load() statements, which need to be resolved
+ // relative to the prelude file. However, we don't have a good way to tell "this should come
+ // from the main repository" in a load() statement, and we don't have a good way to tell if
+ // a load() statement comes from the prelude, since we just prepend those statements before
+ // the actual BUILD file. So we use this evil .endsWith() statement to figure it out.
+ RepositoryName repository =
+ entry.getKey().getPath().endsWith(preludePath)
+ ? PackageIdentifier.DEFAULT_REPOSITORY_NAME : packageId.getRepository();
+ SkyKey importsLookupKey = SkylarkImportLookupValue.key(
+ repository, buildFileFragment, importFile);
+ SkylarkImportLookupValue importLookupValue = (SkylarkImportLookupValue)
+ env.getValueOrThrow(importsLookupKey, SkylarkImportFailedException.class,
+ InconsistentFilesystemException.class, ASTLookupInputException.class,
+ BuildFileNotFoundException.class);
if (importLookupValue != null) {
- importMap.put(loadStmt.getImportPath(), importLookupValue.getEnvironmentExtension());
+ importMap.put(importFile, importLookupValue.getEnvironmentExtension());
fileDependencies.add(importLookupValue.getDependency());
}
}
@@ -595,6 +598,10 @@
} catch (InconsistentFilesystemException e) {
throw new PackageFunctionException(
new InternalInconsistentFilesystemException(packageId, e), Transience.PERSISTENT);
+ } catch (ASTLookupInputException e) {
+ // The load syntax is bad in the BUILD file so BuildFileContainsErrorsException is OK.
+ throw new PackageFunctionException(
+ new BuildFileContainsErrorsException(packageId, e.getMessage()), Transience.PERSISTENT);
} catch (BuildFileNotFoundException e) {
throw new PackageFunctionException(e, Transience.PERSISTENT);
}
@@ -763,6 +770,7 @@
@Nullable String replacementContents,
PackageIdentifier packageId,
Path buildFilePath,
+ PathFragment buildFileFragment,
RuleVisibility defaultVisibility,
List<Statement> preludeStatements,
Environment env)
@@ -799,6 +807,7 @@
SkylarkImportResult importResult =
discoverSkylarkImports(
buildFilePath,
+ buildFileFragment,
packageId,
env,
preprocessingResult.result,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index ced2805..443a444 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -312,7 +312,7 @@
map.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction());
map.put(SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction(deletedPackages));
map.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction());
- map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(ruleClassProvider));
+ map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgLocator, ruleClassProvider));
map.put(SkyFunctions.SKYLARK_IMPORTS_LOOKUP, new SkylarkImportLookupFunction(
ruleClassProvider, pkgFactory));
map.put(SkyFunctions.GLOB, newGlobFunction());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index d918003..29318f3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -17,15 +17,17 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions;
+import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Environment.Extension;
-import com.google.devtools.build.lib.syntax.LoadStatement;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -37,8 +39,6 @@
import java.util.HashMap;
import java.util.Map;
-import javax.annotation.Nullable;
-
/**
* A Skyframe function to look up and import a single Skylark extension.
*/
@@ -56,117 +56,111 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException,
InterruptedException {
- Label fileLabel = (Label) skyKey.argument();
- PathFragment importPath = fileLabel.toPathFragment();
+ PackageIdentifier arg = (PackageIdentifier) skyKey.argument();
+ PathFragment file = arg.getPackageFragment();
ASTFileLookupValue astLookupValue = null;
try {
- SkyKey astLookupKey = ASTFileLookupValue.key(fileLabel);
+ SkyKey astLookupKey = ASTFileLookupValue.key(arg);
astLookupValue = (ASTFileLookupValue) env.getValueOrThrow(astLookupKey,
ErrorReadingSkylarkExtensionException.class, InconsistentFilesystemException.class);
} catch (ErrorReadingSkylarkExtensionException e) {
throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.errorReadingFile(
- importPath, e.getMessage()));
+ file, e.getMessage()));
} catch (InconsistentFilesystemException e) {
throw new SkylarkImportLookupFunctionException(e, Transience.PERSISTENT);
}
if (astLookupValue == null) {
return null;
}
- if (!astLookupValue.lookupSuccessful()) {
+ if (astLookupValue.getAST() == null) {
// Skylark import files have to exist.
- throw new SkylarkImportLookupFunctionException(
- SkylarkImportFailedException.noFile(astLookupValue.getErrorMsg()));
+ throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.noFile(file));
}
Map<PathFragment, Extension> importMap = new HashMap<>();
ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
BuildFileAST ast = astLookupValue.getAST();
- for (LoadStatement loadStmt : ast.getImports()) {
+ // TODO(bazel-team): Refactor this code and PackageFunction to reduce code duplications.
+ for (Map.Entry<Location, PathFragment> entry : ast.getImports().entrySet()) {
try {
- Label importLabel = findLabelForLoadStatement(loadStmt, fileLabel, env);
- if (importLabel == null) {
- return null;
- }
- SkyKey importsLookupKey = SkylarkImportLookupValue.key(importLabel);
+ PathFragment importFile = entry.getValue();
+ // HACK: The prelude sometimes contains load() statements, which need to be resolved
+ // relative to the prelude file. However, we don't have a good way to tell "this should come
+ // from the main repository" in a load() statement, and we don't have a good way to tell if
+ // a load() statement comes from the prelude, since we just prepend those statements before
+ // the actual BUILD file. So we use this evil .endsWith() statement to figure it out.
+ RepositoryName repository =
+ entry.getKey().getPath().endsWith(ruleClassProvider.getPreludePath())
+ ? PackageIdentifier.DEFAULT_REPOSITORY_NAME : arg.getRepository();
+ SkyKey importsLookupKey = SkylarkImportLookupValue.key(repository, file, importFile);
SkylarkImportLookupValue importsLookupValue;
- importsLookupValue = (SkylarkImportLookupValue) env.getValue(importsLookupKey);
+ importsLookupValue = (SkylarkImportLookupValue) env.getValueOrThrow(
+ importsLookupKey, ASTLookupInputException.class);
if (importsLookupValue != null) {
- importMap.put(loadStmt.getImportPath(), importsLookupValue.getEnvironmentExtension());
+ importMap.put(importFile, importsLookupValue.getEnvironmentExtension());
fileDependencies.add(importsLookupValue.getDependency());
}
- } catch (SkylarkImportFailedException e) {
- throw new SkylarkImportLookupFunctionException(e);
+ } catch (ASTLookupInputException e) {
+ throw new SkylarkImportLookupFunctionException(e, Transience.PERSISTENT);
}
}
+ Label label = pathFragmentToLabel(arg.getRepository(), file, env);
if (env.valuesMissing()) {
// This means some imports are unavailable.
return null;
}
if (ast.containsErrors()) {
- throw new SkylarkImportLookupFunctionException(
- SkylarkImportFailedException.skylarkErrors(importPath));
+ throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.skylarkErrors(
+ file));
}
// Skylark UserDefinedFunction-s in that file will share this function definition Environment,
// which will be frozen by the time it is returned by createExtension.
Extension extension =
- createExtension(ast, importPath, importMap, env);
+ createExtension(ast, file, importMap, env);
return new SkylarkImportLookupValue(
- extension, new SkylarkFileDependency(fileLabel, fileDependencies.build()));
+ extension, new SkylarkFileDependency(label, fileDependencies.build()));
}
- /**
- * Given a Skylark {@link LoadStatement} and the {@link Label} of the file containing the load
- * statement, returns a canonical {@link Label} corresponding to the load path in the statement.
- * If no package can be found that contains the loaded file, throws
- * {@link SkylarkImportFailedException}. Returns null if Skyframe dependencies are unavailable.
- */
- @Nullable
- static Label findLabelForLoadStatement(LoadStatement loadStmt, Label containingFileLabel,
- Environment env)
- throws SkylarkImportFailedException {
- PathFragment importPath = loadStmt.getImportPath();
- PackageIdentifier pkgIdForImport;
- String targetNameForImport;
- if (loadStmt.isAbsolute()) {
- PathFragment relativeImportPath = importPath.toRelative();
- PackageIdentifier pkgToLookUp =
- PackageIdentifier.createInDefaultRepo(relativeImportPath.getParentDirectory());
- ContainingPackageLookupValue containingPackageLookupValue = null;
- try {
- containingPackageLookupValue = (ContainingPackageLookupValue) env.getValueOrThrow(
- ContainingPackageLookupValue.key(pkgToLookUp),
- BuildFileNotFoundException.class, InconsistentFilesystemException.class);
- } catch (BuildFileNotFoundException e) {
- // Thrown when there are IO errors looking for BUILD files.
- throw new SkylarkImportFailedException(e);
- } catch (InconsistentFilesystemException e) {
- throw new SkylarkImportFailedException(e);
- }
-
- if (containingPackageLookupValue == null) {
- return null;
- }
-
- if (!containingPackageLookupValue.hasContainingPackage()) {
- throw SkylarkImportFailedException.noBuildFile(importPath);
- }
- pkgIdForImport = containingPackageLookupValue.getContainingPackageName();
- PathFragment containingPkgPath = pkgIdForImport.getPackageFragment();
- targetNameForImport = relativeImportPath.relativeTo(containingPkgPath).toString();
- } else {
- // The load statement has a relative path
- pkgIdForImport = containingFileLabel.getPackageIdentifier();
- PathFragment containingDir =
- (new PathFragment(containingFileLabel.getName())).getParentDirectory();
- targetNameForImport = importPath.getRelative(containingDir).toString();
- }
+ /**
+ * Converts the PathFragment of the Skylark file to a Label using the BUILD file closest to the
+ * Skylark file in its directory hierarchy - finds the package to which the Skylark file belongs.
+ * Throws an exception if no such BUILD file exists.
+ */
+ private Label pathFragmentToLabel(RepositoryName repo, PathFragment file, Environment env)
+ throws SkylarkImportLookupFunctionException {
+ ContainingPackageLookupValue containingPackageLookupValue = null;
try {
- return Label.create(pkgIdForImport, targetNameForImport);
+ PackageIdentifier newPkgId = new PackageIdentifier(repo, file.getParentDirectory());
+ containingPackageLookupValue = (ContainingPackageLookupValue) env.getValueOrThrow(
+ ContainingPackageLookupValue.key(newPkgId),
+ BuildFileNotFoundException.class, InconsistentFilesystemException.class);
+ } catch (BuildFileNotFoundException e) {
+ // Thrown when there are IO errors looking for BUILD files.
+ throw new SkylarkImportLookupFunctionException(e, Transience.PERSISTENT);
+ } catch (InconsistentFilesystemException e) {
+ throw new SkylarkImportLookupFunctionException(e, Transience.PERSISTENT);
+ }
+
+ if (containingPackageLookupValue == null) {
+ return null;
+ }
+
+ if (!containingPackageLookupValue.hasContainingPackage()) {
+ throw new SkylarkImportLookupFunctionException(
+ SkylarkImportFailedException.noBuildFile(file));
+ }
+
+ PathFragment pkgName =
+ containingPackageLookupValue.getContainingPackageName().getPackageFragment();
+ PathFragment fileInPkg = file.relativeTo(pkgName);
+
+ try {
+ // This code relies on PackageIdentifier.RepositoryName.toString()
+ return Label.parseAbsolute(repo + "//" + pkgName.getPathString() + ":" + fileInPkg);
} catch (LabelSyntaxException e) {
- // Shouldn't happen; the Label is well-formed by construction.
throw new IllegalStateException(e);
}
}
@@ -211,14 +205,6 @@
super(errorMessage);
}
- private SkylarkImportFailedException(InconsistentFilesystemException e) {
- super(e.getMessage());
- }
-
- private SkylarkImportFailedException(BuildFileNotFoundException e) {
- super(e.getMessage());
- }
-
static SkylarkImportFailedException errors(PathFragment file) {
return new SkylarkImportFailedException(
String.format("Extension file '%s' has errors", file));
@@ -229,9 +215,9 @@
String.format("Encountered error while reading extension file '%s': %s", file, error));
}
- static SkylarkImportFailedException noFile(String reason) {
+ static SkylarkImportFailedException noFile(PathFragment file) {
return new SkylarkImportFailedException(
- String.format("Extension file not found. %s", reason));
+ String.format("Extension file not found: '%s'", file));
}
static SkylarkImportFailedException noBuildFile(PathFragment file) {
@@ -256,6 +242,11 @@
super(e, transience);
}
+ private SkylarkImportLookupFunctionException(ASTLookupInputException e,
+ Transience transience) {
+ super(e, transience);
+ }
+
private SkylarkImportLookupFunctionException(BuildFileNotFoundException e,
Transience transience) {
super(e, transience);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index e9d050d..0ac9fd4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -13,9 +13,14 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
+import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
import com.google.devtools.build.lib.syntax.Environment.Extension;
+import com.google.devtools.build.lib.syntax.LoadStatement;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -53,7 +58,38 @@
return dependency;
}
- static SkyKey key(Label importLabel) {
- return new SkyKey(SkyFunctions.SKYLARK_IMPORTS_LOOKUP, importLabel);
+ private static void checkInputArgument(PathFragment astFilePathFragment)
+ throws ASTLookupInputException {
+ if (astFilePathFragment.isAbsolute()) {
+ throw new ASTLookupInputException(String.format(
+ "Input file '%s' cannot be an absolute path.", astFilePathFragment));
+ }
+ }
+
+ @VisibleForTesting
+ static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
+ return key(pkgIdentifier.getRepository(), pkgIdentifier.getPackageFragment());
+ }
+
+ static SkyKey key(RepositoryName repo, PathFragment fromFile, PathFragment fileToImport)
+ throws ASTLookupInputException {
+ PathFragment computedPath;
+ if (fileToImport.isAbsolute()) {
+ computedPath = fileToImport.toRelative();
+ } else if (fileToImport.segmentCount() == 1) {
+ computedPath = fromFile.getParentDirectory().getRelative(fileToImport);
+ } else {
+ throw new ASTLookupInputException(String.format(LoadStatement.PATH_ERROR_MSG, fileToImport));
+ }
+ return key(repo, computedPath);
+ }
+
+ private static SkyKey key(RepositoryName repo, PathFragment fileToImport)
+ throws ASTLookupInputException {
+ // Skylark import lookup keys need to be valid AST file lookup keys.
+ checkInputArgument(fileToImport);
+ return new SkyKey(
+ SkyFunctions.SKYLARK_IMPORTS_LOOKUP,
+ new PackageIdentifier(repo, fileToImport));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
index 902234e..5043879 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
@@ -18,7 +18,7 @@
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.skyframe.CycleInfo;
@@ -58,7 +58,7 @@
new Function<SkyKey, String>() {
@Override
public String apply(SkyKey input) {
- return ((Label) input.argument()).toString();
+ return ((PackageIdentifier) input.argument()).toString();
}
});
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index 2faab99..e6d00ef 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -14,11 +14,13 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.hash.HashCode;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.List;
@@ -34,7 +36,7 @@
private final ImmutableList<Comment> comments;
- private ImmutableList<LoadStatement> loads;
+ private ImmutableMap<Location, PathFragment> loads;
/**
* Whether any errors were encountered during scanning or parsing.
@@ -59,13 +61,13 @@
setLocation(result.location);
}
- /** Collects all load statements */
- private ImmutableList<LoadStatement> fetchLoads(List<Statement> stmts) {
- ImmutableList.Builder<LoadStatement> loads = new ImmutableList.Builder<>();
+ /** Collects paths from all load statements */
+ private ImmutableMap<Location, PathFragment> fetchLoads(List<Statement> stmts) {
+ ImmutableMap.Builder<Location, PathFragment> loads = ImmutableMap.builder();
for (Statement stmt : stmts) {
if (stmt instanceof LoadStatement) {
LoadStatement imp = (LoadStatement) stmt;
- loads.add(imp);
+ loads.put(imp.getLocation(), imp.getImportPath());
}
}
return loads.build();
@@ -95,9 +97,9 @@
}
/**
- * Returns a list of loads in this BUILD file.
+ * Returns a set of loads in this BUILD file.
*/
- public synchronized ImmutableList<LoadStatement> getImports() {
+ public synchronized ImmutableMap<Location, PathFragment> getImports() {
if (loads == null) {
loads = fetchLoads(stmts);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
index e4ede56..fc6fa77 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
@@ -25,6 +25,8 @@
*/
public final class LoadStatement extends Statement {
+ public static final String PATH_ERROR_MSG = "Path '%s' is not valid. "
+ + "It should either start with a slash or refer to a file in the current directory.";
private final ImmutableMap<Identifier, String> symbols;
private final ImmutableList<Identifier> cachedSymbols; // to save time
private final PathFragment importPath;
@@ -33,9 +35,10 @@
/**
* Constructs an import statement.
*
- * <p>{@code symbols} maps a symbol to the original name under which it was defined in
- * the bzl file that should be loaded. If aliasing is used, the value differs from its key's
- * {@code symbol.getName()}. Otherwise, both values are identical.
+ * <p>Symbols maps a symbol to its original name under which it was defined in
+ * the bzl file that should be loaded.
+ * If aliasing is used, the value differs from it's key's symbol#getName().
+ * Otherwise, both values are identical.
*/
LoadStatement(StringLiteral path, Map<Identifier, String> symbols) {
this.symbols = ImmutableMap.copyOf(symbols);
@@ -68,7 +71,7 @@
getLocation(), "symbol '" + current + "' is private and cannot be imported");
}
// The key is the original name that was used to define the symbol
- // in the loaded bzl file.
+ // in the loaded bzl file
env.importSymbol(getImportPath(), current, entry.getValue());
} catch (Environment.NoSuchVariableException | Environment.LoadFailedException e) {
throw new EvalException(getLocation(), e.getMessage());
@@ -84,6 +87,10 @@
@Override
void validate(ValidationEnvironment env) throws EvalException {
validatePath();
+
+ if (!importPath.isAbsolute() && importPath.segmentCount() > 1) {
+ throw new EvalException(getLocation(), String.format(PATH_ERROR_MSG, importPath));
+ }
for (Identifier symbol : cachedSymbols) {
env.declare(symbol.getName(), getLocation());
}
@@ -106,19 +113,10 @@
error =
"First argument of load() is a path, not a label. "
+ "It should start with a single slash if it is an absolute path.";
- } else if (!importPath.isAbsolute() && importPath.segmentCount() > 1) {
- error = String.format(
- "Path '%s' is not valid. "
- + "It should either start with a slash or refer to a file in the current directory.",
- importPath);
}
if (error != null) {
throw new EvalException(getLocation(), error);
}
}
-
- public boolean isAbsolute() {
- return getImportPath().isAbsolute();
- }
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
index 30d668a..09d2c4c 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
@@ -22,7 +22,7 @@
* Base class of exceptions thrown by {@link SkyFunction#compute} on failure.
*
* SkyFunctions should declare a subclass {@code C} of {@link SkyFunctionException} whose
- * constructors forward fine-grained exception types (e.g. {@code IOException}) to
+ * constructors forward fine-grained exception types (e.g. {@link IOException}) to
* {@link SkyFunctionException}'s constructor, and they should also declare
* {@link SkyFunction#compute} to throw {@code C}. This way the type system checks that no
* unexpected exceptions are thrown by the {@link SkyFunction}.
