All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] generic/556: Filter touch error message
@ 2022-05-19 22:08 Gabriel Krisman Bertazi
  2022-05-19 23:19 ` Darrick J. Wong
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-05-19 22:08 UTC (permalink / raw)
  To: zlang; +Cc: fstests, ebiggers, djwong, tytso, Gabriel Krisman Bertazi

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@collabora.com>

---
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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] generic/556: Filter touch error message
  2022-05-19 22:08 [PATCH v2] generic/556: Filter touch error message Gabriel Krisman Bertazi
@ 2022-05-19 23:19 ` Darrick J. Wong
  0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2022-05-19 23:19 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: zlang, fstests, ebiggers, tytso

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@collabora.com>

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@kernel.org>

--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
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-19 23:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 22:08 [PATCH v2] generic/556: Filter touch error message Gabriel Krisman Bertazi
2022-05-19 23:19 ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.