phmonte / Buildalyzer

A utility to perform design-time builds of .NET projects without having to think too hard about it.
MIT License
589 stars 92 forks source link

Changes to project.assets.json vs running dotnet build #202

Closed faldor20 closed 2 years ago

faldor20 commented 2 years ago

After running buildalyzer using the command below in a complex project (multiple other projects referenced, and a fair number of libraries) there are subtle changes to project.assets.json. this means that running donet build be forced to do a full rebuild, despite there being no actual project changes, which is a real pain.

let private project(fsproj: FileInfo): ProjectAnalyzer =
    let options = new AnalyzerManagerOptions()
    options.LogWriter <- !diagnosticsLog // TODO this doesn't follow ref changes
    let manager = AnalyzerManager(options)//TODO: i may be able to only make one instance oof this, though beucase the project inof is cached in projmanager we may get double caching
    manager.GetProject(fsproj.FullName):?>ProjectAnalyzer

    let opt = new Environment.EnvironmentOptions()
    opt.TargetsToBuild.Remove("Clean")|>ignore
    opt.EnvironmentVariables.Add("IntermediateOutputPath", Path.Combine(fsproj.Directory.FullName, ".fslsp", "obj" ))
    opt.EnvironmentVariables.Add("OutputPath", Path.Combine(fsproj.Directory.FullName, ".fslsp", "bin"))
    let builds = project(fsproj).Build(opt)

Is there some workaround for this?

The changes I've seen include: Changing file paths to start with c:\ instead of C:\ Changing from "NETStandard.Library/2.0.3" to "NETStandard.Library/2.0.0" similar changes of minor versions. Here are some example files: dotnet build.txt

project.assets.txt

daveaglick commented 2 years ago

That behavior certainly isn't by design - Buildalyzer's job is to find and shell out to MSBuild or dotnet build with the right parameters for a "design-time" build and add an injected logger to capture the build process through a pipe. Other than that, any side effects are being caused by MSBuild itself. Buildalyzer doesn't directly mess with project files, intermediate files, or output files.

It looks like you're changing the intermediate path for the Buildalyzer driven build too, so that's even more surprising - I wouldn't expect anything to happen to the "real" project.assets.json file in that case - do you get a project.assets.json file in the obj directory you're using for the Buildalyzer build?

I also noticed it looks like you're referencing Buildalyzer in the project being built? I.e. for a codegen scenario or something? I don't actually think that's a problem, just interesting.

The thing that's more interesting here is the change to NETStandard.Library. That suggests that perhaps Buildalyzer is finding a different version of dotnet build then what you're using from the CLI. Locating the right MSBuild or SDK isn't trivial and while I think we're covering most of the possible situations right now, it's always possible something on your system is causing Buildalyzer to find a different SDK. I'd be curious what the full output from Buildalyzer calling dotnet build is and to compare that to when you run it yourself. It looks like you're already setting the AnalyzerManagerOptions.LogWriter so you should be able to grab it from there (or set it to a StringWriter and dump the resulting string).

Another way to compare is to output binary log files from both Buildalyzer and your own build. That will let you see exactly what MSBuild is doing and why, including writes to the project.assets.json file. You can output a binary log file from the Buildalyzer build using ProjectAnalyzer.AddBinaryLogger("path-to-binlog");

TL;DR: I've got no idea why MSBuild would be changing the project.assets.json file out from under you, but it's almost certainly due to some sort of difference between how Buildalyzer runs dotnet build, what SDK it runs it from, and what environment variables are set and what happens when you run it yourself. Let me know what you find!

faldor20 commented 2 years ago

Okay so i have done some more testing. It seems that the problem may be at least partially related to paket. I switched all my dependencies to using normal references and that seemed to fix things after i deleted all the obj folders and rebuilt.

I am using buildalyzer in https://github.com/fsprojects/fsharp-language-server for project cracking. Which is why this issue is so annoying, each time i run dontnet build it requires re-cracking and vice versa.

Log for buildalyzer: I'm building the expecto testing project because it has the most deps.

buildlogProjectCracker.fsproj.log buildlogLSP.fsproj.log buildlogFSharpLanguageServer.fsproj.log buildlogFSharpLanguageServer.Expecto.fsproj.log buildlogReferencedProject.fsproj.log

Here is the output of dotnet build -v d: dotnet.log

I couldn't get a binlog it always caused an exception:

 Failed to build FSharpLanguageServer.Expecto.fsproj: Could not load type 'Microsoft.Build.Framework.ProjectEvaluationFinishedEventArgs' from assembly 'StructuredLogger, Version=2.1.0.0, Culture=neutral, PublicKeyToken=d4c7181801cb6448'.
   at Buildalyzer.Logging.EventProcessor.StatusEventRaised(Object sender, BuildStatusEventArgs e)
   at Microsoft.Build.Logging.EventArgsDispatcher.Dispatch(BuildEventArgs buildEvent)
   at MsBuildPipeLogger.PipeLoggerServer`1.Read()
   at MsBuildPipeLogger.PipeLoggerServer`1.ReadAll()
   at Buildalyzer.ProjectAnalyzer.BuildTargets(BuildEnvironment buildEnvironment, String targetFramework, String[] targetsToBuild, AnalyzerResults results)
   at Buildalyzer.ProjectAnalyzer.Build(String targetFramework, BuildEnvironment buildEnvironment)
   at Buildalyzer.ProjectAnalyzer.Build(String targetFramework, EnvironmentOptions environmentOptions)
   at Buildalyzer.ProjectAnalyzer.Build(EnvironmentOptions environmentOptions)
   at ProjectCracker.inferTargetFramework(FileInfo fsproj) in /home/eli/Programming/FSharp/fsharp-language-server/src/ProjectCracker/ProjectCracker.fs:line 496

