-
Notifications
You must be signed in to change notification settings - Fork 830
Proto compiler should use an existing FSharp.Core from package #6536
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
Conversation
|
cc @brettfo |
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\FSharp.Core\FSharp.Core.fsproj"> | ||
| <ProjectReference Include="$(MSBuildThisFileDirectory)..\FSharp.Core\FSharp.Core.fsproj"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why, but in some places we use $(MSBuildThisFileDirectory) and in some we don't. If it's not needed anywhere we should just remove it everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think so. $(MSBuildThisFileDirectory) always expands to the full path, making ms build logs easier to reason about. We should prefer the use of the predefined msbuild variables instead of bare relative paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind, but TBH it's a bit odd to use the variable here if it's not needed
|
For build from source grabbing components from the network is not allowed, but we would use the FSharp.Core package that matches the LKG FSC compiler. Effectively it's whatever dotnet cli is used to build the build from source product. We will have absolutely no control over what the version of that compiler or package is. My guess is the big problem you are is in the nullable stuff, where you are trying to bootstrap nullable into FSharp.Core, using an F# compiler that doesn't understand how to do nullable (the LKG) and want to build a proto compiler that is aware of nullable but that uses the non nullable FSharp.Core. I'm not certain that I agree that we shouldn't build fsharp.core when we build the proto compiler but I can see how that makes bootstrapping the tools easier. I will chat with @brettfo to see what we can do that will satisfy buildfromsource requirments, and that will enable this bootstrap path. I expect we can just let the compiler fall back to the fsharp.core alongside the compiler when building the proto compiler. |
Well, the proto compiler is a LKG program so should use an LKG FSharp.Core. There's no reason why the current FSharp.Core should build with the LKG, so it forces you to scatter
That works, I don't mind either way |
|
The build from source stuff is implicit now, and @KevinRansom is correct; for source-build scenarios we're not allowed to pull in NuGet packages, so we need to keep this as a project reference. If you feel really strongly about a package reference, you can gate it on |
|
@brettfo, whilst in my heart I agree with you. Don's scenario is valid, after lunch we can have a sob about the terrible hacks, but we should figure something out. At the very … very worst we can let it fall back to the FSharp.Core alongside the LKG compiler. Which we can specify quite easily. Kevin |
That's acceptable to me. |
|
closing per: #6557 |
The proto compiler is an F#-LKG program should use an FSharp.Core from a package, rather than trying to use one freshly built using the LKG compiler.
This is important for features like #5790 and #6345 where the FSharp.Core is undergoing updates and there is no need to try to transiently build it with the LKG - it is only designed to be built with the Proto compiler which supports new features such as nullness.
@KevinRansom I need to understand if this affects our build-from-source story. In particular
does build-from-source assume we can build our final FSharp.Core using an LKG compiler. If so, that's a problem - sometimes FSharp.Core has new features which can only be built using Proto.
can build-from-source assume the availability of an FSharp.Core package?
But now I look I notice
src/buildfromsourcehas actually disappeared.... What's the story now? There are still numerous references to it inDEVGUIDE.md..... Does our intergration into .NET SDK build-from-source now just build our entire repo including bootstrap?