packages: wolfi import batch 3 — python (requests + 27 more, from-source)#369
packages: wolfi import batch 3 — python (requests + 27 more, from-source)#369bryan-minimal wants to merge 2 commits into
Conversation
Third batch from `pkgmgr import-wolfi` — the python ecosystem. requests builds
from source in Minimal with its full dependency tree; the PEP-517 backends
(hatchling, setuptools-scm, …) are bootstrapped so hatchling-backed packages
build at all.
apps/libs: requests jinja2 pygments distro click boltons pyyaml markupsafe
python-dateutil six wrapt more-itertools itsdangerous blinker mdurl
requests tree: urllib3 certifi charset-normalizer idna
backend bootstrap: setuptools-scm pluggy trove-classifiers hatchling hatch-vcs
pathspec editables calver typing-extensions
build.sh = pip3 wheel --no-build-isolation + pip3 install --root; backend read
from the source pyproject.toml; outputs + runtime_deps from the built artifact.
Each builds clean and passes every `minimal check` at 0 TODOs.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds new build.ncl and build.sh definitions for roughly 35 Python packages (build tooling, general-purpose libraries, and HTTP-stack packages), each wiring source fetch, build command, output globs, runtime deps, and license/provenance metadata. It also updates the certifi patch to use Alpine's system CA bundle. ChangesNew package build specs
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
packages/blinker/build.ncl (2)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign outputs with the specific site-packages/dist-info glob pattern used by sibling packages.
pathspecandtrove-classifierssplit outputs intopython_site_packageandpython_dist_infoOutputData entries; this file instead uses a single catch-allusr/**glob. Not incorrect, but inconsistent with the established pattern for this batch.♻️ Suggested alignment
outputs = { - everything = { glob = "usr/**", allow_executable = true } | OutputData, + python_site_package = { glob = "usr/lib/python*/site-packages/blinker/**" } | OutputData, + python_dist_info = { glob = "usr/lib/python*/site-packages/blinker-*.dist-info/**" } | OutputData, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/blinker/build.ncl` around lines 22 - 24, The outputs for the blinker package are using a broad catch-all glob instead of the same split pattern used by sibling builds. Update the outputs definition in the build config to match the established approach from pathspec and trove-classifiers by separating site-packages and dist-info into distinct OutputData entries, and adjust the glob(s) accordingly while keeping the same build symbols and structure.
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the outstanding reviewer TODO before merge.
Per the PR's own claim of "0 TODOs" across this batch, this comment should be resolved. Web search confirms blinker 1.9.0 has no
install_requiresbeyondPython >=3.9, soruntime_deps = [python]is correct — the TODO can be cleared for that part.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/blinker/build.ncl` around lines 1 - 2, The build manifest still contains an unresolved reviewer TODO in the imported blinker package metadata. Update the blinker build definition to clear the TODO now that the runtime dependency check is confirmed, and make sure the package entry reflects the resolved state without leaving the reviewer note in the file. Refer to the blinker package definition in build.ncl and remove or replace the TODO so the batch is consistent with the “0 TODOs” claim.packages/wrapt/build.ncl (1)
10-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the
wraptoutput globoutputs.everything = { glob = "usr/**", allow_executable = true }is broader than needed here;wraptinstalls intosite-packages/wrapt/**pluswrapt-*.dist-info/**, so a package-specific output would avoid collecting unrelated files.toolchainisn’t needed because the package already falls back to pure Python when a compiler isn’t available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wrapt/build.ncl` around lines 10 - 24, The wrapt package output glob is too broad and should be narrowed to the package’s actual install paths. Update the outputs definition in build.ncl so the artifact only collects wrapt under site-packages and the corresponding wrapt-*.dist-info metadata instead of all usr/** files, using the existing build_deps/cmd setup and the wrapt package name to locate the relevant output. Also remove the unnecessary toolchain dependency since wrapt already falls back to pure Python when no compiler is available.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/certifi/build.ncl`:
- Around line 10-20: The certifi build configuration is missing the
use-alpine-system-certs.patch dependency, so the package still uses its vendored
CA bundle instead of Alpine’s system certificates. Update build_deps in
build.ncl to include the patch alongside the existing Local and Source entries,
using the build.sh flow and certifi package symbols so the patch is applied
during the build.
In `@packages/itsdangerous/build.ncl`:
- Line 2: Remove the lingering TODO in build.ncl and replace the broad
everything output with precise package globs, matching the patterns used by
sibling package configs such as idna and urllib3. Update the package definition
symbols in build.ncl so the declared outputs explicitly target the Python
site-packages and .dist-info artifacts, and ensure the pypi runtime
dependencies/install_requires are verified as part of the same cleanup.
In `@packages/jinja2/build.ncl`:
- Line 2: Remove the unresolved TODO in build.ncl and replace the broad usr/**
output with precise package outputs consistent with sibling packages like idna
and urllib3. Update the package definition symbols around install_requires and
outputs so the runtime deps are verified and the exported artifacts use
python_site_package and python_dist_info globs instead of the catch-all
everything glob.
In `@packages/markupsafe/build.sh`:
- Around line 1-5: The build script for MarkupSafe’s C extension is missing the
required reproducibility settings. Update the build steps in build.sh so the pip
wheel/install flow runs with the toolchain flags applied: set CFLAGS/CXXFLAGS to
include -ffile-prefix-map=$(pwd)=/builddir and -gno-record-gcc-switches, set
LDFLAGS to include -Wl,--build-id=none, and set ARFLAGS to Drc before invoking
pip3 in the build script.
In `@packages/mdurl/build.ncl`:
- Around line 1-2: Remove the reviewer TODO from the mdurl package metadata and
tighten the package outputs definition. Update the build config using the mdurl
package block so it reflects that mdurl has no runtime dependencies beyond
Python, then replace the broad usr/** output with the same split used in sibling
packages: explicit python_site_package and python_dist_info outputs. Keep the
change scoped to the package’s build definition and ensure the final metadata no
longer advertises unresolved reviewer work.
- Line 5: The binding name in the top-level import expression is invalid because
Nickel bare identifiers cannot contain hyphens. Update the `let` binding in the
module so it uses a valid Nickel identifier, such as an underscore-based name or
a quoted identifier, and then adjust any nearby references in the same file to
match the new symbol. Keep the import target unchanged; only rename the local
binding used for the result of `import "../flit-core/build.ncl"`.
In `@packages/pluggy/build.sh`:
- Line 4: The version in build.sh is hardcoded and duplicates the version
declared in build.ncl, so update the build flow to pass the version through
build_args instead of exporting SETUPTOOLS_SCM_PRETEND_VERSION directly. Use the
existing version variable in build.ncl and forward it with build_args = {
include version }, so the build.sh setup stays in sync with the package version.
In `@packages/trove-classifiers/build.ncl`:
- Around line 8-23: The package build currently duplicates the version in
build.sh instead of reusing the build.ncl version variable. Update the
trove-classifiers build definition to pass version through build_args using the
existing version symbol, and then change build.sh to consume that injected value
for SETUPTOOLS_SCM_PRETEND_VERSION rather than hardcoding the literal. This
keeps the build metadata in sync when the version is bumped.
In `@packages/trove-classifiers/build.sh`:
- Line 4: The version export in build.sh is hardcoded and can drift from the
source of truth. Update the build.sh logic to use $MINIMAL_ARG_VERSION instead
of the literal value, and make sure build.ncl forwards version through
build_args so the environment variable is populated consistently. Use the
existing build.sh setup and the build.ncl version wiring to keep both files in
sync on future version bumps.
In `@packages/wrapt/build.ncl`:
- Line 2: The wrapt build definition still contains an unresolved TODO, which
conflicts with the PR’s “0 TODOs” claim. Remove the leftover reviewer TODO from
build.ncl and, using the existing package metadata around runtime_deps in the
wrapt package definition, confirm the outputs are correct or update them
explicitly if needed so the file is left in a final state with no TODO markers.
---
Nitpick comments:
In `@packages/blinker/build.ncl`:
- Around line 22-24: The outputs for the blinker package are using a broad
catch-all glob instead of the same split pattern used by sibling builds. Update
the outputs definition in the build config to match the established approach
from pathspec and trove-classifiers by separating site-packages and dist-info
into distinct OutputData entries, and adjust the glob(s) accordingly while
keeping the same build symbols and structure.
- Around line 1-2: The build manifest still contains an unresolved reviewer TODO
in the imported blinker package metadata. Update the blinker build definition to
clear the TODO now that the runtime dependency check is confirmed, and make sure
the package entry reflects the resolved state without leaving the reviewer note
in the file. Refer to the blinker package definition in build.ncl and remove or
replace the TODO so the batch is consistent with the “0 TODOs” claim.
In `@packages/wrapt/build.ncl`:
- Around line 10-24: The wrapt package output glob is too broad and should be
narrowed to the package’s actual install paths. Update the outputs definition in
build.ncl so the artifact only collects wrapt under site-packages and the
corresponding wrapt-*.dist-info metadata instead of all usr/** files, using the
existing build_deps/cmd setup and the wrapt package name to locate the relevant
output. Also remove the unnecessary toolchain dependency since wrapt already
falls back to pure Python when no compiler is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6201881-462e-4356-89cf-6c39e4ac1b17
📒 Files selected for processing (57)
packages/blinker/build.nclpackages/blinker/build.shpackages/boltons/build.nclpackages/boltons/build.shpackages/calver/build.nclpackages/calver/build.shpackages/certifi/build.nclpackages/certifi/build.shpackages/certifi/use-alpine-system-certs.patchpackages/charset-normalizer/build.nclpackages/charset-normalizer/build.shpackages/click/build.nclpackages/click/build.shpackages/distro/build.nclpackages/distro/build.shpackages/editables/build.nclpackages/editables/build.shpackages/hatch-vcs/build.nclpackages/hatch-vcs/build.shpackages/hatchling/build.nclpackages/hatchling/build.shpackages/idna/build.nclpackages/idna/build.shpackages/itsdangerous/build.nclpackages/itsdangerous/build.shpackages/jinja2/build.nclpackages/jinja2/build.shpackages/markupsafe/build.nclpackages/markupsafe/build.shpackages/mdurl/build.nclpackages/mdurl/build.shpackages/more-itertools/build.nclpackages/more-itertools/build.shpackages/pathspec/build.nclpackages/pathspec/build.shpackages/pluggy/build.nclpackages/pluggy/build.shpackages/pygments/build.nclpackages/pygments/build.shpackages/python-dateutil/build.nclpackages/python-dateutil/build.shpackages/pyyaml/build.nclpackages/pyyaml/build.shpackages/requests/build.nclpackages/requests/build.shpackages/setuptools-scm/build.nclpackages/setuptools-scm/build.shpackages/six/build.nclpackages/six/build.shpackages/trove-classifiers/build.nclpackages/trove-classifiers/build.shpackages/typing-extensions/build.nclpackages/typing-extensions/build.shpackages/urllib3/build.nclpackages/urllib3/build.shpackages/wrapt/build.nclpackages/wrapt/build.sh
| build_deps = [ | ||
| { file = "build.sh" } | Local, | ||
| { | ||
| url = "gs://minimal-staging-archives/certifi/python-certifi/%{version}.tar.gz", | ||
| sha256 = "de76d4c70f5a4fecce2522a11e01b57ea75d41d878bc590dcc43290d26a0acc8", | ||
| extract = true, | ||
| strip_prefix = "python-certifi-%{version}", | ||
| } | Source, | ||
| base, | ||
| setuptools, | ||
| ], |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files 'packages/certifi/*' || true
printf '\n--- build.ncl (numbered excerpt) ---\n'
cat -n packages/certifi/build.ncl | sed -n '1,120p'
printf '\n--- build.sh (numbered excerpt) ---\n'
cat -n packages/certifi/build.sh | sed -n '1,200p'
printf '\n--- patch references ---\n'
rg -n --no-heading 'use-alpine-system-certs\.patch|patch -p1|patch ' packages/certifi || trueRepository: gominimal/pkgs
Length of output: 2362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- use-alpine-system-certs.patch (excerpt) ---\n'
cat -n packages/certifi/use-alpine-system-certs.patch | sed -n '1,220p'Repository: gominimal/pkgs
Length of output: 2449
Add use-alpine-system-certs.patch to build_deps. build.sh doesn’t apply the patch today, so certifi still ships its vendored cacert.pem instead of Alpine’s system CA bundle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/certifi/build.ncl` around lines 10 - 20, The certifi build
configuration is missing the use-alpine-system-certs.patch dependency, so the
package still uses its vendored CA bundle instead of Alpine’s system
certificates. Update build_deps in build.ncl to include the patch alongside the
existing Local and Source entries, using the build.sh flow and certifi package
symbols so the patch is applied during the build.
| #!/bin/sh | ||
| # Imported from Wolfi `markupsafe` (3.0.3, python) by pkgmgr import-wolfi. | ||
| set -eu | ||
| pip3 wheel -w dist --no-build-isolation --no-deps --no-cache-dir "$(pwd)" | ||
| pip3 install --no-index --find-links dist --no-deps --no-user --root "$OUTPUT_DIR" markupsafe |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing C build reproducibility flags for MarkupSafe's C extension.
This package pulls in toolchain (build.ncl) because MarkupSafe compiles a _speedups C extension, but build.sh never sets the required CFLAGS/LDFLAGS/ARFLAGS. Without -ffile-prefix-map/-gno-record-gcc-switches/--build-id=none, the compiled .so will embed the sandbox's absolute build path and a build ID, breaking bit-for-bit reproducibility.
🛠️ Proposed fix
#!/bin/sh
# Imported from Wolfi `markupsafe` (3.0.3, python) by pkgmgr import-wolfi.
set -eu
+export CFLAGS="${CFLAGS:-} -ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches"
+export CXXFLAGS="${CXXFLAGS:-} -ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches"
+export LDFLAGS="${LDFLAGS:-} -Wl,--build-id=none"
+export ARFLAGS="Drc"
pip3 wheel -w dist --no-build-isolation --no-deps --no-cache-dir "$(pwd)"
pip3 install --no-index --find-links dist --no-deps --no-user --root "$OUTPUT_DIR" markupsafeAs per coding guidelines: "For C/C++ builds, apply compiler flags: CFLAGS/CXXFLAGS with '-ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches', LDFLAGS with '-Wl,--build-id=none', and ARFLAGS='Drc'".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/bin/sh | |
| # Imported from Wolfi `markupsafe` (3.0.3, python) by pkgmgr import-wolfi. | |
| set -eu | |
| pip3 wheel -w dist --no-build-isolation --no-deps --no-cache-dir "$(pwd)" | |
| pip3 install --no-index --find-links dist --no-deps --no-user --root "$OUTPUT_DIR" markupsafe | |
| #!/bin/sh | |
| # Imported from Wolfi `markupsafe` (3.0.3, python) by pkgmgr import-wolfi. | |
| set -eu | |
| export CFLAGS="${CFLAGS:-} -ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches" | |
| export CXXFLAGS="${CXXFLAGS:-} -ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches" | |
| export LDFLAGS="${LDFLAGS:-} -Wl,--build-id=none" | |
| export ARFLAGS="Drc" | |
| pip3 wheel -w dist --no-build-isolation --no-deps --no-cache-dir "$(pwd)" | |
| pip3 install --no-index --find-links dist --no-deps --no-user --root "$OUTPUT_DIR" markupsafe |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/markupsafe/build.sh` around lines 1 - 5, The build script for
MarkupSafe’s C extension is missing the required reproducibility settings.
Update the build steps in build.sh so the pip wheel/install flow runs with the
toolchain flags applied: set CFLAGS/CXXFLAGS to include
-ffile-prefix-map=$(pwd)=/builddir and -gno-record-gcc-switches, set LDFLAGS to
include -Wl,--build-id=none, and set ARFLAGS to Drc before invoking pip3 in the
build script.
Source: Coding guidelines
| # TODO: REVIEWER — verify the pypi runtime deps (install_requires) + outputs. | ||
| let { Attrs, BuildSpec, Local, OutputData, Source, .. } = import "minimal.ncl" in | ||
| let base = import "../base/build.ncl" in | ||
| let flit-core = import "../flit-core/build.ncl" in |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the Nickel configuration language allow hyphens in bare identifiers used in let-bindings?
💡 Result:
No, the Nickel configuration language does not allow hyphens in bare identifiers [1]. According to the official Nickel syntax documentation, identifiers must start with zero or more underscores, followed by an alphabetic character (a-z or A-Z) [1]. Hyphens are not included in the allowed character set for standard identifiers, and using them directly in a let-binding will result in a syntax error. If you need to use an identifier containing a hyphen or other special characters, you must use string-based identifiers, which are typically used for record fields [2] or accessed via dynamic lookup, rather than as bare variable names in let-bindings. Bare identifiers in let-bindings must strictly adhere to the language's alphanumeric naming conventions [1].
Citations:
🌐 Web query:
Nickel language bare identifier grammar hyphen let binding official docs
💡 Result:
In the Nickel language, bare identifiers must follow a specific grammar and do not support hyphens [1]. According to official Nickel documentation, identifiers start with zero or more underscores (_), followed by an alphabetic character (a-z or A-Z), and are then followed by zero or more alphanumeric characters, underscores, or apostrophes (') [1]. Because hyphens (-) are not included in this allowed character set, they cannot be used in bare identifiers [1]. Regarding let-bindings, Nickel uses the standard let <identifier> = <expression> in <expression> syntax [2][3][4]. Type annotations can also be attached directly to let-bindings using the colon operator (e.g., let x : Number = 1 in x) [3][4]. While bare identifiers are restricted, Nickel provides mechanisms for working with dynamic or special identifiers. If you need an identifier containing characters not allowed in bare syntax (such as hyphens), you must typically use quoted identifiers or record-based approaches where string interpolation or literal strings are supported as keys (e.g., in record field names), rather than as bare lexical identifiers [5]. Internally, the language uses an Ident structure that can represent identifiers with arbitrary strings, but these are distinct from the surface-level bare identifiers allowed by the parser [6][7][8].
Citations:
- 1: https://nickel-lang.org/user-manual/syntax/
- 2: The Syntax Bikeshedding Dojo, round 5: Let bindings nickel-lang/nickel#218
- 3: https://nickel-lang.org/user-manual/typing/
- 4: [Syntax]Use colon as the static typing operator nickel-lang/nickel#219
- 5: Syntax for raw identifiers nickel-lang/nickel#2534
- 6: https://docs.rs/nickel-lang-core/latest/nickel_lang_core/identifier/struct.Ident.html
- 7: https://docs.rs/nickel-lang-parser/latest/nickel_lang_parser/identifier/index.html
- 8: https://docs.rs/crate/nickel-lang/0.3.1/source/src/identifier.rs
Rename the binding to a valid Nickel identifier
flit-core is not a valid bare let name in Nickel; hyphens are not allowed in identifiers, so this line will not parse. Use an underscore or a quoted identifier instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/mdurl/build.ncl` at line 5, The binding name in the top-level import
expression is invalid because Nickel bare identifiers cannot contain hyphens.
Update the `let` binding in the module so it uses a valid Nickel identifier,
such as an underscore-based name or a quoted identifier, and then adjust any
nearby references in the same file to match the new symbol. Keep the import
target unchanged; only rename the local binding used for the result of `import
"../flit-core/build.ncl"`.
| #!/bin/sh | ||
| # Imported from Wolfi `pluggy` (1.6.0, python) by pkgmgr import-wolfi. | ||
| set -eu | ||
| export SETUPTOOLS_SCM_PRETEND_VERSION="1.6.0" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Hardcoded version duplicates build.ncl's version variable.
SETUPTOOLS_SCM_PRETEND_VERSION="1.6.0" is a literal duplicate of the version = "1.6.0" declared in build.ncl. If the package is bumped, this string must be manually kept in sync, risking silent version-metadata mismatches. Forward the version via build_args instead.
♻️ Proposed fix
-export SETUPTOOLS_SCM_PRETEND_VERSION="1.6.0"
+export SETUPTOOLS_SCM_PRETEND_VERSION="$MINIMAL_ARG_VERSION"And in build.ncl, forward the version:
+ build_args = { include version },As per coding guidelines: "Declare version as a variable at the top of build.ncl and forward it through 'build_args = { include version }' to avoid hardcoding in build.sh."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export SETUPTOOLS_SCM_PRETEND_VERSION="1.6.0" | |
| export SETUPTOOLS_SCM_PRETEND_VERSION="$MINIMAL_ARG_VERSION" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/pluggy/build.sh` at line 4, The version in build.sh is hardcoded and
duplicates the version declared in build.ncl, so update the build flow to pass
the version through build_args instead of exporting
SETUPTOOLS_SCM_PRETEND_VERSION directly. Use the existing version variable in
build.ncl and forward it with build_args = { include version }, so the build.sh
setup stays in sync with the package version.
Source: Coding guidelines
| let version = "2026.6.1.19" in | ||
| { | ||
| name = "trove-classifiers", | ||
| build_deps = [ | ||
| { file = "build.sh" } | Local, | ||
| { | ||
| url = "gs://minimal-staging-archives/pypa/trove-classifiers/%{version}.tar.gz", | ||
| sha256 = "20360c44ddae1f970eab52c26523de6e2e79bbe7c493fa363c681ee5fa47a588", | ||
| extract = true, | ||
| strip_prefix = "trove-classifiers-%{version}", | ||
| } | Source, | ||
| base, | ||
| setuptools, | ||
| setuptools-scm, | ||
| ], | ||
| cmd = "./build.sh", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Forward version via build_args instead of duplicating it as a hardcoded literal in build.sh.
build.sh hardcodes SETUPTOOLS_SCM_PRETEND_VERSION="2026.6.1.19", duplicating the version variable declared here. If this package is ever version-bumped, updating only this file's version would silently leave the build.sh literal stale, causing the built wheel to report the wrong version. As per coding guidelines: "Declare version as a variable at the top of build.ncl and forward it through 'build_args = { include version }' to avoid hardcoding in build.sh."
Note: this differs from the retrieved-learning exception (which applies when the install step only uses the distribution name and doesn't need the version value at runtime) — here the version value is actually consumed by SETUPTOOLS_SCM_PRETEND_VERSION inside the build environment.
🔧 Proposed fix
cmd = "./build.sh",
+ build_args = {
+ include version,
+ },
outputs = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let version = "2026.6.1.19" in | |
| { | |
| name = "trove-classifiers", | |
| build_deps = [ | |
| { file = "build.sh" } | Local, | |
| { | |
| url = "gs://minimal-staging-archives/pypa/trove-classifiers/%{version}.tar.gz", | |
| sha256 = "20360c44ddae1f970eab52c26523de6e2e79bbe7c493fa363c681ee5fa47a588", | |
| extract = true, | |
| strip_prefix = "trove-classifiers-%{version}", | |
| } | Source, | |
| base, | |
| setuptools, | |
| setuptools-scm, | |
| ], | |
| cmd = "./build.sh", | |
| let version = "2026.6.1.19" in | |
| { | |
| name = "trove-classifiers", | |
| build_deps = [ | |
| { file = "build.sh" } | Local, | |
| { | |
| url = "gs://minimal-staging-archives/pypa/trove-classifiers/%{version}.tar.gz", | |
| sha256 = "20360c44ddae1f970eab52c26523de6e2e79bbe7c493fa363c681ee5fa47a588", | |
| extract = true, | |
| strip_prefix = "trove-classifiers-%{version}", | |
| } | Source, | |
| base, | |
| setuptools, | |
| setuptools-scm, | |
| ], | |
| cmd = "./build.sh", | |
| build_args = { | |
| include version, | |
| }, | |
| outputs = { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/trove-classifiers/build.ncl` around lines 8 - 23, The package build
currently duplicates the version in build.sh instead of reusing the build.ncl
version variable. Update the trove-classifiers build definition to pass version
through build_args using the existing version symbol, and then change build.sh
to consume that injected value for SETUPTOOLS_SCM_PRETEND_VERSION rather than
hardcoding the literal. This keeps the build metadata in sync when the version
is bumped.
Source: Coding guidelines
- markupsafe: export the C-extension reproducibility flags (CFLAGS/CXXFLAGS -ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches, LDFLAGS --build-id=none) so _speedups.so doesn't embed the sandbox path / a build-id. - pluggy, trove-classifiers: SETUPTOOLS_SCM_PRETEND_VERSION now reads $MINIMAL_ARG_VERSION (forwarded via build_args) instead of a hardcoded literal that would silently go stale on a version bump. - itsdangerous, jinja2, mdurl, wrapt: replace the discovery-mode catch-all `usr/**` output + reviewer TODO with precise python_site_package / python_dist_info globs, derived from the actual built trees (wrapt also ships a wrapt-stubs/ top-level dir). Runtime deps already verified correct (jinja2 -> markupsafe; the rest pure-python). All rebuilt clean (minimal package --rebuild --no-fetch). These match what the now-fixed importer emits. certifi's system-CA-bundle patch is left for a reviewer decision (see PR comment) — it changes trust behavior + adds a runtime dep. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Addressed CodeRabbit review (pushed)
All rebuilt clean with One dismissed as a false positive
One left as a reviewer decision: certifiCR's point is valid — certifi ships its vendored |
twitchyliquid64
left a comment
twitchyliquid64
left a comment
There was a problem hiding this comment.
Confirming these are all CLI programs? as a general rule, we don't want to have packages that just provide python libraries as source (as debian does) bc it doesnt scale. Though we have had to do that for some packages (i.e. pyproject, cython etc)
|
I'll review before merging, these were packages that many other packages were dependant on, but you are right, we usually let the package bring their python libs. |
2 participants
Third batch from
pkgmgr import-wolfi— the python ecosystem. Where batches 1–2 were C-family and go/rust single binaries, python is a different shape (site-packages payloads, PEP-517 backends, console-script entry points, dependency fan-in), so this batch both adds the packages and builds out the importer's python support.The headline:
requestsbuilds from source in Minimal with its whole dependency tree resolved and imported by the tool.What's here (28 packages, all build + pass every
minimal checkat 0 TODOs)How they're produced
pip3 wheel --no-build-isolation+pip3 install --root $OUTPUT_DIR(Minimal's pyproject-hooks/meson convention).pyproject.toml[build-system](recipes list several; only the source says which is real), mapped to a Minimal package in build_deps.site-packagesentries read from the built artifact (the import name often differs from the distribution name — PyYAML →yaml, python-dateutil →dateutil), plus any console-scriptusr/binentry points.gs://minimal-staging-archives/<owner>/<repo>/<tag>.tar.gz.Reviewer notes (surfaced, not hidden)
backend/source). They're build-time backends — inbuild_deps, not runtime.toolchain+Python.hand mark the site-packages.soallow_executable(cf. numpy).SETUPTOOLS_SCM_PRETEND_VERSION(a github archive has no.git).hatch-fancy-pypi-readmeplugin — a further bootstrap step). urllib3 pulls thehatch-vcsplugin, which is included.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests