From c35579ad6457869e9593834d125b3f6c8e53fba8 Mon Sep 17 00:00:00 2001 From: Martyn Russell Date: Tue, 14 Nov 2023 08:21:26 +0000 Subject: [PATCH] Fixes #570: Do not remove CMAKE_MODULE_PATH if already set on find_package() (#571) --- conan_provider.cmake | 11 ++-- .../CMakeLists.txt | 48 +++++++++++++++ .../conanfile.txt | 8 +++ .../module_only/CMakeLists.txt | 18 ++++++ .../module_only/conanfile.txt | 5 ++ .../boost}/conanfile.py | 0 .../{libbye => recipes/bye}/conanfile.py | 0 .../recipes/cmake-module-only/conanfile.py | 60 +++++++++++++++++++ .../cmake-module-with-dependency/conanfile.py | 60 +++++++++++++++++++ tests/test_smoke.py | 46 ++++++++++---- 10 files changed, 240 insertions(+), 16 deletions(-) create mode 100644 tests/resources/cmake_module_path/library_with_cmake_module_dir/CMakeLists.txt create mode 100644 tests/resources/cmake_module_path/library_with_cmake_module_dir/conanfile.txt create mode 100644 tests/resources/cmake_module_path/module_only/CMakeLists.txt create mode 100644 tests/resources/cmake_module_path/module_only/conanfile.txt rename tests/resources/{fake_boost_recipe => recipes/boost}/conanfile.py (100%) rename tests/resources/{libbye => recipes/bye}/conanfile.py (100%) create mode 100644 tests/resources/recipes/cmake-module-only/conanfile.py create mode 100644 tests/resources/recipes/cmake-module-with-dependency/conanfile.py diff --git a/conan_provider.cmake b/conan_provider.cmake index 19c4c492..9dbc6837 100644 --- a/conan_provider.cmake +++ b/conan_provider.cmake @@ -524,12 +524,13 @@ macro(conan_provide_dependency method package_name) # this will simply reuse the result. If not, fall back to CMake default search # behaviour, also allowing modules to be searched. if(NOT ${package_name}_FOUND) - #FIXME: https://github.com/conan-io/cmake-conan/issues/570 - set(_cmake_module_path_orig "${CMAKE_MODULE_PATH}") - list(PREPEND CMAKE_MODULE_PATH "${_conan_generators_folder}") + list(FIND CMAKE_MODULE_PATH "${_conan_generators_folder}" _index) + if(_index EQUAL -1) + list(PREPEND CMAKE_MODULE_PATH "${_conan_generators_folder}") + endif() + unset(_index) find_package(${package_name} ${ARGN} BYPASS_PROVIDER) - set(CMAKE_MODULE_PATH "${_cmake_module_path_orig}") - unset(_cmake_module_path_orig) + list(REMOVE_ITEM CMAKE_MODULE_PATH "${_conan_generators_folder}") endif() endmacro() diff --git a/tests/resources/cmake_module_path/library_with_cmake_module_dir/CMakeLists.txt b/tests/resources/cmake_module_path/library_with_cmake_module_dir/CMakeLists.txt new file mode 100644 index 00000000..5b120a8d --- /dev/null +++ b/tests/resources/cmake_module_path/library_with_cmake_module_dir/CMakeLists.txt @@ -0,0 +1,48 @@ +cmake_minimum_required(VERSION 3.24) +project(MyApp CXX) + + +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake") + +# Search only for "Andromeda", which has a requirement on "Orion" +# And both are "MODULE" only - this forces a recursive call to `find_package` via the dependency provider +find_package(Andromeda REQUIRED) + +# Ensure that CMake module path is a list with two values: +# - the `orion-module-subfolder` is first, and the one set above (cmake-source-dir/cmake) is second +# Note: on multi-config generators, CMakeDeps will prepend it twice (one for Debug, one for Release) +get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) +if(is_multi_config) + set(_expected_list_size 3) +else() + set(_expected_list_size 2) +endif() + +list(LENGTH CMAKE_MODULE_PATH _cmake_module_path_length) +if(NOT _cmake_module_path_length EQUAL ${_expected_list_size}) + message(STATUS "CMAKE_MODULE_PATH DOES NOT have expected value 1: ${CMAKE_MODULE_PATH}") +endif() + +list(GET CMAKE_MODULE_PATH 0 _cmake_module_path_first_element) +if(NOT _cmake_module_path_first_element MATCHES "^.*orion-module-subfolder$") + message(STATUS "CMAKE_MODULE_PATH DOES NOT have expected value 2: ${_cmake_module_path_first_element}") +endif() + +if(is_multi_config) + list(GET CMAKE_MODULE_PATH 1 _cmake_module_path_second_element) + if(NOT _cmake_module_path_second_element MATCHES "^.*orion-module-subfolder$") + message(STATUS "CMAKE_MODULE_PATH DOES NOT have expected value 3: ${_cmake_module_path_second_element}") + endif() + set(_expected_cmake_module_path "${_cmake_module_path_first_element};${_cmake_module_path_second_element};${CMAKE_SOURCE_DIR}/cmake") +else() + set(_expected_cmake_module_path "${_cmake_module_path_first_element};${CMAKE_SOURCE_DIR}/cmake") +endif() + +if(CMAKE_MODULE_PATH STREQUAL "${_expected_cmake_module_path}") + message(STATUS "CMAKE_MODULE_PATH has expected value: ${CMAKE_MODULE_PATH}") +else() + message(STATUS "CMAKE_MODULE_PATH DOES NOT have expected value 4: ${CMAKE_MODULE_PATH}") +endif() + diff --git a/tests/resources/cmake_module_path/library_with_cmake_module_dir/conanfile.txt b/tests/resources/cmake_module_path/library_with_cmake_module_dir/conanfile.txt new file mode 100644 index 00000000..64691a78 --- /dev/null +++ b/tests/resources/cmake_module_path/library_with_cmake_module_dir/conanfile.txt @@ -0,0 +1,8 @@ +[requires] +cmake-module-with-dependency/0.1 + +[options] +cmake-module-only/*:with_builddir=True + +[generators] +CMakeDeps diff --git a/tests/resources/cmake_module_path/module_only/CMakeLists.txt b/tests/resources/cmake_module_path/module_only/CMakeLists.txt new file mode 100644 index 00000000..a6d4106c --- /dev/null +++ b/tests/resources/cmake_module_path/module_only/CMakeLists.txt @@ -0,0 +1,18 @@ +cmake_minimum_required(VERSION 3.24) +project(MyApp CXX) + + +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake") + +# Search only for "Andromeda", which has a requirement on "Orion" +# And both are "MODULE" only - this forces a recursive call to `find_package` via the dependency provider +find_package(Andromeda REQUIRED) + +# Ensure that CMake module path has exactly the value it had before +if(CMAKE_MODULE_PATH STREQUAL "${CMAKE_SOURCE_DIR}/cmake") + message(STATUS "CMAKE_MODULE_PATH has expected value") +else() + message(STATUS "CMAKE_MODULE_PATH DOES NOT have expected value") +endif() diff --git a/tests/resources/cmake_module_path/module_only/conanfile.txt b/tests/resources/cmake_module_path/module_only/conanfile.txt new file mode 100644 index 00000000..2bbbbc70 --- /dev/null +++ b/tests/resources/cmake_module_path/module_only/conanfile.txt @@ -0,0 +1,5 @@ +[requires] +cmake-module-with-dependency/0.1 + +[generators] +CMakeDeps diff --git a/tests/resources/fake_boost_recipe/conanfile.py b/tests/resources/recipes/boost/conanfile.py similarity index 100% rename from tests/resources/fake_boost_recipe/conanfile.py rename to tests/resources/recipes/boost/conanfile.py diff --git a/tests/resources/libbye/conanfile.py b/tests/resources/recipes/bye/conanfile.py similarity index 100% rename from tests/resources/libbye/conanfile.py rename to tests/resources/recipes/bye/conanfile.py diff --git a/tests/resources/recipes/cmake-module-only/conanfile.py b/tests/resources/recipes/cmake-module-only/conanfile.py new file mode 100644 index 00000000..e9d81137 --- /dev/null +++ b/tests/resources/recipes/cmake-module-only/conanfile.py @@ -0,0 +1,60 @@ +from conan import ConanFile +from conan.tools.cmake import CMakeToolchain, CMake, cmake_layout, CMakeDeps + + +class cmake_module_onlyRecipe(ConanFile): + name = "cmake-module-only" + version = "0.1" + package_type = "library" + + # Optional metadata + license = "" + author = " " + url = "" + description = "" + topics = ("", "", "") + + # Binary configuration + settings = "os", "compiler", "build_type", "arch" + options = {"shared": [True, False], "fPIC": [True, False], "with_builddir": [True, False]} + default_options = {"shared": False, "fPIC": True, "with_builddir": False} + + # Sources are located in the same place as this recipe, copy them to the recipe + exports_sources = "CMakeLists.txt", "src/*", "include/*" + + def config_options(self): + if self.settings.os == "Windows": + self.options.rm_safe("fPIC") + + def configure(self): + if self.options.shared: + self.options.rm_safe("fPIC") + + def layout(self): + cmake_layout(self) + + def generate(self): + deps = CMakeDeps(self) + deps.generate() + tc = CMakeToolchain(self) + tc.generate() + + def build(self): + cmake = CMake(self) + cmake.configure() + cmake.build() + + def package(self): + cmake = CMake(self) + cmake.install() + + def package_info(self): + self.cpp_info.libs = [] + + if self.options.with_builddir: + self.cpp_info.builddirs.append("orion-module-subfolder") + + # Set this to be MODULE only, to force the case in a test where this is detected by module name + self.cpp_info.set_property("cmake_file_name", "Orion") + self.cpp_info.set_property("cmake_target_name", "Orion::orion") + self.cpp_info.set_property("cmake_find_mode", "module") diff --git a/tests/resources/recipes/cmake-module-with-dependency/conanfile.py b/tests/resources/recipes/cmake-module-with-dependency/conanfile.py new file mode 100644 index 00000000..a26df165 --- /dev/null +++ b/tests/resources/recipes/cmake-module-with-dependency/conanfile.py @@ -0,0 +1,60 @@ +from conan import ConanFile +from conan.tools.cmake import CMakeToolchain, CMake, cmake_layout, CMakeDeps + + +class cmake_module_onlyRecipe(ConanFile): + name = "cmake-module-with-dependency" + version = "0.1" + package_type = "library" + + # Optional metadata + license = "" + author = " " + url = "" + description = "" + topics = ("", "", "") + + # Binary configuration + settings = "os", "compiler", "build_type", "arch" + options = {"shared": [True, False], "fPIC": [True, False]} + default_options = {"shared": False, "fPIC": True} + + # Sources are located in the same place as this recipe, copy them to the recipe + exports_sources = "CMakeLists.txt", "src/*", "include/*" + + def config_options(self): + if self.settings.os == "Windows": + self.options.rm_safe("fPIC") + + def configure(self): + if self.options.shared: + self.options.rm_safe("fPIC") + + def layout(self): + cmake_layout(self) + + def requirements(self): + self.requires("cmake-module-only/0.1") + + def generate(self): + deps = CMakeDeps(self) + deps.generate() + tc = CMakeToolchain(self) + tc.generate() + + def build(self): + cmake = CMake(self) + cmake.configure() + cmake.build() + + def package(self): + cmake = CMake(self) + cmake.install() + + def package_info(self): + self.cpp_info.libs = [] + + # Set this to be MODULE only, to force the case in a test where this is detected by module name + self.cpp_info.set_property("cmake_file_name", "Andromeda") + self.cpp_info.set_property("cmake_target_name", "Andromeda::andromeda") + self.cpp_info.set_property("cmake_find_mode", "module") diff --git a/tests/test_smoke.py b/tests/test_smoke.py index a682fc9a..f50e82e2 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -58,21 +58,24 @@ def setup_conan_home(conan_home_dir, tmp_path_factory): workdir = tmp_path_factory.mktemp("temp_recipes") logging.info(f"Conan home setup, temporary folder: {workdir}") cwd = os.getcwd() - os.chdir(workdir.as_posix()) + + # Detect default profile run("conan profile detect -vquiet") - # libhello + + # Create hello lib from built-in template + os.chdir(workdir.as_posix()) run("conan new cmake_lib -d name=hello -d version=0.1 -vquiet") run("conan export . -vquiet") - # libbye with modified conanfile.py (custom package_info properties) - run("conan new cmake_lib -d name=bye -d version=0.1 -f -vquiet") - shutil.copy2(src_dir / 'tests' / 'resources' / 'libbye' / 'conanfile.py', ".") - run("conan export . -vquiet") + # additional recipes to export from resources, overlay on top of `hello` and export + additional_recipes = ['boost', 'bye', 'cmake-module-only', 'cmake-module-with-dependency'] - # libboost with modified conanfile.py (ensure upper case B cmake package name) - run("conan new cmake_lib -d name=boost -d version=1.77.0 -f -vquiet") - shutil.copy2(src_dir / 'tests' / 'resources' / 'fake_boost_recipe' / 'conanfile.py', ".") - run("conan export . -vquiet") + for recipe in additional_recipes: + recipe_dir = tmp_path_factory.mktemp(f"temp_{recipe}") + os.chdir(recipe_dir.as_posix()) + run(f"conan new cmake_lib -d name={recipe} -d version=0.1 -f -vquiet") + shutil.copy2(src_dir / 'tests' / 'resources' / 'recipes' / recipe / 'conanfile.py', ".") + run("conan export . -vquiet") # Additional profiles for testing config_dir = src_dir / 'tests' / 'resources' / 'custom_config' @@ -234,7 +237,6 @@ def test_msvc_runtime_singleconfig(self, capfd, config, msvc_runtime): runtime = msvc_runtime.replace("$<$:Debug>", debug_tag) expected_runtime_outputs = [f.format(expected_runtime=runtime) for f in expected_app_msvc_runtime] assert all(expected in out for expected in expected_runtime_outputs) - class TestFindModules: def test_find_module(self, capfd, basic_cmake_project): @@ -269,6 +271,28 @@ def test_cmake_builtin_module(self, capfd, basic_cmake_project): out, _ = capfd.readouterr() assert "Found Threads: TRUE" in out + +class TestCMakeModulePath: + + def test_preserve_module_path(self, capfd, basic_cmake_project): + "Ensure that existing CMAKE_MODULE_PATH values remain in place after find_package(XXX) call" + source_dir, binary_dir = basic_cmake_project + shutil.copytree(src_dir / 'tests' / 'resources' / 'cmake_module_path' / 'module_only', source_dir, dirs_exist_ok=True) + run(f"cmake -S {source_dir} -B {binary_dir} -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES={conan_provider} -DCMAKE_BUILD_TYPE=Release", check=False) + out, err = capfd.readouterr() + assert "CMAKE_MODULE_PATH has expected value" in out + assert "CMAKE_MODULE_PATH DOES NOT have expected value" not in out + + def test_module_path_from_dependency(self, capfd, basic_cmake_project): + "Ensure that CMAKE_MODULE_PATH is prepended with value from dependency (builddir in recipe)" + source_dir, binary_dir = basic_cmake_project + shutil.copytree(src_dir / 'tests' / 'resources' / 'cmake_module_path' / 'library_with_cmake_module_dir', source_dir, dirs_exist_ok=True) + run(f"cmake -S {source_dir} -B {binary_dir} -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES={conan_provider} -DCMAKE_BUILD_TYPE=Release", check=False) + out, err = capfd.readouterr() + assert "CMAKE_MODULE_PATH has expected value" in out + assert "CMAKE_MODULE_PATH DOES NOT have expected value" not in out + + class TestGeneratedProfile: @linux def test_propagate_cxx_compiler(self, capfd, basic_cmake_project):