Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the project's package file support direct dependencies only? #38

Closed
agross opened this issue Sep 2, 2014 · 23 comments
Closed

Should the project's package file support direct dependencies only? #38

agross opened this issue Sep 2, 2014 · 23 comments
Labels

Comments

@agross
Copy link
Contributor

agross commented Sep 2, 2014

Currently there's the idea to use a project-specific packages file to define the subset of package IDs that paket needs to reference in the respective project. This is to have any number of dependencies in the packages.fsx but only reference the test-specific subset among these for test projects.

Definition direct dependency: A dependency that is declared in packages.fsx
Definition indirect dependency: A dependency of a direct dependency. Indirect dependencies might change with future releases of direct dependencies (adding or removing indirect dependencies).

Both direct and indirect dependencies and their calculated versions will to be downloaded to <packages.fsx location>/packages/ and saved to packages.lock.

My question is whether it's a good idea to restrict the dependencies that are allowed in the project-specific packages file to direct dependencies.

Now on to the discussion, assuming there is no packages.lock (see below why this is a good idea):

Restricting packages to direct dependencies

Pros

  • The user must be very explicit what packages all of the projects need
  • Future package updates of direct dependencies removing indirect dependencies won't cause any harm. Paket will still download them according to packages.fsx, thus making them available.

Cons

  • If the user requires an indirect dependency that is downloaded through virtue of the fact that a direct dependency requires it, the indirect dependency would have to become a direct dependency for it to be allowed in the project-specific packages file. I.e. both the direct and the indirect dependencies need to be put in packages.fsx.

Allowing direct and indirect dependencies

Pros

  • The packages.fsx might become a bit shorter, listing only high-level direct dependencies.

Cons

  • Future updates of direct dependency removing indirect dependencies will cause paket to not download it, thereby break the build. The user would then have to update packages.fsx to make the previously-indirect dependency a direct dependency.

Why not having a lockfile sometimes is a good idea

In Ruby (with bundler) there is the notion that library developers should not check in their Gemfile.lock files (= packages.lock). The reason is that reproducible builds are generally not that much important for libraries and that you rather want to know whether the library still works with the latest dependencies according to the version constrains in the Gemfile (= packages.fsx).

This makes it very easy to just rerun bundle install and your test suite (even in CI) from time to time without the need to change any files.

I think for .NET libraries the same pattern may emerge, with services like https://gemnasium.com/ that check your libraries' dependencies weekly and notify you about (semver) breaking changes in your libraries' dependency set.

