Skip to content

shell: measure the --sync directory depth on the host's own path - #5340

Merged
unsuman merged 1 commit into
lima-vm:masterfrom
jandubois:shell-sync-depth
Jul 29, 2026
Merged

shell: measure the --sync directory depth on the host's own path#5340
unsuman merged 1 commit into
lima-vm:masterfrom
jandubois:shell-sync-depth

Conversation

@jandubois

@jandubois jandubois commented Jul 26, 2026

Copy link
Copy Markdown
Member

--sync refuses a directory too close to the filesystem root, but on Windows it measured a path already converted to the /c/... form while still splitting on backslashes. Every directory counted as depth 1, so --sync could not run there at all. Measuring the native path keeps one threshold meaning the same thing everywhere.

Only Windows separates on a backslash. Elsewhere it is an ordinary filename character, and counting it as a separator would score a directory just below the root deep enough to pass, so the caller now says which form it holds rather than leaving it implied.

That guard was also the only thing stopping --sync when the conversion failed or returned nothing, because the empty string scored 1. Both now need their own error. An empty directory reaches rsync as the toolchain root, and with stdout not a terminal the sync back runs --delete against it unprompted.

A wsl2 guest already reaches the host directory through the /mnt automount, so --sync cannot isolate it from host files there. It now refuses that combination up front.

This is pre-existing and unrelated to the native Windows OpenSSH series. No test caught it because the BATS --sync tests run only on ubuntu-24.04 and the Windows jobs never invoke --sync. Running BATS on Windows needs submodules, an rsync install, and a vm-type hook in ensure_instance, so that waits for the plain-Windows CI work.

Assisted-by: Claude Opus 5

`--sync` refuses a directory too close to the filesystem root, but on
Windows it measured a path already converted to the /c/... form while
still splitting on backslashes. Every directory counted as depth 1, so
`--sync` could not run there at all. Measuring the native path keeps one
threshold meaning the same thing everywhere.

Only Windows separates on a backslash. Elsewhere it is an ordinary
filename character, and counting it as a separator would score a
directory just below the root deep enough to pass, so the caller now
says which form it holds rather than leaving it implied.

That guard was also the only thing stopping `--sync` when the conversion
failed or returned nothing, because the empty string scored 1. Both now
need their own error. An empty directory reaches rsync as the toolchain
root, and with stdout not a terminal the sync back runs `--delete`
against it unprompted.

A wsl2 guest already reaches the host directory through the /mnt
automount, so `--sync` cannot isolate it from host files there. It now
refuses that combination up front.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
@jandubois

Copy link
Copy Markdown
Member Author

@jandubois
jandubois marked this pull request as ready for review July 28, 2026 00:53
@AkihiroSuda
AkihiroSuda requested a review from unsuman July 29, 2026 08:04
@AkihiroSuda AkihiroSuda added this to the v2.3.0 milestone Jul 29, 2026

@unsuman unsuman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@unsuman
unsuman merged commit de965a3 into lima-vm:master Jul 29, 2026
36 checks passed
@jandubois
jandubois deleted the shell-sync-depth branch July 29, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants