[Review] SE-0158 Package Manager Manifest API Redesign


(Rick Ballard) #1

Hello Swift community,

The review of "SE-0158 Package Manager Manifest API Redesign" begins now and runs through March 13, 2017. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reviews are an important part of the Swift evolution process. All Swift Package Manager reviews should be sent to the swift-evolution and swift-build-dev mailing lists at

  https://lists.swift.org/mailman/listinfo/swift-evolution
  https://lists.swift.org/mailman/listinfo/swift-build-dev

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reply text

Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* What is your evaluation of the proposal?
* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?
* Does this proposal fit well with the feel and direction of Swift?
* If you have used other languages, libraries, or package managers 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?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

- Rick Ballard

Review Manager


(David Hart) #2

Hello Swift community,

The review of "SE-0158 Package Manager Manifest API Redesign" begins now and runs through March 13, 2017. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reviews are an important part of the Swift evolution process. All Swift Package Manager reviews should be sent to the swift-evolution and swift-build-dev mailing lists at

  https://lists.swift.org/mailman/listinfo/swift-evolution
  https://lists.swift.org/mailman/listinfo/swift-build-dev

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

> Reply text

>> Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* What is your evaluation of the proposal?

I’m strongly in favour of the modifications in this proposal. The small API modifications add more consistency and the new version definition APIs are well thought out and approchable for newcomers (compared to cryptic operators).

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Yes.

* Does this proposal fit well with the feel and direction of Swift?

It would feel even better if Swift provided more customisation of enum cases and we could do away with the factory methods!

* If you have used other languages, libraries, or package managers with a similar feature, how do you feel that this proposal compares to those?

I initially wanted a version definition syntax closer to what I’m used in Bundler, Cocoapods, Carthage, but I ended up liking the more wordy choice that was made in this proposal.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read and gave feedback during the discussion period and quick read of the final proposal.

···

On 7 Mar 2017, at 21:53, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

- Rick Ballard

Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Rien) #3

* What is your evaluation of the proposal?

I like the way this provides more harmonisation with Swift coding style.
Future extensibility seems to be well provided for.

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

If it were the only change, no. But anticipating on expected future enhancements, yes.

* Does this proposal fit well with the feel and direction of Swift?

Yes

* If you have used other languages, libraries, or package managers with a similar feature, how do you feel that this proposal compares to those?

N.A. imo

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed the discussion, quid read of proposal.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Balancingrock
Project: http://swiftfire.nl

···

On 07 Mar 2017, at 21:53, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0158 Package Manager Manifest API Redesign" begins now and runs through March 13, 2017. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reviews are an important part of the Swift evolution process. All Swift Package Manager reviews should be sent to the swift-evolution and swift-build-dev mailing lists at

  https://lists.swift.org/mailman/listinfo/swift-evolution
  https://lists.swift.org/mailman/listinfo/swift-build-dev

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

> Reply text

>> Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* What is your evaluation of the proposal?
* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?
* Does this proposal fit well with the feel and direction of Swift?
* If you have used other languages, libraries, or package managers 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?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

- Rick Ballard

Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(David Sweeris) #4

Hello Swift community,

The review of "SE-0158 Package Manager Manifest API Redesign" begins now and runs through March 13, 2017. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reviews are an important part of the Swift evolution process. All Swift Package Manager reviews should be sent to the swift-evolution and swift-build-dev mailing lists at

  https://lists.swift.org/mailman/listinfo/swift-evolution
  https://lists.swift.org/mailman/listinfo/swift-build-dev

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

> Reply text

>> Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* What is your evaluation of the proposal?

+1, although I don’t know why we're supporting this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", from: "1.5.8"),
when, at least as far as I can tell, this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),
does the same thing, and the spelling is, at least to me, clearer as well. Dunno, maybe the “from” version is a term of art that I’m just not familiar with.

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Yep

* Does this proposal fit well with the feel and direction of Swift?

Yep

* If you have used other languages, libraries, or package managers with a similar feature, how do you feel that this proposal compares to those?

N/A

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the discussion thread and the proposal a few times.

- Dave Sweeris

···

On Mar 7, 2017, at 12:53 PM, Rick Ballard via swift-build-dev <swift-build-dev@swift.org> wrote:


(Karl) #5

Hello Swift community,

The review of "SE-0158 Package Manager Manifest API Redesign" begins now and runs through March 13, 2017. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Reviews are an important part of the Swift evolution process. All Swift Package Manager reviews should be sent to the swift-evolution and swift-build-dev mailing lists at

  https://lists.swift.org/mailman/listinfo/swift-evolution
  https://lists.swift.org/mailman/listinfo/swift-build-dev

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

> Reply text

>> Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* What is your evaluation of the proposal?

+0. Meh.

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Not really. The package manifest API has lots of serious deficiencies and this proposal shuffles a couple of names around. Not sure it’s really worth the hassle, to be honest. It’s certainly very far from a “redesign”!

* Does this proposal fit well with the feel and direction of Swift?

Sure, I suppose

* If you have used other languages, libraries, or package managers with a similar feature, how do you feel that this proposal compares to those?

I’ve used other package managers. Again, it’s really neither here nor there.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the prior discussion, and the proposal again.

···

On 7 Mar 2017, at 21:53, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

- Rick Ballard

Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Ankit Aggarwal) #6

+1, although I don’t know why we're supporting this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", from: "1.5.8"),

when, at least as far as I can tell, this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),

does the same thing, and the spelling is, at least to me, clearer as well.
Dunno, maybe the “from” version is a term of art that I’m just not familiar
with.

Hi David,

Thank you for the review.

It is true that `from` and `.uptoNextMajor` are exactly same. We think that
the most widely used requirement will be `.uptoNextMajor`, so we wanted to
provide a shorthand for it.


(Rick Ballard) #7

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Not really. The package manifest API has lots of serious deficiencies and this proposal shuffles a couple of names around. Not sure it’s really worth the hassle, to be honest. It’s certainly very far from a “redesign”!

Hi Karl,

Would you mind expanding on the other deficiencies that you see in this API?

Thanks,

  - Rick

···

On Mar 9, 2017, at 2:30 PM, Karl Wagner <razielim@gmail.com> wrote:


(Karl) #8

I think they’re rather well-known:

- No support for resources; means no test resources (!), framework or application bundles.
- System package API is confusing, doesn’t account for OSX system libraries (“.tbd” files you see in Xcode’s “Link Libraries” panel).
- Source trees are exclude-only with no ability to selectively *include* other trees.
- Every package must have an independent source control repository. Even if it’s just redirecting to a system library (of which you can only sometimes really control the version).
- (Related) It would be nice if we could refer to independent modules within the same repository, who have their own Package.swift detailing their dependencies (like a “sub-package”). This can be helpful when developing modular applications or, as mentioned, when importing some external libraries from the system or another package manager.

- Package manager doesn’t resolve dependencies between files in the same module. You have to resolve it yourself and name the files alphabetically (this may be a bug rather than a missing feature, but I couldn’t find anything in the source which sorted the compiler inputs...)

···

On 9 Mar 2017, at 23:32, Rick Ballard <rballard@apple.com> wrote:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

On Mar 9, 2017, at 2:30 PM, Karl Wagner <razielim@gmail.com <mailto:razielim@gmail.com>> wrote:

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Not really. The package manifest API has lots of serious deficiencies and this proposal shuffles a couple of names around. Not sure it’s really worth the hassle, to be honest. It’s certainly very far from a “redesign”!

Hi Karl,

Would you mind expanding on the other deficiencies that you see in this API?

Thanks,

  - Rick


(David Sweeris) #9

Ok, I just wanted to make sure having both spellings was intentional. I've been trying to think of a shorthand that's clearer, but haven't really come up with anything.

Seems like we're fighting English's grammar... With the non-shorthand spelling, the "normal" way to write that would have the version first, followed by the modifier. Adapted to pseudo-code, it would be `1.5.8.uptoNextMajor()`, which Swift currently can't support. And "from" is typically followed by "to" or "through", which makes the shorthand kinda odd as well.

Sorry, I should've noticed this during the discussion thread. Since this the review thread, though, I want to make it clear that I'm still +1, and think this spelling issue (to the extent that it is one) isn't worth derailing the proposal.

- Dave Sweeris

···

On Mar 8, 2017, at 22:34, Ankit Aggarwal <ankit_aggarwal@apple.com> wrote:

+1, although I don’t know why we're supporting this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", from: "1.5.8"),
when, at least as far as I can tell, this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),
does the same thing, and the spelling is, at least to me, clearer as well. Dunno, maybe the “from” version is a term of art that I’m just not familiar with.

Hi David,

Thank you for the review.

It is true that `from` and `.uptoNextMajor` are exactly same. We think that the most widely used requirement will be `.uptoNextMajor`, so we wanted to provide a shorthand for it.


(Karl) #10

I mean — not to be too negative. The package manager is very cool and I’m looking forward to the day when it works for more projects.

My point was that there are lots of large gaps in the current model, and personally I’d rather we rolled those all together in a “Package.swift v2” rather than making frequent source-breaking changes for cosmetic enhancements. Anything which is additive is fine, but frequent renaming gets annoying.

- Karl

···

On 9 Mar 2017, at 23:48, Karl Wagner <karl.swift@springsup.com> wrote:

On 9 Mar 2017, at 23:32, Rick Ballard <rballard@apple.com <mailto:rballard@apple.com>> wrote:

> Proposal link:
>> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

On Mar 9, 2017, at 2:30 PM, Karl Wagner <razielim@gmail.com <mailto:razielim@gmail.com>> wrote:

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Not really. The package manifest API has lots of serious deficiencies and this proposal shuffles a couple of names around. Not sure it’s really worth the hassle, to be honest. It’s certainly very far from a “redesign”!

Hi Karl,

Would you mind expanding on the other deficiencies that you see in this API?

Thanks,

  - Rick

I think they’re rather well-known:

- No support for resources; means no test resources (!), framework or application bundles.
- System package API is confusing, doesn’t account for OSX system libraries (“.tbd” files you see in Xcode’s “Link Libraries” panel).
- Source trees are exclude-only with no ability to selectively *include* other trees.
- Every package must have an independent source control repository. Even if it’s just redirecting to a system library (of which you can only sometimes really control the version).
- (Related) It would be nice if we could refer to independent modules within the same repository, who have their own Package.swift detailing their dependencies (like a “sub-package”). This can be helpful when developing modular applications or, as mentioned, when importing some external libraries from the system or another package manager.

- Package manager doesn’t resolve dependencies between files in the same module. You have to resolve it yourself and name the files alphabetically (this may be a bug rather than a missing feature, but I couldn’t find anything in the source which sorted the compiler inputs...)


(Xiaodi Wu) #11

Kind of a late reply, but overall I think the proposal looks good with the
exception of some of these `Requirement` vs. shorthand dichotomies. It
looks like the following will be synonyms:

.package(url: "/SwiftyJSON", branch: "develop")
.package(url: "/SwiftyJSON", .branch("develop"))

...as well as...

.package(url: "/SwiftyJSON", revision:
"e74b07278b926c9ec6f9643455ea00d1ce04a021")
.package(url: "/SwiftyJSON",
.revision("e74b07278b926c9ec6f9643455ea00d1ce04a021"))

It seems odd that there are two subtly different but equivalent ways to
spell the same thing like that. But I get that there are possibilities for
chaining made available with the latter notation and that the former was
promised in a fairly new proposal that's been approved. Where it gets more
unsatisfactory, IMO, is that

.package(url: "/SwiftyJSON", .only("1.5.8"))
.package(url: "/SwiftyJSON", .exact("1.5.8"))

also seem to mean the same thing. Here, this is actively confusing, because
every user will inevitably scratch their head trying to parse out the
difference. I also share Dave's concern about

.package(url: "/SwiftyJSON", from: "1.5.8")
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8"))

There is no intuition that would tell a user that the two are the same;
after all, `uptoNextMinor` also starts "from" its argument. I understand
that the latter is wordier, but it is not such a grave burden that the
argument label couldn't be rewritten also as `uptoNextMajor`.

