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

netstandard2.0 references projects which is invalid for netframework #383

Open
simendsjo opened this issue Nov 3, 2020 · 17 comments
Open

Comments

@simendsjo
Copy link
Contributor

Assemblies which has been added out-of-band to nuget, should not be referenced for netframework projects. As netstandard2.0 is usef for both netcore and netframework, adding such assemblies to netstandard, will break netframework projects. The versions referenced, will not be the same versions as those included in netframework, so tools will generate wrong binding redirects for instance. These out-of-band packages is a continuous source of problems for netframework projects.

I'm not up to speed with the current version of the library. I see there's a nuspec in the project, which looks correct, but the nuspec in the nuget repository is wrong

    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="Microsoft.CSharp" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualBasic" version="10.3.0" exclude="Build,Analyzers" />
        <dependency id="Neo4j.Driver.Signed" version="4.1.1" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
        <dependency id="System.ComponentModel.Annotations" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="System.Dynamic.Runtime" version="4.3.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>

Modifying the nuspec file manually and maintaining our own package works, but it's better to fix this upstream

    <dependencies>
      <group targetFramework=".NETStandard2.0">
          <dependency id="Neo4j.Driver.Signed" version="4.1.1" exclude="Build,Analyzers" />
          <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETCoreApp2.0">
        <dependency id="Neo4j.Driver.Signed" version="4.1.1" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
        <dependency id="Microsoft.CSharp" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualBasic" version="10.3.0" exclude="Build,Analyzers" />
        <dependency id="System.ComponentModel.Annotations" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="System.Dynamic.Runtime" version="4.3.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>
@cskardon
Copy link
Member

cskardon commented Nov 3, 2020

Hi @simendsjo

I'm not 100% sure what needs to happen - is this on the 4.x version of the client?

Thanks!

@simendsjo
Copy link
Contributor Author

Yes, it's the 4.x client I'm trying to upgrade to.

I know what the problem is, but I'm not sure what the best fix is -- the situation is chaotic. Everyone "does the right thing", but because of Microsofts failed out-of-band updates, special care needs to be taken to not break netframework.

The problem with having these in netstandard is that they refer to versions that doesn't exist in netframework, and netframework cannot mix and match these core packages from nuget and netframework. Microsoft tried that with System.Net.Http, which caused a lot of pain.

netstandard is supposed to be the common ground for netframework and netcore, but the versions available in nuget and netframework doesn't match.

My proposed fix here will at least work for netframework as it picks up netstandard, and it only contains packages which should be used from nuget, and not the packages which should be used from framework. Will it cause problems for other netstandard implementations..? Maybe? Probably? Do we care? The big players for this library is probably netcore and netframework, so at least those needs to be working.

Maybe a better fix will be a custom block for netframework instead of a block for netcore as it's (hopefully) just netframework which has these issues?

@simendsjo
Copy link
Contributor Author

simendsjo commented Nov 4, 2020

EDIT: Looks like these will add references to the project files. The real fix seems to be in having it's own section for netframework to avoid adding the System.Net* packages. So this will be the "Maybe a better fix.."-approach I mentioned last.


Hmm, I see EventStore has a section using frameworkAssemblies in the nuspec file. Maybe this is Microsofts recommended workaround for this issue?

    <dependencies>
      <group targetFramework=".NETFramework4.5.2">
        <dependency id="Newtonsoft.Json" version="11.0.2" exclude="Build,Analyzers" />
        <dependency id="protobuf-net" version="2.4.0" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETFramework4.6">
        <dependency id="Newtonsoft.Json" version="11.0.2" exclude="Build,Analyzers" />
        <dependency id="protobuf-net" version="2.4.0" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="Newtonsoft.Json" version="11.0.2" exclude="Build,Analyzers" />
        <dependency id="System.Net.Http" version="4.3.4" exclude="Build,Analyzers" />
        <dependency id="System.Net.Requests" version="4.3.0" exclude="Build,Analyzers" />
        <dependency id="System.Net.Security" version="4.3.2" exclude="Build,Analyzers" />
        <dependency id="protobuf-net" version="2.4.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>
    <frameworkAssemblies>
      <frameworkAssembly assemblyName="System.Net.Http" targetFramework=".NETFramework4.5.2, .NETFramework4.6" />
      <frameworkAssembly assemblyName="System.Net.Http.WebRequest" targetFramework=".NETFramework4.5.2, .NETFramework4.6" />
    </frameworkAssemblies>

