Skip to content

Rename ResolvedTarget to ResolvedModule #7459

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 2 commits into from
Apr 15, 2024
Merged

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
5 checks passed
@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 <m_desiatov@apple.com>
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