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

FSharp.Compiler.Service.ProjectCrackerTool.exe crashes #672

Closed
robkuz opened this issue Nov 23, 2016 · 20 comments
Closed

FSharp.Compiler.Service.ProjectCrackerTool.exe crashes #672

robkuz opened this issue Nov 23, 2016 · 20 comments

Comments

@robkuz
Copy link

robkuz commented Nov 23, 2016

ProjectCrackerTool crashes

Repro steps

FSharp.Compiler.Service.ProjectCrackerTool.exe --text "crasher.test.fsproj" true

Expected behavior

No Crashes ;-)

Actual behavior

Using the given project file name ProjectCrackerTools crashes with StackOverflowException when MsBuild (12,14 & 15) and F# 4.0 and 4.1 are installed and the fsproj file name contains dots

Known workarounds

No dots in filename

Related information

Provide any related information

  • Windows 10
  • MsBuild (12,14,15)
  • F# 4.0 & 4.1
  • Visual Studio 2017 RC
@Krzysztof-Cieslak
Copy link
Contributor

I think the problem might be with VS2017 RC / MsBuild 15. I've tried same file on my machine (without those 2 things) and it works OK.

@forki
Copy link
Member

forki commented Nov 23, 2016

So my internal tech radar says Vs2017 is hold.

@dsyme
Copy link
Contributor

dsyme commented Feb 23, 2017

@robkuz Is this still a problem?

@dsyme dsyme added the question label Feb 23, 2017
@robkuz
Copy link
Author

robkuz commented Feb 23, 2017

I have not updated my environment in quite some time. And at the moment is no good time to do so. If it is solved from your perspective close it

@dsyme
Copy link
Contributor

dsyme commented Feb 23, 2017

Do you have crasher.test.fsproj? thanks

@robkuz
Copy link
Author

robkuz commented Mar 2, 2017

No I dont have that anymore. But as described above it wasnt about the content of the file only about the naming. As soon as I removed the dot in the filename it worked

@csmager
Copy link

csmager commented Mar 8, 2017

I installed VS 2017 RTM and am hitting this downstream in Ionide (see the linked issues). It seems FSAC crashes with StackOverflowException.

@csmager
Copy link

csmager commented Mar 8, 2017

I've done a bit more digging. FSharp.Compiler.Service.ProjectCrackerTool.exe fails in this AssemblyResolve handler. The code is below:

let onResolveEvent = new ResolveEventHandler(fun sender evArgs ->
    let requestedAssembly = AssemblyName(evArgs.Name)
    if requestedAssembly.Name.StartsWith("Microsoft.Build") &&
        not (requestedAssembly.Name.EndsWith(".resources")) then
    // If the version of MSBuild that we're using wasn't present on the machine, then 
    // just revert back to 12.0.0.0 since that's normally installed as part of the .NET 
    // Framework.
    requestedAssembly.Version <- Version("12.0.0.0")
    Assembly.Load (requestedAssembly)
    else
    null)

On my machine, it gets called to resolve Microsoft.Build.Tasks.Core Version 15.1.0.0. This is matched by the if and then Version 12.0.0.0 is requested. This isn't found either, and there you have the stack overflow.

@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2017

On my machine, it gets called to resolve Microsoft.Build.Tasks.Core Version 15.1.0.0. This is matched by the if and then Version 12.0.0.0 is requested. This isn't found either, and there you have the stack overflow.

I don't understand why Microsoft.Build.Tasks.Core Version 15.1.0.0 is being requested by Ionide's use of FSharp.Compiler.Service.ProjectCrackerTool.exe - does anyone know? The only references I see to that component in the latest FSharp.Compiler.Service are here:

Find all "Microsoft.Build.Tasks.Core", Match case, Whole word, Subfolders, Find Results 1, "c:\GitHub\dsyme\FSharp.Compiler.Service\src", "*"
  C:\GitHub\dsyme\FSharp.Compiler.Service\src\fsharp\FSharp.Compiler.Service.ProjectCracker\project.json(35):    "Microsoft.Build.Tasks.Core": "14.3.0",
  C:\GitHub\dsyme\FSharp.Compiler.Service\src\fsharp\FSharp.Compiler.Service.ProjectCrackerTool\project.json(23):    "Microsoft.Build.Tasks.Core": "14.3.0",
  Matching lines: 2    Matching files: 2    Total files searched: 233

