Re: [PATCH v2 12/17] xfs_scrub: report optional features in version string

["Darrick J. Wong" <djwong@kernel.org>]

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@kernel.org>
> > 
> > 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@mit.edu>
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > 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;