fsprojects / Paket

A dependency manager for .NET with support for NuGet packages and Git repositories.
https://fsprojects.github.io/Paket/
MIT License
2.02k stars 525 forks source link

How should redirects be created when a package contains different assembly versions for a single assembly? #711

Closed agross closed 9 years ago

agross commented 9 years ago

depfile:

nuget Sample

Sample.nuspec contains <dependency id="Spark" version="[1.8,2.0)" />.

Spark 1.8.0 contains lib\NET45\Spark.dll which has

[assembly: AssemblyVersion("1.7.0.0")]
[assembly: AssemblyFileVersion("1.7.5.3")]
[assembly: AssemblyInformationalVersion("1.7.5.3")]

As far as I know the version the CLR binds to is AssemblyVersion, 1.7.0.0 in this case.

Paket generates this version redirect, though:

<dependentAssembly>
  <assemblyIdentity name="Spark" publicKeyToken="7f8549eed921a12c" culture="neutral" /> 
  <bindingRedirect oldVersion="0.0.0.0-1.8.0.0" newVersion="1.8.0.0" />
</dependentAssembly>
forki commented 9 years ago

/cc @isaacabraham

isaacabraham commented 9 years ago

Hmmm. We use AssemblyVersion to get the version used in the binding redirects file though. Are there other versions of the assembly in the nuget package, which have a different version perhaps? Maybe the binding redirects code is using one of those (incorrectly).

agross commented 9 years ago

WTF, you were right.

lib\NET35\Spark.dll: 1.8.0.0 lib\NET40\Spark.dll: 1.8.0.0 lib\NET45\Spark.dll: 1.7.0.0

My project is 4.5. Seems like the redirects will break when you change the .NET version of your project. At least when package authors decide to use different version numbers for different platforms.

agross commented 9 years ago

It's already reported. https://github.com/SparkViewEngine/spark/issues/54

agross commented 9 years ago

Question is, how would we handle such situations. Should we rather throw or warn and make it explicit that redirects will break than just picking any version?

isaacabraham commented 9 years ago

Yeah, the part of code that determines which version of the assembly to use to pull out the AssemblyVersion isn't aware of which version will ultimately be deployed by Paket. If that was added in, it then might just work.

agross commented 9 years ago

For sure. But only until you change e.g. from 4.5.1 to 4.0 without an additional paket install. :-)

isaacabraham commented 9 years ago

Could we do redirects that are framework-dependent I wonder?

agross commented 9 years ago

I'm not aware of app.config settings depending on the framework version.

Alex

Alexander Groß Tiny phone, tiny mail

On Fri, Mar 20, 2015 at 11:10 PM, Isaac Abraham notifications@github.com wrote:

Could we do redirects that are framework-dependent I wonder?

Reply to this email directly or view it on GitHub: https://github.com/fsprojects/Paket/issues/711#issuecomment-84167164

isaacabraham commented 9 years ago

Not sure there's much we can do then unless we make a custom msbuild task?

agross commented 9 years ago

So redirects would be added at compilation time? Not sure it's worth the effort to work around this obvious bug in Spark. I'm not aware of other packages that deliberately use different version numbers for libs targeting different framework versions.

Alex

Alexander Groß Tiny phone, tiny mail

On Sat, Mar 21, 2015 at 12:13 AM, Isaac Abraham notifications@github.com wrote:

Not sure there's much we can do then unless we make a custom msbuild task?

Reply to this email directly or view it on GitHub: https://github.com/fsprojects/Paket/issues/711#issuecomment-84184166

isaacabraham commented 9 years ago

I'm agree re: cost / benefit. Just pointing out what options are out there :-) Ok with closing this?

forki commented 9 years ago

Yes a warning would be good. On Mar 20, 2015 11:01 PM, "Alexander Groß" notifications@github.com wrote:

Question is, how would we handle such situations. Should we rather throw or warn and make it explicit that redirects will break than just picking any version?

— Reply to this email directly or view it on GitHub https://github.com/fsprojects/Paket/issues/711#issuecomment-84165858.

agross commented 9 years ago

Reopened because of suggested warning.