Skip to content

[Driver] Pass -lld-allow-duplicate-weak for coverage on Windows #75097

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.

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

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented Jul 9, 2024

Legacy driver version of swiftlang/swift-driver#1655 + an integration test for both the legacy and new driver.

rdar://129337999

@hamishknight hamishknight force-pushed the windows-coverage branch 2 times, most recently from 417e444 to 6b1718c Compare July 9, 2024 18:07
@hamishknight hamishknight changed the title [Coverage] Add test for rdar://129337999 [Driver] Pass -lld-allow-duplicate-weak for coverage on Windows Jul 9, 2024
@hamishknight hamishknight changed the title [Driver] Pass -lld-allow-duplicate-weak for coverage on Windows [Driver] Pass -lld-allow-duplicate-weak for coverage on Windows Jul 9, 2024
@hamishknight hamishknight marked this pull request as ready for review July 9, 2024 18:24
@hamishknight
Copy link
Contributor Author

Hrm looks like while Windows CI does build the new driver, it does not seem like it gets picked up:

<unknown>:0: warning: using (deprecated) legacy driver, Swift installation does not contain swift-driver at: 'T:\5\bin\swift-driver-new.exe'

Is that expected @compnerd?

@hamishknight hamishknight requested a review from compnerd July 10, 2024 13:45
@compnerd
Copy link
Member

<unknown>:0: warning: using (deprecated) legacy driver, Swift installation does not contain swift-driver at: 'T:\5\bin\swift-driver-new.exe'

Is that expected @compnerd?

Yes, this is expected. We do not rename the binary, but we do use it with the "old" name in some places.

@hamishknight
Copy link
Contributor Author

Okay, well I have at least tested locally that the swift-driver change works as expected

And add an integration test for both the legacy
and new driver.

rdar://129337999
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@artemcm
Copy link
Contributor

artemcm commented Jul 12, 2024

<unknown>:0: warning: using (deprecated) legacy driver, Swift installation does not contain swift-driver at: 'T:\5\bin\swift-driver-new.exe'

Is that expected @compnerd?

Yes, this is expected. We do not rename the binary, but we do use it with the "old" name in some places.

@compnerd is there a good reason for this? It seems like it's causing some tests to behave differently only in Windows builds.

@compnerd
Copy link
Member

@compnerd is there a good reason for this? It seems like it's causing some tests to behave differently only in Windows builds.

Windows doesn't really do symlinks and the names of the files are important, especially for multi-call scenarios. IIRC, I had some trouble with the renamed binary working properly. If it works, a change to build.ps1 would be fine to do :)

@hamishknight hamishknight enabled auto-merge July 12, 2024 16:36
@hamishknight
Copy link
Contributor Author

@swift-ci please test macOS

@hamishknight hamishknight merged commit b45effc into swiftlang:main Jul 13, 2024
4 of 5 checks passed
@hamishknight hamishknight deleted the windows-coverage branch July 13, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants