Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass Environment To CMake in all Cases #235

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/fprime/fbuild/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,18 @@ def install_dest_exists(self) -> Path:
path = Path(self.settings["install_destination"])
return path if path.exists() else None

def refresh(self):
""" Refresh this build (i.e. the cmake build cache)

Some build systems (e.g. CMake) require the build to be refreshed (i.e. refresh the build cache). When this
happens it is imperative that the appropriate environment is set. For this reason, refresh is exposed as a
helper in this layer rather than the previous pattern of calling builder.cmake.cmake_refresh_cache directly.
"""
self.cmake.cmake_refresh_cache(
self.build_dir,
environment=self.settings.get("environment", None)
)

@staticmethod
def find_nearest_parent_project(path: Path) -> Path:
"""Recurse up the directory stack looking for a valid CMake project.
Expand Down
25 changes: 18 additions & 7 deletions src/fprime/fbuild/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from fprime.common.utils import confirm
from fprime.fbuild.builder import Build
from fprime.fbuild.target import Target
from fprime.fbuild.target import Target, DesignateTargetAction
from fprime.fbuild.types import BuildType


Expand Down Expand Up @@ -180,12 +180,23 @@ def add_target_parser(
if target.allows_pass_args()
else ""
)
parser.add_argument(
f"--{flag}",
action="store_true",
default=False,
help=f"{target.desc}{extra_help}",
)
# Target flag special handling
if flag == "target":
parser.add_argument(
f"--target",
type=str,
action=DesignateTargetAction,
default=None,
nargs=1,
help=f"{target.desc}",
)
else:
parser.add_argument(
f"--{flag}",
action="store_true",
default=False,
help=f"{target.desc}{extra_help}",
)
for flag, description in filter(
lambda flag_desc: flag_desc[0] not in flags, target.option_args()
):
Expand Down
43 changes: 19 additions & 24 deletions src/fprime/fbuild/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def execute_known_target(
:param target: target to execute at the path, using above build_dir
:param path: path to run target against. (default) current working directory
:param cmake_args: cmake args input
:param make_args: arguments to pass to make
:param top_target: top-level target. Do not append path name
:param environment: environment to setup when executing CMake
:param full_cache_rebuild: rebuild the cache fully. Default: False, use the short version
Expand Down Expand Up @@ -151,7 +152,7 @@ def execute_known_target(
print(
"[CMAKE] CMake failed to detect target, attempting CMake cache refresh and retry"
)
self.cmake_refresh_cache(build_dir, full=full_cache_rebuild)
self.cmake_refresh_cache(build_dir, environment, full=full_cache_rebuild)
return self._run_cmake(
run_args + fleshed_args,
write_override=True,
Expand Down Expand Up @@ -477,16 +478,18 @@ def _cmake_validate_build_dir(build_dir):
if not os.path.isfile(cache_file):
raise CMakeInvalidBuildException(build_dir)

def cmake_refresh_cache(self, build_dir, full=False):
"""
def cmake_refresh_cache(self, build_dir, environment=None, full=False):
""" Refresh the CMake cache by calling a known cache-refresh target

Runs the cmake target required to refresh the cmake cache. This will allow for unknown targets to be searched
for before the utility gives up and produces.

:param build_dir: directory to build in
:param full: full re-generate of the cache. Default: false, attempt to build 'refresh_cache' target instead
:param build_dir: cache directory to run in
:param environment: environment to pass in when refreshing
:param full: perform a full rebuild
"""
environment = {} if environment is None else environment
if full:
environment = {}
run_args = ["--build", str(build_dir)]
if self.verbose:
print("[CMAKE] Refreshing CMake build cache")
Expand All @@ -503,24 +506,16 @@ 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_noop_supported(str(build_dir)):
self.execute_known_target(
"noop",
build_dir,
None,
top_target=True,
full_cache_rebuild=True,
print_output=True,
)
else:
self.execute_known_target(
"refresh_cache",
build_dir,
None,
top_target=True,
full_cache_rebuild=True,
print_output=True,
)
refresh_target = "noop" if self._is_noop_supported(str(build_dir)) else "refresh_cache"
self.execute_known_target(
refresh_target,
build_dir,
None,
top_target=True,
full_cache_rebuild=True,
print_output=True,
environment=environment
)

def _run_cmake(
self,
Expand Down
38 changes: 38 additions & 0 deletions src/fprime/fbuild/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import functools
import itertools
from abc import ABC, abstractmethod
from argparse import Action
from enum import Enum
from pathlib import Path
from typing import Dict, List, Set, Tuple
Expand Down Expand Up @@ -341,6 +342,43 @@ def is_supported(self, builder: "Build", context: Path):
return self.build_target in build_target_names


class DesignateTargetAction(Action):
"""This class, when used as the action will set the target of all DesignatedBuildSystemTarget

argparse.Action actions can be used to handle custom behavior when parsing commands. This class will use the
--target flag to specify the build target on all known DesignatedBuildSystemTarget that have registered with this
class.
"""

_DESIGNEES = []

@classmethod
def register_designee(cls, designee):
"""Register designee to this action"""
cls._DESIGNEES.append(designee)

def __call__(self, parser, namespace, values, option_string=None):
"""Required __call__ function triggered by the parse"""
assert len(values) == 1, "Values object should contain 1 value"
for designee in self._DESIGNEES:
designee.set_target(values[0])
# The build target detection looks for true/false flags to be set. Mimic this by setting 'target' to True
setattr(namespace, "target", True)


class DesignatedBuildSystemTarget(BuildSystemTarget):
"""Invokes a designated target using the --target flag"""

def __init__(self, _, *args, **kwargs):
"""Constructor setting child targets"""
super().__init__(None, *args, **kwargs)
DesignateTargetAction.register_designee(self)

def set_target(self, target):
"""Set the target to build"""
self.build_target = target


class DelegatorTarget(Target):
"""Delegates to another target

Expand Down
21 changes: 19 additions & 2 deletions src/fprime/fbuild/target_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""

from .gcovr import GcovrTarget
from .target import BuildSystemTarget, TargetScope
from .target import BuildSystemTarget, DesignatedBuildSystemTarget, TargetScope
from .types import BuildType

#### "build" targets for components, deployments, unittests for both normal and testing builds ####
Expand All @@ -27,12 +27,29 @@
BuildSystemTarget(
"all",
mnemonic="build",
desc="Build components, ports, UTs, and deployments for unittest build",
desc="Build all components, ports, UTs, and deployments for unittest build",
scope=TargetScope.GLOBAL,
flags={"all", "ut"},
build_type=BuildType.BUILD_TESTING,
)

DesignatedBuildSystemTarget(
"target",
mnemonic="build",
desc="Build a specific CMake target by name",
scope=TargetScope.GLOBAL,
flags={"target"},
)

DesignatedBuildSystemTarget(
"target",
mnemonic="build",
desc="Build a specific CMake target by name using the UT build",
scope=TargetScope.GLOBAL,
flags={"ut", "target"},
build_type=BuildType.BUILD_TESTING,
)

#### Check targets ####
check = BuildSystemTarget(
"check",
Expand Down
2 changes: 1 addition & 1 deletion src/fprime/fpp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def execute(
)
return 1

builder.cmake.cmake_refresh_cache(builder.build_dir, False)
builder.refresh()

# Read files and arguments
locations = self.get_locations_file(builder)
Expand Down
25 changes: 25 additions & 0 deletions test/fprime/fbuild/echoer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env python
""" Fake CMake script for testing purposes

This script can be added to the front of the path to echo what arguments are sent to CMake allowing for tests that are
slightly closer to integration tests without involving the entirety of F Prime.
"""
import sys
import os
from pathlib import Path
import json

if __name__ == "__main__":
print("[INFO] Running echoer program (stdout)")
print("[INFO] Running echoer program (stderr)", file=sys.stderr)
executable_path = Path(sys.argv[0])
for i in range(0, 100):
output_file_path = executable_path.parent / f"faux-{executable_path.name}-{i}.json"
if not output_file_path.exists():
with open(output_file_path, "w") as output_file:
json.dump({"arguments": sys.argv, "cwd": str(Path.cwd()), "environment": dict(os.environ)}, output_file)
break
else:
print(f"[ERROR] Too many invocations of: {executable_path.name}", file=sys.stderr)
sys.exit(1)
sys.exit(0)
Loading
Loading