diff options
Diffstat (limited to 'src')
4 files changed, 209 insertions, 49 deletions
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 dcf364a3e1..90f4ea87c8 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 @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Package.Builder; import com.google.devtools.build.lib.packages.Package.LegacyBuilder; +import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; @@ -35,6 +36,7 @@ import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; +import com.google.devtools.build.lib.syntax.Environment.Frame; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.FuncallExpression; import com.google.devtools.build.lib.syntax.FunctionSignature; @@ -60,6 +62,16 @@ public class WorkspaceFactory { public static final String BIND = "bind"; private static final Pattern LEGAL_WORKSPACE_NAME = Pattern.compile("^\\p{Alpha}\\w*$"); + // List of static function added by #addWorkspaceFunctions. Used to trim them out from the + // serialized list of variables bindings. + private static final ImmutableList<String> STATIC_WORKSPACE_FUNCTIONS = + ImmutableList.of( + "workspace", + "__embedded_dir__", // serializable so optional + "__workspace_dir__", // serializable so optional + "DEFAULT_SERVER_JAVABASE", // serializable so optional + PackageFactory.PKG_CONTEXT); + private final LegacyBuilder builder; private final Path installDir; @@ -69,6 +81,18 @@ public class WorkspaceFactory { private final ImmutableMap<String, BaseFunction> workspaceFunctions; private final ImmutableList<EnvironmentExtension> environmentExtensions; + // Values from the previous workspace file parts. + // List of load statements + private ImmutableMap<String, Extension> parentImportMap = ImmutableMap.of(); + // List of top level variable bindings + private ImmutableMap<String, Object> parentVariableBindings = ImmutableMap.of(); + + // Values accumulated up to the currently parsed workspace file part. + // List of load statements + private ImmutableMap<String, Extension> importMap = ImmutableMap.of(); + // List of top level variable bindings + private ImmutableMap<String, Object> variableBindings = ImmutableMap.of(); + /** * @param builder a builder for the Workspace * @param ruleClassProvider a provider for known rule classes @@ -144,7 +168,8 @@ public class WorkspaceFactory { Preconditions.checkNotNull(importedExtensions); execute(ast, importedExtensions, new StoredEventHandler()); } - + + private void execute(BuildFileAST ast, @Nullable Map<String, Extension> importedExtensions, StoredEventHandler localReporter) throws InterruptedException { @@ -152,14 +177,45 @@ public class WorkspaceFactory { .setGlobals(Environment.BUILD) .setEventHandler(localReporter); if (importedExtensions != null) { - environmentBuilder.setImportedExtensions(importedExtensions); + importMap = + ImmutableMap.<String, Extension>builder() + .putAll(parentImportMap) + .putAll(importedExtensions) + .build(); + } else { + importMap = parentImportMap; } + environmentBuilder.setImportedExtensions(importMap); Environment workspaceEnv = environmentBuilder.setLoadingPhase().build(); addWorkspaceFunctions(workspaceEnv, localReporter); + for (Map.Entry<String, Object> binding : parentVariableBindings.entrySet()) { + try { + workspaceEnv.update(binding.getKey(), binding.getValue()); + } catch (EvalException e) { + // This should never happen because everything was already evaluated. + throw new IllegalStateException(e); + } + } if (!ast.exec(workspaceEnv, localReporter)) { localReporter.handle(Event.error("Error evaluating WORKSPACE file")); } + // Save the list of variable bindings for the next part of the workspace file. The list of + // variable bindings of interest are the global variable bindings that are defined by the user, + // so not the workspace functions. + // Workspace functions are not serializable and should not be passed over sky values. They + // also have a package builder specific to the current part and should be reinitialized for + // each workspace file. + ImmutableMap.Builder<String, Object> bindingsBuilder = ImmutableMap.builder(); + Frame globals = workspaceEnv.getGlobals(); + for (String s : globals.getDirectVariableNames()) { + Object o = globals.get(s); + if (!isAWorkspaceFunction(s, o)) { + bindingsBuilder.put(s, o); + } + } + variableBindings = bindingsBuilder.build(); + builder.addEvents(localReporter.getEvents()); if (localReporter.hasErrors()) { builder.setContainsErrors(); @@ -167,11 +223,38 @@ public class WorkspaceFactory { localReporter.clear(); } + private boolean isAWorkspaceFunction(String name, Object o) { + return STATIC_WORKSPACE_FUNCTIONS.contains(name) || (workspaceFunctions.get(name) == o); + } + private static boolean isLegalWorkspaceName(String name) { Matcher matcher = LEGAL_WORKSPACE_NAME.matcher(name); return matcher.matches(); } + /** + * Adds the various values returned by the parsing of the previous workspace file parts. + * {@code aPackage} is the package returned by the parent WorkspaceFileFunction, {@code importMap} + * is the list of load statements imports computed by the parent WorkspaceFileFunction and + * {@code variableBindings} the list of top level variable bindings of that same call. + */ + public void setParent( + Package aPackage, + ImmutableMap<String, Extension> importMap, + ImmutableMap<String, Object> bindings) + throws NameConflictException { + this.parentVariableBindings = bindings; + this.parentImportMap = importMap; + // Transmit the content of the parent package to the new package builder. + builder.addEvents(aPackage.getEvents()); + if (aPackage.containsErrors()) { + builder.setContainsErrors(); + } + for (Target target : aPackage.getTargets(Rule.class)) { + builder.addRule((Rule) target); + } + } + @SkylarkSignature(name = "workspace", objectType = Object.class, returnType = SkylarkList.class, doc = "Sets the name for this workspace. Workspace names should be a Java-package-style " + "description of the project, using underscores as separators, e.g., " @@ -328,4 +411,12 @@ public class WorkspaceFactory { static { SkylarkSignatureProcessor.configureSkylarkFunctions(WorkspaceFactory.class); } + + public Map<String, Extension> getImportMap() { + return importMap; + } + + public Map<String, Object> getVariableBindings() { + return variableBindings; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index fbf079c85b..7f230a7ef6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; -import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; @@ -31,12 +30,12 @@ import com.google.devtools.build.lib.packages.RuleSerializer; import com.google.devtools.build.lib.skyframe.FileSymlinkException; import com.google.devtools.build.lib.skyframe.FileValue; import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException; -import com.google.devtools.build.lib.skyframe.PackageValue; +import com.google.devtools.build.lib.skyframe.PackageLookupValue; +import com.google.devtools.build.lib.skyframe.WorkspaceFileValue; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -340,7 +339,6 @@ public abstract class RepositoryFunction { throws RepositoryFunctionException { try { FileSystemUtils.createDirectoryAndParents(repositoryDirectory); - FileSystem fs = repositoryDirectory.getFileSystem(); if (repositoryDirectory.getFileSystem().supportsSymbolicLinksNatively()) { for (Path target : targetDirectory.getDirectoryEntries()) { Path symlinkPath = @@ -380,33 +378,37 @@ public abstract class RepositoryFunction { @Nullable public static Rule getRule(String repository, Environment env) throws RepositoryFunctionException { - SkyKey packageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); - PackageValue packageValue; - try { - packageValue = (PackageValue) env.getValueOrThrow(packageKey, - NoSuchPackageException.class); - } catch (NoSuchPackageException e) { - throw new RepositoryFunctionException( - new BuildFileNotFoundException( - Label.EXTERNAL_PACKAGE_IDENTIFIER, "Could not load //external package"), - Transience.PERSISTENT); - } - if (packageValue == null) { + + SkyKey packageLookupKey = PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); + PackageLookupValue packageLookupValue; + packageLookupValue = (PackageLookupValue) env.getValue(packageLookupKey); + if (packageLookupValue == null) { return null; } + RootedPath workspacePath = + RootedPath.toRootedPath(packageLookupValue.getRoot(), new PathFragment("WORKSPACE")); - Package externalPackage = packageValue.getPackage(); - if (externalPackage.containsErrors()) { - throw new RepositoryFunctionException( - new BuildFileContainsErrorsException( - Label.EXTERNAL_PACKAGE_IDENTIFIER, "Could not load //external package"), - Transience.PERSISTENT); - } - Rule rule = externalPackage.getRule(repository); - if (rule == null) { - throw new RepositoryNotFoundException(repository); - } - return rule; + SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath); + do { + WorkspaceFileValue value = (WorkspaceFileValue) env.getValue(workspaceKey); + if (value == null) { + return null; + } + // TODO(dmarting): stop at cycle and report a more intelligible error than cycle reporting. + Package externalPackage = value.getPackage(); + if (externalPackage.containsErrors()) { + throw new RepositoryFunctionException( + new BuildFileContainsErrorsException( + Label.EXTERNAL_PACKAGE_IDENTIFIER, "Could not load //external package"), + Transience.PERSISTENT); + } + Rule rule = externalPackage.getRule(repository); + if (rule != null) { + return rule; + } + workspaceKey = value.next(); + } while (workspaceKey != null); + throw new RepositoryNotFoundException(repository); } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index 305d2e89fc..2ab3267d87 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -14,15 +14,19 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Package.LegacyBuilder; +import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.WorkspaceFactory; import com.google.devtools.build.lib.skyframe.PackageFunction.PackageFunctionException; import com.google.devtools.build.lib.skyframe.WorkspaceFileValue.WorkspaceFileKey; import com.google.devtools.build.lib.syntax.BuildFileAST; +import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.RootedPath; @@ -64,10 +68,20 @@ public class WorkspaceFileFunction implements SkyFunction { Path repoWorkspace = workspaceRoot.getRoot().getRelative(workspaceRoot.getRelativePath()); LegacyBuilder builder = - com.google.devtools.build.lib.packages.Package.newExternalPackageBuilder( - repoWorkspace, ruleClassProvider.getRunfilesPrefix()); + Package.newExternalPackageBuilder(repoWorkspace, ruleClassProvider.getRunfilesPrefix()); + + if (workspaceASTValue.getASTs().isEmpty()) { + return new WorkspaceFileValue( + builder.build(), // resulting package + ImmutableMap.<String, Extension>of(), // list of imports + ImmutableMap.<String, Object>of(), // list of symbol bindings + workspaceRoot, // Workspace root + 0, // first fragment, idx = 0 + false); // last fragment + } + WorkspaceFactory parser; try (Mutability mutability = Mutability.create("workspace %s", repoWorkspace)) { - WorkspaceFactory parser = + parser = new WorkspaceFactory( builder, ruleClassProvider, @@ -75,23 +89,39 @@ public class WorkspaceFileFunction implements SkyFunction { mutability, directories.getEmbeddedBinariesRoot(), directories.getWorkspace()); - try { - for (BuildFileAST ast : workspaceASTValue.getASTs()) { - PackageFunction.SkylarkImportResult importResult = - PackageFunction.fetchImportsFromBuildFile( - repoWorkspace, Label.EXTERNAL_PACKAGE_IDENTIFIER, ast, env, null); - if (importResult != null) { - parser.execute(ast, importResult.importMap); - } else { - return null; - } + if (key.getIndex() > 0) { + WorkspaceFileValue prevValue = + (WorkspaceFileValue) + env.getValue(WorkspaceFileValue.key(key.getPath(), key.getIndex() - 1)); + if (prevValue == null) { + return null; } - } catch (PackageFunctionException e) { - throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT); + if (prevValue.next() == null) { + return prevValue; + } + parser.setParent(prevValue.getPackage(), prevValue.getImportMap(), prevValue.getBindings()); + } + BuildFileAST ast = workspaceASTValue.getASTs().get(key.getIndex()); + PackageFunction.SkylarkImportResult importResult = + PackageFunction.fetchImportsFromBuildFile( + repoWorkspace, Label.EXTERNAL_PACKAGE_IDENTIFIER, ast, env, null); + if (importResult == null) { + return null; } + // TODO(dmarting): give a nice error message when redefining a repository name and + // getIndex() > 0. + parser.execute(ast, importResult.importMap); + } catch (PackageFunctionException | NameConflictException e) { + throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT); } - return new WorkspaceFileValue(builder.build(), workspaceRoot, 0, false); + return new WorkspaceFileValue( + builder.build(), + parser.getImportMap(), + parser.getVariableBindings(), + workspaceRoot, + key.getIndex(), + key.getIndex() < workspaceASTValue.getASTs().size() - 1); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileValue.java index 877d7c5e33..cb49728cbc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileValue.java @@ -14,14 +14,17 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; import java.util.Objects; /** @@ -84,12 +87,38 @@ public class WorkspaceFileValue implements SkyValue { private final int idx; private final RootedPath path; private final boolean hasNext; + // TODO(dmarting): The bindings bind to "Object" which we should ultimately replace by a super + // type in the Environment class (that would ease the serialization of this object). + private final ImmutableMap<String, Object> bindings; + private final ImmutableMap<String, Extension> importMap; - public WorkspaceFileValue(Package pkg, RootedPath path, int idx, boolean hasNext) { + /** + * Create a WorkspaceFileValue containing the various values necessary to compute the split + * WORKSPACE file. + * @param pkg Package built by agreggating all parts of the split WORKSPACE file up to this one. + * @param importMap List of imports (i.e., load statements) present in all parts of the split + * WORKSPACE file up to this one. + * @param bindings List of top-level variable bindings from the all parts of the split + * WORKSPACE file up to this one. The key is the name of the bindings and the value is the actual + * object. + * @param path The rooted path to workspace file to parse. + * @param idx The index of this part of the split WORKSPACE file (0 for the first one, 1 for the + * second one and so on). + * @param hasNext Is there a next part in the WORKSPACE file or this part the last one? + */ + public WorkspaceFileValue( + Package pkg, + Map<String, Extension> importMap, + Map<String, Object> bindings, + RootedPath path, + int idx, + boolean hasNext) { this.pkg = Preconditions.checkNotNull(pkg); this.idx = idx; this.path = path; this.hasNext = hasNext; + this.bindings = ImmutableMap.copyOf(bindings); + this.importMap = ImmutableMap.copyOf(importMap); } /** @@ -102,10 +131,10 @@ public class WorkspaceFileValue implements SkyValue { @Override public String toString() { - return "<WorkspaceFileValue idx=" + idx + ">"; + return "<WorkspaceFileValue path=" + path + " idx=" + idx + ">"; } - private static SkyKey key(RootedPath path, int idx) { + static SkyKey key(RootedPath path, int idx) { return new SkyKey(SkyFunctions.WORKSPACE_FILE, new WorkspaceFileKey(path, idx)); } @@ -149,4 +178,12 @@ public class WorkspaceFileValue implements SkyValue { public RootedPath getPath() { return path; } + + public ImmutableMap<String, Object> getBindings() { + return bindings; + } + + public ImmutableMap<String, Extension> getImportMap() { + return importMap; + } } |