Use Labels, rather than PathFragments, to represent Skylark loads internally. The load location for a Skylark Aspect is specified via a PathFragment, for consistency with current non-Aspect Skylark loads.
This should be a semantics-preserving change for users. In a subsequent CL, I'll change the Skylark syntax to allow load statements to use labels as well as paths, with the goal of eventually deprecating the latter.
Also:
- Removed the hack for handling relative loads in the prelude file.
- Refactored some redundant functionality in PackageFunction and SkylarkImportLookupFunction for handling loads.
- Removed the ability to put the BUILD file for the package containing a Skylark file under a different package root than the Skylark file itself. This functionality isn't currently used and is inconsistent with Blaze's handling of the package path elsewhere.
- Added BUILD files to a number of tests that load Skylark files; this is consistent with the requirement that all Skylark files need to be part of some package.
- Changed the constants used to set the location of the prelude file from paths to labels.
--
MOS_MIGRATED_REVID=107741568
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 670380c..67ca431 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
@@ -31,8 +31,6 @@
*/
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;
@@ -41,10 +39,9 @@
/**
* Constructs an import statement.
*
- * <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.
+ * <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.
*/
LoadStatement(StringLiteral path, Map<Identifier, String> symbols) {
this.symbols = ImmutableMap.copyOf(symbols);
@@ -77,7 +74,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());
@@ -93,10 +90,6 @@
@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());
}
@@ -119,6 +112,11 @@
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) {
@@ -133,4 +131,8 @@
"load statements should never appear in method bodies and"
+ " should never be compiled in general");
}
+
+ public boolean isAbsolute() {
+ return getImportPath().isAbsolute();
+ }
}
