dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

Order of projects in solution file affects build results #6198

Open chtenb opened 3 years ago

chtenb commented 3 years ago

Visual Studio Version: 16.8.6

Summary:

The order in which projects are listed in the solution (.sln) file affects the results of the building process. In my case, for some orders the build results are correct, and for others they are not. It seems to me that this is a bug and the order of the projects should not matter at all, since the correct references are set and the derived build order seems valid in both cases.

Moreover, this problem only occurs when executing MSBuild Rubjerg.Graphviz.sln from the commandline (Developer Command Prompt for VS 2019). From within Visual Studio the build always succeeds. MSBuild version is the newest: 16.8.3.61104.

Context:

I encountered this problem in a very small opensource project, so it's easy for other people to reproduce. The solution (Rubjerg.Graphviz.sln) consists of a single .NET library (Rubjerg.Graphviz.csproj) with a dependency on a single C++ library (GraphvizWrapper.vcxproj). The remaining projects are test projects.

Steps to Reproduce:

  1. Check out the master branch of https://github.com/Rubjerg/Graphviz.NetWrapper

  2. Restore the solution nuget packages: nuget restore Rubjerg.Graphviz.sln.

  3. Build the solution on a command prompt: MSBuild Rubjerg.Graphviz.sln.

  4. Run the unittests on a command prompt: packages\nunit.consolerunner\3.12.0\tools\nunit3-console.exe Rubjerg.Graphviz.Test\bin\x64\Debug\net48\Rubjerg.Graphviz.Test.dll

  5. Notice that the build succeeds and the tests pass. This is also indicated by the github workflow run on the latest commit of the master branch.

  6. Clear all build artifacts (e.g. by running git clean -dfx in the root of the repository)

  7. Change the solution file according to the diff shown in this reproduction scenario

  8. Repeat step 2, 3 and 4

Expected Behavior: Succeeding build and succeeding unit tests.

Actual Behavior: Notice that after step 2 the C++ DLLs are not present in the output directories of the managed projects. If you execute step 3, you will notice the System.DllNotFoundExceptions. This is also indicated by the github workflow run on the reproduction scenario.

User Impact: All clients of our project run into this problem and have to meddle with the project order in the solution file to fix it.

benvillalobos commented 3 years ago

Team Triage: Verify the repro works & gather some info, then add the untriaged label.

benvillalobos commented 3 years ago

So I got it to repro and captured binlogs of each build: solutions.zip

Unfortunately I couldn't repro after that. The tests seemed to work for me with the updated solution. Are you still seeing this with an updated msbuild?

It actually looks like you may have fixed the issue with https://github.com/Rubjerg/Graphviz.NetWrapper/pull/35/files?

The general idea here is you want to use ProjectReference to ensure certain projects are built first. When msbuild encounters a .sln file, it creates a metaproj file containing various ProjectReferences. The order of these references happen to be the same order as what it discovered in the .sln file.

chtenb commented 3 years ago

I just re-ran the workflow in the reproduction PR for you, and still the same error occurs: https://github.com/Rubjerg/Graphviz.NetWrapper/pull/33/checks?check_run_id=2465253939

You should be able to reproduce by checking out that PR which is also referred to in the OP.

https://github.com/Rubjerg/Graphviz.NetWrapper/pull/35/files doesn't seem relevant, because the reproduction PR actually branches off from that commit.

image

benvillalobos commented 3 years ago

Using our latest preview Microsoft (R) Build Engine version 16.10.0-preview-21253-02+fa96a2a81 for .NET Framework I can't reproduce this behavior. There's a chance this could have been fixed in the meantime. Could you send the full output of msbuild --version so I can try on that specific version? Along with a specific commit hash (even if latest/master reproduces the behavior).

chtenb commented 3 years ago

On github the build action reports:

> Run msbuild Rubjerg.Graphviz.sln /p:Configuration=Release

Microsoft (R) Build Engine version 16.9.0+5e4b48a27 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
v-codyguan commented 2 years ago

@chtenb I tried to reproduce this issue on 16.8.6 with MSBuild 16.8.3+39993bd9d, and I couldn't reproduce this issue. Could you please help to check the repro steps in case I missed something? And to reproduce this issue, we tried to delete 'Rubjerg.Graphviz.Test.dll' manually from directories after step 3, then execute step 4, and we still couldn't get this issue reproduce, does it make sense? Repro steps:

  1. Clone and check out the master branch of https://github.com/Rubjerg/Graphviz.NetWrapper
  2. Open the solution to restore the packages.
  3. Executing 'MSBuild Rubjerg.Graphviz.sln' on Developer Command Prompt for VS 2019.
  4. Executing 'packages\nunit.consolerunner\3.12.0\tools\nunit3-console.exe'.
  5. Executing 'git clean -dfx'.
  6. Change the solution file.
  7. Repeat step 2, 3 and 4. image
chtenb commented 2 years ago

@v-codyguan It looks like you forgot to pass the test.dll to the testrunner. The full test command of step 4 should be:

packages\nunit.consolerunner\3.12.0\tools\nunit3-console.exe Rubjerg.Graphviz.Test\bin\x64\Debug\net48\Rubjerg.Graphviz.Test.dll

Otherwise your steps look fine. I just repro'd with version 16.11.1+3e40a09f8.

v-codyguan commented 2 years ago

Hi @chtenb thank you for sharing! We can reproduce this issue and contact our engineer in time. Thanks!

v-codyguan commented 2 years ago

Hi @BenVillalobos We verified this issue on 16.8.6 with MSBuild 16.8.3+39993bd9d and 16.11.4 with 16.11.1+3e40a09f8, this issue reproduced after passing the test.dll to the testrunner in step 4. Could you help to take a look? If you need repro machine, please feel free to contact us! Actual

benvillalobos commented 2 years ago

This one's a doozy. I don't fully understand the issue here, but I'm fairly certain it has to do with incremental builds. Marking as needs-attention for bug triage meeting.

note to self: Relevant path where the DLL's need to exist: Graphviz.NetWrapper\Rubjerg.Graphviz.Test\bin\x64\Debug\net48

Side note: Passing /graph to your msbuild invocation should resolve the issue. It did for me

chtenb commented 2 years ago

Side note: Passing /graph to your msbuild invocation should resolve the issue. It did for me

Interesting, I didn't know about that parameter. Any reason why that isn't on by default? And can Visual Studio pass it as well?

benvillalobos commented 2 years ago

Any reason why that isn't on by default?

I don't know the details, just that there are many reasons unfortunately

You should be able to set up Command Line Arguments in the properties of your project. You could specify /graph there.

benvillalobos commented 1 year ago

Dropping some context on this before it all disappears: Judging by the following symptoms:

It's gotta be a build ordering issue, or that a certain project isn't copying the bits to the right place where another project may have been doing that (this is controlled by ReferenceOutputAssembly (see https://github.com/dotnet/msbuild/issues/7986 for notes on that).

So this could be a transitive reference issue, which I'm not too sure of a good solution for.

I think the simplest solution here is for your test project to reference your vcxproj and set ReferenceOutputAssembly=true for it so it can copy the output dll to the right place. This shouldn't cause overbuilds.

Airing out another workaround, you can create a target in your test project that looks for your cpp dll and copies it to where it needs to be in order for this test to run.

Sorry this one took a while to get back to 😅

chtenb commented 1 year ago

Thanks for coming back to this! Unfortunately the two workarounds you proposed are not more convenient for me than using a project order in the solution that works. Having all dependent projects reference this C++ project or add a custom target is unfeasible for large solutions and generally undesirable for a project that is meant to be pretty much plug-and-play.

So this could be a transitive reference issue, which I'm not too sure of a good solution for.

Based on my anecdotal experience with build problems I'd definitely think so. Transitive copying behavior (or the lack thereof) has always caused headaches in my projects, especially when C++ projects were involved.

I hoped that I had provided a tangible problem in this area with this github issue + example project, for which the solution or workaround hopefully could be made an example of for how to solve this nagging problem once and for all :)

KirillOsenkov commented 6 months ago

See if https://github.com/dotnet/msbuild/issues/9709 is related