Skip to content

Conversation

@baronfel
Copy link
Collaborator

@baronfel baronfel commented Aug 8, 2020

The 37.0.0 release included a pull request from upstream that made brought in explicit dependencies on Microsoft.Build.* packages, which have a minimum TFM of 472. It makes little sense to publish FCS as a net461 library when it requires packages that require net472, so this PR bumps FCS to 472.

If this is accepted I'll contribute these same changes upstream, I just wanted to get a fixed release out quickly.

@dsyme @cartermp @Krzysztof-Cieslak thoughts on this?

@Krzysztof-Cieslak
Copy link
Contributor

Krzysztof-Cieslak commented Aug 8, 2020

is there any need for full framework release at all? If we bump to Net472 we could just use netstandard only - someone targeting net472 can use netstandard20 without any problem

@baronfel
Copy link
Collaborator Author

baronfel commented Aug 8, 2020

Personally I'd would be all in favor of that solution as well. We could still maintain netfx and netcoreapp tests based off that same netstandard assembly if we needed to ensure coverage.

@auduchinok
Copy link
Contributor

We still have to net461 in Rider/R# plugins, so it or netstandard2.0 would be something I'd prefer too. :)

@baronfel
Copy link
Collaborator Author

baronfel commented Aug 8, 2020

@auduchinok does that mean you are broken right now off of dotnet/fsharp@master, since these net472 MSBuild references are required?

@auduchinok
Copy link
Contributor

I've probably missed when it had happened and haven't updated our FCS package yet. Ill need to check it and probably raise an issue there.

@baronfel
Copy link
Collaborator Author

baronfel commented Aug 8, 2020

missed when it had happened

well it was just a couple days ago so that's forgivable :D

@nightroman
Copy link
Contributor

nightroman commented Aug 9, 2020

@baronfel , commenting per your tweet request.

This move is probably not a problem for me, the developer of FSharpFar.

This might be a problem for some users. FSharpFar is a plugin for Far Manager,
portable application, powerful tool for developers and system administrators.
One of the scenarios is brining it all to a machine using a removable drive
and using without installing anything. This includes FSharpFar and its
F# scripts supposed to be invoked on that hypothetical machine.
If this machine does not have net472 installed, it's a hiccup.

OTOH, FSharp.Core seems to be moving to netstandard2.0, per dotnet/fsharp#9801
Users have to adopt and adapt. Maybe it's time to move on.
If this is the case, then FCS should probably do the same.

@nightroman
Copy link
Contributor

nightroman commented Aug 9, 2020

@baronfel May I ask some side questions?

  • Does adding these new packages add some value to FCS? What is it? Are they some new "project cracker"?
  • Are these packages "officially part of FCS"? Can I rely on them included and make some use of them?

@baronfel
Copy link
Collaborator Author

@nightroman thank you for chiming in here :) the answer is 'yes' to both of your questions. They have been added (as best as I understand) to flesh out/enhance/make more correct the reference resolver logic that is already baked into FCS.

There was already a simple reference resolver, and an externally-available one in the associated FCS.MsBuild package, but with MSBuild and its libraries being cross-platform these days it makes sense to bring them into the fold.

@dsyme
Copy link
Contributor

dsyme commented Aug 10, 2020

@dsyme @cartermp @Krzysztof-Cieslak thoughts on this?

I'm ok with moving to netstandard2.0

@auduchinok Any chance of netstandard2.0 becoming an acceptable dependency for Rider?

@auduchinok
Copy link
Contributor

auduchinok commented Sep 2, 2020

@dsyme Sorry for the long reply. Yes, we use netstandard2.0 FCS in Rider now.

We still have to target net461 in the plugin itself, though, but it seems to work fine now: there was a broken case with DependencyManager that @KevinRansom has fixed. I'll just keep reporting if something gets broken again.

@baronfel
Copy link
Collaborator Author

baronfel commented Sep 2, 2020

Lovely news :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants