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

Temp csproj gererated by fable break ionide exntension #3719

Open
Fubuchi opened this issue Jan 27, 2024 · 5 comments
Open

Temp csproj gererated by fable break ionide exntension #3719

Fubuchi opened this issue Jan 27, 2024 · 5 comments

Comments

@Fubuchi
Copy link

Fubuchi commented Jan 27, 2024

Description

During the compilation process, fable creates a temporary csproj file and runs the dotnet restore on this file.
This action might pull an old FSharp.Core version and cause errors in the editor if the F# code uses newer features.

Here is an example of my fsproj

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <LangVersion>8</LangVersion>
        <RootNamespace>react_client</RootNamespace>
    </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="Fable.Browser.Dom" Version="2.16.0" />
        <PackageReference Include="Fable.Core" Version="4.3.0" />
        <PackageReference Include="Feliz" Version="2.7.0" />
        <PackageReference Include="Feliz.Router" Version="4.0.0" />
    </ItemGroup>
    <ItemGroup>
        <Compile Include="src\shared\FelizJsxConvert.fs" />
        <Compile Include="src\components\Hello.fs" />
        <Compile Include="src\Main.fs" />
    </ItemGroup>
</Project>

The list of dependencies from the generated csproj

image

Notice the FSharp.Core version is 4.7.2 while it should be 8.x.x since I am using net SDK 8.

That causes the ionide extension complaints where ever I use string interpolation since it is a F# 5 feature.

image

I need to run dotnet restore after compile fable to correct the F# version.

Another workaround is adding <DisableImplicitFSharpCoreReference>true</DisableImplicitFSharpCoreReference> and <PackageReference Include="FSharp.Core" Version="8.0.101" /> to the fsproj so the generated csproj contains the desired F# version

Expected and actual results

The generated csproj and compilation process should not affect editor toolings.

Related information

  • Fable version: 4.10.0
  • Operating system: windows 11 build 22631
@baronfel
Copy link

@MangelMaxime where is the temp csproj being generated? I think the safest thing to do here would be to evaluate the fsproj and get the value of the IntermediateOutputPath, then put your generated csproj inside that location. That location is typically hidden from most editor tooling and shouldn't impact the parent project.

Separately, why have a csproj at all? What specifically was buildalyzer not doing for fsharp projects?

@MangelMaxime
Copy link
Member

@baronfel The temporary csproj is placed alongside the fsproj.

So you end up with:

| src
	| - MyProject.fable-temp.csproj
	| - MyProject.fsproj

Please takes my history explanations with precaution, I only followed it by reading the PR names or things like that at the time ^^

The origin of why we do that is because at some point Fable had trouble with .NET 7. I think it was using ProjInfo for the parsing but it had issues or missing information when new version of .NET came out. So Alfonso switched to using buildanalyzer.

However, the problem with buildanalyzer is that it is/was really slow against fsproj file. This is what lead to a lot of issues about Fable 3 not supporting .NET 7 because it was "freezing". So Alfonso had the idea of using a temporary csproj file to get the information needs for Fable. This has been done in this PR #3265 (it also links to some others discussion).

There has been discussion on moving the temporary csproj into another location but we think that we can't do it because if people use MSBuild features then some links could be broken. For example, if people use Directory.Build.props we need the file to be at the correct place to have the complete view of the project.

Something that has been suggested is that with .NET 8, MSbuild now offer access to the "Design Time tasks or information". Which means that we should be able to ask MsBuild the information we needs.

If I am not mistaken the information we are trying to access are:

projOpts,
Seq.toArray result.ProjectReferences,
result.Properties,
result.TargetFramework

Just by reading the code, I am unsure what projOpts are but here is the place where we retrieve the information we want for Fable

let getProjectOptionsFromProjectFile =
let mutable manager = None
let tryGetResult
(isMain: bool)
(opts: CrackerOptions)
(manager: AnalyzerManager)
(maybeCsprojFile: string)
=
if isMain && not opts.NoRestore then
Process.runSync
(IO.Path.GetDirectoryName opts.ProjFile)
"dotnet"
[
"restore"
IO.Path.GetFileName maybeCsprojFile
// $"-p:TargetFramework={opts.TargetFramework}"
for constant in opts.FableOptions.Define do
$"-p:{constant}=true"
]
|> ignore
let analyzer = manager.GetProject(maybeCsprojFile)
let env =
analyzer.EnvironmentFactory.GetBuildEnvironment(
Environment.EnvironmentOptions(
DesignTime = true,
Restore = false
)
)
// If the project targets multiple frameworks, multiple results will be returned
// For now we just take the first one with non-empty command
let results = analyzer.Build(env)
results |> Seq.tryFind (fun r -> String.IsNullOrEmpty(r.Command) |> not)
fun (isMain: bool) (opts: CrackerOptions) (projFile: string) ->
let manager =
match manager with
| Some m -> m
| None ->
let log = new System.IO.StringWriter()
let options = AnalyzerManagerOptions(LogWriter = log)
let m = AnalyzerManager(options)
m.SetGlobalProperty("Configuration", opts.Configuration)
// m.SetGlobalProperty("TargetFramework", opts.TargetFramework)
for define in opts.FableOptions.Define do
m.SetGlobalProperty(define, "true")
manager <- Some m
m
// Because Buildalyzer works better with .csproj, we first "dress up" the project as if it were a C# one
// and try to adapt the results. If it doesn't work, we try again to analyze the .fsproj directly
let csprojResult =
let csprojFile = projFile.Replace(".fsproj", ".fable-temp.csproj")
if IO.File.Exists(csprojFile) then
None
else
try
IO.File.Copy(projFile, csprojFile)
let xmlDocument = XDocument.Load(csprojFile)
let xmlComment =
XComment(
"""This is a temporary file used by Fable to restore dependencies.
If you see this file in your project, you can delete it safely"""
)
// An fsproj/csproj should always have a root element
// so it should be safe to add the comment as first child
// of the root element
xmlDocument.Root.AddFirst(xmlComment)
xmlDocument.Save(csprojFile)
tryGetResult isMain opts manager csprojFile
|> Option.map (fun (r: IAnalyzerResult) ->
// Careful, options for .csproj start with / but so do root paths in unix
let reg = Regex(@"^\/[^\/]+?(:?:|$)")
let comArgs =
r.CompilerArguments
|> Array.map (fun line ->
if reg.IsMatch(line) then
if
line.StartsWith(
"/reference",
StringComparison.Ordinal
)
then
"-r" + line.Substring(10)
else
"--" + line.Substring(1)
else
line
)
let comArgs =
match r.Properties.TryGetValue("OtherFlags") with
| false, _ -> comArgs
| true, otherFlags ->
let otherFlags =
otherFlags.Split(
' ',
StringSplitOptions.RemoveEmptyEntries
)
Array.append otherFlags comArgs
comArgs, r
)
finally
File.safeDelete csprojFile
let compilerArgs, result =
csprojResult
|> Option.orElseWith (fun () ->
tryGetResult isMain opts manager projFile
|> Option.map (fun r ->
// result.CompilerArguments doesn't seem to work well in Linux
let comArgs = Regex.Split(r.Command, @"\r?\n")
comArgs, r
)
)
|> function
| Some result -> result
// TODO: Get Buildalyzer errors from the log
| None ->
$"Cannot parse {projFile}" |> Fable.FableError |> raise
let projDir = IO.Path.GetDirectoryName(projFile)
let projOpts =
compilerArgs
|> Array.skipWhile (fun line -> not (line.StartsWith('-')))
|> Array.map (fun f ->
if
f.EndsWith(".fs", StringComparison.Ordinal)
|| f.EndsWith(".fsi", StringComparison.Ordinal)
then
if Path.IsPathRooted f then
f
else
Path.Combine(projDir, f)
else
f
)
projOpts,
Seq.toArray result.ProjectReferences,
result.Properties,
result.TargetFramework


I also believe that the dotnet restore against the csproj breaks Ionide intellisense supports. As I often, need to call dotnet restore against the fsproj manually after Fable started in order for Ionide to works as expected.

A temporary workaround, could be to invoke dotnet restore against the fsproj after we retrieved the information from the csproj. It should restore the obj / bin folder to the correct state.

This is a temporary solution while we investigate how to not use the csproj hack or to use it in a better way. The plans up to now, where to investigate MSBuild design time feature which can with .NET 8. Our main question up to now, is if that will require people to move their project to .NET 8 or not. As Fable is invoked locally, and will probably respect the global.json which could set the version of .NET to another version.

@MangelMaxime
Copy link
Member

In #3722, I implemented the workaround of restoring the fsproj after the temporary csproj.

We are currently exploring a solution to not rely on the csproj in #3721, but it can take some times before it is ready.

@nojaf
Copy link
Member

nojaf commented Feb 18, 2024

Hi @Fubuchi, in the latest version (4.12.2) you can pass --test:MSBuildCracker as argument and that should not generate the temporary csproj file.
This is an experimental feature, so I'd be curious if it works for your project.

@Fubuchi
Copy link
Author

Fubuchi commented Feb 18, 2024

I tried to build with --test:MSBuildCracker, and no csproj was created during the building process.

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

No branches or pull requests

4 participants