search

ionide / proj-info

Parse and evaluate MsBuild project files
MIT License
64 stars 37 forks source link

Add missing targets - `BeforeBuild` and `BeforeCompile` #183

Closed safesparrow closed 1 year ago

safesparrow commented 1 year ago

The above two targets are custom hooks that users can use for arbitrary changes to the build process. ProjInfo should run those targets. Some projects use those to add SourceFiles - for example FSharp.Compiler.Service.

More details

F# codebase defines the _GenerateBuildPropertiesFile target, which gets executed if BeforeBuild runs: https://github.com/dotnet/fsharp/blob/875dbcc695d1ffdb20e3e899ba9a006df76fe051/FSharpBuild.Directory.Build.targets#L63-L66

Embarrasingly I don't recall why I needed to add BeforeCompile - I'll look at FCS compilation once again to figure it out.

TODO:

baronfel commented 1 year ago

This looks like a great change, but it looks like the comparisons are failing on CI.

safesparrow commented 1 year ago

Sure, I'm trying to fix those. TBH it's a bit painful to work on those tests. Lack of/poor integration of Expecto in Rider and the test output format not being friendly to spotting where the difference is - they don't make it easier 😰 But I hope to get them working at some point this weekend.

baronfel commented 1 year ago

So with the dotnet test integration, my understanding is that it should just work in Rider. I'd love to take any changes required to make that work, as well!

safesparrow commented 1 year ago

It does work as in I can schedule all the tests from within Rider by clicking on the project in the solution. But:

  1. 'run unit tests' on the project runs it as a CLI app, which produces a lot of text output that is hard to digest.
  2. Sometimes tests do not appear in the Test Explorer, so running individual tests can sometimes be tricky.
  3. Tests do not run in parallel.

I might look into 3. at some point, but for now I really hope to get this thing working - after all, I'm already in a deep rabbit hole, trying not to dig deeper 🙂

baronfel commented 1 year ago

I think with #3 you may run into MSBuild limitations - the evaluation of projects seems to be a singleton process? We have a mailbox processor gating it internally to help enforce this, so even if you made the tests run in parallel, I think the library itself would enforce serial behavior.

baronfel commented 1 year ago

I don't know why macos has started timing out as well, but it's happening on my other small fix branch so I'm happy to take this. @safesparrow did you discover whether you need BeforeCompile or not?

safesparrow commented 1 year ago

BeforeCompile is used in the F# codebase as a trigger for running the following target: https://github.com/dotnet/fsharp/blob/875dbcc695d1ffdb20e3e899ba9a006df76fe051/src/FSharp.Build/Microsoft.FSharp.Targets#L408-L433

That target's result is then used in this target, also a dependency of BeforeCompile: https://github.com/dotnet/fsharp/blob/875dbcc695d1ffdb20e3e899ba9a006df76fe051/src/FSharp.Build/Microsoft.FSharp.Targets#L407-L433

The net result is that not running BeforeCompile makes one of the auto-generated source files disappear.

My end goal is to extract FSC args for an fsproj using ProjInfo, which I can then use to run compilation. Without both these targets, some source files are missing and compilation fails, which is bad for me.

However, I am not entirely sure what effect this change might have on Ionide and other IDEs - it might be adding some source files previously not listed, but this should be... good?

baronfel commented 1 year ago

Yeah, those tools should want to show all included source files (even generated ones), so this would be a good change.