copytool: report a bad path instead of a missing tool - #5337
Open
jandubois wants to merge 1 commit into
Open
Conversation
jandubois
force-pushed
the
copytool-parse-error
branch
from
July 26, 2026 07:18
ef49f82 to
61eae7f
Compare
This was referenced Jul 26, 2026
jandubois
force-pushed
the
copytool-parse-error
branch
from
July 27, 2026 05:31
61eae7f to
749ad63
Compare
A path naming an instance that does not exist reached the user as "neither rsync nor scp found on host", because the auto backend discarded the parse error, skipped rsync, and reported whatever the scp fallback said. Only a host without scp saw that message; everywhere else the auto backend built the copy tool without complaint and the real error surfaced later, from the copy command itself. The explicit rsync backend reduced the same error to "rsync not available on guest(s)". The fallback message no longer guesses which tool is missing, since rsync may well have been found and rejected for another reason. Signed-off-by: Jan Dubois <jan.dubois@suse.com>
jandubois
force-pushed
the
copytool-parse-error
branch
from
July 27, 2026 05:35
749ad63 to
b2eb831
Compare
Member
Author
|
Ready for review. AI review: https://jandubois.github.io/lima/20260726-220537-pr-5337.html The commit message has been updated, and the branch has been rebased since the AI review, but the changes are identical. I don't think any findings are actionable. |
AkihiroSuda
reviewed
Aug 1, 2026
AkihiroSuda
left a comment
AkihiroSuda
left a comment
Member
There was a problem hiding this comment.
This kind of PR could be reviewed easier if the console output were attached
AkihiroSuda
reviewed
Aug 1, 2026
| paths := []string{"nonexistent-instance-for-test:/tmp/x", "/tmp/y"} | ||
|
|
||
| _, err := New(t.Context(), string(BackendAuto), paths, &Options{}) | ||
| assert.ErrorContains(t, err, "instance `nonexistent-instance-for-test`") |
Member
There was a problem hiding this comment.
This should assert the reason of the error, not just the instance name involved in the error?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A path naming an instance that does not exist reached the user as "neither rsync nor scp found on host", because the auto backend discarded the parse error, skipped rsync, and reported whatever the scp fallback said. Only a host without scp saw that message; everywhere else the auto backend built the copy tool without complaint and the real error surfaced later, from the copy command itself. The explicit rsync backend reduced the same error to "rsync not available on guest(s)".
The fallback message no longer guesses which tool is missing, since rsync may well have been found and rejected for another reason.
The fix touches no Windows code and is independent of the native Windows OpenSSH series, so it can merge in any order. But #5299 appends a test to the end of
pkg/copytool/copytool_test.gotoo, so whichever lands second needs a one-line rebase there.Assisted-by: Claude Opus 5