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.

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
Merged
merged 1 commit into from
Jul 19, 2023
Merged

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

merged 1 commit into from
Jul 19, 2023

Conversation

@MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Jul 19, 2023 •
edited
Loading

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.

@rauhul @MaxDesiatov
Update tests to allow multiple -F options (#742)
c167d5b 
- 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

MaxDesiatov commented Jul 19, 2023

@swift-ci test

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

bnbarham commented Jul 19, 2023

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

MaxDesiatov commented Jul 19, 2023

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

Reviewers

@bnbarham bnbarham bnbarham approved these changes

@ahoppen ahoppen ahoppen approved these changes

@benlangmuir benlangmuir Awaiting requested review from benlangmuir

+1 more reviewer

@rauhul rauhul rauhul approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@MaxDesiatov MaxDesiatov

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MaxDesiatov @bnbarham @rauhul @ahoppen