-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
Fix some warnings with ProcessEnvironmentBlock
#7364
base: main
Are you sure you want to change the base?
Conversation
Previous implementation didn't account for case insensitivity on Windows. Cleaning up these warnings fixes that. This doesn't include APIs exposed by Swift Driver yet, as those are harder to adapt for `ProcessEnvironmentBlock` with significant sources compatibility breakage.
|
||
public typealias EnvironmentVariables = [String: String] | ||
public typealias EnvironmentVariables = ProcessEnvironmentBlock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to worry about this accidentally causing build issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we expose it anywhere outside of SwiftPM to any clients, at least not on purpose. IIUC corresponding TSC APIs were widely adopted instead. As soon as package
support is merged in my other PR, I'll try using package
visibility here to prevent any unintended adoption of this typealias in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use SPI in the meantime, would also make it easy to remember all the places we want package
😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can now use package
.
@swift-ci test |
…xd/process-environment-block # Conflicts: # Package.swift # Sources/SPMTestSupport/SwiftPMProduct.swift
…xd/process-environment-block # Conflicts: # Sources/Build/BuildOperation.swift # Sources/Commands/PackageCommands/InstalledPackages.swift # Sources/SPMTestSupport/MockManifestLoader.swift
@swift-ci test |
This APIs should not be marked `public` to prevent SwiftPM clients from adopting those by accident.
…-manager into maxd/process-environment-block # Conflicts: # Sources/Build/BuildOperationBuildSystemDelegateHandler.swift # Sources/LLBuildManifest/LLBuildManifest.swift # Sources/LLBuildManifest/LLBuildManifestWriter.swift # Sources/LLBuildManifest/Tools.swift
This fixes some warnings without more intrusive breaking changes for SwiftPM clients, as previously attempted in #7364. Also fixed some formatting inconsistencies.
This fixes some `ProcessEnv.vars` warnings without more intrusive breaking changes for SwiftPM clients, as previously attempted in #7364. Also fixed some formatting inconsistencies.
This fixes some `ProcessEnv.vars` warnings without more intrusive breaking changes for SwiftPM clients, as previously attempted in swiftlang#7364. Also fixed some formatting inconsistencies.
This fixes some `ProcessEnv.vars` warnings without more intrusive breaking changes for SwiftPM clients, as previously attempted in swiftlang#7364. Also fixed some formatting inconsistencies.
Previous implementation didn't account for case insensitivity on Windows. Cleaning up these warnings fixes that.
This doesn't include APIs exposed by Swift Driver yet, as those are harder to adapt for
ProcessEnvironmentBlock
without significant source compatibility breakage.