Architecture independent CMake config version file may become architecture dependent

[Quincunx271]

The workaround to make the Catch2ConfigVersion.cmake file architecture independent prior to CMake 3.14 with unset(CMAKE_SIZEOF_VOID_P) before write_basic_package_version_file(...) has a bug: afterunset(CMAKE_SIZEOF_VOID_P), ${CMAKE_SIZEOF_VOID_P} may expand to something other than the empty string. https://cmake.org/cmake/help/latest/command/unset.html#unset-normal-variable-or-cache-entry states:

[U]nsetting a normal variable can expose a cache variable that was previously hidden. To force a variable reference of the form ${VAR} to return an empty string, use set(<variable> ""), which clears the normal variable but leaves it defined.

Catch2/CMakeLists.txt

Lines 140 to 147 in 2f631bb

	set(CATCH2_CMAKE_SIZEOF_VOID_P ${CMAKE_SIZEOF_VOID_P})
	unset(CMAKE_SIZEOF_VOID_P)
	write_basic_package_version_file(
	"${CMAKE_CURRENT_BINARY_DIR}/Catch2ConfigVersion.cmake"
	COMPATIBILITY
	SameMajorVersion
	)
	set(CMAKE_SIZEOF_VOID_P ${CATCH2_CMAKE_SIZEOF_VOID_P})

The fix is simple: change unset(CMAKE_SIZEOF_VOID_P) to set(CMAKE_SIZEOF_VOID_P "")

[horenmar]

Okay, I'll fix it.

I wonder though, did you find this by inspection or because things broke?

[Quincunx271]

Found it by inspection. I previously saw the unset(...) and wasn't sure if it was right, because CMake variable rules are strange, so I finally looked it up.

IIUC, this would only break if someone wrote set(CMAKE_SIZEOF_VOID_P ... CACHE ...), which is an odd thing to do, but might possibly make sense in a toolchain file (I'm not sure, though).

---

Looking at the commit, I don't think that fix works, because the unset will just unset the set applied just before it. Just remove the unset and it should work.

[horenmar]

🤦‍♂ That's what happens when I multitask on issues.