Skip to content

Remove implicit availability of Foundation from future manifest versions#5874

Merged
neonichu merged 1 commit into
mainfrom
fix-implicit-foundation
Nov 16, 2022
Merged

Remove implicit availability of Foundation from future manifest versions#5874
neonichu merged 1 commit into
mainfrom
fix-implicit-foundation

Conversation

@neonichu

@neonichu neonichu commented Nov 4, 2022

Copy link
Copy Markdown
Contributor

This changes all imports of Foundation in the PackageDescription module to be implementation-only, making it so that it needs to be explicitly imported if being used. To avoid breaking existing packages, we inject an explicit import into manifests with tools-version 5.7 or earlier. In the next tools-version, Foundation will no longer be implicitly available.

rdar://92879696

@neonichu neonichu self-assigned this Nov 4, 2022
@@ -11,7 +11,6 @@
//===----------------------------------------------------------------------===//

import Foundation
import PackageModel

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since ContextModel is part of PackageLoading and PackageDescription via a symlink, we need to import Foundation implementation-only in both modules. This is incompatible with IdentityResolver since it uses Foundation.URL as part of its public interface. To resolve this, I moved IdentityResolver into PackageModel here.

@neonichu

neonichu commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@neonichu

neonichu commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author
/home/build-user/swiftpm/Sources/PackageLoading/ManifestLoader.swift:68:24: error: cannot use class 'DispatchQueue' here; 'Dispatch' has been imported as implementation-only
        delegateQueue: DispatchQueue,

Interesting...

@MaxDesiatov

Copy link
Copy Markdown
Contributor

Should a note about this be added to CHANGELOG.md, as it is a relatively significant change?

@tomerd

tomerd commented Nov 4, 2022

Copy link
Copy Markdown
Contributor

Should a note about this be added to CHANGELOG.md, as it is a relatively significant change?

+1

@neonichu neonichu force-pushed the fix-implicit-foundation branch from 8737493 to c1d350a Compare November 9, 2022 01:35
@neonichu

neonichu commented Nov 9, 2022

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@neonichu

Copy link
Copy Markdown
Contributor Author
[1016/1064] Emitting module PackageLoading
/home/build-user/swiftpm/Sources/PackageLoading/ManifestLoader.swift:68:24: error: cannot use class 'DispatchQueue' here; 'Dispatch' has been imported as implementation-only
        delegateQueue: DispatchQueue,

Not entirely sure why I am not seeing these locally 🤔

@neonichu neonichu force-pushed the fix-implicit-foundation branch from c1d350a to 228fd6a Compare November 16, 2022 07:43
@neonichu

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@neonichu neonichu force-pushed the fix-implicit-foundation branch from 228fd6a to 202c4db Compare November 16, 2022 16:44
…sions

This changes all imports of `Foundation` in the `PackageDescription` module to be implementation-only, making it so that it needs to be explicitly imported if being used. To avoid breaking existing packages, we inject an explicit import into manifests with tools-version 5.7 or earlier. In the next tools-version, `Foundation` will no longer be implicitly available.

rdar://92879696
@neonichu neonichu force-pushed the fix-implicit-foundation branch from 202c4db to c810bd7 Compare November 16, 2022 16:47
@neonichu

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@neonichu

Copy link
Copy Markdown
Contributor Author

@swift-ci please test package compatibility

@neonichu neonichu merged commit 0ade4d8 into main Nov 16, 2022
@neonichu neonichu deleted the fix-implicit-foundation branch November 16, 2022 21:17
drodriguez added a commit to drodriguez/swift-package-manager that referenced this pull request Feb 15, 2023
Reverts 1998284 / swiftlang#3526

Related to SR-14718 (swiftlang#4416).

Since swiftlang#3526 was merged, swiftlang#5874 tried to apply the original idea of using
`@_implementationOnly Foundation` in PackageDescription/Plugin to avoid
leaking `Foundation` into the manifests.
neonichu pushed a commit that referenced this pull request Feb 21, 2023
…#6157)

Reverts 1998284 / #3526

Related to SR-14718 (#4416).

Since #3526 was merged, #5874 tried to apply the original idea of using
`@_implementationOnly Foundation` in PackageDescription/Plugin to avoid
leaking `Foundation` into the manifests.
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.

3 participants