Fix comparing SYSTEM_PROCESSOR with win32.

[BillyONeal]

This is going to be a processor type like x86, x64, arm, or arm64.

This change was originally authored by @LilyWangLL in https://github.com/microsoft/vcpkg/pull/39475

See https://cmake.org/cmake/help/latest/variable/CMAKE_HOST_SYSTEM_PROCESSOR.html

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.44%. Comparing base (05fce11) to head (d7cbf60). Report is 78 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4374      +/-   ##
==========================================
- Coverage   87.05%   86.44%   -0.61%     
==========================================
  Files          56       56              
  Lines       17354    17378      +24     
==========================================
- Hits        15107    15023      -84     
- Misses       2247     2355     +108     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[nibanks]

@BillyONeal this broke several of our builds in automation

[BillyONeal]

It looks like the values in the repo are broken for non-VS generators, while this change is broken for VS generators. I suppose it can just check for either.

I observe that there's a checked in .pgd file with unusual linker settings getting dragged in here too

[BillyONeal]

Ah, I see. This is a bit o_O:

https://github.com/microsoft/msquic/blob/8a741dc87331a346d38123b886331b240c06c509/CMakeLists.txt#L438-L442

@nibanks Would you like to see a change to consistently compare with CMAKE_SYSTEM_PROCESSOR values or always check for either?

I think the right thing to do is to always supply CMAKE_SYSTEM_PROCESSOR values given the name SYSTEM_PROCESSOR like this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3f8f4d58f..343b3cf47 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -435,10 +435,17 @@ if (NOT MSVC AND NOT APPLE AND NOT ANDROID)
     endif()
 endif()
 
-if (CMAKE_GENERATOR_PLATFORM STREQUAL "")
-string(TOLOWER ${CMAKE_SYSTEM_PROCESSOR} SYSTEM_PROCESSOR)
+if ("${CMAKE_GENERATOR_PLATFORM}" STREQUAL "")
+    string(TOLOWER "${CMAKE_SYSTEM_PROCESSOR}" SYSTEM_PROCESSOR)
 else()
-string(TOLOWER ${CMAKE_GENERATOR_PLATFORM} SYSTEM_PROCESSOR)
+    string(TOLOWER "${CMAKE_GENERATOR_PLATFORM}" SYSTEM_PROCESSOR)
+    # Translate from Visual Studio generator platform values to
+    # CMAKE_SYSTEM_PROCESSOR values (derived from %PROCESSOR_ARCHITECTURE%)
+    if ("${SYSTEM_PROCESSOR}" STREQUAL "win32")
+        set(SYSTEM_PROCESSOR "x86")
+    elseif ("${SYSTEM_PROCESSOR}" STREQUAL "x64")
+        set(SYSTEM_PROCESSOR "amd64")
+    endif()
 endif()
 
 if(WIN32)

But the existing behavior e.g.

https://github.com/microsoft/msquic/blob/8a741dc87331a346d38123b886331b240c06c509/submodules/CMakeLists.txt#L67

checks for both x64 (which would be the CMAKE_GENERATOR_PLATFORM value) and amd64 (which is the CMAKE_SYSTEM_PROCESSOR value).

(And if that seems funny and inconsistent, I'm sorry:

Microsoft Windows [Version 10.0.22631.3737]
(c) Microsoft Corporation. All rights reserved.

D:\>echo %PROCESSOR_ARCHITECTURE%
AMD64

D:\>C:\Windows\Syswow64\cmd.exe
Microsoft Windows [Version 10.0.22631.3737]
(c) Microsoft Corporation. All rights reserved.

D:\>echo %PROCESSOR_ARCHITECTURE%
x86

D:\>

)

[BillyONeal]

If it helps make the choice, I observe that

$/src/bin/winuser/pgo_x64 is used by Visual Studio but not Ninja generators, but $/src/bin/winuser/pgo_x86 is used by Ninja but not Visual Studio. (And the latter is 3 years old)

[BillyONeal]

@BillyONeal this broke several of our builds in automation

@nibanks Would you like to see a change to consistently compare with CMAKE_SYSTEM_PROCESSOR values or always check for either?

Just curious which resolution you would like so I can fix the broken builds?

[nibanks]

@BillyONeal this broke several of our builds in automation

@nibanks Would you like to see a change to consistently compare with CMAKE_SYSTEM_PROCESSOR values or always check for either?

Just curious which resolution you would like so I can fix the broken builds?

I really don't have a strong preference so long as we don't break anything.

[dg0yt]

Still an issue.

[dg0yt]

@BillyONeal @nibanks Is there no capacity to check/apply the suggestion and eventually merge the PR?

[nibanks]

@BillyONeal @nibanks Is there no capacity to check/apply the suggestion and eventually merge the PR?

I'm fine with comparing both.