From d42c2a51fb22097a59ca54249b3b9e56fc88e3dd Mon Sep 17 00:00:00 2001 From: hlopko Date: Tue, 18 Dec 2018 07:02:28 -0800 Subject: [PATCH] Make cc_toolchain.(static|dynamic)_runtime_libs attributes optional Issue for the --incompatible_disable_runtimes_filegroups incompatible flag (which this cl is a step towards to): #6942 Tracking issue for legacy crosstool fields removal: #5883 RELNOTES: cc_toolchain.(static|dynamic)_runtime_libs attributes are now optional PiperOrigin-RevId: 225990391 --- .../rules/cpp/CcToolchainProviderHelper.java | 13 +++ .../build/lib/rules/cpp/CcToolchainRule.java | 4 +- .../rules/cpp/CcToolchainProviderTest.java | 103 ++++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java index 85a6b66b3b9dab..7ede204c121108 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java @@ -477,6 +477,11 @@ static CcToolchainProvider getCcToolchainProvider( final NestedSet staticRuntimeLinkInputs; final Artifact staticRuntimeLinkMiddleman; if (toolchainInfo.supportsEmbeddedRuntimes()) { + if (staticRuntimeLibDep == null) { + throw ruleContext.throwWithRuleError( + "Toolchain supports embedded runtimes, but didn't " + + "provide static_runtime_libs attribute."); + } staticRuntimeLinkInputs = staticRuntimeLibDep.getProvider(FileProvider.class).getFilesToBuild(); } else { @@ -505,6 +510,11 @@ static CcToolchainProvider getCcToolchainProvider( List dynamicRuntimeLinkInputs = new ArrayList<>(); Artifact dynamicRuntimeLinkMiddleman; if (toolchainInfo.supportsEmbeddedRuntimes()) { + if (dynamicRuntimeLibDep == null) { + throw ruleContext.throwWithRuleError( + "Toolchain supports embedded runtimes, but didn't " + + "provide dynamic_runtime_libs attribute."); + } NestedSetBuilder dynamicRuntimeLinkSymlinksBuilder = NestedSetBuilder.stableOrder(); for (Artifact artifact : dynamicRuntimeLibDep.getProvider(FileProvider.class).getFilesToBuild()) { @@ -835,6 +845,9 @@ private static CppModuleMap createCrosstoolModuleMap(CcToolchainAttributesProvid static TransitiveInfoCollection selectDep( ImmutableList deps, Label label) { + if (deps.isEmpty()) { + return null; + } for (TransitiveInfoCollection dep : deps) { if (dep.getLabel().equals(label)) { return dep; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index 6f0329b2c03d7e..63dcbfab00aaa3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -221,7 +221,7 @@ Currently unused (#692

cc_toolchain will select one of these libraries based on the label from crosstool_proto.static_runtimes_filegroup field.

*/ - .add(attr("static_runtime_libs", LABEL_LIST).legacyAllowAnyFileType().mandatory()) + .add(attr("static_runtime_libs", LABEL_LIST).legacyAllowAnyFileType()) /* A collection of artifacts for dynamic libraries for the C++ runtime library (e.g. libstdc++.so). @@ -229,7 +229,7 @@ Currently unused (#692

cc_toolchain will select one of these libraries based on the label from crosstool_proto.dynamic_runtimes_filegroup field.

*/ - .add(attr("dynamic_runtime_libs", LABEL_LIST).legacyAllowAnyFileType().mandatory()) + .add(attr("dynamic_runtime_libs", LABEL_LIST).legacyAllowAnyFileType()) /* Module map artifact to be used for modular builds. */ diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index ed2281386cfc53..af9e0d903cb09e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java @@ -409,4 +409,107 @@ public void testFissionConfigWithMissingDwp() throws Exception { getConfiguredTarget("//a:a"); assertContainsEvent("Tool path for 'dwp' is missing"); } + + @Test + public void testRuntimeLibsAttributesAreNotObligatory() throws Exception { + scratch.file( + "a/BUILD", + "filegroup(name='empty') ", + "cc_toolchain_suite(", + " name = 'a',", + " toolchains = { 'k8': ':b' },", + ")", + "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',", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + "\"\"\")"); + reporter.removeHandler(failFastHandler); + useConfiguration("--cpu=k8", "--host_cpu=k8"); + getConfiguredTarget("//a:a"); + assertNoEvents(); + } + + @Test + public void testWhenRuntimeLibsAtttributesMandatoryWhenSupportsEmbeddedRuntimes() + throws Exception { + scratch.file( + "a/BUILD", + "filegroup(name='empty') ", + "cc_toolchain_suite(", + " name = 'a',", + " toolchains = { 'k8': ':b', 'k9': ':c' },", + ")", + "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',", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + " supports_embedded_runtimes: true,", + "\"\"\")", + "cc_toolchain(", + " name = 'c',", + " 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',", + " static_runtime_libs = [ ':yolo' ],", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + " supports_embedded_runtimes: true,", + "\"\"\")"); + reporter.removeHandler(failFastHandler); + useConfiguration("--cpu=k8", "--host_cpu=k8"); + getConfiguredTarget("//a:a"); + assertContainsEvent( + "Toolchain supports embedded runtimes, but didn't provide static_runtime_libs attribute."); + + useConfiguration("--cpu=k9", "--host_cpu=k9"); + getConfiguredTarget("//a:a"); + assertContainsEvent( + "Toolchain supports embedded runtimes, but didn't provide dynamic_runtime_libs attribute."); + } }