My code


let private project(fsproj: FileInfo): ProjectAnalyzer*StringWriter =
    let options = new AnalyzerManagerOptions()
    let writer=new StringWriter()
    options.LogWriter <- writer // !diagnosticsLog // TODO this doesn't follow ref changes
    let manager = AnalyzerManager(options)//TODO: i may be able to only make one instance of this, though because the project inof is cached in projmanager we may get double caching
    manager.GetProject(fsproj.FullName):?>ProjectAnalyzer,writer

let private inferTargetFramework(fsproj: FileInfo): AnalyzerResult =
    let opt = new Environment.EnvironmentOptions()
    opt.TargetsToBuild.Remove("Clean")|>ignore
    opt.EnvironmentVariables.Add("IntermediateOutputPath", Path.Combine(fsproj.Directory.FullName, ".fslsp", "obj" ))
    opt.EnvironmentVariables.Add("OutputPath", Path.Combine(fsproj.Directory.FullName, ".fslsp", "bin"))  
    let projAnalyzer,logs = project(fsproj)
    projAnalyzer.AddBinaryLogger($"./build{fsproj.Name}.binlog")
    let builds=projAnalyzer.Build(opt)
    logs.Flush()
    logs.Close()
     File.WriteAllText($"./buildlog{fsproj.Name}.log",logs.GetStringBuilder().ToString())
daveaglick commented 2 years ago

That's good detective work.

So if I had to make a guess, I'd say that Paket is instrumenting the build in a way that changes or results in slightly different dependency resolution, which gets cached to the project.assets.json file. That's valid and allowed, though a little unusual for a third-party package (though there are obviously lots of packages that carry MSBuild targets with them - just usually not as integrated as Paket probably is). So when you run dotnet build, Packet instruments the build, and does one thing. Now my guess is that for whatever reason, either Paket isn't being injected into the build or is behavior differently when Buildalyzer builds the project, and that's resulting in a different set of dependencies, which results in change to the cached project.assets.json.

A secondary problem now appears the be the inability to have Buildalyzer output a binlog. That's potentially blocking a good diagnosis because comparing a binlog from a normal dotnet build with what Buildalyzer does is going to be our best way of tracking down what's different (the diagnostic MSBuild log files are usually too dense for this kind of comparison). Any thoughts on the binlog issue? Maybe you've got local Microsoft.Build or MSBuild.StructuredLogger references? In any case, I'm having trouble reproducing so I'm going to work this binlog issue for a bit and see if I can't figure out why you can't produce one.

daveaglick commented 2 years ago

I've tried a bunch of times to reproduce the error getting a binlog here, and still can't seem to replicate it. I wondered if it has something do with the target framework of the host application, but from the logs above it looks like you've got the .NET 6 SDK installed, so that's easy enough for me to duplicate. I also tried going from .NET 6 to .NET 5, to .NET Core 3.1 and all produced a binlog without errors.

That brings me back to this:

Maybe you've got local Microsoft.Build or MSBuild.StructuredLogger references?

So I got curious an tried to see if I could figure out whether your host app already had either reference which might be conflicting with the ones from Buildalyzer. Sure enough:

image

So I think that's what's going on with the binlog problems. The good news is that it's not going to be a wholesale problem for everyone, though the bad news is that I'm not really sure how to resolve the conflict just for you. I suppose maybe using an extern alias/PackageReference alias might do the trick in this case.

faldor20 commented 2 years ago

Thanks so much for continuing to persevere with this. I did some very extensive testing with this and confirmed a few things.

  1. I can produce a binlog, There is just some sort of problem with using the package via packet that makes it not correctly get its dependencies, I just opted to add it as a usual package reference.
  2. The problems with buildalyzer creating different project.assets.json are not only related to paket, It is much worse with paket, involving changing some of the actual structure, however, I do still have a problem with non-paket projects where the drive lettes in paths change from being lower-case to uppercase.
  3. This may no longer matter much to me because I am trialing using https://github.com/ionide/proj-info which was its own whole nightmare to get working but is really much more suited to my porpuses being designed specifically for project cracking which is what I want to do. Plus it doesn't seem to generate any build artifacts or touch the project.assets.json at all, though I suspect this is because it doesn't do any sort of proper build
  4. After comparing the binlogs I could see essentially no difference, just a few things moving around, and times changing Honestly, i think this issue is so niche that it probably isn't worth pursuing and, it seems like it's possible that it should be worked on by the paket people instead.

Feel free to close this issue. Thanks again for all your help, you went way above and beyond :)

daveaglick commented 2 years ago

I'll go ahead and resolve this for now, but it's still in the back of my mind. Would be interesting to do some additional analysis when I've got extra time (ha! like "extra time" is ever going to happen :smile:).

Thanks for sticking with me as we analyzed this one - it was definitely interesting. So many tricky little edge cases and gotchas in this stuff.