On Thu, May 19, 2022 at 06:08:36PM -0400, Gabriel Krisman Bertazi wrote:
> Coreutils commit d435cfc0bc55 ("touch: fix wrong
> diagnostic (Bug#48106)"), released in coreutils v9.0, changed the error
> reported by the tool when openat() fails with EINVAL. Instead of
> reporting a generic message for the failure of either openat() or the
> following utimensat(), it now differentiates both failures with
> different messages.
>
> This change breaks generic/556, which relied on the parsing of that
> message. This test was originally developed by me on a Debian
> Buster (coreutils v8.x), so I used the generic error message. Now that
> I tried to run it on a more modern distro, it reports a different error
> message, which fails the test.
>
> The patch filters out the touch-specific parts of the touch error
> messages, to prevent breakage from future changes, but preserves the
> return code information, which is actually useful (and more stable).
>
> There is no change in behavior on the kernel side, just a broken test.
> On both older and new distros, the kernel correctly rejects this invalid
> sequence with -EINVAL, as shown in the strace hunk below:
>
> ...
> openat(AT_FDCWD, "/scratch_mnt/strict/corac\314\247\303", ...) = -1 EINVAL
> utimensat(AT_FDCWD, "/scratch_mnt/strict/corac\314\247\303", ...) = -1 EINVAL
> ...
>
> Tested on Debian sid (coreutils v8.32) and Fedora (coreutils 9.0).
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx>
Would you mind pasting the failure messages from 8.32 and 9.0 in the
commit message so that it's a bit more obvious how things changed?
With that added,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
>
> ---
> Changes since v1:
> - Dont break on older distros
> ---
> common/filter | 8 ++++++++
> tests/generic/556 | 9 +++++++--
> tests/generic/556.out | 4 ++--
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/common/filter b/common/filter
> index 5b20e848c9b9..a6a42b7a6ad2 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -478,6 +478,14 @@ _filter_stat()
> sed -e "s/\<cannot stat\>/cannot statx/"
> }
>
> +# touch v9.0+ modified part of the message printed on error. Filter the
> +# generic part out, but preserve the strerror() part, which is
> +# actually useful for debugging and usually stable.
> +_filter_touch()
> +{
> + sed -e "s/.* '\(.*\)':\(.*\)/touch: '\1':\2/"
> +}
> +
> _filter_lostfound()
> {
> sed -e '/^lost+found$/d'
> diff --git a/tests/generic/556 b/tests/generic/556
> index 7ef2f6f4106b..1296e241552c 100755
> --- a/tests/generic/556
> +++ b/tests/generic/556
> @@ -44,6 +44,11 @@ jp_file2=$(echo -e "japanese_\xe3\x82\xb1\xe3\x82\x99.txt")
> blob_file1=$(echo -e "corac\xcc\xa7\xc3")
> blob_file2=$(echo -e "coraç\xc3")
>
> +filter_touch()
> +{
> + _filter_touch | _filter_scratch
> +}
> +
> # Test helpers
> basic_create_lookup()
> {
> @@ -456,8 +461,8 @@ test_strict_mode_invalid_filename()
>
> # These creation commands should fail, since we are on strict
> # mode.
> - touch "${basedir}/${blob_file1}" 2>&1 | _filter_scratch
> - touch "${basedir}/${blob_file2}" 2>&1 | _filter_scratch
> + touch "${basedir}/${blob_file1}" 2>&1 | filter_touch
> + touch "${basedir}/${blob_file2}" 2>&1 | filter_touch
> }
>
> #############
> diff --git a/tests/generic/556.out b/tests/generic/556.out
> index f9dd9542fb12..a689a3e2311c 100644
> --- a/tests/generic/556.out
> +++ b/tests/generic/556.out
> @@ -12,5 +12,5 @@ user.foo="bar"
> # file: SCRATCH_MNT/xattrs/x/f1
> user.foo="bar"
>
> -touch: setting times of 'SCRATCH_MNT/strict/corac'$'\314\247\303': Invalid argument
> -touch: setting times of 'SCRATCH_MNT/strict/cora'$'\303\247\303': Invalid argument
> +touch: 'SCRATCH_MNT/strict/corac'$'\314\247\303': Invalid argument
> +touch: 'SCRATCH_MNT/strict/cora'$'\303\247\303': Invalid argument
> --
> 2.36.1
>
