> On Mar 21, 2016, at 4:05 PM, Max Howell <[email protected]> wrote:
>
>> 2. I like VersionLocks.json well enough, but would like to see a discussion
>> about possible alternatives. My personal proposal (in line with #1) is to
>> use "PackageVersions.json" which has a nice agreement with Package.swift and
>> would mean two common metadata files show up adjacent. I don't really want
>> to bike shed on the name, but I suspect whatever we pick first will last for
>> a while so I would at least like to review the various alternatives. I also
>> will throw out that my personal opinion is we don't need to pick a name that
>> bears much resemblance with existing terminology, whatever we pick will
>> eventually become "the standard" for the SwiftPM ecosystem so I would prefer
>> to pick the most-descriptive-possible name up front, not one that alludes to
>> the same concept in other systems.
>
> I like PackageVersions.json
Cool. There are still some instances of `VersionLocks.json` in the doc, btw.
>> 3. I like the terminology section here, I almost feel like we should adopt
>> that as official terminology in our documentation (which I don't think we
>> have yet, correct me if I am wrong).
>
> We don’t, but I agree, we should aim to pick some names and use them
> consistently.
>
>> 4. I would like it if the lock file recorded the exact SHA it received, and
>> validate that when retrieving. This helps protect users against MITM attacks
>> or unexpected changes if an upstream modifies a tag. It also can be used as
>> part of safety checks when migrating to an alternate repository host which
>> is expected to have the same content.
>
> Good point, this should be there.
>
>> 5. The "workflow - build" sections #2,3,4 are rather complicated. Is this
>> because the proposal is trying to work with existing Packages layouts, or
>> because the proposal is trying to handle the various variations of what the
>> user may have checked in inside the Packages subdirectory?
>
> The latter, if we are to support checking in the `Packages` directory, we
> should handle it when it is so. Is there a simpler way you can see?
I think the minimal feature here is that swiftpm grows the ability to read a
specification file which declares how the package dependency tree is resolved.
These seems like it should be primitive, and straightforward, so it is
worrisome that the proposal is so complex.
What is your mental model for what checked in Packages should be? I can see
several ways to interpret them:
A. The checked in copy is just a replica of the dependency repository. It
exists to ensure the product can be built in a self contained manner.
B. The checked in copy *is* the dependency, it is basically a "convenient fork
at a pinned version". SwiftPM recognizes the use case so that it can provide
convenient features for the use case (like easily updating the fork).
C. Something in between. The checked in copy exists to ensure self containment,
but it is also not treated like a fork.
I feel that too many problems are being combined into one proposal, and I think
it makes it hard to work through and discuss the ramifications of all the
changes here. There are at least five problems discussed in the current
proposal:
1. The feature for pinning of package versions.
2. The workflow for interacting with the pinning feature (i.e., --lock, etc.).
3. Interactions between checked in Package trees and package versions.
4. Swift toolchain versioning issues.
5. Local diffs.
#1 and #2 obviously make sense together, and I can see how #1 and #3 might have
to be in the same proposal (since implementing #1 may require solving #3 to not
break things). I would like to see the rest be teased apart just so it is
easier to understand all the implications.
A priori, I don't see how that minimal feature should require significant
discussion w.r.t. checked in Packages. For example, consider the three models I
gave above:
- If the expected model is (A), then the behavior I get with checked in
Packages should always be exactly the same as if I blew it away (assuming the
remote hasn't had content deleted). Therefore, the only new behaviors that need
come with version pinning are probably a few cases of error detection (when
updating versus a remote which has deleted content).
- If the expected model is (B), then I would expect the behavior to be that
there *must* be a PackageVersions.json, and the Packages *must* match those in
that file. This may be what the existing rules are trying to codify, if so I
think it would be most clear to simply specify the intent.
o I can see how there are workflow issues around the user editing their
"local fork" and needing to update both the PackageVersions.json, but it seems
like they follow from the basic behavior of "the PackageVersions *must* match
the local fork".
- If the expected model is (C), then I think it is important to clarify
exactly what a checked in Packages subdirectory is for first. I have trouble
seeing what that is when the implementation uses it *and* the user may modify
it.
>> 6. I wonder if we should be defining, as Eloy alludes to, two different
>> things:
>> - The version lock file, which defines the expected versions for the
>> package manager to use when it is doing package resolution.
>> - The package state file (in Packages.swift), which is used by the package
>> manager to track information on the Packages/ subdir state in order to
>> provide useful features primarily focused at the scenarios when the user is
>> modifying those files.
>> Currently it seems like a lot of the behaviors in the proposal are focused
>> at the latter case, but they feel like they should be decoupled problems to
>> me.
>
> I’m not sure we need a second file, since the versions of the “installed
> dependencies” are recorded in the directory names as well as that we also do
> full clones, so that information is part of the clone & checkout.
Good point. I thought it might help to have all the data combined in a file we
could easily read, but I guess the only thing we can't infer is if the user
makes a modification to a tag or something where we wouldn't have the previous
hash.
- Daniel
>
>
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
