Fixes #570: Do not remove CMAKE_MODULE_PATH if already set on find_package()

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()
 

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()
+

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

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()

tests/resources/cmake_module_path/module_only/conanfile.txt

@@ -0,0 +1,5 @@
+[requires]
+cmake-module-with-dependency/0.1
+
+[generators]
+CMakeDeps

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 = "<Put the package license here>"
+    author = "<Put your name here> <And your email here>"
+    url = "<Package recipe repository url here, for issues about the package>"
+    description = "<Description of cmake-module-only package here>"
+    topics = ("<Put some tag here>", "<here>", "<and here>")
+
+    # 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")

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 = "<Put the package license here>"
+    author = "<Put your name here> <And your email here>"
+    url = "<Package recipe repository url here, for issues about the package>"
+    description = "<Description of cmake-module-only package here>"
+    topics = ("<Put some tag here>", "<here>", "<and here>")
+
+    # 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")

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("$<$<CONFIG:Debug>: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):