microsoft / dotnet-framework-docker

The repo for the official docker images for .NET Framework on Windows Server Core.
https://hub.docker.com/_/microsoft-dotnet-framework
MIT License
698 stars 335 forks source link

Put 64-bit MSBuild on PATH #971

Closed rainersigwald closed 2 years ago

rainersigwald commented 2 years ago

Visual Studio 2022 changed the "Developer Command Prompt for VS 2022" PATH setting to reference the 64-bit copy of MSBuild folder, corresponding with the change to make devenv.exe/Visual Studio itself a 64-bit process.

I think the Docker container experience should match, but there is some risk here: some projects that build with a 32-bit MSBuild will fail with a 64-bit one. Those projects won't build on the standard VS command line, so there should be very few, but it's worth considering the impact (and I don't have a great way to quantify either way).

mthalman commented 2 years ago

Is there more context around the discovery of this that can be linked to?

mthalman commented 2 years ago

Are there scenarios that are broken by default by not having the PATH set to amd64?

NickCraver commented 2 years ago

FWIW we hit this internally - everywhere was using MSBuild 64-bit except official builds using this image which wasn't an issue until another fix actually changed a Task DLL from MSIL to AMD64 alerting us to the difference. That's when we found out all other builds, local sec, etc. are 64-bit but docker image is firing up the 32-bit path.

Practically, I'd want the image to match all other build experiences and would call the current behavior unexpected!

richlander commented 2 years ago

Our policy should and has been to match VS, so this change looks good. We'll have to make an announcement for it.

rainersigwald commented 2 years ago

Are there scenarios that are broken by default by not having the PATH set to amd64?

The basic scenario is that a (very slightly) misauthored build can work in VS and the VS desktop command line but fail in a Docker-based build, like @nickcraver's team hit.

The flip side is that if a build is (slightly) misauthored the other direction, it might work only in a 32-bit MSBuild--that's "reference a 32-bit-only task but don't say it's 32-bit-only". However since the change to the Docker images is coming well after the VS change itself, hopefully those builds have been fixed already. https://devblogs.microsoft.com/dotnet/msbuild-and-64-bit-visual-studio-2022/ has symptoms and mitigations for the problems folks are likely to encounter and hopefully has enough search-engine juice that any issues can be quickly resolved.

I do tend to agree that the strongest argument to take this is "consistency".

NickCraver commented 2 years ago

FWIW, we hit another set of issues ultimately do to this today (ASPNETCOMPILER pathing being 32-bit as a downstream result). What can we to do help get this into an image as soon as possible?

mthalman commented 2 years ago

FWIW, we hit another set of issues ultimately do to this today (ASPNETCOMPILER pathing being 32-bit as a downstream result). What can we to do help get this into an image as soon as possible?

We release updates from this repo on a monthly basis. This change will be rolled out next Tuesday. See https://github.com/microsoft/dotnet-framework-docker/discussions/973.

NickCraver commented 2 years ago

Ah awesome, thanks for the update! I guess I was confused with this not being merged yet - does everything to merged in and upstreamed around the same time with the process here?

mthalman commented 2 years ago

Ah awesome, thanks for the update! I guess I was confused with this not being merged yet - does everything to merged in and upstreamed around the same time with the process here?

Yes. This isn't a very active repo so we only really maintain a single branch. The main branch is kept "clean" until release day to allow us to release any unexpected hotfixes if necessary. So that's all to say that this will get merged Tuesday morning.

NickCraver commented 2 years ago

Great to know for the future - I appreciate the additional detail :) Thanks for helping us out here!