Re: Why polecat and colugos are failing to build back branches

I looked into $SUBJECT. There appear to be two distinct issues:

1. On colugos (OS X with LLVM), the plperl link step is spitting up
because Apple's ExtUtils::Embed puts some -arch switches into
perl_embed_ldflags. We found out about that some time ago, and fixed
it for 9.0 and up here:
http://git.postgresql.org/gitweb?p=postgresql.git&a=commitdiff&h=d69a419e682c2d39c2355105a7e5e2b90357c8f0
However, because when using gcc that only results in a warning,
we didn't back-patch it. Now it appears that it's an error when using
LLVM, so maybe we oughta back-patch it, at least to whichever releases
we think will build with LLVM.

2. Pre-9.0, the installation step is failing like this:

make -C src install
/bin/sh ../config/mkinstalldirs '/Volumes/High Usage/usr/local/src/build-farm-4.4/builds/REL8_2_STABLE/pgsql.14435/src/test/regress/./tmp_check/install/usr/local/src/build-farm-4.4/builds/REL8_2_STABLE/inst/lib/postgresql/pgxs/src'
mkdir /Volumes/High/Usage
mkdir: /Volumes/High/Usage: Permission denied
mkdir /Volumes/High/Usage/usr
mkdir: /Volumes/High/Usage: No such file or directory
mkdir /Volumes/High/Usage/usr/local
mkdir: /Volumes/High/Usage/usr: No such file or directory
[etc]

What is happening here of course is that mkinstalldirs is not prepared
to deal with spaces in the given pathname. In 9.0 we replaced it with a
more modern script that apparently hasn't got that problem. (Or at
least, there's no such failure ... I don't see any reference to
'/Volumes/High Usage/' in the later-version buildfarm logs, so maybe
there is some other factor at work?)

It looks to me like it wouldn't be terribly difficult to make
mkinstalldirs handle this case correctly. We aren't normally in the
habit of patching bugs that only appear in old release branches,
but should we make an exception for this? The alternatives seem to be
to ask Robert to rename the volume, or stop testing pre-9.0 branches on
that machine. Neither seems like a great idea from a test coverage
standpoint. We *should* have at least one test machine that's testing
building and installation into space-containing paths, or the case is
certain to break.

regards, tom lane

On 06/13/2011 08:05 PM, Tom Lane wrote:
> I looked into $SUBJECT. There appear to be two distinct issues:
>
> 1. On colugos (OS X with LLVM), the plperl link step is spitting up
> because Apple's ExtUtils::Embed puts some -arch switches into
> perl_embed_ldflags. We found out about that some time ago, and fixed
> it for 9.0 and up here:
> http://git.postgresql.org/gitweb?p=postgresql.git&a=commitdiff&h=d69a419e682c2d39c2355105a7e5e2b90357c8f0
> However, because when using gcc that only results in a warning,
> we didn't back-patch it. Now it appears that it's an error when using
> LLVM, so maybe we oughta back-patch it, at least to whichever releases
> we think will build with LLVM.
>
> 2. Pre-9.0, the installation step is failing like this:
>
> make -C src install
> /bin/sh ../config/mkinstalldirs '/Volumes/High Usage/usr/local/src/build-farm-4.4/builds/REL8_2_STABLE/pgsql.14435/src/test/regress/./tmp_check/install/usr/local/src/build-farm-4.4/builds/REL8_2_STABLE/inst/lib/postgresql/pgxs/src'
> mkdir /Volumes/High/Usage
> mkdir: /Volumes/High/Usage: Permission denied
> mkdir /Volumes/High/Usage/usr
> mkdir: /Volumes/High/Usage: No such file or directory
> mkdir /Volumes/High/Usage/usr/local
> mkdir: /Volumes/High/Usage/usr: No such file or directory
> [etc]
>
> What is happening here of course is that mkinstalldirs is not prepared
> to deal with spaces in the given pathname. In 9.0 we replaced it with a
> more modern script that apparently hasn't got that problem. (Or at
> least, there's no such failure ... I don't see any reference to
> '/Volumes/High Usage/' in the later-version buildfarm logs, so maybe
> there is some other factor at work?)
>
> It looks to me like it wouldn't be terribly difficult to make
> mkinstalldirs handle this case correctly. We aren't normally in the
> habit of patching bugs that only appear in old release branches,
> but should we make an exception for this? The alternatives seem to be
> to ask Robert to rename the volume, or stop testing pre-9.0 branches on
> that machine. Neither seems like a great idea from a test coverage
> standpoint. We *should* have at least one test machine that's testing
> building and installation into space-containing paths, or the case is
> certain to break.
>
>

