[swift-evolution] [Review] SE-0145: Package Manager Version Pinning (Revised)


(Anders Bertelrud) #1

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

Anders Bertelrud
Review Manager


(Jay) #2

What is your evaluation of the proposal?

It seems like a sensible step on the way to more advanced workflow, stats,
testing, checking upgrades, and reporting features - without closing any
doors.

If you have used other languages or libraries with a similar feature, how

do you feel that this proposal compares to those?

This is a better and more considered approach.

How much effort did you put into your review? A glance, a quick reading,

or an in-depth study?

A quick reading, plus previous consideration of this issue when it has
caused both team and CI problems in other package managers.

···

On Sun, 20 Nov 2016 at 05:48 Anders Bertelrud via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again
after revisions, starting now and running through November 28th. The
proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the
review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to 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
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?

More information about the Swift evolution process is available at

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

Thank you,

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


(Russ Bishop) #3

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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?

Looks good to me. I’ve been following the discussion and I think we landed at a reasonable place.

Russ

···

On Nov 19, 2016, at 9:48 PM, Anders Bertelrud via swift-build-dev <swift-build-dev@swift.org> wrote:

  * 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?

More information about the Swift evolution process is available at

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

Thank you,

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


#4

  * What is your evaluation of the proposal?

Good. The revision is a good improvement over the original proposal. Build reproducability is great improvement to the ecosystem.

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

Yes.

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

Yes. It does the reasonable thing to do by default: auto-pinning.

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

Compared to pip 's "freeze" and "install -r ..." this feels like an enhancement.

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

Followed the discussion of the previous proposal, followed by a shallow study.

-Bouke

···

On 2016-11-20 05:48:48 +0000, Anders Bertelrud via swift-build-dev said:


(Martin Waitz) #5

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

What is your evaluation of the proposal?

+1
I think this is an important change for the Swift package ecosystem.
Having reproducible builds by default is a big step forward.
Storing the version of all dependencies in version control and having an easy way to update them helps to reduce maintenance overhead of packages.

I have two comments / requests for improvement:
* Having two completely separate modes (—enable-autopin vs. —disable-autopin) with a different semantic (e.g. for `swift update`) unnecessarily complicates the design. I understand that there are different use cases where packages have to be handled differently, however I think we should invest some more time on understanding these use cases (see below).
* For me the term ‚pin‘ (just like ‚lock‘) implies that the version of pinned dependencies should be fixed and should therefore not be changed/updated. (But I’m not a native speaker, so you may have other associations here.) We should make clear that we *manage* dependencies and that dependencies should be easy to update even while they can be reproduced exactly.

Let’s look at the features of the proposal and how they can be used:
* Defines two groups of dependencies (pinned vs. unpinned) where one group can be updated independently;
* unpinned dependencies are automatically updated on clone, not just on update;
* updates of unpinned dependencies are not considered a change in the package’s repository.
This can be used to easily update internal dependencies while keeping the same version of external dependencies.
Or to keep some difficult-to-update dependencies while constantly updating all the other dependencies.
Or to not create git changes for some selected dependencies.
You name it.

Basically it boils down to better control when and how dependencies are updated.
However, I’m not yet convinced that the proposed pinning system is the best way to do that.
Why only support two groups? Why these two modes of operation? Why not support a specific set of dependencies which are unmanaged while still tracking all new dependencies?

When we want to create groups of dependencies and handle them differently, then we should do that explicitly, by introducing groups which can be defined by the package maintainer.

E.g.:

  swift package dependency <dep-name> —group <group-name>

We could store each group in a file of its own (e.g. `Package-<group-name>.versions` or whatever) and the maintainer could put each one under version control or not as he likes.

A whole group could be updated in one go:

  swift package update —group internal

This could simulate the pinning as described in the proposal:

  swift package dependency A —group unpinned
  swift package dependency B —group pinned
  echo Package-unpinned.versions > .gitignore
  git add .gitignore Package-pinned.versions

