bug#33468: A bug with yes and --help

[Eric Blake]

On 2/13/19 6:56 PM, Assaf Gordon wrote:
> Hello,
> 
> On 2019-02-12 7:00 p.m., Eric Blake wrote:
>> On 2/12/19 7:21 PM, Assaf Gordon wrote:
>>
>>> +  optind = 1;
>>
>> Why are you doing this in every caller, instead of doing it just once
>> inside the body of parse_gnu_standard_options_only(), so that the state
>> is left unchanged at optind==1 if there were no options parsed?
> 
> That was just an ugly hack.
> 
> Here are a more complete patches (both for gnulib and for coreutils).
> 
> All existing tests pass (including nohup's exit code)
> but I did not yet write new tests for these improvements.
> 
> Comments welcomed.
>  -assaf
> 

> From 089e732b25f02c564e9606e8be7d2ab0b6caff98 Mon Sep 17 00:00:00 2001
> From: Bernhard Voelker <address@hidden>
> Date: Thu, 29 Nov 2018 09:06:26 +0100
> Subject: [PATCH] long-options: add parse_gnu_standard_options_only
> 
> Discussed in https://bugs.gnu.org/33468 .
> 
> * lib/long-options.c (parse_long_options): Use EXIT_SUCCESS instead
> of 0.
> (parse_gnu_standard_options_only): Add function to
> process the GNU default options --help and --version (see also [1]),
> and fail for any other unknown long or short option.
> * lib/long-options.h (parse_gnu_standard_options_only): Declare it.
> [1]
> https://gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html

Putting a [1] footnote in the middle of the ChangeLog section looks odd;
did you mean to sink it lower...

> * modules/long-options (depends-on): Add stdbool, extifail.

exitfail

> * top/maint.mk (sc_prohibit_long_options_without_use): Update
> syntax-check rule, add new function name.
...to here?

> @@ -87,3 +88,65 @@ parse_long_options (int argc,
>       the probably-new parameters when/if getopt is called later.  */
>    optind = 0;
>  }
> +
> +/* Process the GNU default long options --help and --version (see also
> +   
> https://gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html),
> +   and fail for any other unknown long or short option.
> +   Use with SCAN_ALL=true to scan until "--", or with SCAN_ALL=false to stop
> +   at the first non-option argument (or "--", whichever comes first).
> +   if RESET_OPTIND==TRUE, the global optind variable will be reset to zero,

RESET_OPTIND=true, to match your style of SCAN_ALL=true above

> +   preparing (and requiring) a follow-up getopt_long(3) call.

Not just any getopt_long call, but the glibc-flavored getopt_gnu() [the
getopt-gnu module] (since FreeBSD native getopt_long() uses
optreset=optind=1 instead of optind=0 for a reset).

> +   if RESET_OPTIND==FALSE, optind is left as-is (suitible for programs

RESET_OPTIND=false, suitable

> +   which do not process further optional parameters (but could still
> +   process paramaters directly by examining argv[optind]).  */

parameters

> +void
> +parse_gnu_standard_options_only (int argc,
> +                                 char **argv,
> +                                 const char *command_name,
> +                                 const char *package,
> +                                 const char *version,
> +                                 bool scan_all,
> +                                 bool reset_optind,
> +                                 void (*usage_func) (int),
> +                                 /* const char *author1, ...*/ ...)
> +{

> +  /* Reset this to zero so that getopt internals get initialized from
> +     the probably-new parameters when/if getopt is called later.  */
> +  if (reset_optind)
> +    optind = 0;
> +}

Either this should depend on the getopt-gnu module, or you could do a
configure-check for optreset, and write:

#if HAVE_OPTRESET
  optind = optreset = 1;
#else
  optind = 0;
#endif

to work with both common flavors of getopt_long() adn the getopt-posix
module.

> From 46ff493a864449fae1ad5678f1c79ce4c0eea1ab Mon Sep 17 00:00:00 2001
> From: Bernhard Voelker <address@hidden>
> Date: Mon, 26 Nov 2018 09:05:37 +0100
> Subject: [PATCH] all: detect --help and --version more consistently [FIXME]
> 
> FIXME: tests
> 
> For select programs which accept only --help and --version options
> (in addition to non-options arguments), process these options before
> any other options.
> 
> Before:
> 
>   $ yes --help me
>   yes: unrecognized option '--help'
>   Try 'yes --help' for more information.
> 
>   $ yes me --help
>   me --help
>   me --help
>   ...
> 
> After:
> Any occurence of '--help' in yes's arguments will show the help screen.

occurrence

Maybe:

show the help screen, unless -- is used to mark end of options.

> 
> Discussed in https://bugs.gnu.org/33468 .
> 
> * NEWS: Mention change.
> * src/cksum.c, src/dd.c, src/hostid.c, src/hostname.c, src/link.c,
> src/logname.c, src/nohup.c, src/sleep.c, src/tsort.c, src/unlink.c,
> src/uptime.c, src/users.c, src/whoami.c, src/yes.c (main): Replace
> parse_long_options() + getopt_long() calls with
> parse_gnu_standard_options_only(); Remove <getopt.h> inclusion;
> Remove empty 'struct long_options' variable;
> ---
>  NEWS           |  4 ++++
>  src/cksum.c    | 13 +++----------
>  src/dd.c       | 14 +++-----------
>  src/hostid.c   | 13 +++----------
>  src/hostname.c | 13 +++----------
>  src/link.c     | 13 +++----------
>  src/logname.c  | 13 +++----------
>  src/nohup.c    | 13 +++----------
>  src/sleep.c    | 13 +++----------
>  src/tsort.c    | 13 +++----------
>  src/unlink.c   | 13 +++----------
>  src/uptime.c   | 13 +++----------
>  src/users.c    | 13 +++----------
>  src/whoami.c   | 13 +++----------
>  src/yes.c      | 13 +++----------
>  15 files changed, 46 insertions(+), 141 deletions(-)

Nice diffstat.

> 
> diff --git a/NEWS b/NEWS
> index fdde47593..5975c2908 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -37,6 +37,10 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>  
>  ** Changes in behavior
>  
> +  cksum,dd,hostid,hostname,link,logname,nohup,sleep,tsort,unlink,uptime,
> +  users,whoami,yes: these programs now always process --help and --version

Spaces between program names.

> +  before any other option.

Is "argument" better than "option" here?  Or, maybe:

now always process --help and --version options, regardless of any other
arguments present before any optinoal -- end-of-options marker.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org