I think we can be a bit more liberal about build patches than things
that can affect the runtime behaviour.

So +1 for fixing both of these.

cheers

andrew

On Mon, Jun 13, 2011 at 8:19 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> I think we can be a bit more liberal about build patches than things that
> can affect the runtime behaviour.

I agree. I wouldn't go nuts with it, but I don't see any reason to
worry about fixing this case.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Jun 13, 2011, at 6:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I looked into $SUBJECT. There appear to be two distinct issues:
>
> 1. On colugos (OS X with LLVM), the
...
> However, because when using gcc that only results in a warning,
> we didn't back-patch it. Now it appears that it's an error when using
> LLVM, so maybe we oughta back-patch it, at least to whichever releases
> we think will build with LLVM.

FYI, in the latest beta developer XCode release, gcc is llvm. That's not my test machine though.

>
> 2. Pre-9.0, the installation step is failing like this:
...
> alternatives seem to be
> to ask Robert to rename the volume, or stop testing pre-9.0 branches on
> that machine.

Any change is no problem, just let me know.

Later,
Rob

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 06/13/2011 08:05 PM, Tom Lane wrote:
>> I looked into $SUBJECT. There appear to be two distinct issues:
>> ...

> I think we can be a bit more liberal about build patches than things
> that can affect the runtime behaviour.

> So +1 for fixing both of these.

I've committed patches that fix these issues on my own OS X machine,
though it remains to be seen whether polecat and colugos will like
them. It turns out that whatever setup Robert has got with
'/Volumes/High Usage/' is really *not* fully exercising the system
as far as space-containing paths go, because I found bugs in all
active branches when I tried to do builds and installs underneath
'/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter
to exercise the case on a long-term basis? If we don't, I think
we can expect that it'll break regularly.

(I wouldn't care to bet that the MSVC case works yet, either.)

regards, tom lane

On 06/14/2011 05:45 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> On 06/13/2011 08:05 PM, Tom Lane wrote:
>>> I looked into $SUBJECT. There appear to be two distinct issues:
>>> ...
>> I think we can be a bit more liberal about build patches than things
>> that can affect the runtime behaviour.
>> So +1 for fixing both of these.
> I've committed patches that fix these issues on my own OS X machine,
> though it remains to be seen whether polecat and colugos will like
> them. It turns out that whatever setup Robert has got with
> '/Volumes/High Usage/' is really *not* fully exercising the system
> as far as space-containing paths go, because I found bugs in all
> active branches when I tried to do builds and installs underneath
> '/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter
> to exercise the case on a long-term basis? If we don't, I think
> we can expect that it'll break regularly.
>
> (I wouldn't care to bet that the MSVC case works yet, either.)
>
>

Well, OSX is just using our usual *nix paraphernalia, so if it's broken
won't all such platforms probably be broken too? I'd actually bet a
modest amount MSVC is less broken because it uses perl modules like
File::Copy to do most of its work and so will be less prone to shell
parsing breakage.

But yes, we should check regularly.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 06/14/2011 05:45 PM, Tom Lane wrote:
>> I've committed patches that fix these issues on my own OS X machine,

> Well, OSX is just using our usual *nix paraphernalia, so if it's broken
> won't all such platforms probably be broken too?

Yes, certainly. The reason I specified OS X in particular is that I
only tested the darwin branch of Makefile.shlib. The -install_name
switch that was the problem there is specific to OS X, but I wouldn't
be surprised if some of the other branches have their own platform-
specific issues.

> I'd actually bet a modest amount MSVC is less broken

Very possibly, but unless it's being tested it's no sure bet.

regards, tom lane

On Jun 14, 2011, at 3:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I've committed patches that fix these issues on my own OS X machine,
> though it remains to be seen whether polecat and colugos will like
> them. It turns out that whatever setup Robert has got with
> '/Volumes/High Usage/' is really *not* fully exercising the system
> as far as space-containing paths go, because I found bugs in all

'/Volumes/High Usage' is a "new" external drive as I put in a SSD as the main drive. Rather than re-do all my configs, I created a symbolic link from /usr/local/src/pg_buildfarm... to the external disk.

> active branches when I tried to do builds and installs underneath
> '/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter
> to exercise the case on a long-term basis? If we don't, I think
> we can expect that it'll break regularly.

I'd be happy to do that on my iMac, which isn't a build farm member yet, although I had plans to add it. I can easily do a single tester with spaces in the path, or multiple testers with and without spaces.

I'll work on the build farm config setup at least for the space scenario.

Cheers,
Rob

On 06/14/2011 06:09 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> On 06/14/2011 05:45 PM, Tom Lane wrote:
>>> I've committed patches that fix these issues on my own OS X machine,
>> Well, OSX is just using our usual *nix paraphernalia, so if it's broken
>> won't all such platforms probably be broken too?
> Yes, certainly. The reason I specified OS X in particular is that I
> only tested the darwin branch of Makefile.shlib. The -install_name
> switch that was the problem there is specific to OS X, but I wouldn't
> be surprised if some of the other branches have their own platform-
> specific issues.
>
>

We have some more work to do. Vpath builds don't like spaces. :-(

cheers

andrew

On Wed, Jun 15, 2011 at 00:01, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> On 06/14/2011 05:45 PM, Tom Lane wrote:
>>
>> Andrew Dunstan<andrew(at)dunslane(dot)net>  writes:
>>>
>>> On 06/13/2011 08:05 PM, Tom Lane wrote:
>>>>
>>>> I looked into $SUBJECT.  There appear to be two distinct issues:
>>>> ...
>>>
>>> I think we can be a bit more liberal about build patches than things
>>> that can affect the runtime behaviour.
>>> So +1 for fixing both of these.
>>
>> I've committed patches that fix these issues on my own OS X machine,
>> though it remains to be seen whether polecat and colugos will like
>> them.  It turns out that whatever setup Robert has got with
>> '/Volumes/High Usage/' is really *not* fully exercising the system
>> as far as space-containing paths go, because I found bugs in all
>> active branches when I tried to do builds and installs underneath
>> '/Users/tgl/foo bar/'.  Is it worth setting up a buildfarm critter
>> to exercise the case on a long-term basis?  If we don't, I think
>> we can expect that it'll break regularly.
>>
>> (I wouldn't care to bet that the MSVC case works yet, either.)
>>
>>
>
> Well, OSX is just using our usual *nix paraphernalia, so if it's broken
> won't all such platforms probably be broken too? I'd actually bet a modest
> amount MSVC is less broken because it uses perl modules like File::Copy to
> do most of its work and so will be less prone to shell parsing breakage.

Also, once installed, the vast majority of all Windows installs will
be running out of "c:\program files\postgresql", so we know it works
at *runtime*. But yeah, it wouldn't hurt to have a bf animal do the
actual testing out of such a path too.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On tis, 2011-06-14 at 18:09 -0400, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > On 06/14/2011 05:45 PM, Tom Lane wrote:
> >> I've committed patches that fix these issues on my own OS X machine,
>
> > Well, OSX is just using our usual *nix paraphernalia, so if it's broken
> > won't all such platforms probably be broken too?
>
> Yes, certainly. The reason I specified OS X in particular is that I
> only tested the darwin branch of Makefile.shlib. The -install_name
> switch that was the problem there is specific to OS X, but I wouldn't
> be surprised if some of the other branches have their own platform-
> specific issues.
>
> > I'd actually bet a modest amount MSVC is less broken
>
> Very possibly, but unless it's being tested it's no sure bet.

Here is some historical reference:

http://archives.postgresql.org/message-id/200512231739.47400.peter_e@gmx.net

I fixed installation into directories containing spaces back then, in
light of upcoming Windows and Mac support, but apparently some corner
cases had remained or crept back in, for lack of testing.

Building in a directory with spaces has always worked, and AFAICT, what
Tom committed also concerns only the installation directory.

Some problems like vpath that the above message mentioned remain, as
others have pointed out.

As a secondary point, we have so far used mostly single quotes for
quoting the installation directories, in case someone wants to try other
funny characters besides spaces. The most recent patch uses double
quotes. I'm not sure what degree of support we want to achieve there.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> As a secondary point, we have so far used mostly single quotes for
> quoting the installation directories, in case someone wants to try other
> funny characters besides spaces. The most recent patch uses double
> quotes. I'm not sure what degree of support we want to achieve there.

Oh, hm, I had noted double quotes in the one place that wasn't broken in
test/regress/Makefile, and followed that precedent. But you're probably
right that single quotes would be better, since that would prevent funny
business from $ characters for instance. We're not going to be able to
make both types of quote characters safe, unless gmake has some quoting
function I'm not aware of (which is surely possible).

regards, tom lane

I wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> As a secondary point, we have so far used mostly single quotes for
>> quoting the installation directories, in case someone wants to try other
>> funny characters besides spaces. The most recent patch uses double
>> quotes. I'm not sure what degree of support we want to achieve there.

> Oh, hm, I had noted double quotes in the one place that wasn't broken in
> test/regress/Makefile, and followed that precedent. But you're probably
> right that single quotes would be better, since that would prevent funny
> business from $ characters for instance. We're not going to be able to
> make both types of quote characters safe, unless gmake has some quoting
> function I'm not aware of (which is surely possible).

I changed the places I'd modified yesterday to use single quotes not
double, and fixed a couple other spots in passing to do the same.
However, I don't see any simple way to deal with the places where we
are sticking paths into C-string constants, to wit in building pg_config
and pg_config_paths.h. pg_config in particular is nasty because there
are pretty much guaranteed to be single quotes in the value we want for
VAL_LDFLAGS (because of rpath...). So at the moment, we're not really
safe against single *or* double quotes, nor dollar signs, in
installation paths. About the only good thing to be said about it is
that these characters are so troublesome that Unix users are unlikely
to use them in directory names anyway.

regards, tom lane

On Jun 15, 2011, at 7:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> ...
> installation paths. About the only good thing to be said about it is
> that these characters are so troublesome that Unix users are unlikely
> to use them in directory names anyway.

So I'm guessing you don't want this path name? I was going to throw some \n and & in it also, maybe some *,' and " for good measure >-) Shall I just stick with spaces?

drwxr-xr-x 2 Robert wheel 68B Jun 15 17:26 pg bu!ldfa$m\a\$y/
Robert-Creagers-iMac:src Robert$

Later,
Rob

Robert Creager <robert(at)logicalchaos(dot)org> writes:
> On Jun 15, 2011, at 7:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> installation paths. About the only good thing to be said about it is
>> that these characters are so troublesome that Unix users are unlikely
>> to use them in directory names anyway.

> So I'm guessing you don't want this path name? I was going to throw some \n and & in it also, maybe some *,' and " for good measure >-) Shall I just stick with spaces?

> drwxr-xr-x 2 Robert wheel 68B Jun 15 17:26 pg bu!ldfa$m\a\$y/
> Robert-Creagers-iMac:src Robert$

There seems to be about no chance of making that work :-(.

In practice, names with spaces seem to be the only cases we've heard
complaints about, so I doubt it's worth getting worked up over.

regards, tom lane

On Thu, Jun 16, 2011 at 9:48 AM, Robert Creager <robert(at)logicalchaos(dot)org> wrote:
> On Jun 15, 2011, at 7:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> ...
>> installation paths.  About the only good thing to be said about it is
>> that these characters are so troublesome that Unix users are unlikely
>> to use them in directory names anyway.
>
> So I'm guessing you don't want this path name?  I was going to throw some \n and & in it also, maybe some *,' and " for good measure >-)  Shall I just stick with spaces?
>
> drwxr-xr-x   2 Robert  wheel    68B Jun 15 17:26 pg bu!ldfa$m\a\$y/
> Robert-Creagers-iMac:src Robert$

LOL. +1 for including that in the buildfarm, but only if you write
the patch that makes it work yourself...!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company