SE-0291 (2nd review): Package Collections

tomerd · 2020-11-30T20:34:07.039Z

The second review of SE-0291: Package Collections, begins now and runs through December 7, 2020.

Based on the feedback from the pitch and first review, the core team feels the ideas behind Package Collections are useful and put the Swift Packages ecosystem on the right path.

However, during the first review, several community members requested to learn more about the Package Collection data format. The core team felt the proposal should be amended to explicitly call out if the data format is part of the feature specification or not, and provide reasoning for its inclusion or exclusion.

The proposal was amended [1][2] to include this information, as well as address other feedback from the first review, and is now ready for a second review.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager or direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thank you for helping improve the Swift programming language and ecosystem.

Tom Doron
Review Manager

[1] Amendment to SE-0291 Package Collections by yim-lee · Pull Request #1207 · apple/swift-evolution · GitHub
[2] Update package collections proposal with revisions by neonichu · Pull Request #1208 · apple/swift-evolution · GitHub

dabrahams · 2020-11-30T21:04:58.205Z

I know it's a small thing, but can someone explain the rationale for the trailing s in the swift package-collections command prefix? It seems completely unnecessary to me, and IMO most of the commands would read better without it.

glessard · 2020-12-01T00:57:34.404Z

There remain 13 instances of the word "feed", including in the 2nd sentence. It looks to me like they were left behind from the original pitch nomenclature.

yim_lee · 2020-12-01T01:44:26.345Z

Thanks for catching it. Opened Replace out-of-date nomenclature by yim-lee · Pull Request #1213 · apple/swift-evolution · GitHub.

Karl · 2020-12-01T08:19:33.115Z

tomerd:

begins now and runs through December 7, 2020

IMO, 7 days is not adequate time for a review.

Mordil · 2020-12-01T19:23:59.982Z

I want to start off by saying that I highly appreciate each and every Swift Evolution proposal as they move forward the entire tooling suite for the language ecosystem in real ways.

It takes a lot of effort to not only design a solution to an identified problem, but also to distill it into a small enough proposal that addresses the problem without extensive changes that can be reviewed by many who might not have a full-time capacity to review and understand as well as the authors. On top of handling criticism from people who you might not know.

I also appreciate the 2nd review from trying to address the feedback given from the first review in order to reach a better solution for all involved.

That being said, I don't think the main contention of the first review has been adequately addressed.

During the pitch phase, and the first review, the community and authors' opinions have been in disagreement whether or not the collection format should be under review in some capacity as part of the commands.

The proposal has been updated to address this as follows:

Package collections must adhere to a specific JSON format for SwiftPM to be able to consume them. A draft of the JSON format has been posted in the forums, but it is not part of this proposal because it is not considered stable API. Over time as the data format matures, we will consider making it stable API in a separate proposal.

Since the data format is unstable, users should avoid generating package collections on their own. This proposal includes providing the necessary tooling for generating and consuming package collections.

In my opinion this is not an adequate answer:

Hyrum's Law which, roughly, states: "Any observable behavior will be relied upon by someone"
The proposal's own stated motivation is that people, such as educators and "community influencers" will produce collections

Elaborating on the 2nd point, this proposal does not provide commands to create the collection files as it claims in the updated section, so how are not machines to create these files for SwiftPM to use if they do not know what the format is?

If the data format is unstable, and going to change according to whatever force might influence its evolution, what is the point of having the formatVersion field?

I don't disagree that the data format should be considered unstable and be allowed to iterate outside of Swift Evolution as part of an incubation period for the format before a formal acceptance review.

However, given that format changes are very unlikely to change between SwiftPM versions - which are tied to Swift release cycles - why couldn't we publish the format as it stands for each version?

The proposal could have a section with the format as it is currently supported with a disclaimer:

The collection file format is not considered "ABI/API" stable between SwiftPM versions, but the current format as of SE-0291 is available for reference below.

and a document in the SwiftPM repo is maintained with the format of the collection file as it is used by a given SwiftPM release.

I think that appropriately bridges the gap between the community's concerns, as well as the authors' desire to have freedom to iterate and test.

owenv · 2020-12-02T06:31:39.598Z

It looks like the format of ~/.swiftpm/config is described as an implementation detail in the proposal, is that potentially going to be unstable as well? If so, per-toolchain config files might be necessary so that, for example, installing a prerelease snapshot that upgrades the format doesn't make a user's config unreadable by the SPM in their existing toolchain.

