Skip to content

Commit

Permalink
[6.1.0] Rerun the artifact conflict check when --incompatible_strict_…
Browse files Browse the repository at this point in the history
…conflict_checks changes. (bazelbuild#17592)

Related to bazelbuild#16729.

PiperOrigin-RevId: 511432374
Change-Id: I00b0bff5731a3468bf0a56c4a44e95590da7b463

Co-authored-by: kshyanashree <[email protected]>
  • Loading branch information
tjgq and ShreeM01 authored Feb 28, 2023
1 parent aafe123 commit 938e348
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,6 @@ public class AnalysisOptions extends OptionsBase {
help = "Deprecated. No-op.")
public boolean skyframePrepareAnalysis;

@Option(
name = "incompatible_strict_conflict_checks",
oldName = "experimental_strict_conflict_checks",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
metadataTags = OptionMetadataTag.INCOMPATIBLE_CHANGE,
effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
help =
"Check for action prefix file path conflicts, regardless of action-specific overrides.")
public boolean strictConflictChecks;

@Option(
name = "experimental_skyframe_cpu_heavy_skykeys_thread_pool_size",
defaultValue = "HOST_CPUS",
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@ java_library(
":blaze_directories",
":config/build_configuration",
":config/build_options",
":config/core_options",
":config/invalid_configuration_exception",
":configured_target",
":constraints/platform_restrictions_result",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver.TopLevelTargetsAndConfigsResult;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.constraints.PlatformRestrictionsResult;
import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
Expand Down Expand Up @@ -400,7 +401,7 @@ public AnalysisResult update(
bugReporter,
keepGoing,
loadingPhaseThreads,
viewOptions.strictConflictChecks,
targetOptions.get(CoreOptions.class).strictConflictChecks,
checkForActionConflicts,
viewOptions.cpuHeavySkyKeysThreadPoolSize);
setArtifactRoots(skyframeAnalysisResult.getPackageRoots());
Expand All @@ -426,7 +427,7 @@ public AnalysisResult update(
Preconditions.checkNotNull(resourceManager), // non-null for skymeld.
Preconditions.checkNotNull(buildResultListener), // non-null for skymeld.
keepGoing,
viewOptions.strictConflictChecks,
targetOptions.get(CoreOptions.class).strictConflictChecks,
checkForActionConflicts,
loadingPhaseThreads,
viewOptions.cpuHeavySkyKeysThreadPoolSize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
+ "disabled.")
public boolean strictFilesets;

@Option(
name = "incompatible_strict_conflict_checks",
oldName = "experimental_strict_conflict_checks",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
metadataTags = OptionMetadataTag.INCOMPATIBLE_CHANGE,
effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
help =
"Check for action prefix file path conflicts, regardless of action-specific overrides.")
public boolean strictConflictChecks;

@Option(
name = "experimental_strict_fileset_output",
defaultValue = "false",
Expand Down Expand Up @@ -959,6 +970,7 @@ public FragmentOptions getHost() {
host.includeRequiredConfigFragmentsProvider = includeRequiredConfigFragmentsProvider;
host.debugSelectsAlwaysSucceed = debugSelectsAlwaysSucceed;
host.checkTestonlyForOutputFiles = checkTestonlyForOutputFiles;
host.strictConflictChecks = strictConflictChecks;

// === Runfiles ===
host.buildRunfilesManifests = buildRunfilesManifests;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -734,4 +734,54 @@ public void dependencyHasConflict_keepGoing_bothTopLevelTargetsFail(
assertThat(eventListener.failedTargetNames)
.containsExactly("//foo:top_level_a", "//foo:top_level_b");
}

private void setupStrictConflictChecksTest() throws IOException {
write(
"foo/conflict.bzl",
"def _impl(ctx):",
" dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
" file = ctx.actions.declare_file(ctx.label.name + '.dir/file.txt')",
" ctx.actions.run_shell(",
" outputs = [dir, file],",
" command = 'mkdir -p $1 && touch $2',",
" arguments = [dir.path, file.path],",
" )",
" return [DefaultInfo(files = depset([dir, file]))]",
"",
"my_rule = rule(implementation = _impl)");
write("foo/BUILD", "load(':conflict.bzl', 'my_rule')", "my_rule(name = 'bar')");
}

@Test
public void laxFollowedByStrictConflictChecks(@TestParameter boolean mergedAnalysisExecution)
throws Exception {
setupStrictConflictChecksTest();
addOptions("--experimental_merged_skyframe_analysis_execution=" + mergedAnalysisExecution);

addOptions("--noincompatible_strict_conflict_checks");
buildTarget("//foo:bar");
assertNoEvents(events.errors());

addOptions("--incompatible_strict_conflict_checks");
assertThrows(ViewCreationFailedException.class, () -> buildTarget("//foo:bar"));
events.assertContainsError("One of the output paths");
events.assertContainsError("is a prefix of the other");
}

@Test
public void strictFollowedByLaxConflictChecks(@TestParameter boolean mergedAnalysisExecution)
throws Exception {
setupStrictConflictChecksTest();
addOptions("--experimental_merged_skyframe_analysis_execution=" + mergedAnalysisExecution);

addOptions("--incompatible_strict_conflict_checks");
assertThrows(ViewCreationFailedException.class, () -> buildTarget("//foo:bar"));
events.assertContainsError("One of the output paths");
events.assertContainsError("is a prefix of the other");

events.clear();
addOptions("--noincompatible_strict_conflict_checks");
buildTarget("//foo:bar");
assertNoEvents(events.errors());
}
}

0 comments on commit 938e348

Please sign in to comment.