[PackageModel] SwiftTarget: Rename swiftVersion into toolsSwiftVersion
#7544
Conversation
| @@ -129,14 +129,14 @@ public final class SwiftTarget: Target { | |||
|
|
|||
| override public func encode(to encoder: Encoder) throws { | |||
| var container = encoder.container(keyedBy: CodingKeys.self) | |||
| try container.encode(self.swiftVersion, forKey: .swiftVersion) | |||
| try container.encode(self.toolSwiftVersion, forKey: .swiftVersion) | |||
There was a problem hiding this comment.
@MaxDesiatov is it okay to rename case in CodingKeys here? I'm not sure how much do we care about backward compatibility here.
There was a problem hiding this comment.
I'm still unsure where this Codable conformance is used, maybe @neonichu could clarify? I wonder if this could be used in workspace serialization, but also don't know how easily we can break that
There was a problem hiding this comment.
I don't remember.
But removing the Encodable conformance from PackageModel.Package (which is the only thing broken by removing the conformance from Target) doesn't seem to break anything, it seems like this is potentially something that's no longer being used?
There was a problem hiding this comment.
I can do this in a follow up!
|
I need to change this for |
…sion` The value of this field is computed based on the tools version of the manifest and is intended to be used as a fallback if there is no swift language version specified in the build settings for a swift target.
… into `toolsSwiftVersion`
xedin
force-pushed
the
rename-swift-target-swift-version
branch
from
0dd0d44 to
e4f814b
Compare
|
@swift-ci please test |
|
@swift-ci please test Windows platform |
|
I don't think a simple rename is sufficient to fix the model here. It seems like #7439 moved the version into build settings, but only for certain declarations. That is fine, but to me implies Either way, I don't think it is a good idea to carry forward the degrees of freedom of the package manifest into the resolved model and leave it up to clients to figure out which version to actually use. (Another aside, #7504 is also broken because of this since I hadn't realized we changed the contract around |
- Explanation:
Implementation of SE-0435 proposal.
Add a new Swift target setting API, similar to `enable{Upcoming,
Experimental}Feature`,
to specify a Swift language version that should be used to build the
target, if such
version is not specified, fallback to the current language version
determination logic.
- Scope: TargetDescription API and build system.
- Main Branch PRs:
#7439,
#7544,
#7550,
#7557
- Radar: rdar://125732014
- Risk: Low
- Reviewed By: @MaxDesiatov @bnbarham
- Testing: Added new test-cases to the test suite.
Motivation:
The value of this field is computed based on the tools version of the manifest and is intended to be used as a fallback if there is no swift language version specified in the build settings for a swift target.
Modifications:
Renamed
SwiftTarget.swiftVersionfield andPackageBuilder.swiftVersion()method totoolsSwiftVersion.Result:
Code better reflects meaning and intended uses of the data.