Add option rename dll on Windows
@meteorcloudy
This PR will add an option rename dll on windows.
It won't change any name in the cc_binary and cc_library. It will only change the name when we copy the DLL to the binary directory and the name in the def file
https://github.com/bazelbuild/bazel/blob/1c03c8c4fac1e1028fcb5bb342da3a7fdfc88327/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java#L1054
It is just a draft and it can not handle correctly the file name extension ( only dll for now )
Closes #11524.
PiperOrigin-RevId: 316650491
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index e4ed503..0dc31a1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -314,6 +314,8 @@
if (ruleContext.getRule().getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE
|| !ccCompilationOutputs.isEmpty()) {
if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
+ String dllNameSuffix = CppHelper.getDLLHashSuffix(ruleContext, featureConfiguration);
+ linkingHelper.setLinkedDLLNameSuffix(dllNameSuffix);
Artifact generatedDefFile = null;
Artifact defParser = common.getDefParser();
@@ -327,7 +329,8 @@
ccToolchain
.getFeatures()
.getArtifactNameForCategory(
- ArtifactCategory.DYNAMIC_LIBRARY, ruleContext.getLabel().getName()));
+ ArtifactCategory.DYNAMIC_LIBRARY,
+ ruleContext.getLabel().getName() + dllNameSuffix));
targetBuilder.addOutputGroup(DEF_FILE_OUTPUT_GROUP_NAME, generatedDefFile);
} catch (EvalException e) {
throw ruleContext.throwWithRuleError(e);
@@ -631,7 +634,7 @@
ccToolchain,
configuration,
Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY,
- /* linkedArtifactNameSuffix= */ ""));
+ CppHelper.getDLLHashSuffix(ruleContext, featureConfiguration)));
if (CppHelper.useInterfaceSharedLibraries(
cppConfiguration, ccToolchain, featureConfiguration)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
index 91fdc9b..a742ef3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
@@ -117,6 +117,7 @@
private final CcToolchainProvider ccToolchain;
private final FdoContext fdoContext;
private String linkedArtifactNameSuffix = "";
+ private String linkedDLLNameSuffix = "";
private final ActionConstructionContext actionConstructionContext;
private final Label label;
@@ -312,6 +313,18 @@
return this;
}
+ /*
+ * Adds a suffix (_{hash}) for path of linked DLL. This will be enabled only when rename_dll is
+ * enabled and the linked artifact's LinkTargetType is NODEPS_DYNAMIC_LIBRARY
+ * (i.e DLL generated by cc_library). We have to add the suffix to make sure the CppLinkAction
+ * link against the renamed DLL. If not, CppLinkAction will link against the DLL whose name is the
+ * same as the name of cc_library.
+ */
+ public CcLinkingHelper setLinkedDLLNameSuffix(String suffix) {
+ this.linkedDLLNameSuffix = Preconditions.checkNotNull(suffix);
+ return this;
+ }
+
/**
* Enables the optional generation of interface dynamic libraries - this is only used when the
* linker generates a dynamic library, and only if the crosstool supports it. The default is not
@@ -862,9 +875,12 @@
CppHelper.getArtifactNameForCategory(
ruleErrorConsumer, ccToolchain, ArtifactCategory.PIC_FILE, maybePicName);
}
- String linkedName =
- CppHelper.getArtifactNameForCategory(
- ruleErrorConsumer, ccToolchain, linkTargetType.getLinkerOutput(), maybePicName);
+ String linkedName =
+ maybePicName
+ + (linkTargetType == LinkTargetType.NODEPS_DYNAMIC_LIBRARY ? linkedDLLNameSuffix : "");
+ linkedName =
+ CppHelper.getArtifactNameForCategory(
+ ruleErrorConsumer, ccToolchain, linkTargetType.getLinkerOutput(), linkedName);
PathFragment artifactFragment =
PathFragment.create(label.getName()).getParentDirectory().getRelative(linkedName);
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 6b0c89d..246b78b 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
@@ -63,12 +63,14 @@
import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.Linkstamp;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
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.util.FileTypeSet;
+import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
@@ -930,6 +932,23 @@
}
}
+ /** Returns the suffix (_{hash}) for artifacts generated by cc_library on Windows. */
+ public static String getDLLHashSuffix(
+ RuleContext ruleContext, FeatureConfiguration featureConfiguration) {
+ CppOptions cppOptions =
+ Preconditions.checkNotNull(
+ ruleContext.getConfiguration().getOptions().get(CppOptions.class));
+ if (cppOptions.renameDLL
+ && cppOptions.dynamicMode != DynamicMode.OFF
+ && featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
+ Fingerprint digest = new Fingerprint();
+ digest.addString(ruleContext.getRepository().getName());
+ digest.addPath(ruleContext.getPackageDirectory());
+ return "_" + digest.hexDigestAndReset().substring(0, 10);
+ }
+ return "";
+ }
+
/**
* Returns true if the build implied by the given config and toolchain uses --start-lib/--end-lib
* ld options.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 56ff203..16ef0fc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -245,6 +245,23 @@
public DynamicMode dynamicMode;
@Option(
+ name = "incompatible_avoid_conflict_dlls",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If enabled, all C++ dynamic linked libraries (DLLs) generated by cc_library on Windows "
+ + "will be renamed to name_{hash}.dll where hash is calculated based on "
+ + "the RepositoryName and the DLL's package path. This option is useful "
+ + "when you have one package which depends on severals cc_library with the same name "
+ + "(e.g //foo/bar1:utils and //foo/bar2:utils).")
+ public boolean renameDLL;
+
+ @Option(
name = "force_pic",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
index 4e8f17e..fabdc10 100644
--- a/src/test/py/bazel/bazel_windows_cpp_test.py
+++ b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import glob
import os
import unittest
from src.test.py.bazel import test_base
@@ -249,15 +250,25 @@
' linkstatic = 0,'
')',
])
+ bazel_bin = self.getBazelInfo('bazel-bin')
- # //main:main depends on both //lib:A and //:A,
- # their dlls are both called A.dll,
- # so there should be a conflict error
- exit_code, _, stderr = self.RunBazel(['build', '//main:main'])
- self.AssertExitCode(exit_code, 1, stderr)
- self.assertIn(
- 'ERROR: file \'main/A.dll\' is generated by these conflicting '
- 'actions:', ''.join(stderr))
+ # //main:main depends on both //lib:A and //:A
+ exit_code, _, stderr = self.RunBazel(
+ ['build', '//main:main', '--incompatible_avoid_conflict_dlls'])
+ self.AssertExitCode(exit_code, 0, stderr)
+
+ # Run the binary to see if it runs successfully
+ main_bin = os.path.join(bazel_bin, 'main/main.exe')
+ exit_code, stdout, stderr = self.RunProgram([main_bin])
+ self.AssertExitCode(exit_code, 0, stderr)
+ self.assertEqual(['Hello A, 1', 'Hello A, 2', 'Hello B', 'Hello C'], stdout)
+ # There are 2 A_{hash}.dll since //main:main depends on both //lib:A and
+ # //:A
+ self.assertEqual(
+ len(glob.glob(os.path.join(bazel_bin, 'main', 'A_*.dll'))), 2)
+ # There is only 1 B_{hash}.dll
+ self.assertEqual(
+ len(glob.glob(os.path.join(bazel_bin, 'main', 'B_*.dll'))), 1)
def testBuildDifferentCcBinariesDependOnConflictDLLs(self):
self.createProjectFiles()
@@ -278,19 +289,42 @@
' linkstatic = 0,'
')',
])
+ bazel_bin = self.getBazelInfo('bazel-bin')
self.ScratchFile('main/other_main.cc', ['int main() {return 0;}'])
# Building //main:main should succeed
- exit_code, _, stderr = self.RunBazel(['build', '//main:main'])
- self.AssertExitCode(exit_code, 0, stderr)
-
- # Building //main:other_main *and* //main:main should fail
exit_code, _, stderr = self.RunBazel(
- ['build', '//main:main', '//main:other_main'])
- self.AssertExitCode(exit_code, 1, stderr)
- self.assertIn(
- 'ERROR: file \'main/A.dll\' is generated by these conflicting '
- 'actions:', ''.join(stderr))
+ ['build', '//main:main', '--incompatible_avoid_conflict_dlls'])
+ self.AssertExitCode(exit_code, 0, stderr)
+ main_bin = os.path.join(bazel_bin, 'main/main.exe')
+
+ # Run the main_bin binary to see if it runs successfully
+ exit_code, stdout, stderr = self.RunProgram([main_bin])
+ self.AssertExitCode(exit_code, 0, stderr)
+ self.assertEqual(['Hello A, 1', 'Hello A, 2', 'Hello B', 'Hello C'], stdout)
+ # There is only 1 A_{hash}.dll since //main:main depends transitively on
+ # //:A
+ self.assertEqual(
+ len(glob.glob(os.path.join(bazel_bin, 'main', 'A_*.dll'))), 1)
+ # There is only 1 B_{hash}.dll
+ self.assertEqual(
+ len(glob.glob(os.path.join(bazel_bin, 'main', 'B_*.dll'))), 1)
+
+ # Building //main:other_main should succeed
+ exit_code, _, stderr = self.RunBazel([
+ 'build', '//main:main', '//main:other_main',
+ '--incompatible_avoid_conflict_dlls'
+ ])
+ self.AssertExitCode(exit_code, 0, stderr)
+ other_main_bin = os.path.join(bazel_bin, 'main/other_main.exe')
+
+ # Run the other_main_bin binary to see if it runs successfully
+ exit_code, stdout, stderr = self.RunProgram([other_main_bin])
+ self.AssertExitCode(exit_code, 0, stderr)
+ # There are 2 A_{hash}.dll since //main:main depends on //:A
+ # and //main:other_main depends on //lib:A
+ self.assertEqual(
+ len(glob.glob(os.path.join(bazel_bin, 'main', 'A_*.dll'))), 2)
def testDLLIsCopiedFromExternalRepo(self):
self.ScratchFile('ext_repo/WORKSPACE')