yim_lee · 2020-12-02T21:20:11.687Z

Mordil:

this proposal does not provide commands to create the collection files as it claims in the updated section, so how are not machines to create these files for SwiftPM to use if they do not know what the format is?

The proposal does not include commands for creating collection files because they are published as a separate tool: GitHub - apple/swift-package-collection-generator: A collection of packages and tooling for generating and consuming package feeds.. The recommendation of using this tool to generate package collections still holds--if the tool is lacking in any way, we encourage people to contribute and help improve it.

Mordil:

If the data format is unstable, and going to change according to whatever force might influence its evolution, what is the point of having the formatVersion field?

Regardless of whether the data format is stable or not, given a collection file, SwiftPM needs some way of knowing if it can parse it and how, and that's what formatVersion is for. It's almost certain that SwiftPM will need to support more than one format version because we can't expect all publishers to regenerate their collections each time the format gets updated. Each version of SwiftPM should know the collection format(s) that it supports.

Mordil:

However, given that format changes are very unlikely to change between SwiftPM versions - which are tied to Swift release cycles - why couldn't we publish the format as it stands for each version?

An update to the data format doesn't necessarily require a change in SwiftPM; only when it affects how libSwiftPM APIs and/or CLI commands function would it require a new SwiftPM version. As an example, it could be that the implementation of describe command has flexibility built-in to display all properties prefixed with alwaysDisplay_. Adding a new alwaysDisplay_newProperty to the format would not affect SwiftPM. True, this is less likely to happen than having to change both format and SwiftPM together, but we want to allow that freedom and so the format is evolved separately from SwiftPM.

Mordil:

The proposal could have a section with the format as it is currently supported with a disclaimer:

The proposal already includes a link to the data format in the section that you have quoted. Given what I wrote above about keeping the format separate, do you still think that it should be included inline?

Mordil · 2020-12-02T21:47:34.585Z

yim_lee:

The proposal does not include commands for creating collection files because they are published as a separate tool: GitHub - apple/swift-package-collection-generator: A collection of packages and tooling for generating and consuming package feeds.. The recommendation of using this tool to generate package collections still holds--if the tool is lacking in any way, we encourage people to contribute and help improve it.

I wasn't aware that this tool or it's repository existed, as it wasn't mentioned in the proposal in any revision, nor any forum post related to SE-0291 or the package collection format.

This changes things entirely.

If the "Data Format" section of proposal was updated to mention that the format is currently maintained between the new tool (with a link to it) and SwiftPM, and a link to the v1 Format I'd consider the concerns addressed.

yim_lee · 2020-12-02T22:17:18.486Z

I wasn't aware that this tool or it's repository existed, as it wasn't mentioned in the proposal in any revision, nor any forum post related to SE-0291 or the package collection format.

We only mentioned we will provide the tooling, so apologies for not being clearer.

I submitted a PR to update the proposal per your suggestions:

Add link to the generator tool
Update link to the JSON format

Mordil · 2020-12-02T22:57:28.444Z

Thanks you!

I’m not seeing any reason to reject this proposal

mattt · 2020-12-07T18:56:04.882Z

@yim_lee A few questions about the JSON format that I don't believe have come up yet:

According to the v1 format, the toolsVersion property for each package version object corresponds to "The tools (semantic) version specified in Package.swift". Does this account for version-specific manifest selection (e.g. Package@swift-4.2.swift)?
Relatedly, does the swift-package-collection command currently account for / plan to support version-specific tag selection (e.g. 1.2.0@swift-4.2)?
What do you think about representing Swift support as a cross-product / matrix of verifiedSwiftVersions and verifiedPlatforms? For example, a library may work on macOS and Linux, but rely on a change in swift-corelibs-foundation in Swift 5.3 for Linux (e.g. JSONEncoder.withoutEscapingSlashes?) Should there be a way to communicate compatibility exceptions somehow?
Could you speak more to the rationale behind the revision value in the JSON payload? Is the intention for that to be used for cache invalidation?

yim_lee · 2020-12-07T22:44:22.576Z

@mattt Since your questions pertain to the collection format, I answered them under the Package Collection Format post.

tomerd · 2020-12-08T23:06:06.752Z