Skip to content

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Apr 15, 2024

Motivation:

With host/target triples separation in the SwiftPM codebase, it gets very confusing whether at a given moment "target" refers to a module, a triple, or a low level build system target.

Modifications:

Renamed ResolvedTarget to ResolvedModule. Added a deprecated typealias ResolvedTarget = ResolvedModule to allow graceful migration for users of this type.

Result:

Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs like PackageDescription and PackagePlugin, target there can stay as "target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context, like *TargetBuildDescription will be renamed in future PRs.

This has no impact on how these concepts are named in user-visible APIs like `PackageDescription` and `PackagePlugin`, target there can stay as "target" for as long as needed. With host/target triples separation in the SwiftPM codebase, it gets very confusing whether at a given moment "target" refers to a module, a triple, or a low level build system target.
@MaxDesiatov MaxDesiatov added no functional change No user-visible functional changes included dependency resolution labels Apr 15, 2024
…xd/resolved-module

# Conflicts:
#	Sources/PackageGraph/ModulesGraph.swift
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) April 15, 2024 10:57
@MaxDesiatov MaxDesiatov self-assigned this Apr 15, 2024
@MaxDesiatov MaxDesiatov added the public API Changes to the public API of SwiftPM label Apr 15, 2024
@MaxDesiatov MaxDesiatov disabled auto-merge April 15, 2024 12:36
@MaxDesiatov MaxDesiatov enabled auto-merge (squash) April 15, 2024 12:36
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

LGTM but I think it would be as important to follow-up with a PR that changes all the labels related to ResolvedModule from target to module as well as variables and other places where the type is used.

@@ -28,7 +28,7 @@ package final class ClangTargetBuildDescription {
package let package: ResolvedPackage

/// The target described by this target.
package let target: ResolvedTarget
package let target: ResolvedModule
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: looks like the comment needs updating as well :)

@@ -49,7 +49,7 @@ package final class ProductBuildDescription: SPMBuildCore.ProductBuildDescriptio
var additionalFlags: [String] = []

/// The list of targets that are going to be linked statically in this product.
var staticTargets: [ResolvedTarget] = []
var staticTargets: [ResolvedModule] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be updated in a follow-up?

@@ -33,7 +33,7 @@ package final class SwiftTargetBuildDescription {
package let package: ResolvedPackage

/// The target described by this target.
package let target: ResolvedTarget
package let target: ResolvedModule
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Same comment issue here.

@MaxDesiatov MaxDesiatov merged commit 27996b8 into main Apr 15, 2024
@MaxDesiatov MaxDesiatov deleted the maxd/resolved-module branch April 15, 2024 16:38
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
### Motivation:

With host/target triples separation in the SwiftPM codebase, it gets
very confusing whether at a given moment "target" refers to a module, a
triple, or a low level build system target.

### Modifications:

Renamed `ResolvedTarget` to `ResolvedModule`. Added a deprecated
`typealias ResolvedTarget = ResolvedModule` to allow graceful migration
for users of this type.

### Result:

Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs
like `PackageDescription` and `PackagePlugin`, target there can stay as
"target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context,
like `*TargetBuildDescription` will be renamed in future PRs.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
### Motivation:

With host/target triples separation in the SwiftPM codebase, it gets
very confusing whether at a given moment "target" refers to a module, a
triple, or a low level build system target.

### Modifications:

Renamed `ResolvedTarget` to `ResolvedModule`. Added a deprecated
`typealias ResolvedTarget = ResolvedModule` to allow graceful migration
for users of this type.

### Result:

Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs
like `PackageDescription` and `PackagePlugin`, target there can stay as
"target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context,
like `*TargetBuildDescription` will be renamed in future PRs.
xedin pushed a commit to xedin/swift-package-manager that referenced this pull request May 16, 2024
With host/target triples separation in the SwiftPM codebase, it gets
very confusing whether at a given moment "target" refers to a module, a
triple, or a low level build system target.

Renamed `ResolvedTarget` to `ResolvedModule`. Added a deprecated
`typealias ResolvedTarget = ResolvedModule` to allow graceful migration
for users of this type.

Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs
like `PackageDescription` and `PackagePlugin`, target there can stay as
"target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context,
like `*TargetBuildDescription` will be renamed in future PRs.

(cherry picked from commit 27996b8)
xedin pushed a commit to xedin/swift-package-manager that referenced this pull request May 16, 2024
With host/target triples separation in the SwiftPM codebase, it gets
very confusing whether at a given moment "target" refers to a module, a
triple, or a low level build system target.

Renamed `ResolvedTarget` to `ResolvedModule`. Added a deprecated
`typealias ResolvedTarget = ResolvedModule` to allow graceful migration
for users of this type.

Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs
like `PackageDescription` and `PackagePlugin`, target there can stay as
"target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context,
like `*TargetBuildDescription` will be renamed in future PRs.

(cherry picked from commit 27996b8)
xedin added a commit that referenced this pull request May 17, 2024
- Explanation:

With host/target triples separation in the SwiftPM codebase, it gets
very confusing whether at a given moment "target" refers to a module, a
triple, or a low level build system target.

Renamed `ResolvedTarget` to `ResolvedModule`. Added a deprecated
`typealias ResolvedTarget = ResolvedModule` to allow graceful migration
for users of this type.

  Confusion between target triples and package targets is reduced.

This has no impact on how these concepts are named in user-visible APIs
like `PackageDescription` and `PackagePlugin`, target there can stay as
"target" for as long as needed.

Remaining internal uses of "target" outside of "target triple" context,
like `*TargetBuildDescription` will be renamed in future PRs.

- Scope: NFC change

- Main Branch PRs:
#7459

- Risk: Very Low

- Reviewed By: @bnbarham  

- Testing: No new tests are necessary


(cherry picked from commit 27996b8)

Co-authored-by: Max Desiatov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependency resolution no functional change No user-visible functional changes included public API Changes to the public API of SwiftPM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants