Skip to content

Output a Windows-style response file for swift-frontend and clang #1716

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
Oct 18, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Oct 17, 2024

#1000 switched to use POSIX-style response files but swift-frontend expects Windows-style response files on Windows.

Switch back to generating Windows-style response files on Windows and pass --rsp-quoting=windows to clang to tell it to pass the response file in Windows mode.

Alternatives considered:

  • Have a separate response file generation logic for clang and Swift: This seems like a continuous source of bugs if you always need to think about which response file format you want, especially if the behavior is not intuitive (clang.exe expecting POSIX-style response files)
  • Instead of parsing --rsp-quoting=windows on Windows, use clang-cl.exe, which also causes response files to be parsed in Windows-style: I think it’s better to be explicit here instead of relying on clang’s implicit behavior based on which executable name was invoked.
  • Change swift-frontend to accept POSIX-style response files: swift-frontend and the old Swift driver share the same response file parsing logic. Changing swift-frontend without changing the old driver would require a bunch of flag parsing, which isn’t desirable. Also, using Windows-style response files on Windows seems like the better-fitting solution (CMake, for example, outputs Windows-style response files).

Reverts #1000 with some logic added on top.

swiftlang#1000 switched to use POSIX-style response files but swift-frontend expects Windows-style response files on Windows.

Switch back to generating Windows-style response files on Windows and pass `--rsp-quoting=windows` to `clang` to tell it to pass the response file in Windows mode.

Alternatives considered:
- Have a separate response file generation logic for clang and Swift: This seems like a continuous source of bugs if you always need to think about which response file format you want, especially if the behavior is not intuitive (`clang.exe` expecting POSIX-style response files)
- Instead of parsing `--rsp-quoting=windows` on Windows, use `clang-cl.exe`, which also causes response files to be parsed in Windows-style: I think it’s better to be explicit here instead of relying on clang’s implicit behavior based on which executable name was invoked.
- Change swift-frontend to accept POSIX-style response files: swift-frontend and the old Swift driver share the same response file parsing logic. Changing swift-frontend without changing the old driver would require a bunch of flag parsing, which isn’t desirable. Also, using Windows-style response files on Windows seems like the better-fitting solution (CMake, for example, outputs Windows-style response files).

Reverts swiftlang#1000 with some logic added on top.
@ahoppen ahoppen requested review from compnerd and artemcm October 17, 2024 21:44
@ahoppen
Copy link
Member Author

ahoppen commented Oct 17, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Oct 17, 2024

@swift-ci Please test macOS

@ahoppen ahoppen merged commit 1bd6977 into swiftlang:main Oct 18, 2024
3 checks passed
@ahoppen ahoppen deleted the windows-response-file branch October 18, 2024 05:44
@hjyamauchi
Copy link

@ahoppen It seems like we started to get this compile error in our internal app in the last few days with the tip of tree swift on windows:

LINK : warning LNK4044: unrecognized option '/-rsp-quoting=windows'; ignored
LINK : warning LNK4044: unrecognized option '/LIB'; ignored
LINK : fatal error LNK1104: cannot open file 'swiftSwiftOnoneSupport.lib'

Though this may be a cmake issue, do you have thoughts? Does it look like the linker shouldn't get these options?

@ahoppen
Copy link
Member Author

ahoppen commented Oct 22, 2024

While I investigate a little more, do you use lld as a linker (ie. pass --use-lld) or any other linker? I assume the issue is that we should only pass -rsp-quoting=windows to clang.

@ahoppen
Copy link
Member Author

ahoppen commented Oct 22, 2024

#1717 should fix the issue. I assume you were building a static library without --use-lld, which used link.exe, which doesn’t support --rsp-quoting=windows.

@hjyamauchi
Copy link

@ahoppen Yeah probably we were using link.exe. I will try locally test the fix and update here. Thank you for looking into this!

@hjyamauchi
Copy link

@ahoppen I was able to confirm that #1717 fixes the issue. Thank you!

@ahoppen
Copy link
Member Author

ahoppen commented Oct 24, 2024

Thanks for verifying, @hjyamauchi!

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.

3 participants