[Issue]: CMake usage of FindPython3 is buggy and not portable

[ScottTodd]

Problem Description

The setup code and example usage here does not follow recommended practices and is not reliably portable across systems:

rocFFT/library/src/CMakeLists.txt

Lines 85 to 89 in b99dad4

	find_package (Python3 3.6 COMPONENTS Interpreter REQUIRED)
	if( NOT DEFINED PYTHON3_EXE )
	set(PYTHON3_EXE python3)
	endif()

rocFFT/library/src/CMakeLists.txt

Lines 189 to 194 in b99dad4

	add_custom_command(
	OUTPUT ${kgen_embed_cpp}
	COMMAND ${PYTHON3_EXE} ${kgen_embed_command}
	--embed ${kgen_embed_files} --logic ${kgen_logic_files} --output ${kgen_embed_cpp}
	DEPENDS ${kgen_embed_command} ${kgen_embed_files} ${kgen_logic_files}
	)

See the official documentation here: https://cmake.org/cmake/help/latest/module/FindPython3.html. After calling find_package(Python3 COMPONENTS Interpreter), the result variable Python3_EXECUTABLE should be used.

This only happens to work on Windows (where the executable for Python 3 is simply python.exe) today because of a workaround in the toolchain file:

rocFFT/toolchain-windows.cmake

Line 59 in b99dad4

set(PYTHON3_EXE python)

Building on Windows should not require that specific toolchain file though, and the custom PYTHON3_EXE variable could be avoided entirely by using the already available Python3_EXECUTABLE variable.

This is also still not behaving as it should even on Linux, since it ignores the results of the find_package call that might be pointing at a virtual environment or some other explicit Python executable instead of whatever the system resolves python3 to.

Operating System

N/A

CPU

N/A

GPU

N/A

ROCm Version

N/A

ROCm Component

rocFFT

Steps to Reproduce

No response

(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support

No response

Additional Information

The code here seems to have originated in 5c8e3e3

1. I don't really buy the "look for python development files - we also require an interpreter but the version of it needn't match the development version." argument, as Python in CMake doesn't necessarily work that way.
2. The more recent df3b5c1 only looks for Interpreter and not Development anyways, though the result of the search is ignored

I sent a patch for this downstream in TheRock: ROCm/TheRock#189

[ScottTodd]

Here is the downstream patch, now committed: https://github.com/ROCm/TheRock/blob/be435acbfd39e212cd30c8791f04d9adc9a14b91/patches/amd-mainline/rocFFT/0003-Use-Python3_Executable-from-FindPython3-not-custom-P.patch

[harkgill-amd]

Hi @ScottTodd, thanks for bringing this up. We have an internal PR addressing these concerns which should make it's way into the develop branch shortly.

[ScottTodd]

Thanks! Looks like the commit on develop is 9400019.