diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe')
5 files changed, 58 insertions, 84 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java index fe44617e02..2b339046e0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java @@ -20,9 +20,6 @@ import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.syntax.BuildFileAST; -import com.google.devtools.build.lib.syntax.Mutability; -import com.google.devtools.build.lib.syntax.Runtime; -import com.google.devtools.build.lib.syntax.ValidationEnvironment; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; @@ -116,33 +113,26 @@ public class ASTFileLookupFunction implements SkyFunction { if (lookupResult == null) { return null; } + + BuildFileAST ast = null; if (!lookupResult.lookupSuccessful()) { return ASTFileLookupValue.noFile(); - } - BuildFileAST ast = null; - Path path = lookupResult.rootedPath().asPath(); - // Skylark files end with bzl. - boolean parseAsSkylark = astFilePathFragment.getPathString().endsWith(".bzl"); - try { - if (parseAsSkylark) { - try (Mutability mutability = Mutability.create("validate")) { - ast = BuildFileAST.parseSkylarkFile(path, env.getListener(), - packageManager, new ValidationEnvironment( - ruleClassProvider.createSkylarkRuleClassEnvironment( - mutability, - env.getListener(), - // the two below don't matter for extracting the ValidationEnvironment: - /*astFileContentHashCode=*/null, - /*importMap=*/null) - .setupDynamic(Runtime.PKG_NAME, Runtime.NONE))); - } - } else { - ast = BuildFileAST.parseBuildFile(path, env.getListener(), packageManager, false); - } - } catch (IOException e) { + } else { + Path path = lookupResult.rootedPath().asPath(); + // Skylark files end with bzl. + boolean parseAsSkylark = astFilePathFragment.getPathString().endsWith(".bzl"); + try { + ast = parseAsSkylark + ? BuildFileAST.parseSkylarkFile(path, env.getListener(), + packageManager, ruleClassProvider.getSkylarkValidationEnvironment().clone()) + : BuildFileAST.parseBuildFile(path, env.getListener(), + packageManager, false); + } catch (IOException e) { throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException( e.getMessage()), Transience.TRANSIENT); + } } + return ASTFileLookupValue.withFile(ast); } 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 ac3a6e9cbc..9e3b8d516b 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 @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.syntax.ParserInputSource; +import com.google.devtools.build.lib.syntax.SkylarkEnvironment; import com.google.devtools.build.lib.syntax.Statement; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.Path; @@ -552,8 +553,7 @@ public class PackageFunction implements SkyFunction { if (eventHandler.hasErrors()) { importResult = new SkylarkImportResult( - ImmutableMap.<PathFragment, com.google.devtools.build.lib.syntax.Environment>of(), - ImmutableList.<Label>of()); + ImmutableMap.<PathFragment, SkylarkEnvironment>of(), ImmutableList.<Label>of()); includeRepositoriesFetched = true; } else { importResult = @@ -581,7 +581,7 @@ public class PackageFunction implements SkyFunction { Environment env) throws PackageFunctionException { ImmutableMap<Location, PathFragment> imports = buildFileAST.getImports(); - Map<PathFragment, com.google.devtools.build.lib.syntax.Environment> importMap = new HashMap<>(); + Map<PathFragment, SkylarkEnvironment> importMap = new HashMap<>(); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); try { for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) { @@ -884,11 +884,9 @@ public class PackageFunction implements SkyFunction { /** A simple value class to store the result of the Skylark imports.*/ private static final class SkylarkImportResult { - private final Map<PathFragment, com.google.devtools.build.lib.syntax.Environment> importMap; + private final Map<PathFragment, SkylarkEnvironment> importMap; private final ImmutableList<Label> fileDependencies; - private SkylarkImportResult( - Map<PathFragment, - com.google.devtools.build.lib.syntax.Environment> importMap, + private SkylarkImportResult(Map<PathFragment, SkylarkEnvironment> 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 b3eb957019..0bd16ae90a 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 @@ -27,7 +27,7 @@ import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputE import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.syntax.Label.SyntaxException; -import com.google.devtools.build.lib.syntax.Mutability; +import com.google.devtools.build.lib.syntax.SkylarkEnvironment; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; @@ -76,7 +76,7 @@ public class SkylarkImportLookupFunction implements SkyFunction { throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.noFile(file)); } - Map<PathFragment, com.google.devtools.build.lib.syntax.Environment> importMap = new HashMap<>(); + Map<PathFragment, SkylarkEnvironment> importMap = new HashMap<>(); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); BuildFileAST ast = astLookupValue.getAST(); // TODO(bazel-team): Refactor this code and PackageFunction to reduce code duplications. @@ -114,11 +114,10 @@ public class SkylarkImportLookupFunction implements SkyFunction { file)); } - // Skylark UserDefinedFunction-s in that file will share this function definition Environment, - // which will be frozen by the time it is returned by createEnv. - com.google.devtools.build.lib.syntax.Environment extensionEnv = - createEnv(ast, file, importMap, env); - + SkylarkEnvironment extensionEnv = createEnv(ast, file, importMap, env); + // Skylark UserDefinedFunctions are sharing function definition Environments, so it's extremely + // important not to modify them from this point. Ideally they should be only used to import + // symbols and serve as global Environments of UserDefinedFunctions. return new SkylarkImportLookupValue( extensionEnv, new SkylarkFileDependency(label, fileDependencies.build())); } @@ -165,34 +164,29 @@ public class SkylarkImportLookupFunction implements SkyFunction { } /** - * Creates the Environment to be imported. - * After it's returned, the Environment must not be modified. + * Creates the SkylarkEnvironment to be imported. After it's returned, the Environment + * must not be modified. */ - private com.google.devtools.build.lib.syntax.Environment createEnv( - BuildFileAST ast, - PathFragment file, - Map<PathFragment, com.google.devtools.build.lib.syntax.Environment> importMap, - Environment env) + private SkylarkEnvironment createEnv(BuildFileAST ast, PathFragment file, + Map<PathFragment, SkylarkEnvironment> importMap, Environment env) throws InterruptedException, SkylarkImportLookupFunctionException { StoredEventHandler eventHandler = new StoredEventHandler(); // TODO(bazel-team): this method overestimates the changes which can affect the // Skylark RuleClass. For example changes to comments or unused functions can modify the hash. // A more accurate - however much more complicated - way would be to calculate a hash based on // the transitive closure of the accessible AST nodes. - try (Mutability mutability = Mutability.create("importing %s", file)) { - com.google.devtools.build.lib.syntax.Environment extensionEnv = - ruleClassProvider.createSkylarkRuleClassEnvironment( - mutability, eventHandler, ast.getContentHashCode(), importMap) - .setupOverride("native", packageFactory.getNativeModule()); - ast.exec(extensionEnv, eventHandler); - SkylarkRuleClassFunctions.exportRuleFunctions(extensionEnv, file); - - Event.replayEventsOn(env.getListener(), eventHandler.getEvents()); - if (eventHandler.hasErrors()) { - throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.errors(file)); - } - return extensionEnv; - } + SkylarkEnvironment extensionEnv = ruleClassProvider + .createSkylarkRuleClassEnvironment(eventHandler, ast.getContentHashCode()); + extensionEnv.update("native", packageFactory.getNativeModule()); + extensionEnv.setImportedExtensions(importMap); + ast.exec(extensionEnv, eventHandler); + SkylarkRuleClassFunctions.exportRuleFunctions(extensionEnv, file); + + Event.replayEventsOn(env.getListener(), eventHandler.getEvents()); + if (eventHandler.hasErrors()) { + throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.errors(file)); + } + return extensionEnv; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java index 86db05497d..020a426367 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java @@ -18,8 +18,8 @@ import com.google.common.base.Preconditions; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName; import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException; -import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.LoadStatement; +import com.google.devtools.build.lib.syntax.SkylarkEnvironment; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -30,7 +30,7 @@ import com.google.devtools.build.skyframe.SkyValue; */ public class SkylarkImportLookupValue implements SkyValue { - private final Environment importedEnvironment; + private final SkylarkEnvironment importedEnvironment; /** * The immediate Skylark file dependency descriptor class corresponding to this value. * Using this reference it's possible to reach the transitive closure of Skylark files @@ -39,15 +39,15 @@ public class SkylarkImportLookupValue implements SkyValue { private final SkylarkFileDependency dependency; public SkylarkImportLookupValue( - Environment importedEnvironment, SkylarkFileDependency dependency) { + SkylarkEnvironment importedEnvironment, SkylarkFileDependency dependency) { this.importedEnvironment = Preconditions.checkNotNull(importedEnvironment); this.dependency = Preconditions.checkNotNull(dependency); } /** - * Returns the imported Environment. + * Returns the imported SkylarkEnvironment. */ - public Environment getImportedEnvironment() { + public SkylarkEnvironment getImportedEnvironment() { return importedEnvironment; } 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 ce45ca2155..30f0c6a027 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 @@ -20,7 +20,6 @@ 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.syntax.EvalException; -import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -63,25 +62,18 @@ public class WorkspaceFileFunction implements SkyFunction { Path repoWorkspace = workspaceRoot.getRoot().getRelative(workspaceRoot.getRelativePath()); Builder builder = new Builder(repoWorkspace, packageFactory.getRuleClassProvider().getRunfilesPrefix()); - try (Mutability mutability = Mutability.create("workspace %s", repoWorkspace)) { - WorkspaceFactory parser = - new WorkspaceFactory( - builder, - packageFactory.getRuleClassProvider(), - mutability, - installDir.getPathString()); - parser.parse( - ParserInputSource.create( - ruleClassProvider.getDefaultWorkspaceFile(), new PathFragment("DEFAULT.WORKSPACE"))); - if (!workspaceFileValue.exists()) { - return new PackageValue(builder.build()); - } + WorkspaceFactory parser = new WorkspaceFactory( + builder, packageFactory.getRuleClassProvider(), installDir.getPathString()); + parser.parse(ParserInputSource.create( + ruleClassProvider.getDefaultWorkspaceFile(), new PathFragment("DEFAULT.WORKSPACE"))); + if (!workspaceFileValue.exists()) { + return new PackageValue(builder.build()); + } - try { - parser.parse(ParserInputSource.create(repoWorkspace)); - } catch (IOException e) { - throw new WorkspaceFileFunctionException(e, Transience.TRANSIENT); - } + try { + parser.parse(ParserInputSource.create(repoWorkspace)); + } catch (IOException e) { + throw new WorkspaceFileFunctionException(e, Transience.TRANSIENT); } return new PackageValue(builder.build()); |