keep-aux.md

Summary: keep-auxfiles feature work

A chronological summary of the working session on the keep-auxfiles / aux-redirect branches of apt.

1. The task and exploration

• User: methods/mirror.cc should transparently ignore failures from fetching the mirror file if an old copy still exists — the mirror file is kept around in auxfiles/, which makes the mirror method more reliable.
• Agent: explored the code paths end to end: methods/mirror.cc (351 Aux Request flow, MirrorListFileReceived, mirrorfilestate), apt-pkg/acquire-worker.cc (Aux Request handling, ReplyAux, which hides the file behind /nonexistent/... when it exists but the item did not finish with StatDone), apt-pkg/acquire-item.cc (pkgAcqAuxFile::Failed renaming DestFile to .FAILED), apt-pkg/clean.cc (auxfiles never cleaned), and test/integration/test-method-mirror. Checked git history and upstream — no existing implementation.

2. Design corrections from the user

• User: "we should not be renaming to .FAILED in this case in the first place" — the old copy must survive the failure.
• User: "A partial file may be renamed to .FAILED; we should be using the old file in that case."
• Agent: after analyzing the http method (file only opened after successful headers, 416-partial-hit resume logic in basehttp.cc), designed the distinction: record the old copy's mtime in the pkgAcqAuxFile ctor; in Failed(), rename to .FAILED only if the file was actually modified (a real partial), otherwise keep it and mark the item StatDone so the failure is ignored. mirror.cc falls back to the auxfiles copy.

3. Reporting as Ign, TDD

• User: "It should not report the failure as a failure, just as 'Ign:' (ignored failure)".
• User: "Write some test cases first before thinking further", "use testsuccessequal".
• Agent: ran the baseline mirror test — initially failed on SSL errors.
• User: "Sorry I had a wrong openssl version installed with a broken patch, try again" — baseline then passed (783 checks).
• Agent: added the 404 fallback test; first run showed testrun's cleanup deletes auxfiles/, so a priming apt update was needed to populate the old copy. The fallback then worked (Hit: lines) but the mirrorlist failure still printed Err:1 and exited 100.

4. Commit 1: keep old copies + fallback

• User: finish the test to match the actual output for now, commit, and fix output in another commit ("the test should not ignore the output, you should ignore the output not matching what we want").
• Agent: switched to testfailureequal with the real output (810 checks pass) and committed "Keep old mirror file copies around and use them as fallback".

5. Ign reporting + no retries

• User: "hint_ also it retries 3 times of course due to apts retry logic".
• Agent: implemented the StatDone downgrade in pkgAcqAuxFile::Failed when the old copy is kept: progress prints Ign:, AcquireUpdate no longer fails. Test updated to testsuccessequal (811 checks pass).
• User: "I amended the previous commit with your changes. Now focus on the 3 retries with a transient failure. We do not actually want 3 retries if we have a previous copy, just use it directly", and "add the test case first and make sure it fails before fixing the code :)".
• Agent: wrote the transient test (redirect to a dead port) — the run hung.
• User: "seems to hang", "I killed it", then provided a coredump: std::out_of_range from basic_string::substr in ChangeSiteIsMirrorChange (acquire-worker.cc:240) — a real pre-existing crash when an aux file request is redirected (aux item Description is <uri> Mirrorlist, so OldSite == desc.URI and substr(len+1) throws).
• Agent: switched the transient test to a second webserver on 127.0.0.2 that gets killed; discovered bad_addr in methods/connect.cc is keyed by IP only (not IP:port), so killing a server on localhost poisoned the live server too — hence 127.0.0.2. The test then failed correctly: 3 extra retry Ign: lines. Implemented Retries = 0 in the pkgAcqAuxFile ctor when an old copy exists (841 checks pass); committed "Do not retry fetching the mirror file if an old copy exists". Regression-checked related integration tests and the unit suite (initial "failures" were just running libapt_test from the wrong CWD; fixtures require test/libapt).

6. Simplifying the transient test

• User: rebase pause, then "why is this not crashing anymore?" (answer: the crash path was simply no longer exercised — the redirect scenario had been replaced), then "can we rewrite the test case to avoid killing and manually starting aptwebserver?"
• Agent: rewrote the test to point sources at a dead address and seed the old copy manually; failed on Get: vs Hit: because changing the source URI changes all index file names.
• User: "Maybe we just change the port not the hostname? And mv the file that worked fine before to update the port to a fake one?"
• Agent: explained the bad_addr IP-keying still forces a different IP, but adopted the mv idea: mv the previously fetched auxfile and index files to the fake-name variants — keeps Hit: without a priming update (826 checks pass).
• User: "Then use the same port if you use a different IP".
• Agent: tried it — the main webserver binds 0.0.0.0, so 127.0.0.2 on the same port is alive and the fetch succeeds via the resume partial-hit path. Reverted to the truly dead port 127.0.0.2:1 (826 checks pass).
• User: "simplif the sed to use just localhost -> 127.0.0.2".
• Agent: attempted, but the underscore boundary in the filename sed is required with differing ports; corrected and re-verified (826 checks pass).
• User: "We could also perhaps have the webserver respond 429 or something transient?" — discussed; a redirect-based approach needs the crash + redirect-tracking fixes first.
• User: "I committed it for you".

7. Redirect fixes (the crash)

• User: "Fix the crash", then "Well you can fix both, you will need to eventually anyhow" (crash + the aux redirect tracking), and later "make proper commits, add tests if possible".
• Agent: implemented (a) a length guard in ChangeSiteIsMirrorChange, (b) pkgAcqAuxFile::OriginalURI — restored before ReplyAux in Done/Failed so the mirror method can correlate replies after a redirect, and (c) a redirect test (mirrorlist behind a 301 to another "site", 127.0.0.2 vs localhost). Fixed a build error (Desc parameter shadowing the member — used this->Desc.URI).
• User: "also do check that the test indeed failed before your fixes", "Use pipefail setting or your && is pointless."
• Agent: the redirect test passed, but a cascade of failures appeared later in the file. Root-caused it: Configuration::Clear() leaves the item in the tree with an empty value, so a cleared redirect::replace::/mirror.txt was still applied — substituting to the empty string (redirecting /mirror.txt to the server root → an 11.1 kB autoindex page, and /mirror.txt.gz → 404). Verified with manual curl experiments. Fixed aptwebserver to skip empty redirect values, and made the transient block mv the index files back for the following tests. All 844 checks pass. TDD-verified the redirect test aborts with std::out_of_range on unfixed code (exit 134).
• Committed as three commits: "Fix crash when an aux file request is redirected", "Reply to aux requests with the original URI if redirected", "test: mirrorlist fetch via redirect, fix clearing redirect::replace config".

8. Simplification

• User: "check if you really need the whole destfile variable, isn't Oldcopy always the same as Itm->DestFile".
• Agent: confirmed — with the StatDone reporting, ReplyAux hands the real path (the old copy) to the method, so the computed fallback path is redundant. Simplified MirrorListFileReceived back to using Itm->DestFile directly (844 checks pass); committed "Simplify mirror method old-copy fallback".

9. Branch split

• User: "I did some rebasing. I'd like to have the redirect fixes in a separate branch without the offline-server changes".
• Agent: created aux-redirect from main, cherry-picked the crash fix (clean), the OriginalURI fix (conflict-resolved against main's original Failed/Done), and reduced the test commit to just the redirect block (801 checks pass). Reset keep-auxfiles to the two offline-server commits (826 checks pass). Both branches sit on main independently.

10. Documentation

• User: "Based on what you learned about the codebase and building it, write an AGENTS.md" (plus the git-clang-format.sh and fast ninja target facts).
• Agent: wrote AGENTS.md with layout, build (fast iteration targets), test invocation and helpers, aptwebserver usage and gotchas (in6addr_any binding, bad_addr IP-keying, Configuration::Clear empty items), unit-test CWD requirement, and domain notes (auxfiles, mirror method, retry/ignore semantics, index file naming in tests).

Final state

• keep-auxfiles: Keep old mirror file copies around and use them as fallback, Do not retry fetching the mirror file if an old copy exists.
• aux-redirect: Fix crash when an aux file request is redirected, Reply to aux requests with the original URI if redirected, test: mirrorlist fetch via redirect, fix clearing redirect::replace config (later merged into main as 9351e213f).
• All integration (826/801 checks respectively) and unit tests pass.