C#: add System.Memory dependency for net45 too

[jtattermusch]

followup for https://github.com/protocolbuffers/protobuf/pull/5835/files.

[jtattermusch]

CC @mgravell @jskeet

[jskeet]

While I welcome the ability to build netX targets on Linux, I don't think we should depend on preview NuGet packages for a release build.

[jtattermusch]

While I welcome the ability to build netX targets on Linux, I don't think we should depend on preview NuGet packages for a release build.

Is that a problem? It's a PrivateAssets="All" dependency, so there should be no traces of it in the released nuget.

[jskeet]

The trace is in terms of the assembly references etc. It's not like an analyzer, where it doesn't affect the eventual binary. If something is broken in this NuGet package, we could end up with a broken release.

[mgravell]

I'd be surprised if the "pack" step even allowed this. But yeah, I'd second what Jon says there.

[jskeet]

Not quite sure why I've now been assigned as a reviewer, but I have the same concerns that I expressed before.

[jtattermusch]

I asked Microsoft folks offline and this is the reaction I got:

I asked Daniel about this package. I didn't get an ETA about when it will be final, but referencing it should be fine. There are some bugs, but if your build succeeds and unit tests pass then you're not effected.

Alternatively I think you can use a preview of the 3.0 SDK, and it will automatically include the reference - dotnet/designs#33 (comment)

Using private preview packages isn't that big of a deal. The Source Link package is still preview and many projects reference it to enable Source Link: https://github.com/grpc/grpc/blob/5e6a9d997719a0a511d4f1033e583eb6f54c3dc0/src/csharp/Grpc.Core/SourceLink.csproj.include#L11

Based on that, I think using the preview package should be relatively safe and simplifying our build by getting rid of build hacks is worth it. This PR would also unblock some followup work.

[mgravell]

Just to drop an alternative; I have increasingly seen a trend towards "baseline at NS2.0; drop TFMs below that; add TFMs above that if-and-only-if you can exploit specific TFM-specific APIs". This means that net462 would become the effective lowest supported version (net462 can consume netstandard2.0 libs, although it can sometimes get angry about it). I don't know your thoughts on this, but it is something that should at least be considered, especially if you're in the process of making a breaking release anyway.

I used to be firmly on the "I'll keep supporting down-level versions of .NET Framework as long as it doesn't massively inconvenience me", but I'm increasingly leaning towards the wisdom of a hard stop at NS2.0; I've recently applied this to "Dapper", and I'll probably do the same with protobuf-net in the next "major".

[mgravell]

@jtattermusch IMO there's a difference between taking a dependency on preview build-time tools vs preview runtime libraries; IIRC a lot of the tooling really doesn't want you to release a non-preview package that targets preview libraries.

Example (from another library I had locally):

error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "System.Reflection.Emit.Lightweight [4.6.0-preview8.19405.3, )" or update the version field in the nuspec. [C:\Code\Dapper\Dapper\Dapper.csproj]

So: if this is only for the build, then: fine, no problem. If it changes the runtime dependencies it could be problematic.

[jtattermusch]

@jtattermusch IMO there's a difference between taking a dependency on preview build-time tools vs preview runtime libraries; IIRC a lot of the tooling really doesn't want you to release a non-preview package that targets preview libraries.

Example (from another library I had locally):

error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "System.Reflection.Emit.Lightweight [4.6.0-preview8.19405.3, )" or update the version field in the nuspec. [C:\Code\Dapper\Dapper\Dapper.csproj]

So: if this is only for the build, then: fine, no problem. If it changes the runtime dependencies it could be problematic.

I'm aware of the potential problem with depending on pre-release packages, but in this case, it's a build-only dependency (there's a PrivateAssets="All"). We've been using the dependency for Grpc.Core and so far there hasn't been any trouble with it.

[jtattermusch]

Just to drop an alternative; I have increasingly seen a trend towards "baseline at NS2.0; drop TFMs below that; add TFMs above that if-and-only-if you can exploit specific TFM-specific APIs". This means that net462 would become the effective lowest supported version (net462 can consume netstandard2.0 libs, although it can sometimes get angry about it). I don't know your thoughts on this, but it is something that should at least be considered, especially if you're in the process of making a breaking release anyway.

I used to be firmly on the "I'll keep supporting down-level versions of .NET Framework as long as it doesn't massively inconvenience me", but I'm increasingly leaning towards the wisdom of a hard stop at NS2.0; I've recently applied this to "Dapper", and I'll probably do the same with protobuf-net in the next "major".

For Google.Protobuf, we're on the side of maintaining compatibility with older framework as long as this doesn't become very problematic. (Protobuf has traditionally been very backward-compatible and so far we didn't have a good enough reason to change that. Also, we have no plans to do release a new major version any time soon).
I think i'd be willing to drop support for older frameworks once usage statistics show that vast majority of users have already abandoned the older frameworks (I don't have such data at this point though).

[jtattermusch]

@anandolee can you review?

[mgravell]

(I don't have such data at this point though)

Acknowledged - it doesn't impact this piece (agree it looks safe), but yeah, that's a really hard question and I'd really like a data-bound answer to it; nothing to do with gRPC - just: I'd really really like to see numbers. I've pinged a few folks to see if I can try to get some, but I expect the answer is "nobody knows for sure".

[anandolee]

Chat with Jan offline, according to the nuget owners at microsoft this should be relatively safe.