Add Ninja support (#188)

* Pass -j to CMake instead of build tool

* Add ninja option

* Add Ninja test in CI

* Install ninja as dependency in CI

* Fix typing bug

* Code clarity

* Skip v3.0.0 in CI

* Spelling

* Fix build_dir: Path vs str bug

* Add Ninja support for get_available_targets

* Add comments

* Format

* Other comment

* Reword help text

* Fix spelling

* 🤷

* Correct docstrings

* reimplement noop vs refresh for performance

* Let build tool handle default --jobs

* Fix job pass through logic

.github/actions/spelling/expect.txt

@@ -138,6 +138,7 @@ locs
 lstrip
 lxml
 makedirs
+Makefiles
 malloc
 mallocator
 maxdepth

.github/workflows/integration-tests.yml

@@ -25,6 +25,7 @@ jobs:
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip urllib3 setuptools_scm
+          sudo apt-get install ninja-build
       - name: clone fprime and gps-tutorial
         run: |
           git clone --recurse-submodules -b ${{ matrix.fprime-version }} https://github.com/nasa/fprime.git
@@ -48,6 +49,14 @@ jobs:
           fprime-util generate
           fprime-util build
           fprime-util hash-to-file 0x72ad3277
+      - name: Test generation and building on Ref with Ninja
+        working-directory: fprime/Ref
+        # There is a bug in v3.0.0 that causes ninja builds to fail
+        if: matrix.fprime-version != 'v3.0.0'
+        run: |
+          fprime-util generate --ninja --build-cache ./build-ninja
+          fprime-util build --build-cache ./build-ninja
+          fprime-util hash-to-file 0x72ad3277 --build-cache ./build-ninja
       - name: Test special generation on Ref
         working-directory: fprime/Ref
         run: |

src/fprime/fbuild/builder.py

@@ -356,7 +356,7 @@ def execute_build_target(
             build_target: build system target to run as string
             context: context path for local targets
             top: True if it is a top-level (global) target, False if it is a local target
-            make_args: make system arguments directly supplied
+            make_args: args to supply to the build tool (make or ninja)
         """
         self.cmake.execute_known_target(
             build_target,

src/fprime/fbuild/cli.py

@@ -46,7 +46,7 @@ def run_fbuild_cli(
         build: build object, pre-loaded, iff not "generate" or "purged"
         parsed: parsed arguments object.
         cmake_args: cmake commands from parsed output
-        make_args: arguments to the make system
+        make_args: arguments to supply to the build tool (make or ninja)
     """
     if parsed.command == "generate":
         toolchain = build.find_toolchain()
@@ -59,6 +59,8 @@ def run_fbuild_cli(
             cmake_args["ENABLE_SANITIZER_UNDEFINED_BEHAVIOR"] = "ON"
         if toolchain is not None:
             cmake_args["CMAKE_TOOLCHAIN_FILE"] = toolchain
+        if parsed.ninja:
+            cmake_args["CMAKE_GENERATOR"] = "Ninja"
         build.generate(cmake_args)
     elif parsed.command == "purge":
         # Since purge does not load its "base", we need to overload the platform
@@ -141,9 +143,8 @@ def add_target_parser(
         parser.add_argument(
             "-j",
             "--jobs",
-            default=1,
             type=int,
-            help="Parallel build job count. Default: %(default)s.",
+            help="Parallel build job count. Default: build tool default (make: 1, ninja: # of cores)",
         )
     parser, flags = existing[target.mnemonic]
     new_flags = [flag for flag in target.flags if flag not in flags]
@@ -209,6 +210,11 @@ def add_special_targets(
         help="Disable the compiler sanitizers. Sanitizers are only enabled by default when --ut is provided.",
         action="store_true",
     )
+    generate_parser.add_argument(
+        "--ninja",
+        action="store_true",
+        help="Use the Ninja build system instead of Unix Makefiles",
+    )
     # The following option is specified only to show up in --help.
     # It is not handled by argparse, but in fprime.util.cli:validate()
     generate_parser.add_argument(

src/fprime/fbuild/cmake.py

@@ -40,7 +40,7 @@ def __init__(self):
         self.settings = {}
         self._cmake_cache = None
         self.verbose = False
-        self.cached_help_targets = None
+        self.cached_help_targets = []
         try:
             self._run_cmake(["--help"], print_output=False)
         except Exception as exc:
@@ -97,6 +97,13 @@ def execute_known_target(
         self.validate_cmake_cache(cmake_args, build_dir)
 
         make_args = {} if make_args is None else make_args
+
+        run_args = ["--build", build_dir]
+        # CMake 3.12+ supports parallel builds with -j
+        jobs = make_args.pop("--jobs", None)
+        if jobs is not None:
+            run_args.extend(["-j", str(jobs)])
+
         fleshed_args = ["--"] + list(
             map(lambda key: f"{key}={make_args[key]}", make_args.keys())
         )
@@ -112,7 +119,6 @@ def execute_known_target(
             else:
                 cmake_target = f"{module}_{target}".lstrip("_")
 
-        run_args = ["--build", build_dir]
         environment = {} if environment is None else copy.copy(environment)
         if self.verbose:
             environment["VERBOSE"] = "1"
@@ -129,6 +135,11 @@ def execute_known_target(
                 print_output=print_output,
             )
         except CMakeExecutionException as exc:
+            # The below code allows to try and refresh the build cache if the target is not found in the cache.
+            # This can catch the case where the cache has not been refreshed since the requested target was added.
+            # Note that this auto-refresh behavior is native to CMake/Ninja when using Ninja as the generator.
+            # Therefore refreshing the cache here should only be tried for Makefile generators:
+            #   --> "No rule to make target" is the make output. If any other output, we raise the exception.
             no_target = functools.reduce(
                 lambda cur, line: cur or "No rule to make target" in line,
                 exc.get_errors(),
@@ -330,11 +341,25 @@ def get_available_targets(self, build_dir, path):
             stdout, _ = self._run_cmake(
                 run_args, write_override=True, print_output=False
             )
-            self.cached_help_targets = [
-                line.replace("...", "").strip()
-                for line in stdout
-                if line.startswith("...")
-            ]
+            # help target lists all targets but has a different output format for ninja and make
+            if "Makefile" not in stdout[0]:
+                # Ninja output
+                self.cached_help_targets.extend(
+                    [
+                        line.replace(": phony", "").strip()
+                        for line in stdout
+                        if line.endswith(": phony\n")
+                    ]
+                )
+            else:
+                # Makefile output
+                self.cached_help_targets.extend(
+                    [
+                        line.replace("...", "").strip()
+                        for line in stdout
+                        if line.startswith("...")
+                    ]
+                )
 
         prefix = self.get_cmake_module(path, build_dir)
         return [
@@ -354,6 +379,25 @@ def is_target_supported(self, build_dir: str, target: str):
         self.get_available_targets(build_dir, None)
         return target in self.cached_help_targets
 
+    def _is_noop_supported(self, build_dir: str):
+        """Checks if the noop target is supported by the current build directory.
+
+        Note: This function is implemented by checking the CMake files in order to bypass the call
+        to `cmake --target help` in get_available_target() which is very slow when using Makefiles
+
+        Args:
+            build_dir: build directory to use for detecting noop target
+        """
+        noop_file = os.path.join(
+            self._read_cache(build_dir)["FPRIME_FRAMEWORK_PATH"],
+            "cmake",
+            "target",
+            "noop.cmake",
+        )
+        if os.path.isfile(noop_file):
+            return True
+        return False
+
     @staticmethod
     def purge(build_dir):
         """
@@ -442,7 +486,7 @@ def cmake_refresh_cache(self, build_dir, full=False):
         """
         if full:
             environment = {}
-            run_args = ["--build", build_dir]
+            run_args = ["--build", str(build_dir)]
             if self.verbose:
                 print("[CMAKE] Refreshing CMake build cache")
                 environment["VERBOSE"] = "1"
@@ -458,7 +502,7 @@ def cmake_refresh_cache(self, build_dir, full=False):
             if self.verbose:
                 print("[CMAKE] Checking CMake cache for rebuild")
             # Backwards compatibility: refresh_cache was named noop until v3.3.x
-            if self.is_target_supported(build_dir, "noop"):
+            if self._is_noop_supported(str(build_dir)):
                 self.execute_known_target(
                     "noop",
                     build_dir,

src/fprime/util/cli.py

@@ -242,7 +242,8 @@ def validate(parsed, unknown):
     # Build type only for generate, jobs only for non-generate
     elif parsed.command in [target.mnemonic for target in Target.get_all_targets()]:
         parsed.settings = None  # Force to load from cache if possible
-        make_args["--jobs"] = 1 if parsed.jobs <= 0 else parsed.jobs
+        if parsed.jobs is not None and parsed.jobs >= 1:
+            make_args["--jobs"] = parsed.jobs
     # Check if any arguments are still unknown
     if unknown:
         runnable = f"{os.path.basename(sys.argv[0])} {parsed.command}"