Each group can be updated individually and the package maintainer can put this group under version control or not.
One group would have to be special to automatically include all new dependencies.
Other than that we would not need any special handling ala —repin/—enable-autopin. Groups could be defined to make updating related packages easier.
When needed, we could even set some groups to update automatically on build (not only on clone/update).
This could be useful for closely related packages. Why should clone be special anyways?

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

Yes.
Not managing the package dependencies would unnecessarily complicate package maintenance in the long term.

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

Yes.
Reproducible builds fit well into the safe-by-default approach of Swift.

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

I always hated `npm` and all those node packages which had overly restrictive dependency requirements.
I guess these were used in order to have more control about which version to use, but this resulted
in both packages which I could not build out of the box (because some dependencies changed),
and packages I could not update because of conflicting requirements.
Using loose version requirements + semver in `Package.swift` files together with exact versions
of all dependencies stored in the top-level package solves everything: you get reproducible builds
(based on `Package*.versions`) together with easy updates (based on all packages’ `Package.swift`).

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

Read both versions of the proposal, took part in the discussion.

— Martin Waitz


(Alex Blewitt) #6

  * What is your evaluation of the proposal?

A way of building against a specific set of dependencies, rather than a variable set (or master), is needed in Swift. The current build process seems opinionated (but not mandated) to check these explicit dependencies into version control. If dealing with builds-from-source (as currently stands in Swift) then building from a moving target makes sense, and is likely the only sane default, but when binary dependencies are generated the moving target makes less sense.

At the moment, the proposal suggests having a secondary 'pins' file, which exists to allow explicit dependencies to be checked in to version control. This can be done at the moment, using a Version(1,2,3) or range Version(1,2,3)...Version(1.2.3) in the constraints.

When dealing with unreleased software, there is no explicit version - and thus the resolution can't apply. In effect, a version control hash takes place of an explicit version for the purposes of the dependency resolution. Being able to override the version resolved with a hash, without making changes to a checked-in file, seems like a good idea.

What the proposal doesn't bring forward clearly is the versioning of transitive dependencies, and whether or not those dependencies violate the requirement constraints. For example, if A depends on B, and B depends on C version 1.2.3+, can or should A pin C version 0.9.9? What if A depends on B and D, both of which depend on different versions of C that may be semantically equivalent (e.g. C version 1.2.3 and C version 1.4.5)? This will come up more often than the 'fail to build' approach outlined in the proposal.

It would be useful if the tool to update a version dependency could allow for dependencies which used to be present but which have been since removed could be cleaned up, optionally with a warning.

For pin files that are version controlled, having each pinned dependency on a separate line (at least) will permit sensible version control introspection. Any start-of-list parenthesis or commas should be designed in such a way that removing the first pinned dependency or last pinned dependency results in an SCM diff which just has a single line change (so, for example, trailing commas would be useful in any list structures).

Another useful mode would be to re-write the version ranges to increase the lower limit of the dependency in the Package.swift file. If code has been built and tested against a library version 1.2.3, then it may not be appropriate for that build to be built against anything lower. Increasing the lower bound to match (or having a command to do the same) would be a way of keeping track of the moving dependencies for the package itself.

The proposal also doesn't make clear to me what the dependencies are named for. The expectation is that developers will name the target for the repository name that is referring to. Should that short name be used as a dependency, or the URL? What if the target's URL has been replaced with another (say, because you're working on a local fork of an existing project's component?)

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

Yes. At the moment, it is not possible to use swift build to rebuild an old version of a product, without manually cloning many repositories and adjusting their hashes to match. Having the tool handle these dependencies automatically will manage this for the developer, and allow a step back in time to find out what the dependencies were.

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

Yes, this proposal is a natural extension to the way in which swift build works and will ensure repeatable builds.

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

Many languages who have such a dependency management mechanism do so by putting the versions next to the dependency in the format, and the dependencies are updated in the top level when they are needed. For example:

Maven: https://maven.apache.org/pom.html#Dependencies
Gradle: https://docs.gradle.org/current/userguide/artifact_dependencies_tutorial.html#sec:declaring_your_dependencies
Ivy: https://ant.apache.org/ivy/history/latest-milestone/ivyfile/dependency.html

Although these tools allow specifying ranges of versions, this has been effectively deprecated over time as they don't provide for repeatable builds. Instead, the exact version (in the form of major/minor/micro) is encoded in the dependency itself, which allows the build to be repeated subsequently. (Dependencies in this world are identified with a fixed/immutable version number.)

One advantage of having the versions in the main dependency file is that it allows a view over what the history of those dependencies are simply by using the version control log. There are also other tools that can update the versions to the latest (or latest major, latest minor etc.) which update the dependencies in place.

http://www.mojohaus.org/versions-maven-plugin/

What these tools don't have is the ability to record a non-released version of a dependency; for example, if you know a but is fixed in a commit abc123 there isn't a way of depending on that.

Finally, one maven encodes the dependency information in the generated artefact. This allows the JAR to be unzipped and the dependencies investigated. For example, the dependency for the slf4j artefact (in the POM) is stored within the JAR in the META-INF/maven/org.slf4j/slf4j-log4j12/pom.xml of the generated code, along with a pinned stamp of the dependency used in the pom.properties file:

http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.pom <http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.pom>
http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.jar <http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.jar>

The pinning here takes the form of a major/minor/micro version, rather than a commit hash, but is similar in effect. (Unlike the constraints in Swift's package manager, a version in Maven dependencies is an exact version, not a range).

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

I spent some time going through the proposal and am familiar with other dependency management systems which have gone through a different evolution.

Alex

···

On 20 Nov 2016, at 05:48, Anders Bertelrud via swift-evolution <swift-evolution@swift.org> wrote:


#7

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again
after revisions, starting now and running through November 28th. The
proposal is available here:

Overall looks good.

The exact file format is unspecified/implementation defined, however, in

practice it will be a JSON data file.

Please make it VCS friendly format. If JSON, pretty-print please.

There will be change in the behaviors of swift build and swift package

update in presence of the pins file, as noted in the proposal

I couldn't find any mention about "swift build" in Proposed Solution or
Detailed Design section.
In case the local workspace has outdated dependencies,
will it automatically fetch pinned versions?
or error-exit indicating I must run "swift package update" first?

···

2016-11-20 14:48 GMT+09:00 Anders Bertelrud via swift-evolution < swift-evolution@swift.org>:


(Adrian Kashivskyy) #8

What is your evaluation of the proposal?

+1. I think

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

Yes, version pinning is a must-have feature for collaboration in larger scale.

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

Yes, and I admit I like `pinning` (as opposed to `locking` terminology.

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

Yes - Bundler, Carthage, CocoaPods, Composer. This proposal is very similar to all of above.

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

Read the proposal and been following the discussions.

–––

I have a little feedback, though:

The exact file format is unspecified/implementation defined, however, in practice it will be a JSON data file.

Please make it VCS friendly format. If JSON, pretty-print please.

I agree that pins file should be VCS friendly so that diffs are immediately visible during code review and blame.

– Adrian

···

Wiadomość napisana przez Anders Bertelrud via swift-evolution <swift-evolution@swift.org> w dniu 20.11.2016, o godz. 06:48:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

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


(David Sweeris) #9

Just to pick a couple nits...

I'm not sure "pinning" is the best word, but I don't have any thoughts on what would be better.

Does the pinning info have to be in an optional file? Would it be better to have it be an optional part of the manifest? There'd be one less file to carry around.

I didn't follow the discussion thread very closely... If either of these "issues" were debated there, I do *not* want to start rehashing them. Furthermore, IMHO both are pretty minor (especially the first one... it's completely subjective).

+1, either way

- Dave Sweeris

···

On Nov 19, 2016, at 23:48, Anders Bertelrud <anders@apple.com> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

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


(Paul Cantrell) #10

Just a quick mini-review here; sorry, time pressure.

This version of the proposal seems acceptable to me, though I have a nagging feel that it’s more complex than it needs to be.

In particular, the two modes (autopin enabled / disabled) plus the --repin option seem to me to have a high confusion:benefit ratio. I can imagine a much simpler model in which:

general behavior is always like --enable-autopin,
update always acts as if --repin is specified, and
.gitignore is the sole difference in how pinning behaves for different projects.

What is the benefit of the proposal’s more complex model? AFAICT, the one thing this simpler model wouldn’t allow is for a team to share pinned versions of some individual ill-behaved dependencies without pinning all of them. If that’s the only missing behavior, my gut tells me there’s a way to do that with less cognitive burden on SwiftPM users. (Are there other benefits to the proposal’s model that I’m missing?)

Those concerns stated, I’d be fine with the proposal as stated. It would accommodate all the different ways I can imagine using SwiftPM on my own projects. It might be confusing to newcomers, but it won’t be a roadblock for getting work done.

Cheers,

Paul

···

On Nov 19, 2016, at 11:48 PM, Anders Bertelrud via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

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


(Alexis) #11

Haven’t had a chance to catch up on the latest discussion, but I just saw that the Yarn developers posted an excellent piece on lockfiles this week:

https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all

They argue lockfiles should be commited by libraries (but still ignored by applications). The essential point is that this makes it easier for developers of the library to maintain a coherent build of the library when dependencies ship a bug. The focus is particularly on new developers, who would otherwise lack a lockfile.

···

On Nov 20, 2016, at 12:48 AM, Anders Bertelrud <anders@apple.com> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

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


(Martin Waitz) #12

Hello,

At the moment, the proposal suggests having a secondary 'pins' file,
which exists to allow explicit dependencies to be checked in to
version control. This can be done at the moment, using a
Version(1,2,3) or range Version(1,2,3)...Version(1.2.3) in the
constraints.

What the proposal doesn't bring forward clearly is the versioning of
transitive dependencies, and whether or not those dependencies violate
the requirement constraints. For example, if A depends on B, and B
depends on C version 1.2.3+, can or should A pin C version 0.9.9? What
if A depends on B and D, both of which depend on different versions of
C that may be semantically equivalent (e.g. C version 1.2.3 and C
version 1.4.5)? This will come up more often than the 'fail to build'
approach outlined in the proposal.

be careful and do not mix up `Package.swift` and `Package.pins`.

Both are used for different things:
  * Package.swift is used to specify compatible versions of dependencies.
    These always apply.
    In your example above, a `swift update` would need to find a set of versions
    for B, C, and D where all these version requirements are met.
    Therefor, Package.swift ranges should be as broad as possible,
    to make it possible to find compatible versions.
  * Package.pins stores the current set of all (transitive) dependencies.
    It is stored in the toplevel project (A) and would pin one set of compatible
    (and hopefully tested) versions of B, C and D.
    This way you always get a clean state when you start working on A.

Then, when you run `swift package update` in A, it will try to find a newer set of
B, C, and D, by looking at all of the `Package.swift` files.
This new set will then be written to `Package.pins` so that everybody can benefit from the update.
It's important to not hard-code specific versions into `Package.swift` as this complicates the version resolution during update and could lead to the conflicting version requirements you describe.

···

Am 2016-11-24 13:54, schrieb Alex Blewitt via swift-evolution:

--
Martin


(Daniel Dunbar) #13

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

What is your evaluation of the proposal?

+1
I think this is an important change for the Swift package ecosystem.
Having reproducible builds by default is a big step forward.
Storing the version of all dependencies in version control and having an easy way to update them helps to reduce maintenance overhead of packages.

I have two comments / requests for improvement:
* Having two completely separate modes (—enable-autopin vs. —disable-autopin) with a different semantic (e.g. for `swift update`) unnecessarily complicates the design. I understand that there are different use cases where packages have to be handled differently, however I think we should invest some more time on understanding these use cases (see below).

I agree, I am not very happy with this part.

However, I expect that in practice what will happen is almost all projects will simply fall into one of the two camps, they either use auto pinning or not. For developers working on that package, their workflow will then end up being consistent. For developers who end up working on a bunch of projects with inconsistent choices, then I agree it might be somewhat confusing, and I hope the diagnostics will end up addressing this.

I still think that this is a better compromise than a more complex (but consistent) mechanism like the one you propose. I think we should gain some experience with the proposed mechanism and then see if we feel it needs revision in practice.

- Daniel

···

On Nov 23, 2016, at 3:03 PM, Martin Waitz via swift-evolution <swift-evolution@swift.org> wrote:

* For me the term ‚pin‘ (just like ‚lock‘) implies that the version of pinned dependencies should be fixed and should therefore not be changed/updated. (But I’m not a native speaker, so you may have other associations here.) We should make clear that we *manage* dependencies and that dependencies should be easy to update even while they can be reproduced exactly.

Let’s look at the features of the proposal and how they can be used:
* Defines two groups of dependencies (pinned vs. unpinned) where one group can be updated independently;
* unpinned dependencies are automatically updated on clone, not just on update;
* updates of unpinned dependencies are not considered a change in the package’s repository.
This can be used to easily update internal dependencies while keeping the same version of external dependencies.
Or to keep some difficult-to-update dependencies while constantly updating all the other dependencies.
Or to not create git changes for some selected dependencies.
You name it.

Basically it boils down to better control when and how dependencies are updated.
However, I’m not yet convinced that the proposed pinning system is the best way to do that.
Why only support two groups? Why these two modes of operation? Why not support a specific set of dependencies which are unmanaged while still tracking all new dependencies?

When we want to create groups of dependencies and handle them differently, then we should do that explicitly, by introducing groups which can be defined by the package maintainer.

E.g.:

  swift package dependency <dep-name> —group <group-name>

We could store each group in a file of its own (e.g. `Package-<group-name>.versions` or whatever) and the maintainer could put each one under version control or not as he likes.

A whole group could be updated in one go:

  swift package update —group internal

This could simulate the pinning as described in the proposal:

  swift package dependency A —group unpinned
  swift package dependency B —group pinned
  echo Package-unpinned.versions > .gitignore
  git add .gitignore Package-pinned.versions

Each group can be updated individually and the package maintainer can put this group under version control or not.
One group would have to be special to automatically include all new dependencies.
Other than that we would not need any special handling ala —repin/—enable-autopin. Groups could be defined to make updating related packages easier.
When needed, we could even set some groups to update automatically on build (not only on clone/update).
This could be useful for closely related packages. Why should clone be special anyways?

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

Yes.
Not managing the package dependencies would unnecessarily complicate package maintenance in the long term.

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

Yes.
Reproducible builds fit well into the safe-by-default approach of Swift.

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

I always hated `npm` and all those node packages which had overly restrictive dependency requirements.
I guess these were used in order to have more control about which version to use, but this resulted
in both packages which I could not build out of the box (because some dependencies changed),
and packages I could not update because of conflicting requirements.
Using loose version requirements + semver in `Package.swift` files together with exact versions
of all dependencies stored in the top-level package solves everything: you get reproducible builds
(based on `Package*.versions`) together with easy updates (based on all packages’ `Package.swift`).

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

Read both versions of the proposal, took part in the discussion.

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


(Daniel Dunbar) #14

Hi Alex,

  * What is your evaluation of the proposal?

A way of building against a specific set of dependencies, rather than a variable set (or master), is needed in Swift. The current build process seems opinionated (but not mandated) to check these explicit dependencies into version control. If dealing with builds-from-source (as currently stands in Swift) then building from a moving target makes sense, and is likely the only sane default, but when binary dependencies are generated the moving target makes less sense.

At the moment, the proposal suggests having a secondary 'pins' file, which exists to allow explicit dependencies to be checked in to version control. This can be done at the moment, using a Version(1,2,3) or range Version(1,2,3)...Version(1.2.3) in the constraints.

When dealing with unreleased software, there is no explicit version - and thus the resolution can't apply. In effect, a version control hash takes place of an explicit version for the purposes of the dependency resolution. Being able to override the version resolved with a hash, without making changes to a checked-in file, seems like a good idea.

What the proposal doesn't bring forward clearly is the versioning of transitive dependencies, and whether or not those dependencies violate the requirement constraints. For example, if A depends on B, and B depends on C version 1.2.3+, can or should A pin C version 0.9.9? What if A depends on B and D, both of which depend on different versions of C that may be semantically equivalent (e.g. C version 1.2.3 and C version 1.4.5)? This will come up more often than the 'fail to build' approach outlined in the proposal.

It would be useful if the tool to update a version dependency could allow for dependencies which used to be present but which have been since removed could be cleaned up, optionally with a warning.

For pin files that are version controlled, having each pinned dependency on a separate line (at least) will permit sensible version control introspection. Any start-of-list parenthesis or commas should be designed in such a way that removing the first pinned dependency or last pinned dependency results in an SCM diff which just has a single line change (so, for example, trailing commas would be useful in any list structures).

Another useful mode would be to re-write the version ranges to increase the lower limit of the dependency in the Package.swift file. If code has been built and tested against a library version 1.2.3, then it may not be appropriate for that build to be built against anything lower. Increasing the lower bound to match (or having a command to do the same) would be a way of keeping track of the moving dependencies for the package itself.

The proposal also doesn't make clear to me what the dependencies are named for. The expectation is that developers will name the target for the repository name that is referring to. Should that short name be used as a dependency, or the URL? What if the target's URL has been replaced with another (say, because you're working on a local fork of an existing project's component?)

I'm having difficulty understanding here what you are concretely proposing. But here are my answers to the questions I think you are asking (or changes you are requesting):

1. The reason we want the manifest to have the range, and the pin to have the pin, is that we want the manifest to specify the "ideal range" w.r.t. semantic versioning. So it is incorrect to over-specify the range in the manifest by pinning, that breaks the use of semantic versioning in other packages. Hence, we store the pin data in a separate location and have a separate workflow for managing that data (whether that data is checked in is left to the project).

2. I agree we will need mechanisms to pin to a SHA.

If this doesn't answer your questions/concerns, can you restate them individually if this was a request for changing the proposal (something unlikely to happen at this point, but we can always amend the behavior subsequent to implementation)?

- Daniel

···

On Nov 24, 2016, at 4:54 AM, Alex Blewitt via swift-build-dev <swift-build-dev@swift.org> wrote:

On 20 Nov 2016, at 05:48, Anders Bertelrud via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

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

Yes. At the moment, it is not possible to use swift build to rebuild an old version of a product, without manually cloning many repositories and adjusting their hashes to match. Having the tool handle these dependencies automatically will manage this for the developer, and allow a step back in time to find out what the dependencies were.

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

Yes, this proposal is a natural extension to the way in which swift build works and will ensure repeatable builds.

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

Many languages who have such a dependency management mechanism do so by putting the versions next to the dependency in the format, and the dependencies are updated in the top level when they are needed. For example:

Maven: https://maven.apache.org/pom.html#Dependencies
Gradle: https://docs.gradle.org/current/userguide/artifact_dependencies_tutorial.html#sec:declaring_your_dependencies
Ivy: https://ant.apache.org/ivy/history/latest-milestone/ivyfile/dependency.html

Although these tools allow specifying ranges of versions, this has been effectively deprecated over time as they don't provide for repeatable builds. Instead, the exact version (in the form of major/minor/micro) is encoded in the dependency itself, which allows the build to be repeated subsequently. (Dependencies in this world are identified with a fixed/immutable version number.)

One advantage of having the versions in the main dependency file is that it allows a view over what the history of those dependencies are simply by using the version control log. There are also other tools that can update the versions to the latest (or latest major, latest minor etc.) which update the dependencies in place.

http://www.mojohaus.org/versions-maven-plugin/

What these tools don't have is the ability to record a non-released version of a dependency; for example, if you know a but is fixed in a commit abc123 there isn't a way of depending on that.

Finally, one maven encodes the dependency information in the generated artefact. This allows the JAR to be unzipped and the dependencies investigated. For example, the dependency for the slf4j artefact (in the POM) is stored within the JAR in the META-INF/maven/org.slf4j/slf4j-log4j12/pom.xml of the generated code, along with a pinned stamp of the dependency used in the pom.properties file:

http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.pom <http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.pom>
http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.jar <http://search.maven.org/remotecontent?filepath=org/slf4j/slf4j-log4j12/1.7.21/slf4j-log4j12-1.7.21.jar>

The pinning here takes the form of a major/minor/micro version, rather than a commit hash, but is similar in effect. (Unlike the constraints in Swift's package manager, a version in Maven dependencies is an exact version, not a range).

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

I spent some time going through the proposal and am familiar with other dependency management systems which have gone through a different evolution.

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


(Daniel Dunbar) #15

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

Anders Bertelrud
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Just to pick a couple nits...

I'm not sure "pinning" is the best word, but I don't have any thoughts on what would be better.

Does the pinning info have to be in an optional file? Would it be better to have it be an optional part of the manifest? There'd be one less file to carry around.

Even if we wanted to do this, we do not yet have the technical ability to mechanically edit the manifest file, so it needs to be separate in order to be implementable in the short term.

- Daniel

···

On Nov 28, 2016, at 9:44 AM, David Sweeris via swift-build-dev <swift-build-dev@swift.org> wrote:
On Nov 19, 2016, at 23:48, Anders Bertelrud <anders@apple.com <mailto:anders@apple.com>> wrote:

I didn't follow the discussion thread very closely... If either of these "issues" were debated there, I do *not* want to start rehashing them. Furthermore, IMHO both are pretty minor (especially the first one... it's completely subjective).

+1, either way

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


(Daniel Dunbar) #16

Just a quick mini-review here; sorry, time pressure.

This version of the proposal seems acceptable to me, though I have a nagging feel that it’s more complex than it needs to be.

I agree with this feeling, but am not sure how much it will matter in practice.

In particular, the two modes (autopin enabled / disabled) plus the --repin option seem to me to have a high confusion:benefit ratio. I can imagine a much simpler model in which:

general behavior is always like --enable-autopin,
update always acts as if --repin is specified, and
.gitignore is the sole difference in how pinning behaves for different projects.

What is the benefit of the proposal’s more complex model? AFAICT, the one thing this simpler model wouldn’t allow is for a team to share pinned versions of some individual ill-behaved dependencies without pinning all of them. If that’s the only missing behavior, my gut tells me there’s a way to do that with less cognitive burden on SwiftPM users. (Are there other benefits to the proposal’s model that I’m missing?)

This is a benefit I *really* want to retain, because we continue to feel it is important for people not to overly constraint the dependency graph, and I still feel this is the right model for users to follow (pin problematic dependencies, not everything). I really want to retain the technical ability to do it, even if it is off by default.

If you have a concrete proposal on a better way to implement this I would love to hear it.

Those concerns stated, I’d be fine with the proposal as stated. It would accommodate all the different ways I can imagine using SwiftPM on my own projects. It might be confusing to newcomers, but it won’t be a roadblock for getting work done.

At this point, what I want to do is get this implemented and in a usable state, and then gauge how problematic this behavior difference is. If it is a source of confusion, then we should iterate to try and address it.

- Daniel

···

On Nov 28, 2016, at 8:25 PM, Paul Cantrell via swift-evolution <swift-evolution@swift.org> wrote:

Cheers,

Paul

On Nov 19, 2016, at 11:48 PM, Anders Bertelrud via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

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

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


(Anders Bertelrud) #17

One conceptual reason to keep the pinning information in a separate file is that, while a manifest is a single source of truth for the inherent dependencies of a single package, a pin file expresses contextual constraints about a package graph as a whole. The fact that the information in the pin file is context-based also leads to a second big reason to keep them separate: it would be perfectly reasonable to have multiple pin files for the same top-level package, for example in the case of CI systems that should test against the latest release versions of dependencies as well as the latest pre-releases (for example). There would need to be additional facilities for this to work well, but the design should not preclude the use of multiple pin files in the future, and storing multiple sets of pinning info in the manifest would make that harder.

Anders

···

On 2016-11-28, at 09.44, David Sweeris via swift-build-dev <swift-build-dev@swift.org> wrote:

Does the pinning info have to be in an optional file? Would it be better to have it be an optional part of the manifest? There'd be one less file to carry around.


(Daniel Dunbar) #18

Haven’t had a chance to catch up on the latest discussion, but I just saw that the Yarn developers posted an excellent piece on lockfiles this week:

https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all

They argue lockfiles should be commited by libraries (but still ignored by applications). The essential point is that this makes it easier for developers of the library to maintain a coherent build of the library when dependencies ship a bug. The focus is particularly on new developers, who would otherwise lack a lock file.

This post does not acknowledge the social and cultural engineering impact of this policy contributing to a lessening of conformance to semantic versioning. I still believe such an effect will happenl, and that it will have negative repercussions for the ecosystem. I am willing to be convinced (and hopefully am wrong), but I find any post which doesn't acknowledge the potential for this an incomplete argument.

I will also point at that we have `package edit` as a top-level feature exactly to enable the downstream developer contribution behavior in a way which avoids the problem this blog post is referencing.

- Daniel

···

On Dec 1, 2016, at 1:42 PM, Alexis via swift-evolution <swift-evolution@swift.org> wrote:

On Nov 20, 2016, at 12:48 AM, Anders Bertelrud <anders@apple.com <mailto:anders@apple.com>> wrote:

Hello Swift community,

The review of "SE-0145: Package Manager Version Pinning" begins again after revisions, starting now and running through November 28th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0145-package-manager-version-pinning.md

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

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

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 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?

More information about the Swift evolution process is available at

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

Thank you,

Anders Bertelrud
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

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


(TJ Usiyan) #19

+1 Overall. I am not in favor of automatic pinning being the default
behavior but it isn't such a big issue.

···

On Fri, Dec 2, 2016 at 12:18 PM, Anders Bertelrud via swift-evolution < swift-evolution@swift.org> wrote:

On 2016-11-28, at 09.44, David Sweeris via swift-build-dev < > swift-build-dev@swift.org> wrote:

Does the pinning info have to be in an optional file? Would it be better
to have it be an optional part of the manifest? There'd be one less file to
carry around.

One conceptual reason to keep the pinning information in a separate file
is that, while a manifest is a single source of truth for the inherent
dependencies of a single package, a pin file expresses contextual
constraints about a package graph as a whole. The fact that the
information in the pin file is context-based also leads to a second big
reason to keep them separate: it would be perfectly reasonable to have
multiple pin files for the same top-level package, for example in the case
of CI systems that should test against the latest release versions of
dependencies as well as the latest pre-releases (for example). There would
need to be additional facilities for this to work well, but the design
should not preclude the use of multiple pin files in the future, and
storing multiple sets of pinning info in the manifest would make that
harder.

Anders

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


(Paul Cantrell) #20

This version of the proposal seems acceptable to me, though I have a nagging feel that it’s more complex than it needs to be.

I agree with this feeling, but am not sure how much it will matter in practice.

As I said in my review, I suspect it will confuse newcomers and lead to a few much-duplicated questions on Stack Overflow, but not hurt teams whose workflow is already established. One-time cost, not so bad.

AFAICT, the one thing this simpler model wouldn’t allow is for a team to share pinned versions of some individual ill-behaved dependencies without pinning all of them. If that’s the only missing behavior, my gut tells me there’s a way to do that with less cognitive burden on SwiftPM users.

This is a benefit I *really* want to retain, because we continue to feel it is important for people not to overly constraint the dependency graph, and I still feel this is the right model for users to follow (pin problematic dependencies, not everything). I really want to retain the technical ability to do it, even if it is off by default.

Understood.

If you have a concrete proposal on a better way to implement this I would love to hear it.

I chewed this over, and sketched out a variant that I thought was simpler. I then realized that what I came up with is basically identical to the existing proposal, except:

1. I renamed `--enable-autopin` to `pin --new`.
2. It doesn’t have the convenience of new requiring `--repin` if everything is already pinned.

At least I _think_ what I came up with is nearly identical. Here it is in case you want to check me on that:

https://gist.github.com/pcantrell/10cf1cc2cc2d8f1b5011e0ea5f828402

At this point, what I want to do is get this implemented and in a usable state, and then gauge how problematic this behavior difference is. If it is a source of confusion, then we should iterate to try and address it.

Agreed! This is a fine starting point.

Cheers,

Paul

···

On Dec 2, 2016, at 11:11 AM, Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Nov 28, 2016, at 8:25 PM, Paul Cantrell via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote: