Repository: KhronosGroup/SPIRV-Tools (language: python)

Issue:
PYTHON_EXECUTABLE path not in quotations marks causes trouble on Windows.
I recently updated CMake to version 3.19.0-rc3 and now the python path on Windows `C:\Program Files\Python` is not evaluated correctly in CMake's `add_custom_command` because that string is not in quotation marks:
https://github.com/KhronosGroup/SPIRV-Tools/blob/c79edd260c2b503f0eca57310057b4a100999cc5/source/CMakeLists.txt#L34
Putting it into quotation marks solves the problem:
`COMMAND "${PYTHON_EXECUTABLE}" ${GRAMMAR_PROCESSING_SCRIPT}`.
I was wondering if someone else experienced that issue on Windows. We have a few custom changes in the CMake files to make it work for newer CMake versions such as `find_package(Python3 COMPONENTS Interpreter Development)` instead of `find_package(PythonInterp 3 REQUIRED)`.


Discussion / hints:
SPIRV-Tools should probably update its CMake. It's stuck back at 2.8.12, while the rest of the Vulkan ecosystem seems to have moved to 3.10.2.
Once we upgrade, then we can and should fix and clean up these things.

Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 71cdc00c46..2b854a43a6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -250,7 +250,7 @@ if(NOT COMMAND find_host_program)
 endif()
 
 # Tests require Python3
-find_host_package(PythonInterp 3 REQUIRED)
+find_host_package(Python3 REQUIRED)
 
 # Check for symbol exports on Linux.
 # At the moment, this check will fail on the OSX build machines for the Android NDK.
@@ -259,7 +259,7 @@ if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux")
   macro(spvtools_check_symbol_exports TARGET)
     if (NOT "${SPIRV_SKIP_TESTS}")
       add_test(NAME spirv-tools-symbol-exports-${TARGET}
-               COMMAND ${PYTHON_EXECUTABLE}
+               COMMAND Python3::Interpreter
                ${spirv-tools_SOURCE_DIR}/utils/check_symbol_exports.py "$<TARGET_FILE:${TARGET}>")
     endif()
   endmacro()
@@ -358,7 +358,7 @@ endif(ENABLE_SPIRV_TOOLS_INSTALL)
 
 if (NOT "${SPIRV_SKIP_TESTS}")
   add_test(NAME spirv-tools-copyrights
-           COMMAND ${PYTHON_EXECUTABLE} utils/check_copyright.py
+           COMMAND Python3::Interpreter utils/check_copyright.py
            WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 endif()
 
diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index acfa0c123b..748fbf2652 100644
--- a/source/CMakeLists.txt
+++ b/source/CMakeLists.txt
@@ -31,7 +31,7 @@ macro(spvtools_core_tables CONFIG_VERSION)
   set(GRAMMAR_INSTS_INC_FILE "${spirv-tools_BINARY_DIR}/core.insts-${CONFIG_VERSION}.inc")
   set(GRAMMAR_KINDS_INC_FILE "${spirv-tools_BINARY_DIR}/operand.kinds-${CONFIG_VERSION}.inc")
   add_custom_command(OUTPUT ${GRAMMAR_INSTS_INC_FILE} ${GRAMMAR_KINDS_INC_FILE}
-    COMMAND ${PYTHON_EXECUTABLE} ${GRAMMAR_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${GRAMMAR_PROCESSING_SCRIPT}
       --spirv-core-grammar=${GRAMMAR_JSON_FILE}
       --extinst-debuginfo-grammar=${DEBUGINFO_GRAMMAR_JSON_FILE}
       --extinst-cldebuginfo100-grammar=${CLDEBUGINFO100_GRAMMAR_JSON_FILE}
@@ -53,7 +53,7 @@ macro(spvtools_enum_string_mapping CONFIG_VERSION)
   set(GRAMMAR_ENUM_STRING_MAPPING_INC_FILE "${spirv-tools_BINARY_DIR}/enum_string_mapping.inc")
   add_custom_command(OUTPUT ${GRAMMAR_EXTENSION_ENUM_INC_FILE}
      ${GRAMMAR_ENUM_STRING_MAPPING_INC_FILE}
-    COMMAND ${PYTHON_EXECUTABLE} ${GRAMMAR_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${GRAMMAR_PROCESSING_SCRIPT}
       --spirv-core-grammar=${GRAMMAR_JSON_FILE}
       --extinst-debuginfo-grammar=${DEBUGINFO_GRAMMAR_JSON_FILE}
       --extinst-cldebuginfo100-grammar=${CLDEBUGINFO100_GRAMMAR_JSON_FILE}
@@ -75,7 +75,7 @@ macro(spvtools_vimsyntax CONFIG_VERSION CLVERSION)
   set(OPENCL_GRAMMAR_JSON_FILE "${SPIRV_HEADER_INCLUDE_DIR}/spirv/${CONFIG_VERSION}/extinst.opencl.std.100.grammar.json")
   set(VIMSYNTAX_FILE "${spirv-tools_BINARY_DIR}/spvasm.vim")
   add_custom_command(OUTPUT ${VIMSYNTAX_FILE}
-      COMMAND ${PYTHON_EXECUTABLE} ${VIMSYNTAX_PROCESSING_SCRIPT}
+      COMMAND Python3::Interpreter ${VIMSYNTAX_PROCESSING_SCRIPT}
       --spirv-core-grammar=${GRAMMAR_JSON_FILE}
       --extinst-debuginfo-grammar=${DEBUGINFO_GRAMMAR_JSON_FILE}
       --extinst-glsl-grammar=${GLSL_GRAMMAR_JSON_FILE}
@@ -91,7 +91,7 @@ macro(spvtools_glsl_tables CONFIG_VERSION)
   set(GLSL_GRAMMAR_JSON_FILE "${SPIRV_HEADER_INCLUDE_DIR}/spirv/${CONFIG_VERSION}/extinst.glsl.std.450.grammar.json")
   set(GRAMMAR_INC_FILE "${spirv-tools_BINARY_DIR}/glsl.std.450.insts.inc")
   add_custom_command(OUTPUT ${GRAMMAR_INC_FILE}
-    COMMAND ${PYTHON_EXECUTABLE} ${GRAMMAR_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${GRAMMAR_PROCESSING_SCRIPT}
       --extinst-glsl-grammar=${GLSL_GRAMMAR_JSON_FILE}
       --glsl-insts-output=${GRAMMAR_INC_FILE}
       --output-language=c++
@@ -105,7 +105,7 @@ macro(spvtools_opencl_tables CONFIG_VERSION)
   set(OPENCL_GRAMMAR_JSON_FILE "${SPIRV_HEADER_INCLUDE_DIR}/spirv/${CONFIG_VERSION}/extinst.opencl.std.100.grammar.json")
   set(GRAMMAR_INC_FILE "${spirv-tools_BINARY_DIR}/opencl.std.insts.inc")
   add_custom_command(OUTPUT ${GRAMMAR_INC_FILE}
-    COMMAND ${PYTHON_EXECUTABLE} ${GRAMMAR_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${GRAMMAR_PROCESSING_SCRIPT}
       --extinst-opencl-grammar=${OPENCL_GRAMMAR_JSON_FILE}
       --opencl-insts-output=${GRAMMAR_INC_FILE}
     DEPENDS ${GRAMMAR_PROCESSING_SCRIPT} ${CORE_GRAMMAR_JSON_FILE} ${OPENCL_GRAMMAR_JSON_FILE}
@@ -120,7 +120,7 @@ macro(spvtools_vendor_tables VENDOR_TABLE SHORT_NAME OPERAND_KIND_PREFIX)
     set(GRAMMAR_FILE "${spirv-tools_SOURCE_DIR}/source/extinst.${VENDOR_TABLE}.grammar.json")
   endif()
   add_custom_command(OUTPUT ${INSTS_FILE}
-    COMMAND ${PYTHON_EXECUTABLE} ${GRAMMAR_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${GRAMMAR_PROCESSING_SCRIPT}
       --extinst-vendor-grammar=${GRAMMAR_FILE}
       --vendor-insts-output=${INSTS_FILE}
       --vendor-operand-kind-prefix=${OPERAND_KIND_PREFIX}
@@ -134,7 +134,7 @@ endmacro(spvtools_vendor_tables)
 macro(spvtools_extinst_lang_headers NAME GRAMMAR_FILE)
   set(OUT_H ${spirv-tools_BINARY_DIR}/${NAME}.h)
   add_custom_command(OUTPUT ${OUT_H}
-    COMMAND ${PYTHON_EXECUTABLE} ${LANG_HEADER_PROCESSING_SCRIPT}
+    COMMAND Python3::Interpreter ${LANG_HEADER_PROCESSING_SCRIPT}
       --extinst-grammar=${GRAMMAR_FILE}
       --extinst-output-path=${OUT_H}
     DEPENDS ${LANG_HEADER_PROCESSING_SCRIPT} ${GRAMMAR_FILE}
@@ -168,7 +168,7 @@ set_property(TARGET spirv-tools-vimsyntax PROPERTY FOLDER "SPIRV-Tools utilities
 set(GENERATOR_INC_FILE ${spirv-tools_BINARY_DIR}/generators.inc)
 set(SPIRV_XML_REGISTRY_FILE ${SPIRV_HEADER_INCLUDE_DIR}/spirv/spir-v.xml)
 add_custom_command(OUTPUT ${GENERATOR_INC_FILE}
-  COMMAND ${PYTHON_EXECUTABLE} ${XML_REGISTRY_PROCESSING_SCRIPT}
+  COMMAND Python3::Interpreter ${XML_REGISTRY_PROCESSING_SCRIPT}
     --xml=${SPIRV_XML_REGISTRY_FILE}
     --generator-output=${GENERATOR_INC_FILE}
   DEPENDS ${XML_REGISTRY_PROCESSING_SCRIPT} ${SPIRV_XML_REGISTRY_FILE}
@@ -198,7 +198,7 @@ set(SPIRV_TOOLS_BUILD_VERSION_INC_GENERATOR
 set(SPIRV_TOOLS_CHANGES_FILE
   ${spirv-tools_SOURCE_DIR}/CHANGES)
 add_custom_command(OUTPUT ${SPIRV_TOOLS_BUILD_VERSION_INC}
-   COMMAND ${PYTHON_EXECUTABLE}
+   COMMAND Python3::Interpreter
            ${SPIRV_TOOLS_BUILD_VERSION_INC_GENERATOR}
            ${SPIRV_TOOLS_CHANGES_FILE} ${SPIRV_TOOLS_BUILD_VERSION_INC}
    DEPENDS ${SPIRV_TOOLS_BUILD_VERSION_INC_GENERATOR}


Test patch:
diff --git a/test/tools/CMakeLists.txt b/test/tools/CMakeLists.txt
index 4c8989fe3b..37fe2b9786 100644
--- a/test/tools/CMakeLists.txt
+++ b/test/tools/CMakeLists.txt
@@ -13,10 +13,10 @@
 # limitations under the License.
 
 add_test(NAME spirv-tools_expect_unittests
-         COMMAND ${PYTHON_EXECUTABLE} -m unittest expect_unittest.py
+         COMMAND Python3::Interpreter -m unittest expect_unittest.py
          WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 add_test(NAME spirv-tools_spirv_test_framework_unittests
-         COMMAND ${PYTHON_EXECUTABLE} -m unittest spirv_test_framework_unittest.py
+         COMMAND Python3::Interpreter -m unittest spirv_test_framework_unittest.py
          WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 
 add_spvtools_unittest(
diff --git a/test/tools/opt/CMakeLists.txt b/test/tools/opt/CMakeLists.txt
index 21aa247f1f..966ffbb56d 100644
--- a/test/tools/opt/CMakeLists.txt
+++ b/test/tools/opt/CMakeLists.txt
@@ -13,9 +13,9 @@
 # limitations under the License.
 
 if(NOT ${SPIRV_SKIP_TESTS})
-  if(${PYTHONINTERP_FOUND})
+  if(${Python3_Interpreter_FOUND})
     add_test(NAME spirv_opt_cli_tools_tests
-      COMMAND ${PYTHON_EXECUTABLE}
+      COMMAND Python3::Interpreter
       ${CMAKE_CURRENT_SOURCE_DIR}/../spirv_test_framework.py
       $<TARGET_FILE:spirv-opt> $<TARGET_FILE:spirv-as> $<TARGET_FILE:spirv-dis>
       --test-dir ${CMAKE_CURRENT_SOURCE_DIR})