From 260f3c928eac94a815bcb1a1492bda0a5ac1b5b5 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Fri, 21 Aug 2026 22:01:08 +0000 Subject: [PATCH 1/2] Harden cmake target parsing and unify gcovr project root resolution (WP5) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/fprime/fbuild/cmake.py | 6 ++- src/fprime/fbuild/gcovr.py | 36 +++++++++----- test/fprime/fbuild/test_cmake.py | 84 ++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 14 deletions(-) diff --git a/src/fprime/fbuild/cmake.py b/src/fprime/fbuild/cmake.py index 9442b360..26220c48 100644 --- a/src/fprime/fbuild/cmake.py +++ b/src/fprime/fbuild/cmake.py @@ -371,7 +371,8 @@ def get_available_targets(self, build_dir, path): run_args, write_override=True, print_output=False ) # help target lists all targets but has a different output format for ninja and make - if "Makefile" not in stdout[0]: + generator = self._read_cache(build_dir).get("CMAKE_GENERATOR", "") + if "Makefile" not in generator: # Ninja output self.cached_help_targets.extend( [ @@ -392,7 +393,7 @@ def get_available_targets(self, build_dir, path): prefix = self.get_cmake_module(path, build_dir) return [ - make.replace(prefix, "").strip("_") + make[len(prefix) :].strip("_") for make in self.cached_help_targets if make.startswith(prefix) ] @@ -521,6 +522,7 @@ def cmake_refresh_cache(self, build_dir, environment=None, full=False): :param full: perform a full rebuild """ environment = {} if environment is None else environment + self.cached_help_targets.clear() if full: run_args = ["--build", str(build_dir)] if self.verbose: diff --git a/src/fprime/fbuild/gcovr.py b/src/fprime/fbuild/gcovr.py index f6881fe4..18a3f255 100644 --- a/src/fprime/fbuild/gcovr.py +++ b/src/fprime/fbuild/gcovr.py @@ -13,6 +13,24 @@ from .target import CompositeTarget, ExecutableAction, ExecuteContext, TargetScope +def _get_project_root(builder: "Build") -> Path: + """Get the resolved project root for coverage calculations + + Returns the project_root setting, falling back to framework_path, and finally to the + parent of the build cache directory (build caches are generated directly under the + project root). + + Args: + builder: builder object + """ + return Path( + builder.get_settings( + "project_root", + builder.get_settings("framework_path", builder.build_dir.parent), + ) + ).resolve() + + def _get_project_path(builder: "Build", module: Union[str, Path]) -> Path: """Get the project path from a given module string @@ -27,10 +45,7 @@ def _get_project_path(builder: "Build", module: Union[str, Path]) -> Path: if isinstance(module, Path): return module # Calculate the project path from the module name - project_root = builder.get_settings( - "project_root", builder.get_settings("framework_path", builder.build_dir.parent) - ) - return Path(project_root) / module.replace("_", "/") + return _get_project_root(builder) / module.replace("_", "/") def _using_root(builder: "Build", context: Path, scope: TargetScope): @@ -134,18 +149,15 @@ def execute( coverage_output_dir = context.resolve() / "coverage" coverage_output_dir.mkdir(exist_ok=True) - project_root = builder.get_settings( - "project_root", - builder.get_settings("framework_path", builder.build_dir.parent.parent), - ).resolve() + project_root = _get_project_root(builder) filter_path = ( - Path(project_root).resolve() + project_root if _using_root(builder, context, self.scope) else _get_project_path(builder, context) ).resolve() - framework_path = builder.get_settings( - "framework_path", builder.build_dir.parent.parent - ) + framework_path = Path( + builder.get_settings("framework_path", builder.build_dir.parent.parent) + ).resolve() combined_env = os.environ.copy() combined_env.update(builder.settings.get("environment", {})) # gcovr is an unhappy beast diff --git a/test/fprime/fbuild/test_cmake.py b/test/fprime/fbuild/test_cmake.py index ec1f0632..f8c3d9e7 100644 --- a/test/fprime/fbuild/test_cmake.py +++ b/test/fprime/fbuild/test_cmake.py @@ -553,6 +553,90 @@ def test_generate_environment(cmake_handler): generate_build(({}, []), False, cmake_handler, environment={"MY_ENV_VAR": "VALUE1"}) +NINJA_HELP_OUTPUT = [ + "all: phony\n", + "clean: phony\n", + "help: phony\n", + "Svc_Module: phony\n", + "Svc_Module_build: phony\n", + "Svc_Module_Svc_Module_impl: phony\n", + "Svc_Module.o\n", +] + +MAKE_HELP_OUTPUT = [ + "The following are some of the valid targets for this Makefile:\n", + "... all (the default if no target is provided)\n", + "... clean\n", + "... Svc_Module\n", + "... Svc_Module_build\n", + "... Svc_Module_Svc_Module_impl\n", +] + + +@patch("fprime.fbuild.cmake.CMakeHandler.get_cmake_module") +@patch("fprime.fbuild.cmake.CMakeHandler._read_cache") +@patch("fprime.fbuild.cmake.CMakeHandler._run_cmake") +def get_available_targets( + cmake_handler, + generator, + help_output, + mock_run_cmake, + mock_read_cache, + mock_get_cmake_module, +): + """Run get_available_targets with canned generator help output""" + mock_run_cmake.return_value = (help_output, []) + mock_read_cache.return_value = {"CMAKE_GENERATOR": generator} + mock_get_cmake_module.return_value = "Svc_Module" + return cmake_handler.get_available_targets("/fake/build/dir", None) + + +def test_get_available_targets_ninja(cmake_handler): + """Test target parsing of ninja-style help output""" + targets = get_available_targets(cmake_handler, "Ninja", NINJA_HELP_OUTPUT) + assert targets == ["", "build", "Svc_Module_impl"] + assert cmake_handler.cached_help_targets == [ + "all", + "clean", + "help", + "Svc_Module", + "Svc_Module_build", + "Svc_Module_Svc_Module_impl", + ] + + +def test_get_available_targets_make(cmake_handler): + """Test target parsing of make-style help output""" + targets = get_available_targets(cmake_handler, "Unix Makefiles", MAKE_HELP_OUTPUT) + assert targets == ["", "build", "Svc_Module_impl"] + assert cmake_handler.cached_help_targets == [ + "all (the default if no target is provided)", + "clean", + "Svc_Module", + "Svc_Module_build", + "Svc_Module_Svc_Module_impl", + ] + + +def test_get_available_targets_empty_output(cmake_handler): + """Test target parsing tolerates empty help output""" + assert get_available_targets(cmake_handler, "Ninja", []) == [] + cmake_handler.cached_help_targets.clear() + assert get_available_targets(cmake_handler, "Unix Makefiles", []) == [] + + +@patch("fprime.fbuild.cmake.CMakeHandler._is_noop_supported") +@patch("fprime.fbuild.cmake.CMakeHandler.execute_known_target") +def test_refresh_cache_clears_help_targets( + mock_execute_known_target, mock_is_noop_supported, cmake_handler +): + """Test that cmake_refresh_cache clears the cached help targets""" + mock_is_noop_supported.return_value = True + cmake_handler.cached_help_targets.extend(["stale_target"]) + cmake_handler.cmake_refresh_cache("/some/build/dir") + assert cmake_handler.cached_help_targets == [] + + def test_read_cache_per_directory(cmake_handler): """Test that _read_cache memoizes per build directory From 06740c9db30f27b1012494b85783c25c05345cce Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Fri, 21 Aug 2026 22:07:23 +0000 Subject: [PATCH 2/2] Read CMAKE_GENERATOR from CMakeCache.txt for generator detection Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/fprime/fbuild/cmake.py | 19 ++++++++++++++++++- test/fprime/fbuild/test_cmake.py | 17 ++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/fprime/fbuild/cmake.py b/src/fprime/fbuild/cmake.py index 26220c48..c1fff356 100644 --- a/src/fprime/fbuild/cmake.py +++ b/src/fprime/fbuild/cmake.py @@ -371,7 +371,7 @@ def get_available_targets(self, build_dir, path): run_args, write_override=True, print_output=False ) # help target lists all targets but has a different output format for ninja and make - generator = self._read_cache(build_dir).get("CMAKE_GENERATOR", "") + generator = self._get_generator(build_dir) if "Makefile" not in generator: # Ninja output self.cached_help_targets.extend( @@ -475,6 +475,23 @@ def _read_cache(self, build_dir): ) return self._cmake_cache[cache_key] + @staticmethod + def _get_generator(build_dir): + """Read CMAKE_GENERATOR from the CMakeCache.txt file of the given build directory + + CMAKE_GENERATOR is an INTERNAL cache entry, which `cmake -LA` does not list, so it must be + read from the cache file directly. + """ + cache_file = Path(build_dir) / "CMakeCache.txt" + try: + with open(cache_file, encoding="utf8") as file_handle: + for line in file_handle: + if line.startswith("CMAKE_GENERATOR:"): + return line.split("=", 1)[1].strip() + except OSError: + pass + return "" + def _invalidate_cache(self, build_dir): """Drop the memoized cache variables for the given build directory""" self._cmake_cache.pop(str(Path(build_dir).resolve()), None) diff --git a/test/fprime/fbuild/test_cmake.py b/test/fprime/fbuild/test_cmake.py index f8c3d9e7..07705ba9 100644 --- a/test/fprime/fbuild/test_cmake.py +++ b/test/fprime/fbuild/test_cmake.py @@ -574,19 +574,19 @@ def test_generate_environment(cmake_handler): @patch("fprime.fbuild.cmake.CMakeHandler.get_cmake_module") -@patch("fprime.fbuild.cmake.CMakeHandler._read_cache") +@patch("fprime.fbuild.cmake.CMakeHandler._get_generator") @patch("fprime.fbuild.cmake.CMakeHandler._run_cmake") def get_available_targets( cmake_handler, generator, help_output, mock_run_cmake, - mock_read_cache, + mock_get_generator, mock_get_cmake_module, ): """Run get_available_targets with canned generator help output""" mock_run_cmake.return_value = (help_output, []) - mock_read_cache.return_value = {"CMAKE_GENERATOR": generator} + mock_get_generator.return_value = generator mock_get_cmake_module.return_value = "Svc_Module" return cmake_handler.get_available_targets("/fake/build/dir", None) @@ -625,6 +625,17 @@ def test_get_available_targets_empty_output(cmake_handler): assert get_available_targets(cmake_handler, "Unix Makefiles", []) == [] +def test_get_generator_reads_cache_file(cmake_handler, tmp_path): + """Test that the generator is read from CMakeCache.txt, where it is an INTERNAL entry""" + (tmp_path / "CMakeCache.txt").write_text( + "CMAKE_ADDR2LINE:FILEPATH=/usr/bin/addr2line\n" + "CMAKE_GENERATOR:INTERNAL=Unix Makefiles\n" + "CMAKE_GENERATOR_INSTANCE:INTERNAL=\n" + ) + assert cmake_handler._get_generator(tmp_path) == "Unix Makefiles" + assert cmake_handler._get_generator(tmp_path / "missing") == "" + + @patch("fprime.fbuild.cmake.CMakeHandler._is_noop_supported") @patch("fprime.fbuild.cmake.CMakeHandler.execute_known_target") def test_refresh_cache_clears_help_targets(