Обсуждение: buildfarm's typedefs list has gone completely nutso
The current HEAD typedefs list available from https://buildfarm.postgresql.org/cgi-bin/typedefs.pl has the following interesting additions compared to where things were on July 1: 2 ECPGt_bytea connection_name in_addr pg_fprintf send_appname The "2" in particular is causing seriously bad pgindent results for me. But as far as I can tell, none of these have any justification being marked as a typedef. calliphoridae seems to be contributing the "2" and "pg_fprintf". I didn't track down the rest (but calliphoridae is not to blame). Was there any change in calliphoridae's toolchain this month? regards, tom lane
Hi, On 2019-07-10 12:57:08 -0400, Tom Lane wrote: > The current HEAD typedefs list available from > https://buildfarm.postgresql.org/cgi-bin/typedefs.pl > has the following interesting additions compared to where > things were on July 1: > > 2 > ECPGt_bytea > connection_name > in_addr > pg_fprintf > send_appname Huh. > The "2" in particular is causing seriously bad pgindent results for > me. I haven't run pgindent, but I certainly can imagine... > But as far as I can tell, none of these have any justification being > marked as a typedef. > > calliphoridae seems to be contributing the "2" and "pg_fprintf". > I didn't track down the rest (but calliphoridae is not to blame). > Was there any change in calliphoridae's toolchain this month? Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't using that. So it's probably not the compiler side. But I also see a binutils upgrade: 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1 and corresponding upgrades forall the arch specific packages. I suspect it might be that. I can't immediately reproduce that locally though, using the same version of binutils. It's somewhat annoying that the buildfarm uses a different form of computing the typedefs than src/tools/find_typedef ... Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2019-07-10 12:57:08 -0400, Tom Lane wrote:
>> Was there any change in calliphoridae's toolchain this month?
> Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
> using that. So it's probably not the compiler side. But I also see a
> binutils upgrade:
> 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
> and corresponding upgrades forall the arch specific packages. I suspect
> it might be that.
Yeah, a plausible theory is that the output format changed enough
to confuse our typedef-symbol-scraping code.
regards, tom lane
On 7/10/19 1:34 PM, Andres Freund wrote: > > Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't > using that. So it's probably not the compiler side. But I also see a > binutils upgrade: > > 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1 > > and corresponding upgrades forall the arch specific packages. I suspect > it might be that. > > I can't immediately reproduce that locally though, using the same > version of binutils. It's somewhat annoying that the buildfarm uses a > different form of computing the typedefs than src/tools/find_typedef ... > That script is notably non-portable, and hasn't seen any significant update in a decade. If you want to run something like the buildfarm code, see <https://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html> for some clues I ran the client on a new Fedora 30 and it didn't produce the error. cheers andrew
Hi,
On 2019-07-10 16:40:20 -0400, Andrew Dunstan wrote:
> On 7/10/19 1:34 PM, Andres Freund wrote:
> >
> > Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
> > using that. So it's probably not the compiler side. But I also see a
> > binutils upgrade:
> >
> > 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
> >
> > and corresponding upgrades forall the arch specific packages. I suspect
> > it might be that.
> >
> > I can't immediately reproduce that locally though, using the same
> > version of binutils. It's somewhat annoying that the buildfarm uses a
> > different form of computing the typedefs than src/tools/find_typedef ...
> That script is notably non-portable, and hasn't seen any significant
> update in a decade.
I think that's kinda what I'm complaining about... It seems like a bad
idea to have this in the buildfarm code, rather than our code. IMO the
buildfarm code should invoke an updated src/tools/find_typedef - that
way people at least can create typedefs manually locally.
Not yet sure what's actually going on, but there's something odd with
debug information afaict:
objdump -w spits out warnings for a few files, all static libraries:
../install/lib/libpgcommon.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
../install/lib/libecpg.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 8450 unused bytes at the end of section .debug_loc
../install/lib/libpgcommon_shlib.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
../install/lib/libpgfeutils.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3090 unused bytes at the end of section .debug_loc
../install/lib/libpgport.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 38 unused bytes at the end of section .debug_loc
../install/lib/libpgtypes.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 13199 unused bytes at the end of section .debug_loc
../install/lib/libecpg_compat.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 15001 unused bytes at the end of section .debug_loc
../install/lib/libpgport_shlib.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 38 unused bytes at the end of section .debug_loc
../install/lib/libpq.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 5528 unused bytes at the end of section .debug_loc
Not sure if that's related - I don't get that locally (newer compiler,
different compiler options, same binutils).
Interestingly pg_fprintf is only generated by pg_fprintf, as far as I
can tell, and the 1/2 typedefs are from libpq. The relevant entries look
like:
<1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
<4c> DW_AT_name : (indirect string, offset: 0x0): USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 1
<50> DW_AT_decl_file : 2
<51> DW_AT_decl_line : 216
<52> DW_AT_decl_column : 23
<53> DW_AT_type : <0x57>
So I suspect it might be the corruption/misparsing of objdump that's to
blame. Kinda looks like there's an issue with the dwarf stringtable, and
thus the wrong strings (debug information for macros in this case) is
being picked up.
Greetings,
Andres Freund
On 7/10/19 8:24 PM, Andres Freund wrote: > Hi, > > On 2019-07-10 16:40:20 -0400, Andrew Dunstan wrote: >> On 7/10/19 1:34 PM, Andres Freund wrote: >>> Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't >>> using that. So it's probably not the compiler side. But I also see a >>> binutils upgrade: >>> >>> 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1 >>> >>> and corresponding upgrades forall the arch specific packages. I suspect >>> it might be that. >>> >>> I can't immediately reproduce that locally though, using the same >>> version of binutils. It's somewhat annoying that the buildfarm uses a >>> different form of computing the typedefs than src/tools/find_typedef ... >> That script is notably non-portable, and hasn't seen any significant >> update in a decade. > I think that's kinda what I'm complaining about... It seems like a bad > idea to have this in the buildfarm code, rather than our code. IMO the > buildfarm code should invoke an updated src/tools/find_typedef - that > way people at least can create typedefs manually locally. OK, I don't have a problem with that. I think the sane way to go would be to replace it with what the buildfarm is using, more or less. > > Not yet sure what's actually going on, but there's something odd with > debug information afaict: > > objdump -w spits out warnings for a few files, all static libraries: I assume you mean -W rather than -w > > ../install/lib/libpgcommon.a > objdump: Warning: Location list starting at offset 0x0 is not terminated. > objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists. > objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc > That seems new. I didn't get this in my attempt to recreate the setup of your animal. That was on debian/buster which has binutils 2.31.1-16 > Not sure if that's related - I don't get that locally (newer compiler, > different compiler options, same binutils). > > Interestingly pg_fprintf is only generated by pg_fprintf, as far as I > can tell, and the 1/2 typedefs are from libpq. The relevant entries look > like: > > <1><4b>: Abbrev Number: 4 (DW_TAG_typedef) > <4c> DW_AT_name : (indirect string, offset: 0x0): USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 1 > <50> DW_AT_decl_file : 2 > <51> DW_AT_decl_line : 216 > <52> DW_AT_decl_column : 23 > <53> DW_AT_type : <0x57> > > So I suspect it might be the corruption/misparsing of objdump that's to > blame. Kinda looks like there's an issue with the dwarf stringtable, and > thus the wrong strings (debug information for macros in this case) is > being picked up. > This looks like a bug in the version of objdump unless I'm reading it wrong. Why would this be tagged as a typedef? I would tentatively suggest that you revert to the previous version of binutils if possible, and consider reporting a bug in the bleeding edge of objdump. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 7/10/19 8:24 PM, Andres Freund wrote:
>> I think that's kinda what I'm complaining about... It seems like a bad
>> idea to have this in the buildfarm code, rather than our code. IMO the
>> buildfarm code should invoke an updated src/tools/find_typedef - that
>> way people at least can create typedefs manually locally.
> OK, I don't have a problem with that. I think the sane way to go would
> be to replace it with what the buildfarm is using, more or less.
+1 for the idea --- I've not studied the code, but surely the buildfarm's
version of this code is more bulletproof.
> This looks like a bug in the version of objdump unless I'm reading it
> wrong. Why would this be tagged as a typedef?
Maybe. We still need to explain the other non-typedef symbols that have
just appeared and are not being reported by calliphoridae.
regards, tom lane
On 7/11/19 12:50 PM, Tom Lane wrote: > >> This looks like a bug in the version of objdump unless I'm reading it >> wrong. Why would this be tagged as a typedef? > Maybe. We still need to explain the other non-typedef symbols that have > just appeared and are not being reported by calliphoridae. > The others come from komodoensis (also Andres' machine) pgbfprod=> select l.sysname, l.snapshot, l.branch from build_status_log l where snapshot > now() - interval '12 days' and log_stage = 'typedefs.log' and log_text ~ 'ECPGt_bytea|in_addr|connection_name|send_appname'; sysname | snapshot | branch -------------+---------------------+-------- komodoensis | 2019-07-08 23:34:01 | HEAD komodoensis | 2019-07-10 02:38:01 | HEAD (2 rows) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi,
On 2019-07-10 17:24:41 -0700, Andres Freund wrote:
> Not yet sure what's actually going on, but there's something odd with
> debug information afaict:
>
> objdump -W spits out warnings for a few files, all static libraries:
>
> ../install/lib/libpgcommon.a
> objdump: Warning: Location list starting at offset 0x0 is not terminated.
> objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
> objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
...
Interestingly, for the same files, readelf spits out correct
data. E.g. here's a short excerpt from libpq.a:
objdump -W src/interfaces/libpq/libpq.a
...
<0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
<c> DW_AT_producer : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
<10> DW_AT_language : 12 (ANSI C99)
<11> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
<15> DW_AT_comp_dir : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
...
<1><31>: Abbrev Number: 2 (DW_TAG_typedef)
<32> DW_AT_name : Oid
<36> DW_AT_decl_file : 30
<37> DW_AT_decl_line : 31
<38> DW_AT_decl_column : 22
<39> DW_AT_type : <0x3d>
<1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
<3e> DW_AT_byte_size : 4
<3f> DW_AT_encoding : 7 (unsigned)
<40> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
<1><44>: Abbrev Number: 3 (DW_TAG_base_type)
<45> DW_AT_byte_size : 8
<46> DW_AT_encoding : 5 (signed)
<47> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
<1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
<4c> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
<50> DW_AT_decl_file : 2
<51> DW_AT_decl_line : 216
<52> DW_AT_decl_column : 23
<53> DW_AT_type : <0x57>
...
readelf --debug-dump src/interfaces/libpq/libpq.a
...
<0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
<c> DW_AT_producer : (indirect string, offset: 0x29268): GNU C17 8.3.0 -march=skylake -mmmx -mno-3dnow -msse
-msse2-m
<10> DW_AT_language : 12 (ANSI C99)
<11> DW_AT_name : (indirect string, offset: 0x28ef3):
/home/andres/src/postgresql/src/interfaces/libpq/fe-auth.c
<15> DW_AT_comp_dir : (indirect string, offset: 0xf800):
/home/andres/build/postgres/dev-assert/vpath/src/interfaces/l
...
<1><31>: Abbrev Number: 2 (DW_TAG_typedef)
<32> DW_AT_name : Oid
<36> DW_AT_decl_file : 30
<37> DW_AT_decl_line : 31
<38> DW_AT_decl_column : 22
<39> DW_AT_type : <0x3d>
<1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
<3e> DW_AT_byte_size : 4
<3f> DW_AT_encoding : 7 (unsigned)
<40> DW_AT_name : (indirect string, offset: 0x4f12f): unsigned int
<1><44>: Abbrev Number: 3 (DW_TAG_base_type)
<45> DW_AT_byte_size : 8
<46> DW_AT_encoding : 5 (signed)
<47> DW_AT_name : (indirect string, offset: 0x57deb): long int
<1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
<4c> DW_AT_name : (indirect string, offset: 0x26129): size_t
<50> DW_AT_decl_file : 2
<51> DW_AT_decl_line : 216
<52> DW_AT_decl_column : 23
<53> DW_AT_type : <0x57>
...
so it seems that objdump mis-parses all indirect strings - which IIRC is
something like a pointer into a "global" string table, assuming the
offset to be 0. That then just returns the first table entry.
It doesn't happen with a slightly older version of binutils. Bisecting
now.
Greetings,
Andres Freund
Hi,
On 2019-07-16 19:35:39 -0700, Andres Freund wrote:
> Hi,
>
> On 2019-07-10 17:24:41 -0700, Andres Freund wrote:
> > Not yet sure what's actually going on, but there's something odd with
> > debug information afaict:
> >
> > objdump -W spits out warnings for a few files, all static libraries:
> >
> > ../install/lib/libpgcommon.a
> > objdump: Warning: Location list starting at offset 0x0 is not terminated.
> > objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
> > objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
>
> ...
>
>
> Interestingly, for the same files, readelf spits out correct
> data. E.g. here's a short excerpt from libpq.a:
>
> objdump -W src/interfaces/libpq/libpq.a
> ...
> <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
> <c> DW_AT_producer : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> <10> DW_AT_language : 12 (ANSI C99)
> <11> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> <15> DW_AT_comp_dir : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> ...
> <1><31>: Abbrev Number: 2 (DW_TAG_typedef)
> <32> DW_AT_name : Oid
> <36> DW_AT_decl_file : 30
> <37> DW_AT_decl_line : 31
> <38> DW_AT_decl_column : 22
> <39> DW_AT_type : <0x3d>
> <1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
> <3e> DW_AT_byte_size : 4
> <3f> DW_AT_encoding : 7 (unsigned)
> <40> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> <1><44>: Abbrev Number: 3 (DW_TAG_base_type)
> <45> DW_AT_byte_size : 8
> <46> DW_AT_encoding : 5 (signed)
> <47> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
> <4c> DW_AT_name : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> <50> DW_AT_decl_file : 2
> <51> DW_AT_decl_line : 216
> <52> DW_AT_decl_column : 23
> <53> DW_AT_type : <0x57>
> so it seems that objdump mis-parses all indirect strings - which IIRC is
> something like a pointer into a "global" string table, assuming the
> offset to be 0. That then just returns the first table entry.
>
> It doesn't happen with a slightly older version of binutils. Bisecting
> now.
It's
commit 39f0547e554df96608dd041d2a7b3c72882fd515 (HEAD, refs/bisect/bad)
Author: Nick Clifton <nickc@redhat.com>
Date: 2019-02-25 12:15:41 +0000
Extend objdump's --dwarf=follow-links option so that separate debug info files will also be affected by other dump
function,and symbol tables from separate debug info files will be used when disassembling the main file.
* objdump.c (sym_ok): New function.
(find_symbol_for_address): Use new function.
(disassemble_section): Compare sections by name, not pointer.
(dump_dwarf): Move code to initialise byte_get pointer and iterate
over separate debug files from here to ...
(dump_bfd): ... here. Add parameter indicating that a separate
debug info file is being dumped. For main file, pull in the
symbol tables from all separate debug info files.
(display_object): Update call to dump_bfd.
* doc/binutils.texi: Document extened behaviour of the
--dwarf=follow-links option.
* NEWS: Mention this new feature.
* testsuite/binutils-all/objdump.WK2: Update expected output.
* testsuite/binutils-all/objdump.exp (test_follow_debuglink): Add
options and dump file parameters.
Add extra test.
* testsuite/binutils-all/objdump.WK3: New file.
* testsuite/binutils-all/readelf.exp: Change expected output for
readelf -wKis test.
* testsuite/binutils-all/readelf.wKis: New file.
luckily that allowed me to find a workaround too. If objdump -W's k and K
parameters (or --dwarf=links,=follow-links) aren't enabled, the dump
comes out correctly:
objdump -WlLiaprmfFsoRtUuTgAckK /tmp/test.o|grep -A5 '(DW_TAG_compile_unit)'
<0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
<c> DW_AT_producer : (indirect string, offset: 0x0): /home/andres
<10> DW_AT_language : 12 (ANSI C99)
<11> DW_AT_name : (indirect string, offset: 0x0): /home/andres
<15> DW_AT_comp_dir : (indirect string, offset: 0x0): /home/andres
<19> DW_AT_low_pc : 0x0
objdump -WlLiaprmfFsoRtUuTgAc /tmp/test.o|grep -A5 '(DW_TAG_compile_unit)'
<0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
<c> DW_AT_producer : (indirect string, offset: 0x2a): GNU C17 9.1.0 -mtune=generic -march=x86-64 -ggdb
-fasynchronous-
<10> DW_AT_language : 12 (ANSI C99)
<11> DW_AT_name : (indirect string, offset: 0x14): /tmp/test.c
<15> DW_AT_comp_dir : (indirect string, offset: 0x0): /home/andres
<19> DW_AT_low_pc : 0x0
(lLiaprmfFsoRtUuTgAckK just is all sub-parts of -W that my objdump knows
about)
looking at the .debug_str section (with -Ws):
0x00000000 2f686f6d 652f616e 64726573 00646f75 /home/andres.dou
0x00000010 626c6500 2f746d70 2f746573 742e6300 ble./tmp/test.c.
0x00000020 72657431 00726574 3200474e 55204331 ret1.ret2.GNU C1
0x00000030 3720392e 312e3020 2d6d7475 6e653d67 7 9.1.0 -mtune=g
0x00000040 656e6572 6963202d 6d617263 683d7838 eneric -march=x8
0x00000050 362d3634 202d6767 6462202d 66617379 6-64 -ggdb -fasy
0x00000060 6e636872 6f6e6f75 732d756e 77696e64 nchronous-unwind
0x00000070 2d746162 6c657300 666f6f62 61723100 -tables.foobar1.
0x00000080 666f6f62 61723200 foobar2.
it makes sense why in this case all strings are /home/andres, and also
why the precise symbols output into the typedef list appears to be
pretty random - it's just the first string, as the offsets are
mis-computed. And not all strings are put into the string table, which
explains why the output still has some contents left.
It turns out that -Wi is actually all we need - so I'll probably patch
my animals to use that for now, until the bug is fixed.
It might actually be sensible to always do that - it's a lot cheaper
that way:
$ time objdump -WlLiaprmfFsoRtUuTgAc src/interfaces/libpq/libpq.a|wc
747866 5190832 48917526
real 0m0.827s
user 0m1.040s
sys 0m0.074s
$ time objdump -Wi src/interfaces/libpq/libpq.a|wc
78703 378433 3594563
real 0m0.075s
user 0m0.076s
sys 0m0.025s
Greetings,
Andres Freund
On 7/17/19 1:21 AM, Andres Freund wrote: [nice forensics] > > It turns out that -Wi is actually all we need - so I'll probably patch > my animals to use that for now, until the bug is fixed. > > It might actually be sensible to always do that - it's a lot cheaper > that way: > > $ time objdump -WlLiaprmfFsoRtUuTgAc src/interfaces/libpq/libpq.a|wc > 747866 5190832 48917526 > > real 0m0.827s > user 0m1.040s > sys 0m0.074s > > $ time objdump -Wi src/interfaces/libpq/libpq.a|wc > 78703 378433 3594563 > > real 0m0.075s > user 0m0.076s > sys 0m0.025s > WFM, I'll put that in the next buildfarm client release, unless we get a core script to use instead in the meantime. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, On 2019-07-16 22:21:34 -0700, Andres Freund wrote: > It turns out that -Wi is actually all we need - so I'll probably patch > my animals to use that for now did that now. > until the bug is fixed. Bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=24818 Greetings, Andres Freund
Hi, On 2019-07-17 10:33:02 -0700, Andres Freund wrote: > On 2019-07-16 22:21:34 -0700, Andres Freund wrote: > > It turns out that -Wi is actually all we need - so I'll probably patch > > my animals to use that for now > > did that now. Looks like that made the generated typedef lists sane. Any residual complaints? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> Looks like that made the generated typedef lists sane. Any residual
> complaints?
Before ack'ing this, I've been waiting around for komodoensis to update
its typedefs list, in the hopes that the last couple of bogus entries
would go away. It still hasn't, and I just realized from looking at its
config that you have it set to do so at most twice a week:
'optional_steps' => {
'find_typedefs' => {
'branches' => [
'HEAD'
],
'min_hours_since' => 25,
'dow' => [
1,
4
]
}
},
That seems a bit, er, miserly.
regards, tom lane
I wrote:
> ... I just realized from looking at its
> config that you have it set to do so at most twice a week:
> 'dow' => [
> 1,
> 4
> ]
I was still confused, seeing that today is Thursday, as to why
komodoensis didn't update its typedefs list in the run it just
finished. Looking at the buildfarm script (in sub
check_optional_step), it seems the "dow" filter is implemented
like this:
return
if (exists $oconf->{dow}
&& grep { $_ eq $wday } @{ $oconf->{dow} });
I'm the world's worst Perl programmer, but isn't that backwards?
It seems like it will return undef if today matches any entry
of the dow list, making dow a blacklist of weekdays *not* to run
the step on. That's not what I would have expected it to mean,
although build-farm.conf.sample is surely unclear on the point.
regards, tom lane
Gum
On 2019-07-18 11:27:49 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Looks like that made the generated typedef lists sane. Any residual
> > complaints?
>
> Before ack'ing this, I've been waiting around for komodoensis to update
> its typedefs list, in the hopes that the last couple of bogus entries
> would go away. It still hasn't, and I just realized from looking at its
> config that you have it set to do so at most twice a week:
I've changed that yesterday to be just min_hours_since=48. I can easily set it
to more frequent, especially after using -Wi makes the whole step a lot
faster - I just though that swamping the buildfarm with the additional
data would be unnecessary? But perhaps it doesn't matter in comparison
to the normal stage logs.
Boosted to min_hours_since=24 for now.
> 'optional_steps' => {
> 'find_typedefs' => {
> 'branches' => [
> 'HEAD'
> ],
> 'min_hours_since' => 25,
> 'dow' => [
> 1,
> 4
> ]
> }
> },
>
> That seems a bit, er, miserly.
Comes from the default suggestion in build-farm.conf.sample:
# find_typedefs => { branches => ['HEAD'], dow => [1,4],
# min_hours_since => 25 },
I'm happy to boost that to something a lot more aggressive.
At least the REL_11_STABLE logs are from after the change to use -Wi
instead of -W, and therefore ought to be correct.
bf@andres-postgres-edb-buildfarm-v1:~/src/pgbuildfarm-client-stock$ TZ=UTC ls -l run_build.pl
-rwxr-xr-x 1 bf bf 63701 Jul 17 17:16 run_build.pl
komodoensis 2019-07-17 03:04:01 HEAD 3308
komodoensis 2019-07-17 17:32:02 REL_12_STABLE 3296
komodoensis 2019-07-17 23:18:24 REL_11_STABLE 3204
Greetings,
Andres Freund
> On 18 Jul 2019, at 17:42, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> return
> if (exists $oconf->{dow}
> && grep { $_ eq $wday } @{ $oconf->{dow} });
>
> I'm the world's worst Perl programmer, but isn't that backwards?
> It seems like it will return undef if today matches any entry
> of the dow list, making dow a blacklist of weekdays *not* to run
> the step on.
As it’s in a scalar context, grep will return the number of times for which the
condition is true against the list entries, so if $oconf->{dow} doesn’t have
duplicates it will return 1 if $wday is found on the list. This will in turn
make the condition true as the exists expression must be true for grep to at
all execute.
cheers ./daniel
Andres Freund <andres@anarazel.de> writes:
> On 2019-07-18 11:27:49 -0400, Tom Lane wrote:
>> Before ack'ing this, I've been waiting around for komodoensis to update
>> its typedefs list, in the hopes that the last couple of bogus entries
>> would go away. It still hasn't, and I just realized from looking at its
>> config that you have it set to do so at most twice a week:
> Boosted to min_hours_since=24 for now.
OK, I see it's updated, and we now have a sane typedefs list again:
$ diff -u src/tools/pgindent/typedefs.list new
--- src/tools/pgindent/typedefs.list 2019-07-02 10:32:57.078918638 -0400
+++ new/typedefs.list 2019-07-18 12:45:22.533451991 -0400
@@ -674,6 +674,12 @@
FmgrBuiltin
FmgrHookEventType
FmgrInfo
+ForBothCellState
+ForBothState
+ForEachState
+ForFiveState
+ForFourState
+ForThreeState
ForeignDataWrapper
ForeignKeyCacheInfo
ForeignKeyOptInfo
@@ -2981,7 +2987,7 @@
leaf_item
line_t
lineno_t
-list_qsort_comparator
+list_sort_comparator
locale_t
locate_agg_of_level_context
locate_var_of_level_context
@@ -3150,6 +3156,7 @@
pullup_replace_vars_context
pushdown_safety_info
qsort_arg_comparator
+qsort_comparator
query_pathkeys_callback
radius_attribute
radius_packet
All of those changes are correct, so we're good. Thanks!
(I still think the buildfarm's dow filter is wrong though)
regards, tom lane
On 7/18/19 12:17 PM, Daniel Gustafsson wrote:
>> On 18 Jul 2019, at 17:42, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> return
>> if (exists $oconf->{dow}
>> && grep { $_ eq $wday } @{ $oconf->{dow} });
>>
>> I'm the world's worst Perl programmer, but isn't that backwards?
>> It seems like it will return undef if today matches any entry
>> of the dow list, making dow a blacklist of weekdays *not* to run
>> the step on.
> As it’s in a scalar context, grep will return the number of times for which the
> condition is true against the list entries, so if $oconf->{dow} doesn’t have
> duplicates it will return 1 if $wday is found on the list. This will in turn
> make the condition true as the exists expression must be true for grep to at
> all execute.
>
Yes, it's a bug. Will fix.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services