[K1ngfish3r]

As per https://cmake.org/cmake/help/latest/module/FetchContent.html#integrating-with-find-package

[rwmcguir]

Thanks, this is a good change for distro and offline builds. But it breaks CMake versions older than 3.24, and the project minimum is 3.12.

FIND_PACKAGE_ARGS was added to FetchContent_Declare in CMake 3.24. Older versions do not know the keyword, so it breaks the clone. That is why the Ubuntu 22.04 jobs fail (CMake 3.22):

fatal: could not create work tree dir '': No such file or directory
Failed to clone repository: 'https://github.com/google/googletest.git'

Could you add the keyword only on CMake 3.24 and newer, and keep 1.14.0 as the minimum version?

if(BUILD_L0_LOADER_TESTS)
	set(INSTALL_GTEST OFF)
    # FIND_PACKAGE_ARGS (CMake 3.24+) uses an installed GTest 1.14.0 or newer
    # when one is found, and downloads v1.14.0 otherwise.
    if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.24)
        set(GTEST_FIND_PACKAGE_ARGS FIND_PACKAGE_ARGS 1.14.0 NAMES GTest)
    endif()
    FetchContent_Declare(
        googletest
        GIT_REPOSITORY https://github.com/google/googletest.git
        GIT_TAG        v1.14.0
        ${GTEST_FIND_PACKAGE_ARGS}
    )

GTest's package config uses AnyNewerVersion, so 1.14.0 means "1.14.0 or newer". If the installed version is older, FetchContent falls back to the download.

I tested this locally:

CMake	Installed GTest	Result	Tests
3.22.6	1.14.0	Downloads v1.14.0 (same as today)	builds
3.28.3	none	Downloads v1.14.0	builds
3.28.3	1.12.0	Too old, downloads v1.14.0	builds
3.28.3	1.14.0	Uses the installed GTest	263/263 pass
3.28.3	1.17.0	Uses the installed GTest. The test target builds as C++17, because GTest 1.17 requires it.	263/263 pass

Also, please use spaces for the indentation of the new lines, like the lines around them.

[K1ngfish3r]

Thanks, this is a good change for distro and offline builds. But it breaks CMake versions older than 3.24, and the project minimum is 3.12.
...
Could you add the keyword only on CMake 3.24 and newer, and keep 1.14.0 as the minimum version?

it all makes sense, done, have applied your suggestion directly. lemme know if anything else is necessary
EDIT: i had initially assumed the error to be related to github doing github things (ie: failing to resolve). I remember tons of jobs failing back then. didnt realize it was actually a CMake issue

[github-actions]

Level Zero loader ABI check

last-release (v1.35.0): ✅ no ABI problems. Report

Creating compatibility report ...
Binary compatibility: 100%
Source compatibility: 100%
Total binary compatibility problems: 0, warnings: 0
Total source compatibility problems: 0, warnings: 0

master (master): ✅ no ABI problems. Report

Creating compatibility report ...
Binary compatibility: 100%
Source compatibility: 100%
Total binary compatibility problems: 0, warnings: 0
Total source compatibility problems: 0, warnings: 0

Workflow run

If an ABI change is intended, a reviewer with write access can accept it: leave an approval review on the PR head commit with the keyword abi-failure-approved in the review body. That re-runs the check.

[rwmcguir]

Ahh, the latest CI failures are due to it not being rebased, I am confident in this change, so I will rebase, re-run CI and likely merge.. We'll see in a few minutes.

[rwmcguir]

Looks good, I will be merging this.. Thank you