For applications, on the other hand, the situation is a bit different. For them you want reproducible builds, regardless of dependency updates (according to `packages.fsx´). This is where the lockfile comes into play. It records the fact that you proved your app works against a specific set of dependencies in specific versions. Using the lockfile enables you to recreate the same runtime environment over and over again.

You can read more about the way Ruby handles this here:
http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/
http://bundler.io/v1.3/rationale.html#checking-your-code-into-version-control

I hope I could get my point across. Looking forward to your feedback!

@agross agross added the question label Sep 2, 2014
@agross
Copy link
Contributor Author

agross commented Sep 3, 2014

BTW, bundler allows referencing indirect dependencies. I'd say with great power comes great responsibility.

@forki
Copy link
Member

forki commented Sep 3, 2014

I think we make it optional to add indirect dependencies to the packages file.

Will implement this. Would also be good to put this question into the docs

@ilkerde
Copy link
Contributor

ilkerde commented Sep 4, 2014

I opt for allowing direct and indirect dependencies.

I think it is fairly ok to keep direct deps in packages.fsx and have the graph frozen in lock file. I would love to see that Paket is smart enough to provide me a check whether a project starts to reference a yet indirect reference and possibly even lets me elevate it to a direct one.

In the end, Paket would have two sets at hand: with packages.fsx a great summary of what really is demanded (logically associated), plus with all packages.xmls a list of what is needed to run it.

@forki
Copy link
Member

forki commented Sep 4, 2014

@ilkerde

First of all we changed our naming a bit. We now have package.list files. They replace the old packages.config files which are next to csproj files. See https://github.com/fsprojects/Paket/blob/master/src/Paket/packages.list for sample.

At the moment we support both, direct and indirect package names in this file. If you only specify the direct ones, then Paket will figure it out for you.

@ilkerde
Copy link
Contributor

ilkerde commented Sep 4, 2014

@forki I'm obviously not up to the pace you are on. I don't have an idea on how packages.fsx relates to packages.list. If I assume that packages.list is a substitute of packages.config, I would prefer to list all packages (both direct and indirect ones) in packages.list, without giving the "user" the option to omit them. I have no good feeling at making Paket part of the overall dependencies needed to get an application running. But this might equally well be my tendency on conservative dependency paranoia.

@forki
Copy link
Member

forki commented Sep 4, 2014

Paket is not needed to get the app running. Only to update dependency definitions.
Yes packages.list is a replacement for packages.config - but for compilation only the csproj files are needed.
Paket install can modify your csproj files according to packages.lock and packages.list.

Will update docs tomorrow

@ilkerde
Copy link
Contributor

ilkerde commented Sep 4, 2014

@forki Thanks for pointing that out. I am well aware that the references are in *proj files. The case here is that if you only refer to direct dependencies in packages.list. Hence the clarity and consistency is gone (from my pov), since it diverges from *proj content. In case the packages are not in place, I would need Paket to restore everything for me. Just the very same as Nuget.

My point is, that John Average (that's me) may wonder why there are so many (missing) references in project, will have a look at the packages.list file and even wonder more why there are so few packages listed.

@forki
Copy link
Member

forki commented Sep 4, 2014

/cc @agross - I'm not really sure what we should do here
On Sep 4, 2014 11:06 PM, "ilkerde" [email protected] wrote:

@forki https://github.com/forki Thanks for pointing that out. I am well
aware that the references are in *proj files. The case here is that if you
only refer to direct dependencies in packages.list. Hence the clarity and
consistency is gone (from my pov), since it diverges from *proj content. In
case the packages are not in place, I would need Paket to restore
everything for me. Just the very same as Nuget.

My point is, that John Average (that's me) may wonder why there are so
many (missing) references in project, will have a look at the
packages.list file and even wonder more why there are so few packages
listed.


Reply to this email directly or view it on GitHub
#38 (comment).

@agross
Copy link
Contributor Author

agross commented Sep 4, 2014

Well, we've talked about it. There are two options, being explicit about all project dependencies in packages.list or let the tool do the heavy lifting and compute indirect dependencies for the direct ones we specify in packages.list.

Since we don't tell paket about indirect dependencies in packages.fsx, why should we be (IMHO) overly explicit in packages.list? My two cents.

If @ilkerde is favoring to be explicit in packages.list, paket won't stop him from doing so. I think this model serves us both very well.

@ilkerde
Copy link
Contributor

ilkerde commented Sep 4, 2014

Fair enough. Does it mean then that I can configure Paket to be "strict" or "loose"?

@ilkerde
Copy link
Contributor

ilkerde commented Sep 8, 2014

@agross @forki The longer I think about it, the more doubtful I get about omitting indirect dependencies in References.list. As far as I understood, we aim to replace packages.config with References.list (in the long run). IMHO, this alone would satisfy to strictly go for all dependencies instead of only having the direct ones listed.

The main reason is simply sanity. A (quasi) mirror of dependencies in References.list would enable us to perform a few nice sanity checks on .proj files. If a reference is deleted from .proj, we beg the user to decide whether he really wants to ditch it (thereby removing from References.list as well) or restore his faux-pas. In a transition from Nuget scenario (might be interesting), we can check for integrity of packages.config <-> References.list <-> .proj. Speaking of inter-proj-level checks, we can leverage the full Referenes.list for a few consistency checks offline without calling out to package sources to resolve indirect dependencies.

@agross I know you feel fine with the laissez-faire model to just let the user decide what's in References.list. However, I want you to please rethink and judge again. I think it is a quite important decision.

@forki
Copy link
Member

forki commented Sep 10, 2014

@agross came up with a setting in the Paket. dependencies file:

mode "strict"

Or implicit:

mode "loose"

I assume we would also have to transport it to the lockfile

@agross
Copy link
Contributor Author

agross commented Sep 10, 2014

Another option: references "strict". Makes it clear(er) what the setting is about.

@bartelink
Copy link
Member

I've scanned most, but can't say I've necessarily grokked it top to bottom but...

Certainly, for webapi, it's nice to be able to say:

Microsoft.AspNet.WebApi
hyprlinkr

When the .lock file has:

Microsoft.AspNet.WebApi (5.2.2)
  Microsoft.AspNet.WebApi.WebHost (>= 5.2.2, <  5.3.0)
Microsoft.AspNet.WebApi.Client (5.2.2)
  Newtonsoft.Json (>= 6.0.4)
  Newtonsoft.Json (>= 6.0.4)
  Microsoft.Net.Http (>= 2.2.22)
Microsoft.AspNet.WebApi.Core (5.2.2)
  Microsoft.AspNet.WebApi.Client (>= 5.2.2)
Microsoft.AspNet.WebApi.WebHost (5.2.2)
  Microsoft.AspNet.WebApi.Core (>= 5.2.2, <  5.3.0)
Microsoft.Bcl (1.1.9)
  Microsoft.Bcl.Build (>= 1.0.14)
Microsoft.Bcl.Build (1.0.21)
Microsoft.Net.Http (2.2.28)
  Microsoft.Bcl (>= 1.1.9)
  Microsoft.Bcl.Build (>= 1.0.14)
Newtonsoft.Json (6.0.5)
hyprlinkr (1.1.1)
  Microsoft.AspNet.WebApi.Core (>= 4.0.20710.0)

How about the default being implicit refs included and put a ! (or some other such modifier) in front of it to say "only take this explicit" one

@ilkerde
Copy link
Contributor

ilkerde commented Sep 11, 2014

@agross I pretty much favor references strict as a setting. Clear wording.

In general, I want to make sure we are talking about the same thing and therefore quickly reiterate my perspective.

I am very fine with paket.deps only listing direct deps. My concern is about the paket.refs. I anticipate that putting all refs in there can be a significant benefit. I know that paket.lock has the same (and even more) information. Nonetheless, I paket.refs has a straight relation to the proj and thus can be easily used for checks on proj level.

I am well aware that my perspective is conservative (and a little paranoid). Tbh, I'm not sure if we will benefit from the full ref list, but my gut feeling says "have the list for the time being".

Sorry for being so subjective on that one.

My background for bringing this concern onto the table is my hope that Paket will have such proj sanity features in future as well.

@bartelink
Copy link
Member

@ilkerde I assume you're attempting to achieve something specific and important, but I'm unable to infer it from the above. I'd like to get you to clarify some matters by addressing the following:

  1. There's nothing stopping you listing leaves only
  2. What do you want to do about Microsoft.Bcl.Build in the above example? I really wish it wasnt there. And I don't want to tie anything to its existence. I assume they'll eventually come to their senses and it will go away (Side: @agross / @forki I was surprised Microsoft.Net.Http wasnt before Microsoft.Bcl.Build in the .lock file above - i.e. do you agree it's a valid Issue that order should be:
    • Perhaps you could give an example of what you'd like to do/forced to do to represent the hierarchy in my example?
    • Dependency order
    • Alpha order or whatever order we're seeing
  3. Are you proposing to have modal modifiers (and the resulting need to 'run' it sequentially) in the paket.references a la the paket.dependencies ?
    • consider that the paket.references name already conveys "it's about the references managed by Paket"
    • consider that if another layer of stuff was to be developed, it could live in a .paket file
    • consider that the first time a lot of devs will experience Paket (and have a gut reaction to it) will be by seeing the .paket.references file in an open source project

Please don't take the above as rejecting your ideas; I actually want to know what they are specifically :)

What do you mean by "Paket will have such proj sanity features" ? AFAIK AspNetVNext makes references to NuGets a first class thing in a .proj file ? What are you picturing?

BTW for me references strict isn't that clear (thinking Option Strict :P).

Thinking out loud: So you want paket install/update to start flagging (and stopping?) whenever any item in the references file has a reference to stuff not listed in the file?

Could the syntax change to require me to say:

Microsoft.AspNet.WebApi+
hyprlinkr

(i.e. the + is needed for me to opt in to spidering of dependent stuff not listed in the file) ?

If that was the way, Paket could say proj\proj.cs.paket.references refers to package Microsoft.AspNet.WebApi. The following packages are not listed in the references file: 1) Microsoft.AspNet.WebApi.WebHost. To resolve, either add the required items to the the file or specify the reference as Microsoft.AspNet.WebApi+ to have dependencies be traversed

@forki Is there doc somewhere for this mode thing in the deps? Modal stuff like this makes files harder to understand IME... Could one have a modifier on e.g. the version specifier etc. achieve the same effect?

@agross
Copy link
Contributor Author

agross commented Sep 11, 2014

@bartelink The lockfile is sorted alphabetically. Does that clear things up?

@bartelink
Copy link
Member

@agross Yes, I can see that. I'm saying I think it should be sorted by first by dependency hierarchy depth (and alphabetically after that)

(And/or indentation to make the successive derivation/spidering phases explicit. @forki Avoiding temptation to go look at source to see whether these successive phases of downloading / processing are already nicely parallelized in the impl - guessing a yes! :P)

@agross
Copy link
Contributor Author

agross commented Sep 11, 2014

@bartelink Hm, well, that's two perspectives on the same data. I always found alphabetical ordering very helpful when reasoning about my dependencies. Could you give an example of what #38 (comment) would look like with your preferred ordering?

@bartelink
Copy link
Member

Grouped:

hyprlinkr (1.1.1)
  Microsoft.AspNet.WebApi.Core (>= 4.0.20710.0)
Microsoft.AspNet.WebApi (5.2.2)
  Microsoft.AspNet.WebApi.WebHost (>= 5.2.2, <  5.3.0)

Microsoft.AspNet.WebApi.Core (5.2.2)
  Microsoft.AspNet.WebApi.Client (>= 5.2.2)
Microsoft.AspNet.WebApi.WebHost (5.2.2)
  Microsoft.AspNet.WebApi.Core (>= 5.2.2, <  5.3.0)

Microsoft.AspNet.WebApi.Client (5.2.2)
  Newtonsoft.Json (>= 6.0.4)
  Newtonsoft.Json (>= 6.0.4)
  Microsoft.Net.Http (>= 2.2.22)

Microsoft.Net.Http (2.2.28)
  Microsoft.Bcl (>= 1.1.9)
  Microsoft.Bcl.Build (>= 1.0.14)
Newtonsoft.Json (6.0.5)

Microsoft.Bcl (1.1.9)
  Microsoft.Bcl.Build (>= 1.0.14)
Microsoft.Bcl.Build (1.0.21)

@bartelink
Copy link
Member

BTW the hyprlinkr was after Microsoft.AspNet.WebApi in the current output. Not sure if thats coz they are independent top level items or because ordering is case sensitive. Either way I don't think it's a good thing

@bartelink
Copy link
Member

Alternate views with redundant copies of stuff might make tracing individual decisions either but I can't think of a way of not having it confusing. Will think though...

@forki forki mentioned this issue Sep 15, 2014
@forki forki closed this as completed in 03304da Sep 16, 2014
@bartelink
Copy link
Member

@agross @forki @ilkerde So no merit in showing the phases of the resolution process at all then to allow the humans to understand what happened?

I'm not wed to it, but it seems useful and not harmful IMO ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants