Re: Have configure complain about unknown options

> -----Original Message-----
> From: pgsql-patches-owner(at)postgresql(dot)org
> [mailto:pgsql-patches-owner(at)postgresql(dot)org] On Behalf Of Marko Kreen
> Sent: 05 May 2006 12:22
> To: Tom Lane
> Cc: Martijn van Oosterhout; pgsql-patches(at)postgresql(dot)org
> Subject: Re: [PATCHES] [PATCH] Have configure complain about
> unknown options
>
> As PostgreSQL tree is not set up that way, I think for
> clarity sake it would be better to give explicit errors for
> unknown options.
>

I'm not in a position to argue about why autoconf works this way, but I
can say that I'd like to see unsupported options rejected if there is a
sensible way to do it. I've been bitten more than once by mistakenly
using --enable-foo rather than --with-foo, or just plain mis-typing.

Regards, Dave.

I am thinking we would need an option at the start like --strict that
would throw an error for any later invalid options.

---------------------------------------------------------------------------

Dave Page wrote:
>
>
> > -----Original Message-----
> > From: pgsql-patches-owner(at)postgresql(dot)org
> > [mailto:pgsql-patches-owner(at)postgresql(dot)org] On Behalf Of Marko Kreen
> > Sent: 05 May 2006 12:22
> > To: Tom Lane
> > Cc: Martijn van Oosterhout; pgsql-patches(at)postgresql(dot)org
> > Subject: Re: [PATCHES] [PATCH] Have configure complain about
> > unknown options
> >
> > As PostgreSQL tree is not set up that way, I think for
> > clarity sake it would be better to give explicit errors for
> > unknown options.
> >
>
> I'm not in a position to argue about why autoconf works this way, but I
> can say that I'd like to see unsupported options rejected if there is a
> sensible way to do it. I've been bitten more than once by mistakenly
> using --enable-foo rather than --with-foo, or just plain mis-typing.
>
> Regards, Dave.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Fri, May 05, 2006 at 08:34:36AM -0400, Bruce Momjian wrote:
>
> I am thinking we would need an option at the start like --strict that
> would throw an error for any later invalid options.

Well, --strict would be tricky, if it's possible. My reading of the
autoconf code doesn't indicate a means of doing adding abitrary
options. But something like --enable-strict-options would be fairly
straight forward. Problem being, if you mistype that option, it'll seem
to work even when it isn't :)

Maybe an evironment variable: PGAC_STRICT

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
> On Fri, May 05, 2006 at 08:34:36AM -0400, Bruce Momjian wrote:
> >
> > I am thinking we would need an option at the start like --strict that
> > would throw an error for any later invalid options.
>
> Well, --strict would be tricky, if it's possible. My reading of the
> autoconf code doesn't indicate a means of doing adding abitrary
> options. But something like --enable-strict-options would be fairly
> straight forward. Problem being, if you mistype that option, it'll seem
> to work even when it isn't :)

I've been bitten by this in the past as well. I'd vote for applying the
patch as-is, no "strict mode" necessary (because, as you say, it's easy
to get it wrong).

Can the Debian build script be fixed? Does the RPM spec have the same
problem?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, May 05, 2006 at 09:13:54AM -0400, Alvaro Herrera wrote:
> > Well, --strict would be tricky, if it's possible. My reading of the
> > autoconf code doesn't indicate a means of doing adding abitrary
> > options. But something like --enable-strict-options would be fairly
> > straight forward. Problem being, if you mistype that option, it'll seem
> > to work even when it isn't :)
>
> I've been bitten by this in the past as well. I'd vote for applying the
> patch as-is, no "strict mode" necessary (because, as you say, it's easy
> to get it wrong).

How about the reverse, an option to relax the checking.
--disable-strict-options for example?

> Can the Debian build script be fixed? Does the RPM spec have the same
> problem?

Looking at the source it may be an artifact of the CDBS build system
used to build the package. It knows that the autotools are in use and
appends it automatically. FWIW, I'd just add a line to the case
statement accepting the enable_maintainer_mode variable since it's
harmless and we're trying to catch typos here, not actual options that
don't apply in our case.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

I am worried if we change the default behavior that build systems will
fail, but fail after our release when they go to package, and we will
not get feedback until to late.

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Fri, May 05, 2006 at 09:13:54AM -0400, Alvaro Herrera wrote:
> > > Well, --strict would be tricky, if it's possible. My reading of the
> > > autoconf code doesn't indicate a means of doing adding abitrary
> > > options. But something like --enable-strict-options would be fairly
> > > straight forward. Problem being, if you mistype that option, it'll seem
> > > to work even when it isn't :)
> >
> > I've been bitten by this in the past as well. I'd vote for applying the
> > patch as-is, no "strict mode" necessary (because, as you say, it's easy
> > to get it wrong).
>
> How about the reverse, an option to relax the checking.
> --disable-strict-options for example?
>
> > Can the Debian build script be fixed? Does the RPM spec have the same
> > problem?
>
> Looking at the source it may be an artifact of the CDBS build system
> used to build the package. It knows that the autotools are in use and
> appends it automatically. FWIW, I'd just add a line to the case
> statement accepting the enable_maintainer_mode variable since it's
> harmless and we're trying to catch typos here, not actual options that
> don't apply in our case.
>
> Have a nice day,
> --
> Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> > From each according to his ability. To each according to his ability to litigate.
-- End of PGP section, PGP failed!

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Fri, May 05, 2006 at 12:28:48PM -0400, Bruce Momjian wrote:
>
> I am worried if we change the default behavior that build systems will
> fail, but fail after our release when they go to package, and we will
> not get feedback until to late.

I guess there are a number of ways to deal with this:

1. Provide an escape option they can add
2. Package systems can usually apply patches prior to compiling, they can
always remove the offending line if they like.
3. Try and get feedback from them now rather than wait

enable_maintainer_mode is a kind of special case. It's a flag that
disables a number of braindead things done by automake and libtool and
probably others. autoconf OTOH doesn't know about it. So, independant
of the options above, I think it should be accepted without warning.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Am Freitag, 5. Mai 2006 20:07 schrieb Martijn van Oosterhout:
> 1. Provide an escape option they can add
> 2. Package systems can usually apply patches prior to compiling, they can
> always remove the offending line if they like.
> 3. Try and get feedback from them now rather than wait

My feedback is this: You are going to enter a world of pain.

If you are worried about mistyping options, I recommend setting up a shell
alias.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

On 5/9/06, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Am Freitag, 5. Mai 2006 20:07 schrieb Martijn van Oosterhout:
> > 1. Provide an escape option they can add
> > 2. Package systems can usually apply patches prior to compiling, they can
> > always remove the offending line if they like.
> > 3. Try and get feedback from them now rather than wait
>
> My feedback is this: You are going to enter a world of pain.

Seems that way. Especially if the non-PGAC options won't be
handled automatically.

Some projects have solved the problem different way - by printing
out the summary of most important options at the end of configure:

PostgreSQL version X.X.X
OpenSSL: no
Integer datatime: yes
Python: yes
Perl: yes

So at the end of configure the user can visually confirm
his expectations without needing to parse the noise
from full configure output. Maybe this would be better
solution.

--
marko

On Tue, May 09, 2006 at 10:37:43AM +0200, Peter Eisentraut wrote:
> Am Freitag, 5. Mai 2006 20:07 schrieb Martijn van Oosterhout:
> > 1. Provide an escape option they can add
> > 2. Package systems can usually apply patches prior to compiling, they can
> > always remove the offending line if they like.
> > 3. Try and get feedback from them now rather than wait
>
> My feedback is this: You are going to enter a world of pain.

Can you explain why? Unknown options don't do anything, so having users
remove them seems like a good move.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Am Dienstag, 9. Mai 2006 10:55 schrieb Martijn van Oosterhout:
> Can you explain why? Unknown options don't do anything, so having users
> remove them seems like a good move.

Build system frameworks assume that they can pass any option and that unknown
options will be ignored. This grew out of the old Cygnus GNU megatree but as
you saw it is also used by package building frameworks like CDBS. In fact,
if we one day separate the PLs into separate source tarballs, I'd like to set
up a similar megatree system so building everything becomes easier.

I don't object to having a strict mode or printing warnings or printing a
summary at the end, but we are not in a position to redefine build system
practice.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Marko Kreen wrote:
> On 5/9/06, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > Am Freitag, 5. Mai 2006 20:07 schrieb Martijn van Oosterhout:
> > > 1. Provide an escape option they can add
> > > 2. Package systems can usually apply patches prior to compiling, they can
> > > always remove the offending line if they like.
> > > 3. Try and get feedback from them now rather than wait
> >
> > My feedback is this: You are going to enter a world of pain.
>
> Seems that way. Especially if the non-PGAC options won't be
> handled automatically.
>
> Some projects have solved the problem different way - by printing
> out the summary of most important options at the end of configure:
>
> PostgreSQL version X.X.X
> OpenSSL: no
> Integer datatime: yes
> Python: yes
> Perl: yes
>
> So at the end of configure the user can visually confirm
> his expectations without needing to parse the noise
> from full configure output. Maybe this would be better
> solution.

Seems we would be best printing out options we _didn't_ undertand at the
end of the build.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Tue, May 09, 2006 at 11:35:32AM -0400, Bruce Momjian wrote:
> > So at the end of configure the user can visually confirm
> > his expectations without needing to parse the noise
> > from full configure output. Maybe this would be better
> > solution.
>
> Seems we would be best printing out options we _didn't_ undertand at the
> end of the build.

I can live with that. It's a minor tweak...

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

On Tue, May 09, 2006 at 02:00:34PM +0200, Peter Eisentraut wrote:
> Am Dienstag, 9. Mai 2006 10:55 schrieb Martijn van Oosterhout:
> > Can you explain why? Unknown options don't do anything, so having users
> > remove them seems like a good move.
>
> Build system frameworks assume that they can pass any option and that unknown
> options will be ignored. This grew out of the old Cygnus GNU megatree but as
> you saw it is also used by package building frameworks like CDBS. In fact,
> if we one day separate the PLs into separate source tarballs, I'd like to set
> up a similar megatree system so building everything becomes easier.
>
> I don't object to having a strict mode or printing warnings or printing a
> summary at the end, but we are not in a position to redefine build system
> practice.

Then it seems like the best way to go would be to provide
--disable-strict-mode. I dislike the idea of printing a summary, because
it's easy to miss problems there, and it's also very counter-intuitive.
Until now I'd always assumed that configure would always complain about
invalid arguments because I've seen it happen before; I didn't think
you'd actually have to write code to make it do that (talk about a
brain-dead tool...)

In any case, I think the real use case here is catching errors from
general users who are installing from source, which disqualifies
--enable-strict as well as setting a shell alias.

Hopefully no one finds any need to use --disable-strict and it can just
be dropped down the road...
--
Jim C. Nasby, Sr. Engineering Consultant jnasby(at)pervasive(dot)com
Pervasive Software http://pervasive.com work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461