Fable
Fable copied to clipboard
Consider adding `--msbuild-property <key> <value>` allowing user to pass properties to MSBuild from Fable CLI
Description
I have a project which has a custom Exec target to build its Fable artifacts. This works fine for Fable 4 + .NET <= 6. However when upgrading to .NET 7 or 8, my project hangs forever after this output:
.> dotnet restore MyProject.csproj -p:FABLE_COMPILER=true -p:FABLE_COMPILER_4=true -p:FABLE_COMPILER_JAVASCRIPT=true
Determining projects to restore...
All projects are up-to-date for restore.
After a lot of debugging, I determined that this is because it's recursively building itself via the <Exec> task.
Repro code
Put the following in your App.fsproj:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<Compile Include="App.fs" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Fable.Browser.Dom" Version="2.2.0" />
<PackageReference Include="Fable.Core" Version="3.2.3" />
</ItemGroup>
<Target Name="CustomPostBuild" AfterTargets="PostBuildEvent">
<Exec Command="touch some-file.txt" />
</Target>
</Project>
Then, build fable artifacts: dotnet fable .
Expected and actual results
Fable probably shouldn't attempt to run the Exec target, given the fact that this didn't always happen. Instead, you will notice that some-file.txt was created.
Alternatively, if I could set custom MSBuild properties when invoking dotnet fable, I could work around this. In my real project, I already have it set up so that it skips this target if NoFableBuild is set. Due to the way my builds are set up, I really need this Fable build target to be enabled by default.
Related information
- Fable version: 4.1.4
- .NET version: 8.0.100-preview.7.23376.3 (also happens with 7.0.*)
- Operating system: macOS 12.6
Hello @jwosty ,
Sorry to ask but are you sure that by using .NET 6 you don't have the some-file.txt file created?
I am asking because I am able to reproduce your problem on both .NET 6 and .NET 7 with fable 4.1.4
I found a potential workaround this issue.
Fable has a --define argument allowing you to pass compiler directives to be used in F# code via #if ... #endif.
But --define instructions are also forwarded to the dotnet restore command via -p:{constant}=true.
This means you can make a clever (or hacky 😇) workaroud using MSBuild conditions.
In the example below, the <Target> node is executed only if FABLE_INVOKED_FROM_MSBUILD is not provided.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<Compile Include="App.fs" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Fable.Browser.Dom" Version="2.2.0" />
<PackageReference Include="Fable.Core" Version="3.2.3" />
</ItemGroup>
<PropertyGroup>
<FABLE_INVOKED_FROM_MSBUILD Condition=" '$(FABLE_INVOKED_FROM_MSBUILD)' == '' ">false</FABLE_INVOKED_FROM_MSBUILD>
</PropertyGroup>
<Target Condition="'$(FABLE_INVOKED_FROM_MSBUILD)'=='false'" Name="CustomPostBuild" AfterTargets="PostBuildEvent">
<Exec Command="dotnet fable --define FABLE_INVOKED_FROM_MSBUILD --noCache" />
</Target>
</Project>
Note:
This is all supposition from me and here so I can refer to it when looking into your issue
I think I found the line responsible for building the project. At least let results = analyzer.Build(env) seems like a good candidate ^^.
https://github.com/fable-compiler/Fable/blob/dd4d6c986c1083495044ed29e9f9d11abe4dfd66/src/Fable.Cli/ProjectCracker.fs#L426-L442
This is strange because the analyser is run DesignTime=true which I suppose mean don't build stuff?
I don't think we can remove the analyzer.Build stuff because it is required to get the dependencies etc.
But I think we should be able to provide additional MSBuild properties like FABLE_PROJECT_CRACKING.
Which would allow you to something like:
<Target Condition="'$(FABLE_PROJECT_CRACKING)'=='false'" Name="CustomPostBuild" AfterTargets="PostBuildEvent">
Actually, it seems like there is already a DesignTimeBuild property in MSBuild or BuildAnalyser.
Using the example below is enough for me to not be able to reproduce your problem. Can you please check ?
<Target Condition="'$(DesignTimeBuild)' == 'false'" Name="CustomPostBuild" AfterTargets="PostBuildEvent">
<Exec Command="touch some-file.txt" />
</Target>
OK, you were right that all version of Fable 4 do this. Fable 3 plus .NET 6 is actually the combination which doesn't do this. Fable 3.7.18 with .NET 7 or 8 hangs indefinitely, which is this issue: https://github.com/fable-compiler/Fable/issues/3294
Interesting; I saw that Fable passes in FABLE_COMPILER (and some other properties) and tried to use that, but it seemed to never be defined at the build step. I assumed it's because they only get defined for dotnet restore, from the project's perspective? I'll be trying out a custom property to see if that works.
OK, you were right that all version of Fable 4 do this. Fable 3 plus .NET 6 is actually the combination which doesn't do this. Fable 3.7.18 with .NET 7 or 8 hangs indefinitely, which is this issue: https://github.com/fable-compiler/Fable/issues/3294
This is indeed more expected to me as in order to support .NET 7 which was introduce in Fable 4, the ProjectCraker has been changed completely.
Sorry for the spam,
I just saw a note on https://github.com/dotnet/project-system/blob/main/docs/design-time-builds.md
Which says:
NOTE: The DesignTimeBuild property is typically empty ('') in normal builds, so avoid comparisons to 'false'.
So you probably want to use Condition="'$(DesignTimeBuild)' != 'true'" to avoid the comparisons with false.
Actually, it seems like there is already a
DesignTimeBuildproperty in MSBuild or BuildAnalyser.Using the example below is enough for me to not be able to reproduce your problem. Can you please check ?
NOTE: The DesignTimeBuild property is typically empty ('') in normal builds, so avoid comparisons to 'false'.
So you probably want to use
Condition="'$(DesignTimeBuild)' != 'true'"to avoid the comparisons withfalse.
I can confirm this works in the real project. Thank you!
Now I'm not so sure whether or not the bug is with Fable or my project file -- the part that tripped me up the most was that the behavior changed between versions in such a way that in my scenario, it manifested in the exact same way as #3294, leading me in the wrong direction (probably chalk that up to a very unfortunate coincidence that not many people will run into). It could definitely be argued that it's really a bug in my project file, which shouldn't perform a Fable build on itself during design-time, especially since we have $(DesignTimeBuild).
If that ends up being the case, I would recommend that Fable at least print <Message> elements, to make debugging this kind of thing easier.
At the very least I can say that it's amazing to finally have any solution at all to this. I've been trying to upgrade my project since .NET 7 came out, and this has been the hurdle for a year or two at this point (there were other problems related to FAKE for a while too, but I've gotten around those eventually). Yay!
Either way, it would also be nice to have an official bespoke --property argument, separate from --define. Not high priority, though, since there's a better workaround.
Also just want to add that I appreciate all the work y'all do on Fable on a regular basis. I really love using it in my stuff.
If that ends up being the case, I would recommend that Fable at least print <Message> elements, to make debugging this kind of thing easier.
The thing is that I don't think the bug is in Fable but either in the BuildAnalyser that we use to parse project files or in MSBuild.
I also don't think we can identify this problem on Fable side, because I suspect that what is happening is an infinite loop of Fable being spawned by BuildAnalyser in this case.
For now I think using $(DesignTimeBuild) is a good solution and in the future introducing --msbuild-property <key> <value> could be possible.
This will probably happen when switching Fable custom args parser to something like Argu. But that a whole project in it self because Fable CLI is not standard at all and I worry that we would need to introduce breaking changes in the CLI in order to standard on good basis again ...