Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Updated FCS to latest #3564

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Updated FCS to latest #3564

merged 2 commits into from
Oct 24, 2023

Conversation

ncave
Copy link
Collaborator

@ncave ncave commented Oct 23, 2023

@ncave ncave marked this pull request as draft October 23, 2023 15:22
@ncave ncave marked this pull request as ready for review October 23, 2023 17:05
@ncave
Copy link
Collaborator Author

ncave commented Oct 23, 2023

@nojaf
I had to downgrade some dependencies to make the latest FCS work on .NET 6.0 as well.
Otherwise it would only work on .NET 7.0/8.0.

src/Fable.Cli/CHANGELOG.md Outdated Show resolved Hide resolved
@nojaf
Copy link
Member

nojaf commented Oct 23, 2023

Thanks for this @ncave!

@ncave ncave requested a review from MangelMaxime October 23, 2023 18:03
Copy link
Member

@MangelMaxime MangelMaxime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the tests are green, it looks good to me.

Should we keep the PR open and wait for the official .NET 8 release?

Or because everything seems good, should it be included in the next Fable release?

@nojaf
Copy link
Member

nojaf commented Oct 23, 2023

It might be interesting to have an alpha of this if that is possible.
@abonie has implemented dotnet/fsharp#14640 and I believe Fable was a good use-case for this.

People could play around with this ahead of the official release.

@ncave
Copy link
Collaborator Author

ncave commented Oct 23, 2023

@MangelMaxime Feel free to merge before next release, if you're ok with it.

@MangelMaxime MangelMaxime merged commit cfb6d68 into fable-compiler:main Oct 24, 2023
14 checks passed
@nojaf
Copy link
Member

nojaf commented Oct 25, 2023

Hi @ncave

I had to downgrade some dependencies to make the latest FCS work on .NET 6.0 as well.

Would it be possible to do this as well for the other projects in FSharp.Compiler.Service.sln.
I'm no longer able to build the solution in your fork:

C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
 Warning As Error: Detected package downgrade: System.Collections.Immutable from 7.0.0 to 6.0.0. Reference the package
directly from the project to select a different version.  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> FSharp.Build -> Microsoft.Build.Tasks.Core 17.7.0-preview-2
3217-02 -> System.Collections.Immutable (>= 7.0.0)  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> System.Collections.Immutable (>= 6.0.0) [C:\Users\nojaf\Pro
jects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
 Warning As Error: Detected package downgrade: System.Reflection.Metadata from 7.0.0 to 6.0.1. Reference the package di
rectly from the project to select a different version.  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> FSharp.Build -> Microsoft.Build.Tasks.Core 17.7.0-preview-2
3217-02 -> System.Reflection.Metadata (>= 7.0.0)  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> System.Reflection.Metadata (>= 6.0.1) [C:\Users\nojaf\Proje
cts\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
 Warning As Error: Detected package downgrade: System.Collections.Immutable from 7.0.0 to 6.0.0. Reference the package
directly from the project to select a different version.  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> FSharp.Build -> Microsoft.Build.Utilities.Core 17.7.0-previ
ew-23217-02 -> System.Collections.Immutable (>= 7.0.0)  [C:\Users\nojaf\Projects\fsharp\FSharp.Compiler.Service.sln]
C:\Users\nojaf\Projects\fsharp\tests\FSharp.Compiler.Service.Tests\FSharp.Compiler.Service.Tests.fsproj : error NU1605:
  FSharp.Compiler.Service.Tests -> FSharp.Test.Utilities -> System.Collections.Immutable (>= 6.0.0) [C:\Users\nojaf\Pro
jects\fsharp\FSharp.Compiler.Service.sln]

I'm still able to build the compiler project.

@ncave
Copy link
Collaborator Author

ncave commented Oct 25, 2023

@nojaf

I'm no longer able to build the solution in your fork

Are you trying to build it with .NET 6.0? I haven't tried building those other projects in the solution.

I don't think you can build FCS with .NET 6.0, or at least not anymore, without making some changes.
It has to be build with .NET 8.0, or at least .NET 7.0.

What I meant above was that dependency package downgrade makes those errors disappear when the package is consumed on .NET 6.0, i.e. makes it possible to consume the FSharp.Compiler.Service binary from projects built with .NET 6.0 (like Fable), which otherwise would hang at tcImports.GetImportedAssemblies() because of the "Detected package downgrade" errors you posted above.

Technically you don't need to build anything else besides FSharp.Compiler.Service in this fork, but there is a test project in the fcs folder to make sure it works, if you need it. There are build instructions here.

I hope that clarifies it a bit, let me know if it doesn't.
I wish we could soon switch Fable to .NET 8.0, but I guess we're stuck with .NET 6.0 to support for a while, so until then, a package version downgrade seems the easiest thing to make it work on .NET 6.0.

@nojaf
Copy link
Member

nojaf commented Oct 25, 2023

Thanks, I'm building using SDK dotnet 8 RC 2.

Building that test project didn't work for me, but I don't think that is related to the downgrade:

 ~#@❯  dotnet run -c Release --project fcs/fcs-test
C:\Users\nojaf\Projects\fsharp\fcs\fcs-test\fcs-test.fsproj : warning NU1603: fcs-test depends on Fable.Core (>= 4.1.0)
 but Fable.Core 4.1.0 was not found. An approximate best match of Fable.Core 4.1.1 was resolved.
C:\Users\nojaf\Projects\fsharp\fcs\fcs-test\fcs-test.fsproj : error NU1101: Unable to find package Fable.Import.Browser
. No packages exist with this id in source(s): C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\FSharp\library-packs, h
ttps://pkgs.dev.azure.com/azure-public/vside/_packaging/vs-impl-archived/nuget/v3/index.json, https://pkgs.dev.azure.co
m/azure-public/vside/_packaging/vs-impl/nuget/v3/index.json, https://pkgs.dev.azure.com/azure-public/vside/_packaging/v
ssdk-archived/nuget/v3/index.json, https://pkgs.dev.azure.com/azure-public/vside/_packaging/vssdk/nuget/v3/index.json,
https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json, https://pkgs.dev.azure.com/dnceng/p
ublic/_packaging/dotnet-public/nuget/v3/index.json, https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nu
get/v3/index.json, https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7/nuget/v3/index.json, https://pkgs.dev.az
ure.com/dnceng/public/_packaging/dotnet8-transport/nuget/v3/index.json, https://pkgs.dev.azure.com/dnceng/public/_packa
ging/dotnet8/nuget/v3/index.json

The build failed. Fix the build errors and run again.

@ncave
Copy link
Collaborator Author

ncave commented Oct 25, 2023

@nojaf Strange, could be some nuget cache thing. It restores and builds fine for me on WSL2 with .NET 8.0 RC2. Anyway, if you have to, you can reference a newer Fable.Core to make it work.

<PackageReference Include="Fable.Core" Version="4.1.1" />

Actually, looks like the latest version is 4.2.0. Doesn't matter, they should all work.

@ncave
Copy link
Collaborator Author

ncave commented Oct 25, 2023

@nojaf Found another mention of the FCS package downgrade issue.

@vzarytovskii
Copy link

I am still confused with the issue, FCS is ns2.0/ns2.1, all the dependencies should as well be ns2.0/ns2.1, I don't see why do those errors appear.

@ncave
Copy link
Collaborator Author

ncave commented Oct 25, 2023

@vzarytovskii Some sort of packaging issue? I'm not really sure, but it should be easy to reproduce, even with a C# project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants