C++: Remove gcc flags. RELNOTES:Added -incompatible_do_not_split_linking_cmdline flag. See #7670 PiperOrigin-RevId: 237794079
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index d3f683f..a0879c9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -552,6 +552,21 @@ + " target.") public boolean incompatibleUseToolchainProvidersInJavaCommon; + @Option( + name = "incompatible_do_not_split_linking_cmdline", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "When true, Bazel no longer modifies command line flags used for linking, and also " + + "doesn't selectively decide which flags go to the param file and which don't. " + + "See https://github.com/bazelbuild/bazel/issues/7670 for details.") + public boolean incompatibleDoNotSplitLinkingCmdline; + /** Constructs a {@link StarlarkSemantics} object corresponding to this set of option values. */ public StarlarkSemantics toSkylarkSemantics() { return StarlarkSemantics.builder() @@ -596,6 +611,7 @@ .incompatibleUseToolchainProvidersInJavaCommon( incompatibleUseToolchainProvidersInJavaCommon) .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary) + .incompatibleDoNotSplitLinkingCmdline(incompatibleDoNotSplitLinkingCmdline) .build(); } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index 7fea35d..69e9785 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -904,7 +904,8 @@ featureNames, linkerToolPath, /* supportsEmbeddedRuntimes= */ false, - /* supportsInterfaceSharedLibraries= */ false)) { + /* supportsInterfaceSharedLibraries= */ false, + skylarkRuleContext.getSkylarkSemantics().incompatibleDoNotSplitLinkingCmdline())) { legacyFeaturesBuilder.add(new Feature(feature)); } legacyFeaturesBuilder.addAll( @@ -913,7 +914,9 @@ .filter(feature -> !feature.getName().equals(CppRuleClasses.DEFAULT_COMPILE_FLAGS)) .collect(ImmutableList.toImmutableList())); for (CToolchain.Feature feature : - CppActionConfigs.getFeaturesToAppearLastInFeaturesList(featureNames)) { + CppActionConfigs.getFeaturesToAppearLastInFeaturesList( + featureNames, + skylarkRuleContext.getSkylarkSemantics().incompatibleDoNotSplitLinkingCmdline())) { legacyFeaturesBuilder.add(new Feature(feature)); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index b04cf4c0..930e76e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
@@ -608,6 +608,12 @@ return toolchainInfo.supportsEmbeddedRuntimes(); } + public boolean doNotSplitLinkingCmdline() { + return getFeatures() + .getActivatableNames() + .contains(CppRuleClasses.DO_NOT_SPLIT_LINKING_CMDLINE); + } + /** Returns whether the toolchain supports the --start-lib/--end-lib options. */ public boolean supportsStartEndLib(FeatureConfiguration featureConfiguration) { return toolchainInfo.supportsStartEndLib()
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 13154c2..e2360a9 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
@@ -699,7 +699,7 @@ CcSkyframeSupportValue ccSkyframeSupportValue, CToolchain toolchainFromCcToolchainAttribute, CrosstoolRelease crosstoolFromCcToolchainSuiteProtoAttribute) - throws RuleErrorException { + throws RuleErrorException, InterruptedException { CcToolchainConfigInfo configInfo = attributes.getCcToolchainConfigInfo(); @@ -731,7 +731,12 @@ try { toolchain = CppToolchainInfo.addLegacyFeatures( - toolchain, CppToolchainInfo.getToolsDirectory(attributes.getCcToolchainLabel())); + toolchain, + ruleContext + .getAnalysisEnvironment() + .getSkylarkSemantics() + .incompatibleDoNotSplitLinkingCmdline(), + CppToolchainInfo.getToolsDirectory(attributes.getCcToolchainLabel())); CcToolchainConfigInfo ccToolchainConfigInfo = CcToolchainConfigInfo.fromToolchain(ruleContext, toolchain); return CppToolchainInfo.create(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index 5c37cea..e35bd74 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -44,7 +44,8 @@ ImmutableSet<String> existingFeatureNames, String cppLinkDynamicLibraryToolPath, boolean supportsEmbeddedRuntimes, - boolean supportsInterfaceSharedLibraries) { + boolean supportsInterfaceSharedLibraries, + boolean doNotSplitLinkingCmdline) { ImmutableList.Builder<CToolchain.Feature> featureBuilder = ImmutableList.builder(); try { @@ -554,6 +555,12 @@ " action: 'c++-link-executable'", " action: 'c++-link-dynamic-library'", " action: 'c++-link-nodeps-dynamic-library'", + ifTrue( + doNotSplitLinkingCmdline, + " flag_group {", + " expand_if_true: 'thinlto_param_file'", + " flag: '-Wl,@%{thinlto_param_file}'", + " }"), " flag_group {", " expand_if_all_available: 'libraries_to_link'", " iterate_over: 'libraries_to_link'", @@ -698,10 +705,12 @@ " flag: '-Wl,--end-lib'", " }", " }", - " flag_group {", - " expand_if_true: 'thinlto_param_file'", - " flag: '-Wl,@%{thinlto_param_file}'", - " }", + ifTrue( + !doNotSplitLinkingCmdline, + " flag_group {", + " expand_if_true: 'thinlto_param_file'", + " flag: '-Wl,@%{thinlto_param_file}'", + " }"), " }"))); } if (!existingFeatureNames.contains("force_pic_flags")) { @@ -1126,7 +1135,7 @@ // Note: these feaures won't be added to the crosstools that defines no_legacy_features feature // (e.g. ndk, apple, enclave crosstools). Those need to be modified separately. public static ImmutableList<CToolchain.Feature> getFeaturesToAppearLastInFeaturesList( - ImmutableSet<String> existingFeatureNames) { + ImmutableSet<String> existingFeatureNames, boolean doNotSplitLinkingCmdline) { ImmutableList.Builder<CToolchain.Feature> featureBuilder = ImmutableList.builder(); try { if (!existingFeatureNames.contains("fully_static_link")) { @@ -1223,6 +1232,11 @@ " }"))); } if (!existingFeatureNames.contains("linker_param_file")) { + String dynamicLibraryParamFile = " flag: '-Wl,@%{linker_param_file}'"; + if (doNotSplitLinkingCmdline + || existingFeatureNames.contains(CppRuleClasses.DO_NOT_SPLIT_LINKING_CMDLINE)) { + dynamicLibraryParamFile = " flag: '@%{linker_param_file}'"; + } featureBuilder.add( getFeature( Joiner.on("\n") @@ -1234,7 +1248,7 @@ " action: 'c++-link-nodeps-dynamic-library'", " flag_group {", " expand_if_all_available: 'linker_param_file'", - " flag: '-Wl,@%{linker_param_file}'", + dynamicLibraryParamFile, " }", " }", " flag_set {",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 9e7c9d2..ee2990f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -61,6 +61,7 @@ import com.google.devtools.build.lib.rules.proto.ProtoInfo; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Pair; @@ -910,4 +911,10 @@ ? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST) : null; } + + public static boolean doNotSplitLinkingCmdLine( + StarlarkSemantics starlarkSemantics, CcToolchainProvider ccToolchain) { + return starlarkSemantics.incompatibleDoNotSplitLinkingCmdline() + || ccToolchain.doNotSplitLinkingCmdline(); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index a79de1f..bef3938 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -956,6 +956,10 @@ } linkCommandLineBuilder.setBuildVariables(buildVariables); + if (CppHelper.doNotSplitLinkingCmdLine( + actionConstructionContext.getAnalysisEnvironment().getSkylarkSemantics(), toolchain)) { + linkCommandLineBuilder.doNotSplitLinkingCmdLine(); + } LinkCommandLine linkCommandLine = linkCommandLineBuilder.build(); // Compute the set of inputs - we only need stable order here.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 46ac895..3168624 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -387,6 +387,14 @@ /** A feature marking that the target needs to link its deps in --whole-archive block. */ public static final String LEGACY_WHOLE_ARCHIVE = "legacy_whole_archive"; + /** + * TODO(b/113358321): This feature should be enabled for CROSSTOOLs that work without linking + * command line splitting. Eventually when every CROSSTOOL works without linking command line + * splitting, this feature can be deleted. The flag --incompatible_do_not_split_linking_cmdline + * will activate the same code path even if this feature is not present. See GitHub issue #7670. + */ + public static final String DO_NOT_SPLIT_LINKING_CMDLINE = "do_not_split_linking_cmdline"; + /** Ancestor for all rules that do include scanning. */ public static final class CcIncludeScanningRule implements RuleDefinition { @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java index c75cf00..76d20b9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java
@@ -352,7 +352,9 @@ // TODO(bazel-team): Remove this once bazel supports all crosstool flags through // feature configuration, and all crosstools have been converted. public static CToolchain addLegacyFeatures( - CToolchain toolchain, PathFragment crosstoolTopPathFragment) { + CToolchain toolchain, + boolean doNotSplitLinkingCmdLine, + PathFragment crosstoolTopPathFragment) { CToolchain.Builder toolchainBuilder = CToolchain.newBuilder(); Set<ArtifactCategory> definedCategories = new HashSet<>(); @@ -450,14 +452,16 @@ featureNames, linkerToolPath, toolchain.getSupportsEmbeddedRuntimes(), - toolchain.getSupportsInterfaceSharedObjects())); + toolchain.getSupportsInterfaceSharedObjects(), + doNotSplitLinkingCmdLine)); } toolchainBuilder.mergeFrom(toolchain); if (!featureNames.contains(CppRuleClasses.NO_LEGACY_FEATURES)) { toolchainBuilder.addAllFeature( - CppActionConfigs.getFeaturesToAppearLastInFeaturesList(featureNames)); + CppActionConfigs.getFeaturesToAppearLastInFeaturesList( + featureNames, doNotSplitLinkingCmdLine)); } return toolchainBuilder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 06272d0..3ae7a58 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -52,6 +52,7 @@ @Nullable private final PathFragment toolchainLibrariesSolibDir; private final boolean nativeDeps; private final boolean useTestOnlyFlags; + private final boolean doNotSplitLinkingCmdLine; @Nullable private final Artifact paramFile; @@ -68,7 +69,8 @@ boolean useTestOnlyFlags, @Nullable Artifact paramFile, CcToolchainVariables variables, - @Nullable FeatureConfiguration featureConfiguration) { + @Nullable FeatureConfiguration featureConfiguration, + boolean doNotSplitLinkingCmdLine) { this.actionName = actionName; this.forcedToolPath = forcedToolPath; @@ -82,6 +84,7 @@ this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; this.paramFile = paramFile; + this.doNotSplitLinkingCmdLine = doNotSplitLinkingCmdLine; } @Nullable @@ -167,16 +170,21 @@ */ @VisibleForTesting final Pair<List<String>, List<String>> splitCommandline() { - return splitCommandline(paramFile, getRawLinkArgv(null), linkTargetType); + return splitCommandline( + paramFile, getRawLinkArgv(null), linkTargetType, doNotSplitLinkingCmdLine); } @VisibleForTesting final Pair<List<String>, List<String>> splitCommandline(@Nullable ArtifactExpander expander) { - return splitCommandline(paramFile, getRawLinkArgv(expander), linkTargetType); + return splitCommandline( + paramFile, getRawLinkArgv(expander), linkTargetType, doNotSplitLinkingCmdLine); } private static Pair<List<String>, List<String>> splitCommandline( - Artifact paramFile, List<String> args, LinkTargetType linkTargetType) { + Artifact paramFile, + List<String> args, + LinkTargetType linkTargetType, + boolean doNotSplitLinkingCmdline) { Preconditions.checkNotNull(paramFile); if (linkTargetType.linkerOrArchiver() == LinkerOrArchiver.ARCHIVER) { // Ar link commands can also generate huge command lines. @@ -190,8 +198,8 @@ // a parameter file and pass any linker options through it. List<String> paramFileArgs = new ArrayList<>(); List<String> commandlineArgs = new ArrayList<>(); - extractArgumentsForDynamicLinkParamFile(args, commandlineArgs, paramFileArgs); - + extractArgumentsForDynamicLinkParamFile( + args, commandlineArgs, paramFileArgs, doNotSplitLinkingCmdline); return Pair.of(commandlineArgs, paramFileArgs); } } @@ -209,6 +217,7 @@ private final FeatureConfiguration featureConfiguration; private final String actionName; private final CcToolchainVariables variables; + private final boolean doNotSplitLinkingCmdLine; public ParamFileCommandLine( Artifact paramsFile, @@ -216,13 +225,15 @@ String forcedToolPath, FeatureConfiguration featureConfiguration, String actionName, - CcToolchainVariables variables) { + CcToolchainVariables variables, + boolean doNotSplitLinkingCmdLine) { this.paramsFile = paramsFile; this.linkTargetType = linkTargetType; this.forcedToolPath = forcedToolPath; this.featureConfiguration = featureConfiguration; this.actionName = actionName; this.variables = variables; + this.doNotSplitLinkingCmdLine = doNotSplitLinkingCmdLine; } @Override @@ -230,7 +241,8 @@ List<String> argv = getRawLinkArgv( null, forcedToolPath, featureConfiguration, actionName, linkTargetType, variables); - return splitCommandline(paramsFile, argv, linkTargetType).getSecond(); + return splitCommandline(paramsFile, argv, linkTargetType, doNotSplitLinkingCmdLine) + .getSecond(); } @Override @@ -243,7 +255,8 @@ actionName, linkTargetType, variables); - return splitCommandline(paramsFile, argv, linkTargetType).getSecond(); + return splitCommandline(paramsFile, argv, linkTargetType, doNotSplitLinkingCmdLine) + .getSecond(); } } @@ -251,7 +264,13 @@ CommandLine paramCmdLine() { Preconditions.checkNotNull(paramFile); return new ParamFileCommandLine( - paramFile, linkTargetType, forcedToolPath, featureConfiguration, actionName, variables); + paramFile, + linkTargetType, + forcedToolPath, + featureConfiguration, + actionName, + variables, + doNotSplitLinkingCmdLine); } public static void extractArgumentsForStaticLinkParamFile( @@ -269,53 +288,67 @@ } public static void extractArgumentsForDynamicLinkParamFile( - List<String> args, List<String> commandlineArgs, List<String> paramFileArgs) { + List<String> args, + List<String> commandlineArgs, + List<String> paramFileArgs, + boolean doNotSplitLinkingCmdline) { // Note, that it is not important that all linker arguments are extracted so that // they can be moved into a parameter file, but the vast majority should. commandlineArgs.add(args.get(0)); // gcc command, must not be moved! int argsSize = args.size(); - for (int i = 1; i < argsSize; i++) { - String arg = args.get(i); - if (arg.isEmpty()) { - continue; + if (doNotSplitLinkingCmdline) { + for (int i = 1; i < argsSize; i++) { + String arg = args.get(i); + if (arg.startsWith("@")) { + commandlineArgs.add(arg); // params file, keep it in the command line + } else { + paramFileArgs.add(arg); // the rest goes to the params file + } } - if (arg.equals("-Wl,-no-whole-archive")) { - paramFileArgs.add("-no-whole-archive"); - } else if (arg.equals("-Wl,-whole-archive")) { - paramFileArgs.add("-whole-archive"); - } else if (arg.equals("-Wl,--start-group")) { - paramFileArgs.add("--start-group"); - } else if (arg.equals("-Wl,--end-group")) { - paramFileArgs.add("--end-group"); - } else if (arg.equals("-Wl,--start-lib")) { - paramFileArgs.add("--start-lib"); - } else if (arg.equals("-Wl,--end-lib")) { - paramFileArgs.add("--end-lib"); - } else if (arg.charAt(0) == '-') { - if (arg.startsWith("-l")) { + } else { + for (int i = 1; i < argsSize; i++) { + String arg = args.get(i); + if (arg.isEmpty()) { + continue; + } + if (arg.equals("-Wl,-no-whole-archive")) { + paramFileArgs.add("-no-whole-archive"); + } else if (arg.equals("-Wl,-whole-archive")) { + paramFileArgs.add("-whole-archive"); + } else if (arg.equals("-Wl,--start-group")) { + paramFileArgs.add("--start-group"); + } else if (arg.equals("-Wl,--end-group")) { + paramFileArgs.add("--end-group"); + } else if (arg.equals("-Wl,--start-lib")) { + paramFileArgs.add("--start-lib"); + } else if (arg.equals("-Wl,--end-lib")) { + paramFileArgs.add("--end-lib"); + } else if (arg.charAt(0) == '-') { + if (arg.startsWith("-l")) { + paramFileArgs.add(arg); + } else { + // Anything else starting with a '-' can stay on the commandline. + commandlineArgs.add(arg); + if (arg.equals("-o")) { + // Special case for '-o': add the following argument as well - it is the output file! + commandlineArgs.add(args.get(++i)); + } + } + } else if (CppFileTypes.OBJECT_FILE.apply(arg) + || CppFileTypes.PIC_OBJECT_FILE.apply(arg) + || CppFileTypes.ARCHIVE.apply(arg) + || CppFileTypes.PIC_ARCHIVE.apply(arg) + || CppFileTypes.ALWAYS_LINK_LIBRARY.apply(arg) + || CppFileTypes.ALWAYS_LINK_PIC_LIBRARY.apply(arg) + || CppFileTypes.SHARED_LIBRARY.apply(arg) + || CppFileTypes.INTERFACE_SHARED_LIBRARY.apply(arg) + || CppFileTypes.VERSIONED_SHARED_LIBRARY.apply(arg)) { + // All objects of any kind go into the linker parameters. paramFileArgs.add(arg); } else { - // Anything else starting with a '-' can stay on the commandline. + // Everything that's left stays conservatively on the commandline. commandlineArgs.add(arg); - if (arg.equals("-o")) { - // Special case for '-o': add the following argument as well - it is the output file! - commandlineArgs.add(args.get(++i)); - } } - } else if (CppFileTypes.OBJECT_FILE.apply(arg) - || CppFileTypes.PIC_OBJECT_FILE.apply(arg) - || CppFileTypes.ARCHIVE.apply(arg) - || CppFileTypes.PIC_ARCHIVE.apply(arg) - || CppFileTypes.ALWAYS_LINK_LIBRARY.apply(arg) - || CppFileTypes.ALWAYS_LINK_PIC_LIBRARY.apply(arg) - || CppFileTypes.SHARED_LIBRARY.apply(arg) - || CppFileTypes.INTERFACE_SHARED_LIBRARY.apply(arg) - || CppFileTypes.VERSIONED_SHARED_LIBRARY.apply(arg)) { - // All objects of any kind go into the linker parameters. - paramFileArgs.add(arg); - } else { - // Everything that's left stays conservatively on the commandline. - commandlineArgs.add(arg); } } } @@ -398,6 +431,7 @@ @Nullable private Artifact paramFile; private CcToolchainVariables variables; private FeatureConfiguration featureConfiguration; + private boolean doNotSplitLinkingCmdLine; public LinkCommandLine build() { @@ -425,7 +459,8 @@ useTestOnlyFlags, paramFile, variables, - featureConfiguration); + featureConfiguration, + doNotSplitLinkingCmdLine); } /** Use given tool path instead of the one from feature configuration */ @@ -515,5 +550,10 @@ this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir; return this; } + + public Builder doNotSplitLinkingCmdLine() { + this.doNotSplitLinkingCmdLine = true; + return this; + } } }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 9d14165..61f8730 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -188,6 +188,8 @@ public abstract boolean incompatibleUseToolchainProvidersInJavaCommon(); + public abstract boolean incompatibleDoNotSplitLinkingCmdline(); + /** Returns a {@link Builder} initialized with the values of this instance. */ public abstract Builder toBuilder(); @@ -240,6 +242,7 @@ .incompatibleRequireFeatureConfigurationForPic(true) .incompatibleStricArgumentOrdering(true) .internalSkylarkFlagTestCanary(false) + .incompatibleDoNotSplitLinkingCmdline(false) .build(); /** Builder for {@link StarlarkSemantics}. All fields are mandatory. */ @@ -321,6 +324,8 @@ public abstract Builder internalSkylarkFlagTestCanary(boolean value); + public abstract Builder incompatibleDoNotSplitLinkingCmdline(boolean value); + public abstract StarlarkSemantics build(); } }
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 563c864..a1bac4b 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -160,7 +160,8 @@ "--incompatible_require_feature_configuration_for_pic=" + rand.nextBoolean(), "--incompatible_strict_argument_ordering=" + rand.nextBoolean(), "--incompatible_use_toolchain_providers_in_java_common=" + rand.nextBoolean(), - "--internal_skylark_flag_test_canary=" + rand.nextBoolean()); + "--internal_skylark_flag_test_canary=" + rand.nextBoolean(), + "--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean()); } /** @@ -209,6 +210,7 @@ .incompatibleStricArgumentOrdering(rand.nextBoolean()) .incompatibleUseToolchainProvidersInJavaCommon(rand.nextBoolean()) .internalSkylarkFlagTestCanary(rand.nextBoolean()) + .incompatibleDoNotSplitLinkingCmdline(rand.nextBoolean()) .build(); }
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 6db2d7d..b4adb26 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -63,6 +63,9 @@ public static final String DYNAMIC_LINKING_MODE_FEATURE = "feature { name: '" + CppRuleClasses.DYNAMIC_LINKING_MODE + "'}"; + public static final String DO_NOT_SPLIT_LINKING_CMDLINE_FEATURE = + "feature { name: '" + CppRuleClasses.DO_NOT_SPLIT_LINKING_CMDLINE + "' enabled: true}"; + public static final String STARLARK_DYNAMIC_LINKING_MODE_FEATURE = "[feature(name = '" + CppRuleClasses.DYNAMIC_LINKING_MODE + "')]";
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD index f81cf55..fda9ab3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -36,6 +36,7 @@ "//src/main/java/com/google/devtools/build/lib:core-workspace-rules", "//src/main/java/com/google/devtools/build/lib:io", "//src/main/java/com/google/devtools/build/lib:packages-internal", + "//src/main/java/com/google/devtools/build/lib:skylark_semantics", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index d7f3811..fbd0e3b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -52,6 +52,7 @@ import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; +import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; @@ -86,6 +87,11 @@ } @Override + public StarlarkSemantics getSkylarkSemantics() { + return StarlarkSemantics.DEFAULT_SEMANTICS; + } + + @Override public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot root) { return CppLinkActionTest.this.getDerivedArtifact( rootRelativePath, root, ActionsTestUtil.NULL_ARTIFACT_OWNER); @@ -117,8 +123,11 @@ ImmutableSet.of(), "dynamic_library_linker_tool", /* supportsEmbeddedRuntimes= */ true, - /* supportsInterfaceSharedLibraries= */ false)) - .addAll(CppActionConfigs.getFeaturesToAppearLastInFeaturesList(ImmutableSet.of())) + /* supportsInterfaceSharedLibraries= */ false, + /* doNotSplitLinkingCmdline= */ true)) + .addAll( + CppActionConfigs.getFeaturesToAppearLastInFeaturesList( + ImmutableSet.of(), /* doNotSplitLinkingCmdline= */ true)) .add(linkCppStandardLibrary) .build(); @@ -1085,7 +1094,11 @@ } @Test - public void testSplitExecutableLinkCommand() throws Exception { + public void testSplitExecutableLinkCommandStatic() throws Exception { + getAnalysisMock() + .ccSupport() + .setupCrosstool(mockToolsConfig, MockCcSupport.DO_NOT_SPLIT_LINKING_CMDLINE_FEATURE); + RuleContext ruleContext = createDummyRuleContext(); CppLinkAction linkAction = createLinkBuilder(ruleContext, LinkTargetType.EXECUTABLE).build(); @@ -1099,4 +1112,85 @@ assertThat(result.second).contains("-lcpp_standard_library"); } + + private String removeOutDirectory(String s) { + return s.replace("blaze-out", "").replace("bazel-out", ""); + } + + @Test + public void testSplitExecutableLinkCommandDynamicWithNoSplitting() throws Exception { + getAnalysisMock() + .ccSupport() + .setupCrosstool(mockToolsConfig, MockCcSupport.DO_NOT_SPLIT_LINKING_CMDLINE_FEATURE); + RuleContext ruleContext = createDummyRuleContext(); + + FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(ruleContext); + + CppLinkAction linkAction = + createLinkBuilder( + ruleContext, + LinkTargetType.DYNAMIC_LIBRARY, + "dummyRuleContext/out.so", + ImmutableList.of(), + ImmutableList.of(), + featureConfiguration) + .setLibraryIdentifier("library") + .build(); + Pair<List<String>, List<String>> result = linkAction.getLinkCommandLine().splitCommandline(); + + assertThat( + result.first.stream() + .map(x -> removeOutDirectory(x)) + .collect(ImmutableList.toImmutableList())) + .containsExactly( + "crosstool/gcc_tool", "@/k8-fastbuild/bin/dummyRuleContext/out.so-2.params") + .inOrder(); + assertThat( + result.second.stream() + .map(x -> removeOutDirectory(x)) + .collect(ImmutableList.toImmutableList())) + .containsExactly( + "-shared", + "-o", + "/k8-fastbuild/bin/dummyRuleContext/out.so", + "-Wl,-S", + "--sysroot=/usr/grte/v1") + .inOrder(); + } + + @Test + @Deprecated + // TODO(b/113358321): Remove once #7670 is finished. + public void testSplitExecutableLinkCommandDynamicWithSplitting() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + + FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(ruleContext); + + CppLinkAction linkAction = + createLinkBuilder( + ruleContext, + LinkTargetType.DYNAMIC_LIBRARY, + "dummyRuleContext/out.so", + ImmutableList.of(), + ImmutableList.of(), + featureConfiguration) + .setLibraryIdentifier("library") + .build(); + Pair<List<String>, List<String>> result = linkAction.getLinkCommandLine().splitCommandline(); + + assertThat( + result.first.stream() + .map(x -> removeOutDirectory(x)) + .collect(ImmutableList.toImmutableList())) + .containsExactly( + "crosstool/gcc_tool", + "-shared", + "-o", + "/k8-fastbuild/bin/dummyRuleContext/out.so", + "-Wl,-S", + "--sysroot=/usr/grte/v1", + "@/k8-fastbuild/bin/dummyRuleContext/out.so-2.params") + .inOrder(); + assertThat(result.second).isEmpty(); + } }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java index f1a3dc6..5ef42f9 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -788,6 +788,10 @@ @Test public void testParamFileLinkVariables() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool( + mockToolsConfig, "feature {", " name: 'do_not_split_linking_cmdline'", "}"); assertThat( commandLineForVariables( CppActionNames.CPP_LINK_EXECUTABLE, @@ -796,7 +800,7 @@ "cc_toolchain = toolchain,", "param_file = 'foo/bar/params',", ")")) - .contains("-Wl,@foo/bar/params"); + .contains("@foo/bar/params"); } @Test
diff --git a/tools/cpp/link_dynamic_library.sh b/tools/cpp/link_dynamic_library.sh index ae79a80..c71d498 100755 --- a/tools/cpp/link_dynamic_library.sh +++ b/tools/cpp/link_dynamic_library.sh
@@ -28,33 +28,86 @@ E_LINKER_COMMAND_NOT_FOUND=12 E_INTERFACE_BUILDER_NOT_FOUND=13 -# Should generate interface library switch (<yes|no>); if the value is "no", -# following 3 args are ignored (but must be present) -GENERATE_INTERFACE_LIBRARY="$1" -# Tool which can generate interface library from dynamic library file -INTERFACE_LIBRARY_BUILDER="$2" -# Dynamic library from which we want to generate interface library -DYNAMIC_LIBRARY="$3" -# Resulting interface library -INTERFACE_LIBRARY="$4" -# The command used to generate the dynamic library -LINKER_COMMAND="$5" -shift 5 +SUFFIX=".rewritten" -if [ ! -e "$LINKER_COMMAND" ]; then - echo "Linker command ($LINKER_COMMAND) not found." 1>&2; - exit "$E_LINKER_COMMAND_NOT_FOUND" -fi +other_args="" -if [ "no" == "$GENERATE_INTERFACE_LIBRARY" ]; then - INTERFACE_GENERATION=: +if [[ "$#" -eq 1 ]]; then + if [[ "$1" != @* ]]; then + echo "Parameter file must start with @" 1>&2; + exit "$E_LINKER_COMMAND_NOT_FOUND" + fi + + filename=$(echo "$1" | cut -c2-) + first_five_lines=$(head -n 5 $filename) + + # Should generate interface library switch (<yes|no>); if the value is "no", + # following 3 args are ignored (but must be present) + GENERATE_INTERFACE_LIBRARY=$(echo "$first_five_lines" | head -n1 | tail -n1) + # Tool which can generate interface library from dynamic library file + INTERFACE_LIBRARY_BUILDER=$(echo "$first_five_lines" | head -n2 | tail -n1) + # Dynamic library from which we want to generate interface library + DYNAMIC_LIBRARY=$(echo "$first_five_lines" | head -n3 | tail -n1) + # Resulting interface library + INTERFACE_LIBRARY=$(echo "$first_five_lines" | head -n4 | tail -n1) + # The command used to generate the dynamic library + LINKER_COMMAND=$(echo "$first_five_lines" | head -n5 | tail -n1) + + rest_of_lines=$(tail -n +6 $filename) + new_param_file="${filename}${SUFFIX}" + echo "$rest_of_lines" > $new_param_file + other_args="@$new_param_file" + + if [[ ! -e "$LINKER_COMMAND" ]]; then + echo "Linker command ($LINKER_COMMAND) not found." 1>&2; + exit "$E_LINKER_COMMAND_NOT_FOUND" + fi + + if [[ "no" == "$GENERATE_INTERFACE_LIBRARY" ]]; then + INTERFACE_GENERATION=: + else + if [[ ! -e "$INTERFACE_LIBRARY_BUILDER" ]]; then + echo "Interface library builder ($INTERFACE_LIBRARY_BUILDER) + not found." 1>&2; + exit "$E_INTERFACE_BUILDER_NOT_FOUND" + fi + INTERFACE_GENERATION="${INTERFACE_LIBRARY_BUILDER} ${DYNAMIC_LIBRARY} + ${INTERFACE_LIBRARY}" + fi + + ${LINKER_COMMAND} "$other_args" && ${INTERFACE_GENERATION} else - if [ ! -e "$INTERFACE_LIBRARY_BUILDER" ]; then - echo "Interface library builder ($INTERFACE_LIBRARY_BUILDER) not found." 1>&2; - exit "$E_INTERFACE_BUILDER_NOT_FOUND" - fi - INTERFACE_GENERATION="${INTERFACE_LIBRARY_BUILDER} ${DYNAMIC_LIBRARY} ${INTERFACE_LIBRARY}" -fi + # TODO(b/113358321): Remove this branch once projects are migrated to not + # splitting the linking command line. + # Should generate interface library switch (<yes|no>); if the value is "no", + # following 3 args are ignored (but must be present) + GENERATE_INTERFACE_LIBRARY="$1" + # Tool which can generate interface library from dynamic library file + INTERFACE_LIBRARY_BUILDER="$2" + # Dynamic library from which we want to generate interface library + DYNAMIC_LIBRARY="$3" + # Resulting interface library + INTERFACE_LIBRARY="$4" + # The command used to generate the dynamic library + LINKER_COMMAND="$5" + shift 5 + if [[ ! -e "$LINKER_COMMAND" ]]; then + echo "Linker command ($LINKER_COMMAND) not found." 1>&2; + exit "$E_LINKER_COMMAND_NOT_FOUND" + fi -${LINKER_COMMAND} "$@" && ${INTERFACE_GENERATION} + if [[ "no" == "$GENERATE_INTERFACE_LIBRARY" ]]; then + INTERFACE_GENERATION=: + else + if [[ ! -e "$INTERFACE_LIBRARY_BUILDER" ]]; then + echo "Interface library builder ($INTERFACE_LIBRARY_BUILDER) + not found." 1>&2; + exit "$E_INTERFACE_BUILDER_NOT_FOUND" + fi + INTERFACE_GENERATION="${INTERFACE_LIBRARY_BUILDER} ${DYNAMIC_LIBRARY} + ${INTERFACE_LIBRARY}" + fi + + ${LINKER_COMMAND} "$@" && ${INTERFACE_GENERATION} +fi