From 5a768181e3299b68338b584c19222478de421a8d Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 18 Jul 2022 20:31:01 -0700 Subject: [PATCH] Remove --incompatible_merge_genfiles_directory This was flipped over 3 years ago https://github.com/bazelbuild/bazel/issues/6761 --- .../config/BuildConfigurationValue.java | 4 ---- .../lib/analysis/config/CoreOptions.java | 10 -------- .../analysis/config/OutputDirectories.java | 13 +---------- .../build/lib/rules/cpp/CcCommon.java | 3 --- .../StarlarkAttrTransitionProviderTest.java | 6 ++--- .../config/BuildConfigurationValueTest.java | 5 ++-- .../build/lib/rules/cpp/CcCommonTest.java | 23 ------------------- .../cpp/CcLibraryConfiguredTargetTest.java | 1 - .../target_compatible_with_test.sh | 1 - ...rget_compatible_with_test_external_repo.sh | 1 - 10 files changed, 6 insertions(+), 61 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java index 9a59733973261b..3fc6a044e37400 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java @@ -376,10 +376,6 @@ public ArtifactRoot getGenfilesDirectory(RepositoryName repositoryName) { return outputDirectories.getGenfilesDirectory(repositoryName); } - public boolean hasSeparateGenfilesDirectory() { - return !outputDirectories.mergeGenfilesDirectory(); - } - /** * Returns the directory where coverage-related artifacts and metadata files should be stored. * This includes for example uninstrumented class files needed for Jacoco's coverage reporting diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java index 5974065e41f992..2994b596db001f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java @@ -59,15 +59,6 @@ public class CoreOptions extends FragmentOptions implements Cloneable { public static final OptionDefinition CPU = OptionsParser.getOptionDefinitionByName(CoreOptions.class, "cpu"); - @Option( - name = "incompatible_merge_genfiles_directory", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, - effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = "If true, the genfiles directory is folded into the bin directory.") - public boolean mergeGenfilesDirectory; - @Option( name = "experimental_platform_in_output_dir", defaultValue = "false", @@ -916,7 +907,6 @@ public FragmentOptions getHost() { host.executionInfoModifier = executionInfoModifier; host.commandLineBuildVariables = commandLineBuildVariables; host.enforceConstraints = enforceConstraints; - host.mergeGenfilesDirectory = mergeGenfilesDirectory; host.platformInOutputDir = platformInOutputDir; host.cpu = hostCpu; host.includeRequiredConfigFragmentsProvider = includeRequiredConfigFragmentsProvider; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java index 5d565a8a69aa23..8ce5af19a1e809 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java @@ -124,8 +124,6 @@ public ArtifactRoot getRoot( private final ArtifactRoot testlogsDirectory; private final ArtifactRoot middlemanDirectory; - private final boolean mergeGenfilesDirectory; - private final boolean siblingRepositoryLayout; private final Path execRoot; @@ -158,7 +156,6 @@ public ArtifactRoot getRoot( this.middlemanDirectory = OutputDirectory.MIDDLEMAN.getRoot(outputDirName, directories, mainRepositoryName); - this.mergeGenfilesDirectory = options.mergeGenfilesDirectory; this.siblingRepositoryLayout = siblingRepositoryLayout; this.execRoot = directories.getExecRoot(mainRepositoryName.getName()); } @@ -289,11 +286,7 @@ ArtifactRoot getBuildInfoDirectory(RepositoryName repositoryName) { /** Returns the genfiles directory for this build configuration. */ ArtifactRoot getGenfilesDirectory(RepositoryName repositoryName) { - return mergeGenfilesDirectory - ? getBinDirectory(repositoryName) - : siblingRepositoryLayout - ? buildDerivedRoot("genfiles", repositoryName, false) - : genfilesDirectory; + return getBinDirectory(repositoryName); } /** @@ -345,10 +338,6 @@ String getOutputDirName() { return outputDirName; } - boolean mergeGenfilesDirectory() { - return mergeGenfilesDirectory; - } - BlazeDirectories getDirectories() { return directories; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 785a70c3fe9ce2..14370c93bb8a55 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -745,9 +745,6 @@ List getSystemIncludeDirs() { // We don't need to perform the above checks against outIncludesPath again since any errors // must have manifested in includesPath already. PathFragment outIncludesPath = packageSourceRoot.getRelative(includesAttr); - if (ruleContext.getConfiguration().hasSeparateGenfilesDirectory()) { - result.add(ruleContext.getGenfilesFragment().getRelative(outIncludesPath)); - } result.add(ruleContext.getBinFragment().getRelative(outIncludesPath)); } return result; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java index 9760886423bcab..a863c233acfaf5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java @@ -851,9 +851,9 @@ public void testBannedNativeOptionOutput() throws Exception { scratch.file( "test/starlark/my_rule.bzl", "def transition_func(settings, attr):", - " return {'//command_line_option:incompatible_merge_genfiles_directory': True}", + " return {'//command_line_option:incompatible_use_platforms_repo_for_constraints': True}", "my_transition = transition(implementation = transition_func,", - " inputs = [], outputs = ['//command_line_option:incompatible_merge_genfiles_directory'])", + " inputs = [], outputs = ['//command_line_option:incompatible_use_platforms_repo_for_constraints'])", "def impl(ctx): ", " return []", "my_rule = rule(", @@ -874,7 +874,7 @@ public void testBannedNativeOptionOutput() throws Exception { reporter.removeHandler(failFastHandler); getConfiguredTarget("//test/starlark:test"); assertContainsEvent( - "Invalid transition output '//command_line_option:incompatible_merge_genfiles_directory'. " + "Invalid transition output '//command_line_option:incompatible_use_platforms_repo_for_constraints'. " + "Cannot transition on --experimental_* or --incompatible_* options"); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java index 73ecc244909e50..a01addc73ef399 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java @@ -253,8 +253,8 @@ public void testHostCompilationModeNonDefault() throws Exception { @Test public void testIncompatibleMergeGenfilesDirectory() throws Exception { - BuildConfigurationValue target = create("--incompatible_merge_genfiles_directory"); - BuildConfigurationValue host = createHost("--incompatible_merge_genfiles_directory"); + BuildConfigurationValue target = create(); + BuildConfigurationValue host = createHost(); assertThat(target.getGenfilesDirectory(RepositoryName.MAIN)) .isEqualTo(target.getBinDirectory(RepositoryName.MAIN)); assertThat(host.getGenfilesDirectory(RepositoryName.MAIN)) @@ -268,7 +268,6 @@ private ImmutableList getTestConfigurations() throws Ex create("--javacopt=foo"), create("--platform_suffix=-test"), create("--target_environment=//foo", "--target_environment=//bar"), - create("--incompatible_merge_genfiles_directory"), create( "--define", "foo=#foo", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index de1fca049df937..636afcaa61379f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -461,32 +461,9 @@ public void testIsolatedIncludes() throws Exception { targetConfig.getGenfilesFragment(RepositoryName.MAIN).getRelative(includesRoot)); } - @Test - public void testDisabledGenfilesDontShowUpInSystemIncludePaths() throws Exception { - scratch.file( - "bang/BUILD", - "cc_library(name = 'bang',", - " srcs = ['bang.cc'],", - " includes = ['bang_includes'])"); - String includesRoot = "bang/bang_includes"; - - useConfiguration("--noincompatible_merge_genfiles_directory"); - ConfiguredTarget foo = getConfiguredTarget("//bang:bang"); - PathFragment genfilesDir = - targetConfig.getGenfilesFragment(RepositoryName.MAIN).getRelative(includesRoot); - assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getSystemIncludeDirs()) - .contains(genfilesDir); - - useConfiguration("--incompatible_merge_genfiles_directory"); - foo = getConfiguredTarget("//bang:bang"); - assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getSystemIncludeDirs()) - .doesNotContain(genfilesDir); - } - @Test public void testUseIsystemForIncludes() throws Exception { // Tests the effect of --use_isystem_for_includes. - useConfiguration("--incompatible_merge_genfiles_directory=false"); scratch.file( "no_includes/BUILD", "cc_library(name = 'no_includes',", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index e3e0967de42fa7..27b75733e50901 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -1223,7 +1223,6 @@ private CppCompileAction getGeneratingCompileAction( @Test public void testIncludePathOrder() throws Exception { - useConfiguration("--incompatible_merge_genfiles_directory=false"); scratch.file("foo/BUILD", "cc_library(", " name = 'bar',", diff --git a/src/test/shell/integration/target_compatible_with_test.sh b/src/test/shell/integration/target_compatible_with_test.sh index 234ac30a38fdcc..0119954a76312e 100755 --- a/src/test/shell/integration/target_compatible_with_test.sh +++ b/src/test/shell/integration/target_compatible_with_test.sh @@ -203,7 +203,6 @@ EOF } add_to_bazelrc "test --nocache_test_results" -add_to_bazelrc "build --incompatible_merge_genfiles_directory=true" function tear_down() { bazel shutdown diff --git a/src/test/shell/integration/target_compatible_with_test_external_repo.sh b/src/test/shell/integration/target_compatible_with_test_external_repo.sh index 94464946518245..d8bc34fe340c21 100755 --- a/src/test/shell/integration/target_compatible_with_test_external_repo.sh +++ b/src/test/shell/integration/target_compatible_with_test_external_repo.sh @@ -179,7 +179,6 @@ EOF } add_to_bazelrc "test --nocache_test_results" -add_to_bazelrc "build --incompatible_merge_genfiles_directory=true" function tear_down() { bazel shutdown