Skip to content
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

Swift SDKs: fix toolset.linker.path not passed to -ld-path #7021

Merged
merged 3 commits into from
Nov 7, 2023
Merged

Swift SDKs: fix toolset.linker.path not passed to -ld-path #7021

merged 3 commits into from
Nov 7, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Oct 18, 2023 •
edited
Loading

This reverts commit 57d0a55 and PR #6939.

Now that swiftlang/swift-driver#1447 and its 5.10 counterpart swiftlang/swift-driver#1454 were merged, we can reapply the fix for Swift SDKs linker metadata not being handled.

Resolves rdar://117049947.

@MaxDesiatov
Swift SDKs: fix toolset.linker.path not passed to -ld-path
13b9264 
This reverts commit 57d0a55 and PR #6939.

Now that swiftlang/swift-driver#1447 and its 5.10 counterpart swiftlang/swift-driver#1454 were merged, we can reapply the fix for Swift SDKs linker metadata not being handled.
@MaxDesiatov MaxDesiatov self-assigned this Oct 18, 2023
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@neonichu
Copy link
Contributor

So we're saying we only support 5.10 and up going forward?

Can we add an explicit error if someone is using a 5.10 SwiftPM with an older compiler?

@MaxDesiatov
Copy link
Contributor Author

@neonichu what would be the best way to get a compiler version? I tried to reuse *GetVersion* LLBuild targets, but that happens as a part of the build. Here I reckon we need to get a version before the build has even started? Should that be connected to those *GetVersion LLBuild targets at all, or would we prefer to keep that separate?

@neonichu
Copy link
Contributor

I wonder if we can just defer until the build? It's not 100% ideal but probably the best way to avoid code duplication. E.g. I could see that we put a minimum required version into the SwiftSDK or toolset types and basically check them against the actual version early in the build. This would of course mean that we're slightly delaying such errors, but I think that's acceptable.

@neonichu
Copy link
Contributor

We could also have a reason string for the requirement, so we would still be able to emit a good diagnostic.

@kateinoigakukun
Copy link
Member

@MaxDesiatov Recently toolchain ships usr/share/swift/features.json, which describes what features the compiler supports. SwiftDriver is using the file to check whether the selected frontend compiler supports a specific feature. Can we use the same approach in SwiftPM to detect ld-path feature?

@MaxDesiatov
Copy link
Contributor Author

This sounds good to me, @neonichu WDYT?

@neonichu
Copy link
Contributor

Sounds good to me, I think that's the supportedFrontendFeatures API in swift-driver? We could add that to DriverSupport. Since we don't want to link the swift-driver library to anything but the Build module, we'll still have to defer the check to build-time, though.

This was referenced Nov 1, 2023
Merged
Merged
MaxDesiatov added a commit to swiftlang/swift that referenced this pull request Nov 2, 2023
@MaxDesiatov
Add ld-path-driver-option to features.json (#69586)
b7d8a9b 
Dependency of swiftlang/swift-package-manager#7021.

The `-ld-path` option was introduced on `main` in swiftlang/swift-driver#1442 and `release/5.10` in swiftlang/swift-driver#1442. SwiftPM needs to detect this flag to pass options to the driver correctly, and it's suitable to do this via `feature.json` instead of checking for the compiler version via other means.

Partially resolves rdar://117049947.
MaxDesiatov added a commit to swiftlang/swift that referenced this pull request Nov 4, 2023
@MaxDesiatov
[5.10] Add ld-path-driver-option to features.json (#69587)
097554b 
Dependency of swiftlang/swift-package-manager#7021.

Cherry-pick of #69586.

The `-ld-path` option was introduced on `main` in swiftlang/swift-driver#1442 and 5.10 in swiftlang/swift-driver#1442. SwiftPM needs to detect this flag to pass options to the driver correctly, and it's suitable to do this via `feature.json` instead of checking for the compiler version via other means.

Partially resolves rdar://117049947.
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) November 7, 2023 11:38
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

@MaxDesiatov MaxDesiatov merged commit 1d6565c into main Nov 7, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/revert-the-linker-path-revert branch November 7, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neonichu neonichu neonichu approved these changes

@kateinoigakukun kateinoigakukun kateinoigakukun approved these changes

@tomerd tomerd Awaiting requested review from tomerd

Assignees

@MaxDesiatov MaxDesiatov

Labels
bug cross-compilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MaxDesiatov @neonichu @kateinoigakukun