From 053f78bd6379a66d0ffa177bee7643e494251a9e Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 27 Jul 2026 07:48:23 +0200 Subject: [PATCH 1/3] One unlink-then-rename helper for the three copies replace_file() (htsback.c), wacz_rename_over() (htswarc.c) and the inline block in singlefile_rewrite_file() each worked around Windows' non-clobbering rename, with two opposite return conventions and only one of the three converting path separators. Copying the wrong one gets you an inverted success test. hts_rename_over() replaces all four call sites: hts_boolean return, fconv() on both paths. It lands in htsname.c rather than the htstools.c the issue named, to stay clear of an in-flight PR over that file. The wacz test now runs a cacheless second pass over a poisoned copy of the package the first pass wrote, so repackaging has to clobber an existing file. 43 and 74 also assert the failure warnings stay out of the log, which is all an inverted return leaves behind at those two sites. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- src/htsback.c | 12 ++---------- src/htsname.c | 12 ++++++++++++ src/htsname.h | 3 +++ src/htssinglefile.c | 9 ++------- src/htswarc.c | 12 +----------- tests/43_local-update-truncate.test | 1 + tests/74_local-warc-wacz.test | 5 +++++ tests/local-crawl.sh | 16 ++++++++++++++++ 8 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/htsback.c b/src/htsback.c index 63b4eb808..f896cf560 100644 --- a/src/htsback.c +++ b/src/htsback.c @@ -590,14 +590,6 @@ static int create_back_tmpfile(httrackp *opt, lien_back *const back, return 0; } -/* Move src onto dst; RENAME does not clobber an existing target on Windows. */ -static hts_boolean replace_file(const char *src, const char *dst) { - if (RENAME(src, dst) == 0) - return HTS_TRUE; - (void) UNLINK(dst); - return RENAME(src, dst) == 0 ? HTS_TRUE : HTS_FALSE; -} - /* Commit or restore a re-fetch backup (#77 follow-up): a re-fetch over an existing file moved the good copy to back->tmpfile before truncating url_sav. commit keeps the new file and drops the backup; else restore it so an aborted @@ -615,7 +607,7 @@ static void back_finalize_backup(httrackp *opt, lien_back *const back, } /* On failure keep the backup: an orphaned temp beats losing the good copy. */ - if (!replace_file(back->tmpfile, back->url_sav)) + if (!hts_rename_over(back->tmpfile, back->url_sav)) hts_log_print(opt, LOG_WARNING | LOG_ERRNO, "could not restore %s; previous copy kept as %s", back->url_sav, back->tmpfile); @@ -746,7 +738,7 @@ int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback, "Read error when decompressing"); } UNLINK(unpacked); - } else if (replace_file(unpacked, back[p].url_sav)) { + } else if (hts_rename_over(unpacked, back[p].url_sav)) { /* The temp bypassed filecreate(), which is what chmods. */ #ifndef _WIN32 chmod(back[p].url_sav, HTS_ACCESS_FILE); diff --git a/src/htsname.c b/src/htsname.c index 1a41e4df3..037e33804 100644 --- a/src/htsname.c +++ b/src/htsname.c @@ -1851,3 +1851,15 @@ hts_boolean url_savename_refname_remove(httrackp *opt, const char *adr, return UNLINK(filename) == 0 ? HTS_TRUE : HTS_FALSE; } + +hts_boolean hts_rename_over(const char *src, const char *dst) { + char csrc[CATBUFF_SIZE], cdst[CATBUFF_SIZE]; + + fconv(csrc, sizeof(csrc), src); + fconv(cdst, sizeof(cdst), dst); + if (RENAME(csrc, cdst) == 0) + return HTS_TRUE; + /* RENAME does not clobber an existing target on Windows. */ + (void) UNLINK(cdst); + return RENAME(csrc, cdst) == 0 ? HTS_TRUE : HTS_FALSE; +} diff --git a/src/htsname.h b/src/htsname.h index f9fe5d5df..b8c3330d8 100644 --- a/src/htsname.h +++ b/src/htsname.h @@ -109,6 +109,9 @@ char *url_savename_refname_fullpath(httrackp * opt, const char *adr, /* Remove the temp-ref for (adr,fil); HTS_TRUE if it was removed. */ hts_boolean url_savename_refname_remove(httrackp *opt, const char *adr, const char *fil); +/* Move src onto dst, replacing an existing dst; HTS_TRUE on success. Both + paths are fconv()'d. */ +hts_boolean hts_rename_over(const char *src, const char *dst); #endif #endif diff --git a/src/htssinglefile.c b/src/htssinglefile.c index 31a2f6abb..a65b9de25 100644 --- a/src/htssinglefile.c +++ b/src/htssinglefile.c @@ -1108,13 +1108,8 @@ hts_boolean singlefile_rewrite_file(httrackp *opt, const char *root, (void) chmod(fconv(catbuff, sizeof(catbuff), StringBuff(tmp)), HTS_ACCESS_FILE); #endif - if (ok) { - /* RENAME does not clobber an existing target on Windows. */ - if (RENAME(StringBuff(tmp), page_path) != 0) { - (void) UNLINK(page_path); - ok = RENAME(StringBuff(tmp), page_path) == 0 ? HTS_TRUE : HTS_FALSE; - } - } + if (ok) + ok = hts_rename_over(StringBuff(tmp), page_path); if (!ok) { hts_log_print(opt, LOG_ERROR, "single-file: could not rewrite %s", page_path); diff --git a/src/htswarc.c b/src/htswarc.c index 9f23b8167..2b799dcc3 100644 --- a/src/htswarc.c +++ b/src/htswarc.c @@ -964,16 +964,6 @@ static zipFile wacz_zip_open(const char *path) { return zipOpen2_64(path, 0 /*create*/, NULL, &ff); } -/* Move src onto dst (UTF-8/Windows-safe); RENAME won't clobber on Windows, so - fall back to unlink+rename. Returns 0 on success. */ -static int wacz_rename_over(const char *src, const char *dst) { - char cs[CATBUFF_SIZE], cd[CATBUFF_SIZE]; - if (RENAME(fconv(cs, sizeof(cs), src), fconv(cd, sizeof(cd), dst)) == 0) - return 0; - (void) UNLINK(fconv(cd, sizeof(cd), dst)); - return RENAME(fconv(cs, sizeof(cs), src), fconv(cd, sizeof(cd), dst)); -} - /* Package the segment(s) + .cdx + a generated pages.jsonl into .wacz at crawl end (the archive file(s) and .cdx are already closed on disk). */ static void warc_wacz_package(warc_writer *w) { @@ -1092,7 +1082,7 @@ static void warc_wacz_package(warc_writer *w) { hts_log_print(w->opt, LOG_WARNING, "WACZ: packaging failed, kept existing %s untouched", waczpath); - } else if (wacz_rename_over(tmppath, waczpath) != 0) { + } else if (!hts_rename_over(tmppath, waczpath)) { (void) UNLINK(fconv(catbuff, sizeof(catbuff), tmppath)); hts_log_print(w->opt, LOG_WARNING | LOG_ERRNO, "WACZ: could not finalize %s", waczpath); diff --git a/tests/43_local-update-truncate.test b/tests/43_local-update-truncate.test index c3d6fe9a0..4671688d5 100644 --- a/tests/43_local-update-truncate.test +++ b/tests/43_local-update-truncate.test @@ -18,6 +18,7 @@ set -euo pipefail bash "$top_srcdir/tests/local-crawl.sh" \ --rerun-args '--update -M400000' \ --log-found 'More than 400000 bytes have been transferred.. giving up' \ + --log-not-found 'could not restore' \ --found bigtrunc/slow.bin \ --file-min-bytes bigtrunc/slow.bin 655360 \ --file-min-bytes bigtrunc/fast.bin 655360 \ diff --git a/tests/74_local-warc-wacz.test b/tests/74_local-warc-wacz.test index 16c617a18..2e0817998 100755 --- a/tests/74_local-warc-wacz.test +++ b/tests/74_local-warc-wacz.test @@ -5,11 +5,16 @@ # real gate: STORE-mode entries, the fixed layout, recomputed sha256 digests and # the datapackage-digest chain (plus py-wacz/pywb when importable). The crawl # skips cleanly on a build without OpenSSL (no conformant SHA-256 -> no package). +# +# The cacheless second pass repackages over the .wacz the first one left: the +# clobber, not the happy rename, is what breaks when a platform refuses to +# rename onto an existing file (#726). set -eu : "${top_srcdir:=..}" bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --wacz-validate \ + --rerun-args '-C0' --log-not-found 'could not finalize' \ --found 'mini304/index.html' --found 'mini304/page.html' \ httrack 'BASEURL/mini304/index.html' --warc-file warc-out --wacz diff --git a/tests/local-crawl.sh b/tests/local-crawl.sh index ce3777a92..5ab3f41c4 100755 --- a/tests/local-crawl.sh +++ b/tests/local-crawl.sh @@ -57,6 +57,8 @@ rerun_dead= tmpdir= serverpid= crawlpid= +wacz_poisoned= +wacz_poison="stale-wacz-that-a-second-pass-must-replace" function warning { echo "** $*" >&2 @@ -272,6 +274,14 @@ if test -n "$warc_validate"; then test -z "$w1" || cp "$w1" "${tmpdir}/warc-pass1.gz" fi +# Poison the first-pass .wacz when a second pass follows: repackaging moves the +# new archive over it, so the marker must be gone afterwards (#726). Poisoning +# beats comparing the two packages, which can come out byte-identical. +if test -n "$wacz_validate" && test -n "${rerun}${rerun_args}"; then + wacz_poisoned=$(find "$mirrorroot" -maxdepth 2 -name '*.wacz' 2>/dev/null | sort | tail -n1) + test -z "$wacz_poisoned" || echo "$wacz_poison" >"$wacz_poisoned" +fi + # --- optional second pass: re-mirror into the same dir (cache/update path) ---- if test -n "$rerun"; then info "re-running httrack (update pass)" @@ -416,6 +426,12 @@ if test -n "$wacz_validate"; then fi die "no .wacz file produced under $mirrorroot" fi + if test -n "$wacz_poisoned"; then + info "checking the second pass replaced the .wacz" + grep -q "$wacz_poison" "$wacz_poisoned" 2>/dev/null && + die "stale .wacz kept: $wacz_poisoned" + result "OK" + fi validator=$(nativepath "${testdir}/wacz-validate.py") info "validating WACZ package" "$python" "$validator" "$(nativepath "$wacz")" >&2 || die "WACZ validation failed" From 66425d0a0844346b11f8a9f1972ef56a4aeb73ce Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 27 Jul 2026 08:29:01 +0200 Subject: [PATCH 2/3] rename helper: move hts_rename_over to htstools.c Issue #726 asked for htstools.c; it went to htsname.c only because #718 held the file at the time. Kept internal, not HTSEXT_API: htscore.h pulls in both headers, so every call site reaches it either way. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- src/htsname.c | 11 ----------- src/htsname.h | 3 --- src/htstools.c | 12 ++++++++++++ src/htstools.h | 4 ++++ 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/htsname.c b/src/htsname.c index 037e33804..48c4c14ed 100644 --- a/src/htsname.c +++ b/src/htsname.c @@ -1852,14 +1852,3 @@ hts_boolean url_savename_refname_remove(httrackp *opt, const char *adr, return UNLINK(filename) == 0 ? HTS_TRUE : HTS_FALSE; } -hts_boolean hts_rename_over(const char *src, const char *dst) { - char csrc[CATBUFF_SIZE], cdst[CATBUFF_SIZE]; - - fconv(csrc, sizeof(csrc), src); - fconv(cdst, sizeof(cdst), dst); - if (RENAME(csrc, cdst) == 0) - return HTS_TRUE; - /* RENAME does not clobber an existing target on Windows. */ - (void) UNLINK(cdst); - return RENAME(csrc, cdst) == 0 ? HTS_TRUE : HTS_FALSE; -} diff --git a/src/htsname.h b/src/htsname.h index b8c3330d8..f9fe5d5df 100644 --- a/src/htsname.h +++ b/src/htsname.h @@ -109,9 +109,6 @@ char *url_savename_refname_fullpath(httrackp * opt, const char *adr, /* Remove the temp-ref for (adr,fil); HTS_TRUE if it was removed. */ hts_boolean url_savename_refname_remove(httrackp *opt, const char *adr, const char *fil); -/* Move src onto dst, replacing an existing dst; HTS_TRUE on success. Both - paths are fconv()'d. */ -hts_boolean hts_rename_over(const char *src, const char *dst); #endif #endif diff --git a/src/htstools.c b/src/htstools.c index 42b2b94bd..27ef1bbd0 100644 --- a/src/htstools.c +++ b/src/htstools.c @@ -1428,3 +1428,15 @@ HTSEXT_API hts_boolean hts_findissystem(find_handle find) { } return 0; } + +hts_boolean hts_rename_over(const char *src, const char *dst) { + char csrc[CATBUFF_SIZE], cdst[CATBUFF_SIZE]; + + fconv(csrc, sizeof(csrc), src); + fconv(cdst, sizeof(cdst), dst); + if (RENAME(csrc, cdst) == 0) + return HTS_TRUE; + /* RENAME does not clobber an existing target on Windows. */ + (void) UNLINK(cdst); + return RENAME(csrc, cdst) == 0 ? HTS_TRUE : HTS_FALSE; +} diff --git a/src/htstools.h b/src/htstools.h index c0a57d2a4..8a8b373ef 100644 --- a/src/htstools.h +++ b/src/htstools.h @@ -132,6 +132,10 @@ HTSEXT_API hts_boolean hts_findisdir(find_handle find); HTSEXT_API hts_boolean hts_findisfile(find_handle find); HTSEXT_API hts_boolean hts_findissystem(find_handle find); +/* Move src onto dst, replacing an existing dst; HTS_TRUE on success. Both + paths are fconv()'d. */ +hts_boolean hts_rename_over(const char *src, const char *dst); + #endif #endif From f3a83f71ba7714c000a9088268874fc82cc1f0dc Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 27 Jul 2026 08:29:28 +0200 Subject: [PATCH 3/3] format: drop the trailing blank line left by the move Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- src/htsname.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/htsname.c b/src/htsname.c index 48c4c14ed..1a41e4df3 100644 --- a/src/htsname.c +++ b/src/htsname.c @@ -1851,4 +1851,3 @@ hts_boolean url_savename_refname_remove(httrackp *opt, const char *adr, return UNLINK(filename) == 0 ? HTS_TRUE : HTS_FALSE; } -