While we're on the topic, the standard library spelling is `upTo` rather
than `upto`, and while it's nitpicky I think it's important to go with the
former given that "upto" is not a single word in standard English. If we
wanted this to read more grammatically, consider also `.upToNextMajor(from:
"1.5.8")`.

···

On Thu, Mar 9, 2017 at 12:34 AM, Ankit Aggarwal via swift-evolution < swift-evolution@swift.org> wrote:

+1, although I don’t know why we're supporting this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", from: "1.5.8"),

when, at least as far as I can tell, this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),

does the same thing, and the spelling is, at least to me, clearer as
well. Dunno, maybe the “from” version is a term of art that I’m just not
familiar with.

Hi David,

Thank you for the review.

It is true that `from` and `.uptoNextMajor` are exactly same. We think
that the most widely used requirement will be `.uptoNextMajor`, so we
wanted to provide a shorthand for it.


(Rick Ballard) #12

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

* Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?

Not really. The package manifest API has lots of serious deficiencies and this proposal shuffles a couple of names around. Not sure it’s really worth the hassle, to be honest. It’s certainly very far from a “redesign”!

Would you mind expanding on the other deficiencies that you see in this API?

I think they’re rather well-known:

- No support for resources; means no test resources (!), framework or application bundles.
- System package API is confusing, doesn’t account for OSX system libraries (“.tbd” files you see in Xcode’s “Link Libraries” panel).
- Source trees are exclude-only with no ability to selectively *include* other trees.
- Every package must have an independent source control repository. Even if it’s just redirecting to a system library (of which you can only sometimes really control the version).
- (Related) It would be nice if we could refer to independent modules within the same repository, who have their own Package.swift detailing their dependencies (like a “sub-package”). This can be helpful when developing modular applications or, as mentioned, when importing some external libraries from the system or another package manager.

- Package manager doesn’t resolve dependencies between files in the same module. You have to resolve it yourself and name the files alphabetically (this may be a bug rather than a missing feature, but I couldn’t find anything in the source which sorted the compiler inputs...)

I mean — not to be too negative. The package manager is very cool and I’m looking forward to the day when it works for more projects.

My point was that there are lots of large gaps in the current model, and personally I’d rather we rolled those all together in a “Package.swift v2” rather than making frequent source-breaking changes for cosmetic enhancements. Anything which is additive is fine, but frequent renaming gets annoying.

The point of this proposal is to do a one-time clean-up and standardization of existing API. We indeed have a lot of new API we also need to add, including things you mention above, but those are intentionally out of scope for this proposal; instead, each of those additions should have its own proposal and discussion. Getting this clean-up out of the way now of course means that those new APIs will have a clear set of API design standards to follow when they're added. We are not expecting to rename these APIs again in the future, so this won't be a "frequent" change.

So setting aside the things that we should add in the future, if you don't think it's a good idea to clean up the existing API, that would be feedback we can discuss here. It sounds like you're saying that the hassle of existing packages needing to revise their API use when they adopt Swift 4 is not worth the benefits that this proposal brings – consistency, clarity, flexibility, extensibility, language guidelines compliance, etc. I respectfully disagree, but if you have some more details on why this is going to cause a problem to do, or why living with the existing API's problems moving forward isn't a problem, I'd be happy to discuss that. More specifically, if there are changes to the *existing* API (which don't add significant new functionality) that this proposal does make which you disagree with, or which it doesn't make that you think should happen, this would be the time to discuss that.

Thanks,

  - Rick

···

On Mar 9, 2017, at 3:02 PM, Karl Wagner <razielim@gmail.com> wrote:

On 9 Mar 2017, at 23:48, Karl Wagner <karl.swift@springsup.com <mailto:karl.swift@springsup.com>> wrote:

On 9 Mar 2017, at 23:32, Rick Ballard <rballard@apple.com <mailto:rballard@apple.com>> wrote:

On Mar 9, 2017, at 2:30 PM, Karl Wagner <razielim@gmail.com <mailto:razielim@gmail.com>> wrote:


(Rick Ballard) #13

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Hi Xiaodi,

Thanks for your review comments! There are indeed a few mistakes here; we're going to revise the proposal appropriately, and assuming that nothing else comes up to cause us to reject this proposal, it will wind up "accepted with revisions" tomorrow.

Specifically:

– .uptoNextMajor() and .uptoNextMinor were miscapitalized; we will correct them to .upToNextMajor() and .upToNextMinor().

– Dave Sweeris' point about this reading wrong in English as .upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us too. As a result, we're going to clarify this by changing it to .upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

– We will get rid of .only() in favor of .exact().

– We will remove the .package(url:,branch:) and .package(url:,revision:) initializers. They duplicate the functionality provided by the .branch() and .revision() cases without adding any additional value.

– We are still planning to keep .package(url:,from:), however, even though it duplicates the functionality of the .upToNextMajor() case. The duplication is somewhat undesirable for the reason you mention, but we think it's worth it in this case for two reasons:

* We want to encourage this form of version specification to be used when possible, to establish good norms around semantic versioning in our ecosystem. Calling it out as a special initializer makes it more prominent.

* Since we hope that this form of version specification will be used most of the time, we still think that it's worthwhile to make it really quick and easy to type. Other package managers also have special syntax (e.g. ~>) to make the common version specifiers quick and easy. We want to stay swift-y and descriptive, but .upToNextMajor() is pretty annoying to type every time.

  - Rick

···

On Mar 11, 2017, at 4:41 PM, Xiaodi Wu via swift-build-dev <swift-build-dev@swift.org> wrote:

On Thu, Mar 9, 2017 at 12:34 AM, Ankit Aggarwal via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

+1, although I don’t know why we're supporting this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", from: "1.5.8"),
when, at least as far as I can tell, this:
// 1.5.8 ..< 2.0.0
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),
does the same thing, and the spelling is, at least to me, clearer as well. Dunno, maybe the “from” version is a term of art that I’m just not familiar with.

Hi David,

Thank you for the review.

It is true that `from` and `.uptoNextMajor` are exactly same. We think that the most widely used requirement will be `.uptoNextMajor`, so we wanted to provide a shorthand for it.

Kind of a late reply, but overall I think the proposal looks good with the exception of some of these `Requirement` vs. shorthand dichotomies. It looks like the following will be synonyms:

.package(url: "/SwiftyJSON", branch: "develop")
.package(url: "/SwiftyJSON", .branch("develop"))

...as well as...

.package(url: "/SwiftyJSON", revision: "e74b07278b926c9ec6f9643455ea00d1ce04a021")
.package(url: "/SwiftyJSON", .revision("e74b07278b926c9ec6f9643455ea00d1ce04a021"))

It seems odd that there are two subtly different but equivalent ways to spell the same thing like that. But I get that there are possibilities for chaining made available with the latter notation and that the former was promised in a fairly new proposal that's been approved. Where it gets more unsatisfactory, IMO, is that

.package(url: "/SwiftyJSON", .only("1.5.8"))
.package(url: "/SwiftyJSON", .exact("1.5.8"))

also seem to mean the same thing. Here, this is actively confusing, because every user will inevitably scratch their head trying to parse out the difference. I also share Dave's concern about

.package(url: "/SwiftyJSON", from: "1.5.8")
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8"))

There is no intuition that would tell a user that the two are the same; after all, `uptoNextMinor` also starts "from" its argument. I understand that the latter is wordier, but it is not such a grave burden that the argument label couldn't be rewritten also as `uptoNextMajor`.

While we're on the topic, the standard library spelling is `upTo` rather than `upto`, and while it's nitpicky I think it's important to go with the former given that "upto" is not a single word in standard English. If we wanted this to read more grammatically, consider also `.upToNextMajor(from: "1.5.8")`.


(Maxim Veksler) #14

Hi guys,

Sorry for the late repose, Hope I haven't missed the review window.

I've read through the proposal for the versioning, and I feel that there is
an over complication there. I would suggest something like this for
specifying the version:

