From f955319755911572d8e845272e8182d7bd6c5da7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 21 Nov 2022 14:35:30 +0100 Subject: [PATCH 1/4] Set pw_command_launcher from build_example.py CLI This will allow effortless ccache setup like this: ./scripts/build/build_examples.py \ --target linux-x64-light \ --pw-command-launcher=ccache \ build --- scripts/build/build/__init__.py | 7 +++++-- scripts/build/build/target.py | 3 ++- scripts/build/build_examples.py | 10 ++++++++-- scripts/build/builders/builder.py | 7 +++++-- scripts/build/builders/gn.py | 5 ++++- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/scripts/build/build/__init__.py b/scripts/build/build/__init__.py index fc0ee056590aab..479cf6e918290d 100644 --- a/scripts/build/build/__init__.py +++ b/scripts/build/build/__init__.py @@ -25,7 +25,8 @@ def __init__(self, runner, repository_path: str, output_prefix: str): self.completed_steps = set() def SetupBuilders(self, targets: Sequence[str], - enable_flashbundle: bool): + enable_flashbundle: bool, + pw_command_launcher: str): """ Configures internal builders for the given platform/board/app combination. @@ -35,7 +36,9 @@ def SetupBuilders(self, targets: Sequence[str], for target in targets: found = False for choice in BUILD_TARGETS: - builder = choice.Create(target, self.runner, self.repository_path, self.output_prefix, enable_flashbundle) + builder = choice.Create( + target, self.runner, self.repository_path, self.output_prefix, + enable_flashbundle, pw_command_launcher) if builder: self.builders.append(builder) found = True diff --git a/scripts/build/build/target.py b/scripts/build/build/target.py index 28f568b284c027..aa3a16d1600530 100644 --- a/scripts/build/build/target.py +++ b/scripts/build/build/target.py @@ -330,7 +330,7 @@ def StringIntoTargetParts(self, value: str): return _StringIntoParts(value, suffix, self.fixed_targets, self.modifiers) def Create(self, name: str, runner, repository_path: str, output_prefix: str, - enable_flashbundle: bool): + enable_flashbundle: bool, pw_command_launcher: str): parts = self.StringIntoTargetParts(name) @@ -349,5 +349,6 @@ def Create(self, name: str, runner, repository_path: str, output_prefix: str, builder.output_dir = os.path.join(output_prefix, name) builder.chip_dir = repository_path builder.enable_flashbundle(enable_flashbundle) + builder.pw_command_launcher = pw_command_launcher return builder diff --git a/scripts/build/build_examples.py b/scripts/build/build_examples.py index 5f8a7ef2a5c7bd..7abaf014027274 100755 --- a/scripts/build/build_examples.py +++ b/scripts/build/build_examples.py @@ -107,10 +107,15 @@ def ValidateRepoPath(context, parameter, value): default=False, is_flag=True, help='Skip timestaps in log output') +@click.option( + '--pw-command-launcher', + help=( + 'Set pigweed command launcher. E.g.: "--pw-command-launcher=ccache" ' + 'for using ccache when building examples.')) @click.pass_context def main(context, log_level, target, repo, out_prefix, clean, dry_run, dry_run_output, enable_flashbundle, - no_log_timestamps): + no_log_timestamps, pw_command_launcher): # Ensures somewhat pretty logging of what is going on log_fmt = '%(asctime)s %(levelname)-7s %(message)s' if no_log_timestamps: @@ -136,7 +141,8 @@ def main(context, log_level, target, repo, context.obj = build.Context( repository_path=repo, output_prefix=out_prefix, runner=runner) context.obj.SetupBuilders( - targets=requested_targets, enable_flashbundle=enable_flashbundle) + targets=requested_targets, enable_flashbundle=enable_flashbundle, + pw_command_launcher=pw_command_launcher) if clean: context.obj.CleanOutputDirectories() diff --git a/scripts/build/builders/builder.py b/scripts/build/builders/builder.py index c221e76a428091..d54999075a0637 100644 --- a/scripts/build/builders/builder.py +++ b/scripts/build/builders/builder.py @@ -22,8 +22,8 @@ class Builder(ABC): """Generic builder base class for CHIP. - Provides ability to boostrap and copy output artifacts and subclasses can use - a generic shell runner. + Provides ability to bootstrap and copy output artifacts and subclasses can + use a generic shell runner. """ @@ -36,6 +36,9 @@ def __init__(self, root, runner): self.identifier = None self.output_dir = None + # Allow to override the default build command + self.pw_command_launcher = None + def enable_flashbundle(self, enable_flashbundle: bool): self._enable_flashbundle = enable_flashbundle diff --git a/scripts/build/builders/gn.py b/scripts/build/builders/gn.py index 298a9e9c90145b..bd7fdc398dde45 100644 --- a/scripts/build/builders/gn.py +++ b/scripts/build/builders/gn.py @@ -60,7 +60,10 @@ def generate(self): '--root=%s' % self.root ] - extra_args = self.GnBuildArgs() + extra_args = [] + if self.pw_command_launcher: + extra_args.append('pw_command_launcher="%s"' % self.pw_command_launcher) + extra_args.extend(self.GnBuildArgs() or []) if extra_args: cmd += ['--args=%s' % ' '.join(extra_args)] From 85bf09358961a581725640c2117b69e19b292437 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 21 Nov 2022 14:40:33 +0100 Subject: [PATCH 2/4] Do not use setter pattern for simple flag --- scripts/build/build/target.py | 2 +- scripts/build/builders/builder.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/scripts/build/build/target.py b/scripts/build/build/target.py index aa3a16d1600530..4091e62c62cf07 100644 --- a/scripts/build/build/target.py +++ b/scripts/build/build/target.py @@ -348,7 +348,7 @@ def Create(self, name: str, runner, repository_path: str, output_prefix: str, builder.identifier = name builder.output_dir = os.path.join(output_prefix, name) builder.chip_dir = repository_path - builder.enable_flashbundle(enable_flashbundle) + builder.enable_flashbundle = enable_flashbundle builder.pw_command_launcher = pw_command_launcher return builder diff --git a/scripts/build/builders/builder.py b/scripts/build/builders/builder.py index d54999075a0637..0615d1800db581 100644 --- a/scripts/build/builders/builder.py +++ b/scripts/build/builders/builder.py @@ -30,18 +30,16 @@ class Builder(ABC): def __init__(self, root, runner): self.root = os.path.abspath(root) self._runner = runner - self._enable_flashbundle = False # Set post-init once actual build target is known self.identifier = None self.output_dir = None + # Enable flashbundle generation stage + self.enable_flashbundle = False # Allow to override the default build command self.pw_command_launcher = None - def enable_flashbundle(self, enable_flashbundle: bool): - self._enable_flashbundle = enable_flashbundle - @abstractmethod def generate(self): """Generate the build files - generally the ninja/makefiles""" @@ -84,13 +82,13 @@ def flashbundle(self): def outputs(self): artifacts = self.build_outputs() - if self._enable_flashbundle: + if self.enable_flashbundle: artifacts.update(self.flashbundle()) return artifacts def build(self): self._build() - if self._enable_flashbundle: + if self.enable_flashbundle: self._generate_flashbundle() def _Execute(self, cmdarray, title=None): From 246739dcff29fed5c8e5706b4ffee990ed3ff7d7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 21 Nov 2022 19:28:31 +0100 Subject: [PATCH 3/4] Keep all builder options in dedicated dataclass --- scripts/build/build/__init__.py | 10 ++++------ scripts/build/build/target.py | 7 ++++--- scripts/build/build_examples.py | 9 +++++---- scripts/build/builders/builder.py | 19 ++++++++++++------- scripts/build/builders/gn.py | 4 ++-- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/scripts/build/build/__init__.py b/scripts/build/build/__init__.py index 479cf6e918290d..c363f6c3266f84 100644 --- a/scripts/build/build/__init__.py +++ b/scripts/build/build/__init__.py @@ -5,6 +5,7 @@ from typing import Sequence from .targets import BUILD_TARGETS +from builders.builder import BuilderOptions class BuildSteps(Enum): @@ -24,9 +25,7 @@ def __init__(self, runner, repository_path: str, output_prefix: str): self.output_prefix = output_prefix self.completed_steps = set() - def SetupBuilders(self, targets: Sequence[str], - enable_flashbundle: bool, - pw_command_launcher: str): + def SetupBuilders(self, targets: Sequence[str], options: BuilderOptions): """ Configures internal builders for the given platform/board/app combination. @@ -36,9 +35,8 @@ def SetupBuilders(self, targets: Sequence[str], for target in targets: found = False for choice in BUILD_TARGETS: - builder = choice.Create( - target, self.runner, self.repository_path, self.output_prefix, - enable_flashbundle, pw_command_launcher) + builder = choice.Create(target, self.runner, self.repository_path, + self.output_prefix, options) if builder: self.builders.append(builder) found = True diff --git a/scripts/build/build/target.py b/scripts/build/build/target.py index 4091e62c62cf07..8d281906d633d2 100644 --- a/scripts/build/build/target.py +++ b/scripts/build/build/target.py @@ -46,6 +46,8 @@ from dataclasses import dataclass from typing import Any, Dict, List, Iterable, Optional +from builders.builder import BuilderOptions + report_rejected_parts = True @@ -330,7 +332,7 @@ def StringIntoTargetParts(self, value: str): return _StringIntoParts(value, suffix, self.fixed_targets, self.modifiers) def Create(self, name: str, runner, repository_path: str, output_prefix: str, - enable_flashbundle: bool, pw_command_launcher: str): + builder_options: BuilderOptions): parts = self.StringIntoTargetParts(name) @@ -348,7 +350,6 @@ def Create(self, name: str, runner, repository_path: str, output_prefix: str, builder.identifier = name builder.output_dir = os.path.join(output_prefix, name) builder.chip_dir = repository_path - builder.enable_flashbundle = enable_flashbundle - builder.pw_command_launcher = pw_command_launcher + builder.options = builder_options return builder diff --git a/scripts/build/build_examples.py b/scripts/build/build_examples.py index 7abaf014027274..5792bfaec64274 100755 --- a/scripts/build/build_examples.py +++ b/scripts/build/build_examples.py @@ -22,7 +22,7 @@ import coloredlogs import build -from glob_matcher import GlobMatcher +from builders.builder import BuilderOptions from runner import PrintOnlyRunner, ShellRunner sys.path.append(os.path.abspath(os.path.dirname(__file__))) @@ -140,9 +140,10 @@ def main(context, log_level, target, repo, context.obj = build.Context( repository_path=repo, output_prefix=out_prefix, runner=runner) - context.obj.SetupBuilders( - targets=requested_targets, enable_flashbundle=enable_flashbundle, - pw_command_launcher=pw_command_launcher) + context.obj.SetupBuilders(targets=requested_targets, options=BuilderOptions( + enable_flashbundle=enable_flashbundle, + pw_command_launcher=pw_command_launcher, + )) if clean: context.obj.CleanOutputDirectories() diff --git a/scripts/build/builders/builder.py b/scripts/build/builders/builder.py index 0615d1800db581..99bc9255ee5ef9 100644 --- a/scripts/build/builders/builder.py +++ b/scripts/build/builders/builder.py @@ -17,6 +17,15 @@ import shutil import tarfile from abc import ABC, abstractmethod +from dataclasses import dataclass + + +@dataclass +class BuilderOptions: + # Enable flashbundle generation stage + enable_flashbundle: bool = False + # Allow to wrap default build command + pw_command_launcher: str = None class Builder(ABC): @@ -34,11 +43,7 @@ def __init__(self, root, runner): # Set post-init once actual build target is known self.identifier = None self.output_dir = None - - # Enable flashbundle generation stage - self.enable_flashbundle = False - # Allow to override the default build command - self.pw_command_launcher = None + self.options = BuilderOptions() @abstractmethod def generate(self): @@ -82,13 +87,13 @@ def flashbundle(self): def outputs(self): artifacts = self.build_outputs() - if self.enable_flashbundle: + if self.options.enable_flashbundle: artifacts.update(self.flashbundle()) return artifacts def build(self): self._build() - if self.enable_flashbundle: + if self.options.enable_flashbundle: self._generate_flashbundle() def _Execute(self, cmdarray, title=None): diff --git a/scripts/build/builders/gn.py b/scripts/build/builders/gn.py index bd7fdc398dde45..bacd87d2399e02 100644 --- a/scripts/build/builders/gn.py +++ b/scripts/build/builders/gn.py @@ -61,8 +61,8 @@ def generate(self): ] extra_args = [] - if self.pw_command_launcher: - extra_args.append('pw_command_launcher="%s"' % self.pw_command_launcher) + if self.options.pw_command_launcher: + extra_args.append('pw_command_launcher="%s"' % self.options.pw_command_launcher) extra_args.extend(self.GnBuildArgs() or []) if extra_args: cmd += ['--args=%s' % ' '.join(extra_args)] From 1afcbb888f6b9d4bbf978c08e934c5f639465834 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 21 Nov 2022 20:53:22 +0100 Subject: [PATCH 4/4] Fix unit test module import --- scripts/build/build/test_target.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/scripts/build/build/test_target.py b/scripts/build/build/test_target.py index 334e56720f3eb2..02ded283e96e91 100755 --- a/scripts/build/build/test_target.py +++ b/scripts/build/build/test_target.py @@ -14,15 +14,11 @@ # limitations under the License. import unittest +import sys +import os -try: - from build.target import * -except: - import sys - import os - - sys.path.append(os.path.abspath(os.path.dirname(__file__))) - from target import * +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) +from build.target import BuildTarget, TargetPart # noqa: E402 class FakeBuilder: