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

Remove AnalyzeFSharpProjectUsingFscArgs #164

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Nov 14, 2023

Alas, I must shatter my dream of reusing the fsc arguments provided by MSBuild.
I noticed some drawbacks when doing a DesignTimeBuild for projects that have project references.

Let us take Fantomas.Client.fsproj and Fantomas.Client.Tests.fsproj as example:

  • Performing the design time build for Fantomas.Client.Tests.fsproj will trigger a build for Fantomas.Client.fsproj as well. However, this won't produce a binary for Fantomas.Client, while Fantomas.Client.Tests does depend on its existence from the fsc args perspective.
  • Because Fantomas.Client.Tests already built Fantomas.Client (without producing a binary), CoreCompile will be skipped when we try and do the design time build for Fantomas.Core and thus FscCommandLineArgs will not be populated. I believe this to be a larger problem in the FSharpTargets but I digress.

So in short, the idea of reusing FscCommandLineArgs from MSBuild only works when:

  • All dependent projects did produce a binary.
  • CoreCompile of the current project is not skipped.

Initially, we did a full build of fantomas.sln before invoking the targets, which helps with the dependent projects having binaries. But doesn't help that CoreCompile is being skipped.

I don't see a scenario where we can have the best of both worlds, so I recommend we stick to AnalyzeFSharpProject and keep using proj-info to crack the projects.

That being said, I do still see a use-case for --fsc-args as cli parameter input. This can be great for debugging and exploring.

@nojaf nojaf requested a review from dawedawe November 14, 2023 15:58
@dawedawe
Copy link
Contributor

I trust you here, of course.
But for my naive mind:
Can't we still depend on a full solution build and force CoreCompile by that to not being skipped?
Or is it somehow intrinsic to getting the FscArgs that CoreCompile is skipped?

@nojaf
Copy link
Contributor Author

nojaf commented Nov 14, 2023

force CoreCompile by that to not being skipped?

Yeah, was at one point thinking along those lines as well but you can't really do that in MSBuild.
There is no easy way to tell it during a run that a Target needs to executed either way.

@TheAngryByrd
Copy link
Member

I'd recommend looking at all the properties we set in proj-info and FSAC and using them as closely as possible to see if this makes a difference first.

@nojaf
Copy link
Contributor Author

nojaf commented Nov 14, 2023

@TheAngryByrd how do you think I spend my afternoon?

This whole idea was stolen from the get-go from proj-info.
Where proj-info differs is that it loads the F# project references.

@nojaf nojaf merged commit ae263a5 into ionide:master Nov 15, 2023
2 checks passed
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.

3 participants