If you want to have latest version of SwiftyJSON (you don't even care about
when major version:

.package(url: "http://github.com/SwiftyJSON")

If you want to have latest of major 1

.package(url: "http://github.com/SwiftyJSON", from: "1")

if you want to have latest of minor 1.5

.package(url: "http://github.com/SwiftyJSON", from: "1.5)

if you want to specify exact 1.5.2 (but the author might choose to be
maintaining 1.5.2-patch1, 1.5.2-patch2 and co..)

.package(url: "http://github.com/SwiftyJSON", from: "1.5.2")

Adopting this structure I think removes the need to have a uptoNextMajor &
uptoNextMinor which behavior I find not intuitive.

-m.

···

On Mon, Mar 13, 2017 at 10:30 PM Rick Ballard via swift-evolution < swift-evolution@swift.org> wrote:

Proposal link:
>
https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Hi Xiaodi,

Thanks for your review comments! There are indeed a few mistakes here;
we're going to revise the proposal appropriately, and assuming that nothing
else comes up to cause us to reject this proposal, it will wind up
"accepted with revisions" tomorrow.

Specifically:

– .uptoNextMajor() and .uptoNextMinor were miscapitalized; we will correct
them to .upToNextMajor() and .upToNextMinor().

– Dave Sweeris' point about this reading wrong in English as
.upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us
too. As a result, we're going to clarify this by changing it to
.upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

– We will get rid of .only() in favor of .exact().

– We will remove the .package(url:,branch:) and .package(url:,revision:)
initializers. They duplicate the functionality provided by the .branch()
and .revision() cases without adding any additional value.

– We are still planning to keep .package(url:,from:), however, even though
it duplicates the functionality of the .upToNextMajor() case. The
duplication is somewhat undesirable for the reason you mention, but we
think it's worth it in this case for two reasons:

* We want to encourage this form of version specification to be used when
possible, to establish good norms around semantic versioning in our
ecosystem. Calling it out as a special initializer makes it more prominent.

* Since we hope that this form of version specification will be used most
of the time, we still think that it's worthwhile to make it really quick
and easy to type. Other package managers also have special syntax (e.g. ~>)
to make the common version specifiers quick and easy. We want to stay
swift-y and descriptive, but .upToNextMajor() is pretty annoying to type
every time.

- Rick

On Mar 11, 2017, at 4:41 PM, Xiaodi Wu via swift-build-dev < > swift-build-dev@swift.org> wrote:

On Thu, Mar 9, 2017 at 12:34 AM, Ankit Aggarwal via swift-evolution < > swift-evolution@swift.org> wrote:

+1, although I don’t know why we're supporting this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", from: "1.5.8"),

when, at least as far as I can tell, this:

// 1.5.8 ..< 2.0.0.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),

does the same thing, and the spelling is, at least to me, clearer as well.
Dunno, maybe the “from” version is a term of art that I’m just not familiar
with.

Hi David,

Thank you for the review.

It is true that `from` and `.uptoNextMajor` are exactly same. We think
that the most widely used requirement will be `.uptoNextMajor`, so we
wanted to provide a shorthand for it.

Kind of a late reply, but overall I think the proposal looks good with the
exception of some of these `Requirement` vs. shorthand dichotomies. It
looks like the following will be synonyms:

.package(url: "/SwiftyJSON", branch: "develop")
.package(url: "/SwiftyJSON", .branch("develop"))

...as well as...

.package(url: "/SwiftyJSON", revision:
"e74b07278b926c9ec6f9643455ea00d1ce04a021")
.package(url: "/SwiftyJSON",
.revision("e74b07278b926c9ec6f9643455ea00d1ce04a021"))

It seems odd that there are two subtly different but equivalent ways to
spell the same thing like that. But I get that there are possibilities for
chaining made available with the latter notation and that the former was
promised in a fairly new proposal that's been approved. Where it gets more
unsatisfactory, IMO, is that

.package(url: "/SwiftyJSON", .only("1.5.8"))
.package(url: "/SwiftyJSON", .exact("1.5.8"))

also seem to mean the same thing. Here, this is actively confusing,
because every user will inevitably scratch their head trying to parse out
the difference. I also share Dave's concern about

.package(url: "/SwiftyJSON", from: "1.5.8")
.package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8"))

There is no intuition that would tell a user that the two are the same;
after all, `uptoNextMinor` also starts "from" its argument. I understand
that the latter is wordier, but it is not such a grave burden that the
argument label couldn't be rewritten also as `uptoNextMajor`.

While we're on the topic, the standard library spelling is `upTo` rather
than `upto`, and while it's nitpicky I think it's important to go with the
former given that "upto" is not a single word in standard English. If we
wanted this to read more grammatically, consider also `.upToNextMajor(from:
"1.5.8")`.

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Ankit Aggarwal) #15

Hi,

– Dave Sweeris' point about this reading wrong in English as .upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us too. As a result, we're going to clarify this by changing it to .upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

The "after" in .upToNextMajor(after: "x.y.z") sounds like we're going to pick the version after `x.y.z`, for e.g. `x.y.(z+1)`, and go upto the next major version. I think we should use `from` instead of `after`, which is more clear IMO. It also feels like `after` and `from` (in shorthand) do different things, and if we use `form`, it will also be reasonable to assume that the `.package(url:from:)` is a shorthand form.

- Ankit


(Rick Ballard) #16

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Hi Maxim,

Thanks for taking the time to read the proposal. My replies are inline:

Hi guys,

Sorry for the late repose, Hope I haven't missed the review window.

I've read through the proposal for the versioning, and I feel that there is an over complication there. I would suggest something like this for specifying the version:

If you want to have latest version of SwiftyJSON (you don't even care about when major version:
.package(url: "http://github.com/SwiftyJSON")

This is not a safe thing to do and we don't intend to support it at all. New major semantic versions of a package aren't necessarily compatible without requiring clients of that package to adapt to them. If you fail to specify the major version that you're compatible with, your package might work today but fail to build tomorrow (when a new major version of SwiftyJSON is released). And, if your package has clients, they're now broken too, etc.

If you want to have latest of major 1
.package(url: "http://github.com/SwiftyJSON", from: "1")

if you want to have latest of minor 1.5
.package(url: "http://github.com/SwiftyJSON", from: "1.5)

if you want to specify exact 1.5.2 (but the author might choose to be maintaining 1.5.2-patch1, 1.5.2-patch2 and co..)
.package(url: "http://github.com/SwiftyJSON", from: "1.5.2")

Adopting this structure I think removes the need to have a uptoNextMajor & uptoNextMinor which behavior I find not intuitive.

What you're suggesting here is not equivalent to upToNextMajor and upToNextMinor, because it doesn't allow you to fully specify a minimum version (including the minimum patch version required) while still allowing a flexible upper bound for the major or minor version.

Thanks,

  - Rick

···

On Mar 13, 2017, at 3:41 PM, Maxim Veksler <maxim@vekslers.org> wrote:


(David Hart) #17

Yep, after does sound confusing. I prefer from.

···

On 14 Mar 2017, at 08:22, Ankit Aggarwal via swift-evolution <swift-evolution@swift.org> wrote:

Hi,

– Dave Sweeris' point about this reading wrong in English as .upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us too. As a result, we're going to clarify this by changing it to .upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

The "after" in .upToNextMajor(after: "x.y.z") sounds like we're going to pick the version after `x.y.z`, for e.g. `x.y.(z+1)`, and go upto the next major version. I think we should use `from` instead of `after`, which is more clear IMO. It also feels like `after` and `from` (in shorthand) do different things, and if we use `form`, it will also be reasonable to assume that the `.package(url:from:)` is a shorthand form.

- Ankit
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Rick Ballard) #18

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Thanks for this feedback. We'll revise it to "from:".

Since we're still making minor revisions to this proposal, I'm going to extend the review period by another day to make sure we don't have any other important feedback pending.

  - Rick

···

On Mar 14, 2017, at 12:28 AM, David Hart <david@hartbit.com> wrote:

Yep, after does sound confusing. I prefer from.

On 14 Mar 2017, at 08:22, Ankit Aggarwal via swift-evolution <swift-evolution@swift.org> wrote:

Hi,

– Dave Sweeris' point about this reading wrong in English as .upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us too. As a result, we're going to clarify this by changing it to .upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

The "after" in .upToNextMajor(after: "x.y.z") sounds like we're going to pick the version after `x.y.z`, for e.g. `x.y.(z+1)`, and go upto the next major version. I think we should use `from` instead of `after`, which is more clear IMO. It also feels like `after` and `from` (in shorthand) do different things, and if we use `form`, it will also be reasonable to assume that the `.package(url:from:)` is a shorthand form.

- Ankit
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Andrey Fidrya) #19

Hi Ankit,

I like the proposal very much. One question about SystemPackageProvider:

Sometimes it's neccessary to pass options to brew or apt-get, for example:
brew install libxml2 --with-python

Should they be added to package name or have a separate field? Adding the field in the future will break compatibility.

let package = Package(
    providers: [
        .brew(["libxml2 --with-python"]),
    ]
)

Another thing which comes to mind is a human-readable description field which could be helpful in some situations. I.e. "don't forget to generate a private key by running XXX". I think of something like this:

let package = Package(
    providers: [
        .brew([
          "openssl",
          SystemPackage(name: "libxml2", options: "--with-python", description: "........")
        ]),
    ]
)

Regards,
Andrey

···

On 14 Mar 2017, at 19:29, Rick Ballard via swift-build-dev <swift-build-dev@swift.org> wrote:

Proposal link:
> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Thanks for this feedback. We'll revise it to "from:".

Since we're still making minor revisions to this proposal, I'm going to extend the review period by another day to make sure we don't have any other important feedback pending.

  - Rick

On Mar 14, 2017, at 12:28 AM, David Hart <david@hartbit.com <mailto:david@hartbit.com>> wrote:

Yep, after does sound confusing. I prefer from.

On 14 Mar 2017, at 08:22, Ankit Aggarwal via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hi,

– Dave Sweeris' point about this reading wrong in English as .upToNextMajor("x.y.z") (vs. of "x.y.z".upToNextMajor) makes sense to us too. As a result, we're going to clarify this by changing it to .upToNextMajor(after:"x.y.z") and .upToNextMinor(after:"x.y.z").

The "after" in .upToNextMajor(after: "x.y.z") sounds like we're going to pick the version after `x.y.z`, for e.g. `x.y.(z+1)`, and go upto the next major version. I think we should use `from` instead of `after`, which is more clear IMO. It also feels like `after` and `from` (in shorthand) do different things, and if we use `form`, it will also be reasonable to assume that the `.package(url:from:)` is a shorthand form.

- Ankit
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-build-dev mailing list
swift-build-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-build-dev


(Rick Ballard) #20

Proposal link:
> https://github.com/apple/swift-evolution/blob/master/proposals/0158-package-manager-manifest-api-redesign.md

Hi Andrey,

You suggestions here sound like they'd only require the addition of new API, not changes to existing API. We have a compatibility mechanism designed to support easily adding new API without causing breakage in the ecosystem; you can read about it at https://github.com/apple/swift-evolution/blob/master/proposals/0152-package-manager-tools-version.md.

The Package Manager Manifest API Redesign proposal is intentionally scoped to revising existing API and doesn't cover additive features. Please feel free to spin up a new discussion thread about this and/or create a proposal for these additions. I don't think we should hold up the API Redesign for this, however.

Thank for taking the time to contribute,

  - Rick

···

On Mar 14, 2017, at 4:50 PM, Andrey Fidrya <af@zabiyaka.com> wrote:

Hi Ankit,

I like the proposal very much. One question about SystemPackageProvider:

Sometimes it's neccessary to pass options to brew or apt-get, for example:
brew install libxml2 --with-python

Should they be added to package name or have a separate field? Adding the field in the future will break compatibility.

let package = Package(
    providers: [
        .brew(["libxml2 --with-python"]),
    ]
)

Another thing which comes to mind is a human-readable description field which could be helpful in some situations. I.e. "don't forget to generate a private key by running XXX". I think of something like this:

let package = Package(
    providers: [
        .brew([
          "openssl",
          SystemPackage(name: "libxml2", options: "--with-python", description: "........")
        ]),
    ]
)

Regards,
Andrey