Skip to content
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

Use canonical names like x86, x64 for architecture #2290

Closed
vatsan-madhavan opened this issue Apr 5, 2020 · 3 comments
Closed

Use canonical names like x86, x64 for architecture #2290

vatsan-madhavan opened this issue Apr 5, 2020 · 3 comments

Comments

@vatsan-madhavan
Copy link

vatsan-madhavan commented Apr 5, 2020

I'm a long time Cmder user and for idiosyncratic reasons I've historically run the 32-bit version of cmd.exe, so I didn't encounter this problem until just now.

I was building dotnet/wpf just now and noticed that it had trouble building correctly.

I debugged it down to these lines that fail to initialize the MSBuild property $(Architecture) correctly (it's initialized to 64 instead of x64), which in turn lead to my (dev) build failures in dotnet/wpf.

    <Architecture Condition="'$(Platform)'=='x64' or '$(Platform)'=='x86' or '$(Platform)'=='arm' or '$(Platform)'=='arm64'">$(Platform)</Architecture>
    <Architecture Condition="'$(Platform)'=='Win32'">x86</Architecture>
    <Architecture Condition="'$(Architecture)'==''">x64</Architecture>

Ultimately, this seems to the what caused my troubles, where %architecture% is (seemingly) set for script-local use, but the environment-variable ultimately leaks out of the script:

cmder/vendor/init.bat

Lines 125 to 132 in 1071221

:: Pick right version of clink
if "%PROCESSOR_ARCHITECTURE%"=="x86" (
set architecture=86
set architecture_bits=32
) else (
set architecture=64
set architecture_bits=64
)

I have two suggestions:

  • Use setlocal at the top of the batch file wherever it makes sense. I realize that the 'fix' may not be as simple as adding a one-line change, since some env-vars might be exported deliberately, whereas others might be leaking merely as a side-effect.
  • Consider using (more) canonical names like x86, x64, arm, arm64 etc. for architecture (instead of 86, 64 etc. ).
@daxgames
Copy link
Member

daxgames commented Apr 6, 2020

@vatsan-madhavan
Copy link
Author

Thanks for handling this quickly!

@vatsan-madhavan
Copy link
Author

vatsan-madhavan commented Apr 6, 2020

Tested - works well.

Feel free to close - I'll look for this in the next release.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants