Pádraig Brady <[email protected]> writes:
> I'd s/EEXIST/ENOENT/ in the commit summary.
Oops, that is a silly mistake...
> I'd mention the error in the commit message to
> help folks looking for a solution, something like:
>
> mv: cannot exchange 'exist' and 'missing': Unknown error -1
>
> I'd also adjust the source comment like:
>
> diff --git a/src/copy.c b/src/copy.c
> index cfd9deaad..273ed676f 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -1749,9 +1749,8 @@ copy_internal (char const *src_name, char const
> *dst_name,
> }
> else if (errno == ENOENT)
> {
> - /* Using renameatu would fail with ENOENT as well.
> - Prefer the error message describing the failed rename
> - instead of the failed stat. */
> + /* Using RENAME_EXCHANGE would fail with ENOENT as well,
> + so set accordingly for that implied but skipped renamatu().
> */
> if (x->exchange)
> rename_errno = ENOENT;
> new_dst = true;
Makes sense.
> The tests may fail where RENAME_EXCHANGE returns ENOTSUP I think.
> In that case it's better to use a separate directory for the ENOENT test:
>
> diff --git a/tests/mv/mv-exchange.sh b/tests/mv/mv-exchange.sh
> index cd237dcb0..42846c638 100755
> --- a/tests/mv/mv-exchange.sh
> +++ b/tests/mv/mv-exchange.sh
> @@ -49,9 +49,10 @@ compare /dev/null out || fail=1
> compare exp-err err || fail=1
>
> # Test the behavior with a missing destination in a target directory.
> -echo "mv: cannot exchange 'b' and 'a/b': $ENOENT" >exp-err ||
> framework_failure_
> +mkdir d || framework_failure_
> +echo "mv: cannot exchange 'b' and 'd/b': $ENOENT" >exp-err ||
> framework_failure_
> for opt in '' --target-directory=; do
> - returns_ 1 mv --exchange b ${opt}a >out 2>err || fail=1
> + returns_ 1 mv --exchange b ${opt}d >out 2>err || fail=1
> compare /dev/null out || fail=1
> compare exp-err err || fail=1
> done
Nice catch. I pushed the patch with those changes.
Closing this report now. Thanks again, Igor.
Collin
