adding support for dev dependencies

Topics: General
Jan 20, 2013 at 10:44 AM
Edited Jan 20, 2013 at 12:33 PM

The proposed idea is to add the ability to install a package into a project as a dev dependency only, as opposed to a runtime dependency.

The idea was suggested to me by dfowler whilst we were discussing possible solutions to issue 1956 'When using nuspec / csproj pair, provide a way to exclude package dependencies'.

Having packages installed as dev dependencies would allow the pack command to disregard them when packaging the project using the csproj and would solve issue 1956.

A similar feature is already implemented in NPM - see http://www.devthought.com/2012/02/17/npm-tricks/ 'Dev dependencies'.

The proposed implementation stages are:-

  1. add support for a new optional 'type' attribute in packages.config with values 'dev' or 'runtime', e.g.
    <package id="Foo" version="1.0.0" type="dev" />
    <package id="Bar" version="1.0.0" type="runtime" />
    ('runtime' would be the assumed default value if the attribute is missing)
    and use this in the pack command to ignore packages marked with 'dev' 
  2. add a new 'dev' switch to Install-Package (or a 'type' switch with argument values 'dev' or 'runtime')
  3. change the 'Install' button in the GUI to a split button (e.g. http://wyday.com/splitbutton/) with the second option being 'As dev dependency'

(The naming in all of the above is open to discussion.)

I think stage 1 has independent value and I'd like to get to work on it ASAP if the core team feels this proposal is valid. Stages 2 and 3 can come later.

(FYI I cancelled my previous pull request for issue 1956 and, as per dotnetjunky's comment on that request, I'm opening this discussion here before attempting a new implementation.)

Developer
Jan 21, 2013 at 4:36 PM

Is the 'dev' switch for Install-Package to tell the command to put type='dev' into packages.config? Is that all it does?

Jan 21, 2013 at 4:41 PM

That's correct. Similarly with the GUI.

Developer
Jan 21, 2013 at 8:23 PM

We'll probably not do it for the GUI. We want to keep it simple.

Jan 22, 2013 at 7:37 AM

Fair enough. Personally I'm not too bothered about the command line either. Even if just stage 1 is done and I have to hack in the attribute manually it will have a lot of value, so I'm keen to concentrate on just that initially.

Any thoughts about naming? I'm not a huge fan of 'type' for the attribute name. Perhaps 'usage' instead? Or is there a better term to describe dev/runtime?

I also dislike abbreviation so I'd rather the attributes values where 'development' and 'runtime' (if we stick with those terms).

Developer
Jan 22, 2013 at 8:05 AM

What about target?

Jan 22, 2013 at 8:54 AM

I quite like 'target' but do you think it's OK given that we have various target* attributes in nuspec which mean something else?

Developer
Jan 22, 2013 at 5:27 PM

So let's only do #1. We don't need #2 and #3. 

I don't like 'target'. We already have the 'targetFramework' in there. I'm OK with the initial suggestion 'type'.

Jan 22, 2013 at 6:36 PM

OK, let's go with type="development|runtime" for now.

I'll put together a pull request for stage 1 ASAP and submit it as a solution for issue 1956.

Jan 22, 2013 at 9:56 PM
Edited Jan 22, 2013 at 10:36 PM

pull request created http://nuget.codeplex.com/SourceControl/network/forks/adamralph/nuget/contribution/3959

docs pull request https://github.com/NuGet/NuGetDocs/pull/59

Coordinator
Jan 23, 2013 at 4:30 PM

Thanks for the contribution, @adamralph!

I think we might be better off defining behavior rather than defining a type or mode. Does "dev" simply mean "don't include this as a dependency when packing?" If there is only one behavior as a result, I would rather have something like ignoreDependency=true. That way, if another "dev" behavior is identified, we can add another attribute and leave this behavior alone, as people will certainly want to be able to control each switch individually. My concern is that as soon as a second behavior is added into the existing "dev" setting, we will inevitably break someone who doesn't want the second behavior.  If we can never add a se one behavior to it, then why name it generically?

Thoughts?

Jan 23, 2013 at 9:08 PM

I've given it some thought and I like the idea of a Boolean.

The idea for dev dependences was inspired by the proven approach in NPM, which has devDependencies listed separately to 'standard' dependencies. Since we probably want to keep listing the packages in a single list in packages.config then the artefact which would convey the equivalent information would indeed be a Boolean flag.

Also, I think the dev/runtime naming is not such a great fit when you develop a package which is an extension to a package which is typically used for development only. E.g. There exists a package called CoolTools which is intended for use as a dev package only. I import CoolsTools into my project SuperCoolTools, which is an enhanced version of CoolTools. SuperCoolTools needs to be shipped with CoolTools as a dependency, but I don't think 'runtime' describes the dependency accurately and could lead to confusion.

Given the two above reasons, I vote for making it a Boolean as you suggest.

As for the naming of the Boolean, I'm keen for it to represent the base fact. The fact that we don't want to include certain packages as dependencies is derived from the fact that those packages are required when developing the package but not after it has been published. I can't think of a scenario where packages are only required for development, but we'd still choose to bundle them in as dependencies. Also, if we communicate the base fact there is more chance that we can derive further facts from it which would allow us to introduce further behaviours, without having to introduce new artefacts into packages.config.

For this reason, I propose developmentDependency="true". This is also semantically equivalent to NPM's separate devDependency listing).

Jan 24, 2013 at 6:32 AM
Edited Jan 24, 2013 at 6:39 AM

I don't like "dev" or "developmentDependency", but "runtime" is clear (needed for the app to run).  "developmentDependency" doesn't describe what is done, IMO - both tool packages and library packages are used during development.

I think this is an area that the NuGet design apparently missed - and it needs to be shored up.  Thank you Adam for digging into this - I'd be happy to help, as well.

Maven uses `<scope>` on dependencies to indicate what's being discussed here; and it also supports excluding transitive dependencies of direct dependencies:

http://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html

While this is probably a bigger discussion, Maven has gone through several major design iterations and has had more time to evolve than NuGet, so it's at least worth considering.  Borrowing from many years of Maven experience (but also simplifying a lot), I think 2 attributes for each dependency are needed in NuGet:

  1. Is this a compiled and linked dependency? (Perhaps "referenced" is a better term, since it matches the MSBuild schema.)  If yes, it should be a `<Reference>` in the MSBuild project, which could be verified if there was a clean way to designate that the dependency requires a compile-time reference.  If no, there could be verification that the there is no `<Reference>` in the MSBuild project on this packages contents.  Note that right now, no verification can be done on the MSBuild file missing <Reference> elements, b/c there's no way to know whether it should be there or not.
  2. Is this a run-time dependency?  You can have no linked dependency, but a package can still be required for the dependee to run.

The question of transitive dependencies is obviously important.  For clarity, consider project A has a NuGet dependency on B, and project B has a NuGet dependency on C (A -> B -> C).  Should C be included when A is built?

If C is a run-time dependency of B, and B is a run-time dependency of A, then C should be included in the build output of A.

If C is not a run-time dependency of B; or if B is not a run-time dependency of A, then C should not be included as a reference or in the build output of A.

If C is a referenced dependency of B, then B.csproj should have a `<Reference>` to C; if C is not a referenced dependency of B, then B.csproj should not have a `<Reference>` to C.

Regardless of whether B has a reference on C, A would never have a transitive reference on C - it would have to be a declared reference.

So a proposed schema would be:

<package id="Library" version="1.2.2.0" targetFramework="net403" referenced="true" />
<package id="Library2" version="1.0" />
<package id="Tool" version="0.5" referenced="false" runTime="false" />

If these new attributes were omitted, the implied values would be referenced="true" runtime="true", which is the only behavior that's possible today.

It would be legal to have referenced="true" and runtime="false" - that could be legitimate if the DLL were provided by the environment, but it should explicitly not be part of the build output.

In addition, it would be great if a project could indicate in its nuspec whether it's a tool project or a dependency (the 2 most common cases, IMO).  That way the command-line tools and UI could intelligently guess the right values for referenced and runtime; so tweaking them manually would be a rare occurrence.

As an aside, Maven has the concept of a plugin, which is a package that is part of the build process, but not included in the dependency tree.  This is needed b/c Maven is equivalent to MSBuild plus NuGet.  My point is that in Maven, tools packages are clearly indicated with a different XML element, and different configuration options.  This might be a good choice in NuGet; but would be a big change.  A simpler approach would be just to designate tools packages with `referenced="false" runTime="false"`.

 

 

Jan 24, 2013 at 8:22 AM
Edited Jan 24, 2013 at 8:23 AM

@johncrim thanks for the input.

I think your suggestions are aimed at providing a large set of potential behaviours than we've focused on so far. To clarify, the immediate problem we are tying to solve is http://nuget.codeplex.com/workitem/1956. However I do appreciate that whatever solution we use for this issue should fit on the road map for a wider vision if one exists. I think the referenced attribute might be useful but I think it's out of scope for this discussion.

As you say, both a tools package and a lib package are used during development, but only one of them is used at runtime, so I guess 'developmentDependency' really ought to be named 'developmentDependencyOnly', or 'developmentOnly', which I think would be the semantic inverse of the proposed 'runtime'. I don't really mind which one we use but I guess 'runtime' is cleaner.

Would everyone be happy with runtime="false" (defaulting to true)?

Jan 24, 2013 at 9:15 PM

It seems we've reached consensus on at least using a Boolean attribute so I've updated the pull request with an implementation of runtime="false".

http://nuget.codeplex.com/SourceControl/network/forks/adamralph/nuget/contribution/3959

If the core team would prefer a different name (possibly with an inversion of the Boolean) just let me know - that will be a quick change.

Developer
Jan 25, 2013 at 1:58 AM

I think it's non conventional to have a bool value default to 'true'. Normally bool value has 'false' as the default value. So I'd vote for reversing the attribute name to have default value as 'false'.

Coordinator
Jan 25, 2013 at 2:14 AM

I agree - defaulting to false makes more sense to me.  I'm still not sure I get "development" or "runtime" either--it doesn't tell me what it means.

What about "excludeDependency='true'" so that it's more explicit?  The other benefit to naming the property to match its behavior is that it can then be used in other scenarios without it being awkward.

Here's another scenario where a package only makes sense at development time:

  1. Package_A contains source code only, no binaries;
  2. Package_B has a package reference to Package_A, and compiles that source code into its lib folder;
  3. Project_C installs Package_B - it should not carry Package_A as a dependency.

Having Package_B's packages.config file use <package id="Package_A" version="1.0.0" targetframework="net40" excludeDependency="true" /> is clear to me that when I package Package_B, Package_A will be excluded from the dependency list.

Jan 25, 2013 at 9:08 AM

I agree that the bool should default to false, that is normally the approach I like to take as well. If I'm not mistaken the semantic inverse of 'runtime' in this context would be 'developmentOnly' so if we only invert the value then I think that would be the name to use.

Regarding the name itself, it seems we have two candidate solutions

  1. we communicate the underlying reason that we don't want to include the package, i.e. 'this package is only required during development and not at runtime' and we derive the fact that we shouldn't include it as a dependency when packaging (and possibly other behaviours further down the line)
  2. we communicate the explicit behaviour that we want, i.e. 'do not include this package as a dependency when packaging this project'.

For solution 1 the best name I can think of right now is 'developmentDependencyOnly'.

For solution 2, I'm not convinced that 'ignoreDependency' or 'excludeDependency' fully convey the information unless the reader already has the context of packaging in their mind. E.g. I open a project with which I'm relatively unfamiliar. I look at packages.config and I see excludeDependency="true". What does that mean? Intuitively I would guess that someone has installed the package but they've temporarily excluded it since they haven't yet written any code which uses it. The best suggestions I can think of right now are 'excludeDependencyFromPackage' or 'skipDependencyWhenPackaging'. Both are a bit of a mouthful ;-).

I'm still leaning towards solution 1. I think it's better to communicate the base facts and derive behaviours from them. @jeffhandley is leaning towards solution 2. I don't have a strong preference either way. If the core team feels that solution 2 is better then I'll be happy with that.

Ultimately I don't want the naming conversation to drag on forever since I'm really keen to get this change into the next release as it will bring so much benefit to my team. If the core team feel like they want to go with 'excludeDependency' or 'ignoreDependency' then I'm happy to go with that too. I just wanted to make sure the above points were taken into consideration before a final decision is taken.

Coordinator
Jan 25, 2013 at 2:31 PM

Thanks for the good write-ups, @adamralph!  Thinking out loud...

We're really creating NuGet's equivalent of "Copy Local" that we have on assembly references.  Packages themselves will never set this value, and it would only be the developer consuming the package that would make the decision to exclude this package from dependencies.  Most likely, for some time to come anyway, this would only be a power-user feature in the packages.config file, with no UI around it, and no discoverability that the feature exists (other than documentation).

Since this is a power user, package author scenario only, I expect we'd never be able to add more behavior behind this attribute's value, we could only add new attributes if we need to introduce new optional behaviors.  If we cannot think of any other behaviors that would make sense to add to "developmentDependendency," then we can stick with that name.  We just need to recognize we likely won't be able to overload new behaviors on that attribute without introducing unintended consequences on some package authors.  If you're okay with that, I can settle on the name.

