diff options
Diffstat (limited to 'src')
12 files changed, 118 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index dd0df6a4dd..8a666b0ecf 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -209,7 +209,7 @@ public class Package { * @precondition {@code name} must be a suffix of * {@code filename.getParentDirectory())}. */ - private Package(PackageIdentifier packageId, String runfilesPrefix) { + protected Package(PackageIdentifier packageId, String runfilesPrefix) { this.packageIdentifier = packageId; this.workspaceName = runfilesPrefix; this.nameFragment = Canonicalizer.fragments().intern(packageId.getPackageFragment()); @@ -531,13 +531,23 @@ public class Package { suffix = ""; } + throw makeNoSuchTargetException(targetName, suffix); + } + + protected NoSuchTargetException makeNoSuchTargetException(String targetName, String suffix) { + Label label; try { - throw new NoSuchTargetException(createLabel(targetName), "target '" + targetName - + "' not declared in package '" + name + "'" + suffix + " defined by " - + this.filename); + label = createLabel(targetName); } catch (LabelSyntaxException e) { throw new IllegalArgumentException(targetName); } + String msg = String.format( + "target '%s' not declared in package '%s'%s defined by %s", + targetName, + name, + suffix, + filename); + return new NoSuchTargetException(label, msg); } /** @@ -666,16 +676,36 @@ public class Package { } } - public static Builder newExternalPackageBuilder(Path workspacePath, String runfilesPrefix) { - Builder b = new Builder(Label.EXTERNAL_PACKAGE_IDENTIFIER, runfilesPrefix); + public static Builder newExternalPackageBuilder(Builder.Helper helper, Path workspacePath, + String runfilesPrefix) { + Builder b = new Builder(helper.createFreshPackage( + Label.EXTERNAL_PACKAGE_IDENTIFIER, runfilesPrefix)); b.setFilename(workspacePath); b.setMakeEnv(new MakeEnvironment.Builder()); return b; } + /** A builder for {@link Package} objects. Only intended to be used by {@link PackageFactory}. */ public static class Builder { - protected static Package newPackage(PackageIdentifier packageId, String runfilesPrefix) { - return new Package(packageId, runfilesPrefix); + public static interface Helper { + /** + * Returns a fresh {@link Package} instance that a {@link Builder} will internally mutate + * during package loading. + */ + Package createFreshPackage(PackageIdentifier packageId, String runfilesPrefix); + } + + /** {@link Helper} that simply calls the {@link Package} constructor. */ + public static class DefaultHelper implements Helper { + public static final DefaultHelper INSTANCE = new DefaultHelper(); + + private DefaultHelper() { + } + + @Override + public Package createFreshPackage(PackageIdentifier packageId, String runfilesPrefix) { + return new Package(packageId, runfilesPrefix); + } } /** @@ -740,8 +770,8 @@ public class Package { } } - public Builder(PackageIdentifier id, String runfilesPrefix) { - this(newPackage(id, runfilesPrefix)); + public Builder(Helper helper, PackageIdentifier id, String runfilesPrefix) { + this(helper.createFreshPackage(id, runfilesPrefix)); } protected PackageIdentifier getPackageIdentifier() { 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 b1ad3558dc..73ff55df03 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 @@ -331,6 +331,8 @@ public final class PackageFactory { private final ImmutableList<EnvironmentExtension> environmentExtensions; private final ImmutableMap<String, PackageArgument<?>> packageArguments; + private final Package.Builder.Helper packageBuilderHelper; + /** * Constructs a {@code PackageFactory} instance with the given rule factory. */ @@ -341,7 +343,8 @@ public final class PackageFactory { null, AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY, ImmutableList.<EnvironmentExtension>of(), - "test"); + "test", + Package.Builder.DefaultHelper.INSTANCE); } @VisibleForTesting @@ -352,7 +355,8 @@ public final class PackageFactory { null, AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY, ImmutableList.of(environmentExtension), - "test"); + "test", + Package.Builder.DefaultHelper.INSTANCE); } @VisibleForTesting @@ -363,7 +367,8 @@ public final class PackageFactory { null, AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY, environmentExtensions, - "test"); + "test", + Package.Builder.DefaultHelper.INSTANCE); } /** @@ -375,7 +380,8 @@ public final class PackageFactory { Map<String, String> platformSetRegexps, Function<RuleClass, AttributeContainer> attributeContainerFactory, Iterable<EnvironmentExtension> environmentExtensions, - String version) { + String version, + Package.Builder.Helper packageBuilderHelper) { this.platformSetRegexps = platformSetRegexps; this.ruleFactory = new RuleFactory(ruleClassProvider, attributeContainerFactory); this.ruleClassProvider = ruleClassProvider; @@ -388,6 +394,7 @@ public final class PackageFactory { this.packageArguments = createPackageArguments(); this.nativeModule = newNativeModule(); this.workspaceNativeModule = WorkspaceFactory.newNativeModule(ruleClassProvider, version); + this.packageBuilderHelper = packageBuilderHelper; } /** @@ -1276,14 +1283,24 @@ public final class PackageFactory { } @VisibleForTesting + public Package.Builder newExternalPackageBuilder(Path workspacePath, String runfilesPrefix) { + return Package.newExternalPackageBuilder(packageBuilderHelper, workspacePath, runfilesPrefix); + } + + @VisibleForTesting + public Package.Builder newPackageBuilder(PackageIdentifier packageId, String runfilesPrefix) { + return new Package.Builder(packageBuilderHelper, packageId, runfilesPrefix); + } + + @VisibleForTesting public Package createPackageForTesting( PackageIdentifier packageId, Path buildFile, CachingPackageLocator locator, EventHandler eventHandler) throws NoSuchPackageException, InterruptedException { - Package externalPkg = - Package.newExternalPackageBuilder(buildFile.getRelative("WORKSPACE"), "TESTING").build(); + Package externalPkg = newExternalPackageBuilder( + buildFile.getRelative("WORKSPACE"), "TESTING").build(); return createPackageForTesting(packageId, externalPkg, buildFile, locator, eventHandler); } @@ -1533,8 +1550,8 @@ public final class PackageFactory { Map<String, Extension> imports, ImmutableList<Label> skylarkFileDependencies) throws InterruptedException { - Package.Builder pkgBuilder = new Package.Builder( - packageId, ruleClassProvider.getRunfilesPrefix()); + Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage( + packageId, ruleClassProvider.getRunfilesPrefix())); StoredEventHandler eventHandler = new StoredEventHandler(); try (Mutability mutability = Mutability.create("package %s", packageId)) { @@ -1621,8 +1638,8 @@ public final class PackageFactory { .setPhase(Phase.LOADING) .build(); - Package.Builder pkgBuilder = new Package.Builder(packageId, - ruleClassProvider.getRunfilesPrefix()); + Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage( + packageId, ruleClassProvider.getRunfilesPrefix())); pkgBuilder.setFilename(buildFilePath) .setMakeEnv(pkgMakeEnv) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java index 25f692a723..743cff57b1 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.exec.OutputService; import com.google.devtools.build.lib.packages.AttributeContainer; import com.google.devtools.build.lib.packages.NoSuchThingException; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Preprocessor; import com.google.devtools.build.lib.packages.RuleClass; @@ -342,6 +343,15 @@ public abstract class BlazeModule { } /** + * Returns a helper that the {@link PackageFactory} will use during package loading. If the module + * does not provide any heloer, it should return null. Note that only one helper per Bazel/Blaze + * runtime is allowed. + */ + public Package.Builder.Helper getPackageBuilderHelper() { + return null; + } + + /** * Returns a factory for creating {@link AbstractBlazeQueryEnvironment} objects. * If the module does not provide any {@link QueryEnvironmentFactory}, it should return null. Note * that only one factory per Bazel/Blaze runtime is allowed. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 8c9af364ce..ce6b1f5c0d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.OutputFilter; import com.google.devtools.build.lib.flags.CommandNameCache; import com.google.devtools.build.lib.packages.AttributeContainer; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Preprocessor; import com.google.devtools.build.lib.packages.RuleClass; @@ -1260,13 +1261,27 @@ public final class BlazeRuntime { extensions.add(module.getPackageEnvironmentExtension()); } + Package.Builder.Helper packageBuilderHelper = null; + for (BlazeModule module : blazeModules) { + Package.Builder.Helper candidateHelper = module.getPackageBuilderHelper(); + if (candidateHelper != null) { + Preconditions.checkState(packageBuilderHelper == null, + "more than one module defines a package builder helper"); + packageBuilderHelper = candidateHelper; + } + } + if (packageBuilderHelper == null) { + packageBuilderHelper = Package.Builder.DefaultHelper.INSTANCE; + } + PackageFactory packageFactory = new PackageFactory( ruleClassProvider, platformRegexps, attributeContainerFactory, extensions, - BlazeVersionInfo.instance().getVersion()); + BlazeVersionInfo.instance().getVersion(), + packageBuilderHelper); if (configurationFactory == null) { configurationFactory = new ConfigurationFactory( 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 c84e42049f..87ec20c159 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 @@ -67,8 +67,8 @@ public class WorkspaceFileFunction implements SkyFunction { } Path repoWorkspace = workspaceRoot.getRoot().getRelative(workspaceRoot.getRelativePath()); - Package.Builder builder = - Package.newExternalPackageBuilder(repoWorkspace, ruleClassProvider.getRunfilesPrefix()); + Package.Builder builder = packageFactory.newExternalPackageBuilder( + repoWorkspace, ruleClassProvider.getRunfilesPrefix()); if (workspaceASTValue.getASTs().isEmpty()) { return new WorkspaceFileValue( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java index 246b13d26d..cbfa252525 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java @@ -79,7 +79,8 @@ public class SkylarkRepositoryContextTest { protected void setUpContextForRule(Map<String, Object> kwargs, Attribute... attributes) throws Exception { - Package.Builder packageBuilder = Package.newExternalPackageBuilder(workspaceFile, "runfiles"); + Package.Builder packageBuilder = Package.newExternalPackageBuilder( + Package.Builder.DefaultHelper.INSTANCE, workspaceFile, "runfiles"); FuncallExpression ast = new FuncallExpression(new Identifier("test"), ImmutableList.<Passed>of()); ast.setLocation(Location.BUILTIN); diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index 6185190035..24965818f0 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -50,7 +50,6 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.events.Location.LineAndColumn; import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; -import com.google.devtools.build.lib.packages.Package.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.RuleClass.Configurator; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory; @@ -261,7 +260,8 @@ public class RuleClassTest extends PackageLoadingTestCase { } private Package.Builder createDummyPackageBuilder() { - return new Builder(PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME), "TESTING") + return packageFactory.newPackageBuilder( + PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME), "TESTING") .setFilename(testBuildfilePath) .setMakeEnv(new MakeEnvironment.Builder()); } diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java index a716e22f8a..95d3c45466 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java @@ -53,7 +53,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { public void testCreateRule() throws Exception { Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD"); Package.Builder pkgBuilder = - new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") + packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") .setFilename(myPkgPath) .setMakeEnv(new MakeEnvironment.Builder()); @@ -116,7 +116,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { @Test public void testCreateWorkspaceRule() throws Exception { Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE"); - Package.Builder pkgBuilder = Package.newExternalPackageBuilder(myPkgPath, "TESTING"); + Package.Builder pkgBuilder = packageFactory.newExternalPackageBuilder(myPkgPath, "TESTING"); Map<String, Object> attributeValues = new HashMap<>(); attributeValues.put("name", "foo"); @@ -140,7 +140,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { public void testWorkspaceRuleFailsInBuildFile() throws Exception { Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD"); Package.Builder pkgBuilder = - new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") + packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") .setFilename(myPkgPath) .setMakeEnv(new MakeEnvironment.Builder()); @@ -169,7 +169,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { public void testBuildRuleFailsInWorkspaceFile() throws Exception { Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE"); Package.Builder pkgBuilder = - new Package.Builder(Label.EXTERNAL_PACKAGE_IDENTIFIER, "TESTING") + packageFactory.newPackageBuilder(Label.EXTERNAL_PACKAGE_IDENTIFIER, "TESTING") .setFilename(myPkgPath) .setMakeEnv(new MakeEnvironment.Builder()); @@ -210,7 +210,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { public void testOutputFileNotEqualDot() throws Exception { Path myPkgPath = scratch.resolve("/foo"); Package.Builder pkgBuilder = - new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") + packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") .setFilename(myPkgPath) .setMakeEnv(new MakeEnvironment.Builder()); @@ -243,7 +243,7 @@ public class RuleFactoryTest extends PackageLoadingTestCase { public void testTestRules() throws Exception { Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD"); Package pkg = - new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") + packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING") .setFilename(myPkgPath) .setMakeEnv(new MakeEnvironment.Builder()) .build(); diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java index 8bcbd5a53e..cd0e3d2d8c 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java @@ -115,7 +115,8 @@ public class WorkspaceFactoryTest { fail("Shouldn't happen: " + e.getMessage()); } StoredEventHandler eventHandler = new StoredEventHandler(); - builder = Package.newExternalPackageBuilder(workspaceFilePath, ""); + builder = Package.newExternalPackageBuilder( + Package.Builder.DefaultHelper.INSTANCE, workspaceFilePath, ""); this.factory = new WorkspaceFactory( builder, TestRuleClassProvider.getRuleClassProvider(), 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 ac3f401dc1..ddda9ff709 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 @@ -57,7 +57,8 @@ public class PackageFactoryApparatus { null, AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY, ImmutableList.copyOf(environmentExtensions), - "test"); + "test", + Package.Builder.DefaultHelper.INSTANCE); } /** @@ -123,7 +124,7 @@ public class PackageFactoryApparatus { TestUtils.getPool()); LegacyGlobber globber = new LegacyGlobber(globCache); Package externalPkg = - Package.newExternalPackageBuilder( + factory.newExternalPackageBuilder( buildFile.getParentDirectory().getRelative("WORKSPACE"), "TESTING") .build(); Builder resultBuilder = diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java index 4cbff5222b..e160d6d696 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java @@ -69,6 +69,7 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { private static final int GLOBBING_THREADS = 7; protected ConfiguredRuleClassProvider ruleClassProvider; + protected PackageFactory packageFactory; protected SkyframeExecutor skyframeExecutor; @Before @@ -84,8 +85,8 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { } else { ruleClassProvider = TestRuleClassProvider.getRuleClassProvider(); } - skyframeExecutor = createSkyframeExecutor(getEnvironmentExtensions(), - getPreprocessorFactorySupplier()); + packageFactory = new PackageFactory(ruleClassProvider, getEnvironmentExtensions()); + skyframeExecutor = createSkyframeExecutor(getPreprocessorFactorySupplier()); setUpSkyframe(parsePackageCacheOptions()); } @@ -95,11 +96,10 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { } private SkyframeExecutor createSkyframeExecutor( - Iterable<EnvironmentExtension> environmentExtensions, Preprocessor.Factory.Supplier preprocessorFactorySupplier) { SkyframeExecutor skyframeExecutor = SequencedSkyframeExecutor.create( - new PackageFactory(ruleClassProvider, environmentExtensions), + packageFactory, new BlazeDirectories(outputBase, outputBase, rootDirectory, TestConstants.PRODUCT_NAME), null, /* BinTools */ null, /* workspaceStatusActionFactory */ diff --git a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java index 60a347c80b..53af210cce 100644 --- a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java +++ b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java @@ -76,8 +76,10 @@ public class WorkspaceResolver { * Converts the WORKSPACE file content into an ExternalPackage. */ public Package parse(Path workspacePath) { - Package.Builder builder = - Package.newExternalPackageBuilder(workspacePath, ruleClassProvider.getRunfilesPrefix()); + Package.Builder builder = Package.newExternalPackageBuilder( + Package.Builder.DefaultHelper.INSTANCE, + workspacePath, + ruleClassProvider.getRunfilesPrefix()); try (Mutability mutability = Mutability.create("External Package %s", workspacePath)) { new WorkspaceFactory(builder, ruleClassProvider, environmentExtensions, mutability) .parse(ParserInputSource.create(workspacePath)); |