On Fri, Feb 25, 2022 at 04:14:13PM -0600, Eric Sandeen wrote:
> On 1/19/22 7:32 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Ted Ts'o reported brittleness in the fstests logic in generic/45[34] to
> > detect whether or not xfs_scrub is capable of detecting Unicode mischief
> > in directory and xattr names. This is a compile-time feature, since we
> > do not assume that all distros will want to ship xfsprogs with libicu.
> >
> > Rather than relying on ldd tests (which don't work at all if xfs_scrub
> > is compiled statically), let's have -V print whether or not the feature
> > is built into the tool. Phase 5 still requires the presence of "UTF-8"
> > in LC_MESSAGES to enable Unicode confusable detection; this merely makes
> > the feature easier to discover.
> >
> > Reported-by: Theodore Ts'o <tytso@xxxxxxx>
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > v2: correct the name of the reporter
> > ---
>
> Hum, every single other utility just does "$progname version $version"
> and I'm not that keen to tack on something for everyone, if it won't
> really mean anything to anyone except xfstests scripts ;)
>
> What about adding an "-F" to display features, and xfstests can use that,
> and xfs_scrub -V will keep acting like every other utility?
>
> Other utilities could use this too if we ever cared (though xfs_db
> and xfs_io already have an "-F" option ... we could choose -Z for
> featureZ, which is unused as a primary option anywhere ...)
>
> like so:
>
> ===
>
> diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8
> index e881ae76..65d8f4a2 100644
> --- a/man/man8/xfs_scrub.8
> +++ b/man/man8/xfs_scrub.8
> @@ -8,7 +8,7 @@ xfs_scrub \- check and repair the contents of a mounted XFS filesystem
> ]
> .I mount-point
> .br
> -.B xfs_scrub \-V
> +.B xfs_scrub \-V | \-F
> .SH DESCRIPTION
> .B xfs_scrub
> attempts to check and repair all metadata in a mounted XFS filesystem.
> @@ -76,6 +76,9 @@ If
> is given, no action is taken if errors are found; this is the default
> behavior.
> .TP
> +.B \-F
> +Prints the version number along with optional build-time features and exits.
> +.TP
> .B \-k
> Do not call TRIM on the free space.
> .TP
> diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
> index bc2e84a7..9e9a098c 100644
> --- a/scrub/xfs_scrub.c
> +++ b/scrub/xfs_scrub.c
> @@ -582,6 +582,13 @@ report_outcome(
> }
> }
>
> +/* Compile-time features discoverable via version strings */
> +#ifdef HAVE_LIBICU
> +# define XFS_SCRUB_HAVE_UNICODE "+"
> +#else
> +# define XFS_SCRUB_HAVE_UNICODE "-"
> +#endif
> +
> int
> main(
> int argc,
> @@ -613,7 +620,7 @@ main(
> pthread_mutex_init(&ctx.lock, NULL);
> ctx.mode = SCRUB_MODE_REPAIR;
> ctx.error_action = ERRORS_CONTINUE;
> - while ((c = getopt(argc, argv, "a:bC:de:km:nTvxV")) != EOF) {
> + while ((c = getopt(argc, argv, "a:bC:de:Fkm:nTvxV")) != EOF) {
> switch (c) {
> case 'a':
> ctx.max_errors = cvt_u64(optarg, 10);
> @@ -654,6 +661,12 @@ main(
> usage();
> }
> break;
> + case 'F':
> + fprintf(stdout, _("%s version %s %sUnicode\n"),
> + progname, VERSION,
> + XFS_SCRUB_HAVE_UNICODE);
> + fflush(stdout);
> + return SCRUB_RET_SUCCESS;
Works for me!
--D
> case 'k':
> want_fstrim = false;
> break;
