Fix package_folder_prefix quoting so the bundle contains its libraries again - #1
Merged
Merged
Conversation
circuitpython-build-bundles splits --package_folder_prefix on ", " and matches each entry with str.startswith(). The gawk that builds the list wrapped it in literal double quotes, so the first and last entries came through as '"sensirion_i2c_driver' and 'sensirion_i2c_sen5x"' and matched no folder. Both libraries then fell back to legacy autodetection, which found only the top-level conftest.py, and the bundle shipped one module instead of the drivers. With only two libraries here, both entries carry a stray quote, so every release asset since the tooling moved on has been effectively empty. The same bug is in the upstream community bundle, where it silently drops the first and last library of the ls -U ordering. Also allow build.yml to be started by hand, as there was no way to re-run it without pushing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tyeth
added a commit
to tyeth/deepsleep_espnow_wifi_and_ble_env_collector
that referenced
this pull request
Sep 10, 2026
The firmware run this file recorded as failed has been superseded: the integration-pico2w-ble branch it was missing now exists on tyeth/hal_rpi_pico, and Build board (custom) produces both Pico 2 W (34467425010) and Pico W (34494018766) firmware. Take mpy-cross from the latter -- it is green, expires two days later, and is the same build as the firmware. Both compilers emit mpy v6.3, so no compiled output changes. The 'otherwise-red run' caveat is also gone: tests / zephyr is green again (tyeth/circuitpython#11). The custom bundle's missing 10.x-mpy turned out to be a build bug, not a dormant repo: its build.sh passes --package_folder_prefix wrapped in literal quotes, which startswith() then never matches, so both libraries fell back to autodetection and the bundle shipped only conftest. Fixed in good-enough-technology/CircuitPython_GoodEnough_Bundle#1; the 10.x-mpy reaches circup once that is merged and released. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing the literal quotes changed one failure mode for the worse. If the
gawk matches nothing, the prefix is empty; build-tools splits on ", " and
matches with startswith(), and "".startswith("") is True, so every folder
becomes a package (module_name came out as 'docs'). Previously the empty
value reached an unquoted ${{ }} and click failed on the missing argument,
which at least stopped the build. Fail loudly instead.
The character check covers the part quoting cannot: ${{ }} splices the
value into the run: block as text before any shell parses it, so a folder
name containing a quote or backtick escapes the quotes added around the
expansion. Restricting the list to what a module name can contain closes
that, rather than relying on the quotes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… versions
The previous commit guarded the symptom. The structure was the problem:
the prefix went out through $GITHUB_OUTPUT and came back as a ${{ }}
expression, which GitHub splices into the next run: block as script text
before any shell parses it.
That hop is worse than what it replaced. The original 'echo prefix=$( ... )'
used an unquoted command substitution, so word splitting collapsed newlines
into spaces and only ever one line reached $GITHUB_OUTPUT. Assigning to a
variable and echoing it quoted preserves them: a directory name containing
a newline writes extra variables into $GITHUB_OUTPUT, which then chain into
the next step. Measured: old form 1 line, new form 3.
So drop the hop. The prefix is computed in the Build assets step and passed
as "$prefix" -- real shell quoting this time, since bash does the expansion
rather than the templating engine. filename_prefix comes in through env for
the same reason. No expression is now spliced into any run: block, and the
character allowlist added in the previous commit is no longer load-bearing,
so it goes.
Also takes upstream's actions/checkout@v6 and actions/setup-python@v6,
which is what the Node 20 deprecation notice on our runs was about. The
upstream AWS S3 upload step is deliberately not adopted: it targets
Adafruit's bucket.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bundle was shipping nothing
build.shandrelease.ymlbuilt the--package_folder_prefixlist with a gawk that wrapped it in literal double quotes.circuitpython-build-bundlessplits that argument on", "and matches each entry withstr.startswith(), so the entries arrived as\"sensirion_i2c_driverandsensirion_i2c_sen5x\"and matched no folder. Both libraries fell back to legacy autodetection, which finds only the top-levelconftest.py.Measured with build-tools 1.20.1 against the pinned submodules:
--package_folder_prefixpybundle\"sensirion_i2c_driver, sensirion_i2c_sen5x\"(as CI passed it)is_package=False,module_name=conftestsensirion_i2c_driver, sensirion_i2c_sen5xis_package=True, 20 + 33 filesTwo things make it total here rather than partial: with exactly two libraries, both entries are the damaged ones, and both drivers are third-party Sensirion repos with no
[tool.setuptools]metadata, so they depend on legacy autodetection and therefore on the prefix.Upstream
adafruit/CircuitPython_Community_Bundlehas the identicalbuild.sh, but there the defect is latent: of its 47 prefix entries only 11 belong to libraries still using legacy autodetection — the rest declaretool.setuptools.packagesinpyproject.toml, which takes precedence — and neither damaged entry is currently one of the 11. Its bundle is byte-identical with and without the fix. Reported as hardening in adafruit#290.Getting the fix right took three passes
764b924).33c7c06). Removing the quotes made one failure mode worse: an empty prefix used to reach an unquoted expression and fail on the missing argument, but as""it splits to[''], and''.startswith('')is True, so every folder becomes a package (module_namecame outdocs). Now a hard error.f53aef0) — the real fix.That third one matters. The original
echo prefix=$( ... )used an unquoted command substitution, so word splitting collapsed newlines into spaces and only one line could ever reach$GITHUB_OUTPUT. Assigning to a variable and echoing it quoted preserves them, so a directory name containing a newline writes extra variables that chain into the next step's command line. Measured: old form 1 line, new form 3.Rather than patch that with an allowlist, the prefix is now computed in the
Build assetsstep and passed as"$prefix"— real shell quoting, because bash does the expansion rather than the templating engine.filename_prefixcomes in viaenv:for the same reason. No expression is spliced into anyrun:block any more, which also closes the pre-existing case of a folder name carrying a quote or backtick.Also
Takes upstream's
actions/checkout@v6andactions/setup-python@v6(what the Node 20 deprecation notice was about), and addsworkflow_dispatchtobuild.yml— there was previously no way to re-run it without pushing. The upstream AWS S3 upload step is deliberately not adopted; it targets Adafruit's bucket. The library set is untouched.Verified
Run 34542512867 is green and produces
py(178.5 KB),9.x-mpyand10.x-mpy(52.5 KB, mpy-cross 10.0.0), all containingsensirion_i2c_sen5xandsensirion_i2c_driver.Known wart, not addressed: build-tools'
is_packageis sticky across files, so the drivers'tests/anddocs/trees are swept intolib/too (52.5 KB against 32 KB in 2023). Harmless for circup, which installs per-module.🤖 Generated with Claude Code