From 07d6dc1315ea3e461e7bca133b0a4111713d9b2b Mon Sep 17 00:00:00 2001 From: armandomontanez Date: Sat, 12 Oct 2024 15:41:43 -0700 Subject: [PATCH] [Bazel] Fix bazel build, add presubmit (#1973) * [Bazel] Fix bazel build, add presubmit * Fixes a missing dep in the Bazel build breaking the host build. * Automagically finds all board headers. * Improves presubmit script polish for GH Action readiness. * Adds a GitHub action workflow for the Bazel build. * Disable failing checks * Disables Windows, as there's a mix of real build errors and overly-ambitious checks that don't work on Windows. * Disables extra checks temporarily since it's currently failing. --- .github/workflows/bazel_build.yml | 76 +++++++++++++++++++++ src/boards/BUILD.bazel | 101 ++-------------------------- src/host/hardware_timer/BUILD.bazel | 5 +- src/host/pico_platform/BUILD.bazel | 1 + tools/bazel_build.py | 5 +- tools/bazel_common.py | 18 +++-- tools/compare_build_systems.py | 22 +++--- tools/run_all_bazel_checks.py | 41 +++++++++-- 8 files changed, 150 insertions(+), 119 deletions(-) create mode 100644 .github/workflows/bazel_build.yml diff --git a/.github/workflows/bazel_build.yml b/.github/workflows/bazel_build.yml new file mode 100644 index 000000000..183543298 --- /dev/null +++ b/.github/workflows/bazel_build.yml @@ -0,0 +1,76 @@ +name: Bazel presubmit checks + +on: + push: + pull_request: + +jobs: + bazel-build-check: + strategy: + matrix: + # TODO: Windows is currently broken. + os: [ubuntu-latest, macos-latest] + fail-fast: false + runs-on: ${{ matrix.os }} + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Get Bazel + uses: bazel-contrib/setup-bazel@0.9.0 + with: + # Avoid downloading Bazel every time. + bazelisk-cache: true + # Store build cache per workflow. + disk-cache: ${{ github.workflow }} + # Share repository cache between workflows. + repository-cache: true + # Only needed to drive the presbumit scripts. + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.10' + - name: Fetch latest Picotool + uses: actions/checkout@v4 + with: + repository: raspberrypi/picotool + ref: develop + fetch-depth: 0 + path: lib/picotool + - name: Full Bazel build with develop Picotool + run: python3 tools/run_all_bazel_checks.py --program=build --picotool-dir=lib/picotool + # Checks that the current BCR-requested version of Picotool builds. + - name: Bazel Picotool backwards compatibility + run: bazel build @picotool//:picotool + # Add back when it begins to pass. + # other-bazel-checks: + # runs-on: ubuntu-latest + # steps: + # - name: Checkout + # uses: actions/checkout@v4 + # with: + # fetch-depth: 0 + # - name: Get Bazel + # uses: bazel-contrib/setup-bazel@0.9.0 + # with: + # # Avoid downloading Bazel every time. + # bazelisk-cache: true + # # Store build cache per workflow. + # disk-cache: ${{ github.workflow }} + # # Share repository cache between workflows. + # repository-cache: true + # # Only needed to drive the presbumit scripts. + # - name: Setup Python + # uses: actions/setup-python@v5 + # with: + # python-version: '3.10' + # - name: Fetch latest Picotool + # uses: actions/checkout@v4 + # with: + # repository: raspberrypi/picotool + # ref: develop + # fetch-depth: 0 + # path: lib/picotool + # - name: Other Bazel checks + # run: python3 tools/run_all_bazel_checks.py --program=other --picotool-dir=lib/picotool diff --git a/src/boards/BUILD.bazel b/src/boards/BUILD.bazel index f7a6f0a13..797f753ce 100644 --- a/src/boards/BUILD.bazel +++ b/src/boards/BUILD.bazel @@ -3,104 +3,15 @@ load("//bazel/util:multiple_choice_flag.bzl", "declare_flag_choices", "flag_choi package(default_visibility = ["//visibility:public"]) -# Known board choices: +# Find all boards. +BOARD_CHOICE_FILES = glob(["include/boards/*.h"]) + +# Extract just the name of the board. BOARD_CHOICES = [ - "0xcb_helios", - "adafruit_feather_rp2040_usb_host", - "adafruit_feather_rp2040", - "adafruit_feather_rp2350", - "adafruit_itsybitsy_rp2040", - "adafruit_kb2040", - "adafruit_macropad_rp2040", - "adafruit_qtpy_rp2040", - "adafruit_trinkey_qt2040", - "amethyst_fpga", - "archi", - "arduino_nano_rp2040_connect", - "cytron_maker_pi_rp2040", - "datanoisetv_rp2040_dsp", - "datanoisetv_rp2350_dsp", - "defcon32_badge", - "eetree_gamekit_rp2040", - "garatronic_pybstick26_rp2040", - "gen4_rp2350_24", - "gen4_rp2350_24ct", - "gen4_rp2350_24t", - "gen4_rp2350_28", - "gen4_rp2350_28ct", - "gen4_rp2350_28t", - "gen4_rp2350_32", - "gen4_rp2350_32ct", - "gen4_rp2350_32t", - "gen4_rp2350_35", - "gen4_rp2350_35ct", - "gen4_rp2350_35t", - "hellbender_2350A_devboard", - "ilabs_challenger_rp2350_bconnect", - "ilabs_challenger_rp2350_wifi_ble", - "ilabs_opendec02", - "melopero_perpetuo_rp2350_lora", - "melopero_shake_rp2040", - "metrotech_xerxes_rp2040", - "net8086_usb_interposer", - "none", - "nullbits_bit_c_pro", - "phyx_rick_tny_rp2350", - "pi-plates_micropi", - "pico_w", - "pico", - "pico2", - "pimoroni_badger2040", - "pimoroni_interstate75", - "pimoroni_keybow2040", - "pimoroni_motor2040", - "pimoroni_pga2040", - "pimoroni_pga2350", - "pimoroni_pico_plus2_rp2350", - "pimoroni_picolipo_16mb", - "pimoroni_picolipo_4mb", - "pimoroni_picosystem", - "pimoroni_plasma2040", - "pimoroni_plasma2350", - "pimoroni_servo2040", - "pimoroni_tiny2040_2mb", - "pimoroni_tiny2040", - "pimoroni_tiny2350", - "pololu_3pi_2040_robot", - "pololu_zumo_2040_robot", - "seeed_xiao_rp2040", - "seeed_xiao_rp2350", - "solderparty_rp2040_stamp_carrier", - "solderparty_rp2040_stamp_round_carrier", - "solderparty_rp2040_stamp", - "solderparty_rp2350_stamp_xl", - "solderparty_rp2350_stamp", - "sparkfun_micromod", - "sparkfun_promicro_rp2350", - "sparkfun_promicro", - "sparkfun_thingplus", - "switchscience_picossci2_conta_base", - "switchscience_picossci2_dev_board", - "switchscience_picossci2_micro", - "switchscience_picossci2_rp2350_breakout", - "switchscience_picossci2_tiny", - "tinycircuits_thumby_color_rp2350", - "vgaboard", - "waveshare_rp2040_lcd_0.96", - "waveshare_rp2040_lcd_1.28", - "waveshare_rp2040_one", - "waveshare_rp2040_plus_16mb", - "waveshare_rp2040_plus_4mb", - "waveshare_rp2040_zero", - "weact_studio_rp2040_16mb", - "weact_studio_rp2040_2mb", - "weact_studio_rp2040_4mb", - "weact_studio_rp2040_8mb", - "wiznet_w5100s_evb_pico", + path.removeprefix("include/boards/").removesuffix(".h") + for path in BOARD_CHOICE_FILES ] -BOARD_CHOICE_FILES = ["include/boards/" + c + ".h" for c in BOARD_CHOICES] - BOARD_CHOICE_MAP = {c: [":{}".format(c)] for c in BOARD_CHOICES} # PICO_BUILD_DEFINE: PICO_BOARD, Name of board, type=string, default=CMake PICO_BOARD variable, group=pico_base diff --git a/src/host/hardware_timer/BUILD.bazel b/src/host/hardware_timer/BUILD.bazel index 57bc42555..4c255a925 100644 --- a/src/host/hardware_timer/BUILD.bazel +++ b/src/host/hardware_timer/BUILD.bazel @@ -15,7 +15,10 @@ cc_library( defines = _DEFINES, includes = ["include"], target_compatible_with = ["//bazel/constraint:host"], - visibility = ["//src/common/pico_time:__pkg__"], + visibility = [ + "//src/common/pico_time:__pkg__", + "//src/host/pico_platform:__pkg__", + ], deps = ["//src/common/pico_base_headers"], ) diff --git a/src/host/pico_platform/BUILD.bazel b/src/host/pico_platform/BUILD.bazel index 610a38352..c29a5bbaf 100644 --- a/src/host/pico_platform/BUILD.bazel +++ b/src/host/pico_platform/BUILD.bazel @@ -30,5 +30,6 @@ cc_library( ":pico_platform_internal", ":platform_defs", "//src/common/pico_base_headers", + "//src/host/hardware_timer:hardware_timer_headers", ], ) diff --git a/tools/bazel_build.py b/tools/bazel_build.py index b303e3402..546eb7d9e 100755 --- a/tools/bazel_build.py +++ b/tools/bazel_build.py @@ -15,6 +15,7 @@ override_picotool_arg, parse_common_args, print_framed_string, + print_to_stderr, run_bazel, setup_logging, ) @@ -196,12 +197,12 @@ def build_all_configurations(picotool_dir): ) if result.returncode != 0: failed_builds.append(config["name"]) - print() + print_to_stderr() if failed_builds: print_framed_string("ERROR: One or more builds failed.") for build in failed_builds: - print(f" * FAILED: {build} build") + print_to_stderr(f" * FAILED: {build} build") return 1 print_framed_string("All builds successfully passed!") diff --git a/tools/bazel_common.py b/tools/bazel_common.py index 487044a46..e0468c991 100644 --- a/tools/bazel_common.py +++ b/tools/bazel_common.py @@ -13,6 +13,7 @@ import shlex import shutil import subprocess +import sys _LOG = logging.getLogger(__file__) @@ -34,13 +35,16 @@ def parse_common_args(): parser = argparse.ArgumentParser() + add_common_args(parser) + return parser.parse_args() + +def add_common_args(parser): parser.add_argument( "--picotool-dir", help="Use a local copy of Picotool rather than the dynamically fetching it", default=None, type=Path, ) - return parser.parse_args() def override_picotool_arg(picotool_dir): return f"--override_module=picotool={picotool_dir.resolve()}" @@ -49,7 +53,7 @@ def override_picotool_arg(picotool_dir): def bazel_command() -> str: """Return the path to bazelisk or bazel.""" if shutil.which("bazelisk"): - return "bazelisk" + return shutil.which("bazelisk") if shutil.which("bazel"): return "bazel" @@ -93,12 +97,16 @@ def run_bazel(args, check=False, **kwargs): return proc +def print_to_stderr(*args, **kwargs): + print(*args, file=sys.stderr, **kwargs) + + def print_framed_string(s): """Frames a string of text and prints it to highlight script steps.""" header_spacer = "#" * (len(s) + 12) - print(header_spacer) - print("### " + s + " ###") - print(header_spacer) + print_to_stderr(header_spacer) + print_to_stderr("### " + s + " ###") + print_to_stderr(header_spacer) def setup_logging(): diff --git a/tools/compare_build_systems.py b/tools/compare_build_systems.py index c5762817b..4a568aec4 100755 --- a/tools/compare_build_systems.py +++ b/tools/compare_build_systems.py @@ -14,12 +14,15 @@ from dataclasses import dataclass import glob +import logging import os import re import sys from typing import Dict -from bazel_common import SDK_ROOT +from bazel_common import SDK_ROOT, setup_logging + +_LOG = logging.getLogger(__file__) CMAKE_FILE_TYPES = ( "**/CMakeLists.txt", @@ -182,17 +185,17 @@ def OptionsAreEqual(bazel_option, cmake_option): if bazel_option is None: if cmake_option.name in CMAKE_ONLY_ALLOWLIST: return True - print(f" {cmake_option.name} does not exist in Bazel") + _LOG.warning(f" {cmake_option.name} does not exist in Bazel") return False elif cmake_option is None: if bazel_option.name in BAZEL_ONLY_ALLOWLIST: return True - print(f" {bazel_option.name} does not exist in CMake") + _LOG.warning(f" {bazel_option.name} does not exist in CMake") return False elif not bazel_option.matches(cmake_option): - print(" Bazel and CMAKE definitions do not match:") - print(f" [CMAKE] {bazel_option}") - print(f" [BAZEL] {cmake_option}") + _LOG.error(" Bazel and CMAKE definitions do not match:") + _LOG.error(f" [CMAKE] {bazel_option}") + _LOG.error(f" [BAZEL] {cmake_option}") return False return True @@ -224,22 +227,23 @@ def compare_build_systems(): for f in glob.glob(os.path.join(SDK_ROOT, p), recursive=True) ] - print("[1/2] Checking build system configuration flags...") + _LOG.info("[1/2] Checking build system configuration flags...") build_options_ok = CompareOptions( "PICO_BAZEL_CONFIG", bazel_files, "PICO_CMAKE_CONFIG", cmake_files ) - print("[2/2] Checking build system defines...") + _LOG.info("[2/2] Checking build system defines...") build_defines_ok = CompareOptions( "PICO_BUILD_DEFINE", bazel_files, "PICO_BUILD_DEFINE", cmake_files ) if build_options_ok and build_defines_ok: - print("OK") + _LOG.info("OK") return 0 return 1 if __name__ == "__main__": + setup_logging() sys.exit(compare_build_systems()) diff --git a/tools/run_all_bazel_checks.py b/tools/run_all_bazel_checks.py index dfdc24633..13a0656c6 100755 --- a/tools/run_all_bazel_checks.py +++ b/tools/run_all_bazel_checks.py @@ -6,10 +6,16 @@ # # Runs all Bazel checks. +import argparse import sys from bazel_build import build_all_configurations -from bazel_common import setup_logging, print_framed_string, parse_common_args +from bazel_common import ( + setup_logging, + print_framed_string, + print_to_stderr, + add_common_args, +) from compare_build_systems import compare_build_systems from check_source_files_in_bazel_build import check_sources_in_bazel_build @@ -18,34 +24,55 @@ def main(): setup_logging() failed_steps = [] - args = parse_common_args() - - steps = ( + parser = argparse.ArgumentParser() + add_common_args(parser) + parser.add_argument( + "--program", + help="A program to run", + choices = [ + "all", + "build", + "other", + ], + default="all", + ) + args = parser.parse_args() + build_steps = ( { + "step_name": "build", "description": "Bazel build", "action": lambda : build_all_configurations(args.picotool_dir), }, + ) + other_steps = ( { "description": "Ensure build system configurations options match", "action": compare_build_systems, }, { + "step_name": "check_srcs_in_build", "description": "Ensure source files are present in Bazel build", "action": lambda : check_sources_in_bazel_build(args.picotool_dir), }, ) + steps_to_run = [] + run_all_steps = args.program == "all" + if args.program == "build" or run_all_steps: + steps_to_run.extend(build_steps) + if args.program == "other" or run_all_steps: + steps_to_run.extend(other_steps) - for step in steps: + for step in steps_to_run: print_framed_string(f"{step['description']}...") returncode = step["action"]() if returncode != 0: failed_steps.append(step["description"]) - print() + print_to_stderr() if failed_steps: print_framed_string("ERROR: One or more steps failed.") for build in failed_steps: - print(f" * FAILED: {build}") + print_to_stderr(f" * FAILED: {build}") return 1 print_framed_string("All checks successfully passed!")