@cskardon
Copy link
Member

cskardon commented Nov 4, 2020

OK, so just so I think I've got this right in my head.

The version of System.Net.Http referenced in the .NETStandard2.0 group doesn't exist for the .NETFramework's and the frameworkAssembly element basically says that for the netframework versions - pull the GAC versions.

The problem is caused because when you try to use Neo4jClient on a Framework 4.6 codebase, it tries to pull down other packages that don't exist - and as such can't be added?

@simendsjo
Copy link
Contributor Author

As EventStore is correctly defined, this isn't what happens. For netframework,
only Newntonsoft.Json and prototbuf-net is referenced. frameworkAssemblies state
that installing the package should also add the two project references
System.Net.Http and System.Net.Http.WebRequest, and these will be picked up from
the GAC.

If on the other hand .NETFramework4.6 wasn't defined, as is the case for
Neo4jClient and Neo4j.Driver, it's more like you state. Nuget/paket will see
that the nuget packages is a dependency. It will download these packages, and
use their versions for assemblyredirects. The packages from nuget are the same
as the ones in netframework (same public key), but does not follow the same
release cycle. (System.Net.Http is a special case that's even more evil as they
tried to use the nuget version for netframework is order to give value to
netframework users without waiting for the next framework version.)

In effect, you're saying things like "download and use library A v2 from nuget",
but this isn't a valid version for that netframework version. The assembly from
nuget isn't used, but the netframework version. That verison is maybe v1 og v3.
So when the application loads, there's a redirect to v2, which is what the
nuspec told us to use, but only A v1 (or v3 or whatever) exists in the
netframework version.

So while netframework implements netstandard, it doesn't necessarily have the
same versions as the ones existing in nuget. By specifying the nuget version of
these packages, we start specifying specific versions which might not be
available for netframework even though it looks that way if you look in nuget as
those libraries implements netstandard.

... so it's a messy situation. As far as I understand, Microsoft has given up
trying to find a fix for the issue, and they are not even fixing some of their
own packages. I had to poke the Azure team several times for one of their
packages. And it's an error everyone is doing, because everyone is "doing the
right thing". Our local nuget repository has several packages where all the
changes we've done is removing various packages from the nuspec reference to
avoid the alternative, which is avoiding redirects for certain (often
transitive) dependencies per project.

And even though it's an issue for all of framework, you only get into the
situation when you use redirects, and only when the versions differ. And for
simpler projects, you might even get away with excluding redirects for
transitive dependencies, and thus never generate redirects for the System*
assemblies. But when you're using several libraries, which all can reference a
different version of a transitive dependency, you need a redirect. Manually
maintaining redirects is a blast from the past I don't want to get back to :)

Hopefully NET5 will make it easier for us to upgrade so I no longer care about
these issues :)

@simendsjo
Copy link
Contributor Author

@cskardon
Copy link
Member

OK, so, the CI is using dotnet pack which I'm assuming isn't using the .nuspec file.
Which means it's pulling from the .csproj

But I still don't know how to fix this. I don't know what to do that would get it working. I'm also confused by your mention of EventSource what is that coming from?

I'd like to help, but I don't know how.

@simendsjo
Copy link
Contributor Author

OK, so, the CI is using dotnet pack which I'm assuming isn't using the .nuspec file.
Which means it's pulling from the .csproj

