diff options
20 files changed, 531 insertions, 423 deletions
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 d061dea901..ab0a534622 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 @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.SkylarkType; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionsClassProvider; import java.lang.reflect.Constructor; @@ -484,7 +483,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { Environment.Frame globals, EventHandler eventHandler, String astFileContentHashCode, - Map<PathFragment, Extension> importMap) { + Map<String, Extension> importMap) { Environment env = Environment.builder(mutability) .setSkylark() .setGlobals(globals) @@ -501,7 +500,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { Mutability mutability, EventHandler eventHandler, String astFileContentHashCode, - Map<PathFragment, Extension> importMap) { + Map<String, Extension> importMap) { return createSkylarkRuleClassEnvironment( mutability, globals, eventHandler, astFileContentHashCode, importMap); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index b81dfdf2ed..436a631027 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -62,7 +62,6 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.UnixGlob; import java.io.IOException; @@ -998,7 +997,7 @@ public final class PackageFactory { Path buildFile, Preprocessor.Result preprocessingResult, List<Statement> preludeStatements, - Map<PathFragment, Extension> imports, + Map<String, Extension> imports, ImmutableList<Label> skylarkFileDependencies, RuleVisibility defaultVisibility, Globber globber) throws InterruptedException { @@ -1035,7 +1034,7 @@ public final class PackageFactory { PackageIdentifier packageId, Path buildFile, Preprocessor.AstAfterPreprocessing astAfterPreprocessing, - Map<PathFragment, Extension> imports, + Map<String, Extension> imports, ImmutableList<Label> skylarkFileDependencies, RuleVisibility defaultVisibility, Globber globber) throws InterruptedException { @@ -1120,7 +1119,7 @@ public final class PackageFactory { buildFile, preprocessingResult, /*preludeStatements=*/ImmutableList.<Statement>of(), - /*imports=*/ImmutableMap.<PathFragment, Extension>of(), + /*imports=*/ImmutableMap.<String, Extension>of(), /*skylarkFileDependencies=*/ImmutableList.<Label>of(), /*defaultVisibility=*/ConstantRuleVisibility.PUBLIC, globber) @@ -1307,7 +1306,7 @@ public final class PackageFactory { RuleVisibility defaultVisibility, boolean containsError, MakeEnvironment.Builder pkgMakeEnv, - Map<PathFragment, Extension> imports, + Map<String, Extension> imports, ImmutableList<Label> skylarkFileDependencies) throws InterruptedException { Package.LegacyBuilder pkgBuilder = new Package.LegacyBuilder( 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 55aa950d2d..f6918cbb69 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 @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFact import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Mutability; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Map; @@ -59,7 +58,7 @@ public interface RuleClassProvider { Mutability mutability, EventHandler eventHandler, @Nullable String astFileContentHashCode, - @Nullable Map<PathFragment, Extension> importMap); + @Nullable Map<String, Extension> importMap); /** * Returns a map from aspect names to aspect factory objects. diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 4a089c33f1..55f9b0ab39 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import java.io.File; import java.util.Map; @@ -153,7 +152,7 @@ public class WorkspaceFactory { return buildFileAST; } - public void setImportedExtensions(Map<PathFragment, Extension> importedExtensions) { + public void setImportedExtensions(Map<String, Extension> importedExtensions) { environmentBuilder.setImportedExtensions(importedExtensions); } 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 0307f728c9..62e4cf0664 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 @@ -48,8 +48,8 @@ import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.Skylar 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.SkylarkImport; import com.google.devtools.build.lib.syntax.Statement; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; @@ -535,7 +535,7 @@ public class PackageFunction implements SkyFunction { if (astAfterPreprocessing.containsAstParsingErrors) { importResult = new SkylarkImportResult( - ImmutableMap.<PathFragment, Extension>of(), + ImmutableMap.<String, Extension>of(), ImmutableList.<Label>of()); } else { importResult = @@ -562,10 +562,10 @@ public class PackageFunction implements SkyFunction { Environment env, SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining) throws PackageFunctionException, InterruptedException { - ImmutableList<LoadStatement> loadStmts = buildFileAST.getImports(); - Map<PathFragment, Extension> importMap = Maps.newHashMapWithExpectedSize(loadStmts.size()); + ImmutableList<SkylarkImport> imports = buildFileAST.getImports(); + Map<String, Extension> importMap = Maps.newHashMapWithExpectedSize(imports.size()); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); - ImmutableMap<PathFragment, Label> importPathMap; + ImmutableMap<String, Label> importPathMap; // Find the labels corresponding to the load statements. Label labelForCurrBuildFile; @@ -577,12 +577,11 @@ public class PackageFunction implements SkyFunction { } try { importPathMap = SkylarkImportLookupFunction.findLabelsForLoadStatements( - loadStmts, labelForCurrBuildFile, env); + imports, labelForCurrBuildFile, env); if (importPathMap == null) { return null; } } catch (SkylarkImportFailedException e) { - env.getListener().handle(Event.error(Location.fromFile(buildFilePath), e.getMessage())); throw new PackageFunctionException( new BuildFileContainsErrorsException(packageId, e.getMessage()), Transience.PERSISTENT); } @@ -635,7 +634,6 @@ public class PackageFunction implements SkyFunction { } } catch (SkylarkImportFailedException e) { - env.getListener().handle(Event.error(Location.fromFile(buildFilePath), e.getMessage())); throw new PackageFunctionException( new BuildFileContainsErrorsException(packageId, e.getMessage()), Transience.PERSISTENT); } catch (InconsistentFilesystemException e) { @@ -649,13 +647,13 @@ public class PackageFunction implements SkyFunction { } // Process the loaded imports. - for (Entry<PathFragment, Label> importEntry : importPathMap.entrySet()) { - PathFragment importPath = importEntry.getKey(); + for (Entry<String, Label> importEntry : importPathMap.entrySet()) { + String importString = importEntry.getKey(); Label importLabel = importEntry.getValue(); SkyKey keyForLabel = SkylarkImportLookupValue.key(importLabel, inWorkspace); SkylarkImportLookupValue importLookupValue = (SkylarkImportLookupValue) skylarkImportMap.get(keyForLabel); - importMap.put(importPath, importLookupValue.getEnvironmentExtension()); + importMap.put(importString, importLookupValue.getEnvironmentExtension()); fileDependencies.add(importLookupValue.getDependency()); } @@ -849,8 +847,6 @@ public class PackageFunction implements SkyFunction { ? FileSystemUtils.readContent(buildFilePath) : FileSystemUtils.readWithKnownFileSize(buildFilePath, buildFileValue.getSize()); } catch (IOException e) { - env.getListener().handle(Event.error(Location.fromFile(buildFilePath), - e.getMessage())); // Note that we did this work, so we should conservatively report this error as // transient. throw new PackageFunctionException(new BuildFileContainsErrorsException( @@ -860,12 +856,9 @@ public class PackageFunction implements SkyFunction { preprocessingResult = packageFactory.preprocess(buildFilePath, packageId, buildFileBytes, globber); } catch (IOException e) { - env.getListener().handle(Event.error( - Location.fromFile(buildFilePath), - "preprocessing failed: " + e.getMessage())); throw new PackageFunctionException( - new BuildFileContainsErrorsException(packageId, "preprocessing failed", e), - Transience.TRANSIENT); + new BuildFileContainsErrorsException( + packageId, "preprocessing failed" + e.getMessage(), e), Transience.TRANSIENT); } } else { ParserInputSource replacementSource = @@ -970,10 +963,10 @@ public class PackageFunction implements SkyFunction { /** A simple value class to store the result of the Skylark imports.*/ static final class SkylarkImportResult { - final Map<PathFragment, Extension> importMap; + final Map<String, Extension> importMap; final ImmutableList<Label> fileDependencies; private SkylarkImportResult( - Map<PathFragment, Extension> importMap, + Map<String, Extension> importMap, ImmutableList<Label> fileDependencies) { this.importMap = importMap; this.fileDependencies = fileDependencies; 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 67c6907fe8..801fbb3372 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,6 +17,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Lists; @@ -37,6 +38,7 @@ 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.Mutability; +import com.google.devtools.build.lib.syntax.SkylarkImport; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; @@ -133,19 +135,19 @@ public class SkylarkImportLookupFunction implements SkyFunction { } // Process the load statements in the file. - ImmutableList<LoadStatement> loadStmts = ast.getImports(); - Map<PathFragment, Extension> importMap = Maps.newHashMapWithExpectedSize(loadStmts.size()); + ImmutableList<SkylarkImport> imports = ast.getImports(); + Map<String, Extension> extensionsForImports = Maps.newHashMapWithExpectedSize(imports.size()); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); - ImmutableMap<PathFragment, Label> importPathMap; + ImmutableMap<String, Label> labelsForImports; // Find the labels corresponding to the load statements. - importPathMap = findLabelsForLoadStatements(loadStmts, fileLabel, env); - if (importPathMap == null) { + labelsForImports = findLabelsForLoadStatements(imports, fileLabel, env); + if (labelsForImports == null) { return null; } // Look up and load the imports. - ImmutableCollection<Label> importLabels = importPathMap.values(); + ImmutableCollection<Label> importLabels = labelsForImports.values(); List<SkyKey> importLookupKeys = Lists.newArrayListWithExpectedSize(importLabels.size()); for (Label importLabel : importLabels) { @@ -168,7 +170,7 @@ public class SkylarkImportLookupFunction implements SkyFunction { } throw new SkylarkImportFailedException("Skylark import cycle"); } - skylarkImportMap = Maps.newHashMapWithExpectedSize(loadStmts.size()); + skylarkImportMap = Maps.newHashMapWithExpectedSize(imports.size()); for (SkyKey importLookupKey : importLookupKeys) { SkyValue skyValue = this.computeWithInlineCallsInternal(importLookupKey, env, visited); if (skyValue == null) { @@ -191,45 +193,23 @@ public class SkylarkImportLookupFunction implements SkyFunction { } // Process the loaded imports. - for (Entry<PathFragment, Label> importEntry : importPathMap.entrySet()) { - PathFragment importPath = importEntry.getKey(); + for (Entry<String, Label> importEntry : labelsForImports.entrySet()) { + String importString = importEntry.getKey(); Label importLabel = importEntry.getValue(); SkyKey keyForLabel = SkylarkImportLookupValue.key(importLabel, inWorkspace); SkylarkImportLookupValue importLookupValue = (SkylarkImportLookupValue) skylarkImportMap.get(keyForLabel); - importMap.put(importPath, importLookupValue.getEnvironmentExtension()); + extensionsForImports.put(importString, importLookupValue.getEnvironmentExtension()); fileDependencies.add(importLookupValue.getDependency()); } + // 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, fileLabel, importMap, env, inWorkspace); - + Extension extension = createExtension(ast, fileLabel, extensionsForImports, env, inWorkspace); return new SkylarkImportLookupValue( extension, new SkylarkFileDependency(fileLabel, fileDependencies.build())); } - /** Computes the Label corresponding to a relative import path. */ - private static Label labelForRelativeImport(PathFragment importPath, Label containingFileLabel) - throws SkylarkImportFailedException { - // The twistiness of the code below is due to the fact that the containing file may be in - // a subdirectory of the package that contains it. We need to construct a Label with - // the import file in the same subdirectory. - PackageIdentifier pkgIdForImport = containingFileLabel.getPackageIdentifier(); - PathFragment containingDirInPkg = - (new PathFragment(containingFileLabel.getName())).getParentDirectory(); - String targetNameForImport = containingDirInPkg.getRelative(importPath).toString(); - try { - return Label.create(pkgIdForImport, targetNameForImport); - } catch (LabelSyntaxException e) { - // While the Label is for the most part guaranteed to be well-formed by construction, an - // error is still possible if the filename itself is malformed, e.g., contains control - // characters. Since we expect this error to be very rare, for code simplicity, we allow the - // error message to refer to a Label even though the filename was specified via a simple path. - throw new SkylarkImportFailedException(e); - } - } - /** * Computes the set of Labels corresponding to a collection of PathFragments representing * absolute import paths. @@ -311,50 +291,51 @@ public class SkylarkImportLookupFunction implements SkyFunction { /** * Computes the set of {@link Label}s corresponding to a set of Skylark {@link LoadStatement}s. * - * @param loadStmts a collection of Skylark {@link LoadStatement}s + * @param imports a collection of Skylark {@link LoadStatement}s * @param containingFileLabel the {@link Label} of the file containing the load statements - * @return an {@link ImmutableMap} which maps a {@link PathFragment} corresponding - * to one of the files to be loaded to the corresponding {@Label}. Returns {@code null} if any - * Skyframe dependencies are unavailable. (attempt to retrieve an AST + * @return an {@link ImmutableMap} which maps a {@link String} used in the load statement to + * its corresponding {@Label}. Returns {@code null} if any Skyframe dependencies are + * unavailable. * @throws SkylarkImportFailedException if no package can be found that contains the * loaded file */ @Nullable - static ImmutableMap<PathFragment, Label> findLabelsForLoadStatements( - Iterable<LoadStatement> loadStmts, Label containingFileLabel, Environment env) + static ImmutableMap<String, Label> findLabelsForLoadStatements( + ImmutableCollection<SkylarkImport> imports, Label containingFileLabel, Environment env) throws SkylarkImportFailedException { - ImmutableMap.Builder<PathFragment, Label> outputMap = new ImmutableMap.Builder<>(); + Map<String, Label> outputMap = Maps.newHashMapWithExpectedSize(imports.size()); // Filter relative vs. absolute paths. - ImmutableSet.Builder<PathFragment> absolutePathsToLookup = new ImmutableSet.Builder<>(); - ImmutableSet.Builder<PathFragment> relativePathsToConvert = new ImmutableSet.Builder<>(); - for (LoadStatement loadStmt : loadStmts) { - PathFragment importPath = loadStmt.getImportPath(); - if (loadStmt.isAbsolute()) { - absolutePathsToLookup.add(importPath); + ImmutableSet.Builder<PathFragment> absoluteImportsToLookup = new ImmutableSet.Builder<>(); + // We maintain a multimap from path fragments to their correspond import strings, to cover the + // (unlikely) case where two distinct import strings generate the same path fragment. + ImmutableMultimap.Builder<PathFragment, String> pathToImports = + new ImmutableMultimap.Builder<>(); + for (SkylarkImport imp : imports) { + if (imp.hasAbsolutePath()) { + absoluteImportsToLookup.add(imp.getAbsolutePath()); + pathToImports.put(imp.getAbsolutePath(), imp.getImportString()); } else { - relativePathsToConvert.add(importPath); + outputMap.put(imp.getImportString(), imp.getLabel(containingFileLabel)); } } - // Compute labels for absolute paths. + // Look up labels for absolute paths. ImmutableMap<PathFragment, Label> absoluteLabels = - labelsForAbsoluteImports(absolutePathsToLookup.build(), env); + labelsForAbsoluteImports(absoluteImportsToLookup.build(), env); if (absoluteLabels == null) { return null; } - outputMap.putAll(absoluteLabels); - - // Compute labels for relative paths. - for (PathFragment importPath : relativePathsToConvert.build()) { - // Relative paths don't require package lookups since they can only refer to files in the - // same directory as the file containing the load statement; i.e., they can't refer to - // subdirectories. We can therefore compute the corresponding label directly from the label - // of the containing file (whose package has already been validated). - outputMap.put(importPath, labelForRelativeImport(importPath, containingFileLabel)); + for (Entry<PathFragment, Label> entry : absoluteLabels.entrySet()) { + PathFragment currPath = entry.getKey(); + Label currLabel = entry.getValue(); + for (String importString : pathToImports.build().get(currPath)) { + outputMap.put(importString, currLabel); + } } - return outputMap.build(); + ImmutableMap<String, Label> immutableOutputMap = ImmutableMap.copyOf(outputMap); + return immutableOutputMap; } /** @@ -363,7 +344,7 @@ public class SkylarkImportLookupFunction implements SkyFunction { private Extension createExtension( BuildFileAST ast, Label extensionLabel, - Map<PathFragment, Extension> importMap, + Map<String, Extension> importMap, Environment env, boolean inWorkspace) throws SkylarkImportFailedException, InterruptedException { 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 d1fc8b7d8b..1ff08835a2 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 @@ -34,7 +34,7 @@ public class BuildFileAST extends ASTNode { private final ImmutableList<Comment> comments; - private ImmutableList<LoadStatement> loads; + private ImmutableList<SkylarkImport> imports; /** * Whether any errors were encountered during scanning or parsing. @@ -60,15 +60,15 @@ public class BuildFileAST extends ASTNode { } /** Collects all load statements */ - private ImmutableList<LoadStatement> fetchLoads(List<Statement> stmts) { - ImmutableList.Builder<LoadStatement> loads = new ImmutableList.Builder<>(); + private ImmutableList<SkylarkImport> fetchLoads(List<Statement> stmts) { + ImmutableList.Builder<SkylarkImport> imports = new ImmutableList.Builder<>(); for (Statement stmt : stmts) { if (stmt instanceof LoadStatement) { - LoadStatement imp = (LoadStatement) stmt; - loads.add(imp); + SkylarkImport imp = ((LoadStatement) stmt).getImport(); + imports.add(imp); } } - return loads.build(); + return imports.build(); } /** @@ -97,11 +97,11 @@ public class BuildFileAST extends ASTNode { /** * Returns a list of loads in this BUILD file. */ - public synchronized ImmutableList<LoadStatement> getImports() { - if (loads == null) { - loads = fetchLoads(stmts); + public synchronized ImmutableList<SkylarkImport> getImports() { + if (imports == null) { + imports = fetchLoads(stmts); } - return loads; + return imports; } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index b7f24c30e2..bf77b9b2dc 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.syntax.Mutability.MutabilityException; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.build.lib.vfs.PathFragment; import java.io.Serializable; import java.util.HashMap; @@ -294,9 +293,9 @@ public final class Environment implements Freezable { private final EventHandler eventHandler; /** - * For each imported extensions, a global Skylark frame from which to load() individual bindings. + * For each imported extension, a global Skylark frame from which to load() individual bindings. */ - private final Map<PathFragment, Extension> importedExtensions; + private final Map<String, Extension> importedExtensions; /** * Is this Environment being executed in Skylark context? @@ -468,7 +467,7 @@ public final class Environment implements Freezable { Frame globalFrame, Frame dynamicFrame, EventHandler eventHandler, - Map<PathFragment, Extension> importedExtensions, + Map<String, Extension> importedExtensions, boolean isSkylark, @Nullable String fileContentHashCode, boolean isLoadingPhase) { @@ -492,7 +491,7 @@ public final class Environment implements Freezable { private boolean isLoadingPhase = false; @Nullable private Frame parent; @Nullable private EventHandler eventHandler; - @Nullable private Map<PathFragment, Extension> importedExtensions; + @Nullable private Map<String, Extension> importedExtensions; @Nullable private String fileContentHashCode; Builder(Mutability mutability) { @@ -528,9 +527,9 @@ public final class Environment implements Freezable { } /** Declares imported extensions for load() statements. */ - public Builder setImportedExtensions (Map<PathFragment, Extension> importedExtensions) { + public Builder setImportedExtensions (Map<String, Extension> importMap) { Preconditions.checkState(this.importedExtensions == null); - this.importedExtensions = importedExtensions; + this.importedExtensions = importMap; return this; } @@ -773,22 +772,22 @@ public final class Environment implements Freezable { * that was not properly loaded. */ public static class LoadFailedException extends Exception { - LoadFailedException(PathFragment extension) { + LoadFailedException(String importString) { super(String.format("file '%s' was not correctly loaded. " + "Make sure the 'load' statement appears in the global scope in your file", - extension)); + importString)); } } - public void importSymbol(PathFragment extension, Identifier symbol, String nameInLoadedFile) + public void importSymbol(String importString, Identifier symbol, String nameInLoadedFile) throws NoSuchVariableException, LoadFailedException { Preconditions.checkState(isGlobal()); // loading is only allowed at global scope. - if (!importedExtensions.containsKey(extension)) { - throw new LoadFailedException(extension); + if (!importedExtensions.containsKey(importString)) { + throw new LoadFailedException(importString); } - Extension ext = importedExtensions.get(extension); + Extension ext = importedExtensions.get(importString); // TODO(bazel-team): Throw a LoadFailedException instead, with an appropriate message. // Throwing a NoSuchVariableException is backward compatible, but backward. @@ -801,7 +800,7 @@ public final class Environment implements Freezable { try { update(symbol.getName(), value); } catch (EvalException e) { - throw new LoadFailedException(extension); + throw new LoadFailedException(importString); } } @@ -813,9 +812,9 @@ public final class Environment implements Freezable { // Calculate a new hash from the hash of the loaded Extension-s. Fingerprint fingerprint = new Fingerprint(); fingerprint.addString(Preconditions.checkNotNull(fileContentHashCode)); - TreeSet<PathFragment> paths = new TreeSet<>(importedExtensions.keySet()); - for (PathFragment path : paths) { - fingerprint.addString(importedExtensions.get(path).getTransitiveContentHashCode()); + TreeSet<String> importStrings = new TreeSet<>(importedExtensions.keySet()); + for (String importString : importStrings) { + fingerprint.addString(importedExtensions.get(importString).getTransitiveContentHashCode()); } return fingerprint.hexDigestAndReset(); } 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 67ca431931..d373c46021 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 @@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.syntax.compiler.DebugInfo; import com.google.devtools.build.lib.syntax.compiler.LoopLabels; import com.google.devtools.build.lib.syntax.compiler.VariableScope; -import com.google.devtools.build.lib.vfs.PathFragment; import net.bytebuddy.implementation.bytecode.ByteCodeAppender; @@ -33,8 +32,7 @@ public final class LoadStatement extends Statement { private final ImmutableMap<Identifier, String> symbols; private final ImmutableList<Identifier> cachedSymbols; // to save time - private final PathFragment importPath; - private final StringLiteral pathString; + private final SkylarkImport imp; /** * Constructs an import statement. @@ -43,24 +41,24 @@ public final class LoadStatement extends Statement { * 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) { + LoadStatement(SkylarkImport imp, Map<Identifier, String> symbols) { + this.imp = imp; this.symbols = ImmutableMap.copyOf(symbols); this.cachedSymbols = ImmutableList.copyOf(symbols.keySet()); - this.importPath = new PathFragment(path.getValue() + ".bzl"); - this.pathString = path; } public ImmutableList<Identifier> getSymbols() { return cachedSymbols; } - public PathFragment getImportPath() { - return importPath; + public SkylarkImport getImport() { + return imp; } @Override public String toString() { - return String.format("load(\"%s\", %s)", importPath, Joiner.on(", ").join(cachedSymbols)); + return String.format( + "load(\"%s\", %s)", imp.getImportString(), Joiner.on(", ").join(cachedSymbols)); } @Override @@ -75,7 +73,7 @@ public final class LoadStatement extends Statement { } // The key is the original name that was used to define the symbol // in the loaded bzl file. - env.importSymbol(getImportPath(), current, entry.getValue()); + env.importSymbol(imp.getImportString(), current, entry.getValue()); } catch (Environment.NoSuchVariableException | Environment.LoadFailedException e) { throw new EvalException(getLocation(), e.getMessage()); } @@ -89,41 +87,11 @@ public final class LoadStatement extends Statement { @Override void validate(ValidationEnvironment env) throws EvalException { - validatePath(); for (Identifier symbol : cachedSymbols) { env.declare(symbol.getName(), getLocation()); } } - public StringLiteral getPath() { - return pathString; - } - - /** - * Throws an exception if the path argument to load() starts with more than one forward - * slash ('/') - */ - public void validatePath() throws EvalException { - String error = null; - - if (pathString.getValue().isEmpty()) { - error = "Path argument to load() must not be empty."; - } else if (pathString.getValue().startsWith("//")) { - 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); - } - } - @Override ByteCodeAppender compile( VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo) { @@ -131,8 +99,4 @@ public final class LoadStatement extends Statement { "load statements should never appear in method bodies and" + " should never be compiled in general"); } - - public boolean isAbsolute() { - return getImportPath().isAbsolute(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java index c5f612f32d..023a85bee9 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral; import com.google.devtools.build.lib.syntax.IfStatement.ConditionalStatements; +import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException; import com.google.devtools.build.lib.util.Preconditions; import java.util.ArrayList; @@ -1067,18 +1068,16 @@ public class Parser { } expect(TokenKind.RPAREN); - LoadStatement stmt = new LoadStatement(path, symbols); - - // Although validateLoadPath() is invoked as part of validate(ValidationEnvironment), - // this only happens in Skylark. Consequently, we invoke it here to discover - // invalid load paths in BUILD mode, too. + SkylarkImport imp; try { - stmt.validatePath(); - } catch (EvalException e) { - reportError(path.getLocation(), e.getMessage()); + imp = SkylarkImports.create(path.getValue()); + LoadStatement stmt = new LoadStatement(imp, symbols); + list.add(setLocation(stmt, start, token.left)); + } catch (SkylarkImportSyntaxException e) { + String msg = "Load statement parameter '" + path + "' is invalid. " + + e.getMessage(); + reportError(path.getLocation(), msg); } - - list.add(setLocation(stmt, start, token.left)); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java index 2410c23a34..5aa744a2ba 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java @@ -16,13 +16,20 @@ package com.google.devtools.build.lib.syntax; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.Serializable; + /** * Encapsulates the four syntactic variants of Skylark imports: Absolute paths, relative * paths, absolute labels, and relative labels. */ -public interface SkylarkImport { +public interface SkylarkImport extends Serializable { /** + * Returns the string originally used to specify the import (may represent a label or a path). + */ + String getImportString(); + + /** * Given a {@link Label} representing the file that contains this import, returns a {@link Label} * representing the .bzl file to be imported. * diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java index 6c11ac2cbd..6c82a56b65 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java @@ -24,15 +24,39 @@ import com.google.devtools.build.lib.vfs.PathFragment; * Factory class for creating appropriate instances of {@link SkylarkImports}. */ public class SkylarkImports { - + private SkylarkImports() { throw new IllegalStateException("This class should not be instantiated"); } - private static final class AbsolutePathImport implements SkylarkImport { + // Default implementation class for SkylarkImport. + private abstract static class SkylarkImportImpl implements SkylarkImport { + protected String importString; + + @Override + public String getImportString() { + return importString; + } + + @Override + public abstract Label getLabel(Label containingFileLabel); + + @Override + public boolean hasAbsolutePath() { + return false; + } + + @Override + public PathFragment getAbsolutePath() { + throw new IllegalStateException("can't request absolute path from a non-absolute import"); + } + } + + private static final class AbsolutePathImport extends SkylarkImportImpl { private PathFragment importPath; - private AbsolutePathImport(PathFragment importPath) { + private AbsolutePathImport(String importString, PathFragment importPath) { + this.importString = importString; this.importPath = importPath; } @@ -52,10 +76,11 @@ public class SkylarkImports { } } - private static final class RelativePathImport implements SkylarkImport { + private static final class RelativePathImport extends SkylarkImportImpl { private String importFile; - private RelativePathImport(String importFile) { + private RelativePathImport(String importString, String importFile) { + this.importString = importString; this.importFile = importFile; } @@ -75,46 +100,29 @@ public class SkylarkImports { throw new IllegalStateException(e); } } - - @Override - public boolean hasAbsolutePath() { - return false; - } - - @Override - public PathFragment getAbsolutePath() { - throw new IllegalStateException("can't request absolute path from a non-absolute import"); - } } - private static final class AbsoluteLabelImport implements SkylarkImport { + private static final class AbsoluteLabelImport extends SkylarkImportImpl { private Label importLabel; - private AbsoluteLabelImport(Label importLabel) { + private AbsoluteLabelImport(String importString, Label importLabel) { + this.importString = importString; this.importLabel = importLabel; } @Override public Label getLabel(Label containingFileLabel) { - // The containing file label is irrelevant here since this is an absolute path. - return importLabel; - } - - @Override - public boolean hasAbsolutePath() { - return false; - } - - @Override - public PathFragment getAbsolutePath() { - throw new IllegalStateException("can't request absolute path from a non-absolute import"); + // When the import label contains no explicit repository identifier, we resolve it relative + // to the repo of the containing file. + return containingFileLabel.resolveRepositoryRelative(importLabel); } } - private static final class RelativeLabelImport implements SkylarkImport { + private static final class RelativeLabelImport extends SkylarkImportImpl { private String importTarget; - private RelativeLabelImport(String importTarget) { + private RelativeLabelImport(String importString, String importTarget) { + this.importString = importString; this.importTarget = importTarget; } @@ -130,16 +138,6 @@ public class SkylarkImports { throw new IllegalStateException(e); } } - - @Override - public boolean hasAbsolutePath() { - return false; - } - - @Override - public PathFragment getAbsolutePath() { - throw new IllegalStateException("can't request absolute path from a non-absolute import"); - } } /** @@ -152,10 +150,11 @@ public class SkylarkImports { } @VisibleForTesting - static final String INVALID_LABEL_PREFIX = "invalid label: "; + static final String INVALID_LABEL_PREFIX = "Invalid label: "; @VisibleForTesting - static final String MUST_HAVE_BZL_EXT_MSG = "must reference a file with extension '.bzl'"; + static final String MUST_HAVE_BZL_EXT_MSG = + "The label must reference a file with extension '.bzl'"; @VisibleForTesting static final String EXTERNAL_PKG_NOT_ALLOWED_MSG = @@ -163,17 +162,17 @@ public class SkylarkImports { @VisibleForTesting static final String BZL_EXT_IMPLICIT_MSG = - "the '.bzl' file extension is implicit; remove it from the path"; + "The '.bzl' file extension is implicit; remove it from the path"; @VisibleForTesting - static final String INVALID_TARGET_PREFIX = "invalid target: "; + static final String INVALID_TARGET_PREFIX = "Invalid target: "; @VisibleForTesting - static final String INVALID_FILENAME_PREFIX = "invalid filename: "; + static final String INVALID_FILENAME_PREFIX = "Invalid filename: "; @VisibleForTesting static final String RELATIVE_PATH_NO_SUBDIRS_MSG = - "relative import path may not contain subdirectories"; + "A relative import path may not contain subdirectories"; /** * Creates and syntactically validates a {@link SkylarkImports} instance from a string. @@ -200,14 +199,14 @@ public class SkylarkImports { if (packageId.equals(Label.EXTERNAL_PACKAGE_IDENTIFIER)) { throw new SkylarkImportSyntaxException(EXTERNAL_PKG_NOT_ALLOWED_MSG); } - return new AbsoluteLabelImport(importLabel); + return new AbsoluteLabelImport(importString, importLabel); } else if (importString.startsWith("/")) { // Absolute path. if (importString.endsWith(".bzl")) { throw new SkylarkImportSyntaxException(BZL_EXT_IMPLICIT_MSG); } - PathFragment importPath = new PathFragment(importString); - return new AbsolutePathImport(importPath); + PathFragment importPath = new PathFragment(importString + ".bzl"); + return new AbsolutePathImport(importString, importPath); } else if (importString.startsWith(":")) { // Relative label. We require that relative labels use an explicit ':' prefix to distinguish // them from relative paths, which have a different semantics. @@ -220,7 +219,7 @@ public class SkylarkImports { // Null indicates successful target validation. throw new SkylarkImportSyntaxException(INVALID_TARGET_PREFIX + maybeErrMsg); } - return new RelativeLabelImport(importTarget); + return new RelativeLabelImport(importString, importTarget); } else { // Relative path. if (importString.endsWith(".bzl")) { @@ -235,7 +234,7 @@ public class SkylarkImports { // Null indicates successful target validation. throw new SkylarkImportSyntaxException(INVALID_FILENAME_PREFIX + maybeErrMsg); } - return new RelativePathImport(importTarget); + return new RelativePathImport(importString, importTarget); } } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java index a9e9a18bc6..ac99bc9484 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; @@ -132,7 +131,7 @@ public class PackageFactoryApparatus { ConstantRuleVisibility.PUBLIC, false, new MakeEnvironment.Builder(), - ImmutableMap.<PathFragment, Extension>of(), + ImmutableMap.<String, Extension>of(), ImmutableList.<Label>of()); Package result = resultBuilder.build(); Event.replayEventsOn(eventHandler, result.getEvents()); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java index f35577ff92..5a5d7cc913 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java @@ -385,10 +385,11 @@ public class PackageFunctionTest extends BuildViewTestCase { getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter); assertTrue(result.hasError()); ErrorInfo errorInfo = result.getError(skyKey); + String expectedMsg = "error loading package 'test/skylark': " + + "Extension file not found. Unable to load file '//test/skylark:bad_extension.bzl': " + + "file doesn't exist or isn't a file"; assertThat(errorInfo.getException()) - .hasMessage("error loading package 'test/skylark': Extension file not found. " - + "Unable to load file '//test/skylark:bad_extension.bzl': " - + "file doesn't exist or isn't a file"); + .hasMessage(expectedMsg); } @Test @@ -406,9 +407,12 @@ public class PackageFunctionTest extends BuildViewTestCase { EvaluationResult<PackageValue> result = SkyframeExecutorTestUtils.evaluate( getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter); assertTrue(result.hasError()); - assertContainsEvent("Extension file not found. " - + "Unable to load file '//test/skylark:bad_extension.bzl': " - + "file doesn't exist or isn't a file"); + ErrorInfo errorInfo = result.getError(skyKey); + String expectedMsg = "error loading package 'test/skylark': " + + "Extension file not found. Unable to load file '//test/skylark:bad_extension.bzl': " + + "file doesn't exist or isn't a file"; + assertThat(errorInfo.getException()) + .hasMessage(expectedMsg); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java index 3f8d1d1630..b174296907 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java @@ -58,22 +58,22 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { public void testSkylarkImportLabels() throws Exception { scratch.file("pkg1/BUILD"); scratch.file("pkg1/ext.bzl"); - checkLabel("//pkg1:ext.bzl", "//pkg1:ext.bzl"); + checkSuccessfulLookup("//pkg1:ext.bzl"); scratch.file("pkg2/BUILD"); scratch.file("pkg2/dir/ext.bzl"); - checkLabel("//pkg2:dir/ext.bzl", "//pkg2:dir/ext.bzl"); + checkSuccessfulLookup("//pkg2:dir/ext.bzl"); scratch.file("dir/pkg3/BUILD"); scratch.file("dir/pkg3/dir/ext.bzl"); - checkLabel("//dir/pkg3:dir/ext.bzl", "//dir/pkg3:dir/ext.bzl"); + checkSuccessfulLookup("//dir/pkg3:dir/ext.bzl"); } @Test public void testSkylarkImportLabelsAlternativeRoot() throws Exception { scratch.file("/root_2/pkg4/BUILD"); scratch.file("/root_2/pkg4/ext.bzl"); - checkLabel("//pkg4:ext.bzl", "//pkg4:ext.bzl"); + checkSuccessfulLookup("//pkg4:ext.bzl"); } @Test @@ -81,7 +81,23 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { scratch.file("dir1/BUILD"); scratch.file("dir1/dir2/BUILD"); scratch.file("dir1/dir2/ext.bzl"); - checkLabel("//dir1/dir2:ext.bzl", "//dir1/dir2:ext.bzl"); + checkSuccessfulLookup("//dir1/dir2:ext.bzl"); + } + + @Test + public void testLoadFromSkylarkFileInRemoteRepo() throws Exception { + scratch.deleteFile("tools/build_rules/prelude_blaze"); + scratch.overwriteFile("WORKSPACE", + "local_repository(", + " name = 'a_remote_repo',", + " path = '/a_remote_repo'", + ")"); + scratch.file("/a_remote_repo/remote_pkg/BUILD"); + scratch.file("/a_remote_repo/remote_pkg/ext1.bzl", + "load(':ext2.bzl', 'CONST')"); + scratch.file("/a_remote_repo/remote_pkg/ext2.bzl", + "CONST = 17"); + checkSuccessfulLookup("@a_remote_repo//remote_pkg:ext1.bzl"); } @Test @@ -141,11 +157,13 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { return SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked(label), false); } - private void checkLabel(String labelRequested, String labelFound) throws Exception { - SkyKey skylarkImportLookupKey = key(labelRequested); + // Ensures that a Skylark file has been successfully processed by checking that the + // the label in its dependency set corresponds to the requested label. + private void checkSuccessfulLookup(String label) throws Exception { + SkyKey skylarkImportLookupKey = key(label); EvaluationResult<SkylarkImportLookupValue> result = get(skylarkImportLookupKey); - assertEquals(labelFound, - result.get(skylarkImportLookupKey).getDependency().getLabel().toString()); + assertEquals(result.get(skylarkImportLookupKey).getDependency().getLabel().toString(), + label); } @Test @@ -195,21 +213,4 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { assertEquals("invalid target name 'oops<?>.bzl': " + "target names may not contain non-printable characters: '\\x00'", errorMessage); } - - @Test - public void testSkylarkRelativeImportFilenameWithControlChars() throws Exception { - scratch.file("pkg/BUILD", ""); - scratch.file("pkg/ext.bzl", "load('oops\u0000', 'a')"); - SkyKey skylarkImportLookupKey = - SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl"), false); - EvaluationResult<SkylarkImportLookupValue> result = - SkyframeExecutorTestUtils.evaluate( - getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter); - assertTrue(result.hasError()); - ErrorInfo errorInfo = result.getError(skylarkImportLookupKey); - String errorMessage = errorInfo.getException().getMessage(); - assertEquals("invalid target name 'oops<?>.bzl': " - + "target names may not contain non-printable characters: '\\x00'", errorMessage); - } - } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java index de81e4e4e7..2985fd1ff7 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java @@ -22,10 +22,12 @@ import static org.junit.Assert.fail; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.syntax.Argument.Passed; import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral; import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; +import com.google.devtools.build.lib.vfs.PathFragment; import org.junit.Before; import org.junit.Test; @@ -1079,26 +1081,135 @@ public class ParserTest extends EvaluationTestCase { assertContainsError("syntax error"); } - private static final String DOUBLE_SLASH_LOAD = "load('//foo/bar/file', 'test')\n"; - private static final String DOUBLE_SLASH_ERROR = - "First argument of load() is a path, not a label. It should start with a " - + "single slash if it is an absolute path."; - @Test - public void testLoadDoubleSlashBuild() throws Exception { + public void testValidAbsoluteImportPath() { + List<Statement> statements = + parseFileForSkylark("load('/some/skylark/file', 'fun_test')\n"); + LoadStatement stmt = (LoadStatement) statements.get(0); + SkylarkImport imp = stmt.getImport(); + assertThat(imp.getImportString()).isEqualTo("/some/skylark/file"); + assertThat(imp.hasAbsolutePath()).isTrue(); + assertThat(imp.getAbsolutePath()).isEqualTo(new PathFragment("/some/skylark/file.bzl")); + } + + private void validNonAbsoluteImportTest(String importString, String containingFileLabelString, + String expectedLabelString) { + List<Statement> statements = + parseFileForSkylark("load('" + importString + "', 'fun_test')\n"); + LoadStatement stmt = (LoadStatement) statements.get(0); + SkylarkImport imp = stmt.getImport(); + assertThat(imp.getImportString()).isEqualTo(importString); + assertThat(imp.hasAbsolutePath()).isFalse(); + Label containingFileLabel = Label.parseAbsoluteUnchecked(containingFileLabelString); + assertThat(imp.getLabel(containingFileLabel)) + .isEqualTo(Label.parseAbsoluteUnchecked(expectedLabelString)); + } + + private void invalidImportTest(String importString, String expectedMsg) { setFailFast(false); - parseFile(DOUBLE_SLASH_LOAD); - assertContainsError(DOUBLE_SLASH_ERROR); + parseFileForSkylark("load('" + importString + "', 'fun_test')\n"); + assertContainsError(expectedMsg); } @Test - public void testLoadDoubleSlashSkylark() throws Exception { - setFailFast(false); - parseFileForSkylark(DOUBLE_SLASH_LOAD); - assertContainsError(DOUBLE_SLASH_ERROR); + public void testValidRelativeImportPathInPackageDir() throws Exception { + validNonAbsoluteImportTest("file", /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:file.bzl"); + } + + @Test + public void testValidRelativeImportPathInPackageSubdir() throws Exception { + validNonAbsoluteImportTest("file", /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:skylark/file.bzl"); + } + + @Test + public void testInvalidRelativePathBzlExtImplicit() throws Exception { + invalidImportTest("file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG); + } + + @Test + public void testInvalidRelativePathNoSubdirs() throws Exception { + invalidImportTest("path/to/file", SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG); + } + + @Test + public void testInvalidRelativePathInvalidFilename() throws Exception { + invalidImportTest("\tfile", SkylarkImports.INVALID_FILENAME_PREFIX); + } + + private void validAbsoluteImportLabelTest(String importString) { + validNonAbsoluteImportTest(importString, /*irrelevant*/ "//another/path:BUILD", + /*expected*/ importString); + } + + @Test + public void testValidAbsoluteImportLabel() throws Exception { + validAbsoluteImportLabelTest("//some/skylark:file.bzl"); + } + + @Test + public void testValidAbsoluteImportLabelWithRepo() throws Exception { + validAbsoluteImportLabelTest("@my_repo//some/skylark:file.bzl"); } @Test + public void testInvalidAbsoluteImportLabel() throws Exception { + invalidImportTest("//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX); + } + + @Test + public void testInvalidAbsoluteImportLabelWithRepo() throws Exception { + invalidImportTest("@my_repo//some/skylark/:file.bzl", + SkylarkImports.INVALID_LABEL_PREFIX); + } + + @Test + public void testInvalidAbsoluteImportLabelMissingBzlExt() throws Exception { + invalidImportTest("//some/skylark:file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG); + } + + @Test + public void testInvalidAbsoluteImportReferencesExternalPkg() throws Exception { + invalidImportTest("//external:file.bzl", SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG); + } + + @Test + public void testValidRelativeImportSimpleLabelInPackageDir() throws Exception { + validNonAbsoluteImportTest(":file.bzl", /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:file.bzl"); + } + + @Test + public void testValidRelativeImportSimpleLabelInPackageSubdir() throws Exception { + validNonAbsoluteImportTest(":file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:file.bzl"); + } + + @Test + public void testValidRelativeImportComplexLabelInPackageDir() throws Exception { + validNonAbsoluteImportTest(":subdir/containing/file.bzl", /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:subdir/containing/file.bzl"); + } + + @Test + public void testValidRelativeImportComplexLabelInPackageSubdir() throws Exception { + validNonAbsoluteImportTest(":subdir/containing/file.bzl", + /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:subdir/containing/file.bzl"); + } + + @Test + public void testInvalidRelativeImportLabelMissingBzlExt() throws Exception { + invalidImportTest(":file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG); + } + + @Test + public void testInvalidRelativeImportLabelSyntax() throws Exception { + invalidImportTest("::file.bzl", SkylarkImports.INVALID_TARGET_PREFIX); + } + + @Test public void testLoadNoSymbol() throws Exception { setFailFast(false); parseFileForSkylark("load('/foo/bar/file')\n"); @@ -1110,7 +1221,7 @@ public class ParserTest extends EvaluationTestCase { List<Statement> statements = parseFileForSkylark( "load('/foo/bar/file', 'fun_test')\n"); LoadStatement stmt = (LoadStatement) statements.get(0); - assertEquals("/foo/bar/file.bzl", stmt.getImportPath().toString()); + assertEquals("/foo/bar/file", stmt.getImport().getImportString()); assertThat(stmt.getSymbols()).hasSize(1); } @@ -1119,7 +1230,7 @@ public class ParserTest extends EvaluationTestCase { List<Statement> statements = parseFileForSkylark( "load('/foo/bar/file', 'fun_test',)\n"); LoadStatement stmt = (LoadStatement) statements.get(0); - assertEquals("/foo/bar/file.bzl", stmt.getImportPath().toString()); + assertEquals("/foo/bar/file", stmt.getImport().getImportString()); assertThat(stmt.getSymbols()).hasSize(1); } @@ -1128,7 +1239,7 @@ public class ParserTest extends EvaluationTestCase { List<Statement> statements = parseFileForSkylark( "load('file', 'foo', 'bar')\n"); LoadStatement stmt = (LoadStatement) statements.get(0); - assertEquals("file.bzl", stmt.getImportPath().toString()); + assertEquals("file", stmt.getImport().getImportString()); assertThat(stmt.getSymbols()).hasSize(2); } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java index c233a593d9..de3452fe11 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java @@ -34,35 +34,28 @@ public class SkylarkImportTest { @Rule public ExpectedException thrown = ExpectedException.none(); - @Test - public void testValidAbsoluteLabel() throws Exception { - String labelToTest = "//some/skylark:file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); + private void validAbsoluteLabelTest(String labelString) throws Exception { + SkylarkImport importForLabel = SkylarkImports.create(labelString); assertThat(importForLabel.hasAbsolutePath()).isFalse(); + assertThat(importForLabel.getImportString()).isEqualTo(labelString); Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD"); assertThat(importForLabel.getLabel(irrelevantContainingFile)) - .isEqualTo(Label.parseAbsoluteUnchecked("//some/skylark:file.bzl")); + .isEqualTo(Label.parseAbsoluteUnchecked(labelString)); thrown.expect(IllegalStateException.class); - importForLabel.getAbsolutePath(); + importForLabel.getAbsolutePath(); } + @Test + public void testValidAbsoluteLabel() throws Exception { + validAbsoluteLabelTest("//some/skylark:file.bzl"); + } @Test public void testValidAbsoluteLabelWithRepo() throws Exception { - String labelToTest = "@my_repo//some/skylark:file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); - - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - - Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD"); - assertThat(importForLabel.getLabel(irrelevantContainingFile)) - .isEqualTo(Label.parseAbsoluteUnchecked("@my_repo//some/skylark:file.bzl")); - - thrown.expect(IllegalStateException.class); - importForLabel.getAbsolutePath(); + validAbsoluteLabelTest("@my_repo//some/skylark:file.bzl"); } @Test @@ -71,197 +64,141 @@ public class SkylarkImportTest { SkylarkImport importForPath = SkylarkImports.create(pathToTest); assertThat(importForPath.hasAbsolutePath()).isTrue(); + assertThat(importForPath.getImportString()).isEqualTo(pathToTest); Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD"); - assertThat(importForPath.getAbsolutePath()).isEqualTo(new PathFragment(pathToTest)); + assertThat(importForPath.getAbsolutePath()).isEqualTo(new PathFragment(pathToTest + ".bzl")); thrown.expect(IllegalStateException.class); importForPath.getLabel(irrelevantContainingFile); } - @Test - public void testValidRelativeSimpleLabelInPackageDir() throws Exception { - String labelToTest = ":file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); + private void validRelativeLabelTest(String labelString, + String containingLabelString, String expectedLabelString) throws Exception { + SkylarkImport importForLabel = SkylarkImports.create(labelString); assertThat(importForLabel.hasAbsolutePath()).isFalse(); + assertThat(importForLabel.getImportString()).isEqualTo(labelString); // The import label is relative to the parent's package, not the parent's directory. - Label containingFile = Label.parseAbsolute("//some/skylark:BUILD"); - assertThat(importForLabel.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/skylark:file.bzl")); + Label containingLabel = Label.parseAbsolute(containingLabelString); + assertThat(importForLabel.getLabel(containingLabel)) + .isEqualTo(Label.parseAbsolute(expectedLabelString)); thrown.expect(IllegalStateException.class); importForLabel.getAbsolutePath(); } @Test - public void testValidRelativeSimpleLabelInPackageSubdir() throws Exception { - String labelToTest = ":file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); - - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - - // The import label is relative to the parent's package, not the parent's directory. - Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl"); - assertThat(importForLabel.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/path/to:file.bzl")); + public void testValidRelativeSimpleLabelInPackageDir() throws Exception { + validRelativeLabelTest(":file.bzl", /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:file.bzl"); + } - thrown.expect(IllegalStateException.class); - importForLabel.getAbsolutePath(); + @Test + public void testValidRelativeSimpleLabelInPackageSubdir() throws Exception { + validRelativeLabelTest(":file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:file.bzl"); } @Test public void testValidRelativeComplexLabelInPackageDir() throws Exception { - String labelToTest = ":subdir/containing/file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); - - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - - // The import label is relative to the parent's package, not the parent's directory. - Label containingFile = Label.parseAbsolute("//some/skylark:BUILD"); - assertThat(importForLabel.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/skylark:subdir/containing/file.bzl")); - - thrown.expect(IllegalStateException.class); - importForLabel.getAbsolutePath(); + validRelativeLabelTest(":subdir/containing/file.bzl", + /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:subdir/containing/file.bzl"); } @Test public void testValidRelativeComplexLabelInPackageSubdir() throws Exception { - String labelToTest = ":subdir/containing/file.bzl"; - SkylarkImport importForLabel = SkylarkImports.create(labelToTest); - - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - - // The import label is relative to the parent's package, not the parent's directory. - Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl"); - assertThat(importForLabel.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/path/to:subdir/containing/file.bzl")); - - thrown.expect(IllegalStateException.class); - importForLabel.getAbsolutePath(); + validRelativeLabelTest(":subdir/containing/file.bzl", + /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:subdir/containing/file.bzl"); } - @Test - public void testValidRelativePathInPackageDir() throws Exception { - String pathToTest = "file"; - SkylarkImport importForPath = SkylarkImports.create(pathToTest); + private void validRelativePathTest(String pathString, String containingLabelString, + String expectedLabelString) throws Exception { + SkylarkImport importForPath = SkylarkImports.create(pathString); assertThat(importForPath.hasAbsolutePath()).isFalse(); // The import label is relative to the parent's directory not the parent's package. - Label containingFile = Label.parseAbsolute("//some/skylark:BUILD"); - assertThat(importForPath.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/skylark:file.bzl")); + Label containingLabel = Label.parseAbsolute(containingLabelString); + assertThat(importForPath.getLabel(containingLabel)) + .isEqualTo(Label.parseAbsolute(expectedLabelString)); thrown.expect(IllegalStateException.class); importForPath.getAbsolutePath(); } @Test - public void testValidRelativePathInPackageSubdir() throws Exception { - String pathToTest = "file"; - SkylarkImport importForPath = SkylarkImports.create(pathToTest); - assertThat(importForPath.hasAbsolutePath()).isFalse(); + public void testValidRelativePathInPackageDir() throws Exception { + validRelativePathTest("file", /*containing*/ "//some/skylark:BUILD", + /*expected*/ "//some/skylark:file.bzl"); + } - // The import label is relative to the parent's directory not the parent's package. - Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl"); - assertThat(importForPath.getLabel(containingFile)) - .isEqualTo(Label.parseAbsolute("//some/path/to:skylark/file.bzl")); + @Test + public void testValidRelativePathInPackageSubdir() throws Exception { + validRelativePathTest("file", /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected*/ "//some/path/to:skylark/file.bzl"); + } - thrown.expect(IllegalStateException.class); - importForPath.getAbsolutePath(); + private void invalidImportTest(String importString, String expectedMsgPrefix) throws Exception { + thrown.expect(SkylarkImportSyntaxException.class); + thrown.expectMessage(startsWith(expectedMsgPrefix)); + SkylarkImports.create(importString); } @Test public void testInvalidAbsoluteLabelSyntax() throws Exception { - String labelToTest = "//some/skylark/:file.bzl"; // final '/' is illegal - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(startsWith(SkylarkImports.INVALID_LABEL_PREFIX)); - SkylarkImports.create(labelToTest); + // final '/' is illegal + invalidImportTest("//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX); } @Test public void testInvalidAbsoluteLabelSyntaxWithRepo() throws Exception { - String labelToTest = "@my_repo//some/skylark/:file.bzl"; // final '/' is illegal - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(startsWith(SkylarkImports.INVALID_LABEL_PREFIX)); - SkylarkImports.create(labelToTest); + // final '/' is illegal + invalidImportTest("@my_repo//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX); } @Test public void tesInvalidAbsoluteLabelMissingBzlExt() throws Exception { - String labelToTest = "//some/skylark:file"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.MUST_HAVE_BZL_EXT_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest("//some/skylark:file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG); } @Test public void tesInvalidAbsoluteLabelReferencesExternalPkg() throws Exception { - String labelToTest = "//external:file.bzl"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest("//external:file.bzl", SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG); } - @Test + @Test public void tesInvalidAbsolutePathBzlExtImplicit() throws Exception { - String labelToTest = "/some/skylark/file.bzl"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.BZL_EXT_IMPLICIT_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest("/some/skylark/file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG); } @Test public void testInvalidRelativeLabelMissingBzlExt() throws Exception { - String labelToTest = ":file"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.MUST_HAVE_BZL_EXT_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest(":file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG); } @Test public void testInvalidRelativeLabelSyntax() throws Exception { - String labelToTest = "::file.bzl"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(startsWith(SkylarkImports.INVALID_TARGET_PREFIX)); - SkylarkImports.create(labelToTest); + invalidImportTest("::file.bzl", SkylarkImports.INVALID_TARGET_PREFIX); } @Test public void testInvalidRelativePathBzlExtImplicit() throws Exception { - String labelToTest = "file.bzl"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.BZL_EXT_IMPLICIT_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest("file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG); } @Test public void testInvalidRelativePathNoSubdirs() throws Exception { - String labelToTest = "path/to/file"; - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG); - SkylarkImports.create(labelToTest); + invalidImportTest("path/to/file", SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG); } @Test public void testInvalidRelativePathInvalidFilename() throws Exception { - String labelToTest = "\tfile"; // tab character is invalid - - thrown.expect(SkylarkImportSyntaxException.class); - thrown.expectMessage(startsWith(SkylarkImports.INVALID_FILENAME_PREFIX)); - SkylarkImports.create(labelToTest); + // tab character is invalid + invalidImportTest("\tfile", SkylarkImports.INVALID_FILENAME_PREFIX); } } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java index 5adba9354c..4c6eb6ac63 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java @@ -270,24 +270,6 @@ public class ValidationTest extends EvaluationTestCase { } @Test - public void testLoadRelativePathOneSegment() throws Exception { - parse("load('extension', 'a')\n"); - } - - @Test - public void testLoadAbsolutePathMultipleSegments() throws Exception { - parse("load('/pkg/extension', 'a')\n"); - } - - @Test - public void testLoadRelativePathMultipleSegments() throws Exception { - checkError( - "Path 'pkg/extension.bzl' is not valid. It should either start with " - + "a slash or refer to a file in the current directory.", - "load('pkg/extension', 'a')\n"); - } - - @Test public void testDollarErrorDoesNotLeak() throws Exception { setFailFast(false); parseFile( diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 6570a337a3..158d5d7ecf 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -178,6 +178,14 @@ sh_test( ) sh_test( + name = "external_skylark_load_test", + size = "large", + srcs = ["external_skylark_load_test.sh"], + data = [":test-deps"], + shard_count = 6, +) + +sh_test( name = "skylark_repository_test", size = "large", srcs = ["skylark_repository_test.sh"], diff --git a/src/test/shell/bazel/external_skylark_load_test.sh b/src/test/shell/bazel/external_skylark_load_test.sh new file mode 100755 index 0000000000..03b08efa56 --- /dev/null +++ b/src/test/shell/bazel/external_skylark_load_test.sh @@ -0,0 +1,128 @@ +#!/bin/bash +# +# Copyright 2015 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Test handling of Skylark loads from and in external repositories. +# + +# Load test environment +source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-setup.sh \ + || { echo "test-setup.sh not found!" >&2; exit 1; } + +# The following tests build an instance of a Skylark macro loaded from a +# local_repository, which in turns loads another Skylark file either from +# the external repo or the default repo, depending on the test parameters. +# The tests cover all the valid syntactic variants of the second load. The +# package structure used for the tests is as follows: +# +# ${WORKSPACE_DIR}/ +# WORKSPACE +# local_pkg/ +# BUILD +# another_local_pkg/ +# BUILD +# local_constants.bzl +# ${external_repo}/ +# external_pkg/ +# BUILD +# macro_def.bzl +# external_constants.bzl +function run_external_skylark_load_test() { + load_target_to_test=$1 + expected_test_output=$2 + + create_new_workspace + external_repo=${new_workspace_dir} + + cat > ${WORKSPACE_DIR}/WORKSPACE <<EOF +local_repository(name = "external_repo", path = "${external_repo}") +EOF + + mkdir ${WORKSPACE_DIR}/local_pkg + cat > ${WORKSPACE_DIR}/local_pkg/BUILD <<EOF +load("@external_repo//external_pkg:macro_def.bzl", "macro") +macro(name="macro_instance") +EOF + + mkdir ${WORKSPACE_DIR}/another_local_pkg + touch ${WORKSPACE_DIR}/another_local_pkg/BUILD + cat > ${WORKSPACE_DIR}/another_local_pkg/local_constants.bzl <<EOF +OUTPUT_STRING = "LOCAL!" +EOF + + mkdir ${external_repo}/external_pkg + touch ${external_repo}/external_pkg/BUILD + cat > ${external_repo}/external_pkg/macro_def.bzl <<EOF +load("${load_target_to_test}", "OUTPUT_STRING") +def macro(name): + native.genrule( + name = name, + outs = [name + ".txt"], + cmd = "echo " + OUTPUT_STRING + " > \$@", + ) +EOF + + cat > ${external_repo}/external_pkg/external_constants.bzl <<EOF +OUTPUT_STRING = "EXTERNAL!" +EOF + + cd ${WORKSPACE_DIR} + bazel build local_pkg:macro_instance >& $TEST_log || \ + fail "Expected build to succeed" + assert_contains "${expected_test_output}" \ + bazel-genfiles/local_pkg/macro_instance.txt +} + +# A label with an explicit external repo reference should be resolved relative +# to the external repo. +function test_load_skylark_from_external_repo_with_pkg_relative_label_load() { + run_external_skylark_load_test \ + "@external_repo//external_pkg:external_constants.bzl" "EXTERNAL!" +} + +# A relative label should be resolved relative to the external package. +function test_load_skylark_from_external_repo_with_pkg_relative_label_load() { + run_external_skylark_load_test ":external_constants.bzl" "EXTERNAL!" +} + +# A relative path should be resolved relative to the current (external) +# directory. +function test_load_skylark_from_external_repo_with_pkg_relative_path_load() { + run_external_skylark_load_test "external_constants" "EXTERNAL!" +} + +# An absolute label with no repo prefix should be resolved relative to the +# current (external) repo. +function test_load_skylark_from_external_repo_with_pkg_relative_path_load() { + run_external_skylark_load_test "//external_pkg:external_constants.bzl" \ + "EXTERNAL!" +} + +# An absolute label with the special "@" prefix should cause be resolved +# relative to the default repo. +function test_load_skylark_from_external_repo_with_repo_relative_label_load() { + run_external_skylark_load_test "@//another_local_pkg:local_constants.bzl" \ + "LOCAL!" +} + +# An absolute path should always be resolved in the default repo. +function test_load_skylark_from_external_repo_with_repo_relative_label_load() { + run_external_skylark_load_test "/another_local_pkg/local_constants" \ + "LOCAL!" +} + +run_suite "Test Skylark loads from/in external repositories" + + |