[DONT MERGE] support tooling 2.0

[0x53A]

ref https://docs.microsoft.com/en-us/dotnet/standard/library

partial revert of #2380

---

Locally I have a unit test error in #1259 install via script:

Conflict detected:
   - Dependencies file requested package runtime.win10-arm64.runtime.native.System.IO.Compression: >= 4.3
   - Available versions:
     - (4.3.0-preview1-24530-04, [https://www.nuget.org/api/v2])

maybe related to #2514?

[matthid]

Please only include the minimal supported list,
ie if net461 supports netstandard20 we don't need to mark net462 to support netstandard20 as it is already supported via net462 -> net461 -> netstandard20

[matthid]

This section looks wrong somehow ...

[0x53A]

done

[0x53A]

Hi @matthid I would like to continue with this. dotnet cli 2 is nearing release, so paket should be ready.

My first question is: should this remapping be done unconditionally, or should we add a flag somewhere, e.g. to the paket.dependencies file?

Luckily we have the lockfile, so already resolved projects won't break, but this may cause confusion and issues on the next install/update.

I don't yet completely understand how the cli will implement this.

From what I've gathered, downlevel support will require facades that map from the netstandard libraries to mscorlib. In the case of 1.x that is about ~90 small facade dlls (e.g. System.Collections.dll, System.IO.dll, ...). In the case of 2.x, it is a single dll (netstandard.dll).

These facades will be added to net471 inbox, but for the existing netfx versions (4.6.1 -> 4.7.0) an additional nuget package is required.

We will need some level of support for this supporting package, otherwise this will cause a lot of issues (and worse, it will compile fine, but fail at runtime!).

I've come to dislike automagic stuff, so my recommendation would be to emit diagnostics, either failing installation, or at the least a warning, instead of automatically adding the package.

[matthid]

I don't yet completely understand how the cli will implement this.

This is what we need to figure out. I fear that they abuse the runtime graph for this.

[0x53A]

I believe the thinking here is changing again. I think this is getting moved out of a package and into an installer of some kind.

dotnet/standard#342 (comment)

[matthid]

@0x53A What does that mean?

[forki]

It means Microsoft understood that spreading everything in small packages and adding meta packages wasn't a good idea.

[matthid]

I don't understand what that means for us. And in particular what this means for this PR. When is what platform supported and how do we know?

[forki]

It doesn't mean anything right now

[0x53A]

I know it sounds crazy - but from my limited testing I just did, it looks like we don't need to do anything special.

Attached is a sample app which builds and runs successfully using paket: https://github.com/0x53A/n16-net461-test

The output contains all the little shims we need:

Sure, the big question is still - how the dotnet cli do that? And where do they come from?

It seems to be included in the dotnet cli: dotnet/sdk#1267

[matthid]

Well that probably only means that nuget can resolve the missing pieces itself :/

[0x53A]

Well that probably only means that nuget can resolve the missing pieces itself :/

Which is bad from a pure viewpoint, because that resolution would be outside of paket, but good from a pragmatic pov because "stuff works".

[matthid]

@0x53A I think we can finally re-add this mapping from 461 -> netstandard16 into paket
It was never really an issue afaics. It was only an annoyance in #2378 case because of incorrect FSharp.Core packaging (which had an easy workaround by adding System.ValueType dependency).

[matthid]

Another even funnier feature of netstandard20 is the ability to use net461 packages on it, therefore we have a circular deps, this can be enabled by the user with the AssetTargetFallback msbuild variable, very funny stuff.

[matthid]

Took a some minutes to understand the diff again but it looks fine now

[matthid]

@0x53A Is there a reason to not merge this, afaics this should be safe to take (will maybe restart the builds)

[0x53A]

It was never really an issue afaics.

Yes it was. Before Microsoft updated the Sdk, it would succeed at compile time but fail at runtime because the shims were missing. This is now fixed in the Sdk. We may want to investigate how they do that, and whether paket can/should take over resolving these packages, but that can be done unrelated to this PR.

If you have some time, please test how this works with an old-sdk net461 project referencing a netstandard1.6 package. With the new sdk this now works automagically, but no idea about old-sdk.

Is there a reason to not merge this, afaics this should be safe to take (will maybe restart the builds)

We may want to disable this behind a feature toggle. Otherwise this may break people on old sdk, or on old visual studio versions. Resolution may now take netstandard1.6 instead of something else, which may fail at runtime if the support shims are missing.

I think this should be a conscious decision.

We can always enable it if we detect new-sdk version >= 2.0, but for everything else it will break people at runtime.

(I think. Always hard to say without actually verifying it)

[matthid]

I think this should be a conscious decision.

Yes no need to rush anything, just trying to argue.
Lets have some facts/thoughts:

• Yes it is a runtime error, but in practice it is detected fast and the workaround is to add System.ValueType to your packages
• Ultimately this is not and never was a bug in Paket. That this revert to tooling1.0 behavior fixes [5.0.0-beta008] Paket does not add dependencies correctly #2378 is merely a coincidence with how FSharp.Core was packaged. I totally agreed with the "hotfix" of unbreaking people at the time for the greater benefit of the ecosystem and because tooling 2.0 was not relevant at the time.
• In theory you only get "additional" references/packages from paket point of view. So this change should never* break old projects considering how our integration works.
• Keeping Tooling 1.0 behavior means (or should mean = untested) that you cannot consume netstandard20-only packages when targeting net461. I assume this will be a more common situation than the above very soon.
• Most people think netstandard20 will replace netstandard16 very soon.

• There is the exception we saw with FSharp.Core (-> which is obvious we can't protect people from incorrectly packaged packages) and one other exception where nuget generates a failure because a particular package is not compatible. This might happen with netstandard20-only packages when compiling for net461 with new SDK (Maybe this is even a warning only then everything should be fine).

This is now fixed in the Sdk. We may want to investigate how they do that, and whether paket can/should take over resolving these packages, but that can be done unrelated to this PR.

I think what happens is that the SDK adds additional PackageReference objects into the msbuild process. NuGet picks them up later.

We may want to disable this behind a feature toggle. Otherwise this may break people on old sdk, or on old visual studio versions.

I thought about that but never came up with a good way to do that.

[0x53A]

@matthid I'm not talking about that System.ValueTuple package. This will cause a compile time error and I don't actually care about it anyway.

I'm talking about the support packages that provide the net461 <> netstandard1.6 shims. (If they even use packages for that, some other thread on github mentioned a system-wide installation of something. It may be included in the dotnet sdk. The question then becomes whether it is possible in old-sdk projects at all.)

I think what happens is that the SDK adds additional PackageReference objects into the msbuild process. NuGet picks them up later.

Yes, but if you use an old version of Visual Studio, it can't do that. I think this is not actually part of the resolver, but the part that decides which "best matching lib" to reference.

---

For example, say you have two dlls:

lib/foo.dll and and lib/netstandard1.6/foo.dll.

Your project targets net461.

The old paket takes lib/foo.dll. The new paket takes lib/netstandard1.6/foo.dll, which will fail at runtime.

---

I think we should have a toggle at group-level. Or maybe even at project-level (in paket.references), since it does not affect resolve (afaic), only the "best matching group" stuff. Or does it?

And auto-detect of new-sdk v2.0, if that is possible? (I am also fine with dropping support for newsdk 1.x)

How does it work in new-sdk? Afaic we only pass in the packages to the normal nuget pipeline, right? Then we don't need to change anything, nuget/the sdk will take care of it.

This leaves only old-sdk projects.

[matthid]

Ah you are talking about projects without SDK. Thanks :)

So one way of solving this would be to add some msbuild magic at every netstandard16 reference that makes the build fail when the compat package is not found? The lib/foo.dll and lib/netstandard1.6/foo.dll situation is real but putting libs directly in the lib folder is deprecated for a long time and considered as a fallback for ALL frameworks (and to be honest you can't even build such a library in reality). So I consider this particular situation (again) as an incorrectly packaged package.

Or does it?

it definitely does. Framework-restrictions and framework dependency groups are honored by paket.

[0x53A]

So I consider this particular situation (again) as an incorrectly packaged package.

What about

lib/netstandard1.3/foo.dll
lib/netstandard1.6/foo.dll

?

This PR would change the reference from 1.3 to 1.6. Could this break anyone?

This is not a rhetorical question, I really don't know.

How does referencing netstandard1.3 from netfx461 (or lower) even work? Do you also need a support shim, like for ns1.6?

[matthid]

This PR would change the reference from 1.3 to 1.6. Could this break anyone?

Yes it probably would. I think the reason this breaks is because they didn't ship net461 with the required redirects and they need to be distributed with the application itself.

[0x53A]

Yes it probably would. I think the reason this breaks is because they didn't ship net461 with the required redirects and they need to be distributed with the application itself.

That's what I meant by "shims" above. The question is how these are distributed. Nuget package, that is injected by the nuget pipeline in the sdk? Or machine-wide installation and special logic? If it is a nuget package, we should error/warn if it is missing. Or we could auto-inject it, but then the resolution depends on the project TFMs, which I don't like.

[0x53A]

I know. :\

But otherwise this will break everyone still on VS (x < 15.3)

---

A quick hack would be to globally enable this via some environment variable.
But that would mean discrepancies between two installs on two different environments, and probably lots of bug reports.

[Gonnagle]

Would these changes on FrameworkHandling.fs be enough to fix #2705? And if so, would these break anything (for example the VS pre 15.3 support that was discussed)? If not, merging these changes separately could be considered.

[0x53A]

Yes and Yes

[0x53A]

@matthid dotnet/standard#481 (comment)

This relies on a globally installed msi, otherwise it fails silently.

[matthid]

hm I'd like this to stay open for a while for others to see...

[0x53A]

sure. But the more information I get, the more convinced I am we need an explicit group-level switch. And preferably some sanity checks on both install and restore.

[matthid]

You definitely changed my position on this. There is definitely no easy merge on this "hack".

[0x53A]

My biggest gripe currently is their choice of deployment model - a system-wide installed msi.

And the fact that failures seem to be silent - if the CI server does not have this installed, the build succeeds, but the shims are missing and you get runtime failures.

At least that is what my preliminary test suggested (dotnet/standard#481 (comment)).

---

And I'm pretty sure this will also bite nuget developers hard.

Take my contrived example above again:

lib/netstandard1.3/foo.dll
lib/netstandard1.6/foo.dll

If you install this package into a net461 project, the result depends on whether you have this msi installed - and you can't controll it!

So if you have the msi installed, netstandard1.6 is a better match, and it will take that ........... and this will break everyone else who doesn't have the msi installed yet.

---

Take this all with a few grains of salt, I didn't test it too much yet.

[matthid]

You will be our net461 expert forever ;)
I don't even want to understand that hack

[0x53A]

• Add a group level switch
• add a check on install: IF the switch is enabled, then the msi must be installed
• IF the switch is enabled, insert a check into the project file which verifies at compile time that the msi is still installed. because dev and ci may (and will) differ.

[0x53A]

it could of course also be that I misunderstand something, microsoft actually thought this through and there are no issues at all.

I mean I had originally also thought that the F# sdk was completely broken, until kevin helped me understand that pbkac.

There would be no issues (well, none that I can think of currently) if they had gone through with the original plan and deployed everything via some nuget packages.

of course I would probably still have found a reason to bitch about it, even if it was only because these ~100 little dlls pollute your build output (but that is a small non-issue in comparison ;-) ).

---

so let's continue to wait a few days until the dust settles and details become more clear.

If the situation is as bad as I think, then everyone else will hopefully put enough pressure on MS.
If not, we can happily continue, and I can apologise to Immo ;-)

[matthid]

This also means this is not working xplat, but only when building on windows. That would be really really bad.

[0x53A]

This also means this is not working xplat, but only when building on windows. That would be really really bad.

I have no clue. 🌴 🏖 🍸
I guess it would be the job of mono to package an updated msbuild that includes these changes? Do you know anyone we could ask?

[baronfel]

Some useful links:

mono's OSX packaging script for msbuild: https://github.com/mono/mono/blob/master/packaging/MacSDK/msbuild.py

the mono homebrew package: https://github.com/Homebrew/homebrew-core/blob/master/Formula/mono.rb
^ currently doesn't package msbuild at all (PR WIP, see https://github.com/baronfel/homebrew-core/blob/mono5_msbuild/Formula/mono.rb and https://github.com/manchoz/homebrew-core/blob/manchoz_mono_5.2.0.215_fsharp_4.1.25/Formula/mono.rb for attempts to get it in)

mono linux packaging of msbuild: https://github.com/mono/linux-packaging-mono

[miloszes]

Hi.

Do you have any further information about this subject?

Best Regards,
MiloszeS

[0x53A]

Do you have any further information about this subject?

No. Microsoft didn't care about any compatibility issues when they published their updated tooling, but paket should work with older VS versions, and so can't take the same shortcut.

To correctly implement this, the compatibility level would need to be opt-in by the user. This would require more effort from me than I currently want to invest, so unless someone else does the work, it won't be done in the near future.

[miloszes]

Is it possible, to force Paket to link package in csproj (.Net Framework 4.6.1) in case it has a target framework 2.0?
Currently we've found a workaround. We didn't specify the target framework in our package so the Paket has linked this library. Thanks to the csproj entry: true the whole solution works for us, but still this is a workaround :(. Probably we'll cut some function which wanted to store in that package and migrate to .net standard 1.4 :/.

[matthid]

netstandard20 -> net461
If that is what you ask you can see the difficulties discussed here and the related issues.

Downgrade your dependency to netstandard14 is in fact a sensible workaround considering the pain you will run into if you 'force' it (see all the issues on nuget where people try to actually use that feature). Also think about your users if you implement libraries.
Another solution is to upgrade to net47X instead of net461.

It is likely that we in paket sit this one out as we prefer to not break existing users in such a subtle way. But if someone really wants this feature and can implement it with a feature toogle or a nonbreaking way we will happily accept that

[forki]

Ping

[0x53A]

@forki My personal preference is to sit this out and wait for netfx471.

The mapping net461 <> ns2 can imo only be merged with an explicit toggle that is off by default, otherwise everyone else breaks.

[forki]

thanks