From 57c348e8c882e3c6907763fb316dea8cb882bf1a Mon Sep 17 00:00:00 2001 From: rajputa-deshaw <142570235+rajputa-deshaw@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:54:08 +0000 Subject: [PATCH 1/4] Add --delay-symlinks to create symlinks after all files are in place The generator creates a symlink as soon as it reaches it in the file list, while regular files arrive later through the sender and receiver (and, with --delay-updates, are only renamed into place at the end). For the rest of the transfer a new symlink can point at a file that isn't there yet, so anything reading the destination during the sync -- e.g. a loader resolving libfoo.so -> libfoo.so.2 -- can fail. The new --delay-symlinks option makes the generator defer new and changed symlinks and create them once the receiver reports that every file, including any --delay-updates renames, is in place. Each link is itemized and logged after it has been created, before the generator ends the delay-updates phase, and the deletion stats are sent after any directory that a deferred link replaces. The option implies --no-inc-recursive on the receiving side and is passed to a remote receiver, which must also support it. It is an error to combine it with --remove-source-files, --hard-links, --compare-dest, --copy-dest, or --link-dest, or to use it with a protocol older than 29. support/rrsync accepts the option and refuses it under -no-overwrite, as it does --delay-updates. Add delay-symlinks_test.py, which holds a push partway through the link's target to check that new and changed links aren't created early, and rrsync-no-overwrite-delay-symlinks_test.py. --- compat.c | 11 +- generator.c | 91 ++++- options.c | 22 ++ rsync.1.md | 25 ++ support/rrsync | 5 + support/rrsync.1.md | 10 +- testsuite/delay-symlinks_test.py | 336 ++++++++++++++++++ ...rrsync-no-overwrite-delay-symlinks_test.py | 76 ++++ 8 files changed, 569 insertions(+), 7 deletions(-) create mode 100644 testsuite/delay-symlinks_test.py create mode 100644 testsuite/rrsync-no-overwrite-delay-symlinks_test.py diff --git a/compat.c b/compat.c index ba1b0c949..e159c7f52 100644 --- a/compat.c +++ b/compat.c @@ -36,6 +36,7 @@ extern int fuzzy_basis; extern int read_batch; extern int write_batch; extern int delay_updates; +extern int delay_symlinks; extern int checksum_seed; extern int basis_dir_cnt; extern int prune_empty_dirs; @@ -173,7 +174,7 @@ void set_allow_inc_recurse(void) allow_inc_recurse = 0; else if (!am_sender && (delete_before || delete_after - || delay_updates || prune_empty_dirs)) + || delay_updates || delay_symlinks || prune_empty_dirs)) allow_inc_recurse = 0; else if (am_server && strchr(client_info, 'i') == NULL) allow_inc_recurse = 0; @@ -719,6 +720,14 @@ void setup_protocol(int f_out,int f_in) protocol_version); exit_cleanup(RERR_PROTOCOL); } + + if (delay_symlinks) { + rprintf(FERROR, + "--delay-symlinks requires protocol 29 or higher" + " (negotiated %d).\n", + protocol_version); + exit_cleanup(RERR_PROTOCOL); + } } else if (protocol_version >= 30) { if (am_server) { compat_flags = allow_inc_recurse ? CF_INC_RECURSE : 0; diff --git a/generator.c b/generator.c index c80498eaa..d2b5e1b10 100644 --- a/generator.c +++ b/generator.c @@ -58,6 +58,7 @@ extern int msgdone_cnt; extern int ignore_errors; extern int remove_source_files; extern int delay_updates; +extern int delay_symlinks; extern int update_only; extern int human_readable; extern int ignore_existing; @@ -117,14 +118,48 @@ static int need_retouch_dir_times; static int need_retouch_dir_perms; static const char *solo_file = NULL; +/* With --delay-symlinks, new and changed symlinks are deferred here and created + * only after the receiver has put every file into place (including any + * --delay-updates renames). Each entry keeps what recv_generator() needs to + * itemize and log the link once it has been created. */ +struct deferred_symlink { + struct file_struct *file; + int ndx; + char *fname; + int itemizing; + enum logcode code; +}; +static int defer_symlinks; +static item_list deferred_symlinks = EMPTY_ITEM_LIST; + +#ifdef SUPPORT_LINKS +static void defer_symlink(struct file_struct *file, int ndx, const char *fname, + int itemizing, enum logcode code) +{ + struct deferred_symlink *ds + = EXPAND_ITEM_LIST(&deferred_symlinks, struct deferred_symlink, 100); + + ds->file = file; + ds->ndx = ndx; + if (!(ds->fname = strdup(fname))) + out_of_memory("defer_symlink"); + ds->itemizing = itemizing; + ds->code = code; +} +#endif + /* Forward declarations. */ #ifdef SUPPORT_HARD_LINKS static void handle_skipped_hlink(struct file_struct *file, int itemizing, enum logcode code, int f_out); #endif -#define EARLY_DELAY_DONE_MSG() (!delay_updates) -#define EARLY_DELETE_DONE_MSG() (!(delete_during == 2 || delete_after)) +/* Deferred symlinks are created and itemized in the delay-updates phase, so + * that phase must not be ended early either, and the deletion stats must wait + * for any directory that a deferred symlink replaces. */ +#define EARLY_DELAY_DONE_MSG() (!delay_updates && !defer_symlinks) +#define EARLY_DELETE_DONE_MSG() \ + (!(delete_during == 2 || delete_after || defer_symlinks)) static int start_delete_delay_temp(void) { @@ -2001,6 +2036,10 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, fnamecmp = fnamecmpbuf; } } + if (defer_symlinks) { + defer_symlink(file, ndx, fname, itemizing, code); + goto cleanup; + } if (atomic_create(file, fname, sl, NULL, MAKEDEV(0, 0), &sx, statret == 0 ? DEL_FOR_SYMLINK : 0)) { set_file_attrs(fname, file, NULL, NULL, 0); if (itemizing) { @@ -2715,6 +2754,46 @@ void check_for_finished_files(int itemizing, enum logcode code, int check_redo) } } +/* Create the symlinks deferred by recv_generator(), then itemize and log each + * one as recv_generator() would have. We re-stat each destination because the + * transfer may have changed what's there. A link that can't be created is + * reported as an error and not itemized. */ +static void create_deferred_symlinks(void) +{ +#ifdef SUPPORT_LINKS + struct deferred_symlink *ds = deferred_symlinks.items; + size_t i; + + for (i = 0; i < deferred_symlinks.count; i++) { + struct file_struct *file = ds[i].file; + /* atomic_create() can delete a directory in the way, which builds + * its entries' paths in this buffer, so it must be MAXPATHLEN. */ + char fname[MAXPATHLEN]; + stat_x sx; + int statret; + + strlcpy(fname, ds[i].fname, sizeof fname); + free(ds[i].fname); + init_stat_x(&sx); + statret = gen_entry_stat(fname, file, &sx.st, 0); + if (atomic_create(file, fname, F_SYMLINK(file), NULL, MAKEDEV(0, 0), + &sx, statret == 0 ? DEL_FOR_SYMLINK : 0)) { + set_file_attrs(fname, file, NULL, NULL, 0); + if (ds[i].itemizing) { + if (statret == 0 && !S_ISLNK(sx.st.st_mode)) + statret = -1; + itemize(fname, file, ds[i].ndx, statret, &sx, + ITEM_LOCAL_CHANGE|ITEM_REPORT_CHANGE, 0, NULL); + } + if (ds[i].code != FNONE && INFO_GTE(NAME, 1)) + rprintf(ds[i].code, "%s -> %s\n", fname, F_SYMLINK(file)); + } + free_stat_x(&sx); + } + deferred_symlinks.count = 0; +#endif +} + void generate_files(int f_out, const char *local_name) { int i, ndx, next_loopchk = 0; @@ -2748,6 +2827,8 @@ void generate_files(int f_out, const char *local_name) symlink_timeset_failed_flags = ITEM_REPORT_TIME | (protocol_version >= 30 || !am_server ? ITEM_REPORT_TIMEFAIL : 0); implied_dirs_are_missing = relative_paths && !implied_dirs && protocol_version < 30; + /* A dry run creates nothing, so its symlinks take the normal path. */ + defer_symlinks = delay_symlinks && !dry_run; if (DEBUG_GTE(GENR, 1)) rprintf(FINFO, "generator starting pid=%d\n", (int)getpid()); @@ -2881,6 +2962,12 @@ void generate_files(int f_out, const char *local_name) wait_for_receiver(); } + /* The receiver has now put every file into place, including any + * --delay-updates renames, and the sender is still reading the itemized + * output, so the deferred symlinks can be created and reported. */ + if (defer_symlinks) + create_deferred_symlinks(); + if (protocol_version >= 29) { phase++; if (DEBUG_GTE(GENR, 1)) diff --git a/options.c b/options.c index 3a2ce2bda..90e94bb7c 100644 --- a/options.c +++ b/options.c @@ -151,6 +151,7 @@ int blocking_io = -1; int checksum_seed = 0; int inplace = 0; int delay_updates = 0; +int delay_symlinks = 0; int32 block_size = 0; time_t stop_at_utime = 0; char *skip_compress = NULL; @@ -790,6 +791,8 @@ static struct poptOption long_options[] = { {"partial-dir", 0, POPT_ARG_STRING, &partial_dir, 0, 0, 0 }, {"delay-updates", 0, POPT_ARG_VAL, &delay_updates, 1, 0, 0 }, {"no-delay-updates", 0, POPT_ARG_VAL, &delay_updates, 0, 0, 0 }, + {"delay-symlinks", 0, POPT_ARG_VAL, &delay_symlinks, 1, 0, 0 }, + {"no-delay-symlinks",0, POPT_ARG_VAL, &delay_symlinks, 0, 0, 0 }, {"prune-empty-dirs",'m', POPT_ARG_VAL, &prune_empty_dirs, 1, 0, 0 }, {"no-prune-empty-dirs",0,POPT_ARG_VAL, &prune_empty_dirs, 0, 0, 0 }, {"no-m", 0, POPT_ARG_VAL, &prune_empty_dirs, 0, 0, 0 }, @@ -2292,6 +2295,22 @@ int parse_arguments(int *argc_p, const char ***argv_p) remove_source_files == 1 ? "source" : "sent"); goto cleanup; } + if (delay_symlinks && remove_source_files) { + snprintf(err_buf, sizeof err_buf, + "--delay-symlinks cannot be used with --remove-%s-files\n", + remove_source_files == 1 ? "source" : "sent"); + goto cleanup; + } + if (delay_symlinks && preserve_hard_links) { + snprintf(err_buf, sizeof err_buf, + "--delay-symlinks cannot be used with --hard-links\n"); + goto cleanup; + } + if (delay_symlinks && basis_dir_cnt) { + snprintf(err_buf, sizeof err_buf, + "--delay-symlinks cannot be used with %s\n", alt_dest_opt(0)); + goto cleanup; + } if (batch_name && strlen(batch_name) > MAX_BATCH_NAME_LEN) { snprintf(err_buf, sizeof err_buf, "the batch-file name must be %d characters or less.\n", @@ -3069,6 +3088,9 @@ void server_options(char **args, int *argc_p) } else if (keep_partial && am_sender) args[ac++] = "--partial"; + if (delay_symlinks && am_sender) + args[ac++] = "--delay-symlinks"; + if (ignore_errors) args[ac++] = "--ignore-errors"; diff --git a/rsync.1.md b/rsync.1.md index 04df25d2a..699986926 100644 --- a/rsync.1.md +++ b/rsync.1.md @@ -621,6 +621,7 @@ has its own detailed description later in this manpage. --partial keep partially transferred files --partial-dir=DIR put a partially transferred file into DIR --delay-updates put all updated files into place at end +--delay-symlinks create symlinks after all files are in place --prune-empty-dirs, -m prune empty directory chains from file-list --numeric-ids don't map uid/gid values by user/group name --usermap=STRING custom username mapping @@ -1012,6 +1013,7 @@ sign) if you want the local shell to expand it. - [`--delete-after`](#opt) - [`--prune-empty-dirs`](#opt) - [`--delay-updates`](#opt) + - [`--delay-symlinks`](#opt) In order to be compatible with incremental recursion, [`--delete-during`](#opt) is the default delete mode for [`--delete`](#opt). @@ -3745,6 +3747,29 @@ sign) if you want the local shell to expand it. update algorithm that is even closer to atomic (it uses [`--link-dest`](#opt) and a parallel hierarchy of files). +0. `--delay-symlinks` + + This option tells the receiving rsync to hold back the creation of new and + changed symlinks until every other file in the transfer has been put into + place, including the renames done by [`--delay-updates`](#opt). Without + it, a symlink is created as soon as rsync reaches it in the file list, so + for the rest of the transfer it can point at a file that hasn't arrived + yet. + + If a symlink is moved to a new target and the old target is deleted, also + use [`--delete-delay`](#opt) or [`--delete-after`](#opt), since the default + [`--delete-during`](#opt) can remove the old target while the old symlink + still points at it. + + This option implies [`--no-inc-recursive`](#opt) since it needs the full + file list in memory in order to be able to iterate over it at the end. + + Conflicts with [`--remove-source-files`](#opt), [`--hard-links`](#opt), + [`--compare-dest`](#opt), [`--copy-dest`](#opt), and [`--link-dest`](#opt). + This option is incompatible with rsync versions prior to 2.6.4 (March + 2005), and when the receiving side is remote, the remote rsync must also + support this option. + 0. `--prune-empty-dirs`, `-m` This option tells the receiving rsync to get rid of empty directories from diff --git a/support/rrsync b/support/rrsync index 060c8f086..ace183fbe 100755 --- a/support/rrsync +++ b/support/rrsync @@ -52,6 +52,7 @@ long_opts = { 'copy-unsafe-links': 0, 'daemon': -1, 'debug': -1, + 'delay-symlinks': 0, 'delay-updates': 0, 'delete': 0, 'delete-after': 0, @@ -519,7 +520,11 @@ def main(): # backup onto a name that already exists deletes what is there # (backup.c make_backup()), and deletion backs files up as well # (delete.c), so an unrelated --delete can land on a protected name. + # --delay-symlinks creates its symlinks at the end of the transfer and + # replaces whatever has appeared at that name since the + # --ignore-existing check. long_opts['log-file'] = long_opts['partial-dir'] = long_opts['delay-updates'] = -1 + long_opts['delay-symlinks'] = -1 long_opts['backup-dir'] = -1 short_disabled += 'b' # must precede the short_no_arg_re build below diff --git a/support/rrsync.1.md b/support/rrsync.1.md index ae0042576..2ab351de2 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -98,11 +98,13 @@ The remainder of this manpage is dedicated to using the rrsync script. Because `--ignore-existing` protects only the file being transferred, this also refuses the options that can reach a *different* existing file in the - restricted dir: `--log-file`, `--partial-dir`, `--delay-updates`, and - backup mode (`-b`, `--backup-dir`, whose published backup replaces whatever + restricted dir: `--log-file`, `--partial-dir`, `--delay-updates`, + `--delay-symlinks` (which creates its symlinks at the end of the transfer, + replacing whatever has appeared at that name since the check), and backup + mode (`-b`, `--backup-dir`, whose published backup replaces whatever already occupies the backup name). Resumable uploads with an explicit - `--partial-dir`, `--delay-updates`, and server-side logging are therefore - unavailable under this option. + `--partial-dir`, `--delay-updates`, `--delay-symlinks`, and server-side + logging are therefore unavailable under this option. 0. `-help`, `-h` diff --git a/testsuite/delay-symlinks_test.py b/testsuite/delay-symlinks_test.py new file mode 100644 index 000000000..6dbfc6163 --- /dev/null +++ b/testsuite/delay-symlinks_test.py @@ -0,0 +1,336 @@ +#!/usr/bin/env python3 +# Coverage of --delay-symlinks, alone and together with --delay-updates. +# +# The generator holds back new and changed symlinks, and generate_files() +# creates them with create_deferred_symlinks() once the receiver's redo-phase +# MSG_DONE says every file (including any --delay-updates renames) is in place, +# itemizing each one only after it has been created. +# +# The ordering checks push through a remote shell that relays the sender's data +# and stops partway through a large target file until the test lets it go on. +# The symlink sorts before its target, so by the time any of the target's data +# is sent the generator has already handled the symlink. At the pause, the +# link therefore exists and dangles without the option, and doesn't exist yet +# with it -- no timing is involved. + +import os +import shutil +import subprocess +import sys +import time + +from rsyncfns import ( + FROMDIR, SCRATCHDIR, TODIR, + assert_is_symlink, assert_same, checkit, forced_protocol, make_data_file, + rmtree, rsh_cmd, rsync_argv, rsync_path_arg, run_rsync, test_fail, + test_skipped, +) + +proto = forced_protocol() +if proto is not None and proto < 29: + test_skipped(f"--delay-symlinks requires protocol 29+ (negotiated {proto})") + +os.environ['RSYNC_RSH'] = rsh_cmd() + +TARGET_SIZE = 4 * 1024 * 1024 +PAUSE_AT = 1024 * 1024 # bytes of the sender's stream to let through +LINK = os.path.join('a', 'libfoo.so') +LINK_TARGET = '../lib/libfoo.so.1' +TARGET = os.path.join('lib', 'libfoo.so.1') +OLD_LINK_TARGET = '../lib/libfoo.so.0' +OLD_TARGET = os.path.join('lib', 'libfoo.so.0') + +CTL = SCRATCHDIR / 'relay-ctl' +RELAY = SCRATCHDIR / 'relay-rsh' +RELAY.write_text(f'''#!{sys.executable} +# Remote shell that runs the command locally and relays its stdin, stopping +# after DS_PAUSE_AT bytes until the file DS_CTL/resume exists. Like ssh, it +# exits as soon as the command does. +import os, subprocess, sys, threading, time +args = sys.argv[1:] +while args and args[0].startswith('-'): + args.pop(0) +args.pop(0) # the host +pause_at = int(os.environ['DS_PAUSE_AT']) +ctl = os.environ['DS_CTL'] +child = subprocess.Popen(['sh', '-c', ' '.join(args)], stdin=subprocess.PIPE) +# Only the command writes to the client, so it sees EOF when the command exits. +os.close(1) + +def relay(): + sent = 0 + try: + while True: + if sent == pause_at: + open(os.path.join(ctl, 'paused'), 'w').close() + while not os.path.exists(os.path.join(ctl, 'resume')): + time.sleep(0.01) + data = os.read(0, 65536 if sent >= pause_at else min(65536, pause_at - sent)) + if not data: + break + child.stdin.write(data) + child.stdin.flush() + sent += len(data) + child.stdin.close() + except (BrokenPipeError, ValueError): + pass + +threading.Thread(target=relay, daemon=True).start() +os._exit(child.wait()) +''') +RELAY.chmod(0o755) + + +def seed_pair(): + rmtree(FROMDIR) + rmtree(TODIR) + (FROMDIR / 'a').mkdir(parents=True) + (FROMDIR / 'lib').mkdir() + make_data_file(FROMDIR / TARGET, TARGET_SIZE) + os.symlink(LINK_TARGET, FROMDIR / LINK) + + +def seed_changed(): + """Like seed_pair(), but the destination already has the link pointing at + an old target, so the transfer changes it.""" + seed_pair() + (TODIR / 'a').mkdir(parents=True) + (TODIR / 'lib').mkdir() + (TODIR / OLD_TARGET).write_text("old\n") + os.symlink(OLD_LINK_TARGET, TODIR / LINK) + + +def paused_push(extra, at_pause): + """Push FROMDIR to TODIR through the relay, call at_pause() while the + sender's data is held back partway through the target, then let the + transfer finish. Returns (exit code, stdout, stderr).""" + rmtree(CTL) + CTL.mkdir() + env = dict(os.environ, DS_PAUSE_AT=str(PAUSE_AT), DS_CTL=str(CTL)) + argv = rsync_argv('-ai', '--no-inc-recursive', *extra, + f'--rsh={rsh_cmd(str(RELAY))}', + f'--rsync-path={rsync_path_arg()}', + f'{FROMDIR}/', f'lh:{TODIR}/') + with open(CTL / 'out', 'w') as out, open(CTL / 'err', 'w') as err: + proc = subprocess.Popen(argv, stdout=out, stderr=err, env=env) + deadline = time.monotonic() + 60 + while not (CTL / 'paused').exists(): + if proc.poll() is not None or time.monotonic() > deadline: + proc.kill() + proc.wait() + test_fail(f"{' '.join(extra)}: the transfer never reached the " + f"pause point:\n{(CTL / 'err').read_text()}") + time.sleep(0.01) + try: + at_pause() + finally: + (CTL / 'resume').touch() + rc = proc.wait(timeout=60) + return rc, (CTL / 'out').read_text(), (CTL / 'err').read_text() + + +def link_dangles(): + link = TODIR / LINK + return os.path.islink(link) and not os.path.exists(link) + + +def link_target(): + link = TODIR / LINK + return os.readlink(link) if os.path.islink(link) else None + + +def readonly_dirs_enforced(): + """Does a 0555 directory refuse new entries here? It doesn't for root, or + for a user holding CAP_DAC_OVERRIDE.""" + probe = SCRATCHDIR / 'ro-probe' + rmtree(probe) + probe.mkdir() + probe.chmod(0o555) + try: + (probe / 'entry').touch() + except PermissionError: + return True + finally: + probe.chmod(0o755) + rmtree(probe) + return False + + +itemized_link = f'cL+++++++++ {LINK} -> {LINK_TARGET}' +can_fail_link = readonly_dirs_enforced() + +# Control: without the option the symlink exists, and dangles, at the pause. +seed_pair() +seen = {} +rc, out, err = paused_push([], lambda: seen.update(dangling=link_dangles())) +if not seen['dangling']: + test_fail("without --delay-symlinks the link should dangle at the pause " + "point; the pause point isn't where the test expects it") +if rc != 0: + test_fail(f"control transfer exited {rc}:\n{err}") + +# Control: without the option an existing link has already been repointed at +# the new target, and dangles, at the pause. +seed_changed() +seen = {} +rc, out, err = paused_push( + [], lambda: seen.update(target=link_target(), dangling=link_dangles())) +if seen['target'] != LINK_TARGET or not seen['dangling']: + test_fail("without --delay-symlinks the changed link should already point " + f"at the new target and dangle at the pause point, got {seen}") +if rc != 0: + test_fail(f"control transfer exited {rc}:\n{err}") + +for extra in ([], ['--delay-updates']): + label = ' '.join(['--delay-symlinks', *extra]) + + # With the option, nothing exists at that path until the target is in place. + seed_pair() + seen = {} + rc, out, err = paused_push( + ['--delay-symlinks', *extra], + lambda: seen.update(link=os.path.lexists(TODIR / LINK), + target=os.path.exists(TODIR / TARGET))) + if seen['target']: + test_fail(f"{label}: the target was already in place at the pause point") + if seen['link']: + test_fail(f"{label}: the symlink was created before its target") + if rc != 0: + test_fail(f"{label}: transfer exited {rc}:\n{err}") + assert_is_symlink(TODIR / LINK, target=LINK_TARGET, label=label) + assert_same(TODIR / TARGET, FROMDIR / TARGET, label=label) + if itemized_link not in out.splitlines(): + test_fail(f"{label}: the created symlink wasn't itemized:\n{out}") + + # An existing link that changes keeps pointing at its old target until the + # new target is in place. + seed_changed() + seen = {} + rc, out, err = paused_push( + ['--delay-symlinks', *extra], + lambda: seen.update(target=link_target(), + resolves=os.path.exists(TODIR / LINK))) + if seen['target'] != OLD_LINK_TARGET or not seen['resolves']: + test_fail(f"{label}: the changed symlink was replaced before its new " + f"target was in place, got {seen}") + if rc != 0: + test_fail(f"{label}: transfer exited {rc}:\n{err}") + assert_is_symlink(TODIR / LINK, target=LINK_TARGET, label=label) + lines = out.splitlines() + if not any(line.startswith('cL') and LINK in line for line in lines): + test_fail(f"{label}: the changed symlink wasn't itemized:\n{out}") + + # A symlink that can't be created is an error, and isn't itemized. The + # link's directory is made read-only at the pause point, so this is skipped + # where a read-only directory doesn't stop the link being created. + if can_fail_link: + seed_pair() + try: + rc, out, err = paused_push(['--delay-symlinks', *extra], + lambda: os.chmod(TODIR / 'a', 0o555)) + finally: + if os.path.isdir(TODIR / 'a'): + os.chmod(TODIR / 'a', 0o755) + if rc != 23: + test_fail(f"{label}: a failed symlink should exit 23, got {rc}:\n{err}") + if any(LINK in line for line in out.splitlines()): + test_fail(f"{label}: a symlink that failed was itemized:\n{out}") + if LINK not in err: + test_fail(f"{label}: the failed symlink wasn't reported:\n{err}") + if os.path.lexists(TODIR / LINK): + test_fail(f"{label}: the symlink exists although creating it failed") + + +def seed_tree(): + rmtree(FROMDIR) + rmtree(TODIR) + FROMDIR.mkdir(parents=True) + make_data_file(FROMDIR / 'libfoo.so.1', 64 * 1024) + os.symlink('libfoo.so.1', FROMDIR / 'libfoo.so') + (FROMDIR / 'v1').mkdir() + make_data_file(FROMDIR / 'v1' / 'data', 16 * 1024) + os.symlink('v1', FROMDIR / 'current') + + +# New targets plus new symlinks to them, locally. +seed_tree() +checkit(['-ai', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/'], FROMDIR, TODIR) + +# Move both symlinks to new targets and delete the old ones. +os.unlink(FROMDIR / 'libfoo.so.1') +os.unlink(FROMDIR / 'libfoo.so') +make_data_file(FROMDIR / 'libfoo.so.2', 64 * 1024) +os.symlink('libfoo.so.2', FROMDIR / 'libfoo.so') +rmtree(FROMDIR / 'v1') +(FROMDIR / 'v2').mkdir() +make_data_file(FROMDIR / 'v2' / 'data', 16 * 1024) +os.unlink(FROMDIR / 'current') +os.symlink('v2', FROMDIR / 'current') +checkit(['-ai', '--delete-delay', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/'], + FROMDIR, TODIR) + +# Swap a regular file and a symlink for each other. +os.unlink(FROMDIR / 'libfoo.so') +(FROMDIR / 'libfoo.so').write_text("now a file\n") +os.unlink(FROMDIR / 'libfoo.so.2') +os.symlink('libfoo.so', FROMDIR / 'libfoo.so.2') +checkit(['-ai', '--delete', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/'], + FROMDIR, TODIR) + +# Replace a non-empty directory with a symlink: --force deletes the directory +# tree when the deferred link is created, and those deletions are counted the +# same as without the option. +os.unlink(FROMDIR / 'current') +os.symlink('v2', FROMDIR / 'current') +os.unlink(TODIR / 'current') +(TODIR / 'current' / 'sub').mkdir(parents=True) +(TODIR / 'current' / 'file').write_text("in the way\n") +(TODIR / 'current' / 'sub' / 'file').write_text("in the way\n") +plain = SCRATCHDIR / 'to-plain' +rmtree(plain) +shutil.copytree(TODIR, plain, symlinks=True) + + +def deletion_stats(*args): + proc = run_rsync('-a', '--force', '--stats', *args, capture_output=True) + return [line for line in proc.stdout.splitlines() + if line.startswith('Number of deleted files')] + + +expected = deletion_stats(f'{FROMDIR}/', f'{plain}/') +got = deletion_stats('--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/') +if got != expected: + test_fail(f"--delay-symlinks reported {got} deletions, " + f"{expected} without it") +rmtree(plain) +checkit(['-ai', '--force', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/'], + FROMDIR, TODIR) + +# An unchanged symlink is left alone and not itemized. +ino = os.lstat(TODIR / 'current').st_ino +proc = run_rsync('-ai', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/', + capture_output=True) +if os.lstat(TODIR / 'current').st_ino != ino: + test_fail("an unchanged symlink was recreated") +if 'current' in proc.stdout: + test_fail(f"an unchanged symlink was itemized:\n{proc.stdout}") + +# A symlink inside a read-only directory: rsync makes the directory writable +# during the transfer and restores its mode at the end, after the symlinks. +seed_tree() +(FROMDIR / 'ro').mkdir() +os.symlink('../libfoo.so.1', FROMDIR / 'ro' / 'link') +os.chmod(FROMDIR / 'ro', 0o555) +try: + checkit(['-ai', '--delay-symlinks', f'{FROMDIR}/', f'{TODIR}/'], + FROMDIR, TODIR) +finally: + os.chmod(FROMDIR / 'ro', 0o755) + if os.path.isdir(TODIR / 'ro'): + os.chmod(TODIR / 'ro', 0o755) + +# --dry-run creates nothing. +seed_tree() +run_rsync('-a', '--delay-symlinks', '--dry-run', f'{FROMDIR}/', f'{TODIR}/') +if os.path.lexists(TODIR): + test_fail("--dry-run created the destination") diff --git a/testsuite/rrsync-no-overwrite-delay-symlinks_test.py b/testsuite/rrsync-no-overwrite-delay-symlinks_test.py new file mode 100644 index 000000000..d9cadfecd --- /dev/null +++ b/testsuite/rrsync-no-overwrite-delay-symlinks_test.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""rrsync accepts --delay-symlinks, and -no-overwrite refuses it. + +The deferred symlinks are created at the end of the transfer, replacing +whatever has appeared at that name since the --ignore-existing check, so +-no-overwrite has to refuse the option as it does --delay-updates.""" + +import os +import shlex +import signal +import subprocess +import sys + +signal.signal(signal.SIGUSR1, signal.SIG_IGN) +signal.signal(signal.SIGUSR2, signal.SIG_IGN) +if '--shell' in sys.argv: + i = sys.argv.index('--shell') + 2 + env = {**os.environ, 'SSH_ORIGINAL_COMMAND': ' '.join(sys.argv[i:])} + signal.signal(signal.SIGUSR1, signal.SIG_DFL) + signal.signal(signal.SIGUSR2, signal.SIG_DFL) + wrapper, root = env['RRSYNC_WRAPPER'], env['RRSYNC_ROOT'] + flags = env.get('RRSYNC_FLAGS', '-wo -no-overwrite -no-lock').split() + os.execve(wrapper, [wrapper, *flags, root], env) + +from rsyncfns import ( + RSYNC, SCRATCHDIR, forced_protocol, makepath, patched_rrsync, rmtree, + rsync_argv, rsync_path_arg, test_skipped, +) + +proto = forced_protocol() +if proto is not None and proto < 29: + test_skipped(f"--delay-symlinks requires protocol 29+ (negotiated {proto})") + +base = SCRATCHDIR / 'rrsync-no-overwrite-delay-symlinks' +rmtree(base) +src = base / 'src' +makepath(src) +(src / 'target').write_bytes(b'NEW') +os.symlink('target', src / 'link') + +# RSYNC may be a multi-word command (the runner's --protocol=N, or valgrind) +# while rrsync hands its RSYNC to execlp() as a single executable name, so it +# has to be wrapped before patched_rrsync() sees it. +shim = base / 'rsync-shim' +shim.write_text('#!/bin/sh\nexec ' + rsync_path_arg(RSYNC) + ' "$@"\n') +shim.chmod(0o755) +wrapper = patched_rrsync(base, rsync_path=str(shim)) +rsh = f'{shlex.quote(sys.executable)} {shlex.quote(os.path.abspath(__file__))} --shell' + + +def push(root, flags): + rmtree(root) + makepath(root) + env = {**os.environ, 'RRSYNC_WRAPPER': str(wrapper), 'RRSYNC_ROOT': str(root), + 'RRSYNC_FLAGS': flags} + return subprocess.run( + rsync_argv('-rl', '--delay-symlinks', '-e', rsh, f'{src}/', 'ignored:'), + env=env, capture_output=True, text=True, timeout=20) + + +root = base / 'root' +got = push(root, '-wo -no-overwrite -no-lock') +assert got.returncode != 0 and 'option --delay-symlinks has been disabled' in got.stderr, ( + f'rrsync -no-overwrite accepted --delay-symlinks: rc={got.returncode}, ' + f'stdout={got.stdout!r}, stderr={got.stderr!r}') +assert not os.listdir(root), f'a refused transfer wrote into the root: {os.listdir(root)}' + +# The refusal must be CONDITIONAL on -no-overwrite, and an ordinary rrsync +# must accept the option: run the same push through a wrapper without it. +plain_root = base / 'root-plain' +plain = push(plain_root, '-wo -no-lock') +assert plain.returncode == 0, ( + 'without -no-overwrite rrsync must accept --delay-symlinks: ' + f'rc={plain.returncode}, stderr={plain.stderr!r}') +assert os.readlink(plain_root / 'link') == 'target', 'the symlink was not transferred' +assert (plain_root / 'target').read_bytes() == b'NEW', 'the target was not transferred' From 3f46747856c249df45f8f855d5e0ebafdb2bf2d3 Mon Sep 17 00:00:00 2001 From: rajputa-deshaw <142570235+rajputa-deshaw@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:51:10 +0000 Subject: [PATCH 2/4] Document --delay-symlinks limits and test how it is passed to the server The man page now says the option narrows the inconsistency window rather than making the update atomic, that an interrupted transfer leaves new symlinks missing and changed ones at their old targets, and that --delete-before can also remove an old target early. Add delay-symlinks-remote_test.py. Through a remote shell that records the server command, it checks that the option is passed to a remote receiver on a push but not to a remote sender on a pull, and that incremental recursion is off in both directions. It also checks that --protocol=28 fails with the protocol error, and that a push to an older rsync (old_versions/rsync_3.4.1) is refused before anything is transferred while a pull from it works. --- rsync.1.md | 11 ++- testsuite/delay-symlinks-remote_test.py | 124 ++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 testsuite/delay-symlinks-remote_test.py diff --git a/rsync.1.md b/rsync.1.md index 699986926..6f51140cd 100644 --- a/rsync.1.md +++ b/rsync.1.md @@ -3756,10 +3756,15 @@ sign) if you want the local shell to expand it. for the rest of the transfer it can point at a file that hasn't arrived yet. + This narrows the window in which the destination is inconsistent; it does + not make the update atomic. A transfer that is interrupted before the end + leaves new symlinks missing and changed symlinks pointing at their old + targets. + If a symlink is moved to a new target and the old target is deleted, also - use [`--delete-delay`](#opt) or [`--delete-after`](#opt), since the default - [`--delete-during`](#opt) can remove the old target while the old symlink - still points at it. + use [`--delete-delay`](#opt) or [`--delete-after`](#opt), since + [`--delete-before`](#opt) and the default [`--delete-during`](#opt) can + remove the old target while the old symlink still points at it. This option implies [`--no-inc-recursive`](#opt) since it needs the full file list in memory in order to be able to iterate over it at the end. diff --git a/testsuite/delay-symlinks-remote_test.py b/testsuite/delay-symlinks-remote_test.py new file mode 100644 index 000000000..925f0c699 --- /dev/null +++ b/testsuite/delay-symlinks-remote_test.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +# Coverage of how --delay-symlinks is passed between client and server. +# +# Only the receiving side defers symlinks, so server_options() passes the +# option to a remote receiver (a push) but not to a remote sender (a pull). +# Either way the receiver turns off incremental recursion, which -v shows by +# not printing "sending/receiving incremental file list". Below protocol 29 +# the option is an error, and a remote receiver too old to know the option +# rejects it before anything is transferred. + +import os +import subprocess +import sys + +from rsyncfns import ( + FROMDIR, SCRATCHDIR, SRCDIR, TODIR, + forced_protocol, rmtree, rsh_cmd, rsync_path_arg, run_rsync, test_fail, + test_skipped, +) + +proto = forced_protocol() +if proto is not None and proto < 29: + test_skipped(f"--delay-symlinks requires protocol 29+ (negotiated {proto})") +# Incremental recursion needs protocol 30, so below that there's none to +# turn off. +inc_recurse = proto is None or proto >= 30 + +# A remote shell that records the command it is asked to run, then runs it +# the way support/lsh.sh does. +LOG = SCRATCHDIR / 'remote-commands' +RSH = SCRATCHDIR / 'logging-rsh' +RSH.write_text(f'''#!{sys.executable} +import os, sys +with open({str(LOG)!r}, 'a') as fh: + fh.write(' '.join(sys.argv[1:]) + '\\n') +os.execv('/bin/sh', ['/bin/sh', {str(SRCDIR / 'support' / 'lsh.sh')!r}, + *sys.argv[1:]]) +''') +RSH.chmod(0o755) + +rmtree(FROMDIR) +(FROMDIR / 'sub').mkdir(parents=True) +(FROMDIR / 'sub' / 'target').write_text("target\n") +os.symlink('target', FROMDIR / 'sub' / 'link') + + +def remote_run(direction, *opts, rsync_path=None): + """Push or pull FROMDIR to TODIR through the logging remote shell. + Returns the finished process and the remote command it ran.""" + rmtree(TODIR) + if LOG.exists(): + LOG.unlink() + if direction == 'push': + paths = (f'{FROMDIR}/', f'lh:{TODIR}/') + else: + paths = (f'lh:{FROMDIR}/', f'{TODIR}/') + proc = run_rsync('-rlv', *opts, f'--rsh={rsh_cmd(str(RSH))}', + f'--rsync-path={rsync_path or rsync_path_arg()}', *paths, + check=False, capture_output=True) + return proc, LOG.read_text() if LOG.exists() else '' + + +for direction in ('push', 'pull'): + # Control: without the option the transfer recurses incrementally. + proc, remote = remote_run(direction) + if proc.returncode != 0: + test_fail(f"{direction}: control transfer exited {proc.returncode}:\n" + f"{proc.stderr}") + if inc_recurse and 'incremental file list' not in proc.stdout: + test_fail(f"{direction}: the control transfer didn't recurse " + f"incrementally, so the check below proves nothing:\n" + f"{proc.stdout}") + + proc, remote = remote_run(direction, '--delay-symlinks') + if proc.returncode != 0: + test_fail(f"{direction}: transfer exited {proc.returncode}:\n" + f"{proc.stderr}") + if 'incremental file list' in proc.stdout: + test_fail(f"{direction}: --delay-symlinks didn't turn off incremental " + f"recursion:\n{proc.stdout}") + if ('--delay-symlinks' in remote) != (direction == 'push'): + whom = 'remote receiver' if direction == 'push' else 'remote sender' + test_fail(f"{direction}: the option should be passed only to a remote " + f"receiver, but the {whom} was run as: {remote}") + if os.readlink(TODIR / 'sub' / 'link') != 'target': + test_fail(f"{direction}: the symlink wasn't transferred") + +# Below protocol 29 the option is an error, locally and for a push. +for paths in ((f'{FROMDIR}/', f'{TODIR}/'), (f'{FROMDIR}/', f'lh:{TODIR}/')): + rmtree(TODIR) + proc = run_rsync('-rl', '--delay-symlinks', '--protocol=28', + f'--rsh={rsh_cmd()}', f'--rsync-path={rsync_path_arg()}', + *paths, check=False, capture_output=True) + if (proc.returncode != 2 + or 'requires protocol 29 or higher' not in proc.stderr): + test_fail(f"--protocol=28 to {paths[1]}: expected the protocol error " + f"(exit 2), got exit {proc.returncode}:\n{proc.stderr}") + if os.path.lexists(TODIR / 'sub' / 'link'): + test_fail(f"--protocol=28 to {paths[1]}: the transfer went ahead") + +# A remote rsync from before the option rejects it when it is the receiver, +# before anything is transferred, and isn't sent it when it is the sender. +old = SRCDIR / 'old_versions' / 'rsync_3.4.1' +try: + runnable = subprocess.run([str(old), '--version'], capture_output=True, + timeout=10).returncode == 0 +except (OSError, subprocess.TimeoutExpired): + runnable = False +if runnable: + old_path = rsync_path_arg(str(old)) + proc, remote = remote_run('push', '--delay-symlinks', rsync_path=old_path) + if (proc.returncode == 0 + or '--delay-symlinks: unknown option' not in proc.stderr): + test_fail(f"push to rsync 3.4.1: expected it to reject the option, got " + f"exit {proc.returncode}:\n{proc.stderr}") + if os.path.lexists(TODIR / 'sub'): + test_fail("push to rsync 3.4.1: files were transferred anyway") + + proc, remote = remote_run('pull', '--delay-symlinks', rsync_path=old_path) + if proc.returncode != 0: + test_fail(f"pull from rsync 3.4.1: exited {proc.returncode}:\n" + f"{proc.stderr}") + if os.readlink(TODIR / 'sub' / 'link') != 'target': + test_fail("pull from rsync 3.4.1: the symlink wasn't transferred") From 11e99ec932494c6957f7abf5c94d96b7c43a7121 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 11 Oct 2026 13:35:20 +1000 Subject: [PATCH 3/4] testsuite: seed delay-symlinks fixture with rsync --- testsuite/delay-symlinks_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/testsuite/delay-symlinks_test.py b/testsuite/delay-symlinks_test.py index 6dbfc6163..c09f2d9d9 100644 --- a/testsuite/delay-symlinks_test.py +++ b/testsuite/delay-symlinks_test.py @@ -14,7 +14,6 @@ # with it -- no timing is involved. import os -import shutil import subprocess import sys import time @@ -288,7 +287,7 @@ def seed_tree(): (TODIR / 'current' / 'sub' / 'file').write_text("in the way\n") plain = SCRATCHDIR / 'to-plain' rmtree(plain) -shutil.copytree(TODIR, plain, symlinks=True) +run_rsync('-a', f'{TODIR}/', f'{plain}/') def deletion_stats(*args): From 5c97710f3ca84375b3eb90f567f43a150a3ff876 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 11 Oct 2026 13:02:30 +1000 Subject: [PATCH 4/4] NEWS: document --delay-symlinks --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 3f0b2ae71..936b6c00a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,8 @@ `B/s`. Example output includes `46.7M`, `48.8Mi`, `782,448B` and `113,295B/s`. File-list, created-directory and skipped-deletion counts also use thousands separators. +- Added `--delay-symlinks` to defer creating new and changed symlinks until + other transferred files are in place. ------------------------------------------------------------------------------