Skip to content

Commit

Permalink
Flip --incompatible_disable_cc_configuration_make_variables incompati…
Browse files Browse the repository at this point in the history 
…ble flag.

#6381

RELNOTES: Incompatible flag --incompatible_disable_cc_configuration_make_variables was flipped (#6381)
PiperOrigin-RevId: 218164627
  • Loading branch information
@hlopko
hlopko authored and Copybara-Service committed Oct 22, 2018
1 parent 2087584 commit d4e3ad8
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ public Label getFdoPrefetchHintsLabel() {

@Option(
name = "incompatible_disable_cc_configuration_make_variables",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {
Expand Down
6 changes: 0 additions & 6 deletions src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,6 @@ public void testHostCrosstoolTop() throws Exception {
Label.parseAbsoluteUnchecked("//third_party/crosstool/mock:cc-compiler-k8-gcc-4.4.0"));
}

@Test
public void testMakeEnvFlags() throws Exception {
BuildConfiguration config = create();
assertThat(config.getMakeEnvironment().get("STRIP")).contains("strip");
}

@Test
public void testCaching() throws Exception {
BuildConfiguration.Options a = Options.getDefaults(BuildConfiguration.Options.class);
Expand Down
98 changes: 98 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
import static com.google.common.truth.Truth.assertThat;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.CompilationMode;
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
Expand Down Expand Up @@ -301,4 +303,100 @@ public void testDisablingLegacyCrosstoolFields() throws Exception {
assertThat(ccToolchainProvider.getTestOnlyLinkOptions())
.doesNotContain("-foo_test_only_linker_flag");
}

/*
* Crosstools should load fine with or without 'gcov-tool'. Those that define 'gcov-tool'
* should also add a make variable.
*/
@Test
public void testOptionalGcovTool() throws Exception {
// Crosstool without gcov-tool
scratch.file(
"a/BUILD",
"filegroup(",
" name='empty')",
"cc_toolchain(",
" name = 'a',",
" cpu = 'banana',",
" all_files = ':empty',",
" ar_files = ':empty',",
" as_files = ':empty',",
" compiler_files = ':empty',",
" dwp_files = ':empty',",
" linker_files = ':empty',",
" strip_files = ':empty',",
" objcopy_files = ':empty',",
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'],",
" proto=\"\"\"",
" feature { name: 'no_legacy_features' }",
" tool_path { name: 'gcc' path: 'path-to-gcc-tool' }",
" tool_path { name: 'ar' path: 'ar' }",
" tool_path { name: 'cpp' path: 'cpp' }",
" tool_path { name: 'gcov' path: 'gcov' }",
" tool_path { name: 'ld' path: 'ld' }",
" tool_path { name: 'nm' path: 'nm' }",
" tool_path { name: 'objdump' path: 'objdump' }",
" tool_path { name: 'strip' path: 'strip' }",
" toolchain_identifier: \"banana\"",
" abi_version: \"banana\"",
" abi_libc_version: \"banana\"",
" compiler: \"banana\"",
" host_system_name: \"banana\"",
" target_system_name: \"banana\"",
" target_cpu: \"banana\"",
" target_libc: \"banana\"",
" \"\"\")");

CcToolchainProvider ccToolchainProvider =
(CcToolchainProvider) getConfiguredTarget("//a:a").get(ToolchainInfo.PROVIDER);
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
ccToolchainProvider.addGlobalMakeVariables(builder);
assertThat(builder.build().get("GCOVTOOL")).isNull();

// Crosstool with gcov-tool
scratch.file(
"b/BUILD",
"filegroup(",
" name='empty')",
"cc_toolchain(",
" name = 'b',",
" cpu = 'banana',",
" all_files = ':empty',",
" ar_files = ':empty',",
" as_files = ':empty',",
" compiler_files = ':empty',",
" dwp_files = ':empty',",
" linker_files = ':empty',",
" strip_files = ':empty',",
" objcopy_files = ':empty',",
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'],",
" proto=\"\"\"",
" feature { name: 'no_legacy_features' }",
" tool_path { name: 'gcc' path: 'path-to-gcc-tool' }",
" tool_path { name: 'gcov-tool' path: 'path-to-gcov-tool' }",
" tool_path { name: 'ar' path: 'ar' }",
" tool_path { name: 'cpp' path: 'cpp' }",
" tool_path { name: 'gcov' path: 'gcov' }",
" tool_path { name: 'ld' path: 'ld' }",
" tool_path { name: 'nm' path: 'nm' }",
" tool_path { name: 'objdump' path: 'objdump' }",
" tool_path { name: 'strip' path: 'strip' }",
" toolchain_identifier: \"banana\"",
" abi_version: \"banana\"",
" abi_libc_version: \"banana\"",
" compiler: \"banana\"",
" host_system_name: \"banana\"",
" target_system_name: \"banana\"",
" target_cpu: \"banana\"",
" target_libc: \"banana\"",
" \"\"\")");

ccToolchainProvider =
(CcToolchainProvider) getConfiguredTarget("//b:b").get(ToolchainInfo.PROVIDER);
builder = ImmutableMap.builder();
ccToolchainProvider.addGlobalMakeVariables(builder);
assertThat(builder.build().get("GCOVTOOL")).isNotNull();
}
}
22 changes: 0 additions & 22 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -964,26 +964,4 @@ public void testCustomRuntimeLibraryPaths() throws Exception {
assertThat(customLibsToolchain.getDynamicRuntimeLibsLabel())
.isEqualTo(Label.create(ctTop, "dynamic-group"));
}

/*
* Crosstools should load fine with or without 'gcov-tool'. Those that define 'gcov-tool'
* should also add a make variable.
*/
@Test
public void testOptionalGcovTool() throws Exception {
// Crosstool with gcov-tool
CppConfigurationLoader loader =
loaderWithOptionalTool(" tool_path { name: \"gcov-tool\" path: \"path-to-gcov-tool\" }");
CppConfiguration cppConfig = create(loader, "--cpu=k8");
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
cppConfig.addGlobalMakeVariables(builder);
assertThat(builder.build().get("GCOVTOOL")).isNotNull();

// Crosstool without gcov-tool
loader = loaderWithOptionalTool("");
cppConfig = create(loader, "--cpu=k8");
builder = ImmutableMap.builder();
cppConfig.addGlobalMakeVariables(builder);
assertThat(builder.build()).doesNotContainKey("GCOVTOOL");
}
}
3 changes: 2 additions & 1 deletion src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,9 +783,10 @@ private void setUpMakeVarToolchain() throws Exception {
"vars/BUILD",
"load(':vars.bzl', 'make_var_supplier', 'make_var_user')",
"make_var_supplier(name = 'supplier', value = 'foo')",
"cc_toolchain_alias(name = 'current_cc_toolchain')",
"make_var_user(",
" name = 'vars',",
" toolchains = [':supplier'],",
" toolchains = [':supplier', ':current_cc_toolchain'],",
")");
}

Expand Down

0 comments on commit d4e3ad8

Please sign in to comment.