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.

Sign up for GitHub

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

Jump to bottom

Extend build system comparison checks #2039

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Extend build system comparison checks #2039

merged 1 commit into from
Nov 13, 2024

Conversation

armandomontanez
Copy link
Contributor

  • Adds check that ensures the Bazel and CMake Pico SDK version strings stay in sync.
  • Adds check that ensures the Bazel pins for external dependencies stays in sync with the git submodule pins.
  • Updates cyw43-driver pin in Bazel.
  • Makes the checks for some build configurability options non-blocking.

@armandomontanez
Extend build system checks
c52d0f4 
* Adds check that ensures the Bazel and CMake Pico SDK version strings
  stay in sync.
* Adds check that ensures the Bazel pins for external dependencies stays
  in sync with the git submodule pins.
* Updates cyw43-driver pin in Bazel.
* Makes the checks for some build configurability options non-blocking.
@armandomontanez
Copy link
Contributor Author

Example output:

$ python3 tools/run_all_bazel_checks.py --program=other
###############################################################
###   Ensure build system configurations options match...   ###
###############################################################
[INFO] [1/3] Checking build system configuration flags...
[WARNING]     PICO_DEFAULT_RP2350_PLATFORM does not exist in Bazel
[WARNING]     PICO_GCC_TRIPLE does not exist in Bazel
[WARNING]     PICO_NO_FLASH does not exist in Bazel
[WARNING]     PICO_COPY_TO_RAM does not exist in Bazel
[WARNING]     PICO_NO_COPRO_DIS does not exist in Bazel
[WARNING]     PICO_RP2350_ARM_S_CONFIG_HEADER_FILES does not exist in Bazel
[WARNING]     PICO_RP2350_RISCV_CONFIG_HEADER_FILES does not exist in Bazel
[INFO] [2/4] Checking build system defines...
[INFO] [3/4] Checking submodule pins...
[INFO]     External pins for tinyusb match!
[INFO]     External pins for btstack match!
[INFO]     External pins for cyw43-driver match!
[INFO]     External pins for lwip match!
[INFO] [4/4] Checking version strings...
[INFO] Passed with no blocking failures

#############################################################
###   Ensure source files are present in Bazel build...   ###
#############################################################
[DEBUG] Ignoring files:

  docs/index.h
  src/common/pico_util/doc.h
  src/rp2040/boot_stage2/doc.h
  src/rp2350/boot_stage2/doc.h
  src/rp2_common/hardware_dcp/doc.h
  src/rp2_common/pico_btstack/doc.h
  src/rp2_common/pico_clib_interface/doc.h
  src/rp2_common/pico_crt0/doc.h
  src/rp2_common/pico_cxx_options/doc.h
  src/rp2_common/pico_lwip/doc.h
  src/rp2_common/pico_standard_binary_info/doc.h
  src/rp2_common/pico_standard_link/doc.h
  src/rp2_common/tinyusb/doc.h

[INFO] Checking all source files are accounted for in Bazel.
[INFO] Success! All files accounted for in Bazel.

###########################################
###   All checks successfully passed!   ###
###########################################

@kilograham kilograham merged commit 1b64cac into raspberrypi:develop Nov 13, 2024
4 checks passed
lurch
lurch reviewed Nov 13, 2024
View reviewed changes
tools/compare_build_systems.py
"CYW43_DEFAULT_PIN_WL_CLOCK",
"CYW43_DEFAULT_PIN_WL_CS",
"CYW43_PIO_CLOCK_DIV_INT",
"CYW43_PIO_CLOCK_DIV_FRAC",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be changed from CYW43_PIO_CLOCK_DIV_FRAC to CYW43_PIO_CLOCK_DIV_FRAC8 to match the change in #1926 ?
Also, this list might be missing CYW43_PIN_WL_DYNAMIC which is one of the things that #2033 fixes in the CMake-world?

Copy link
Contributor Author

@armandomontanez armandomontanez Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bleh, I forgot to pull latest changes before adding these. This just squelches the warnings, the findings shouldn't get in the way of anything. I can put up a separate PR to add those.

lurch
lurch reviewed Nov 13, 2024
View reviewed changes
tools/compare_build_systems.py

current_submodule_pin = subprocess.run(
("git", "-C", SDK_ROOT, "rev-parse", f'HEAD:lib/{maybe_match.group("dependency")}'),
text=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is assuming that our submodules are always in the lib/ subdirectory? Whilst this will probably always be true for pico-sdk, I guess moving the lib/ part of the path into the MODULE.bazel file might give you an extra bit of flexibility? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent out #2040 for this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lurch lurch lurch left review comments

@kilograham kilograham kilograham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@armandomontanez @lurch @kilograham