Skip to content

Add justifications for missing PICO_BAZEL_CONFIG and PICO_CMAKE_CONFIG entries#2875

Open
will-v-pi wants to merge 1 commit intodevelopfrom
bazel-missing-configs
Open

Add justifications for missing PICO_BAZEL_CONFIG and PICO_CMAKE_CONFIG entries#2875
will-v-pi wants to merge 1 commit intodevelopfrom
bazel-missing-configs

Conversation

@will-v-pi
Copy link
Copy Markdown
Contributor

Add justifications for why PICO_BAZEL_CONFIG and PICO_CMAKE_CONFIG entries differ - mostly they are configs that only make sense in one or the other, but some added to TODOs to implement in Bazel in the future

This means compare_build_systems.py doesn't raise any more warnings

Fixes #2868

…NFIG` entries

This means compare_build_systems.py doesn't raise any more warnings
@will-v-pi will-v-pi added this to the 2.2.1 milestone Mar 20, 2026
@will-v-pi will-v-pi requested a review from kilograham March 20, 2026 14:54
Comment on lines +129 to +130
# These aren't supported as build flags in CMake either, but they appear
# as comments which this script can't tell.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternatively, it looks like this simple tweak allows this script to ignore "commented out" PICO_CMAKE_CONFIG options?

--- a/tools/compare_build_systems.py
+++ b/tools/compare_build_systems.py
@@ -187,6 +187,9 @@ def FindKnownOptions(option_pattern_matcher, file_paths):
     for p in file_paths:
         with open(p, "r") as f:
             for line in f:
+                if re.match("\s*#\s*#", line):
+                    continue
+
                 match = re.search(pattern, line)
                 if not match:
                     continue

# have to write out a bespoke run_binary.
"PICO_NO_UF2",
"PICO_NO_COPRO_DIS",
"PICO_ALLOW_EXAMPLE_KEYS",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should there be an explicit TODO: to add a similar option to Bazel?

"PICO_DEFAULT_BOARD_rp2350",
"PICO_DEFAULT_BOARD_host",
# Bazel doesn't provide a way of overriding the defaults for this.
# TODO: Provide a way to customise these toolchain flags.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should be pretty easy. I'll take a look.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants