Thread: fix in --help output
I attach fix for --help output. I replaced --NAME... with -NAME and add some
example. getopt parse -- as a end of options and rest of line is not parsed.
This should be backported for 8.3 and 8.2 as well. PG8.1 does not have this options.
Thanks Zdenek
Index: src/backend/main/main.c
===================================================================
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/main/main.c,v
retrieving revision 1.110
diff -c -r1.110 main.c
*** src/backend/main/main.c 1 Jan 2008 19:45:49 -0000 1.110
--- src/backend/main/main.c 21 Feb 2008 17:02:16 -0000
***************
*** 288,294 ****
printf(_(" -p PORT port number to listen on\n"));
printf(_(" -s show statistics after each query\n"));
printf(_(" -S WORK-MEM set amount of memory for sorts (in kB)\n"));
! printf(_(" --NAME=VALUE set run-time parameter\n"));
printf(_(" --describe-config describe configuration parameters, then exit\n"));
printf(_(" --help show this help, then exit\n"));
printf(_(" --version output version information, then exit\n"));
--- 288,294 ----
printf(_(" -p PORT port number to listen on\n"));
printf(_(" -s show statistics after each query\n"));
printf(_(" -S WORK-MEM set amount of memory for sorts (in kB)\n"));
! printf(_(" -NAME=VALUE set run-time parameter (e.g. -shared_buffers=16384)\n"));
printf(_(" --describe-config describe configuration parameters, then exit\n"));
printf(_(" --help show this help, then exit\n"));
printf(_(" --version output version information, then exit\n"));
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> I attach fix for --help output. I replaced --NAME... with -NAME and add some
> example. getopt parse -- as a end of options and rest of line is not parsed.
Surely this is outright wrong. Or if you do have a getopt that acts
that way, the bug is that we are using it rather than one that acts
the way we want.
regards, tom lane
Zdenek Kotala wrote: > I attach fix for --help output. I replaced --NAME... with -NAME But that is wrong. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Tom Lane napsal(a): > Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: >> I attach fix for --help output. I replaced --NAME... with -NAME and add some >> example. getopt parse -- as a end of options and rest of line is not parsed. > > Surely this is outright wrong. Or if you do have a getopt that acts > that way, the bug is that we are using it rather than one that acts > the way we want. Ah, sorry it really does not work. However, I get following error on Solaris: bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db --share_buffers=16000 /usr/postgres/8.2/bin/postgres: illegal option -- share_buffers=16000 Try "postgres --help" for more information. but following command works fine: /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000 By my opinion problem is in getopt which interprets -- as a end of options list. See http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html#tag_12_02 Guideline 10 It maybe work on linux but I think it is not portable solution. Zdenek
Zdenek Kotala wrote: > It maybe work on linux but I think it is not portable solution. What we should do is avoid using Solaris' getopt_long and instead use the copy we have in src/port/. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>
>> It maybe work on linux but I think it is not portable solution.
>
> What we should do is avoid using Solaris' getopt_long and instead use
> the copy we have in src/port/.
>
If I looked correctly there is no getopt_long. There is only getopt with
- as a option. See PostmasterMain:
while ((opt = getopt(argc, argv,
"A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
If I understand correctly the POSIX standard "-" should not used in a
option list.
Zdenek
Zdenek Kotala wrote: > Tom Lane napsal(a): >> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: >>> I attach fix for --help output. I replaced --NAME... with -NAME and >>> add some example. getopt parse -- as a end of options and rest of >>> line is not parsed. >> >> Surely this is outright wrong. Or if you do have a getopt that acts >> that way, the bug is that we are using it rather than one that acts >> the way we want. > > Ah, sorry it really does not work. > > However, I get following error on Solaris: > > bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db --share_buffers=16000 > /usr/postgres/8.2/bin/postgres: illegal option -- share_buffers=16000 > Try "postgres --help" for more information. > > but following command works fine: > > /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000 > > > By my opinion problem is in getopt which interprets -- as a end of > options list. > > See > http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html#tag_12_02 > > Guideline 10 > > It maybe work on linux but I think it is not portable solution. > > > -- on its own might indicate the end of arguments, but that's quite different from --foo=bar. Guideline 10 of the above surely only refers to -- as an entire argument, not to -- as the first two characters of an argument. If your getopt treats *any* -- as the end of options then I think it is broken (complain to your vendor ;-) ). And the answer is known - use the one we have in src/port. cheers andrew
Andrew Dunstan napsal(a): > > > Zdenek Kotala wrote: >> Tom Lane napsal(a): >>> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: >>>> I attach fix for --help output. I replaced --NAME... with -NAME and >>>> add some example. getopt parse -- as a end of options and rest of >>>> line is not parsed. >>> >>> Surely this is outright wrong. Or if you do have a getopt that acts >>> that way, the bug is that we are using it rather than one that acts >>> the way we want. >> >> Ah, sorry it really does not work. >> >> However, I get following error on Solaris: >> >> bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db --share_buffers=16000 >> /usr/postgres/8.2/bin/postgres: illegal option -- share_buffers=16000 >> Try "postgres --help" for more information. >> >> but following command works fine: >> >> /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000 >> >> >> By my opinion problem is in getopt which interprets -- as a end of >> options list. >> >> See >> http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html#tag_12_02 >> >> Guideline 10 >> >> It maybe work on linux but I think it is not portable solution. >> >> >> > > -- on its own might indicate the end of arguments, but that's quite > different from --foo=bar. Guideline 10 of the above surely only refers > to -- as an entire argument, not to -- as the first two characters of an > argument. If your getopt treats *any* -- as the end of options then I > think it is broken (complain to your vendor ;-) ). And the answer is > known - use the one we have in src/port. You are not correct (I pointed to wrong Guidline). If you look to Guidline 3 it specific only alphanumeric character. And you can read "The implementation may accept other characters as an extension." in http://www.opengroup.org/onlinepubs/009695399/functions/getopt.html It means that Solaris implementation is OK. Linux uses some extensions but it is not portable. If we want to use long options. We have getop_long for it. Zdenek
Zdenek Kotala wrote: > If I looked correctly there is no getopt_long. There is only getopt with > - as a option. See PostmasterMain: > > > while ((opt = getopt(argc, argv, > "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1) > > If I understand correctly the POSIX standard "-" should not used in a > option list. Hmm, right. Our current parsing of --long-opts is quite a hack, it seems :-( Having to list all GUC options in the getopt_long array would be a mess. Any other ideas? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Zdenek Kotala wrote: > However, I get following error on Solaris: > > bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db --share_buffers=16000 > /usr/postgres/8.2/bin/postgres: illegal option -- share_buffers=16000 > Try "postgres --help" for more information. > > but following command works fine: > > /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000 share_buffers is not the same as shared_buffers. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut napsal(a):
> Zdenek Kotala wrote:
>> However, I get following error on Solaris:
>>
>> bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db --share_buffers=16000
>> /usr/postgres/8.2/bin/postgres: illegal option -- share_buffers=16000
>> Try "postgres --help" for more information.
>>
>> but following command works fine:
>>
>> /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000
>
> share_buffers is not the same as shared_buffers.
Yeah, it was a typo but it does not work anyway.
Zdenek
Am Freitag, 22. Februar 2008 schrieb Zdenek Kotala: > Peter Eisentraut napsal(a): > > Zdenek Kotala wrote: > >> However, I get following error on Solaris: > >> > >> bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db > >> --share_buffers=16000 /usr/postgres/8.2/bin/postgres: illegal option -- > >> share_buffers=16000 Try "postgres --help" for more information. > >> > >> but following command works fine: > >> > >> /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000 > > > > share_buffers is not the same as shared_buffers. > > Yeah, it was a typo but it does not work anyway. Well, if it doesn't work you can use the -c option. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>
>> If I looked correctly there is no getopt_long. There is only getopt with
>> - as a option. See PostmasterMain:
>>
>>
>> while ((opt = getopt(argc, argv,
>> "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
>>
>> If I understand correctly the POSIX standard "-" should not used in a
>> option list.
>
> Hmm, right. Our current parsing of --long-opts is quite a hack, it
> seems :-( Having to list all GUC options in the getopt_long array would
> be a mess. Any other ideas?
>
I attached patch which replaces any "--..." occurrence with "-c..." on
command line.
Zdenek
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.552
diff -c -r1.552 postmaster.c
*** src/backend/postmaster/postmaster.c 20 Feb 2008 22:46:24 -0000 1.552
--- src/backend/postmaster/postmaster.c 22 Feb 2008 09:32:15 -0000
***************
*** 483,490 ****
* Parse command-line options. CAUTION: keep this in sync with
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
*/
! while ((opt = getopt(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
{
switch (opt)
{
--- 483,511 ----
* Parse command-line options. CAUTION: keep this in sync with
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
+ *
+ * We want to support --NAME=VALUE args type, but getopt_long is not suitable
+ * for porcessing all GUC variables and we cannot use "-" as in a getopt option
+ * list, because it is not supported on some platforms.
+ * Regarding this issues a preprocessing which replaces "--" with "-c" is necessary.
*/
!
! for(int n=0; n<argc ; n++)
! {
! int arglen;
! arglen = strlen(argv[n]);
! if(arglen >= 2)
! {
! if(argv[n][0] == '-' && argv[n][1] == '-')
! {
! if(arglen == 2)
! break; // we found -- end of option list
! argv[n][1]='c';
! }
! }
! }
!
! while ((opt = getopt(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:")) != -1)
{
switch (opt)
{
***************
*** 617,623 ****
break;
case 'c':
- case '-':
{
char *name,
*value;
--- 638,643 ----
Peter Eisentraut napsal(a):
> Am Freitag, 22. Februar 2008 schrieb Zdenek Kotala:
>> Peter Eisentraut napsal(a):
>>> Zdenek Kotala wrote:
>>>> However, I get following error on Solaris:
>>>>
>>>> bash-3.2$ /usr/postgres/8.2/bin/postgres -D /tmp/db
>>>> --share_buffers=16000 /usr/postgres/8.2/bin/postgres: illegal option --
>>>> share_buffers=16000 Try "postgres --help" for more information.
>>>>
>>>> but following command works fine:
>>>>
>>>> /usr/postgres/8.2/bin/postgres -D /tmp/db -c shared_buffers=16000
>>> share_buffers is not the same as shared_buffers.
>> Yeah, it was a typo but it does not work anyway.
>
> Well, if it doesn't work you can use the -c option.
Yes, but it is confusing. For example see following message:
postgres does not know where to find the server configuration file.
You must specify the --config-file or -D invocation option or set the
PGDATA environment variable.
config-file is there mention as a hint, but it does not work and I think
there are more places where usage of "--.." is mentioned. It is
confusing for the user and there is no clue.
Zdenek
Zdenek Kotala wrote: > Alvaro Herrera napsal(a): >> Hmm, right. Our current parsing of --long-opts is quite a hack, it >> seems :-( Having to list all GUC options in the getopt_long array would >> be a mess. Any other ideas? > > I attached patch which replaces any "--..." occurrence with "-c..." on > command line. Actually I meant a way to make our current hack work on your platform. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> I attached patch which replaces any "--..." occurrence with "-c..." on
> command line.
Please see whether forcibly using src/port/getopt.c fixes this,
instead. A saner patch would probably add something like this
to configure.in:
if test "$PORTNAME" = "solaris"; then
AC_LIBOBJ(getopt)
AC_LIBOBJ(getopt_long)
fi
regards, tom lane
Tom Lane napsal(a): > Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: >> I attached patch which replaces any "--..." occurrence with "-c..." on >> command line. > > Please see whether forcibly using src/port/getopt.c fixes this, > instead. A saner patch would probably add something like this > to configure.in: > > if test "$PORTNAME" = "solaris"; then > AC_LIBOBJ(getopt) > AC_LIBOBJ(getopt_long) > fi Yeah, this is the best solution. I attach a patch. Is possible to backport it back to 8.3, 8.2? Just for completeness Solaris getopt function has a extension which processes long option argument. This extension is called CLIP. This implementation collides with unusual getopt usage for long option processing in PostgreSQL. More info are there: http://docs.sun.com/app/docs/doc/819-2243/getopt-3c?l=en&a=view http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/getopt.c http://docs.sun.com/app/docs/doc/819-2239/6n4hsf6e5?l=en&a=view By the way old solaris getopt version is similary with postgres implementation. :-) http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libbc/libc/gen/common/getopt.c Thanks for help Zdenek
Ups. Patch attached
Zdenek
Zdenek Kotala napsal(a):
> Tom Lane napsal(a):
>> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>>> I attached patch which replaces any "--..." occurrence with "-c..."
>>> on command line.
>>
>> Please see whether forcibly using src/port/getopt.c fixes this,
>> instead. A saner patch would probably add something like this
>> to configure.in:
>>
>> if test "$PORTNAME" = "solaris"; then
>> AC_LIBOBJ(getopt)
>> AC_LIBOBJ(getopt_long)
>> fi
>
> Yeah, this is the best solution. I attach a patch. Is possible to
> backport it back to 8.3, 8.2?
>
> Just for completeness Solaris getopt function has a extension which
> processes long option argument. This extension is called CLIP. This
> implementation collides with unusual getopt usage for long option
> processing in PostgreSQL.
>
> More info are there:
> http://docs.sun.com/app/docs/doc/819-2243/getopt-3c?l=en&a=view
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/getopt.c
>
> http://docs.sun.com/app/docs/doc/819-2239/6n4hsf6e5?l=en&a=view
>
> By the way old solaris getopt version is similary with postgres
> implementation. :-)
>
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libbc/libc/gen/common/getopt.c
>
>
>
> Thanks for help
> Zdenek
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
Index: configure.in
===================================================================
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/configure.in,v
retrieving revision 1.551
diff -c -r1.551 configure.in
*** configure.in 19 Feb 2008 01:05:28 -0000 1.551
--- configure.in 22 Feb 2008 20:16:05 -0000
***************
*** 1102,1107 ****
--- 1102,1115 ----
AC_LIBOBJ(getopt_long)
fi
+ # Solaris has getopt implementation which is not compatible with unusual
+ # parsing for long options used in PostgreSQL. Replace it by own implementation
+ # is best solution how to avoid complication.
+ if test "$PORTNAME" = "solaris"; then
+ AC_LIBOBJ(getopt)
+ AC_LIBOBJ(getopt_long)
+ fi
+
# Win32 support
if test "$PORTNAME" = "win32"; then
AC_REPLACE_FUNCS(gettimeofday)
Zdenek Kotala wrote: > Ups. Patch attached Tom has applied this patch to CVS HEAD and all relevant back-branches. --------------------------------------------------------------------------- > > Zdenek > > Zdenek Kotala napsal(a): > > Tom Lane napsal(a): > >> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > >>> I attached patch which replaces any "--..." occurrence with "-c..." > >>> on command line. > >> > >> Please see whether forcibly using src/port/getopt.c fixes this, > >> instead. A saner patch would probably add something like this > >> to configure.in: > >> > >> if test "$PORTNAME" = "solaris"; then > >> AC_LIBOBJ(getopt) > >> AC_LIBOBJ(getopt_long) > >> fi > > > > Yeah, this is the best solution. I attach a patch. Is possible to > > backport it back to 8.3, 8.2? > > > > Just for completeness Solaris getopt function has a extension which > > processes long option argument. This extension is called CLIP. This > > implementation collides with unusual getopt usage for long option > > processing in PostgreSQL. > > > > More info are there: > > http://docs.sun.com/app/docs/doc/819-2243/getopt-3c?l=en&a=view > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/getopt.c > > > > http://docs.sun.com/app/docs/doc/819-2239/6n4hsf6e5?l=en&a=view > > > > By the way old solaris getopt version is similary with postgres > > implementation. :-) > > > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libbc/libc/gen/common/getopt.c > > > > > > > > Thanks for help > > Zdenek > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: Don't 'kill -9' the postmaster > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +