Repository navigation
fix(import-audit): preserve JSON reports when writing fails - #4425
Conversation
|
One filesystem behavior question: when the destination does not exist, |
|
The Windows relinker check has one reproducible failure: |
f6e0356 to
05d52e3
Compare
|
@MarcZX100 Thx for catching both issues! Fixed the diagnostic path and the permissions for new reports, with regression tests added. The relinker CI now passes on Windows, Linux and macOS :) |
|
I checked the overlap using #4425 at
With those resolutions, the combined Windows checkout ran 99 focused Python tests: 96 passed and 3 platform-specific cases were skipped. This was a local integration check on the pinned base above, not a full suite or Linux/macOS run. No dependency commits have been added to #4425. @Nixitov has offered to rebase #4606, and #4545's description also offers a rebase if #4425 lands first. I propose #4425 first, followed by rebases preserving the changes above. Would that order work for everyone? If a different order is preferred, I can rebase this PR instead. AI-assisted: yes (Codex). |
|
Thanks @Lorkydey — that order works for me. #4425 first, then I'll rebase #4606 after it lands: keep the AI-assisted: yes |
|
Confirm their proposed merge order works, and explain that your #4415 was closed, so they can proceed without waiting for it. |
|
#4545 was merged first, so the overlap is now only the AI-assisted: yes (Claude Code) |
05d52e3 to
d45fee9
Compare
What
import_audit.py --json audit.jsontruncates the destination before writing the new report. A write error leaves an incomplete JSON file and destroys the previous audit. For a new destination it leaves a partial file that looks like an available report.Write the complete report inside a temporary directory beside the resolved destination, then atomically replace the destination. New reports retain the normal 0666 permissions filtered by the process umask, without changing that process-wide setting. Preserve existing report permissions and symlink targets. Keep the caller-supplied output path in the directory-error diagnostic, including Windows short paths. Failed writes or replacements keep the old report, return the existing failure exit code and remove staging files, including Windows read-only temporaries. Input-overwrite protection and import classification stay unchanged.
Eight new test methods exercise the CLI entry point and real temporary files. POSIX permission cases run with isolated subprocess umasks. The disk-full injection writes an actual partial prefix before raising ENOSPC; with main
dacd78525, an existing report is truncated and a new report is left behind. A replacement denial is also covered. This is controlled fault injection, not a claim of exhausting the host disk.Tested
Rebased onto main
dacd78525e537a779682032c0ace7f7af120af05, including #4545 and #4606, at exact headd45fee9b69c8f59e11818d3e25fbdfc0b6e296fd:python -m unittest discover -s tools/tests -p 'test_*.py' -qruns 287 tests, 285 pass and the two POSIX-only permission tests skip.import_auditCTest also passes.stops, module imports, atomic replacement, partial writes, symlinks, permissions and input-overwrite protection. Windows skips the two POSIX-only permission tests; Linux/macOS skip the Windows-only read-only test.import_audit.py: the old report is truncated, or a partial new report is left behind. Restoring the exact PR source passes. This is controlled fault injection, not actual disk exhaustion.git diff --checkpass. No native runtime change or title run.Overlap coordination
#4545 and #4606 are now merged. This rebase preserves main's
stopsconstruction and JSON field, then passes the complete result towrite_json(args.json, result). It also retains the merged hard-link test changes, with oneosimport and thestatimport required for permission tests. Their module-import,stopsand hard-link regressions pass alongside the atomic-write tests. #4415 is closed without merging. No outstanding dependency on those PRs remains.Checklist
main; no other open PR implements the same functions