@csmager
Copy link

csmager commented Mar 8, 2017

This isn't in ionide - I've cloned the repo and am running the ProjectCrackerTool.exe from inside VS 2015 with my project file in the debug arguments.

I just uninstalled VS 2017 and I still have the same issue, so it's one of the other packages VS 2017 has installed that's causing the change in behaviour. I'll keep looking.

FWIW, if I change 12.0.0.0 to 14.0.0.0 in that handler, it works.

@cloudRoutine
Copy link
Contributor

cloudRoutine commented Mar 8, 2017

The order of precedence, from highest to lowest, used to determine the ToolsVersion is:

  1. The ToolsVersion attribute on the MSBuild task used to build the project, if any.
  2. The /toolsversion (or /tv) switch that's used in the msbuild.exe command, if any.
  3. If the environment variable MSBUILDTREATALLTOOLSVERSIONSASCURRENT is set, then use the current ToolsVersion.
  4. If the environment variable MSBUILDTREATHIGHERTOOLSVERSIONASCURRENT is set and the ToolsVersion defined in the project file is greater than the current ToolsVersion, use the current ToolsVersion.
  5. If the environment variable MSBUILDLEGACYDEFAULTTOOLSVERSION is set, or if ToolsVersion is not set, then the following steps are used:
    a. The ToolsVersion attribute of the Project element of the project file. If this attribute doesn’t exist, it is assumed to be the current version.
    b. The default tools version in the MSBuild.exe.config file.
    c. The default tools version in the registry. For more information, see Standard and Custom Toolset Configurations.
  6. If the environment variable MSBUILDLEGACYDEFAULTTOOLSVERSION is not set, then the following steps are used:
    a. If the environment variable MSBUILDDEFAULTTOOLSVERSION is set to a ToolsVersion that exists, use it.
    b. If DefaultOverrideToolsVersion is set in MSBuild.exe.config, use it.
    c. If DefaultOverrideToolsVersion is set in the registry, use it.
    d. Otherwise, use the current ToolsVersion.

https://msdn.microsoft.com/en-us/library/bb383985.aspx#Anchor_2

once MSBuild15 had been installed on my machine a lot of things started going wonky, especially since msbuild15 and msbuild14 can have very different opinions about which project files are valid

I've had to start strictly enforcing the tool version in my FAKE build scripts, since it doesn't care enough about the number the project specifies

Target "Build" (fun _ ->
    !! solutionFile
    |> MSBuildReleaseExt "" [
            "VisualStudioVersion", "14.0"
            "ToolsVersion"       , "14.0"  
    ] "Rebuild" |> ignore
)

Although the toolsversion number isn't even the best baromterer, a project can be set to use tools version 15 but still have the old msbuild format (aka the default for F# projects in vs2017) I just had to make an adjustment to Paket to account for that.

It might be necessary to start calling the project cracker with explicit global properties set for the version you want it to use.

Microsoft.Build.Runtime is xplat now and could be coupled with the project cracker so that it only does what it's been explicitly told to and doesn't go searching for a different version to use.

But I thought @enricosada was going to replace all of this with https://github.com/enricosada/dotnet-proj-info anyway

@csmager
Copy link

csmager commented Mar 8, 2017

I've tried and can't actually get my PC back to a state where this works.

As a workaround I've amended FSharp.Compiler.Service.ProjectCrackerTool.exe.config for the copy used by ionide to add a redirect:

<dependentAssembly>
  <assemblyIdentity name="Microsoft.Build.Tasks.Core" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
  <bindingRedirect oldVersion="15.1.0.0" newVersion="14.0.0.0"/>
</dependentAssembly>

@robkuz
Copy link
Author

robkuz commented Mar 8, 2017

@csmager where do I find FSharp.Compiler.Service.ProjectCrackerTool.exe.config?

@csmager
Copy link

csmager commented Mar 8, 2017

@robkuz I explain in more detail on the related ionide issue.

@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2017

@cloudRoutine Wow, I never understand this ToolsVersion stuff. MSBuild versioning is really one of the hardest things I've ever dealt with. It's so fractal.

Any idea what we should change that assembly resolver to do? The intent is that it just finds a version of MSBuild and uses it, presumably in order of priority 15 -> 14 -> 12.

