Giraffe: Remove dependency on deprecated packages

[forki]

https://docs.microsoft.com/en-us/aspnet/core/migration/22-to-30?view=aspnetcore-3.0&tabs=visual-studio#remove-obsolete-package-references

Giraffe (4.0)
  FSharp.Core (>= 4.7) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.AspNetCore.Authentication (>= 2.2) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.AspNetCore.Authorization (>= 3.0) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.AspNetCore.Diagnostics (>= 2.2) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.AspNetCore.Hosting.Abstractions (>= 2.2) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.AspNetCore.ResponseCaching (>= 2.2) - restriction: || (>= net461) (>= netstandard2.0)
  Microsoft.IO.RecyclableMemoryStream (>= 1.2.2) - restriction: || (>= net461) (>= netstandard2.0)
  Newtonsoft.Json (>= 12.0.2) - restriction: || (>= net461) (>= netstandard2.0)
  System.Text.RegularExpressions (>= 4.3.1) - restriction: || (>= net461) (>= netstandard2.0)
  System.ValueTuple (>= 4.4) - restriction: >= net461
  System.ValueTuple (>= 4.5) - restriction: && (< net461) (>= netstandard2.0)
  System.Xml.XmlSerializer (>= 4.3) - restriction: || (>= net461) (>= netstandard2.0)
  TaskBuilder.fs (>= 2.1) - restriction: || (>= net461) (>= netstandard2.0)
  Utf8Json (>= 1.3.7) - restriction: || (>= net461) (>= netstandard2.0)

so please remove deps to Microsoft.AspNetCore.*

[slang25]

That would require us to switch completely over to netcoreapp3.0, dropping netstandard2.0 and net4x, might be the right time to do it.

[forki]

Go for it.

Stuart Lang [e-mail] schrieb am Mo., 30. Sep. 2019, 10:15:

That would require us to switch completely over to netcoreapp3.0,
dropping netstandard2.0 and net4x, might be the right time to do it.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/giraffe-fsharp/Giraffe/issues/377?email_source=notifications&email_token=AAAOANBWXOFJSXJAF7H4QITQMGYSHA5CNFSM4I3XNI3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD742JAI#issuecomment-536454273,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAOANFYUJQ7VGVBTZVBHC3QMGYSHANCNFSM4I3XNI3A
.

[slang25]

@forki WIP here: #378

[dustinmoris]

Maybe we can keep support for net461 and netcoreapp2.0 for just a bit longer as not everyone has immediate time to switch to the latest framework, but otherwise I agree let's always make sure Giraffe is on the latest with all the recommendations and deprecate support for the other frameworks at the latest by the time MS also stops supporting them. For now, I'd suggest to remove these packages through conditions in the .fsproj file and #ifdefs in code where necessary.

[slang25]

Agreed, updated: #378

[forki]

ok serious question: is there anyone not using netcore for giraffe?

[slang25]

I would bet not a single soul 😆 Here it doesn't cost us anything on top of ns2.0 IMO

[dustinmoris]

@forki I hope not :), but you never know!

[forki]

Yes but those can happily use giraffe 4. Forever

Dustin Moris Gorski [e-mail] schrieb am Mo., 30. Sep.
2019, 11:51:

@forki https://github.com/forki I hope not :), but you never know!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/giraffe-fsharp/Giraffe/issues/377?email_source=notifications&email_token=AAAOANFNC2O37YLKLWDY32DQMHD3PA5CNFSM4I3XNI3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD75C55Y#issuecomment-536489719,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAOANABEQBELZNDP6MU32DQMHD3PANCNFSM4I3XNI3A
.

[maxdeviant]

ok serious question: is there anyone not using netcore for giraffe?

I didn't even realize you could use Giraffe with .NET Framework.

[NinoFloris]

netcoreapp 2.1 will be LTS for another 2 years, I think that should be the minimum bar.

[forki]

@NinoFloris it doesn't seem possible to support both frameworks with one giraffe package.

[NinoFloris]

Care to elaborate? @forki

[forki]

for netcore 2.2 we need to reference those packages from giraffe.
for netcore 3 we should not because they no longer exist for that version

so we could in theory add different dependency groups for netcoreapp3 and nectoreapp2.2 in the nuspec of Giraffe. But how does this work with being netstandard2.0?

It's pretty much fucked up.

[forki]

/cc @Krzysztof-Cieslak since the same applies to Saturn

[slang25]

It's not great I'll admit, but do you have any particular objection to this:
https://github.com/giraffe-fsharp/Giraffe/blob/develop/src/Giraffe/Giraffe.fsproj#L46-L56

[forki]

The problem is that we are netstandard lib. It's totally unclear what's
happening downstream

Stuart Lang [e-mail] schrieb am Do., 17. Okt. 2019, 12:21:

It's not great I'll admit, but do you have any particular objection to
this:

https://github.com/giraffe-fsharp/Giraffe/blob/develop/src/Giraffe/Giraffe.fsproj#L46-L56

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/giraffe-fsharp/Giraffe/issues/377?email_source=notifications&email_token=AAAOANFBRHKIBCYP2Y3VJCLQPA4DXA5CNFSM4I3XNI3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBPTCDA#issuecomment-543109388,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAOANCTOWPEWBDY72BLIKTQPA4DXANCNFSM4I3XNI3A
.

[NinoFloris]

As I see it it's pretty 'simple' netcoreapp3.0 has precendence over netstandard2.0.

Hence netstandard should just references all the packages like it did, and netcoreapp3.0 should only have a framework reference + supplemental extension packages etc.

That's exactly how it's setup right now.

EDIT:
Downstream packages take the TFM deps from giraffe for the TFM they're building for afaik, that should all work (for newestget at least).

[forki]

That's not completely correct. There are compiled references into concrete dlls. The platform resolver needs to find the exact corresponding dlls. And it's up to luck that this works.

That said: we should release a version with the proposed changes so that we can try out if it works. Also saturn will need it

[forki]

My assumption is that the proposed change will just work as long as the sdk ships dlls witj same name and same signatures of the stuff that we use

[NinoFloris]

What's not completely correct? It's a simplification of all the involved parts but I believe it's not wrong.

The fact you can have restore resolving different concrete dll deps for Giraffe that are not actually referenced in the concrete Giraffe dll for that TFM is an implementation detail and a bug if it goes wrong no?

[forki]

Giraffe compiles against something in aspnet 2.2. As in calling a method in
somthing.dll. There is no guarantee that this method exists in 3.0. It's
not even clear if something.dll even exists anymore.

Nino Floris [e-mail] schrieb am Do., 17. Okt. 2019, 12:38:

What's not completely correct? It's a simplification of all the involved
parts but I believe it's not wrong.

The fact you can have restore resolving different concrete dll deps for
Giraffe that are not actually referenced in the concrete Giraffe dll for
that TFM is an implementation detail and a bug if it goes wrong no?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/giraffe-fsharp/Giraffe/issues/377?email_source=notifications&email_token=AAAOANB4LXGRZKU5M3VUKKLQPA6B7A5CNFSM4I3XNI3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBPUKNI#issuecomment-543114549,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAOANELNLTU7Z5NUDC3TA3QPA6B7ANCNFSM4I3XNI3A
.

[NinoFloris]

That's why we multi target though.

And to continue my previous comment, multi targetting would be entirely useless if restore didn't at least try to keep restore deps (which ends up in publish) vs concrete dll deps mismatches from happening.

Now the fact FrameworkReferences aren't actually participating in version unification (in nuget) is a whole other fun part and I consider it very bad. It means that packages you reference in your project that aren't already recompiled for netcoreapp3.0 break at runtime when you have a FrameworkReference in your proj (courtesy of web sdk). As it won't warn you during restore that FrameworkReference Microsoft.AspnetCore.App (shipped with sdk 3.0) is not compatible with package that references Microsoft.AspnetCore.App 2.2 (the package on nuget). That part is garbage...

[forki]

The thing is: we should try it out. I'd appreciate a new release and
volunteer for testing it immediately.

Nino Floris [e-mail] schrieb am Do., 17. Okt. 2019, 12:47:

That's why we multi target though.

And to continue my previous comment, multi targetting would be entirely
useless if restore didn't at least try to keep restore deps (which ends up
in publish) vs concrete dll deps mismatches from happening.

Now the fact FrameworkReferences aren't actually participating in version
unification (in nuget) is a whole other fun part and I consider it very
bad. It means that packages you reference in your project that aren't
already recompiled for netcoreapp3.0, and you have a FrameworkReference in
your proj (courtesy of web sdk) it won't warn you during restore that
FrameworkReference Microsoft.AspnetCore.App is not compatible with package
that references Microsoft.AspnetCore.App 2.2 as a package reference. That
part is garbage...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/giraffe-fsharp/Giraffe/issues/377?email_source=notifications&email_token=AAAOANEHF7FFWNYF65VY5E3QPA7DFA5CNFSM4I3XNI3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBPVAVQ#issuecomment-543117398,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAOANAH2PQ3FVYAVS4YRJDQPA7DFANCNFSM4I3XNI3A
.

[NinoFloris]

What should be changed? @slang25 your changes went in during 4.0 right? @forki does 4.0 work with newestget on core 3.0?

[slang25]

No, we need to do another release to satisfy @forki concerns.

[forki]

@Ninofloris the current released package unconditionally references 2.2 stuff.

[NinoFloris]

Ah so #378 just didn't make it. @dustinmoris mind releasing a patch with #378 in there?
I think we should also deprecate the 4.0.0 package afterwards, it'll never work properly on 3.0 without the conditional refs.

[slang25]

Looks like @dustinmoris has done a new release:
https://github.com/giraffe-fsharp/Giraffe/releases/tag/v4.0.1

Thanks Dustin 👍

[forki]

looks like it's working. Thanks

[forki]

Corresponding TechEmpower PR: https://github.com/TechEmpower/FrameworkBenchmarks/pull/5162

[atlemann]

Is it possible to do these conditional package references with Paket?

[forki]

If you do manual template files.