Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[6.0] Suppress redundant linkage warnings #7414

Merged
merged 1 commit into from Mar 22, 2024
Merged

[6.0] Suppress redundant linkage warnings #7414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift
View file
Open in desktop
Expand Up @@ -8,9 +8,7 @@ let package = Package(
.executableTarget(
name: "DoNotFilterLinkerDiagnostics",
linkerSettings: [
.linkedLibrary("z"),
.unsafeFlags(["-lz"]),
// should produce: ld: warning: ignoring duplicate libraries: '-lz'
.unsafeFlags(["-Lfoobar"]),
]
),
]
Expand Down
6 changes: 6 additions & 0 deletions Sources/Build/BuildDescription/ProductBuildDescription.swift
View file
Open in desktop
Expand Up @@ -182,6 +182,12 @@ package final class ProductBuildDescription: SPMBuildCore.ProductBuildDescriptio

var isLinkingStaticStdlib = false
let triple = self.buildParameters.triple

// radar://112671586 supress unnecessary warnings
if triple.isMacOSX {
args += ["-Xlinker", "-no_warn_duplicate_libraries"]
}

switch derivedProductType {
case .macro:
throw InternalError("macro not supported") // should never be reached
Expand Down
3 changes: 3 additions & 0 deletions Sources/XCBuildSupport/PIFBuilder.swift
View file
Open in desktop
Expand Up @@ -685,6 +685,9 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder {
impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-lc++")
}

// radar://112671586 supress unnecessary warnings
impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-Wl,-no_warn_duplicate_libraries")

addSources(target.sources, to: pifTarget)

// Handle the target's dependencies (but don't link against them).
Expand Down
15 changes: 15 additions & 0 deletions Tests/BuildTests/BuildPlanTests.swift
View file
Open in desktop
Expand Up @@ -807,6 +807,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -1220,6 +1221,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-dead_strip",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
Expand Down Expand Up @@ -1310,6 +1312,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -1469,6 +1472,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -1659,6 +1663,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -1813,6 +1818,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -2061,6 +2067,7 @@ final class BuildPlanTests: XCTestCase {
buildPath.appending(components: "PkgPackageTests.xctest", "Contents", "MacOS", "PkgPackageTests")
.pathString,
"-module-name", "PkgPackageTests",
"-Xlinker", "-no_warn_duplicate_libraries",
"-Xlinker", "-bundle",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path/../../../",
"@\(buildPath.appending(components: "PkgPackageTests.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -2159,6 +2166,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-dead_strip",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
Expand Down Expand Up @@ -2523,6 +2531,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -2670,6 +2679,7 @@ final class BuildPlanTests: XCTestCase {
"-o", buildPath.appending(components: "Foo").pathString,
"-module-name", "Foo",
"-lBar-Baz",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "Foo.product", "Objects.LinkFileList"))",
Expand All @@ -2685,6 +2695,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "libBar-Baz.dylib").pathString,
"-module-name", "Bar_Baz",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-library",
"-Xlinker", "-install_name", "-Xlinker", "@rpath/libBar-Baz.dylib",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
Expand Down Expand Up @@ -2837,6 +2848,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "liblib.dylib").pathString,
"-module-name", "lib",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-library",
"-Xlinker", "-install_name", "-Xlinker", "@rpath/liblib.dylib",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
Expand Down Expand Up @@ -2976,6 +2988,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "liblib.dylib").pathString,
"-module-name", "lib",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-library",
"-Xlinker", "-install_name", "-Xlinker", "@rpath/liblib.dylib",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
Expand All @@ -2990,6 +3003,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"-Xlinker", "-rpath", "-Xlinker", "@loader_path",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
Expand Down Expand Up @@ -6087,6 +6101,7 @@ final class BuildPlanTests: XCTestCase {
"-L", buildPath.pathString,
"-o", buildPath.appending(components: "exe").pathString,
"-module-name", "exe",
"-Xlinker", "-no_warn_duplicate_libraries",
"-emit-executable",
"@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))",
"-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift-5.5/macosx",
Expand Down
2 changes: 1 addition & 1 deletion Tests/BuildTests/BuildSystemDelegateTests.swift
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@ final class BuildSystemDelegateTests: XCTestCase {
try XCTSkipIf(true, "test is only supported on macOS")
#endif
let (fullLog, _) = try executeSwiftBuild(fixturePath)
XCTAssertTrue(fullLog.contains("ld: warning: ignoring duplicate libraries: '-lz'"), "log didn't contain expected linker diagnostics")
XCTAssertTrue(fullLog.contains("ld: warning: search path 'foobar' not found"), "log didn't contain expected linker diagnostics")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, isn't this now not testing what it was before? ie. with the change to the fixture, we wouldn't expect the ignoring duplicate libraries warning with or without your actual change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to diagnostics handling in this PR is supposed to silence the warning we were previously testing for filtering linker diagnostics. Since that warning is always suppressed, we can no longer test for it and need to trigger a different warning, hence the changes to the fixture.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apparently got distracted from editing this. I saw the comment in the original PR, but do we have another test to check that duplicate libraries are now suppressed? I assume the argument would be that the command line tests are enough and that the purpose of this test was just to make sure we didn't filter linker diagnostics out? If so, sure 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have another test to check that duplicate libraries are now suppressed?

@pusukuri would you be able to add such test in a follow-up PR?

}
}

Expand Down
6 changes: 4 additions & 2 deletions Tests/XCBuildSupportTests/PIFBuilderTests.swift
View file
Open in desktop
Expand Up @@ -1312,6 +1312,7 @@ class PIFBuilderTests: XCTestCase {
"-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib1.modulemap"
])
XCTAssertEqual(settings[.OTHER_LDRFLAGS], [])
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries"])
}
}

Expand Down Expand Up @@ -1384,7 +1385,7 @@ class PIFBuilderTests: XCTestCase {
"-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib2.modulemap"
])
XCTAssertEqual(settings[.OTHER_LDRFLAGS], [])
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lc++"])
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lc++", "-Wl,-no_warn_duplicate_libraries"])
XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], [
"$(inherited)",
"-Xcc", "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib2.modulemap"
Expand Down Expand Up @@ -1463,6 +1464,7 @@ class PIFBuilderTests: XCTestCase {
"-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/BarLib.modulemap"
])
XCTAssertEqual(settings[.OTHER_LDRFLAGS], [])
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries"])
XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], [
"$(inherited)",
"-Xcc", "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/BarLib.modulemap"
Expand Down Expand Up @@ -2406,7 +2408,7 @@ class PIFBuilderTests: XCTestCase {
target.checkImpartedBuildSettings { settings in
XCTAssertEqual(settings[.GCC_PREPROCESSOR_DEFINITIONS], nil)
XCTAssertEqual(settings[.HEADER_SEARCH_PATHS], nil)
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lz"])
XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries", "-lz"])
XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], nil)
}
}
Expand Down