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 59fb92b75994ba..29c577dc545ba7 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 @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.rules.cpp; +import static com.google.common.base.StandardSystemProperty.LINE_SEPARATOR; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Strings; @@ -358,17 +360,64 @@ public LibraryToLinkWrapper createLibraryLinkerInput( Artifact interfaceLibrary = nullIfNone(interfaceLibraryObject, Artifact.class); Artifact notNullArtifactForIdentifier = null; + StringBuilder extensionErrorsBuilder = new StringBuilder(); + String extensionErrorMessage = "does not have any of the allowed extensions"; if (staticLibrary != null) { + String filename = staticLibrary.getFilename(); + if (!Link.ARCHIVE_FILETYPES.matches(filename) + && (!alwayslink || !Link.LINK_LIBRARY_FILETYPES.matches(filename))) { + String extensions = Link.ARCHIVE_FILETYPES.toString(); + if (alwayslink) { + extensions += ", " + Link.LINK_LIBRARY_FILETYPES; + } + extensionErrorsBuilder.append( + String.format("'%s' %s %s", filename, extensionErrorMessage, extensions)); + extensionErrorsBuilder.append(LINE_SEPARATOR.value()); + } notNullArtifactForIdentifier = staticLibrary; - } else if (picStaticLibrary != null) { + } + if (picStaticLibrary != null) { + String filename = picStaticLibrary.getFilename(); + if (!Link.ARCHIVE_FILETYPES.matches(filename) + && (!alwayslink || !Link.LINK_LIBRARY_FILETYPES.matches(filename))) { + String extensions = Link.ARCHIVE_FILETYPES.toString(); + if (alwayslink) { + extensions += ", " + Link.LINK_LIBRARY_FILETYPES; + } + extensionErrorsBuilder.append( + String.format("'%s' %s %s", filename, extensionErrorMessage, extensions)); + extensionErrorsBuilder.append(LINE_SEPARATOR.value()); + } notNullArtifactForIdentifier = picStaticLibrary; - } else if (dynamicLibrary != null) { + } + if (dynamicLibrary != null) { + String filename = dynamicLibrary.getFilename(); + if (!Link.ONLY_SHARED_LIBRARY_FILETYPES.matches(filename)) { + extensionErrorsBuilder.append( + String.format( + "'%s' %s %s", filename, extensionErrorMessage, Link.ONLY_SHARED_LIBRARY_FILETYPES)); + extensionErrorsBuilder.append(LINE_SEPARATOR.value()); + } notNullArtifactForIdentifier = dynamicLibrary; - } else if (interfaceLibrary != null) { + } + if (interfaceLibrary != null) { + String filename = interfaceLibrary.getFilename(); + if (!Link.ONLY_INTERFACE_LIBRARY_FILETYPES.matches(filename)) { + extensionErrorsBuilder.append( + String.format( + "'%s' %s %s", + filename, extensionErrorMessage, Link.ONLY_INTERFACE_LIBRARY_FILETYPES)); + extensionErrorsBuilder.append(LINE_SEPARATOR.value()); + } notNullArtifactForIdentifier = interfaceLibrary; - } else { + } + if (notNullArtifactForIdentifier == null) { throw new EvalException(location, "Must pass at least one artifact"); } + String extensionErrors = extensionErrorsBuilder.toString(); + if (!extensionErrors.isEmpty()) { + throw new EvalException(location, extensionErrors); + } Artifact resolvedSymlinkDynamicLibrary = null; Artifact resolvedSymlinkInterfaceLibrary = null; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java index 54c0aca96d26e7..a029a9d1efd30a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java @@ -40,6 +40,12 @@ private Link() {} // uninstantiable CppFileTypes.VERSIONED_SHARED_LIBRARY, CppFileTypes.INTERFACE_SHARED_LIBRARY); + public static final FileTypeSet ONLY_SHARED_LIBRARY_FILETYPES = + FileTypeSet.of(CppFileTypes.SHARED_LIBRARY, CppFileTypes.VERSIONED_SHARED_LIBRARY); + + public static final FileTypeSet ONLY_INTERFACE_LIBRARY_FILETYPES = + FileTypeSet.of(CppFileTypes.INTERFACE_SHARED_LIBRARY); + public static final FileTypeSet ARCHIVE_LIBRARY_FILETYPES = FileTypeSet.of( CppFileTypes.ARCHIVE, CppFileTypes.PIC_ARCHIVE, 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 1fc345b7a4964a..a9c598f3aa8cac 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 @@ -4676,4 +4676,34 @@ public void testIsToolchainResolutionEnabled_enabled() throws Exception { assertThat(toolchainResolutionEnabled()).isTrue(); } + + @Test + public void testWrongExtensionThrowsError() throws Exception { + setUpCcLinkingContextTest(); + scratch.file( + "foo/BUILD", + "load('//tools/build_defs/cc:rule.bzl', 'crule')", + "cc_binary(name='bin',", + " deps = [':a'],", + ")", + "crule(name='a',", + " static_library = 'a.o',", + " pic_static_library = 'a.pic.o',", + " dynamic_library = 'a.ifso',", + " interface_library = 'a.so',", + ")"); + AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:bin")); + assertThat(e) + .hasMessageThat() + .contains("'a.o' does not have any of the allowed extensions .a, .lib or .pic.a"); + assertThat(e) + .hasMessageThat() + .contains("'a.pic.o' does not have any of the allowed extensions .a, .lib or .pic.a"); + assertThat(e) + .hasMessageThat() + .contains("'a.ifso' does not have any of the allowed extensions .so, .dylib or .dll"); + assertThat(e) + .hasMessageThat() + .contains("'a.so' does not have any of the allowed extensions .ifso, .tbd or .lib"); + } }