Skip to content

[5.9] Update tests to allow multiple -F options #775

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 19, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Jul 19, 2023

Cherry-pick of #742 required to unblock swiftlang/swift-package-manager#6709.

Updates tests to allow compile arguments with multiple -F options such as -F <foo> -Xcc -F -Xcc <foo>. This change is required to support SwiftPM changes which forward C compiler flags to the Swift compiler in more cases.

- Updates tests to allow compile arguments with multiple -F options such
  as -F <foo> -Xcc -F -Xcc <foo>. This change is required to support spm
  changes which forward c compiler flags to the swift compiler in more
  cases.
@MaxDesiatov MaxDesiatov requested a review from rauhul July 19, 2023 08:27
@MaxDesiatov MaxDesiatov self-assigned this Jul 19, 2023
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov requested a review from bnbarham July 19, 2023 15:08
@bnbarham
Copy link
Contributor

Just to be clear, is there ever any -F foo -F other or is it just to accommodate -Xcc -F -Xcc foo? I'm okay taking this to unblock things either way, but we could handle that better rather than just arbitrarily allowing any number. The original PR (#28) is more about there being a single -target though, so maybe allowing any number of -F is still fine 🤷

@rauhul
Copy link
Member

rauhul commented Jul 19, 2023

The change is needed because -F flags can arise in a number of places in SPM and it is not valid for sourcekit to assume only a single occurrence. In this case -F now appears twice, once for swift-frontend as -F foo and once for cc1 as -Xcc -F -Xcc foo.

@MaxDesiatov
Copy link
Contributor Author

Thanks for the clarification! Fine to merge this then?

@MaxDesiatov MaxDesiatov merged commit 0c78e30 into release/5.9 Jul 19, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/5.9-multiple-f-options branch July 19, 2023 18:40
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