@cloudRoutine
Copy link
Contributor

this is why I think it'd be easier just to bundle the runtime and only use it in the way you want it to be, It's basically what I've been doing with my project inspector although I haven't gotten it fully fleshed out yet (and it'll crap out on that edge case I mentioned before).

I'm totally against the approach the project cracker takes, that's why I wrote my own tool so I wouldn't have to use it anymore. Why go searching for things that you may not be able to find when you can carry everything you need with you?

Unzipped it's only 1.5MB, since FCS is already ~42MB unzipped, to me it's not throwing on much extra weight.

I get that the situation was different in the past, there was no xplat msbuild option, but now that it's viable it simplifies the process and adds consistency.

@dsyme dsyme added bug and removed question labels Mar 8, 2017
@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2017

@cloudRoutine Could you outline specifically what change you think we should make? For example do you mean that we should bundle a specific version of Microsoft.Build 15.x.x.x alongside ProjectCrackerTool.exe in this directory of the nuget package?

    ../bin/v4.5/FSharp.Compiler.Service.ProjectCrackerTool.exe ==> utilities/net45
    ../bin/v4.5/FSharp.Compiler.Service.ProjectCrackerTool.exe.config ==> utilities/net45
    ../bin/v4.5/FSharp.Compiler.Service.ProjectCrackerTool.?db ==> utilities/net45

Or do you mean we should get rid of the ProjectCrackerTool.exe altogether? Or .... ? I really want to solve this problem once and for all - if bundling is needed then that's fine, we just need to do it right.

dsyme pushed a commit to dsyme/FSharp.Compiler.Service that referenced this issue Mar 8, 2017
@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2017

@cloudRoutine Here is some bandaid sticking plaster. But I would be really grateful for assistance to fix this problem at its root and find a way to never really have to worry about this again :)

@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2017

I merged the hacky fix above, but I'd be really grateful with help to solve this more robustly.

@cloudRoutine
Copy link
Contributor

we should bundle a specific version of Microsoft.Build 15.x.x.x alongside ProjectCrackerTool.exe in this directory of the nuget package

Yup, that's what I meant. It has it's own personal msbuild tool bundled with it and that's what it always defaults to using. I suppose there should still be the option to supply it with a different tool use in case it needs to bring in a bunch of bespoke targets and tasks to resolve some strange project context but that seems unlikely.

Ultimately I do think the project cracker should be retired and replaced with something better. I certainly don't use it anymore due to its limitations. For the tooling I've been building using workspaces I need a bit more info than it can provide -

type ProjectFileInfo = {
    ProjectId                 : ProjectId
    ProjectGuid               : Guid option
    Name                      : string option
    ProjectFilePath           : string
    TargetFramework           : string option
    FrameworkVersion          : string option
    AssemblyName              : string
    OutputPath                : string
    OutputType                : OutputType
    SignAssembly              : bool
    AssemblyOriginatorKeyFile : string option
    GenerateXmlDocumentation  : string option
    Options                   : string []
    PreprocessorSymbolNames   : string []
    CompileFiles              : string []
    ResourceFiles             : string []
    EmbeddedResourceFiles     : string []
    ContentFiles              : string []
    PageFiles                 : string []
    ScriptFiles               : string []
    OtherFiles                : string []
    References                : string []   
    ProjectReferences         : string []
    Analyzers                 : string []
    LogOutput                 : string
}
  • it doesn't handle netcore projects (although that's not hard to fix)
  • doesn't handle failure gracefully or informatively
  • no easy way to evaluate projects in the larger context of a solution
  • can't extract specific categories of information, what e.g. frameworks does this project target
  • it doesn't extract enough information
  • doesn't cache or track changes

It's hard for me to say what the shape and scope should be for a tool like the project cracker until the
CPS project system gets ironed out and how robust its xplat implementation can be, because I can see it
addressing many of them.

I think @enrico has the right idea dotnet-proj-info and it looks like it'll be better at extracting just the specific information you want. Although I think I want more information from it than he does so I'll probably end up incorporating some parts of it into or running it underneath my projectInspector.

A strongly typed interface over msbuild that can help cut down on the superfluous msbuild evaluation and stage and sequence the evaluation so it can get results back to FCS faster seems like the best role for a projectCracker-like tool to play for now.

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

6 participants