Skip to content

[6.0] Cherry-pick recent NFC changes to reduce merge conflicts #7721

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

Merged
merged 9 commits into from
Jun 27, 2024

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Jun 27, 2024

Includes these PRs cherry-picked off main

Explanation: Cherry-pick of recent NFC changes, which makes it easier to cherry-pick actual bug fixes onto 6.0 due to the reduced number of merge conflicts.
Scope: broad, includes both modules graph and llbuild-related changes.
Risk: low, the test suite is passing, no functional changes are included, and cherry-picked changes were incubated on main for some time.
Testing: Existing automated test suite.
Issue: N/A
Reviewers: @xedin @MaxDesiatov @rauhul

MaxDesiatov and others added 9 commits June 27, 2024 11:35
`#if swift(<6.0)` is a wrong check to apply, since it has no effect with 6.0 compiler versions, unlike `#if compiler(<6.0)`.

(cherry picked from commit 067136b)
…ng (#7660)

### Motivation:

Trying to make it possible to share `createBuildOperation` between
multiple implementations of `SPMCoreBuild.BuildSystem` in preparation to
introduce an operation that would build plugin tools.

### Modifications:

- Rename `BuildOperationBuildSystemDelegateHandler` into
`LLBuildProgressTracker` which is a more neutral name that could be used
by different llbuild operations if necessary.
- Integrate `commandFailureHandler` into the progress tracker
- Make `BuildOperation.createBuildSystem` stateless and use a single
member to set both a new build system and its tracker.

### Result:

The change it make it much easier to move `createBuildSystem` out of
`BuildOperation` and into `SPMCoreBuild.BuildSystem` itself.

(cherry picked from commit 9c0e48e)
#7667)

…ildCommands, LLBuildDescription and LLBuildProgressTracker

### Motivation:

There are 3 logically distinct things in this giant file which are
better kept separate.

### Modifications:

- Splits BuildOperationBuildSystemDelegateHandler.swift into 3 files -
one for commands, one for build description and one for progress
tracking.

### Result:

Concerns are properly separated.

(cherry picked from commit 8a4364c)
* Remove mention of `nil` name which is not allowed due to the parameters being non-optional.
* Synchronize wording describing local package paths.

(cherry picked from commit 83214f0)

# Conflicts:
#	Sources/PackageDescription/PackageDependency.swift
The conformances are unused and pull in an additional
`TSCUtility.PolymorphicCodableProtocol` dependency. This change removes
about 0.2-0.4 MB from final executables on macOS when comparing with
`swift build -c release` to `main`.

Also allows us to rename these types more easily to disambiguate between
host/target triples and manifest targets/modules in our code.

If there's an existing user of this JSON manifest serialization we're
unaware of, they should rely on `swift package describe --type json`
instead, which has stable, consistent, and tested output.

(cherry picked from commit 9e5f8b7)

# Conflicts:
#	Sources/PackageModel/BuildSettings.swift
#	Sources/PackageModel/Manifest/PackageConditionDescription.swift
Cleaning up `PackageGraph` and `Build` code naming inconsistencies after
`ResolvedTarget` to `ResolvedModule` renaming. User-visible strings and
symbols still reference it as "target", but at least it's easier to
navigate for contributors. `public` symbols gained deprecation notices
where this was possible and easy to add.

(cherry picked from commit e13a3aa)

# Conflicts:
#	Sources/Build/BuildManifest/LLBuildManifestBuilder.swift
#	Sources/PackageGraph/ModulesGraph+Loading.swift
#	Sources/PackageGraph/ModulesGraph.swift
#	Sources/PackageGraph/Resolution/ResolvedPackage.swift
#	Sources/PackageLoading/ModuleMapGenerator.swift
#	Sources/SPMTestSupport/MockBuildTestHelper.swift
#	Sources/SPMTestSupport/MockWorkspace.swift
#	Sources/SourceKitLSPAPI/BuildDescription.swift
#	Tests/BuildTests/ClangTargetBuildDescriptionTests.swift
#	Tests/WorkspaceTests/WorkspaceTests.swift
Adds a new `Environment` type to replace TSCBasics EnvironmentVariables
and ProcessEnvironmentBlock types. Updates points of use of the older
APIs with the newer API and adds unit tests for `Environment`.

(cherry picked from commit 74aea87)
We'd like to implement more changes to `TSCBasic.Process` without
fearing of breaking clients, in addition to the fact that TSC is
deprecated.

One thing in particular we need is backpressure logic (coming in a
future PR) when reading from process instances in plugins communication
with the host SwiftPM process. Currently without this logic, a naive
rewrite of plugins communication with structured concurrency is quite
bug-prone, as it loses messages ordering and can quickly fill up all
memory with resource-intensive plugins.

Thus `TSCBasic.Process` is slightly cleaned up and is vendored as
`AsyncProcess` in this change. We also give it more coverage in tests by
utilizing `async` overloads, which are the overloads we should be using
more in the structured concurrency conversion of the SwiftPM codebase.

(cherry picked from commit b86d22e)

# Conflicts:
#	Tests/CommandsTests/PackageCommandTests.swift
#	Tests/FunctionalTests/MiscellaneousTests.swift
#	Tests/FunctionalTests/PluginTests.swift
#	Tests/FunctionalTests/TraitTests.swift
@MaxDesiatov MaxDesiatov requested a review from a team as a code owner June 27, 2024 11:02
@MaxDesiatov MaxDesiatov requested a review from bnbarham June 27, 2024 11:02
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov removed the request for review from bnbarham June 27, 2024 11:02
@MaxDesiatov
Copy link
Contributor Author

CI seems to be blocked on swift-format errors apparently unrelated to this change.

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) June 27, 2024 16:56
@MaxDesiatov MaxDesiatov added no functional change No user-visible functional changes included swift 6.0 Related to Swift 6.0 release branch labels Jun 27, 2024
@MaxDesiatov MaxDesiatov merged commit 4e03b81 into release/6.0 Jun 27, 2024
5 checks passed
@MaxDesiatov MaxDesiatov deleted the maxd/6.0-nfc branch June 27, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no functional change No user-visible functional changes included swift 6.0 Related to Swift 6.0 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants