Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
On Fri, Jul 15, 2005 at 08:06:15PM -0500, Kris Jurka wrote: On Fri, 15 Jul 2005, Marko Kreen wrote: [buildfarm machine dragonfly] On Tue, Jul 12, 2005 at 01:06:46PM -0500, Kris Jurka wrote: Well the buildfarm machine kudu is actually the same machine just building with the Sun compiler and it works fine. It links all of libz.a into libpgcrypto.so while gcc refuses to. I googled a bit and found two suggestions: 1. http://curl.haxx.se/mail/lib-2002-01/0092.html (Use -mimpure-text on linking line) The attached patch does #1. Could you try it and see if it fixes it? This patch works, pgcrypto links and passes its installcheck test now. Kris Jurka Thanks. Here is the patch with a little comment. It should not break anything as it just disables a extra argument -assert pure-text to linker. Linking static libraries into shared one is bad idea, as the static parts wont be shared between processes, but erroring out is worse, especially if another compiler for a platform allows it. This makes gcc act same way as Sun's cc. -- marko Index: src/Makefile.shlib === RCS file: /opt/arc/cvs2/pgsql/src/Makefile.shlib,v retrieving revision 1.95 diff -u -c -r1.95 Makefile.shlib *** src/Makefile.shlib 13 Jul 2005 17:00:44 - 1.95 --- src/Makefile.shlib 16 Jul 2005 09:59:18 - *** *** 188,194 ifeq ($(PORTNAME), solaris) ifeq ($(GCC), yes) ! LINK.shared = $(CC) -shared else LINK.shared = $(CC) -G endif --- 188,196 ifeq ($(PORTNAME), solaris) ifeq ($(GCC), yes) ! # -mimpure-text disables passing '-assert pure-text' to linker, ! # to allow linking static library into shared one, like Sun's cc does. ! LINK.shared = $(CC) -shared -mimpure-text else LINK.shared = $(CC) -G endif ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] backslashes in pgindent
Luke Lonergan wrote: Bruce, On 7/15/05 9:59 PM, Bruce Momjian pgman@candle.pha.pa.us wrote: Actually, mine returns ')' too for the last command. I didn't copy that into the email. How about the top tests? Notice I get an error on the first one without the backslash. Are you OK escaping '(' but not ')'? That might be a solution. You know, I'm not sure - I don't know the intended meaning of this line: awk ' BEGIN {line1 = ; line2 = } { line2 = $0; if (NR = 2) print line1; if (NR = 2 line2 ~ ^{[]*$ line1 !~ ^struct line1 !~ ^enum line1 !~ ^typedef line1 !~ ^extern[ ][ ]*\C\ line1 !~ = = line1 ~ \)) print int pgindent_func_no_var_fix;; line1 = line2; } END Is the escaped paren within meant to be a literal? Yes, all parentheses tests in pgindent are meant to test literals. The '\(' is required so it doesn't think it is defining a regex group. Here is the output from my tests: $ echo '('|awk '$0 ~ /(/ {print $0}' awk: cmd. line:1: fatal: Unmatched ( or \(: /(/ $ echo '('|awk '$0 ~ /\(/ {print $0}' ( $ echo ')'|awk '$0 ~ /)/ {print $0}' ) $ echo ')'|awk '$0 ~ /\)/ {print $0}' ) I just tried a machine that has awk 3.1.3 (you have 3.1.4) and I see: $ echo '('|awk '$0 ~ /(/ {print $0}' awk: cmd. line:1: fatal: Invalid regular expression: /(/ $ echo '('|awk '$0 ~ /\(/ {print $0}' ( $ echo ')'|awk '$0 ~ /)/ {print $0}' ) $ echo ')'|awk '$0 ~ /\)/ {print $0}' ) which exactly matches the 3.0.4 version I usually use. What are your outputs for these tests? Have you tried the current CVS version of pgindent? I think it might now work for you. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
Marko Kreen marko@l-t.ee writes: On Tue, Jul 12, 2005 at 01:06:46PM -0500, Kris Jurka wrote: Well the buildfarm machine kudu is actually the same machine just building with the Sun compiler and it works fine. It links all of libz.a into libpgcrypto.so while gcc refuses to. I googled a bit and found two suggestions: 1. http://curl.haxx.se/mail/lib-2002-01/0092.html (Use -mimpure-text on linking line) The attached patch does #1. Could you try it and see if it fixes it? This sure seems like a crude band-aid rather than an actual solution. The bug as I see it is that gcc is choosing to link libz.a rather than libz.so --- why is that happening? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
On Sat, 16 Jul 2005, Tom Lane wrote: Marko Kreen marko@l-t.ee writes: I googled a bit and found two suggestions: 1. http://curl.haxx.se/mail/lib-2002-01/0092.html (Use -mimpure-text on linking line) This sure seems like a crude band-aid rather than an actual solution. The bug as I see it is that gcc is choosing to link libz.a rather than libz.so --- why is that happening? The link line says -L/usr/local/lib -lz and libz.a is in /usr/local/lib while libz.so is in /usr/lib. Kris Jurka ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] fixing REL7_3_STABLE build issues
Andrew Dunstan [EMAIL PROTECTED] writes: The attached (new) src/test/regress/expected/geometry_9.out, intended only for the 7.3 stable branch, allows a clean regression pass on my FC4 box. I called it that to avoid conflicts with other geometry_n files on later branches. I'd like to have a more principled approach to fixing the back branches than we'll do whatever it takes to have a clean buildfarm board on the set of machines that happen to have volunteered to run buildfarm on that branch. The geometry test has been such a consistent bugaboo that I'd sooner remove it from the back branch test lists than follow the path your proposal leads down. I can see we'd need at least one more geometry instance immediately in the 7.3 branch, and what about 7.2? The attached patch for contrib/seg/segparse.y allows a clean 7.3 contrib build. The latter fix looks like one that should be made on the 7.4 branch also - man 3 errno on my box says: Agreed --- this is outright unportable code. Patched in 7.2-7-4. (It may be that 7.4 fails to fail on your machine because it's already included errno.h by way of c.h, but the extern is an invitation to trouble in any case.) regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] fixing REL7_3_STABLE build issues
On Sat, 16 Jul 2005, Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: The attached (new) src/test/regress/expected/geometry_9.out, intended only for the 7.3 stable branch, allows a clean regression pass on my FC4 box. I called it that to avoid conflicts with other geometry_n files on later branches. I'd like to have a more principled approach to fixing the back branches than we'll do whatever it takes to have a clean buildfarm board on the set of machines that happen to have volunteered to run buildfarm on that branch. I think the emphasis on the buildfarm (at least for the principled approach) is wrong. The policy should be that for any platform all supported branches should pass all tests unless something is legitimately broken and cannot be fixed without major surgery. If this was the stated policy I know more buildfarm members would run the 7.2/3 branches to help enforce it. Also getting the regression tests to pass on even older versions(=7.1) seems like a waste of time, but ensuring that they at least compile and start to allow data extraction may not be, as a recent -general thread has shown. Kris Jurka ---(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
Re: [PATCHES] fixing REL7_3_STABLE build issues
Kris Jurka [EMAIL PROTECTED] writes: On Sat, 16 Jul 2005, Tom Lane wrote: I'd like to have a more principled approach to fixing the back branches than we'll do whatever it takes to have a clean buildfarm board on the set of machines that happen to have volunteered to run buildfarm on that branch. I think the emphasis on the buildfarm (at least for the principled approach) is wrong. The policy should be that for any platform all supported branches should pass all tests unless something is legitimately broken and cannot be fixed without major surgery. I am suggesting that the pre-7.4 geometry test *is* broken, and that fixing it only for those machines that join the buildfarm is (a) wrongheaded and (b) a waste of effort. It'll still be broken on an unknown (but ever-growing due to changes in toolchains) population of allegedly-supported platforms that happen not to be represented in the buildfarm. I agree that fixing compile problems is a worthwhile activity (and you'll note I applied that part of Andrew's patch). I don't agree that fixing known-problematic regression tests is a good use of time. We are not going to learn anything new by keeping geometry enabled in the back branches, so why not cut our losses? If you don't like removing it altogether, how about marking it ignore as we do for the random test, so that failures are treated as noncritical? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
Kris Jurka [EMAIL PROTECTED] writes: On Sat, 16 Jul 2005, Tom Lane wrote: This sure seems like a crude band-aid rather than an actual solution. The bug as I see it is that gcc is choosing to link libz.a rather than libz.so --- why is that happening? The link line says -L/usr/local/lib -lz and libz.a is in /usr/local/lib while libz.so is in /usr/lib. Well, that is a flat-out configuration error on the local sysadmin's part. I can't think of any good reason for the .so and .a versions of a library to live in different places. We certainly shouldn't hack our build process to build deliberately-inefficient object files in order to accommodate such a setup. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] fixing REL7_3_STABLE build issues
Tom Lane said: Andrew Dunstan [EMAIL PROTECTED] writes: The attached (new) src/test/regress/expected/geometry_9.out, intended only for the 7.3 stable branch, allows a clean regression pass on my FC4 box. I called it that to avoid conflicts with other geometry_n files on later branches. I'd like to have a more principled approach to fixing the back branches than we'll do whatever it takes to have a clean buildfarm board on the set of machines that happen to have volunteered to run buildfarm on that branch. The geometry test has been such a consistent bugaboo that I'd sooner remove it from the back branch test lists than follow the path your proposal leads down. I can see we'd need at least one more geometry instance immediately in the 7.3 branch, and what about 7.2? I have no objection to disabling the geometry tests by default for 7.2 and 7.3. I think you're right to suggest that fixing them is not worth the bother. Note that because of the way the buildfarm script works, this failure was masking the seg errno bogosity. Maybe I should reverse the test order to make contrib before running and regression tests. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
On Sat, 16 Jul 2005, Tom Lane wrote: Kris Jurka [EMAIL PROTECTED] writes: The link line says -L/usr/local/lib -lz and libz.a is in /usr/local/lib while libz.so is in /usr/lib. Well, that is a flat-out configuration error on the local sysadmin's part. I can't think of any good reason for the .so and .a versions of a library to live in different places. We certainly shouldn't hack our build process to build deliberately-inefficient object files in order to accommodate such a setup. Well the OS only came with the shared library and I needed the static one for some reason, so I installed it alone under /usr/local. This works fine with Sun's cc and Marko's research indicates that this will also work fine using GNU ld instead of Sun's ld. This is certainly an unusual thing to do, but I don't believe it is a flat-out configuration error, consider what would happen if the shared library didn't exist at all and only a static version were available. Until this recent batch of pgcrypto changes everything built fine. Kris Jurka ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] fixing REL7_3_STABLE build issues
Andrew Dunstan [EMAIL PROTECTED] writes: Note that because of the way the buildfarm script works, this failure was masking the seg errno bogosity. Maybe I should reverse the test order to make contrib before running and regression tests. Seems like that'd just mask a different set of failures. Is it practical to teach the script to run the tests that depend only on what you were able to build? I guess reporting the results would get more complicated ... regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] 4 pgcrypto regressions failures - 1 unsolved
Kris Jurka [EMAIL PROTECTED] writes: consider what would happen if the shared library didn't exist at all and only a static version were available. Until this recent batch of pgcrypto changes everything built fine. Well, the right answer to that really is that pgcrypto ought not try to link to libz unless a shared libz is available (compare for instance the situation with plperl and an unshared libperl). However, I'm not sure that we could reasonably expect to make a configuration test that would detect a situation like this --- that is, if we did look for shared libz, we would find it, and the fact that a nonshared libz in a different place would cause the actual link to fail seems like something that configure would be unlikely to be able to realize. I'm still of the opinion that your libz installation is broken; the fact that some other products chance not to fail with it is not evidence that it's OK. You could for instance have installed both libz.a and libz.so from the same build in /usr/local/lib, and that would work fine, independently of the existence of a version in /usr/lib. Come to think of it, are you sure that the versions in /usr/lib and /usr/local/lib are even ABI-compatible? If they are from different zlib releases, I think you're risking trouble regardless. Really the right way to deal with this sort of thing is that you put libz.a and libz.so in /usr/local/lib and corresponding headers in /usr/local/include, and then you don't need to sweat whether they are exactly compatible with what appears in /usr/lib and /usr/include. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] fixing REL7_3_STABLE build issues
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Note that because of the way the buildfarm script works, this failure was masking the seg errno bogosity. Maybe I should reverse the test order to make contrib before running and regression tests. Seems like that'd just mask a different set of failures. Yeah. I think I'd be more concerned by core regression failures than contrib build failures - especially as they are often likely to have more far reaching consequences. Is it practical to teach the script to run the tests that depend only on what you were able to build? I guess reporting the results would get more complicated ... Not easily. It's really not much more on the client side than an automation layer over existing build and test infrastructure. The script itself has very little intelligence. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fixing REL7_3_STABLE build issues
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Seems like that'd just mask a different set of failures. Yeah. I think I'd be more concerned by core regression failures than contrib build failures - especially as they are often likely to have more far reaching consequences. Agreed. I guess that the order of importance of the pieces you have is build main (this includes building PLs) run main tests run PL tests build contrib run contrib tests I'm not sure where the proposed-to-be-added multibyte regression tests go in this order. On practical grounds I would put them last; I rather suspect that porting failures in that code will be rare. Could be wrong though. It's slightly annoying that the PLs are built as part of the main build; I would rather run the main tests and then try to build and test the PLs (that is, the ones that have external dependencies --- plpgsql can be treated as part of the core for our purposes here). Not sure if it's worth hacking the makefiles to make that possible. regards, tom lane ---(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
Re: [PATCHES] fixing REL7_3_STABLE build issues
Tom Lane wrote: Yeah. I think I'd be more concerned by core regression failures than contrib build failures - especially as they are often likely to have more far reaching consequences. Agreed. I guess that the order of importance of the pieces you have is build main (this includes building PLs) run main tests run PL tests build contrib run contrib tests That's almost what we do, but it's a bit more complex. Slightly simplified, the sequence runs something like this (PL checks only run on HEAD or branches 8.0): configure make make check cd contrib make make install cd $installdir bin/initdb --no-locale data cd $installdir bin/pg_ctl -D data -w start make installcheck cd src/pl make installcheck cd contrib make installcheck cd $installdir bin/pg_ctl -D data stop I'm not sure where the proposed-to-be-added multibyte regression tests go in this order. On practical grounds I would put them last; I rather suspect that porting failures in that code will be rare. Could be wrong though. Yes, that makes sense. I don't know when I'll get time to make that happen though. It's slightly annoying that the PLs are built as part of the main build; I would rather run the main tests and then try to build and test the PLs (that is, the ones that have external dependencies --- plpgsql can be treated as part of the core for our purposes here). Not sure if it's worth hacking the makefiles to make that possible. I don't think so. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Interval-day patch
I am close to completing work on this patch and will post an updated version in a few days. --- Michael Glaesemann wrote: Please find attached a patch which adds a day field to the interval struct so that we can treat INTERVAL '1 day' differently from INTERVAL '24 hours' in DST-aware situations. It also includes a function called interval_simplify() which takes an interval argument and returns an interval where hours over 24 are promoted to days, e.g., template1=# select interval_simplify('3 months -11 days 79 hours 2 minutes'::interval); interval_simplify -- 3 mons -7 days -16:58:00 (1 row) If anyone has better ideas for the name of this function, please let me know. I've modified the regression tests, but still need to add additional tests for the interval_simplify function, and I want to add a few more tests for the new interval behavior. Also, the docs will need to be updated to mention the new behavior. I plan on doing this in over the next couple of days. This is some of the first C I've hacked, and the first patch I've submitted that's more than a documentation or a simple one-liner (and even that one got worked over pretty good :) ), so I fully expect some mistakes to be found. Please let me know and I'll do my best to fix them. In timestamp.c, I suspect that AdjustIntervalForTypmod, interval_scale will need some modifications, though I'm not quite sure what this code is doing. I've left them as-is. I've made some changes to interval2tm, but believe that the changes I've made may not be adequate. Given sufficient instruction, I'll be happy to make the necessary changes to these functions. A few things I noticed while I was working: In interval_mul and interval_div, I'm wondering whether 30.0 and 24.0 shouldn't be substituted for 30 and 24 in the non-integer-timestamp code path, as these are floats. Perhaps it doesn't make a difference for multiplication, but I see similar usage in interval_cmp_interval. I've left the code as-is. In the deconstruct_array calls in interval_accum and interval_avg, the size of interval is passed as a magic number (16). I think this could be abstracted out, such as #define SIZEOF_INTERVAL 16 to make the code a bit more robust (albeit just a little). Is this a reasonable change? Michael Glaesemann grzm myrealbox com [ Attachment, skipping... ] ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster