Thread: 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@dunslane.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@sss.pgh.pa.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@dunslane.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@dunslane.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@dunslane.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@sss.pgh.pa.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, Icreated 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 doa 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@dunslane.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@dunslane.net> wrote: > > On 06/14/2011 05:45 PM, Tom Lane wrote: >> >> Andrew Dunstan<andrew@dunslane.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@dunslane.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@gmx.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@gmx.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@sss.pgh.pa.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 goodmeasure >-) 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@logicalchaos.org> writes: > On Jun 15, 2011, at 7:51 PM, Tom Lane <tgl@sss.pgh.pa.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 " forgood 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@logicalchaos.org> wrote: > On Jun 15, 2011, at 7:51 PM, Tom Lane <tgl@sss.pgh.pa.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 " forgood 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