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 microsoft/vcpkg#39475

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.59%. Comparing base (05fce11) to head (e4cc0ff).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4374      +/-   ##
==========================================
- Coverage   87.05%   86.59%   -0.47%     
==========================================
  Files          56       56              
  Lines       17354    17354              
==========================================
- Hits        15107    15027      -80     
- Misses       2247     2327      +80     

☔ View full report in Codecov by Sentry.
📢 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:

msquic/CMakeLists.txt

Lines 438 to 442 in 8a741dc

	if (CMAKE_GENERATOR_PLATFORM STREQUAL "")
	string(TOLOWER ${CMAKE_SYSTEM_PROCESSOR} SYSTEM_PROCESSOR)
	else()
	string(TOLOWER ${CMAKE_GENERATOR_PLATFORM} SYSTEM_PROCESSOR)
	endif()

@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.

msquic/submodules/CMakeLists.txt

Line 67 in 8a741dc

elseif (${SYSTEM_PROCESSOR} STREQUAL "x64" OR ${SYSTEM_PROCESSOR} STREQUAL "amd64")

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.