Issue with External nugets used directly via #addin directive

[jrnail23]

I did some experimenting with using external nuget packages through the #addin directive, and I found that it only works when there is only a single dll file for each dll name in the referenced nupkg file.

Before I get into detail here, I should say that I assume what I'm trying to do here isn't really what the developer of Cake's #addin directive intended when writing it (there's really no documentation that I could find to explain the intended usage for the various ways to reference such dependencies).

I'm sure my initial explanation was about as clear as mud, so here's an example:

Let's say I want to use Polly in my Cake script, so I do this:

#r "./tools/Polly/lib/net45/Polly.dll"
using Polly;

Task("Default")
    .Does(() => Policy.Handle<IOException>()
        .WaitAndRetry(10, i => TimeSpan.FromMilliseconds(50))
        .Execute(() => CleanDirectory(@"c:\MyFakeBinaries")));

RunTarget("Default");

Ok, so I run that, and it works.

However, if I want to use Polly as an addin like this (because I don't want to have to specify the path to Polly.dll for various reasons -- among them being that I might like to specify the tools directory at runtime), I'm out of luck:

#addin Polly
using Polly;

Task("Default")
    .Does(() => Policy.Handle<IOException>()
        .WaitAndRetry(10, i => TimeSpan.FromMilliseconds(50))
        .Execute(() => CleanDirectory(@"c:\MyFakeBinaries")));

RunTarget("Default");

Results in this:

All packages listed in packages.config are already installed.
Installing 'Polly 2.2.3'.
Successfully installed 'Polly 2.2.3'.
Addin: Polly, adding Reference C:/Dev/tools/Addins/Polly/lib/net35/Polly.dll
Addin: Polly, adding Reference C:/Dev/tools/Addins/Polly/lib/net40/Polly.dll
Addin: Polly, adding Reference C:/Dev/tools/Addins/Polly/lib/net45/Polly.dll
Addin: Polly, adding Reference C:/Dev/tools/Addins/Polly/lib/portable-net45+netcore45+wpa81+wp8+MonoAndroid1+MonoTouch1/Polly.dll
Error: error CS1704: An assembly with the same simple name 'Polly' has already been imported.  Try removing one of the references (e.g. 'C:\Dev\tools\Addins\Polly\lib\net45\Polly.dll') or sign them to enable side-by-side.

Okay, so apparently if I want to (and I don't really want to), I can use Polly-Signed instead to resolve this, although when I did, I got this:

All packages listed in packages.config are already installed.
Addin: Polly-Signed, adding Reference C:/Dev/tools/Addins/Polly-Signed/lib/net35/Polly.dll
Addin: Polly-Signed, adding Reference C:/Dev/tools/Addins/Polly-Signed/lib/net40/Polly.dll
Addin: Polly-Signed, adding Reference C:/Dev/tools/Addins/Polly-Signed/lib/net45/Polly.dll
Addin: Polly-Signed, adding Reference C:/Dev/tools/Addins/Polly-Signed/lib/portable-net45+netcore45+wpa81+wp8+MonoAndroid1+MonoTouch1/Polly.dll
Error: C:/Dev/PollyTest.csx(5,14): error CS0012: The type 'System.Object' is defined in an assembly that is not referenced.  You must add a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.

Since I never really wanted to use Polly-Signed anyway, let's ignore this -- I just wanted to include that for your info.

As you can see from the output above, the Polly nupkg has the following file structure:

• /Polly
  • /lib
    • /net35
      • Polly.dll
    • /net40
      • Polly.dll
    • /net45
      • Polly.dll
    • /portable-net45+netcore45+wpa81+wp8+MonoAndroid1+MonoTouch1
      • Polly.dll

Cake's AddInDirectiveProcessor recursively finds all DLLs (not named Cake.Core.dll) under /Addins/Polly/ and loads them all, regardless of name or target version, causing the above error (Error: error CS1704: An assembly with the same simple name 'Polly' has already been imported.).

So that seems to be the heart of the issue -- that Cake doesn't use a particular target version to load assemblies from nuget. Further research suggested that because it's being added outside the context of Visual Studio, we don't have the context to decide which version is appropriate, but I managed to figure out how this can be remedied.

In NuGet.Core, there's a NuGet.VersionUtility class that can be used to match those target-version-specific folder names to the current runtime framework version, like this:

var version = VersionUtility.GetShortFrameworkName(new FrameworkName(AppDomain.CurrentDomain.SetupInformation.TargetFrameworkName)));

// version == "net452" when running under 4.5.2 (AppDomain.CurrentDomain.SetupInformation.TargetFrameworkName == ".NETFramework,Version=v4.5.2")

Of course the downside of this is that it would require taking a dependency on NuGet.Core (unless simply duplicating NuGet's implementation of GetShortFrameworkName() is preferable).

So.... any thoughts? Am I barking up the wrong tree entirely?

[RichiCoder1]

👍
We've been having a separate discussion about dependency resolution and version strings, and I'm thinking that we might want to just have a general project to make #addin far more robust then it currently is. We really should be grabbing all of the dlls under the appropriate lib folder and all the libs of addin dependencies.

[devlead]

@jrnail23 you're absolutely correct, this is a known issue I've haven't had time to get around to fix yet. Initially we'd like to pick correct DLL based on framework preferably without adding dependencies. Nuget targets are a bit messy but should be doable to get something so we can compare running framework against highest avail matching that.

[jrnail23]

@devlead, It seems like that VersionUtility.GetShortFrameworkName() method doesn't really do any black magic, so it may not be too tough to duplicate its functionality. I can take a stab at it if you like.

[marcosnz]

I took a look at this a while back but only got as far as taking the parsing of the line into own class (see commit above).
It doesn't add any new functionality but verifies current functionality so that if we add to it we will hopefully know if we've broken anything.
Don't know if it will fit into grand scheme of things but thought I'd share what I had done.
EDIT: actually I was heading down a different path - I was going to add a switch to explicitly say which folder I wanted to use e.g.

#addin MyAddin -folder net45

Hence my attacking the argument parsing. Looks like you guys are heading towards an automatic way of getting this.
I may still do my change at some point as I want to handle the -Pre switch (and possibly other NuGet options)

[marcosnz]

Ah, so I got back into this and kept going and now have something that works (see PR). Not sure it will be what you were after. Happy to take feedback... or if you want to use some of the code in a different manner be my guest :)

[jrnail23]

@marcosnz, IMHO, getting that -prerelease switch is key to making this thing more usable. In particular, this is very valuable when developing and testing our own add-ins, as it allows us to follow semantic versioning in development (using prerelease SemVer suffixes for works-in-progress).

I think the -folder switch is less useful, since it requires the consuming Dev to know too much about the (possibly fluid) structure of the nuget package being used. That's also the same objection I have to using #reference/#r.

I'd personally prefer locating the appropriate package at runtime, based on the currently targeted framework, as described above. That being said, it's just an idea I had, and it's obviously wide-open for debate.

[RichiCoder1]

I'm going to be contrarian here and say that cake should take a dependency on NuGet. We already take a soft dependency on it, it's the defacto package manager for C#, and it's basically the defacto package manager for delivering Cake addins as well. We need Cake itself to be smart enough to reason about how to install packages, and where to grab references from. Half done solutions where we're piping arguments to nuget aren't good enough.

[jrnail23]

@RichiCoder1, I tend to agree with you, with one caveat -- which version of NuGet.Core (assuming that's the lib you're talking about) do you take a dependency on?
Managing that dependency over time could be tricky, although that also depends on the degree of coupling we introduce (and of course interfaces & wrappers certainly help mitigate that).

[patriksvensson]

Cake already have a dependency on NuGet.Core. The core library should not have one.

We could define a contract in Cake.Core that wraps functionality around NuGet.Core and implement the contract in Cake.

[marcosnz]

@jrnail23

I think the -folder switch is less useful,... I'd personally prefer locating the appropriate package at runtime

Agreed and that is the fallback if none is selected in my PR. However, the package I am targeting is MSBuild.Extension.Tasks. It has a structure like

I am running from on .net 4.5. Trying to find that I need tools\net40 would be tricky for cake but trivial for me. Hence my switch (now named -VersionFolder).

Having said all that, I tend to agree that taking a dependency on NuGet may be the right way to go but can think of 2 things to consider:

1. At present we can choose which version of Nuget to install
2. I swear I had 2 considerations but now I can't think of the other. :)

Also, my PR is a (fully backwards compatible) working solution which means I can target the package I want in the manner I want (and greatly eases the friction of an addin I have written). So I would ask that it is considered for inclusion for now until a "proper" solution is created.

[RichiCoder1]

@jrnail23 We should target the version that makes the most sense when interoping with Nuget feeds. If we really want the ability to potentially target a newer (or older) nuget, we could make the resolution library external and then have Cake import it so you can swap out your own. Use the interface that @patriksvensson mentioned and MEF and barring any major changes you're good. It basically needs to do two things: Install this package to this folder with this info (version, prerelease, etc...), and tell me what dlls I can/should reference for that package.

[jrnail23]

@RichiCoder1, sounds like good ol' fashioned Dependency Inversion principle, via Separated Interface Pattern, FTW! 😄