Conversation
test: was empty, so ci.yaml tested nothing. Wire it to the shared unit and race recipes, add bench, and report whether mkfs.erofs is reachable: the image-backed tests skip themselves without it, and under the hermetic PATH they always do — a green run that covered 37 fewer tests should say so rather than look identical to a full one. Drop the inherited lint job: it ran an action-supplied golangci-lint v2.1 while aqua.yaml pins v2.12.2. Rename the rest to erofs-utils-integration, which is what it is — the coverage needing an Drop go-test-platforms.yml (ci.yaml's matrix covers more, and now runs the 386 leg too) and codeql.yml: the baseline SAST posture is golangci's gosec/staticcheck plus govulncheck per GOOS, and limen wants CodeQL as default setup opted into via limen.yaml, not a workflow pinned by tag. Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Rebase the Windows patch stack onto the new tree. The v1.9.2 relicense (Apache-2.0 -> MIT) rewrote every SPDX header the patches used as context, so the stack is regenerated against v1.9.3; hunks are written context-free (full remove/re-add blocks) so the patch files themselves stay whitespace-clean. The code the patches inject is unchanged. Drop 004-windows-gzran-zlib-guard: v1.9.3 guards the zlib include in lib/gzran.c itself, so the patch is upstreamed. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MSVCRT never fills st_ino, and mkfs deduplicates hardlinks by (st_dev, st_ino) with no nlink gate — so on Windows every entry collided with the root directory's inode 0 and images built from local trees came out self-referential (/a.txt aliased the root nid; TestReadReferenceImage walked a.txt/a.txt/... until the path bound). Replace the CRT stat family in posix_compat.h with Win32-native implementations: _ino_t is widened to 64 bits before any CRT header runs, st_ino/st_dev come from the NTFS file index and volume serial (the identity Windows hardlinks actually share, so dedup is now correct rather than disabled), lstat detects symlink reparse points, and readlink extracts the target from the reparse data — the old stub just warned and failed. The stat/fstat/lstat macros never call the CRT versions, whose struct layout still has the 16-bit field. Add -D_GNU_SOURCE to the cross-compile make lines: v1.9.3's rebuild.c uses asprintf, which llvm-mingw does not expose without it (gcc-mingw does regardless). Verified on a Windows 11 ARM64 VM against an aarch64 llvm-mingw build of erofs-utils v1.9.3: the full go test suite passes, including both TestReadReferenceImage legs with their symlink assertions — coverage Windows never had. x86_64-w64-mingw32 compile-checked. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
v1.9.3's configure auto-probes libxml2 for the new OCI/S3 remotes and adds it to LIBS globally. PKG_CONFIG_PATH only prepends to the host's pkg-config search path, so the ubuntu runner's libxml-2.0.pc leaked into the cross build and mkfs failed to link with -lxml2. Use PKG_CONFIG_LIBDIR instead, which replaces the search path: only the mingw sysroot (where the cross-compiled lz4 installs its .pc) is visible, so every host-library auto-probe now fails closed — this fixes the whole class, not just libxml2. Pass --without-libxml2 explicitly as well, since the oracle has no use for image remotes. Signed-off-by: apostasie <spam_blackhole@farcloser.world>
…oval Removal existed only as the whiteout machinery behind CopyFrom(Merge): a source tree had to spell deletions as AUFS .wh. files. Expose it as Writer.Remove (one name; ErrDirNotEmpty for a populated directory, fs.ErrNotExist when missing, fs.ErrInvalid for "/") and RemoveAll (recursive; missing is not an error, "/" is refused, traversing a non-directory is ErrNotDirectory). Both refuse the file currently open from Create. Removal now has unlink(2) semantics for hard links. Removing an alias decrements the target's link count as before; removing the target while aliases survive used to leave the inode with no owner and fail Close with "target no longer exists" — a real gap for overlay merging, where a layer can legitimately white out one name of a multiply-linked file. unlinkEntry now promotes the lowest-pathed surviving alias to carry the inode and repoints the rest at it, so nothing downstream sees a dangling linkTo, and the choice is deterministic so identical inputs still yield identical images. Remove deliberately does not go through lookup(): that resolves a link name to the shared inode, right for metadata and wrong for unlink, where the name itself is what goes away. Tests: the removal matrix from erofs/go-erofs (file, empty and populated dirs, missing, root, symlink, alias, canonical promotion, all-aliases, RemoveAll recursive/missing/non-dir ancestor/root/file/ hardlink inside), plus fork-specific coverage of a Merge whiteout hitting a hardlink target, order-independence of alias promotion, and open-file protection. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous stat family redefined _ino_t as 64-bit and named `struct stat` in its prototypes. Both are wrong on the toolchain CI actually uses (Ubuntu's mingw-w64 v11, msvcrt): there the CRT itself rewrites `stat`/`fstat` to `_stat64`/`_fstat64` under _FILE_OFFSET_BITS=64, so the struct behind the name depends on which header ran first in a given translation unit, and st_ino has more than one definition site (sys/types.h and wchar.h) that a single guard does not cover. mkfs ended up with several struct stat layouts in one binary: st_size written at one offset and read at another came back as zero, which is the "dir/b.txt = \"\" / empty symlink target" CI saw. The v12+ UCRT toolchains I had verified on agree with themselves, which is why the ARM64 VM run passed and CI did not. Rework the shims so no CRT type is ever redefined and no prototype names struct stat: a Win32 core fills a POD of our own (__pc_finfo), and stat/lstat are statement-expression macros that copy it into the caller's struct at the call site — whatever layout that TU has, the fields land at its offsets by construction. fstat is an object-like macro to a void*-taking function (erofs_vfops has a member named fstat, so a function-like macro cannot be used). The CRT's own redirect is left in force and its target names taken over, so `struct stat` keeps meaning the same thing everywhere. The 64-bit NTFS file index — the identity hard links share, which the CRT's 16-bit st_ino cannot hold — now travels beside the struct: __pc_lstat_id returns it, and patch 001 makes erofs_iget_from_local key the hardlink hash on it (widening erofs_iget's ino parameter on Windows). Hardlink dedup is therefore correct, not merely not-broken. Pin MAX_BLOCK_SIZE=16384 in every configure call. configure caps the block size at the build host's page size, bumped to 16K only when the build CPU is aarch64 — so the same source produced a 4K-capped mkfs on x86_64 runners and a 16K one elsewhere, and the 16384 leg of TestReadReferenceImage silently skipped on CI. Verified against a byte-equivalent CI build (ubuntu:24.04 container, apt mingw-w64 → gcc 13 / mingw-w64 v11 / msvcrt) run on a Windows 11 ARM64 VM under x64 emulation: full go test suite passes, both TestReadReferenceImage legs included. Re-verified the aarch64 llvm-mingw (UCRT) build on the same VM: still passes, so both header generations are covered. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FuzzWideOpen went red with "context deadline exceeded" at 10.10s — exactly fuzztime — after a clean corpus pass and ten seconds of fuzzing with no failing input written. That is the coordinator cancelling a worker caught mid-iteration as the budget expires, not a finding: a real fuzz failure always leaves the input under testdata/fuzz/<Target>/. Wide targets are the most exposed since each iteration is a ReadDir over 200 entries. Make the step decide from the crasher: a non-zero exit with a new file under testdata/fuzz/<Target> is a failure and is reported as such; a non-zero exit with no new file is retried once, and only a second hiccup in a row fails the job. Real bugs still fail — and now with an explicit ::error naming the crasher, instead of being lost among timing noise. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Fuzz job prebuilt one test binary with `go test -c` and drove each
target through -test.fuzz on it. Only the go tool's own fuzz build
compiles in the coverage counters, so that binary carried none — the
engine said so on every run ("not built with coverage instrumentation
... may be inefficient") — and what ran was blind mutation of the
seeds: no feedback about which inputs reach new code, which is the
entire mechanism by which a fuzzer works its way toward the deep
parser corners the audit work has been hardening. Forty-five targets
times ten seconds of that was barely more than a second seed-corpus
pass, presented as continuous fuzzing.
Drive each target with `go test -fuzz='^T$' -run='^$'` instead. The
per-target rebuild the old comment wanted to avoid is a cached second
or two after the first target warms it.
Cache GOCACHE/fuzz across runs. Fuzzing is only cumulative if the
generated corpus survives: each run then starts from every input
earlier runs found interesting rather than from the seeds, so ten
seconds per run compounds into depth over months. Key on the fuzz
sources so a changed target restarts its own corpus; restore-keys keep
the rest.
Upload testdata/fuzz on failure. The log names the target; the input
file is what reproduces the bug locally.
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Co-Authored-By: Claude Fable 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.
No description provided.