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

read fsc args from new fsproj #707

Closed
enricosada opened this issue Feb 22, 2017 · 11 comments
Closed

read fsc args from new fsproj #707

enricosada opened this issue Feb 22, 2017 · 11 comments

Comments

@enricosada
Copy link
Contributor

enricosada commented Feb 22, 2017

This will enable to get fsc arguments from fsproj.

Current status: FSharp.NET.Sdk 1.0.0 removed the generation of dotnet-compile-fsc.rsp response file used by FSAC, projecracker doesnt understnd new fsproj. So FSAC doesnt work (no vscode intellisense)

Proposed: No more guessing from projectcracker (fragile) and is similar to what VF# does in VS, just without hosting the fsharp.build.dll.

Atm build (dotnet build = dotnet msbuild /t:Build, so is for all msbuild) is:

Build target -> Compile target -> CoreCompile target -> Fsc task invocation (task is contained in FSharp.Build.dll) -> fsc

  • the Build and Compile targets are in Microsoft.NET.Sdk
  • The CoreCompile target is in FSharp.NET.Sdk (community)
  • The FscTask and fsc are in FSharp.Compiler.Tools package (community)

NOTE The FscTask contains the logic on how to compose fsc args, but properties are passed by CoreCompile target.
NOTE compiler args may change in future, so workflow should be stable, without guessing version.

My idea is: Run compile, but not invoke fsc, just save compiler args somewhere

Extended is:

  1. run Compile target (so like dotnet msbuild my.fsproj /t:Compile)
  2. pass the properties to force compilation (the job of _ComputeNonExistentFileProperty target) to force CoreCompile also if already compiled
  3. pass a property to Fsc task to not run fsc (like c# does with SkipCompilerExecution property
  4. add output property of Fsc task with composed compiler args, as FscCommandLineArgs item (like c#/csc does here with CscCommandLineArgs

After that:

  • or evaluate FscCommandLineArgs item with a tool, but dumb, so simple
  • or target will save FscCommandLineArgs to a file (passed from a property?), so will be possible to save from command line invocation

The changes needed are minor, just adding optional props, no breaking:

  • add SkipCompilerExecution to FscTask. default to false so no issue for old users
  • add FscCommandLineArgs output items, is ignored if not read, so np for old users
  • fix CoreCompile target to pass these props
  • optionally change CoreCompile to save FscCommandLineArgs to file (discuss)

Some notes from my dev notes (is a msbuild response file):

# Target to run:
#`Build` no. check output files only (obj -> bin). cannot be used if these are uptodate
# `CoreCompile` no. doesnt compute references as dependency
# `Compile` is okish, but trigger `AfterCompile` target too
# `ResolveReferences;CoreCompile` seems ok

/t:ResolveReferences;CoreCompile

# Force compilation. Otherwise Compile can be already up-to-date (obj files).
# These args enable the target `_ComputeNonExistentFileProperty`
# who add another input to `CoreCompile`, making target always not uptodate with outputs
/p:BuildingInsideVisualStudio=true
/p:BuildingOutOfProcess=false
/p:BuildingProject=false

# use this property to choose response file output path
/p:FscToolResponseFileOutPath=dotnet-compile-fsc.rsp

/cc @tpetricek @dsyme @7sharp9 @latkin @vladima FCS team
/cc @rneatherway emacs
/cc @kjnilsson vim
/cc @guillermooo sublime
/cc @KevinRansom @cartermp vs
/cc @nosami @mrward vs on mac
/cc @Krzysztof-Cieslak ionide vscode/atom
/cc @alfonsogarciacaro fable

@enricosada
Copy link
Contributor Author

@alfonsogarciacaro , this can be used by fable too to get fsc args (replacing project cracker)

@enricosada
Copy link
Contributor Author

btw i'll finish to implement this proposed solution soon, so we can see if works ok enough :D

@KevinRansom
Copy link
Contributor

@enricosada

What is the problem you are trying to solve? Do you want the Compiler Service to expose the commands selected for a compilation or do you want the build to record the build options passed to the compiler?

The proposal you make is fine, just add a property to the build task and modify:
this line https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/FSharp.Build/Fsc.fs#L511
to write out the response file arguments to a file (Probably named in another task property) rather than execute the compiler.

However, that seems like the wrong way to go if you want to integrate the results in the compiler service APIs. Probably the Compiler Service should instead, use the settings on TcGlobalBuilder or TcGlobal to expose the settings deliberately, not exposing the TcGlobal object of course .... that would be nutso.

But the list of referenced assemblies, the optimization options ... the output file ... etc.
If you do that then whatever the options the host has for specifying options and setting defaults ... (they are different even between FSI and FSC) will be exposed consistently.

Anyway ... my 2 cents

@rneatherway
Copy link
Member

FYI this is exactly what the project cracker tries to do (essentially run the actual compilation in its own process after overriding the compilation step with an empty function). I tried to make that as similar as possible to what VF# did at the time. I am out of the loop with regards to the new fsproj structure and tooling around it, so it may be much easier now.

@dsyme
Copy link
Contributor

dsyme commented Feb 22, 2017

@KevinRansom He's trying to replace this method

static member GetProjectOptionsFromProjectFile : projectFileName : string * ?properties : (string * string) list  -> FsharpProjectOptions

which is basically the same as converting an .fsproj to an fsc command line. The current implementation has an MSBuild API dependency.

@KevinRansom
Copy link
Contributor

Update the buildtask and send a pr to visualfsharp tools, with the changes the buildtask already creates a .rsp file, it's really just a case of persisting it somewhere.

@enricosada
Copy link
Contributor Author

Thanks a lot @KevinRansom for feedback! i'll send a PR asap.

What is the problem you are trying to solve? Do you want the Compiler Service to expose the commands selected for a compilation or do you want the build to record the build options passed to the compiler?

I dont want to change FCS itself (initialized by with fsc args array is perfect ihmo), just projectcracker.
I need just the build options passed to compiler (without running the fsc, just msbuild). that's enough to initialize the FCS, so intellisense works. that's what FSAC is doing today for vscode.
And doing that can help changing options in editors too i think (so i can get fsc args for configuration Release) easier for FCS consumers.

AFAIK it's pretty much what the vf# does (AFAIK) in Project.fs
and later read by Compile method called with reflection, but doing a soft integration with msbuild instead.

member x.Compile(compile:System.Converter<int,int>, flags:string[], sources:string[]) = 
  // Note: This method may be called from non-UI thread!  The Fsc task in FSharp.Build.dll invokes this method via reflection, and
  // the Fsc task is typically created by MSBuild on a background thread.  So be careful.

@rneatherway is really similar to old (i am not trying to invent something new, i hope, so is tested), but saving compiler args will make possibile to use these directly, instead of guess from msbuild props/items, like is done here in projectcracker.
Instead FCS will just parse fsc args, because the meaning in known (msbuild props/items instead may change a bit).
Obv for other things like ProjectReference items, that's the old story (i'd like to see what omnisharp does about that).

maybe i miss something, but should at least work like now tested in vscode without requiring a build before opening vscode, for intellisense

@rneatherway
Copy link
Member

Understood, this sounds good. To have it supported unofficially by VF# team should help to keep it more robust! Should be a great improvement if we can remove the shaky project cracker eventually.

@enricosada
Copy link
Contributor Author

Ok i am satified by my current design, and already sent pr.

I removed usage of msbuild api programmaticailyl (fragile, binding redirect, assembly already loaded. => pain).
I use a different mechanism, with auto-imported target file put in obj directory. It's already used by other cli tools (dotnet-ef, dotnet-user-secrets) who interface with csproj/fsproj.

image

pratically add a target file, with some targets and execute dotnet msbuild /t:MyAddedTarget, target MyAddedTarget evaluate some props and save to a temp file for communication (cannot trust msbuild outputstream)

Added a console app [dotnet-proj-info](ref https://github.com/enricosada/dotnet-proj-info/tree/master/examples) to read fsc args and other info from fsproj/csproj. Can also be modified as lib if needed (please i need feedback about that)

So usage is like

dotnet proj-info -p ..\c1\c1.fsproj --fsc-args

will write all fsc args.

Example:

rec2

@alfonsogarciacaro
Copy link
Contributor

alfonsogarciacaro commented Mar 3, 2017

Fantastic work @enricosada, thanks for this!

I think it'd be interesting to have this as a library to make it more easily callable from other programs. And maybe also making it asynchronous?

It should be clear that this depends on the dotnet sdk being installed in the machine. I guess this will be the case in most situations (with projectcracker there were many problems with Windows machines that didn't have the expected version of VS installed) but some machines may only have the dotnet runtime installed.

BTW, as Fable won't be supporting nuget PackageReferences the next version will probably use a custom simplistic resolution, but we'll probably move soon to use this to read all the options in the project. And definitely needed this is needed for the IDEs.

@enricosada
Copy link
Contributor Author

Closing.

Published:

both can read fsc args, any msbuild property, p2p references of a project, with just one msbuild invocation batching requests

both require FSharp.NET.Sdk >= 1.0.2 to get fsc args, because i added new properties to msbuild Fsc task

added support to FSAC with ionide/FsAutoComplete#55 to replace projectcracker

dsyme added a commit to dsyme/FSharp.Compiler.Service that referenced this issue Apr 29, 2017
[msbuild] Fix regression in Microsoft.Build.FSharp.targets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants