Skip to content

Commit

Permalink
Remove --incompatible_merge_genfiles_directory
Browse files Browse the repository at this point in the history
This was flipped over 3 years ago bazelbuild#6761
  • Loading branch information
keith committed Oct 11, 2022
1 parent ff927f7 commit 5a76818
Show file tree
Hide file tree
Showing 10 changed files with 6 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -345,10 +338,6 @@ String getOutputDirName() {
return outputDirName;
}

boolean mergeGenfilesDirectory() {
return mergeGenfilesDirectory;
}

BlazeDirectories getDirectories() {
return directories;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -745,9 +745,6 @@ List<PathFragment> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(",
Expand All @@ -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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -268,7 +268,6 @@ private ImmutableList<BuildConfigurationValue> 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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',",
Expand Down
1 change: 0 additions & 1 deletion src/test/shell/integration/target_compatible_with_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5a76818

Please sign in to comment.