Al.exe finds the correct tool based on platform

[benvillalobos]

Finally puts #5981 to bed.
(to trigger an auto-close: fixes #5981)

Context

The underlying problem with the Al issues is that the Al task finds its own tool based on the current process' architecture. This means:

x86 msbuild.exe -> x86 Al.exe
x64 msbuild.exe -> x64 Al.exe

Now, if your x86 msbuild.exe is building with Platform=x64, the x86 Al.exe gets passed platform:x64 and logs its own warning because of mismatched platforms.

So we fixed that by checking if the platform was x64, then look in the x64 tools directory in common.targets.

Now we're hitting problems where the x64 msbuild.exe is calling the x64 Al.exe and being passed platform:x86, causing the reverse of the original issue!

Changes Made

The Al task checks the platform that was passed. If it's x86, it will find the 32 bit al.exe. If x64, it will append x64 to the path before finding the tool.

This also reverts appending x64 to the tools directory in common.currentversion.targets before calling the Al task. It shouldn't have worked in today's x64 msbuild.exe, but does because of the x86 fallback behavior. Apending x64 before the task is called is no longer required.

Testing

Notes

I allowed _ALExeToolPath to be overridden to account for projects that may be using it today with older msbuild binaries.

A mismatched common.currentversion.targets & microsoft.build.dll can fail.

Old targets & new dll: if platform is x64 it will double append x64, but the fallback will find the x64 al.exe.
no fix required.
if platform is x86 and x64 is appended to the path: will likely find the x64 tool and log the error.
fix: customer should no longer append x64 to the path.
new targets & old dll: for platform=x64, x64 needs to be manually appended. The "source of truth" is setting AlToolPath to end in x64.
for platform=x86, no fix needed.

[rainersigwald]

I don't think I like adding this since it's redundant with AMD64. Can it be avoided, perhaps by mapping x64->amd64 higher in the callstack?

[benvillalobos]

You mean modifying the switch case to be "x64" => Path.Combine(sdkToolsPath, "x64"),? Or checking if the platform is x64, then pass AMD64 into GeneratePathToTool?

I'll add another switch 1 stack frame higher than GeneratePathToTool

[rainersigwald]

I was thinking the latter (around the archToLookFor thing).

[ladipro]

The task has a Prefer32Bit parameters which also plays a role in the tool invocation. It is intentional to ignore it here and use only Platform, is that correct?

[Forgind]

I think that's only if the platform is AnyCPU.

[benvillalobos]

The task has a Prefer32Bit parameters which also plays a role in the tool invocation. It is intentional to ignore it here and use only Platform, is that correct?

I was sort of torn here. Prefer32Bit has existed here for a while, but I decided not to use it for the sake of the customer. I haven't seen a single customer want to use x64 Al on a 32 bit platform (and vise-versa), and I'm not sure a customer would want that warning? If we take on Prefer32Bit (which csc uses), we increase complexity unless it's some new AlPrefer32Bit which is a new opt in for customers.

If we just use Platform customer's don't need to do anything and it "just works."

[Forgind]

Nit:
Delete extra space in untouched file.

[Forgind]

I think that's only if the platform is AnyCPU.

[Forgind]

Is this one of those cases where SdkToolsPathUtility.FileInfoExists is secretly slower than f => SdkToolsPathUtility.FileInfoExists(f)?

[benvillalobos]

😮 can you post an example of us making that change?

[Forgind]

#6983

[rainersigwald]

Yeah, this thing is devious. It's not a big deal here since this isn't a hot loop but please go ahead and fix.

[rainersigwald]

Yeah, this thing is devious. It's not a big deal here since this isn't a hot loop but please go ahead and fix.