Skip to content

Fix symlinked swift-experimental-destination invocations #6382

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 4 commits into from
Apr 5, 2023
Merged

Fix symlinked swift-experimental-destination invocations #6382

merged 4 commits into from
Apr 5, 2023

Conversation

@MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Apr 4, 2023

swift experimental-destination subcommands no longer work and always display help output. This is a regression introduced in #6362 after making SwiftDestinationTool conform to AsyncParsableCommand instead of ParsableCommand but not updating swift-package-manager command sources to call async overload of main() function on SwiftDestinationTool.

rdar://107618266

@MaxDesiatov
Fix symlinked swift-experimental-destination invocations
ebb8feb 
`swift experimental-destination` subcommands no longer work and always display help output. This is a regression introduced in #6362 after making `SwiftDestinationTool` conform to  `AsyncParsableCommand` instead of `ParsableCommand` but not updating `swift-package-manager` command sources to call `async` overload of `main()` function on `SwiftDestinationTool`.

rdar://107618266
@MaxDesiatov MaxDesiatov added the bug label Apr 4, 2023
@MaxDesiatov MaxDesiatov self-assigned this Apr 4, 2023
neonichu
neonichu reviewed Apr 4, 2023
View reviewed changes
Sources/swift-package-manager/SwiftPM.swift Outdated
@main
struct SwiftPM {
static func main() {
static func main() async {
Copy link
Contributor

@neonichu neonichu Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can do this until the CI has been updated to macOS 12?

Copy link
Contributor Author

@MaxDesiatov MaxDesiatov Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right 😭

Copy link
Contributor

@tomerd tomerd Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main.swift here we come

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

Sporadic macOS test failure?

"/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/TemporaryDirectory.nBShZq/miscellaneous_atmainsupport-manifest"])
    /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk/usr/lib/swift/Foundation.swiftmodule/x86_64-apple-macos.swiftinterface:4:8: error: no such module 'Combine'
    import Combine
           ^
    /private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_AtMainSupport.Elc6ri/Miscellaneous_AtMainSupport/Package.swift:1:8: error: failed to build module 'Foundation'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.5 (swiftlang-1300.0.27.6 clang-1300.0.27.2)', while this compiler is 'Apple Swift version 5.9-dev (LLVM 2d2bb9c7a619033, Swift 2f99a4ee8978e3d)'). Please select a toolchain which matches the SDK.
    import Foundation
           ^
    "Test Case '-[CommandsTests.BuildToolTests testAtMainSupport]' failed (21.090 seconds).

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test macos

@MaxDesiatov
[5.9] Fix symlinked swift-experimental-destination invocations
2ac29a1 
`swift experimental-destination` subcommands no longer work and always display help output. This is a regression introduced in #6362 after making `SwiftDestinationTool` conform to  `AsyncParsableCommand` instead of `ParsableCommand` but not updating `swift-package-manager` command sources to call `async` overload of `main()` function on `SwiftDestinationTool`.

rdar://107618266
MaxDesiatov
MaxDesiatov commented Apr 4, 2023
View reviewed changes
Sources/PackageModel/DestinationBundle.swift
_ = try await withTemporaryDirectory(
fileSystem: fileSystem,
) throws {
_ = try withTemporaryDirectory(
Copy link
Contributor Author

@MaxDesiatov MaxDesiatov Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The downside of this change is that non-async withTemporaryDirectory does not take a FileSystem argument, which makes it harder to test. But this is the only way until our macOS CI nodes have Xcode with support for concurrency back-deployment.

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci test windows

1 similar comment
@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test windows

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test

@MaxDesiatov MaxDesiatov requested a review from neonichu April 4, 2023 21:04
@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 4, 2023

@swift-ci smoke test macos

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 5, 2023

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Apr 5, 2023

@swift-ci smoke test macos

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) April 5, 2023 08:34
@MaxDesiatov MaxDesiatov merged commit b2d7a82 into main Apr 5, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/fix-async-experimental-destination branch April 5, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abertelrud abertelrud Awaiting requested review from abertelrud

@neonichu neonichu Awaiting requested review from neonichu

1 more reviewer

@tomerd tomerd tomerd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@MaxDesiatov MaxDesiatov

Labels

bug cross-compilation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MaxDesiatov @neonichu @tomerd