From 2c03dee64d587f6919683cfa8c53cbf40a4ba42c Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 27 Dec 2023 16:10:10 -0800 Subject: [PATCH] Factor StarlarkSemantics usage out of Package.java The two Package.Builder factory methods take a StarlarkSemantics just to extract a single boolean flag. This CL has the callers do it instead, to make the Package.Builder API more about individual features than Starlark environments. By contrast, the wrapper methods in PackageFactory are left unchanged, because PackageFactory is already knee-deep in Starlark logic thanks to executeBuildFile[Impl](). Minor change spun out of unknown commit. Work toward #19922. PiperOrigin-RevId: 594115666 Change-Id: I7af4388f35b6393a3d8debd829452fe5fd633d54 --- .../com/google/devtools/build/lib/bazel/bzlmod/BUILD | 1 + .../build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java | 3 ++- .../google/devtools/build/lib/packages/Package.java | 10 ++++------ .../devtools/build/lib/packages/PackageFactory.java | 2 +- .../starlark/StarlarkRepositoryContextTest.java | 2 +- .../java/com/google/devtools/build/lib/packages/BUILD | 1 + .../build/lib/packages/WorkspaceFactoryTestHelper.java | 4 +++- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 14336b7768e24c..6a2fbabeefbc31 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -263,6 +263,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java index 64501f92c8a370..5e2d3f1ddcef20 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.packages.RuleFactory; import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import java.util.Map; @@ -66,7 +67,7 @@ public static Rule createRule( RootedPath.toRootedPath( Root.fromPath(directories.getWorkspace()), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), - semantics, + semantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), basePackageId, repoMapping); BuildLangTypedAttributeValuesMap attributeValues = 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 33afa49bb0c431..0cc0c6ff7536c0 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 @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; @@ -78,7 +77,6 @@ import java.util.concurrent.Semaphore; import javax.annotation.Nullable; import net.starlark.java.eval.Module; -import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.eval.StarlarkThread; import net.starlark.java.syntax.Location; @@ -730,7 +728,7 @@ public static Builder newExternalPackageBuilder( RootedPath workspacePath, String workspaceName, RepositoryMapping mainRepoMapping, - StarlarkSemantics starlarkSemantics, + boolean noImplicitFileExport, PackageOverheadEstimator packageOverheadEstimator) { return new Builder( helper, @@ -738,7 +736,7 @@ public static Builder newExternalPackageBuilder( workspaceName, /* associatedModuleName= */ Optional.empty(), /* associatedModuleVersion= */ Optional.empty(), - starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), + noImplicitFileExport, mainRepoMapping, mainRepoMapping, /* cpuBoundSemaphore= */ null, @@ -748,7 +746,7 @@ public static Builder newExternalPackageBuilder( public static Builder newExternalPackageBuilderForBzlmod( RootedPath moduleFilePath, - StarlarkSemantics starlarkSemantics, + boolean noImplicitFileExport, PackageIdentifier basePackageId, RepositoryMapping repoMapping) { return new Builder( @@ -757,7 +755,7 @@ public static Builder newExternalPackageBuilderForBzlmod( DUMMY_WORKSPACE_NAME_FOR_BZLMOD_PACKAGES, /* associatedModuleName= */ Optional.empty(), /* associatedModuleVersion= */ Optional.empty(), - starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), + noImplicitFileExport, repoMapping, // This mapping is *not* the main repository's mapping, but since it is only used to // construct a query command in an error message and the package built here can't be 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 6d3b20e1b9e2da..13484e2c66bd7d 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 @@ -232,7 +232,7 @@ public Package.Builder newExternalPackageBuilder( workspacePath, workspaceName, mainRepoMapping, - starlarkSemantics, + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), packageOverheadEstimator); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java index 72126bb2ba50f8..c12cb185e6216e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java @@ -140,7 +140,7 @@ private void setUpContextForRule( RootedPath.toRootedPath(root, workspaceFile), "runfiles", RepositoryMapping.ALWAYS_FALLBACK, - starlarkSemantics, + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), PackageOverheadEstimator.NOOP_ESTIMATOR); ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class); Rule rule = diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD index 0a69e7d6428018..d505f5448d59b6 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/BUILD +++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD @@ -22,6 +22,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java index 96e0ac74a000d3..fa8ada16e696ff 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java +++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; @@ -70,7 +71,8 @@ void parse(String... args) { RootedPath.toRootedPath(root, workspaceFilePath), "", RepositoryMapping.ALWAYS_FALLBACK, - StarlarkSemantics.DEFAULT, + starlarkSemantics.getBool( + BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), PackageOverheadEstimator.NOOP_ESTIMATOR) .setLoads(ImmutableList.of()); WorkspaceFactory factory =