Updating locator from 1.4.1 to 1.5.3 breaks x86 processes

[AArnott]

This simple upgrade PR updates Microsoft.Build.Locator from 1.4.1 to 1.5.3, the PR validation shows that the program now fails, exclusively when run in an x86 process. x64 is fine.

https://dev.azure.com/andrewarnott/OSS/_build/results?buildId=6997&view=logs&j=0bc77094-9fcd-5c38-f6e4-27d2ae131589&t=9d945b7d-bc5d-5e49-44ea-3bb51e12f2d5

[xUnit.net 00:00:25.18]     BuildIntegrationLibGit2Tests.PublicRelease_RegEx_SatisfiedByCI(serverProperties: [[APPVEYOR_REPO_TAG_NAME, ], [BUILD_SOURCEBRANCH, refs/heads/release], [APPVEYOR_PULL_REQUEST_NUMBER, ], [APPVEYOR_REPO_TAG, ], [APPVEYOR, ], ...]) [FAIL]
  Error Message:
   System.BadImageFormatException : An attempt was made to load a program with an incorrect format. (0x8007000B)
  Stack Trace:
     at BuildIntegrationTests.Init()
   at BuildIntegrationTests..ctor(ITestOutputHelper logger) in D:\a\1\s\test\Nerdbank.GitVersioning.Tests\BuildIntegrationTests.cs:line 117
   at BuildIntegrationLibGit2Tests..ctor(ITestOutputHelper logger) in D:\a\1\s\test\Nerdbank.GitVersioning.Tests\BuildIntegrationTests.cs:line 71
  Failed BuildIntegrationLibGit2Tests.GetBuildVersion_CustomBuildNumberOffset [1 ms]
  Error Message:
   System.BadImageFormatException : An attempt was made to load a program with an incorrect format. (0x8007000B)
  Stack Trace:
     at BuildIntegrationTests.Init()
   at BuildIntegrationTests..ctor(ITestOutputHelper logger) in D:\a\1\s\test\Nerdbank.GitVersioning.Tests\BuildIntegrationTests.cs:line 117

Is the Locator finding the x64 binaries when it should be finding x86 binaries?

[rainersigwald]

Do you have an x64 .NET SDK installed, and no x86 SDK? That is typical and would explain this, I think.

Forgind, we probably need to fall back to the old behavior in this case.

[AArnott]

No, I explicitly install an x86 SDK in this step. This was required to get this to work in the first place.

[Forgind]

I'm a little unclear on what exactly the problem is. BadImageFormatException to me sounds like you're loading an x64 MSBuild into your x86 process, as rainersigwald alluded to. How did you choose from the SDK you load MSBuild from? Even if you have the x86 SDK installed, if you choose an x64 SDK, that should still fail.

So I guess the change in behavior here is that it implicitly checked the bitness of the current process and chose an SDK from which to call dotnet --info, and now it doesn't do that; it just chooses the first one on the path. Rather than adding a fallback for x86 processes, I'd propose continuing through PATH until we find one including Program Files (x86) and take that SDK if we're an x86 process. Seem reasonable?

[Forgind]

I spoke with marcpopMSFT about this, and he said you may be getting bitten by the fact that the SDK no longer adds SDKs that don't match your architecture to the PATH upon installation. In other words, what I proposed above by itself would not work, but he also said he wouldn't expect rainersigwald's suggestion of falling back to the old behavior to work in the long run.

His suggestion is to add the path to the x86 SDK to the path yourself. If you want something that can handle multiple SDKs locations on the path, that's a reasonable ask, and we can implement my suggestion.

[AArnott]

How did you choose from the SDK you load MSBuild from?

By explicitly running the dotnet.exe from the x86 SDK, as you can see here.
C:\hostedtoolcache\windows\x86\dotnet\sdk\6.0.301\MSBuild.dll -nologo -bl:D:\a\1\a/build_logs/test.binlog -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,C:\hostedtoolcache\windows\x86\dotnet\sdk\6.0.301\dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,C:\hostedtoolcache\windows\x86\dotnet\sdk\6.0.301\dotnet.dll -maxcpucount -nodereuse:false -property:VSTestSetting=D:\a\1\s\azure-pipelines\test.runsettings -property:VSTestTestCaseFilter=TestCategory!=FailsInCloudTest -property:VSTestLogger=trx -property:VSTestDiag=D:\a\1\a\test_logs\diag.log;TraceLevel=info -property:VSTestNoBuild=true -property:VSTestCollect=Code Coverage%3bFormat=cobertura -property:VSTestBlameCrash=true -property:VSTestBlameHang=true -property:VSTestBlameHangTimeout=60s -property:Configuration=Release -target:VSTest -verbosity:m /clp:Summary;ForceNoAlign D:\a\1\s\Nerdbank.GitVersioning.sln

I'd propose continuing through PATH until we find one including Program Files (x86) and take that SDK if we're an x86 process.

The x86 SDK may not be on the PATH. It certainly isn't under Program Files (x86) in this case.

the SDK no longer adds SDKs that don't match your architecture to the PATH upon installation

These SDKs aren't installed by MSI. They are installed via the dotnet-install.ps1 script. And anyway, this scenario works with the SDKs I'm installing. It's the upgrade of the MSBuild Locator that broke this.

[jzabroski]

This would be so much easier if we stopped and built unit tests using my suggestion of building docker scaffolding.

[Forgind]

This would be so much easier if we stopped and built unit tests using my suggestion of building docker scaffolding.

With the low frequency of releasing MSBuildLocator, we generally think it's easier to just test it manually. That said, if you want to write unit tests that:

1. Don't break our build
2. Run reasonably fast
3. Catch real errors
4. Are simple enough that they're fairly easy to review

Then I, at least, would be happy to look at them and potentially merge them.

[YuliiaKovalova]

Hi Andrew Arnott (@AArnott) ,

Do you have any issues now with x86? I can see the repo uses 1.5.5 version now.

https://github.com/dotnet/Nerdbank.GitVersioning/blob/9631e3504847614443080f80d17312d81869e171/Directory.Packages.props#L20

[AArnott]

The issue still exists.

I just worked around it with an ugly hack that requires me to hard-code the absolute path on the machine as to where the x86 SDK ought to be found.

[YuliiaKovalova]

Chet Husk (@baronfel), we need your opinion here
After delivering this fix #230, this issue wouldn't be relevant if DOTNET_ROOT(x86) env is set. But Andrew uses install-script for sdk acquisition that doesn't set any variables.

[AArnott]

Andrew uses install-script for sdk acquisition that doesn't set any variables.

If you're referring to my init.ps1 script, it can and does set env vars as part of its work, particularly when installing user-local or repo-local runtimes. I don't think it does for machine-wide installs, which SXS x86/x64 dotnet installs unfortunately requires. But I could enhance that so that a machine-wide x86 .NET install sets DOTNET_ROOT(x86) if that would help.
Does that env var get set naturally by installing the MSI?