Thanks again,
Jeff

Jan 25, 2013 at 6:21 PM

I don't really like "development" either - historically "designtime" or "compiletime" is more appropriate, IMO.

Jan 26, 2013 at 7:09 PM
Edited Jan 26, 2013 at 7:09 PM

I have to admit that this is confusing:

<dependency ... excludeDependency="true" />

But I would definitely take "excludeDependency='true'" over "developmentDependencyOnly='true'" - the former describes more or less what it does (excludes both direct and transitive dependency), the latter is vague and still indicates that it's a dependency; and really doesn't make any sense to me - both libraries and tools are used during development.

An alternative name could be "excludeReference='true'",  since part of what is happening excluding a reference in the project.  It's still counted as a NuGet `<dependency>` in the packages.xml.  However, I care more about transitive dependencies not including the dependency (see https://github.com/Fody/Fody/issues/33 for detailed info on my current problem - which I don't think is the only problem with the current implementation, but it definitely should be addressable with the fix for this problem), so my vote would still be for "excludeDependency" over "excludeReference".

Other alternative names are "buildTool", "buildOnly", or "excludeTransitive". 

I suppose of all of the options discussed so far, my top 2 favorites are:

1. My proposal above - referenced="false" runTime="false" - because it indicates that it should not be a transitive dependency of projects that include this project (runTime="false");.  And it indicates that no `<Reference>` should be present in the project ( referenced="false").  In other words, this is a build tool.  I understand the desirability of having defaults be false, but I think the need for clarity and simplicity outweighs that guideline.

2. buildOnly="false" - this indicates effectively the same things - it's not needed at runtime, so it shouldn't be a transitive dependency.  It's slightly weaker in indicating whether a `<Reference>` should exist in the project or not.

 

 

Jan 26, 2013 at 7:29 PM

I disagree that this is equivalent to `<Reference CopyLocal="False">`.  That's still a compile reference, it's just one that is provided by the environment (usually the Gac, but could be another host) instead of being included in the OutputDir.  In Maven lingo, `<Reference CopyLocal="False">` is equivalent to `<scope>provided</scope>`.

But for tools projects (which I think is the primary case being addressed here - would be good to be clear about that), there should be no `<Reference>`.  There may be an MSBuild dependency, but not a compile time reference.

If someone requires `<Reference CopyLocal="False">`, that's currently possible in NuGet - they'll just have to manually re-add `CopyLocal="False"` every time they update that dependency.  However, it's not currently possible to exclude transitive dependencies (for tools packages), which is why I think being able to exclude the `<Reference>` and exclude transitive dependencies is a more critical need.

Jan 27, 2013 at 11:24 AM
Edited Jan 27, 2013 at 11:26 AM

This discussion is picking the scab around one of the areas we have struggled with regarding NuGet usage.  Just to clarify my understanding on a few points (came to the discussion a bit late!):

  1. NuGet by default will add the full transitive package dependencies to the packages.config, and the .csproj as references.  This in no way has to represent the actual runtime requirements, just what is stated in the packages themselves.  There is no way to turn this on or off, or otherwise control this behaviour when adding a package.
  2. Due to the default behaviour of MSBuild, combined with NuGet dropping packages into a set of directories individually by default, the compiler is not provided with the usual slew of auto-detected actual runtime dependencies when compiling, as this is generally gathered by walking assembly manifests within a known set of directories, including the same directory as a referenced file.  This requires NuGet to add the .csproj references for any assemblies added to the packages.config "just in case" as there is no direct mechanism to get any actual transitive runtime dependencies into the output directory (this is a ROYAL pain, and there are better approaches).
  3. This discussion has been raised to address issue #1956 regarding the "pack" command when targeting a .csproj, which of course is combining both problems above to give a superset of the actual required runtime dependencies defined as package dependencies for the resulting package.

If the requirements is to only include the actual runtime dependencies in the resulting package, why not just walk the output assemblies dependencies prior to building the package?  The .csproj holds the assembly output location, you can easily (and very quickly) parse and build the full transitive runtime tree, and then simply examine which packages are required to provide the exact assemblies stated by the assembly manifest.  And you can do this without extending anything other than the pack command.

If the intent is purely to solve the stated problem, there is additional overhead suggesting the developer should double guess the compiler and the runtime and add details to the packages.config manually (not ruling out that I have misunderstood the stated problem!!!).

We are already doing a bunch of this type of thing on our internal feeds, including many discussions around extending the pack command.  I have a bunch of related code here that may be of use if this approach makes sense.

Jan 28, 2013 at 7:39 AM

That's a great point Ben.  Why add more knobs when it's possible to automatically do things right?

On that note, I see two fixes (only one of these two alternatives would suffice) for this that would address my problem (I think), and that wouldn't require a change to the packages.xml schema:

  1. Per your recommendation, scan the build output for actual runtime dependencies (those that are referenced), and only include those in the .nuspec file within the .nupkg.
    This might get complex if the .nupkg includes multiple DLLs - for example a Silverlight 5 version and a .NET 4.0.3 version with different dependencies.  But it should be doable.
    In this case, build tools could be included in the /lib dirs., and have unneeded project `<Reference>`s on any build tool DLLs, but not worry about being included as a transitive dependency for all projects that use the current project.
  2. For build tools, support putting build tool DLLs under `/tools` in the .nupkg, so they can be referenced from the project's msbuild but not with a `<Reference>`.  This seems intuitive given the design of packages, but per http://nuget.codeplex.com/workitem/1910 , this approach currently won't work.
    I would expect that tools would not be included as transitive dependencies - but I could be wrong - it's difficult to determine given 1910.  Certainly a package with nothing in a `/lib` dir shouldn't be considered as a runtime dependency, thus it shouldn't be included as a transitive dependency.

There are probably use-cases not handled by either of these options - but the only one that comes to mind is dependencies that are provided by the environment, such as a host app-domain.  IMO it's ok to postpone this use-case, b/c packaging for a host application seems outside the intended use of NuGet.

 

 

Jan 28, 2013 at 6:52 PM

I really like the idea of automatically determining dependencies. These kinds of extensions would make NuGet a much richer tool than it is today. There are many points above which I believe are worth pursuing and it would be great to see that done.

I do still think there is a need for a manual override to switch off a dependency. Static analysis can never tell the whole story. I think we should press ahead with the current proposal of the single bool switch defaulting to false. If people want to pursue the other discussion points then I think there's no problem with that as a separate exercise. I don't think any of the above implementation proposals will be incompatible with the explicit override switch.

@jeffhandley I'll send a new pull request ASAP with an implementation of developmentDependency="true".

Jan 28, 2013 at 11:01 PM

@adamralph adding a manual flag in the repositories.config to override the behaviour of a static analysis derived set of dependencies would only need to support exceptions, right?  And for most people, this would not be required.  And where the "pack" command is WAY wrong, there is always the fallback to a manually defined nuspec.  The proposal to move knowledge required to "pack" a NuGet package into the configuration used to define the code dependencies feels a little like a blurring of responsibilities.

Would it not be easier adding a +exceptions/-exceptions command line flag to the "pack" command to extend any statically detected dependencies?  This would require a modified command line to "pack" when you know there are exceptions, and that knowledge would not by default sit with the project, but it does involve a lot less modification to the core.

@johncrim Detecting which assembly from a package should be pretty easy as we can cross-reference against the .csproj to see which one was actually referenced.  I already do something similar for an audit tool.

Jan 29, 2013 at 7:09 PM
Edited Jan 29, 2013 at 7:09 PM

@BenPhegan the reason that issue 1956 causes us so much pain is that we pack a large number of projects which have tools/content only nuget packages installed.

Using manually defined nuspecs would cause us a lot of work. At the moment we have identical nuspecs defined for each project. We discover these at build time and then rely heavily on NuGet's ability to pack against the corresponding csproj files in order for package dependencies to be automatically picked up (the ones which are actually required at least!) and for variable substitution to take place.

Regarding blurring of responsibilities, the packing of a project using csproj already uses packages.config to determine which packages should be listed as dependencies. It would seem natural to me to override that behaviour in the same place. Moreover @dfowler, who proposed this solution to me in the first place, was of the strong opinion that nuspec and packages.config should have always been implemented as one file. I'm not sure if this is something that the core team are considering, but if so then the exclusion of dependencies in packages.config would again seem like a natural choice.

I'm not really in favour of adding +exception/-exception flags to the pack command. Since we are discovering nuspecs at build time for a large number of projects, this would mean we would have to store the list of exceptions for each project somewhere in order for us to be able to alter the command accordingly each time. This could be yet another text file which sits next to the csproj, nuspec and packages.config files but again, I'm drawn to packages.config as the natural place for this.

Now, I do think it's true that automatically determining the dependencies via static analysis should catch the majority of cases but, correct me if I'm wrong, I see this as quite an undertaking. I'm not sure that it's something I will have the time to look at myself, and the core team have already stated that issue 1956 is not something they are going to pick up and have left open for contributors to pick up. For this reason, I'd really like to see the solution using a boolean switch on packages.config go into the code base and go out in the next release or ASAP. If someone wants to contribute the better and more versatile solution later (and deprecate accordingly) then that would be great too.

Jan 29, 2013 at 8:05 PM

New pull request sent http://nuget.codeplex.com/SourceControl/network/forks/adamralph/nuget/contribution/3998

Jan 30, 2013 at 10:34 AM

@adamralph Manual nuspecs are a pain, agreed.  As to blurring responsibilities, and packages.config being the right place for both dependency information and packaging details, I have to strongly disagree...in most instances the  project file is the right place for this type of information.  Tempting to say it is a bit late now (but it really isn't, as MS own the whole lot) but the decision to have dependencies stated in two places makes NuGet feel like a bolt-on.  Which makes sense historically as an Open Source effort, but less and less so as MS pushes it forward.  Moving all this into the actual file that defines the project by extending the MSBuild schema for it feels like a better solution (but would have impacts to others such as XBuild).

As for a lot of work to get the full transitive checking, probably not really that much to be honest...at least getting it to work outside of the pack command itself.  Will take a look, I already have much of the code in a bunch of other commands.  Won't commit to doing anything concrete until someone from the team with commit rights pipes up though, otherwise its just a waste of time.