Skip to content

Commit

Permalink
Factor StarlarkSemantics usage out of Package.java
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brandjon authored and copybara-github committed Dec 28, 2023
1 parent 4c4a1a7 commit 2c03dee
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -730,15 +728,15 @@ public static Builder newExternalPackageBuilder(
RootedPath workspacePath,
String workspaceName,
RepositoryMapping mainRepoMapping,
StarlarkSemantics starlarkSemantics,
boolean noImplicitFileExport,
PackageOverheadEstimator packageOverheadEstimator) {
return new Builder(
helper,
LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
workspaceName,
/* associatedModuleName= */ Optional.empty(),
/* associatedModuleVersion= */ Optional.empty(),
starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
noImplicitFileExport,
mainRepoMapping,
mainRepoMapping,
/* cpuBoundSemaphore= */ null,
Expand All @@ -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(
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public Package.Builder newExternalPackageBuilder(
workspacePath,
workspaceName,
mainRepoMapping,
starlarkSemantics,
starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
packageOverheadEstimator);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/packages/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit 2c03dee

Please sign in to comment.