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

use shipped FSharp.Core when using FSharp.Compiler.Service.sln #12149

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Sep 15, 2021

@dsyme dsyme changed the title use shipped FSharp.Core when using FSHarp.Compiler.Service.sln use shipped FSharp.Core when using FSharp.Compiler.Service.sln Sep 15, 2021
@auduchinok
Copy link
Member

@dsyme Thanks a lot!

@dsyme dsyme merged commit 63bef2c into dotnet:main Sep 15, 2021
@baronfel
Copy link
Member

How does this change interact with preview features like the resumable code builders, which may not be in the currently published FSharp.Core? I was expecting there to be failures or mismatches there.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 15, 2021

How does this change interact with preview features like the resumable code builders, which may not be in the currently published FSharp.Core? I was expecting there to be failures or mismatches there.

The code of FCS currently builds without using these features

Note this is only when explicitly building src/fsharp/FSharp.Compiler.Service/FSharp.Compiler.Service.sln, which is not a mainline path, just a developer-friendly path

@auduchinok
Copy link
Member

auduchinok commented Sep 16, 2021

@dsyme This doesn't seem to fix #12142, and the errors are still there in the tooling. The error in #12142 comes from a recent change in FSharp.Core that hasn't been published in a stable FSharp.Core version yet. #11888 changed both FSharp.Core and FCS code, which made FCS rely on a non-stable-published-yet FSharp.Core version.

@auduchinok
Copy link
Member

It seems wrong that FCS solution actually builds despite the errors in the tools. It seems it uses another FSharp.Core reference for build.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 16, 2021

This doesn't seem to fix #12142, and the errors are still there in the tooling. The error in #12142 comes from a recent change in FSharp.Core that hasn't been published in a stable FSharp.Core version yet

Really? My impression is that this PR means that when using FSharp.Compiler.Service.sln, a public FSharp.Core is now referenced. Can you send build logs and/or any other information showing the references being used in the IDE? Also is the IDE Rider, and does that set SolutionName correctly when interpreting the project files? Maybe you need to do a flush and restore?

@auduchinok
Copy link
Member

auduchinok commented Sep 16, 2021

Really? My impression is that this PR means that when using FSharp.Compiler.Service.sln, a public FSharp.Core is now referenced.

This is the exact source of the issue. The current FSharp.Core (5.0.2) doesn't include changes from #11888 yet, but FCS already uses them.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 16, 2021

I see, so this passes because it doesn't build using FSharp.Compiler.Service.sln

We should

  1. add a CI job to build that solution.
  2. put in #if of some kind to allow building against the shipped FSharp.Core.

@auduchinok
Copy link
Member

Also is the IDE Rider, and does that set SolutionName correctly when interpreting the project files? Maybe you need to do a flush and restore?

The issue occurs in both Visual Studio and Rider for me. I've just tried to git clean and run build.cmd just in case something would behave differently, but it's the same, as I'd expect.

@vzarytovskii
Copy link
Member

vzarytovskii commented Sep 16, 2021

I see, so this passes because it doesn't build using FSharp.Compiler.Service.sln

We should

  1. add a CI job to build that solution.

We do build it on Windows, Linux and macOS on CI, see

- script: dotnet build .\FSharp.Compiler.Service.sln /bl:\"artifacts/log/$(_BuildConfig)/ServiceRegularBuild.binlog\"

But the problem only occurs in ide, as far as I understand.

@auduchinok
Copy link
Member

It seems this change does a wrong thing? The preview package is exactly what should've been used (the preview of 5.0.3, needed for the recent FSharp.Core changes) instead of the published release package (5.0.2).

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.

Unresolved symbols in FCS solution
4 participants