-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid appending x64 to AL path if x64 is already appended #6884
Avoid appending x64 to AL path if x64 is already appended #6884
Conversation
e776dec
to
c7c9410
Compare
… avoids breaks with customers previous workarounds
c7c9410
to
d0a7769
Compare
@@ -3878,7 +3878,7 @@ Copyright (C) Microsoft Corporation. All rights reserved. | |||
|
|||
<PropertyGroup> | |||
<_ALExeToolPath>$(TargetFrameworkSDKToolsDirectory)</_ALExeToolPath> | |||
<_ALExeToolPath Condition="'$(PlatformTarget)' == 'x64'">$(TargetFrameworkSDKToolsDirectory)$(PlatformTarget)\</_ALExeToolPath> | |||
<_ALExeToolPath Condition="'$(PlatformTarget)' == 'x64' and !$(_ALExeToolPath.EndsWith('x64\')">$(TargetFrameworkSDKToolsDirectory)x64\</_ALExeToolPath> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a missing )
. So, .EndsWith('x64\')
-> .EndsWith('x64\'))
Good news is, this change gets rid of a ton of MSB3084 warnings I was getting. Here's the warning I was getting in case others report it:
Task attempted to find "al.exe" in two locations. 1) Under the "C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\x64\x64" processor specific directory which is generated based on SdkToolsPath 2) The x86 specific directory under "C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\x64\x64" which is specified by the SDKToolsPath property. You may be able to solve the problem by doing one of the following: 1) Set the "SDKToolsPath" property to the location of the Microsoft Windows SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah good catch!
Do you happen to know where it had x64\
appended prior to this target running? The original issue suggested it was never getting appended, but it sounds like other packages were manually appending it and now this is conflicting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you happen to know where it had x64\ appended prior to this target running? The original issue suggested it was never getting appended, but it sounds like other packages were manually appending it and now this is conflicting.
Oh, geez...It looks like someone implemented a suggestion from stackoverflow in our codebase to alleviate AL1073
warnings. So your fix in #6484 means that can be removed. Kindly disregard 😊.
Fixes users who previously worked around the issue of msbuild not using the x64 version of AL.exe.
Context
Changes Made
Only append
x64\
to_AlExeToolPath
whenPlatformTarget
==x64
AND_AlExeToolPath
doesn't already end withx64\
Testing
Tested locally
Notes