Skip to content

Add DriverKit as a supported platform #3293

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
Feb 20, 2021
Merged

Add DriverKit as a supported platform #3293

merged 1 commit into from
Feb 20, 2021

Conversation

neonichu
Copy link
Contributor

This adds DriverKit as a support platform, so that a minimum deployment target can be specified and conditional build settings can be configured.

rdar://66656200

neonichu
neonichu commented Feb 18, 2021
View reviewed changes
Sources/PackageDescription/SupportedPlatforms.swift
@@ -31,6 +31,9 @@ public struct Platform: Encodable, Equatable {
/// The watchOS platform.
public static let watchOS: Platform = Platform(name: "watchos")

/// The DriverKit platform
public static let driverKit: Platform = Platform(name: "driverkit")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to deal with availability here since it seems like if #available does not work for _PackageDescription. That means if I add availability here, the static functions can't be compiled.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just recently found out that _PackageDescription is a special thing — is it a bug that #ifavailable doesn't work for it, do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yah, that's what I was thinking. It seems we so far only added platform constants for platforms without versioning, so we may not have noticed that something is missing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not too bad to leave this without availability for now, but we will have to figure something out before we ship another release in the future.

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

abertelrud
abertelrud approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

Nice. Does it make sense to also updated PackageDescription.md and ChangeLog.md at the same time?

jakepetroules
jakepetroules requested changes Feb 19, 2021
View reviewed changes
@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Feb 19, 2021
@neonichu
Copy link
Contributor Author

Nice. Does it make sense to also updated PackageDescription.md and ChangeLog.md at the same time?

Not sure what the process should be, but it doesn't seem right to add API scoped to 999 to the PackageDescription docs.

@neonichu neonichu force-pushed the support-platform-driverkit branch from db4073f to 86fdd2b Compare February 19, 2021 23:12
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test linux

jakepetroules
jakepetroules approved these changes Feb 20, 2021
View reviewed changes
Copy link
Contributor

@jakepetroules jakepetroules left a comment

Choose a reason for hiding this comment

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

Just one small nit in the version example comment, otherwise LGTM! Thanks!

@neonichu
Add DriverKit as a supported platform
66fee51 
This adds DriverKit as a support platform, so that a minimum deployment target can be specified and conditional build settings can be configured.

rdar://66656200
@neonichu neonichu force-pushed the support-platform-driverkit branch from 86fdd2b to 66fee51 Compare February 20, 2021 01:42
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

/Users/buildnode/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swift-driver/Sources/SwiftDriverExecution/llbuild.swift:21:29: error: compiled module was created by an older version of the compiler; rebuild 'llbuildSwift' and try again: /Users/buildnode/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/buildbot_incremental/llbuild-macosx-x86_64/products/llbuildSwift/llbuildSwift.swiftmodule

🕺

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test macOS

@neonichu neonichu merged commit 1a3b735 into swiftlang:main Feb 20, 2021
@neonichu neonichu deleted the support-platform-driverkit branch February 20, 2021 16:09
@jakepetroules jakepetroules mentioned this pull request Apr 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakepetroules jakepetroules jakepetroules approved these changes

@abertelrud abertelrud abertelrud approved these changes

@friedbunny friedbunny Awaiting requested review from friedbunny

@tomerd tomerd Awaiting requested review from tomerd

Assignees

@neonichu neonichu

Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@neonichu @jakepetroules @abertelrud @tomerd