Re: cvs -- sanity.sh invalid option nit

[David Taylor]

Mark D. Baushke <mdb@gnu.org> wrote:

> Hi David,
> 
> David Taylor <dtaylor@emc.com> writes:
> 
> > In the file sanity.sh in cvs's src directory (on the trunk), test
> > basicb-21, there's the line:
> > 
> >     "admin: invalid option -- H
> > 
> > but what is actually printed by getopt in glibc (and hence by cvs) is:
> > 
> >     "admin: invalid option -- 'H'
> > 
> > (note the single quotes around the H).  This causes the test to fail.
> > 
> > Glibc has had the single quotes since, at least, release 2.9; the
> > current release is 2.13.
> 
> Interesting. The most recent release of GNU/Linux I have access to
> (RHEL 4) does not have that change...

I haven't kept up on numbering of Red Hat Enterprise Linux.  Is RHEL 4
recent?  My desktop is Ubuntu 10.10 (Ubuntu releases numbers are YY.MM
for a release in month MM of year 20YY.

> In addition, the 'compatibility' functions in the file lib/getopt.c
> in the CVS distribution does not have the quotes it uses this printf
> string:
> 
>     "%s: illegal option -- %c\n"

The test accepts both illegal option and invalid option.  Based on
comments, I believe that the change from illegal option to invalid
option was done for posix compatibility.

Assuming that there are systems out there that still say illegal option
rather than invalid option, then how about something like replacing

    admin: invalid option -- H

with something like

    admin: invalid option -- '*H'*

(assuming that expr doesn't accept ? meaning 0 or 1 times)

> which means that the src/sanity.sh is correct for a great many
> platforms right now.

Well, glibc 2.9 came out, based on the file date on the *.tar.gz file on
ftp.gnu.org, in Feb 2009.  I don't know how far back the change goes.
2.8?  2.7?  earlier?

> I suppose we could hack the m4/getopt.m4 script to test to see if the
> native OS has a getopt which returns an error with the error character
> in quotes or not and then 'fix' our lib/getopt.c to do the right thing.
> 
> The alternative would be to declare the glibc 2.9+ versions to be the
> 'buggy' ones due to incompatible behavior... tricky problem.
> 
>       -- Mark

My vote (for what little it's worth) is:

. leave glibc alone -- I consider the change a good one since it
'highlights' what option is invalid / illegal.

. change cvs's lib/getopt.c to use

    "%s: invalid option -- '%c'\n"

since posix wants 'invalid' rather than 'illegal' and I consider the
single quotes to be "a good thing".

. change the regular expression in sanity.sh to accept both

    "%s: invalid option -- '%c'\n"
and
    "%s: invalid option -- %c\n"

by replacing H with '*H'*

(Unless expr accepts ?, then I'd replace the *'s with ?'s.  I haven't
tried either; I just put the quotes in for local usage.)

Then,

    "%s: invalid option -- '%c'\n"
    "%s: invalid option -- %c\n"
    "%s: illegal option -- %c\n"

will all be matched.

Then I can get back to trying to figure out why the testsuite has timing
issues (hopefully before I get yanked onto something else).  For files
outside of the CVS directories, does cvs ever use stdio?  The testsuite
failures I'm seeing are making it hard for me to tell whether I've fixed
the bug I'm chasing.