SE-0305: Package Manager Binary Target Improvements

The review of SE-0305, begins now and runs through March 23, 2021.

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 programing language and ecosystem.

Tom Doron
Review Manager

9 Likes

There is a spelling mistake in the heading “Introdction”. Otherwise, I am pleased SPM is becoming more mature and can’t wait to see these features available to dependencies that need extra tooling.

Overall, I think this solves an important problem. The proposal fits well with the feel and direction of Swift with a few exceptions noted below. I am not expert on how other package managers handle this (and it would be interesting to get the perspective of those who are well versed in any alternative designs). I read through the proposal and gave it a moderate amount of consideration.

Notes on the text:

  • Please provide an example (can be minimal and contrived) of the structure of the archive and the manifest. The text is very formalistic, and a reader has to read through multiple paragraphs to begin to understand "< name >", "< artifact >", "< executable >", and "< variant >", and then to imagine what ultimately this would look like. An actual example alongside this detail would, I suspect, be immediately understandable to many.

Notes on the proposed design:

  • It is unclear to me why the proposed suffixes are .arar and .ari: these are not easy to search, meaningless when read aloud, and do not fit with the pattern we've established in Swift of suffixes such as .swiftdoc, .swiftmodule, etc. I would strongly suggest something with a swift prefix and without unusual abbreviations: .swiftpmartifacts and .swiftpmartifactindex seem fine. I see no need for brevity here.

  • There is duplication between directory structure and manifest regarding how variant paths are specified. It seems error-prone, when a user is already encouraged to use target triples as variant directories, then to list "path" and "supportedTriples" separately: if there is copy-pasta error, then the wrong path could be associated with a target, while the directory structure is just fine and the error needn't have occurred. It certainly seems reasonable to allow manifests to specify "path" and "supportedTriples" for each variant, but there should be a single source of truth where this duplication isn't necessary (i.e., where the package observes the convention of using a target triple as the variant directory name). Alternatively or in addition, there should be described some way that users can test that their artifact archive layout, manifests, and indexes are valid without setting up CI to build for each supported target triple.

8 Likes

looks good, agree with @xwu on the .ari .arar extensions... thats not very swift sounding...

.swiftarchive / .swiftarchiveindex or something like that sounds better

Given how fine grained target-triples can be, path and supportedTriples are almost never exactly the same. As an extreme example take a universal binary that supports many/all target-triples. In such case, you would not name the directory after all the triples but rather something like "universal". This also applies in less extreme examples, as many/most binary will support more than one triple.

That’s an argument for not encouraging the use of target triples in the directory name, not for encouraging its use and then requiring the same to be restated elsewhere.

Fundamentally, what one would wish to ensure does not happen is a misleading directory name. Either the one-variant-one-triple scenario is relevant often enough to merit a recommendation for such directory structure, or it is not; if it is, then the target triple should not need to be restated in the manifest; if it is not, then it should not be recommended to state it in the directory name in the first place.

Are you referring to the phrasing of the following statement?

Within each artifact directory are variant directories. These names are arbitrary but would normally describe the target triples to which the artifact variants apply. These names are case sensitive.

Well yes, the following two statements seem incompatible, at least in the practical sense if not strictly in the logical sense:

I see your and this is a good feedback for the final proposal language.

SE-0305 review has concluded. While the proposed idea was accepted, the proposal was returned for revision to refine some of the API details, along side SE-0303.

Thank you to everyone for the feedback and contributions to this proposal.