diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java index aff6ab94683c78..514ba54badbee3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java @@ -24,8 +24,11 @@ import com.google.devtools.build.lib.packages.StarlarkProviderWrapper; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Starlark; /** * Utility functions for use during analysis. @@ -73,6 +76,44 @@ public static boolean isStampingEnabled(RuleContext ruleContext) { return isStampingEnabled(ruleContext, ruleContext.getConfiguration()); } + /** + * Returns whether workspace status files ({@code ctx.info_file} / {@code ctx.version_file}) may + * be accessed for the given rule. + */ + public static boolean areWorkspaceStatusFilesAvailable(RuleContext ruleContext) { + BuildConfigurationValue config = ruleContext.getConfiguration(); + if (config.isToolConfiguration()) { + return false; + } + if (ruleContext.attributes().has("stamp", BuildType.TRISTATE) + || ruleContext.attributes().has("stamp", Type.INTEGER)) { + return isStampingEnabled(ruleContext, config); + } + return config.stampBinaries(); + } + + /** + * Verifies that workspace status files may be accessed, or fails with an error pointing at the + * calling rule implementation. + */ + public static void checkWorkspaceStatusFileAccess(RuleContext ruleContext, String apiName) + throws EvalException { + if (!ruleContext + .getAnalysisEnvironment() + .getStarlarkSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_PREVENT_STATUS_FILES_WITHOUT_STAMP)) { + return; + } + if (areWorkspaceStatusFilesAvailable(ruleContext)) { + return; + } + throw Starlark.errorf( + "%s cannot be accessed without stamping when" + + " --incompatible_prevent_status_files_without_stamp is enabled. Enable stamping with" + + " the stamp attribute or --stamp.", + apiName); + } + // TODO(bazel-team): These need Iterable because they need to // be called with Iterable. Once the configured target lockdown is complete, we // can eliminate the "extends" clauses. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index d00394c5a815f4..6a5e8a958bbfbd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; import com.google.devtools.build.lib.actions.extra.SpawnInfo; +import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.BashCommandConstructor; import com.google.devtools.build.lib.analysis.CommandHelper; import com.google.devtools.build.lib.analysis.FilesToRunProvider; @@ -490,6 +491,9 @@ private Artifact transformBuildInfoFile( StarlarkThread thread) throws InterruptedException, EvalException { RuleContext ruleContext = getRuleContext(); + String apiName = + isVolatile ? "ctx.actions.transform_version_file" : "ctx.actions.transform_info_file"; + AnalysisUtils.checkWorkspaceStatusFileAccess(ruleContext, apiName); Artifact templateFile = (Artifact) templateObject; PathFragment fragment = ruleContext.getPackageDirectory().getRelative(PathFragment.create(outputFileName)); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java index 81879cba5c6522..edf8e6e9f6f3a2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.analysis.ActionsProvider; import com.google.devtools.build.lib.analysis.AliasProvider; +import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.AspectContext; import com.google.devtools.build.lib.analysis.BashCommandConstructor; import com.google.devtools.build.lib.analysis.CommandHelper; @@ -982,12 +983,14 @@ public boolean areRunfilesFromDeps(FilesToRunProvider executable) { @Override public Artifact getStableWorkspaceStatus() throws InterruptedException, EvalException { checkMutable("info_file"); + AnalysisUtils.checkWorkspaceStatusFileAccess(ruleContext, "ctx.info_file"); return ruleContext.getAnalysisEnvironment().getStableWorkspaceStatusArtifact(); } @Override public Artifact getVolatileWorkspaceStatus() throws InterruptedException, EvalException { checkMutable("version_file"); + AnalysisUtils.checkWorkspaceStatusFileAccess(ruleContext, "ctx.version_file"); return ruleContext.getAnalysisEnvironment().getVolatileWorkspaceStatusArtifact(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 658c5f5ec6e58d..f5b18fd836fd5a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -681,6 +681,20 @@ public final class BuildLanguageOptions extends OptionsBase { + "from the top level target instead") public boolean incompatibleDisableObjcLibraryTransition; + @Option( + name = "incompatible_prevent_status_files_without_stamp", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If enabled, ctx.info_file and ctx.version_file (and the corresponding" + + " ctx.actions.transform_*_file functions) are only available when stamping is" + + " enabled for the target (via the stamp attribute or --stamp). Otherwise, rule" + + " implementations that access these files fail during analysis. See" + + " https://github.com/bazelbuild/bazel/issues/14341.") + public boolean incompatiblePreventStatusFilesWithoutStamp; + // remove after Bazel LTS in Nov 2023 @Option( name = "incompatible_fail_on_unknown_attributes", @@ -842,6 +856,9 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool( INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION, incompatibleDisableObjcLibraryTransition) + .setBool( + INCOMPATIBLE_PREVENT_STATUS_FILES_WITHOUT_STAMP, + incompatiblePreventStatusFilesWithoutStamp) .setBool(INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES, incompatibleFailOnUnknownAttributes) .setBool( INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION, @@ -943,6 +960,8 @@ public StarlarkSemantics toStarlarkSemantics() { "+incompatible_objc_provider_remove_linking_info"; public static final String INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION = "+incompatible_disable_objc_library_transition"; + public static final String INCOMPATIBLE_PREVENT_STATUS_FILES_WITHOUT_STAMP = + "-incompatible_prevent_status_files_without_stamp"; public static final String INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES = "+incompatible_fail_on_unknown_attributes"; public static final String INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java index ee1ed9385cbf1f..e95871cc8da3c4 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.analysis.actions.BuildInfoFileWriteAction; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.actions.StarlarkAction; +import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget; import com.google.devtools.build.lib.analysis.starlark.StarlarkExecGroupCollection; import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext; @@ -4081,4 +4082,115 @@ public void transformFile_cannotBeAccessedOutsideOfAllowlist( " template = ':template.txt',", ")"); } + + @Test + public void infoFile_allowedWithoutStampByDefault() throws Exception { + scratch.file( + "test/rules.bzl", + "def _impl(ctx):", + " ctx.actions.write(ctx.outputs.out, ctx.info_file.path)", + " return DefaultInfo(files = depset([ctx.outputs.out]))", + "status_rule = rule(", + " implementation = _impl,", + " attrs = {\"stamp\": attr.int(default = -1)},", + " outputs = {\"out\": \"%{name}.txt\"},", + ")", + testingRuleDefinition); + scratch.file( + "test/BUILD", + "load(':rules.bzl', 'status_rule')", + "status_rule(name = 'target')"); + + getConfiguredTarget("//test:target"); + } + + @Test + public void infoFile_errorsWithoutStampWhenPrevented() throws Exception { + setBuildLanguageOptions("--incompatible_prevent_status_files_without_stamp"); + scratch.file( + "test/rules.bzl", + "def _impl(ctx):", + " ctx.actions.write(ctx.outputs.out, ctx.info_file.path)", + " return DefaultInfo(files = depset([ctx.outputs.out]))", + "status_rule = rule(", + " implementation = _impl,", + " attrs = {\"stamp\": attr.int(default = -1)},", + " outputs = {\"out\": \"%{name}.txt\"},", + ")", + testingRuleDefinition); + scratch.file( + "test/BUILD", + "load(':rules.bzl', 'status_rule')", + "status_rule(name = 'target')"); + + checkError( + "//test:target", + "ctx.info_file cannot be accessed without stamping", + "incompatible_prevent_status_files_without_stamp"); + } + + @Test + public void infoFile_allowedWithStampWhenPrevented() throws Exception { + setBuildLanguageOptions("--incompatible_prevent_status_files_without_stamp", "--stamp"); + scratch.file( + "test/rules.bzl", + "def _impl(ctx):", + " ctx.actions.write(ctx.outputs.out, ctx.info_file.path)", + " return DefaultInfo(files = depset([ctx.outputs.out]))", + "status_rule = rule(", + " implementation = _impl,", + " attrs = {\"stamp\": attr.int(default = 1)},", + " outputs = {\"out\": \"%{name}.txt\"},", + ")", + testingRuleDefinition); + scratch.file( + "test/BUILD", + "load(':rules.bzl', 'status_rule')", + "status_rule(name = 'target')"); + + getConfiguredTarget("//test:target"); + } + + @Test + public void infoFile_allowedWhenStampEnabledByDependencyTransition() throws Exception { + setBuildLanguageOptions("--incompatible_prevent_status_files_without_stamp"); + useConfiguration("--nostamp"); + scratch.file( + "test/rules.bzl", + "def _stamp_transition_impl(settings, _attr):", + " return {'//command_line_option:stamp': not settings['//command_line_option:stamp']}", + "stamp_transition = transition(", + " implementation = _stamp_transition_impl,", + " inputs = ['//command_line_option:stamp'],", + " outputs = ['//command_line_option:stamp'],", + ")", + "def _stamped_dep_impl(ctx):", + " ctx.actions.write(ctx.outputs.out, ctx.info_file.path)", + " return DefaultInfo(files = depset([ctx.outputs.out]))", + "stamped_dep = rule(", + " implementation = _stamped_dep_impl,", + " outputs = {\"out\": \"%{name}.txt\"},", + ")", + "def _consumer_impl(ctx):", + " return DefaultInfo()", + "consumer = rule(", + " implementation = _consumer_impl,", + " attrs = {\"dep\": attr.label(cfg = stamp_transition)},", + ")", + testingRuleDefinition); + scratch.file( + "test/BUILD", + "load(':rules.bzl', 'consumer', 'stamped_dep')", + "stamped_dep(name = 'dep')", + "consumer(name = 'top', dep = ':dep')"); + + ConfiguredTarget top = getConfiguredTarget("//test:top"); + ConfiguredTarget dep = Iterables.getOnlyElement(getPrerequisites(top, "dep")); + + assertThat(getConfiguration(top).getOptions().get(CoreOptions.class).stampBinaries) + .isFalse(); + assertThat(getConfiguration(dep).getOptions().get(CoreOptions.class).stampBinaries) + .isTrue(); + assertThat(getFilesToBuild(dep)).isNotEmpty(); + } } diff --git a/src/test/shell/bazel/bazel_workspace_status_test.sh b/src/test/shell/bazel/bazel_workspace_status_test.sh index ebac7f696493da..ecb4bb4df05a78 100755 --- a/src/test/shell/bazel/bazel_workspace_status_test.sh +++ b/src/test/shell/bazel/bazel_workspace_status_test.sh @@ -258,4 +258,40 @@ EOF } +function test_prevent_status_files_without_stamp() { + create_new_workspace + + cat > rules.bzl <<'EOF' +def _impl(ctx): + ctx.actions.write(ctx.outputs.out, ctx.info_file.path) + return DefaultInfo(files = depset([ctx.outputs.out])) + +uses_status = rule( + implementation = _impl, + attrs = {"stamp": attr.int(default = -1)}, + outputs = {"out": "%{name}.txt"}, +) +EOF + + cat > BUILD <<'EOF' +load(":rules.bzl", "uses_status") + +uses_status(name = "unstamped") +uses_status(name = "stamped", stamp = 1) +EOF + + # By default, status files are available without stamping. + bazel build //:unstamped &> $TEST_log || fail "expected build to succeed by default" + + # With the incompatible flag enabled, unstamped targets fail and point at the rule implementation. + bazel build --incompatible_prevent_status_files_without_stamp //:unstamped &> $TEST_log \ + && fail "expected build to fail" || true + expect_log "ctx.info_file cannot be accessed without stamping" + expect_log "rules.bzl" + + # Stamped targets succeed with the incompatible flag enabled. + bazel build --incompatible_prevent_status_files_without_stamp --stamp //:stamped \ + &> $TEST_log || fail "expected stamped build to succeed" +} + run_suite "workspace status tests"