AI-assisted fork master: build reproducibility and build-step fixes (removes some upstream CI -- see description) - #7
Open
assisted-by-ai wants to merge 2464 commits into
Open
Conversation
adrelanos
pushed a commit
that referenced
this pull request
Sep 8, 2026
- advance submodule gitlinks to upstream's current pointers so Kicksecure sees zero gitlink delta when pulling our master (fixes PR #7 gitlink changes) - absorbs Kicksecure's own arraybolt3/trixie merge and the kicksecure-debug -> kicksecure-ci-tiny-do-not-use flavor rename - keep our newer frozen pin 20260821T022305Z; our SMBIOS-reader design retained
Document the tested behaviour of dm-reproducible-compare-artifacts on real multi-gigabyte iso/ova images: the sha256 verdict is authoritative, but the best-effort diffoscope EXPLANATION can OOM (measured: a 1.4 GB differing .iso -> diffoscope exit 2 'Out of memory'), and the tool degrades gracefully to the sha256 verdict. Note that extending the qcow2 mounted route (attach read-only, diffoscope the mount points) to iso/virtualbox would give them a memory-safe explanation -- a future improvement, not required for a correct verdict.
- libpam-tmpdir (security-misc) points TMPDIR at a path absent in the chroot; debootstrap inheriting it aborts opaquely (Debian #823651) - must clear at the invocation: sudo env_reset + PAM re-set it after the pbuilder configfile is sourced, mirroring the adjacent LD_PRELOAD unset - add R-010 export LC_ALL=C to the strict block (inert here; variables re-sets C.UTF-8 on source) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- comparator now exits 1 (not 4) for "differs"; update the reproducible lanes and docs comment to match - 2100: source reprepro-freshness.bsh (renamed) and gate --skip on the already-published check, passing the repository name - 1300: drop the cowbuilder TMPDIR/TMP env unset - 2100: read the Architecture field once into a case, dropping the quiet grep that consumed a pipe (R-161) and 4x redundant pipeline re-runs - restore/add the LC_ALL=C strict-mode directive in the touched scripts
- approx/curlrc: X-Download-Directly is snapshot.debian.org's direct-serve header (Varnish serves body vs a relative-Location 302 approx can't parse); kept, WHY added - 4400: cluster_size is set by extended_l2=on (128k); an earlier -o would be overridden - cowbuilder-base-cache: drop --preserve-permissions on tar --create (extract-only flag, proven no-op on create) - diagnose-build-kill: OOM greps made consistent; oom_reaper is a redundant follow-up - memory-sampler: document the kB->bare-int normalization - docker-start: header comment now matches actual behavior (no tag verify / git pull) - sign-tag-head: drop dead commented pre-check, subsumed by verify_ref
- 2100: hoist manifest_path out of the reprepro loop (loop-invariant on codename) - cache-make-readable: test -d not -e (cache_dirs are all directories) - locate-boot-image: explicit qcow2 case; error (exit 64) on unrecognized IMAGE_KIND - help-steps/pre: descriptive tolerate-failure|abort-on-failure failure mode instead of the true/false boolean trap in exception_handler_cleanup_run
- 4350: reproducible-by-default is intentional; --freshness frozen|current is the control (reject arraybolt's opt-in proposal) - 3200: --no-reimage note -> REMINDER (grml-debootstrap fork dependency) - 5100: wiki-doc note -> REMINDER
- mount-cleanup, unmount-tree: shorten the '--' arg loop to an errexit-safe if (arraybolt's bare '&& shift' would trip errexit when '--' is absent) - pbuilder-chroot-create-vbox-vm: arm64 VBox VMs ARE supported (Debian_arm64); the x86-pae guard is not obsolete - parse-cmd: pkg_list is the deliberate shared package list (buildconfig default, --only-packages override, 2100 consumer); clarified, not renamed
- 5100: report extension .diffoscope.txt -> .compare-report.txt (contains more than bare diffoscope output) - memory-sampler: read /proc/meminfo once per cycle; mem_field reads stdin - cache-make-readable: drop the superfluous style-ok (no rm present; scanner does not warn -- no scanner change needed) - 25_apt_sources + parse-cmd: remove dead Ubuntu xenial/bionic build support (their .sources files do not exist); host-OS-detection refs kept
- local-build.yml: use all runner cores (REPRO_PACKAGE_JOBS 2->4; ubuntu-24.04 = 4 vCPU) - local-build.yml: drop the .torrent upload -- CI images are not distributed over BitTorrent - 2100: drop the move-into-main() TODO; the --skip-packages guard stays a function-entry check
- shows the 302 relative-Location (approx cannot parse) vs the 200 direct body serve, captured live from snapshot.debian.org
- packages/* must contain NO git-tracked symlinks -- core.symlinks=false materializes them as regular files and silently breaks the package; real symlinks are unwanted there too - repos with legit tracked symlinks (live-build, qubes-template-whonix, the parent's live-build-data) are outside packages/, exempt by scope - fix the repo-detection holes: test the submodule's own .git directly and clear GIT_DIR/GIT_WORK_TREE (arraybolt's stray-GIT_DIR + subdir false-pass) - re-enabled the check (was disabled) - verified: canary drives the real function -- errors on a package symlink, passes without one, and passes on the real tree (packages/* symlink-free)
…sion) 7be37d4 (trixie 'polish') reverted normalize-symlinks to a naive real->text 'find -type l' that is a NO-OP on core.symlinks=false hosts (the Kicksecure default, via security-misc) -- the very hosts the bug occurs on -- and leaves core.symlinks=true (CI) trees DIRTY (type change): the 'unexplained dirty tree' that broke sign-and-tag. It also deleted normalize_file_modes_in_tree, the umask/mode reproducibility fix (90a906c). Restored the text->real materializer + mode normalizer + submodule/non-ASCII handling. Verified in a synthetic repo: a core.symlinks=false clone's text logo becomes a real symlink; both true/false hosts have a CLEAN git status; the two trees are byte-identical (reproducible), submodules included.
The reproducibility bug it detected is now prevented at the root by the restored 1200 normalize-symlinks (materializes real symlinks + normalizes modes on every host), and symlink TARGETS are already whitelist-verified by dm-check-unicode (git ls-files --recurse-submodules, handles real + text via stcat). Nothing to add.
…wline ai-review (codex, agy) on the restored normalizer, both reproduced: - git -C <tree> honours an inherited GIT_DIR/GIT_WORK_TREE (e.g. exported by an enclosing 'git submodule foreach'), so it read the WRONG index and silently normalised nothing (or an unrelated tree). Clear them with env --unset on every git call (parent submodule enumeration + both per-tree ls-files). - command substitution ate a trailing newline, so a mode-120000 target ending in '\n' (or a mistagged file) slipped past the multi-line guard and was mis-converted. Append+strip a sentinel to preserve it.
- called twice per CI job; a plain 'git config' set died on the already-multivalued insteadOf key (exit 5)
- pin is a bare Unix epoch (generator writes date +%s; consumers do date --date=@pin; used as SOURCE_DATE_EPOCH), so reject NON-integers, not integers - clears the pre-existing style gate on the touched file: two grep|pipe -> here-string (R-161), waivers for the chroot /tmp path and internal printf -v name
- it needs only SUDO_TO_ROOT from the build config; sourcing the whole 'variables' dragged in a full build context (root guard, changelog, user_name) and made the umount wrapper impossible to unit-test. Production still defaults to sudo.
- reproducibility needs one canonical form fleet-wide; both work, so security decides -- an inert path-string placeholder cannot point outside the tree or escape a chroot the way a materialised symlink can - flatten real symlinks (parent + submodules) to the target bytes git records (no trailing newline); leave existing placeholders' content; pin placeholder mode to 0644
- a plain-Debian (core.symlinks=true) checkout flattened symlinks to placeholders showed every one as a 'T' typechange; pinning false makes git read mode-120000 as its text blob (clean) and points a plain builder at the Kicksecure security setting
- skipping git_sanity_test verification is a human escape hatch; an AI agent (CLAUDECODE) must sign+tag with a throwaway key (--sign-and-tag) instead - dist_build_forbid_allow_unsigned lets CI / a hardened policy forbid it too; dangerous-options unlock is the human override
- git_sanity_test: refuse the ignore-unsigned skip for an AI session (CLAUDECODE) / dist_build_forbid_allow_unsigned at the CONSUMPTION point (env + buildconfig.d bypass parse-cmd's flag-only check); made source-able so the gate is unit-testable - parse-cmd: :- default on the BUILD_*_PKGS reads (--kernel/--headers/--initramfs crashed under nounset); exit after the --package-jobs integer error
- the source-able port moved errexit into main(), so 'bash git_sanity_test' (how mode_submodules re-invokes it) ran the bootstrap with errexit OFF and could exit 0 on an UNVERIFIED repo; guard each bootstrap source with || exit 1 - --mode/--context/--ref/--ref-type die with a named error on a missing value instead of a bare nounset 'unbound variable'
- git never writes core.symlinks locally, so one --global false overrides the compiled default in the parent and all submodules alike -- replaces the per-tree config loop (flatten + mode-pin still iterate per repo, inherently)
- the two independent reproducibility builds now emit derivative-maker-source-<version>.xz (Phase 4; dist_build_source_release_flavor set to the built flavor); ci/compare-source-tarballs asserts sha256(a)==sha256(b) so source-tarball reproducibility rides the existing build, no extra build
- parse-cmd error_dangerous_option_maybe read dist_build_unlock_dangerous_options BARE under nounset -> 'unbound variable' crash that masked its own actionable message; :- default. (Both CI lanes died at Phase 1 sanity-tests on it.) - local-build.yml + local-boot-test.yml: unlock dangerous options so the kicksecure-ci-tiny-do-not-use flavor (made dangerous by the Aug-25 rename, which never updated the lanes) actually builds
2100 sourced developer-meta-files' reprepro-freshness.bsh, renamed to package-build-freshness.bsh upstream (197f2a11); the gitlink bumped past the rename but this consumer did not -- every build reaching 2100 died.
unmount-tree now requires its target argument (dies 'no parameter given!' without one); mount-cleanup called it bare, so the handoff failed on every invocation -- swallowed where a caller had '|| true', fatal in 2100's get_newer_packages_from_third_party_repositories. Forward the validated file_system_object, as the sibling callers already do.
mount-cleanup refuses a non-root EUID; these two callers invoked it bare
(all 7 others use ${SUDO_TO_ROOT}), so it died 'MUST be run as root'
during 3500 teardown once the build reached it. Also add the missing
'export LC_ALL=C' both files' strict blocks lacked (R-010).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- EFI-signing set (efivar, sbsigntool, keyutils, shim-signed-common, fwupd, fwupd-signed, efibootmgr) is installed by 3500's availability-checked efi_weak_recommended list; a hard ISO entry instead breaks a build on an arch that lacks the package. - base / boot packages (ca-certificates, less, mokutil, dracut, grub-live-dracut, sudo, tirdad, legacy-dist, kicksecure-packages-dependencies-pre, mate-polkit) already come from the flavor metapackage graph or 3500 itself.
- the tool ships no GRUB config: --grub-overlay is required and must carry config.cfg/grub.cfg/loopback.cfg/esp-redirect.cfg; @APPEND_LIVE@ and @timeout@ are substituted in staged cfgs and --serial-console injects the serial terminal. - iso-build-data/grub-config is now the single ISO GRUB config source (loopback.cfg and esp-redirect.cfg moved in from the deleted dm-raw-to-iso.d/); 4310 stages them. - boot-verified in the sandbox: BIOS + EFI + EFI-secureboot reach a login prompt.
- variables: the SUDO_TO_ROOT re-entrance guard flag was exported, so a child process (build steps, recursive derivative-maker/genmkfile) inherited the flag but not SUDO_TO_ROOT, rebuilt the bare default, and skipped the --preserve-env append -- dropping DIST_APTGETOPT_SERIALIZED (and the other passed vars) from sudo. Keep the guard same-shell only: do not export it. - mmdebstrap: define a local exit_with_error; the wrapper cannot source pre, so a failed sanity check aborted with 'error: command not found' instead of the real cause. Also add the missing 'export LC_ALL=C'.
audit + prune landed; the curation rule now lives in the package-list-kicksecure header.
commit 9d61edd removed exception_handler_unmount_lb + the unmount-lb cleanup step; no lb_ identifier remains in any own script.
- the lane is the reproducibility test (build twice + diffoscope), not a generic build; keeps the mandated local- prefix. Updates the hashFiles cache key, dm-ci-dispatch usage, workflow display name, and all doc/comment refs. - add the missing export LC_ALL=C (R-010 7th strict-mode directive) to the four ci/ helpers the rename touches.
- git_sanity_test's verify_ref() was refactored to sq_git_verify, so no name collision remains (and verify-ref only exec's it as a subprocess, never sources it); verify_ref is unique tree-wide. - add the ## style-ok: no-strict waiver: it is a pure sourced library that must not set a top-level strict-mode block (R-010).
the pinned genmkfile now defines make_cow_suffix (make-helper-one.bsh), so the "no support -> serial" fallback is unreachable; parallel --package-jobs used directly under cowbuilder. Workflow comment updated to match.
- mke2fs -O initializes from the rebuild host's /etc/mke2fs.conf then edits with
the list, so a bare -O "${fs_features}" leaked host-default features into the
rebuilt filesystem (non-reproducible across hosts). Prefix 'none,' so the set
is EXACTLY the original image's extracted features. Verified: bare form added
~13 host-default features vs the none, form.
- guard against an empty extracted feature list (none, + empty = featureless fs).
- reword the ext4-features FIXME: the rebuild is now deterministic w.r.t. the
input; a hardcoded per-release list for original-build host-independence is a
separate broader task.
- variables: declare DIST_APTGETOPT=() unconditionally. aptgetopt_add appends to
it only from conditional blocks, so a build hitting none (e.g.
APPROX_PROXY_ENABLE=no on ordinary defaults) left it fully unset and
'${#DIST_APTGETOPT[@]}' tripped 'set -o nounset' -- crashing every build step
that sources variables, before cowbuilder ran.
- mmdebstrap: strip a PATH component named exactly 'help-steps' (trailing slash
trimmed), not the '*help-steps*' substring, which also dropped an unrelated
entry like /opt/legacy-help-steps-scripts.
- Init the array only when unset ('[ -v ] ||') instead of resetting it: a caller
that pre-populates DIST_APTGETOPT in the same shell before sourcing variables
keeps its options (aptgetopt_add appends), while an unset array still gets an
empty declaration so '${#DIST_APTGETOPT[@]}' stays nounset-safe.
- The earlier '${#DIST_APTGETOPT[@]}' nounset-crash concern does not hold:
buildconfig.d/30_apt_opts.conf's aptgetopt_add calls run unconditionally and
populate the array before that use, so a build with no in-file aptgetopt_add
(e.g. APPROX_PROXY_ENABLE=no) does not leave it unset. Restore the original
commented form, which also preserves a caller-pre-populated DIST_APTGETOPT.
- '${#DIST_APTGETOPT[@]}' in the pbuilder APTGETOPT block runs under 'set -o
nounset' and aborts with "unbound variable" on a fully-unset array. The array
is populated only by conditional 'aptgetopt_add' calls, so a build that trips
none of them (relying on buildconfig.d being sourced first is a non-local
invariant) reaches that use undeclared and crashes.
- '[ -v ] || DIST_APTGETOPT=()' declares it locally without clobbering a
caller-pre-populated array.
- af6e404 removed the probe as "dead code" (pinned genmkfile has the feature), but dm-help-steps package_jobs_precondition_test enforces it as a required RUNTIME precondition: a genmkfile lacking make_cow_suffix makes parallel workers share one cow snapshot and corrupt it. Removal broke CI and the safety guard. - keep the probe + serial fallback; reword the FIXME to a REMINDER (do not drop it just because the current pin ships the feature). Restore the workflow comment.
source_tarball_reproducible_test.sh requires the xz tool (the source tarball is .tar.xz); the CI env lacked it, so the test FATALed.
- genmkfile is a pinned submodule and derivative-maker keeps no backwards compatibility, so a genmkfile lacking make_cow_suffix cannot occur; the runtime probe + serial fallback defended an impossible case. Parallel --package-jobs is used directly under cowbuilder. (Supersedes the restore in 476e355.)
keyboard_preseed_model_test.sh validates the model against /usr/share/X11/xkb/rules/*.lst (xkb-data); CI lacked it and the test FATALed.
source_tarball_reproducible_test.sh requires xz + strip-nondeterminism; xz-utils landed earlier, this adds the second required tool.
Loader keeps base-path resolution from its own location; modules inherit it and must not recompute from BASH_SOURCE (they live in variables.d/). Verbatim contiguous split: resolved values byte-identical across all flavor/target combos per dm-varname-snapshot.
A second 'source help-steps/variables' in one shell re-appended accumulators (DIST_APTGETOPT, pkg_list, SKIP_SCRIPTS, ...), doubling them. Loader now returns early on a 'variables_finalized' flag set after all modules. Flag is NOT exported: a fresh process (every build step) must still resolve; on a --dump run the dump hook exits before the flag is set, so snapshots are unaffected.
The helper library (set_default_variable, derive_unified_image_paths) now lives with the other resolver modules; the loader sources 05_lib first so later modules can call the helpers. Still a separate sourced-only file so its unit test can source the helpers directly.
buildconfig.d/--conffile is sourced late (its derivations need resolved inputs), so an unconditional assignment there used to override an env value or a --CLI flag. Capture the environment layer (before parse-cmd) and CLI layer (parse-cmd diff), then re-apply them over exactly the variables buildconfig.d changed; config still beats a code default. Whitelist-free (shell vars never enter the changed-by-config set); scalars only; snapshots cleared before the dump hook so variable snapshots are unaffected. Also fail-close the source-tree cd.
The snapshot-and-reapply layering was disproportionate: buildconfig.d sets almost only derivations, so it barely conflicts with CLI/env. Precedence now falls out of the plain guard idiom (set_default_variable, highest-priority-first): CLI > env > config(set_default_variable) > default. An unconditional config assignment is a documented forced override.
One directory for the whole resolver instead of a variables.d / buildconfig.d split: the loader (help-steps/variables) sources buildconfig.d/*.bsh in explicit order; the *.conf config snippets in the same dir stay sourced as the config layer. Pure relocation -- resolved values and function definitions unchanged (dm-varname-snapshot byte-identical).
User config tiers (/etc/buildconfig-dist.d, ~/buildconfig.d, --confdir, --conffile) now sourced in 10_core (before the code defaults) so a config file overrides a built-in default, while fill-if-empty still yields to env/CLI. Shipped in-tree tier stays late (its snippets derive from resolved inputs). dist_build_source_config_dir moved to 05_lib.
Convert the 213 scalar '[ -n "${x:-}" ] || x=val' fill-if-empty defaults to 'set_default_variable x val' for one consistent idiom. Arrays keep the guard form (helper is scalar-only); [[ -v ]] preserve-empty and plain-= derivations unchanged. dm-varname-snapshot byte-identical across all combos (values + function defs), so behavior is preserved.
- source 05_lib before 00_preamble so preamble defaults can call set_default_variable Co-Authored-By: Claude Opus 4.8 (1M context) <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
2 participants
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.
This is the accumulated work on the org-ai-assisted fork's master, offered
upstream. It is AI-assisted work, reviewed by a human before this PR was
opened. Please read the removals below before the diff.
Read this first: what this PR REMOVES from upstream
Four removals are upstream-affecting and were made for the fork's needs, not
yours. They are called out here so they are not discovered inside a 100-file
diff. I am happy to restore any or all of them on request:
.github/workflows/local-test-build.yml-- thebuild:job is deleted.That is the automated builder (ansible,
ANSIBLE_VAULT_PASSWORD, artifacthandling, 240 minute timeout), gated
if: github.repository == 'Whonix/derivative-maker'. The fork cannot run it(no secret, and the gate never matches), so it was dropped; upstream can.
The rest of the file is retained and rescoped to the dry-run gate.
ci/builddeleted (229 lines).ci/github-builddeleted (207 lines).ci/dry-run.d/400_reproducible-buildinfodeleted (62 lines).The fork's lanes drive
help-steps/dm-build-officialdirectly instead of goingthrough
ci/build, which is why those entry points went away here. If upstreamstill uses them, they should be restored before merging.
What it adds
228 commits, 100 files, +5126 -2619. By area:
help-steps/(35 files) --variables,pre,parse-cmd,unmount-treeand the mount/unmount helpers.
build-steps.d/(19 files) -- notably1100_sanity-tests,1200_prepare-build-machine,2100_create-debian-packages,3500_install-packages,3600_convert-raw-to-iso, and a new4350_reimage-raw-reproducible.docker/(8 files) -- the run/setup scripts, including pinning apt to thebuild's frozen snapshot so a container rebuild does not silently change the
package set.
ci/(15 files) and.github/workflows/(7 files) -- see the caveat below.A large share of the work targets build reproducibility: normalising the EFI
FAT ESP,
update-alternativessymlink mtimes, debconf owner spellings, and ucftemp paths, plus a reproducible re-image step.
Caveat: the fork's CI lanes are included
The
local-*.ymlworkflows are the fork's own lanes. Five of the six are gatedif: vars.CI_ENABLED_ORG_AI_ASSISTED == 'true', so in a repository that neversets that variable they resolve false and consume no runner.
local-test-build.ymlis not gated that way, but it is pre-existing upstream and this PR only rescopes
it (and fixes its tag filter:
'*'does not match a slash, so a slash-delimitedtag silently skipped the gate; it is
'**'now).They are included because this PR is the fork's master as-is. Say the word and I
will strip them.
Submodules
No submodule gitlink is touched. Verified: merging this branch changes zero
mode-160000 entries, and the merge is clean in a clone with no submodules
checked out, which is the case that otherwise conflicts.
Testing status, honestly
locally versus CI on the fork's lanes.
(
11269ee1), with no gitlink changes and no conflicts.upstream's own CI, which has secrets and runners the fork does not.
so it can be reviewed or taken piecemeal.
Happy to split this into smaller PRs by area if that is easier to review.