Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce FSharp.Core dep version? #169

Closed
bartelink opened this issue Apr 6, 2024 · 9 comments · Fixed by #172
Closed

Reduce FSharp.Core dep version? #169

bartelink opened this issue Apr 6, 2024 · 9 comments · Fixed by #172
Milestone

Comments

@bartelink
Copy link
Contributor

See #166 (comment) TL;DR could the FSharp.Core dep be lowered back down to 6.0.7 (or lower) please?

I'll make a PR soon, unless this is being done intentionally for some reason?


Updating the FSharp.Core causes a lot of run on implications
For example: fsprojects/Argu#237
This is not dissimilar to how FsHttp also did a similar update: fsprojects/FsHttp#183
FSharp.AWS.DynamoDB and other low level components depend on unquote
TaskSeq follows this approach too: fsprojects/FSharp.Control.TaskSeq#123

My personal stuff depends on 6.0.7 for two reasons

  • there's a bug fixed in there where the process can be torn down without a stacktrace under Async.Parallel.
  • I used Async.Parallel in an internal implementation

NOTE: I do get that there's no actual reason why some old thing from 10 yars ago can't 'Just' update it's FSharp.Core from 4.3.2 to 8.0.100 without expecting it to just work, but that's not the point - it should simply depend on the lowest version possible

@bartelink bartelink changed the title Reduce FSharo.Core dep? Reduce FSharo.Core dep version? Apr 6, 2024
@bartelink bartelink changed the title Reduce FSharo.Core dep version? Reduce FSharp.Core dep version? Apr 6, 2024
@stephen-swensen
Copy link
Contributor

stephen-swensen commented Apr 6, 2024

Hi @bartelink - I haven't looked at this closely yet, but some thoughts:

  • I generally agree with the sentiment: packages should strive to target lowest version of FSharp.Core possible (though I say that with reservations about getting so far behind that it becomes difficult to upgrade, or you miss out on important bug fixes like Async.Parallel you mentioned, or you become more likely to have your dependency surface a security vulnerability)
  • But in the case of Unquote, there is a motivation to keep fairly up-to-date with newer versions of FSharp.Core, so that we ensure we cover new language features and quotation representations that are implemented in FSharp.Core (e.g. dd89b44#diff-c9235d7d565e02f168df2f48c7217f467fe691dc7d9e08725c68d98e8f277116R111)
  • Unquote's public surface area is mature and doesn't change much, for example there were no public changes introduced between Unquote 6.1.0 and 7.0.0. We just expanded support for newer language features and... latest FSharp.Core. So I wonder if you are already constraining yourself to older FSharp.Core version, why not also constrain yourself to an older Unquote version?

Just thinking out loud. It might be that at this point in time, FSharp.Core 6.0.7 is indeed a sweet spot. I think I'd need to have a better understanding of what changes there are between FSharp.Core 6.0.7 and 8.0.100, specifically with regard to quotations to make sure we don't limit our feature set coverage.

@bartelink
Copy link
Contributor Author

Hm, good points. It's admittedly a pretty wierd thing to be concerned about as being on the latest in the context of an app is a no-brainer. I guess it's libs like Argu and FsAwsDdb that wind up with the real choices to make. I'm reluctant to up e.g. Argu's requirements as you don't know what upping the requirement winds up transitively causing a mess - I just know that the FsHttp thing cost us time (though it was the incorrect nuspect that actually caused that pain).

Obviously multitargeting is definitely something to avoid, but as an idea, doing a netstandard build that only argets v4 or v6 and then e.g. a net6.0-specific TFM build that demands 8.0 might be a middle way.

For me, the main issue is that I use unquote for tests, and I have them on latest. Amusingly, latest for me is net6.0 atm. So that probably means I'm some bugfixes behind.
For Argu, that means dependabot spam like the one I cited, which is suggesting to up the lib's dependency (which is not something I want), but being up to date for tests is fine. Anyway that's more about me and depenadabot than unquote.

One final thing though:

or you miss out on important bug fixes like Async.Parallel you mentioned, or you become more likely to have your dependency surface a security vulnerability

I think that misses the point though - apps should have the latest of everything; and for libs, forcing that is rarely a consideration (I was just giving an example of an exception to the rule, but then I also hadn't even considered the need to cover new language things as I typed that)

@bartelink
Copy link
Contributor Author

I did the basic PR in #170
FWIW it also compiles against 4.7.2, as 6.x did (and 4.3.2 as Argu does)
I've just noticed that FSharp.AWS.DynamoDB does not pin FSharp.Core - I'm off to make a PR to make that as conservative as possible...

@stephen-swensen
Copy link
Contributor

Hi @bartelink - sorry for letting this go stale. I gave it a go but ran into difficulties trying to downgrade the FSharp.Core dependency. Some tests were failing and I started going down the multi-targeting path and it just got too complex. My feeling is that the complexity outweighs the benefit at this point, so will close this issue for now.

@bartelink
Copy link
Contributor Author

Thanks for making the attempt. I can well appreciate making the build a mess for a short term gain is a step too far.

Is there something special in 8.0 specifically? FSharp.AWS.DynamoDB (which depends on unquote) and lots of other libs [that don't] make do with 6.0.7. It and transitive dependencies of it are ultimately where this comes into play, so if 8.0.100 is where it's at, that's where fsprojects/FSharp.AWS.DynamoDB#75 will need to go as there's not point in it relying on an unmaintained versiion of unquote - that'll just cause people pain with vuln scanners in the medium to long term.

@stephen-swensen
Copy link
Contributor

OK - I put a little more effort to it and it didn't turn out too bad: #172

Look good?

I went with the multi-targeting approach. I think as long as we don't get into a situation where there is some new F# language feature that depends on a newer version of FSharp.Core in such a way that it makes it difficult to continue supporting FSharp.Core 6.0.0 it will be OK. And if that time comes we can again ask ourselves "what is the min version FSharp.Core we can support while not making maintenance too burdensome or being held back".

@stephen-swensen
Copy link
Contributor

And thank you for your efforts and contributions, I appreciate your interest in improving Unquote and the F# ecosystem at-large!

@stephen-swensen stephen-swensen added this to the 7.0.1 milestone Nov 30, 2024
@bartelink
Copy link
Contributor Author

Wonderful, thanks so much!

@stephen-swensen
Copy link
Contributor

Released!: https://www.nuget.org/packages/Unquote/7.0.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
2 participants