Обсуждение: src/port/getopt_long.c lossy with arguments having no option characters

Hi all,

The implementation of getopt_long in src/port has some limitations,
for example such commands do not work but they should:
$ createdb foobar3 -E win1252
createdb: too many command-line arguments (first is "win1252")
Try "createdb --help" for more information.
$ initdb pgdata --noclean
initdb: too many command-line arguments (first is "--noclean")
Try "initdb --help" for more information.

And those ones work:
createdb -E win1252 foobar3
initdb --noclean pgdata

I bumped into this problem when running the TAP tests on Windows, but
I guess that it easy to reproduce it with any builds of Postgres on
Windows as getopt_long is used unconditionally, on any version, for
all commands. Or on any platform that has not getopt_long, if any
exists.
Regards,
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> The implementation of getopt_long in src/port has some limitations,
> for example such commands do not work but they should:

No, those should not work.  You're too used to versions that don't insist
on switches-before-non-switch-arguments.  Such behavior is an extension
that is not standard, is not documented in any Postgres documentation,
and tends to have bad corner-case behaviors.  I don't feel a need to make
our implementation replicate that.

            regards, tom lane

On 4/3/15 10:08 AM, Tom Lane wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> The implementation of getopt_long in src/port has some limitations,
>> for example such commands do not work but they should:
>
> No, those should not work.  You're too used to versions that don't insist
> on switches-before-non-switch-arguments.  Such behavior is an extension
> that is not standard,

It is true that options after non-option arguments are a GNU extension,
but long options are also a GNU extension.  So the behavior we provide
is neither pure POSIX nor pure GNU.  I can see how that would be confusing.

On Sat, Apr 4, 2015 at 4:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 4/3/15 10:08 AM, Tom Lane wrote:
>> Michael Paquier <michael.paquier@gmail.com> writes:
>>> The implementation of getopt_long in src/port has some limitations,
>>> for example such commands do not work but they should:
>>
>> No, those should not work.  You're too used to versions that don't insist
>> on switches-before-non-switch-arguments.  Such behavior is an extension
>> that is not standard,
>
> It is true that options after non-option arguments are a GNU extension,
> but long options are also a GNU extension.  So the behavior we provide
> is neither pure POSIX nor pure GNU.  I can see how that would be confusing.

Well, OK. Then we had better update a bit the TAP tests of initdb and
createdb because they break on any platform which is not compliant
with that.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> On Sat, Apr 4, 2015 at 4:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> It is true that options after non-option arguments are a GNU extension,
>> but long options are also a GNU extension.  So the behavior we provide
>> is neither pure POSIX nor pure GNU.  I can see how that would be confusing.

> Well, OK. Then we had better update a bit the TAP tests of initdb and
> createdb because they break on any platform which is not compliant
> with that.

If those tests are dependent on behavior we don't document as allowed,
I think we should change the tests.  Want to send a patch?

            regards, tom lane

Peter Eisentraut wrote:
> On 4/3/15 10:08 AM, Tom Lane wrote:
> > Michael Paquier <michael.paquier@gmail.com> writes:
> >> The implementation of getopt_long in src/port has some limitations,
> >> for example such commands do not work but they should:
> >
> > No, those should not work.  You're too used to versions that don't insist
> > on switches-before-non-switch-arguments.  Such behavior is an extension
> > that is not standard,
>
> It is true that options after non-option arguments are a GNU extension,
> but long options are also a GNU extension.  So the behavior we provide
> is neither pure POSIX nor pure GNU.  I can see how that would be confusing.

The thing I hate the most about this issue is how some commands fail
with "--help: unrecognized option" or some such, when called as

command --foo=bar --baz --help

I am used to such calls to emit the help, then exit, and the command is
kept in history; that way, it's easy to adjust for the info found in the
help and edit later.  As is, I tend to esc-# to save the commented-out
command in history, then call with only --help, then recall the
commented-out version.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2015-04-03 19:06:38 -0300, Alvaro Herrera wrote:
> The thing I hate the most about this issue is how some commands fail
> with "--help: unrecognized option" or some such, when called as
>
> command --foo=bar --baz --help

I think that's primarily a different issue though. For some reason I
have yet to figure out --help is usually treated explicitly. E.g.:
    if (argc > 1)
    {
        if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
        {
            usage();
            exit(0);
        }
        else if (strcmp(argv[1], "-V") == 0
                 || strcmp(argv[1], "--version") == 0)
        {
            puts("pg_basebackup (PostgreSQL) " PG_VERSION);
            exit(0);
        }
    }

    while ((c = getopt_long(argc, argv, "D:F:r:RT:xX:l:zZ:d:c:h:p:U:s:wWvP",
                            long_options, &option_index)) != -1)

Some tools then copy the --help/-? handling to the normal option parsing
(like IIRC psql) but others don't (like pg_basebackup).

Greetings,

Andres Freund

On Sat, Apr 4, 2015 at 6:47 AM, Tom Lane wrote:
> If those tests are dependent on behavior we don't document as allowed,
> I think we should change the tests.  Want to send a patch?

Attached are patches for HEAD and 9.4. There are problematic tests in
the ones of initdb, clusterdb, reindexdb and createdb.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> On Sat, Apr 4, 2015 at 6:47 AM, Tom Lane wrote:
>> If those tests are dependent on behavior we don't document as allowed,
>> I think we should change the tests.  Want to send a patch?

> Attached are patches for HEAD and 9.4. There are problematic tests in
> the ones of initdb, clusterdb, reindexdb and createdb.

Pushed, thanks.

            regards, tom lane

On 4/3/15 6:45 PM, Andres Freund wrote:
> I think that's primarily a different issue though. For some reason I
> have yet to figure out --help is usually treated explicitly.

Because at one point we did not have our own copy of getopt_long.c, so
long options might not have been available.