[6.0] Suppress redundant linkage warnings #7414
Conversation
…tion (#7404) Suppress redundant linkage warnings due to dependency linkage propagation rdar://112671586 ### Motivation: Improve user experience by suppressing redundant linkage warnings ### Modifications: Pass "-no_warn_duplicate_libraries" to linker. ### Result: We should not see warnings such as "ld: warning: ignoring duplicate libraries: '-lsqlite3'" --------- Co-authored-by: Max Desiatov <[email protected]> (cherry picked from commit e73cb5b)
|
@swift-ci test |
|
@swift-ci test windows |
|
This PR is identical to the original one, was it submitted twice by mistake? |
bnbarham
changed the title
|
@MaxDesiatov it is done. Please take a look. Thanks |
| 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") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 👍
There was a problem hiding this comment.
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?
Explanation: Improve user experience by suppressing redundant linkage warnings due to dependency linkage propagation
Scope: We should not see warnings such as "ld: warning: ignoring duplicate libraries: '-lsqlite3'"
Risk: No risks
Testing: Tests continue passing.
Issue: rdar://112671586
Reviewer: @MaxDesiatov and @neonichu
Co-authored-by: Max Desiatov [email protected]
(cherry picked from commit e73cb5b)
Original PR #7404