diff --git a/main.c b/main.c index 5aac6cfc6..9b52bbe6a 100644 --- a/main.c +++ b/main.c @@ -832,7 +832,16 @@ static char *get_local_name(struct file_list *flist, char *dest_path) dest_path = "/"; *cp = '\0'; - if (!change_dir(dest_path, CD_NORMAL)) { + if (dry_run && mkpath_dest_arg && do_stat(dest_path, &st) < 0) { + /* --mkpath would have created this parent dir, but a dry run did + * not, so don't chdir into it; flag the destination as not yet + * present (as the dir-creation path above does) so the generator + * doesn't try to compare against the missing tree (#880). Only + * the missing-parent case is touched, so an ordinary file-to-file + * dry run still itemizes against an existing destination. */ + dry_run++; + change_dir(dest_path, CD_SKIP_CHDIR); + } else if (!change_dir(dest_path, CD_NORMAL)) { rsyserr(FERROR, errno, "change_dir#3 %s failed", full_fname(dest_path)); exit_cleanup(RERR_FILESELECT); diff --git a/testsuite/file-to-file-mkpath-dry-run_test.py b/testsuite/file-to-file-mkpath-dry-run_test.py new file mode 100644 index 000000000..dd340b4b9 --- /dev/null +++ b/testsuite/file-to-file-mkpath-dry-run_test.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +# Regression test for issue #880 (and the dry-run itemize regression that the +# first proposed fix, PR #952, would have introduced). +# +# (1) Copying file-to-file with --mkpath and --dry-run used to abort with +# "change_dir#3 ... failed", because make_path() only *reports* (does not +# create) directories in a dry run, so the later chdir found no parent. +# +# (2) The fix must stay scoped to the missing-parent case: a plain +# file-to-file --dry-run onto an *existing*, differing destination must +# still itemize the real change, not report the file as brand new (PR #952 +# bumped dry_run unconditionally, which broke this). +# +# In both cases a "--dry-run -i" must produce the same itemized output as the +# real run. Based on the test from PR #952 by Stiliyan Tonev. + +import os +import subprocess + +from rsyncfns import SCRATCHDIR, makepath, rmtree, rsync_argv, test_fail + + +def itemize(*args): + p = subprocess.run(rsync_argv('-ai', *args), capture_output=True, text=True) + return p.returncode, p.stdout + p.stderr + + +# (1) --mkpath file-to-file: the dry run must succeed and match the real run. +mk = SCRATCHDIR / 'mk' +rmtree(mk) +makepath(mk / 'from') +(mk / 'from' / 'src').write_text("payload\n") + +drc, dry = itemize('--dry-run', '--mkpath', + str(mk / 'from' / 'src'), str(mk / 'dndir' / 'dst')) +rc, real = itemize('--mkpath', str(mk / 'from' / 'src'), str(mk / 'rdir' / 'dst')) +if drc != 0: + print(dry) + test_fail("--mkpath file-to-file --dry-run failed (#880)") +if not (mk / 'rdir' / 'dst').exists(): + test_fail("--mkpath real run did not create the file") +if dry.replace('dndir', 'X') != real.replace('rdir', 'X'): + test_fail(f"--mkpath dry-run output differs from the real run:\n" + f" dry : {dry!r}\n real: {real!r}") + +# (2) Plain file-to-file onto an existing, differing destination: the dry run +# must itemize the same change as the real run (a/dst and b/dst share the +# basename 'dst', so the itemized lines are directly comparable). +ex = SCRATCHDIR / 'ex' +rmtree(ex) +makepath(ex / 'a') +makepath(ex / 'b') +(ex / 'src').write_text("brand new content\n") +for d in ('a', 'b'): + (ex / d / 'dst').write_text("old\n") + os.utime(ex / d / 'dst', (0, 0)) # make size + mtime differ + +_, dry2 = itemize('--dry-run', str(ex / 'src'), str(ex / 'a' / 'dst')) +_, real2 = itemize(str(ex / 'src'), str(ex / 'b' / 'dst')) +if dry2 != real2: + test_fail(f"file-to-file --dry-run misreports an existing destination:\n" + f" dry : {dry2!r}\n real: {real2!r}")