Thread: [PATCH] Have configure complain about unknown options
Currently, configure ignores unknown --enable/disable/with/without options. Personally I find this behaviour annoying, so here is a patch that fixes it. Example: $ ./configure --enable-depends checking build system type... i686-pc-linux-gnu checking host system type... i686-pc-linux-gnu <snip> checking whether to build with Bonjour support... no checking whether to build with OpenSSL support... no *** Unknown feature: enable_depends *** $ Now, while the error message could do with some help (feedback welcome) I personally find this to be quite useful. This patch will detect any argument that isn't declared using one of the PGAC_ARG macros. This means that if there are any other --enable/with arguments we accept but don't declare, this will barf on them too. Do we have any of those? If so, we should document them somewhere, if necessary creating a PGAC_ARG_HIDDEN so they can pass this test. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
Martijn van Oosterhout <kleptog@svana.org> writes:
> Currently, configure ignores unknown --enable/disable/with/without
> options.
The autoconf people consider that a feature, not a bug. I'm
disinclined to second-guess the designers of the tool, especially
with a patch like this that mucks with the internals to the extent that
it'll probably break in every future autoconf revision.
Feel free to try to convince them to change it though ...
regards, tom lane
On Thu, May 04, 2006 at 12:51:34PM -0400, Tom Lane wrote: > Martijn van Oosterhout <kleptog@svana.org> writes: > > Currently, configure ignores unknown --enable/disable/with/without > > options. > > The autoconf people consider that a feature, not a bug. I'm > disinclined to second-guess the designers of the tool, especially > with a patch like this that mucks with the internals to the extent that > it'll probably break in every future autoconf revision. Fair enough. It should work on any version since 2000 since this is the way you build up strings. Diversions are a feature of m4 not of autoconf so it's not like they're going away... > Feel free to try to convince them to change it though ... Not me, I'll keep it in my local repository then. I'm always misspelling --enable-depends and this catches it... Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
On 5/4/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Martijn van Oosterhout <kleptog@svana.org> writes: > > Currently, configure ignores unknown --enable/disable/with/without > > options. > > The autoconf people consider that a feature, not a bug. I'm > disinclined to second-guess the designers of the tool, especially > with a patch like this that mucks with the internals to the extent that > it'll probably break in every future autoconf revision. > > Feel free to try to convince them to change it though ... AFAIK that 'feature' is there to support configuring a 'tree' of projects (like gcc), where subprojects have their own configure scripts with different options. That way you can give all options to top-level configure script which passes them to other scripts and each picks only whats needed. In such setting all scripts need to ignore 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. -- marko
On Fri, May 05, 2006 at 02:22:25PM +0300, Marko Kreen wrote: > AFAIK that 'feature' is there to support configuring a 'tree' > of projects (like gcc), where subprojects have their own configure > scripts with different options. That way you can give all options > to top-level configure script which passes them to other scripts and > each picks only whats needed. In such setting all scripts need to > ignore unknown options. I was wondering about that. I think it's also because there are possibly a number of tools working together (autoheader/automake/etc) and autoconf is not in the position to know all possible options. You're not required to declare all the options you understand in configure.in because they may be used elsewhere. > 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. Someone in the past has gone to some effort to create a bunch of macros for postgres that declares all the options our configure script understands and simultaneously create help for them (this is also why the patch is so small, the hard work of identifying the options was done long ago). So we are in a position to know what is allowed and what isn't. One thing I've noticed so far is that the Debian package scripts use --enable-maintainer-mode. No idea why, that affects automake, which we don't use. It is however an excellent example of why complaining about unknown options can't be done in the general case. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
> -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Marko Kreen > Sent: 05 May 2006 12:22 > To: Tom Lane > Cc: Martijn van Oosterhout; pgsql-patches@postgresql.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@postgresql.org > > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Marko Kreen > > Sent: 05 May 2006 12:22 > > To: Tom Lane > > Cc: Martijn van Oosterhout; pgsql-patches@postgresql.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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
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@svana.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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
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@gmx.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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
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@gmx.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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Attachment
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@pervasive.com Pervasive Software http://pervasive.com work: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461