Re: [HACKERS] Sanity checking for ./configure options?

2016-03-14 Thread Tom Lane
Jim Nasby writes: > All issues should now be addressed. Pushed with some more tweaking: the test syntax wasn't terribly portable, and the error messages weren't at all consistent. regards, tom lane -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Sanity checking for ./configure options?

2016-03-14 Thread Alex Shulgin
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Looks good to me. It only allows valid number between 1 and 65535,

Re: [HACKERS] Sanity checking for ./configure options?

2016-03-13 Thread Jim Nasby
On 2/26/16 9:29 PM, Peter Eisentraut wrote: Your code and comments suggest that you can specify the port to configure by setting PGPORT, but that is not the case. test == is not portable (bashism). Error messages should have consistent capitalization. Indentation in configure is two spaces.

Re: [HACKERS] Sanity checking for ./configure options?

2016-03-11 Thread Robert Haas
On Sat, Feb 27, 2016 at 3:15 PM, Jim Nasby wrote: > On 2/26/16 9:29 PM, Peter Eisentraut wrote: >> To make this really robust, you might need to do pattern matching on the >> value. > > Yeah, and I don't see any reasonable way to do that... we don't require sed > or the

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-27 Thread Andres Freund
On 2016-02-27 14:15:45 -0600, Jim Nasby wrote: > Yeah, and I don't see any reasonable way to do that... we don't require sed > or the like, do we? We actually do. Check the bottom of configure.in. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-27 Thread Jim Nasby
On 2/26/16 9:29 PM, Peter Eisentraut wrote: To make this really robust, you might need to do pattern matching on the value. Yeah, and I don't see any reasonable way to do that... we don't require sed or the like, do we? I'll look at the other things you mentioned. -- Jim Nasby, Data

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-27 Thread Jim Nasby
On 2/26/16 9:34 AM, Ivan Kartyshov wrote: The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed Tested, I think

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-26 Thread Peter Eisentraut
On 2/22/16 6:24 PM, Jim Nasby wrote: > On 2/5/16 10:08 AM, David Fetter wrote: >> On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: >>> I just discovered that ./configure will happily accept >>> '--with-pgport=' (I >>> was actually doing =$PGPORT, and didn't realize $PGPORT was empty).

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-26 Thread Ivan Kartyshov
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed Tested, I think it`s rather important to make cleanup work

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-26 Thread Tom Lane
David Fetter writes: > On Fri, Feb 26, 2016 at 04:55:23PM +0530, Robert Haas wrote: >> On Wed, Feb 24, 2016 at 4:01 AM, David Fetter wrote: >>> I'm thinking that both the GUC check and the configure one should >>> restrict it to [1024..65535]. >> Doesn't

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-26 Thread David Fetter
On Fri, Feb 26, 2016 at 04:55:23PM +0530, Robert Haas wrote: > On Wed, Feb 24, 2016 at 4:01 AM, David Fetter wrote: > > I'm thinking that both the GUC check and the configure one should > > restrict it to [1024..65535]. > > Doesn't sound like a good idea to me. If somebody has

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-26 Thread Robert Haas
On Wed, Feb 24, 2016 at 4:01 AM, David Fetter wrote: > I'm thinking that both the GUC check and the configure one should > restrict it to [1024..65535]. Doesn't sound like a good idea to me. If somebody has a reason they want to do that, they shouldn't have to hack the source

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-23 Thread David Fetter
On Tue, Feb 23, 2016 at 04:09:00PM -0600, Jim Nasby wrote: > On 2/23/16 9:37 AM, Alvaro Herrera wrote: > >Jim Nasby wrote: > >>On 2/5/16 10:08 AM, David Fetter wrote: > >>>On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: > I just discovered that ./configure will happily accept

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-23 Thread Jim Nasby
On 2/23/16 9:37 AM, Alvaro Herrera wrote: Jim Nasby wrote: On 2/5/16 10:08 AM, David Fetter wrote: On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: I just discovered that ./configure will happily accept '--with-pgport=' (I was actually doing =$PGPORT, and didn't realize $PGPORT was

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-23 Thread Alvaro Herrera
Jim Nasby wrote: > On 2/5/16 10:08 AM, David Fetter wrote: > >On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: > >>I just discovered that ./configure will happily accept '--with-pgport=' (I > >>was actually doing =$PGPORT, and didn't realize $PGPORT was empty). What you > >>end up with

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-22 Thread Jim Nasby
On 2/5/16 10:08 AM, David Fetter wrote: On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: I just discovered that ./configure will happily accept '--with-pgport=' (I was actually doing =$PGPORT, and didn't realize $PGPORT was empty). What you end up with is a compile error in guc.c,

Re: [HACKERS] Sanity checking for ./configure options?

2016-02-05 Thread David Fetter
On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote: > I just discovered that ./configure will happily accept '--with-pgport=' (I > was actually doing =$PGPORT, and didn't realize $PGPORT was empty). What you > end up with is a compile error in guc.c, with no idea why it's broken. Any >