Nuget will first look for a nuspec file, then a csproj file. But your CI is probably using the csproj file directly instead?

$ nuget pack
Attempting to build package from 'Neo4jClient.nuspec'.
An error occured while trying to parse the value '' of property 'version' in the manifest file.
  Value cannot be null or an empty string.
  Parameter name: value

But I still don't know how to fix this. I don't know what to do that would get it working.

Is the CI publically available? My guess is the same as yours - the csproj file is used instead of the nuspec file. Specifying the nuspec file directly might solve it.

We have build steps like the following in our CI
image

I'm also confused by your mention of EventSource what is that coming from?

I'm just referring to other nuget packages for reference, but don't mind that. We need to avoid referencing nuget packages for assemblies which exists in netframework for netframework projects. God knows how many hours of my life Microsoft has wasted on this issue :(

@cskardon
Copy link
Member

OK, if I get the chance today, I'll play with a pre-release version and see if that can use the nuspec properly.

@simendsjo
Copy link
Contributor Author

Did you get around to test this?

@cskardon
Copy link
Member

Not in the CI - largely as I've gone through the .nuspec and it's not fine - the references are incorrect - do you have a way I can test the package generated - i.e. an sln that would cause the issue with the current package, so I can test the nuspec one?

i.e. is it just a NET 4.6.2 project with references from Neo4jClient and...?

@simendsjo
Copy link
Contributor Author

i.e. is it just a NET 4.6.2 project with references from Neo4jClient and...?

You need to create a redirect for the version your nuspec targets and then try to use the version from framework. In the following project I did "New ASP.NET project", "paket convert-from-nuget", added "redirects:force", and then added "Neo4jClient". After adding Neo4jClient, it no longer works as Neo4j specifies a version from nuget which isn't the same version as exist in netframework: simendsjo/neo4jclient-netframework-package-problems@8d74a28

@cskardon
Copy link
Member

Would you be able to try https://www.nuget.org/packages/Neo4jClient/4.1.5-experimental-5 ?

@simendsjo
Copy link
Contributor Author

Would you be able to try https://www.nuget.org/packages/Neo4jClient/4.1.5-experimental-5 ?

It will still clash with netframework.

Our local hack is to replace this in the nuspec file

    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="Microsoft.CSharp" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualBasic" version="10.3.0" exclude="Build,Analyzers" />
        <dependency id="Neo4j.Driver.Signed" version="4.2.0" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
        <dependency id="System.ComponentModel.Annotations" version="5.0.0" exclude="Build,Analyzers" />
        <dependency id="System.Dynamic.Runtime" version="4.3.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>

With this

    <dependencies>
      <group targetFramework="net472">
        <dependency id="Neo4j.Driver.Signed" version="4.2.0" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="Microsoft.CSharp" version="4.7.0" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualBasic" version="10.3.0" exclude="Build,Analyzers" />
        <dependency id="Neo4j.Driver.Signed" version="4.2.0" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.3" exclude="Build,Analyzers" />
        <dependency id="System.ComponentModel.Annotations" version="5.0.0" exclude="Build,Analyzers" />
        <dependency id="System.Dynamic.Runtime" version="4.3.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>

I guess "net45" should also work and will work for more than just net472.

@cskardon
Copy link
Member

Hmmmm

It's packaged from the .nuspec file using:

nuget pack neo4jclient.nuspec ...

Which looks like what you have,

https://www.nuget.org/packages/Neo4jClient/4.1.5-experimental-7

Is the most recent one, which has all of that. I don't know how to get nuget to pick it up

@simendsjo
Copy link
Contributor Author

Maybe it help to add the framework to TargetFrameworks in the csproj file? The netstandard build should work on netframework, but perhaps nuget does some filtering here?

@cskardon
Copy link
Member

Hmmm, that is proving complicated
But - it might be something to try, I'll see if I can get another experimental version up with it in the .csproj

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

No branches or pull requests

2 participants