Thread: pgindent run coming
It is time to run pgindent on CVS HEAD for 8.4. I am thinking of running it at zero-hour GMT tomorrow, meaning five hours from now. Any objections? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > It is time to run pgindent on CVS HEAD for 8.4. I am thinking of > running it at zero-hour GMT tomorrow, meaning five hours from now. > Any objections? I ran pgindent and was concerned enough about the results so I am posting here rather than applying any changes. I used the old way of generating typedefs and the new buildfarm method output from diffs from http://www.pgbuildfarm.org/cgi-bin/typedefs.pl. The typedef lists and diff are here: http://momjian.us/expire/pgindent/ You can see the typedef lists are of similar size: 2775 typedefs.old2123 typedefs.new and the diffs generates are a similar number of lines:133657 diff.old_typedefs135042 diff.new_typedefs I saw a few odd things. Most importantly, it seems 'stat' was introduced as a typedef on _both_ lists, yielding weird changes like: - ret = stat(indir, &st); + ret = stat (indir, &st); and even odder: - stat->weight |= 1 << 3; + stat ->weight |= 1 << 3; stat was not a typedef in 8.3 or pgindent would have done this for 8.3, but I can't figure out what has changed to make it appear for 8.4. I see this in the objdump output (my OS has not changed from 8.3): 31357 EXCL 0 0 00003e64 97648 /usr/include/time.h31358 EXCL 0 0 00007638 97624 /usr/include/sys/time.h31359 EXCL 0 0 00000000 97648 /usr/include/time.h31360 EXCL 0 0 00001dff25540 /usr/include/fcntl.h31361 BINCL 0 0 000144be 449667 /usr/include/sys/stat.h 31362 LSYM 0 0 00000000 449691 ostat:T(51,1)=s64st_dev:(0,9),0,16;st_ino:(9,20),32,32;st_mode:(9,22),64,16;st_nlink:(9,23),80,16;st_uid:(0,9),96,16;st_gid:(0,9),112,16;st_rdev:(0,9),128,16;st_size:(0,3),160,32;st_atimespec:(48,2),192,64;st_mtimespec:(48,2),256,64;st_ctimespec:(48,2),320,64;st_blksize:(0,3),384,32;st_blocks:(0,3),416,32;st_flags:(0,5),448,32;st_gen:(0,5),480,32;; 31363 LSYM 0 0 00000000 450042 stat32:T(51,2)=s96st_dev:(9,17),0,32;st_ino:(9,20),32,32;st_mode:(9,22),64,16;st_nlink:(9,23),80,16;st_uid:(9,28),96,32;st_gid:(9,19),128,32;st_rdev:(9,17),160,32;st_atimespec:(48,2),192,64;st_mtimespec:(48,2),256,64;st_ctimespec:(48,2),320,64;st_size:(0,3),384,32;st_size1:(0,3),416,32;st_blocks:(0,3),448,32;st_blocks1:(0,3),480,32;st_blksize:(0,5),512,32;st_flags:(0,5),544,32;st_gen:(0,5),576,32;st_lspare:(0,3),608,32;st_qspare:(51,3)=ar(0,1);0;3;(0,3),640,128;; --> 31364 LSYM 0 0 00000000 450510 stat:T(51,4)=s96st_dev:(9,17),0,32;st_ino:(9,20),32,32;st_mode:(9,22),64,16;st_nlink:(9,23),80,16;st_uid:(9,28),96,32;st_gid:(9,19),128,32;st_rdev:(9,17),160,32;st_atimespec:(48,2),192,64;st_mtimespec:(48,2),256,64;st_ctimespec:(48,2),320,64;st_size:(9,24),384,64;st_blocks:(9,8),448,64;st_blksize:(0,5),512,32;st_flags:(0,5),544,32;st_gen:(0,5),576,32;st_lspare:(0,3),608,32;st_qspare:(51,5)=ar(0,1);0;1;(9,8),640,128;; It is coming from the postgres binary. The typedef is coming from the indicated line, and from /usr/include/sys/stat.h, where there is no typedef for stat. Obviously Linux or the buildfarm is finding the same issue, but I have no idea why. My only guess right now is that we are linking postgres differently than we did for 8.3 and that is bringing in new wrong typedef symbols. I will have to research this further tomorrow. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > The typedef is coming from the indicated line, and from > /usr/include/sys/stat.h, where there is no typedef for stat. Obviously > Linux or the buildfarm is finding the same issue, but I have no idea > why. > > My only guess right now is that we are linking postgres differently than > we did for 8.3 and that is bringing in new wrong typedef symbols. > > I will have to research this further tomorrow. I was able to reproduce the incorrect stat typedef here in a small test program by just including "sys/stat.h" so I will research tomorrow how to fix this. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> I saw a few odd things. Most importantly, it seems 'stat' was
> introduced as a typedef on _both_ lists, yielding weird changes like:
The standard headers do define "struct stat". I wonder whether the
objdump kluge we are using is unable to distinguish typedef names
from struct tags.
> I will have to research this further tomorrow.
We don't have a lot of time for research. Maybe the best thing is to
just manually remove stat from the typedef list (along with anything
else that clearly shouldn't be there)?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I saw a few odd things. Most importantly, it seems 'stat' was > > introduced as a typedef on _both_ lists, yielding weird changes like: > > The standard headers do define "struct stat". I wonder whether the > objdump kluge we are using is unable to distinguish typedef names > from struct tags. > > > I will have to research this further tomorrow. > > We don't have a lot of time for research. Maybe the best thing is to > just manually remove stat from the typedef list (along with anything > else that clearly shouldn't be there)? The problem is that there are other symbols I don't know about and the diff is very large. I have found that the problem was caused when we added Linux support to find_typedef and I have a way to get an accurate list on my machine. I will generate a proper list on my machine tomorrow then test Linux here to see if I can get it to generate the right list too. But odds are we are not going to have time to re-run the list on the build farm even if I can get Linux working here. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I saw a few odd things. Most importantly, it seems 'stat' was > > introduced as a typedef on _both_ lists, yielding weird changes like: > > The standard headers do define "struct stat". I wonder whether the > objdump kluge we are using is unable to distinguish typedef names > from struct tags. > > > I will have to research this further tomorrow. > > We don't have a lot of time for research. Maybe the best thing is to > just manually remove stat from the typedef list (along with anything > else that clearly shouldn't be there)? Do you want me to just run with my old typedef list now and apply it? We an always rerun tomorrow if we get a better typedef list. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> We don't have a lot of time for research. Maybe the best thing is to
>> just manually remove stat from the typedef list (along with anything
>> else that clearly shouldn't be there)?
> Do you want me to just run with my old typedef list now and apply it?
> We an always rerun tomorrow if we get a better typedef list.
I'd rather have *one* run with the final typedef list. If you don't
have that list yet, wait till you do.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> We don't have a lot of time for research. Maybe the best thing is to > >> just manually remove stat from the typedef list (along with anything > >> else that clearly shouldn't be there)? > > > Do you want me to just run with my old typedef list now and apply it? > > We an always rerun tomorrow if we get a better typedef list. > > I'd rather have *one* run with the final typedef list. If you don't > have that list yet, wait till you do. OK, Andrew, would you use the find_typedef file that is in CVS HEAD and run that. I think that will fix our problem and then I can use the buildfarm version. How often does that run and does it pull the script from CVS HEAD? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote:
> OK, Andrew, would you use the find_typedef file that is in CVS HEAD and
> run that. I think that will fix our problem and then I can use the
> buildfarm version. How often does that run and does it pull the script
> from CVS HEAD?
>
>
The buildfarm does not run the find-typedefs script. Its code for this
is below. My Unix machine runs this once a day. I can do runs on Windows
and Cygwin manually.
If there is changed logic tell me what it is and I'll try to get it done.
cheers
andrew
-----------------
sub find_typedefs
{
my @err = `objdump -W 2>&1`;
@err = () if `uname -s 2>&1` =~ /CYGWIN/i;
my %syms;
my @dumpout;
my @flds;
foreach my $bin (glob("$installdir/bin/*"),
glob("$installdir/lib/*"),
glob("$installdir/lib/postgresql/*"))
{
next if $bin =~ m!bin/(ipcclean|pltcl_)!;
next unless -f $bin;
if (@err == 1) # Linux
{
@dumpout = `objdump -W $bin 2>/dev/null | egrep -A3
'(DW_TAG_typedef|DW_TAG_structure_type|DW_TAG_union_type)' 2>/dev/null`;
foreach (@dumpout)
{
@flds = split;
next if (($flds[0] ne 'DW_AT_name' && $flds[1] ne 'DW_AT_name' ) ||
$flds[-1] =~ /^DW_FORM_str/);
$syms{$flds[-1]} =1;
}
}
else
{
@dumpout = `objdump --stabs $bin 2>/dev/null`;
foreach (@dumpout)
{
@flds = split;
next if (@flds < 7);
next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):[tT]/);
$syms{$1} =1;
}
}
}
my @badsyms = grep { /\s/ } keys %syms;
push(@badsyms,'date','interval','timestamp','ANY');
delete @syms{@badsyms};
my @goodsyms = sort keys %syms;
my @foundsyms;
my %foundwords;
my $setfound = sub
{
return unless (-f $_ && /^.*\.[chly]\z/);
my @lines;
my $handle;
open ($handle,$_);
while (my $line=<$handle>)
{
foreach my $word (split(/\W+/,$line))
{
$foundwords{$word} = 1;
}
}
close($handle);
};
File::Find::find($setfound,"$branch_root/pgsql");
foreach my $sym (@goodsyms)
{
push(@foundsyms,"$sym\n") if exists $foundwords{$sym};
}
writelog('typedefs',\@foundsyms);
$steps_completed .= " find-typedefs";
}
On Tue, 2009-06-09 at 13:21 -0400, Bruce Momjian wrote: > It is time to run pgindent on CVS HEAD for 8.4. I am thinking of > running it at zero-hour GMT tomorrow, meaning five hours from now. > Any objections? Why don't we do this automatically after each individual commit? That way each commit would be associated directly with any required tidy-up that must occur because of it. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > OK, Andrew, would you use the find_typedef file that is in CVS HEAD and > > run that. I think that will fix our problem and then I can use the > > buildfarm version. How often does that run and does it pull the script > > from CVS HEAD? > > > > > > The buildfarm does not run the find-typedefs script. Its code for this > is below. My Unix machine runs this once a day. I can do runs on Windows > and Cygwin manually. > > If there is changed logic tell me what it is and I'll try to get it done. Yes, please make the attached change, and rerun the script. This change reflects the changes I made in src/tools/find_typedef. What value is there if the URL I have just runs on Linux? It probably has the same coverage I have for BSD. I asked for something that was automated for more platforms a year ago and I still don't have it? I might as well just use the same method I have have used for years. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + *** ./script.pl.orig Wed Jun 10 09:39:27 2009 --- ./script.pl Wed Jun 10 09:39:39 2009 *************** *** 30,36 **** { @flds = split; next if (@flds < 7); ! next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):[tT]/); $syms{$1} =1; } } --- 30,36 ---- { @flds = split; next if (@flds < 7); ! next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):t/); $syms{$1} =1; } }
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Tue, 2009-06-09 at 13:21 -0400, Bruce Momjian wrote:
>> It is time to run pgindent on CVS HEAD for 8.4. I am thinking of
>> running it at zero-hour GMT tomorrow, meaning five hours from now.
> Why don't we do this automatically after each individual commit?
It's not very practical until the typedef extraction stuff is fully
automatic and fully trustworthy; right now some manual review and
intervention still seems like a good idea.
Also, that would risk breaking series of interdependent patches.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I saw a few odd things. Most importantly, it seems 'stat' was > > introduced as a typedef on _both_ lists, yielding weird changes like: > > The standard headers do define "struct stat". I wonder whether the > objdump kluge we are using is unable to distinguish typedef names > from struct tags. > > > I will have to research this further tomorrow. > > We don't have a lot of time for research. Maybe the best thing is to > just manually remove stat from the typedef list (along with anything > else that clearly shouldn't be there)? I agree we are running out of time so I will be running pgindent in one hour using the same BSD-based typedef method I have used in the past. If a better typedef list appears, we can always rerun pgindent. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> We don't have a lot of time for research. Maybe the best thing is to
>> just manually remove stat from the typedef list (along with anything
>> else that clearly shouldn't be there)?
> I agree we are running out of time so I will be running pgindent in one
> hour using the same BSD-based typedef method I have used in the past.
> If a better typedef list appears, we can always rerun pgindent.
Why not compare the BSD list with the new one and adopt anything that
seems sane? We know that your old method misses quite a lot of typedefs.
regards, tom lane
Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Bruce Momjian wrote:
>>
>>> OK, Andrew, would you use the find_typedef file that is in CVS HEAD and
>>> run that. I think that will fix our problem and then I can use the
>>> buildfarm version. How often does that run and does it pull the script
>>> from CVS HEAD?
>>>
>>>
>>>
>> The buildfarm does not run the find-typedefs script. Its code for this
>> is below. My Unix machine runs this once a day. I can do runs on Windows
>> and Cygwin manually.
>>
>> If there is changed logic tell me what it is and I'll try to get it done.
>>
>
> Yes, please make the attached change, and rerun the script. This change
> reflects the changes I made in src/tools/find_typedef.
>
> What value is there if the URL I have just runs on Linux? It probably
> has the same coverage I have for BSD. I asked for something that was
> automated for more platforms a year ago and I still don't have it? I
> might as well just use the same method I have have used for years.
>
>
Well, sometimes I build it and they don't come ;-).
I don't have every platform under the sun that I can run this on,
although I do now have an FBSD VM that I didn't have when I worked on
this previously. If you're actually going to use it I'll set it up as a
buildfarm member and run the find-typedefs there.
I will make the change you request and rerun the stuff on the platforms
I have that are currently buildfarm members, but AFAICT the Linux output
will still include the stat symbol (your change doesn't affect the Linux
branch at all).
Both my version and yours contains provision for filtering out certain
symbols ('timestamp' etc). Maybe we need to add 'stat' to that list.
cheers
andrew
> ------------------------------------------------------------------------
>
> *** ./script.pl.orig Wed Jun 10 09:39:27 2009
> --- ./script.pl Wed Jun 10 09:39:39 2009
> ***************
> *** 30,36 ****
> {
> @flds = split;
> next if (@flds < 7);
> ! next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):[tT]/);
> $syms{$1} =1;
> }
> }
> --- 30,36 ----
> {
> @flds = split;
> next if (@flds < 7);
> ! next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):t/);
> $syms{$1} =1;
> }
> }
>
Andrew Dunstan wrote: > Well, sometimes I build it and they don't come ;-). > > I don't have every platform under the sun that I can run this on, > although I do now have an FBSD VM that I didn't have when I worked on > this previously. If you're actually going to use it I'll set it up as a > buildfarm member and run the find-typedefs there. > > I will make the change you request and rerun the stuff on the platforms > I have that are currently buildfarm members, but AFAICT the Linux output > will still include the stat symbol (your change doesn't affect the Linux > branch at all). Good point. Here is another diff I need you to make to the pl file. If you want to make your pl file the official version and replace the shell script in CVS, that is fine with me. Do you want me to do that? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + *** ./script.pl.orig Wed Jun 10 09:39:27 2009 --- ./script.pl Wed Jun 10 11:09:36 2009 *************** *** 14,20 **** if (@err == 1) # Linux { @dumpout = `objdump -W $bin 2>/dev/null | egrep -A3 ! '(DW_TAG_typedef|DW_TAG_structure_type|DW_TAG_union_type)' 2>/dev/null`; foreach (@dumpout) { @flds = split; --- 14,20 ---- if (@err == 1) # Linux { @dumpout = `objdump -W $bin 2>/dev/null | egrep -A3 ! 'DW_TAG_typedef' 2>/dev/null`; foreach (@dumpout) { @flds = split;
Bruce Momjian wrote: > > Good point. Here is another diff I need you to make to the pl file. > Done. Linux run under way. > If you want to make your pl file the official version and replace the > shell script in CVS, that is fine with me. Do you want me to do that? > > It needs to be done in two pieces, I think: a perl module that exports a function that can be called from the buildfarm, and program that calls it so it can be run standalone. The function needs two inputs: an installation directory and a source directory. Then we will be able to stay in sync nicely. I will try to do that over the next day or so if you like. Or we can just wing it for now and do this after the release. cheers andrew
Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > > I saw a few odd things. Most importantly, it seems 'stat' was > > > introduced as a typedef on _both_ lists, yielding weird changes like: > > > > The standard headers do define "struct stat". I wonder whether the > > objdump kluge we are using is unable to distinguish typedef names > > from struct tags. > > > > > I will have to research this further tomorrow. > > > > We don't have a lot of time for research. Maybe the best thing is to > > just manually remove stat from the typedef list (along with anything > > else that clearly shouldn't be there)? > > The problem is that there are other symbols I don't know about and the > diff is very large. I have found that the problem was caused when we > added Linux support to find_typedef and I have a way to get an accurate > list on my machine. OK, I have found the cause of the script error, and it was my fault. A month after we ran pgindent for 8.3 (December 2007), I received this issue from Tom: http://archives.postgresql.org/pgsql-hackers/2007-12/msg00800.php > Something I noticed the other day is that pgindent doesn't seem to treat > "struct foo" or "union foo" as a type name, which is pretty silly > because no context is needed at all to recognize that. We tend not to > do that too much --- the project style is to use a typedef name --- but > there are some places that do it, particularly the regex code. For > instance there are extra spaces here: > > static void > cmtreefree(struct colormap * cm, > union tree * tree, > int level) /* level number (top == 0) of this > block */ > { > > Fixable? Not understanding the ramifications of adding struct and union tags to the typedef list, I modified the BSD code: http://archives.postgresql.org/pgsql-hackers/2007-12/msg00810.php > Yes, I found those are 't' STABS rather than "T" which are used in cases > where you do typedef struct {} name. The next pgindent will have those > typedefs you want. and that modification was propogated to the Linux code. This has now been fixed in the BSD and Linux code (and Perl script) and we can move forward with running pgindent once Andrew has lists for all the platforms he wants. As for Tom's original complaint about, it seems BSD indent is just not smart enough about struct/union tags. I will look into fixing that after 8.4 pgindent is run. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > > > Good point. Here is another diff I need you to make to the pl file. > > > > Done. Linux run under way. > > > If you want to make your pl file the official version and replace the > > shell script in CVS, that is fine with me. Do you want me to do that? > > > > > > It needs to be done in two pieces, I think: a perl module that exports a > function that can be called from the buildfarm, and program that calls > it so it can be run standalone. The function needs two inputs: an > installation directory and a source directory. Then we will be able to > stay in sync nicely. I will try to do that over the next day or so if > you like. > > Or we can just wing it for now and do this after the release. We have to wing it for now. Please let me know when whatever lists you can generate are ready, hopefully in the next few hours. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes: > OK, I have found the cause of the script error, and it was my fault. A > month after we ran pgindent for 8.3 (December 2007), I received this > issue from Tom: > http://archives.postgresql.org/pgsql-hackers/2007-12/msg00800.php >> Something I noticed the other day is that pgindent doesn't seem to treat >> "struct foo" or "union foo" as a type name, which is pretty silly >> because no context is needed at all to recognize that. Ah. So really the point here is that we want to specifically exclude "struct stat" because there are too many places in our code where "stat" is used as a regular identifier. Are there any other special cases like that? regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > OK, I have found the cause of the script error, and it was my fault. A > > month after we ran pgindent for 8.3 (December 2007), I received this > > issue from Tom: > > > http://archives.postgresql.org/pgsql-hackers/2007-12/msg00800.php > >> Something I noticed the other day is that pgindent doesn't seem to treat > >> "struct foo" or "union foo" as a type name, which is pretty silly > >> because no context is needed at all to recognize that. > > Ah. So really the point here is that we want to specifically exclude > "struct stat" because there are too many places in our code where "stat" > is used as a regular identifier. Are there any other special cases like > that? Yep, lots. I see "option" also doing strange things, and some others. You can see the diff here: http://momjian.us/expire/pgindent/http://momjian.us/expire/pgindent/ Basically that list is meant for typedefs, not struct or union tags. The BSD indent manual page says: -Ttypename Adds typename to the list of type keywords. Names accu- mulate: -T can be specifiedmore than once. You need to specify all the typenames that appear in your program that are defined by typedef - nothing will be harmed if you miss a few, but the program won'tbe formatted as nicely as it should. This sounds like a painful thing to haveto do, but it's really a symptom of a problem in C: typedef causes a syntactic change in the languageand indent can't find all instances of typedef. I am unclear why struct pointers are not being formatted properly in function headers but will research it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> I am unclear why struct pointers are not being formatted properly in
> function headers but will research it.
Yeah, if we can fix that directly without adding the names to the
typedef list, it would be better. But not something to do right now.
Have you started the pgindent run yet? I have a patch ready for
the cursor stability issue, but will hold off committing if it might
create a merge problem for you.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I am unclear why struct pointers are not being formatted properly in > > function headers but will research it. > > Yeah, if we can fix that directly without adding the names to the > typedef list, it would be better. But not something to do right now. > > Have you started the pgindent run yet? I have a patch ready for > the cursor stability issue, but will hold off committing if it might > create a merge problem for you. I am waiting for Andrew to tell me he is ready with updated lists for his platforms. His CGI output is now _not_ showing stat so I am comparing the lists now. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
>> Have you started the pgindent run yet? I have a patch ready for
>> the cursor stability issue, but will hold off committing if it might
>> create a merge problem for you.
> I am waiting for Andrew to tell me he is ready with updated lists for
> his platforms. His CGI output is now _not_ showing stat so I am
> comparing the lists now.
OK, I'll commit now and then stay out of your way.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > >> I am unclear why struct pointers are not being formatted properly in >> function headers but will research it. >> > > Yeah, if we can fix that directly without adding the names to the > typedef list, it would be better. But not something to do right now. > > Have you started the pgindent run yet? I have a patch ready for > the cursor stability issue, but will hold off committing if it might > create a merge problem for you. > > > I am doing runs as requested on various platforms to extract the typedef lists. Linux is done, Windows (mingw) is running, FBSD and Cygwin to come. Results in a few hours. The buildfarm will have a consolidated list. cheers andrew
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> >
> >> I am unclear why struct pointers are not being formatted properly in
> >> function headers but will research it.
> >>
> >
> > Yeah, if we can fix that directly without adding the names to the
> > typedef list, it would be better. But not something to do right now.
> >
> > Have you started the pgindent run yet? I have a patch ready for
> > the cursor stability issue, but will hold off committing if it might
> > create a merge problem for you.
> >
> >
> >
>
> I am doing runs as requested on various platforms to extract the typedef
> lists. Linux is done, Windows (mingw) is running, FBSD and Cygwin to come.
>
> Results in a few hours. The buildfarm will have a consolidated list.
OK, good. However, I am seeing missing typedefs in the Linux list you
posted at:
http://www.pgbuildfarm.org/cgi-bin/typedefs.pl
I pulled this list 20 minutes ago. I compared your list to the BSD list
I usually used and found that the Linux list had fewer typedefs:
2270 typedefs.old
1848 typedefs.new
I did a diff, attached, and found some typedefs that don't appear, like
PortalData. That is defined in our code as:
typedef struct PortalData *Portal;
typedef struct PortalData
{
/* Bookkeeping data */
...
bool visible; /* include this portal in pg_cursors? */
} PortalData;
I will try to build on Linux here and see how objdump displays that,
unless you can get me the output.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
5,16d4
< AMT
< AMTS
< ASN1_BIT_STRING
< ASN1_BMPSTRING
< ASN1_BOOLEAN
< ASN1_CTX
< ASN1_ENCODING
< ASN1_ENUMERATED
< ASN1_GENERALIZEDTIME
< ASN1_GENERALSTRING
< ASN1_HEADER
< ASN1_IA5STRING
18,24d5
< ASN1_ITEM
< ASN1_ITEM_EXP
< ASN1_METHOD
< ASN1_NULL
< ASN1_OBJECT
< ASN1_OCTET_STRING
< ASN1_PRINTABLESTRING
26,36d6
< ASN1_STRING_TABLE
< ASN1_T61STRING
< ASN1_TEMPLATE
< ASN1_TIME
< ASN1_TLC
< ASN1_TYPE
< ASN1_UNIVERSALSTRING
< ASN1_UTCTIME
< ASN1_UTF8STRING
< ASN1_VALUE
< ASN1_VISIBLESTRING
59d28
< AfterTriggerEventDataOneCtid
68d36
< AggHashEntryData
72d39
< AggStatePerAggData
80d46
< AllocBlockData
82d47
< AllocChunkData
142d106
< AutoVacuumSignal
149d112
< BINOP
151d113
< BIO_F_BUFFER_CTX
153d114
< BIO_dummy
156,157d116
< BIT_STRING_BITNAME
< BLOCK
159d117
< BN_BLINDING
161,162d118
< BN_MONT_CTX
< BN_RECP_CTX
175d130
< BTScanPos
184d138
< BUF_MEM
187d140
< BackslashQuoteType
212d164
< BoolPtr
221d172
< BufferAccessStrategyData
231,233d181
< BulkInsertStateData
< Byte
< Bytef
241,259d188
< COMP_CTX
< COMP_METHOD
< CONDOP
< CONF
< CONF_IMODULE
< CONF_METHOD
< CONF_MODULE
< CONF_VALUE
< COP
< CPFunction
< CPPFunction
< CRYPTO_EX_DATA
< CRYPTO_EX_DATA_FUNCS
< CRYPTO_EX_DATA_IMPL
< CRYPTO_EX_dup
< CRYPTO_EX_free
< CRYPTO_EX_new
< CRYPTO_MEM_LEAK_CB
< CRYPTO_dynlock
261d189
< CURCUR
298d225
< CoercionCodes
301d227
< CoercionMethod
326d251
< ConstraintExclusionType
360d284
< CreateStmtLikeOption
365d288
< Cred
376d298
< DCH_poz
381d302
< DH_METHOD
388,389d308
< DSA_METHOD
< DSA_SIG
395d313
< DatumPtr
417d334
< Dl_info
442,448d358
< ENGINE_CIPHERS_PTR
< ENGINE_CMD_DEFN
< ENGINE_CTRL_FUNC_PTR
< ENGINE_DIGESTS_PTR
< ENGINE_GEN_FUNC_PTR
< ENGINE_GEN_INT_FUNC_PTR
< ENGINE_LOAD_KEY_PTR
450,452d359
< ERR_FNS
< ERR_STATE
< ERR_STRING_DATA
454,457d360
< EVP_CIPHER
< EVP_CIPHER_CTX
< EVP_CIPHER_INFO
< EVP_ENCODE_CTX
460d362
< EVP_PBE_KEYGEN
495d396
< FF
500d400
< FUNMAP
535d434
< FormData_pg_description
553d451
< FormData_pg_shdescription
578d475
< Form_pg_description
596d492
< Form_pg_shdescription
618d513
< Function
629d523
< GEN_SESSION_CB
642,644d535
< GP
< GV
< GVOP
647d537
< GenericOptionFlags
660d549
< GinScanEntryData
676d564
< GlobalTransactionData
701,702d588
< HE
< HEK
704d589
< HISTORY_STATE
721d605
< HashJoinTableData
723d606
< HashJoinTupleData
740d622
< HeapScanDescData
747d628
< I16
749d629
< I8
751d630
< IO
755d633
< IV64
759,760d636
< IndexAttributeBitMap
< IndexAttributeBitMapData
773d648
< IndexScanDescData
784d658
< InhPaths
805,806d678
< ItemLength
< ItemOffset
810d681
< JMPENV
818,819d688
< KEYMAP_ENTRY
< KEYMAP_ENTRY_ARRAY
822,828c691,693
< Keymap
< LHASH
< LHASH_COMP_FN_TYPE
< LHASH_DOALL_ARG_FN_TYPE
< LHASH_DOALL_FN_TYPE
< LHASH_HASH_FN_TYPE
< LHASH_NODE
---
> LDAP
> LDAPMessage
> LDAP_TIMEVAL
830d694
< LISTOP
839,840d702
< LOGOP
< LOOP
870d731
< LockInfo
876d736
< LockTagType
882d741
< MAGIC
884,888d742
< MD2_CTX
< MD4_CTX
< MD5_CTX
< MDC2_CTX
< MGVTBL
898d751
< MergeJoinClauseData
914,916d766
< NETSCAPE_CERT_SEQUENCE
< NETSCAPE_SPKAC
< NETSCAPE_SPKI
921d770
< NUM_poz
944c793
< OBJ_NAME
---
> OM_uint32
976d824
< Outrec
979d826
< PADOFFSET
981,988d827
< PBE2PARAM
< PBEPARAM
< PBKDF2PARAM
< PEM_CTX
< PEM_ENCODE_SEAL_CTX
< PEM_USER
< PERL_CONTEXT
< PERL_SI
992d830
< PGErrorVerbosity
1018d855
< PGSSTrackLevel
1037,1047d873
< PKCS7
< PKCS7_DIGEST
< PKCS7_ENCRYPT
< PKCS7_ENC_CONTENT
< PKCS7_ENVELOPE
< PKCS7_ISSUER_AND_SERIAL
< PKCS7_RECIP_INFO
< PKCS7_SIGNED
< PKCS7_SIGNER_INFO
< PKCS7_SIGN_ENVELOPE
< PKCS8_PRIV_KEY_INFO
1097c923,933
< PMOP
---
> PLyDatumToOb
> PLyDatumToObFunc
> PLyObToDatum
> PLyObToTuple
> PLyPlanObject
> PLyProcedure
> PLyResultObject
> PLyTupleToOb
> PLyTypeInfo
> PLyTypeInput
> PLyTypeOutput
1117d952
< PV
1119d953
< PVOP
1125d958
< PacketLen
1157d989
< PerlExitListEntry
1206d1037
< PortalData
1233d1063
< PsqlScanStateData
1239a1070,1074
> PyMethodDef
> PyObject
> PySequenceMethods
> PyTypeObject
> Py_ssize_t
1256,1260d1090
< RAND_METHOD
< RC2_KEY
< RC4_KEY
< REGEXP
< RIPEMD160_CTX
1268d1097
< RSA_METHOD
1271d1099
< RUHashEntryData
1316d1143
< ResourceOwnerData
1330d1156
< RewriteStateData
1344,1348d1169
< SHA224_CTX
< SHA256_CTX
< SHA384_CTX
< SHA512_CTX
< SHA_CTX
1362,1367d1182
< SSL2_STATE
< SSL3_BUFFER
< SSL3_RECORD
< SSL3_STATE
< SSL_CIPHER
< SSL_COMP
1369,1370d1183
< SSL_METHOD
< SSL_SESSION
1373d1185
< SUBLEXINFO
1375d1186
< SVOP
1377d1187
< SaveArchivePtr
1398d1207
< SetConstraintTrigger
1404d1212
< SetOpHashEntryData
1420,1421d1227
< SigHandler
< Sighandler_t
1423d1228
< Sigsave_t
1478d1282
< SysScanDescData
1520,1537d1323
< TclStubHooks
< TclStubs
< Tcl_AppInitProc
< Tcl_AsyncHandler
< Tcl_AsyncProc
< Tcl_CallFrame
< Tcl_Channel
< Tcl_ChannelProc
< Tcl_ChannelType
< Tcl_ChannelTypeVersion
< Tcl_CloseProc
< Tcl_CmdDeleteProc
< Tcl_CmdInfo
< Tcl_CmdProc
< Tcl_CmdTraceProc
< Tcl_Command
< Tcl_Condition
< Tcl_CreateFileHandlerProc
1539,1565d1324
< Tcl_DeleteFileHandlerProc
< Tcl_DriverBlockModeProc
< Tcl_DriverClose2Proc
< Tcl_DriverCloseProc
< Tcl_DriverFlushProc
< Tcl_DriverGetHandleProc
< Tcl_DriverGetOptionProc
< Tcl_DriverHandlerProc
< Tcl_DriverInputProc
< Tcl_DriverOutputProc
< Tcl_DriverSeekProc
< Tcl_DriverSetOptionProc
< Tcl_DriverWatchProc
< Tcl_DupInternalRepProc
< Tcl_Encoding
< Tcl_EncodingConvertProc
< Tcl_EncodingFreeProc
< Tcl_EncodingState
< Tcl_EncodingType
< Tcl_EolTranslation
< Tcl_Event
< Tcl_EventCheckProc
< Tcl_EventDeleteProc
< Tcl_EventProc
< Tcl_EventSetupProc
< Tcl_ExitProc
< Tcl_FileFreeProc
1567,1568d1325
< Tcl_FreeInternalRepProc
< Tcl_FreeProc
1570d1326
< Tcl_HashSearch
1572d1327
< Tcl_IdleProc
1574,1579d1328
< Tcl_InterpDeleteProc
< Tcl_MainLoopProc
< Tcl_MathProc
< Tcl_Mutex
< Tcl_Namespace
< Tcl_NamespaceDeleteProc
1581,1600d1329
< Tcl_Obj
< Tcl_ObjCmdProc
< Tcl_ObjType
< Tcl_PackageInitProc
< Tcl_PanicProc
< Tcl_Parse
< Tcl_PathType
< Tcl_Pid
< Tcl_QueuePosition
< Tcl_RegExp
< Tcl_RegExpIndices
< Tcl_RegExpInfo
< Tcl_SavedResult
< Tcl_SetFromAnyProc
< Tcl_SetTimerProc
< Tcl_Stat_
< Tcl_TcpAcceptProc
< Tcl_ThreadCreateProc
< Tcl_ThreadDataKey
< Tcl_ThreadId
1602,1612d1330
< Tcl_TimerProc
< Tcl_TimerToken
< Tcl_Token
< Tcl_Trace
< Tcl_UniChar
< Tcl_UpdateStringProc
< Tcl_Value
< Tcl_ValueType
< Tcl_Var
< Tcl_VarTraceProc
< Tcl_WaitForEventProc
1618d1335
< Thread
1634d1350
< TrackFunctionsLevel
1659d1374
< TupleHashTableData
1680d1394
< U16
1682,1687d1395
< U8
< UI
< UI_METHOD
< UI_STRING
< UNDO_LIST
< UNOP
1699d1406
< VFunction
1741d1447
< WindowObjectData
1745d1450
< WindowStatePerFuncData
1754d1458
< WorkerInfoData
1761,1772d1464
< X509_ALGOR
< X509_ATTRIBUTE
< X509_CERT_AUX
< X509_CERT_FILE_CTX
< X509_CINF
< X509_CRL
< X509_CRL_INFO
< X509_EXTENSION
< X509_HASH_DIR_CTX
< X509_INFO
< X509_LOOKUP
< X509_LOOKUP_METHOD
1775,1782d1466
< X509_OBJECT
< X509_OBJECTS
< X509_PKEY
< X509_PUBKEY
< X509_REQ
< X509_REQ_INFO
< X509_REVOKED
< X509_SIG
1785,1786d1468
< X509_TRUST
< X509_VAL
1800,1813d1481
< XPV
< XPVAV
< XPVBM
< XPVCV
< XPVFM
< XPVGV
< XPVHV
< XPVIO
< XPVIV
< XPVLV
< XPVMG
< XPVNV
< XPVUV
< XRV
1818d1485
< XmlBinaryType
1824d1490
< XmlStandaloneType
1829,1832d1494
< _LIB_VERSION_TYPE
< _RuneEntry
< _RuneLocale
< _RuneRange
1835,1836d1496
< _ossl_old_des_cblock
< _ossl_old_des_key_schedule
1838d1497
< alloc_func
1840d1498
< assoc_list
1849d1506
< bio_info_cb
1858d1514
< caddr_t
1860d1515
< cc_t
1862,1863d1516
< char
< charf
1868d1520
< clockid_t
1869a1522
> cmpfunc
1870a1524
> coercion
1872,1875d1525
< comment_t
< conf_finish_func
< conf_init_func
< const_DES_cblock
1882,1883d1531
< cv_flags_t
< daddr_t
1888a1537
> destructor
1891,1892d1539
< div_t
< double
1894,1906d1540
< dyn_MEM_free_cb
< dyn_MEM_malloc_cb
< dyn_MEM_realloc_cb
< dyn_dynlock_create_cb
< dyn_dynlock_destroy_cb
< dyn_dynlock_lock_cb
< dyn_lock_add_lock_cb
< dyn_lock_locking_cb
< dynamic_LOCK_fns
< dynamic_MEM_fns
< dynamic_bind_engine
< dynamic_fns
< dynamic_v_check_fn
1912d1545
< expectation
1915d1547
< fd_mask
1917d1548
< filter_t
1924d1554
< fixpt_t
1926d1555
< float
1932,1936c1561
< fmStringInfo
< formarray
< formfloat
< fpos_t
< free_func
---
> freefunc
1964a1590,1594
> gss_OID
> gss_buffer_desc
> gss_cred_id_t
> gss_ctx_id_t
> gss_name_t
1965a1596
> hashfunc
1971,1972d1601
< in_addr_t
< in_port_t
1977d1605
< init_f
1978a1607
> inquiry
1980d1608
< int
1983,1984d1610
< int16_t
< int16m_t
1990d1615
< int32m_t
1994,1995d1618
< int64_t
< int64m_t
1997,1999d1619
< int8_t
< int8m_t
< intf
2003d1622
< jmp_buf
2005a1625,1632
> krb5_auth_context
> krb5_ccache
> krb5_context
> krb5_error
> krb5_error_code
> krb5_keytab
> krb5_principal
> krb5_ticket
2008d1634
< ldiv_t
2033d1658
< nlink_t
2042,2045d1666
< op_tr_array
< opcode
< opindex
< optType
2046a1668
> pam_handle_t
2048d1669
< pem_password_cb
2056a1678
> pg_gssinfo
2097,2105d1718
< pthread_attr_t
< pthread_cond_t
< pthread_condattr_t
< pthread_key_t
< pthread_mutex_t
< pthread_mutexattr_t
< pthread_once_t
< pthread_t
< ptrdiff_t
2108,2110d1720
< pvcontents
< q128_t
< qaddr_t
2112d1721
< quad_t
2116d1724
< regcomp_t
2118d1725
< regexec_t
2121d1727
< register_t
2123d1728
< regnode
2141,2143d1745
< rune_t
< runops_proc_t
< sa_family_t
2145d1746
< segsz_t
2149,2150d1749
< sig_t
< sighandler_cxt
2152d1750
< signedbitmapword
2157,2158d1754
< socklen_t
< speed_t
2165,2166d1760
< ssl_crock_st
< stack_t
2169d1762
< strconst
2173,2175d1765
< svindex
< svtype
< swblk_t
2177,2178d1766
< tcflag_t
< tcp_seq
2192,2195d1779
< uInt
< uIntf
< uLong
< uLongf
2198,2208d1781
< u_int16_t
< u_int16m_t
< u_int32_t
< u_int32m_t
< u_int64_t
< u_int64m_t
< u_int8_t
< u_int8m_t
< u_long
< u_quad_t
< u_short
2211d1783
< uint
2213a1786
> uint32_t
2216,2217c1789,1790
< unknown
< ushort
---
> uuid_rc_t
> uuid_t
2221d1793
< varattrib_1b
2223,2228d1794
< varattrib_4b
< vm_offset_t
< vm_size_t
< void
< voidp
< voidpf
2230,2231d1795
< wctype_t
< wint_t
2240d1803
< xl_dbase_create_rec_old
2242d1804
< xl_dbase_drop_rec_old
2264a1827,1838
> xmlBuffer
> xmlBufferPtr
> xmlChar
> xmlDocPtr
> xmlNodePtr
> xmlNodeSetPtr
> xmlParserCtxtPtr
> xmlTextWriter
> xmlTextWriterPtr
> xmlXPathCompExprPtr
> xmlXPathContextPtr
> xmlXPathObjectPtr
2265a1840
> xsltStylesheetPtr
2268c1843,1846
< yysigned_char
---
> yytype_int16
> yytype_int8
> yytype_uint16
> yytype_uint8
Out of curiosity how different is the output if we don't pass the typedef list at all? I'm wondering if the formatting differences are things we actually care much about anyways.
Bruce Momjian wrote:
> I did a diff, attached, and found some typedefs that don't appear, like
> PortalData. That is defined in our code as:
>
> typedef struct PortalData *Portal;
>
> typedef struct PortalData
> {
> /* Bookkeeping data */
> ...
> bool visible; /* include this portal in pg_cursors? */
> } PortalData;
>
> I will try to build on Linux here and see how objdump displays that,
> unless you can get me the output.
>
Well, that is almost certainly a result of the change you asked me to
make :-)
The symbol is in the run done early this morning before those changes.
See
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dungbeetle&dt=2009-06-10%20064401&stg=typedefs>
So we need to look and see what tag that symbol has in the objdump output.
cheers
andrew
Greg Stark <stark@enterprisedb.com> writes:
> Out of curiosity how different is the output if we don't pass the
> typedef list at all? I'm wondering if the formatting differences are
> things we actually care much about anyways.
It tends to put extra spaces in variable declarations that are using
the typedef. Not sure about other effects, but it is kinda ugly when
you are used to it being right.
regards, tom lane
On Tuesday 09 June 2009 20:21:35 Bruce Momjian wrote: > It is time to run pgindent on CVS HEAD for 8.4. I am thinking of > running it at zero-hour GMT tomorrow, meaning five hours from now. > Any objections? Btw., can you make pgindent remove whitespace at the end of lines?
Peter Eisentraut <peter_e@gmx.net> writes:
> Btw., can you make pgindent remove whitespace at the end of lines?
I think it usually does that already ...
regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > Btw., can you make pgindent remove whitespace at the end of lines? > > I think it usually does that already ... Yes. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
> > I did a diff, attached, and found some typedefs that don't appear, like
> > PortalData. That is defined in our code as:
> >
> > typedef struct PortalData *Portal;
> >
> > typedef struct PortalData
> > {
> > /* Bookkeeping data */
> > ...
> > bool visible; /* include this portal in pg_cursors? */
> > } PortalData;
> >
> > I will try to build on Linux here and see how objdump displays that,
> > unless you can get me the output.
> >
>
>
> Well, that is almost certainly a result of the change you asked me to
> make :-)
>
> The symbol is in the run done early this morning before those changes.
> See
> <http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dungbeetle&dt=2009-06-10%20064401&stg=typedefs>
>
> So we need to look and see what tag that symbol has in the objdump output.
OK, I got the answer for Linux. I built 8.4 RC2 and ran:
objdump -W postgres |grep -3 PortalData
and the output is attached. You will notice every mention of PortalData
has 'DW_TAG_structure_type' (stucture member) above it, and none have
DW_TAG_typedef (typedef tag). This is caused by this documented
behavior from find_typedef:
# Linux
# Unfortunately the Linux version doesn't show unreferenced typedefs.
# The problem is that they are still in the source code so should be
# indented properly. However, I think pgindent only cares about
# the typedef references, not the definitions, so I think it might
# be fine
So that is why the Linux list is shorter, but again, I think that is fine.
Andrew, let me know when your list is ready.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
<8db7> DW_AT_byte_size : 4
<8db8> DW_AT_type : <8dbc>
<1><8dbc>: Abbrev Number: 24 (DW_TAG_structure_type)
<8dbd> DW_AT_name : PortalData
<8dc8> DW_AT_byte_size : 96
<8dc9> DW_AT_decl_file : 40
<8dca> DW_AT_decl_line : 105
--
<14706e> DW_AT_byte_size : 4
<14706f> DW_AT_type : <147073>
<1><147073>: Abbrev Number: 24 (DW_TAG_structure_type)
<147074> DW_AT_name : PortalData
<14707f> DW_AT_byte_size : 96
<147080> DW_AT_decl_file : 43
<147081> DW_AT_decl_line : 105
--
<20dd10> DW_AT_byte_size : 4
<20dd11> DW_AT_type : <20dd15>
<1><20dd15>: Abbrev Number: 17 (DW_TAG_structure_type)
<20dd16> DW_AT_name : PortalData
<20dd21> DW_AT_byte_size : 96
<20dd22> DW_AT_decl_file : 46
<20dd23> DW_AT_decl_line : 105
--
<2144bc> DW_AT_byte_size : 4
<2144bd> DW_AT_type : <2144c1>
<1><2144c1>: Abbrev Number: 16 (DW_TAG_structure_type)
<2144c2> DW_AT_name : PortalData
<2144cd> DW_AT_byte_size : 96
<2144ce> DW_AT_decl_file : 46
<2144cf> DW_AT_decl_line : 105
--
<273c96> DW_AT_byte_size : 4
<273c97> DW_AT_type : <273c9b>
<1><273c9b>: Abbrev Number: 19 (DW_TAG_structure_type)
<273c9c> DW_AT_name : PortalData
<273ca7> DW_AT_byte_size : 96
<273ca8> DW_AT_decl_file : 40
<273ca9> DW_AT_decl_line : 105
--
<348c3f> DW_AT_byte_size : 4
<348c40> DW_AT_type : <348c44>
<1><348c44>: Abbrev Number: 20 (DW_TAG_structure_type)
<348c45> DW_AT_name : PortalData
<348c50> DW_AT_byte_size : 96
<348c51> DW_AT_decl_file : 42
<348c52> DW_AT_decl_line : 105
--
<4b0f00> DW_AT_byte_size : 4
<4b0f01> DW_AT_type : <4b0f05>
<1><4b0f05>: Abbrev Number: 7 (DW_TAG_structure_type)
<4b0f06> DW_AT_name : PortalData
<4b0f11> DW_AT_byte_size : 96
<4b0f12> DW_AT_decl_file : 57
<4b0f13> DW_AT_decl_line : 105
--
<4b9974> DW_AT_byte_size : 4
<4b9975> DW_AT_type : <4b9979>
<1><4b9979>: Abbrev Number: 17 (DW_TAG_structure_type)
<4b997a> DW_AT_name : PortalData
<4b9985> DW_AT_byte_size : 96
<4b9986> DW_AT_decl_file : 47
<4b9987> DW_AT_decl_line : 105
--
<4c28e3> DW_AT_byte_size : 4
<4c28e4> DW_AT_type : <4c28e8>
<1><4c28e8>: Abbrev Number: 22 (DW_TAG_structure_type)
<4c28e9> DW_AT_name : PortalData
<4c28f4> DW_AT_byte_size : 96
<4c28f5> DW_AT_decl_file : 42
<4c28f6> DW_AT_decl_line : 105
--
<5d8c98> DW_AT_byte_size : 4
<5d8c99> DW_AT_type : <5d8c9d>
<1><5d8c9d>: Abbrev Number: 26 (DW_TAG_structure_type)
<5d8c9e> DW_AT_name : PortalData
<5d8ca9> DW_AT_byte_size : 96
<5d8caa> DW_AT_decl_file : 39
<5d8cab> DW_AT_decl_line : 105
--
<5e42f0> DW_AT_byte_size : 4
<5e42f1> DW_AT_type : <5e42f5>
<1><5e42f5>: Abbrev Number: 26 (DW_TAG_structure_type)
<5e42f6> DW_AT_name : PortalData
<5e4301> DW_AT_byte_size : 96
<5e4302> DW_AT_decl_file : 40
<5e4303> DW_AT_decl_line : 105
--
<5f681d> DW_AT_byte_size : 4
<5f681e> DW_AT_type : <5f6822>
<1><5f6822>: Abbrev Number: 17 (DW_TAG_structure_type)
<5f6823> DW_AT_name : PortalData
<5f682e> DW_AT_byte_size : 96
<5f682f> DW_AT_decl_file : 45
<5f6830> DW_AT_decl_line : 105
--
<668927> DW_AT_byte_size : 4
<668928> DW_AT_type : <66892c>
<1><66892c>: Abbrev Number: 16 (DW_TAG_structure_type)
<66892d> DW_AT_name : PortalData
<668938> DW_AT_byte_size : 96
<668939> DW_AT_decl_file : 43
<66893a> DW_AT_decl_line : 105
Tom Lane wrote: > Greg Stark <stark@enterprisedb.com> writes: > > Out of curiosity how different is the output if we don't pass the > > typedef list at all? I'm wondering if the formatting differences are > > things we actually care much about anyways. > > It tends to put extra spaces in variable declarations that are using > the typedef. Not sure about other effects, but it is kinda ugly when > you are used to it being right. Yea, I used a short pgindent typedef list once and people noticed right away so I had to rerun. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Wednesday 10 June 2009 22:50:15 Bruce Momjian wrote: > Tom Lane wrote: > > Peter Eisentraut <peter_e@gmx.net> writes: > > > Btw., can you make pgindent remove whitespace at the end of lines? > > > > I think it usually does that already ... > > Yes. Um, attached you will find a bunch of counterexamples.
Peter Eisentraut <peter_e@gmx.net> writes:
>>> I think it usually does that already ...
> Um, attached you will find a bunch of counterexamples.
At a quick look, I'm not sure that any of these are in code that hasn't
been edited since the 8.3 pgindent run.
regards, tom lane
On Wed, Jun 10, 2009 at 9:54 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >>>> I think it usually does that already ... > >> Um, attached you will find a bunch of counterexamples. > > At a quick look, I'm not sure that any of these are in code that hasn't > been edited since the 8.3 pgindent run. On a tangent: git warns about or fixes up white-space problems like that when you're applying a patch. I've actually found this to be a bit of a dilemma though. For code i want it to just go ahead and fix up anything it finds. But for regression test expected output files I don't want it to. I don't think you can control it per-directory though. -- Gregory Stark http://mit.edu/~gsstark/resume.pdf
Andrew Dunstan wrote: > > > I am doing runs as requested on various platforms to extract the > typedef lists. Linux is done, Windows (mingw) is running, FBSD and > Cygwin to come. > > Results in a few hours. The buildfarm will have a consolidated list. > > The consolidated list comes from Windows(mingw) and Linux. My Cygwin run broke for some reason, and 'objdump --stabs' doesn't seem to do what we need on FBSD, so the output there was empty. If someone knows how to get the typedefs out via objdump on FBSD would they please let us know ASAP? Bruce, I think that's the best I can do today. thanks andrew
Andrew Dunstan wrote: > > > Andrew Dunstan wrote: > > > > > > I am doing runs as requested on various platforms to extract the > > typedef lists. Linux is done, Windows (mingw) is running, FBSD and > > Cygwin to come. > > > > Results in a few hours. The buildfarm will have a consolidated list. > > > > > > The consolidated list comes from Windows(mingw) and Linux. My Cygwin > run broke for some reason, and 'objdump --stabs' doesn't seem to do what > we need on FBSD, so the output there was empty. If someone knows how to > get the typedefs out via objdump on FBSD would they please let us know ASAP? I will check on our Postgres shell server right away. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: >> The consolidated list comes from Windows(mingw) and Linux. My Cygwin >> run broke for some reason, and 'objdump --stabs' doesn't seem to do what >> we need on FBSD, so the output there was empty. If someone knows how to >> get the typedefs out via objdump on FBSD would they please let us know ASAP? >> > > I will check on our Postgres shell server right away. > OK, so we got that working, and the consolidated list now contains FBSD data as well. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Bruce Momjian wrote:
>> I will check on our Postgres shell server right away.
> OK, so we got that working, and the consolidated list now contains FBSD
> data as well.
Um, let's *go* guys. RC1 wrap is scheduled for 18 hours from now.
That means it is already too late to be sure that we'll have a full
cycle of buildfarm checks on the pgindent run. Quit fooling around
and get it done.
regards, tom lane
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > >> The consolidated list comes from Windows(mingw) and Linux. My Cygwin > >> run broke for some reason, and 'objdump --stabs' doesn't seem to do what > >> we need on FBSD, so the output there was empty. If someone knows how to > >> get the typedefs out via objdump on FBSD would they please let us know ASAP? > >> > > > > I will check on our Postgres shell server right away. > > > > OK, so we got that working, and the consolidated list now contains FBSD > data as well. And where do I get the consolodated list? From? http://www.pgbuildfarm.org/cgi-bin/typedefs.pl -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Andrew Dunstan wrote: > >> Bruce Momjian wrote: >> >>>> The consolidated list comes from Windows(mingw) and Linux. My Cygwin >>>> run broke for some reason, and 'objdump --stabs' doesn't seem to do what >>>> we need on FBSD, so the output there was empty. If someone knows how to >>>> get the typedefs out via objdump on FBSD would they please let us know ASAP? >>>> >>>> >>> I will check on our Postgres shell server right away. >>> >>> >> OK, so we got that working, and the consolidated list now contains FBSD >> data as well. >> > > And where do I get the consolodated list? From? > > http://www.pgbuildfarm.org/cgi-bin/typedefs.pl > yes. As previously advertised. If you want to see the individual components, use <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list=1> cheers andrew
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > Andrew Dunstan wrote: > > > >> Bruce Momjian wrote: > >> > >>>> The consolidated list comes from Windows(mingw) and Linux. My Cygwin > >>>> run broke for some reason, and 'objdump --stabs' doesn't seem to do what > >>>> we need on FBSD, so the output there was empty. If someone knows how to > >>>> get the typedefs out via objdump on FBSD would they please let us know ASAP? > >>>> > >>>> > >>> I will check on our Postgres shell server right away. > >>> > >>> > >> OK, so we got that working, and the consolidated list now contains FBSD > >> data as well. > >> > > > > And where do I get the consolodated list? From? > > > > http://www.pgbuildfarm.org/cgi-bin/typedefs.pl > > > > yes. As previously advertised. > > If you want to see the individual components, use > <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list=1> OK, pgindent run with updated list and applied to CVS HEAD. I eyeballed the patch and it looked clean, and it tested successfully. Thanks. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> OK, pgindent run with updated list and applied to CVS HEAD. I eyeballed
> the patch and it looked clean, and it tested successfully. Thanks.
Do we have any TODO items concerning pgindent at this point? You had
mentioned wanting to research its behavior for 'struct foo var'
declarations, and I'm not too clear about whether we're happy with the
typedef collection code yet.
regards, tom lane
Tom Lane wrote: > Do we have any TODO items concerning pgindent at this point? Y > Yes, we will make the buildfarm and standalone find-typedefs run from a common pieces of code so they are always in sync. cheers andrew
Andrew Dunstan wrote: > > > Tom Lane wrote: >> Do we have any TODO items concerning pgindent at this point? Y >> > > Yes, we will make the buildfarm and standalone find-typedefs run from a > common pieces of code so they are always in sync. BTW if we had an "official" typedef list that could be used for the length of a whole major release, we could run pgindent on a regular basis (say fortnightly or monthly); patch submitters would just need to run it on their own trees to avoid merge conflicts. (Hmm, but I'm unsure how would that work with git merge etc). -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote: > BTW if we had an "official" typedef list that could be used for the > length of a whole major release, we could run pgindent on a regular > basis (say fortnightly or monthly); patch submitters would just need to > run it on their own trees to avoid merge conflicts. > > (Hmm, but I'm unsure how would that work with git merge etc). > > Well, step one was finding a way to get a more comprehensive typedef list. Now we have some sort of mechanism for doing that, we can look at regularly pulling the list into the source, I guess. Maybe then we could look at either regular runs or even making it a part of a commit to the central repo, if we trust it enough. That would have the advantage of avoiding any later merge problems. After the release, we can play around some more. cheers andrew
Merlin Moncure <mmoncure@gmail.com> writes:
> I confirmed the aix problem on 4.3.3. Installed the patches and
> updated postgres 8.4b2 removing the aix hack. Server starts fine:
> $ LOG: could not bind IPv6 socket: Addr family not supported by protocol
> HINT: Is another postmaster already running on port 5432? If not,
> wait a few seconds and retry.
> LOG: database system was shut down at 2009-06-11 13:23:32 EDT
> LOG: autovacuum launcher started
> LOG: database system is ready to accept connection
> psql logs in. I don't know if the problem we identified this sprint
> is still present...not too concered about that. pull the AIX
> getaddrinfo check and go with it... no problems as best I can tell.
> (out of time for this today).
OK, we'll go with that for RC1. I'll put a nonspecific note into the
docs suggesting a system update if trouble is encountered, but it would
be nice if we could add some information about exactly which fixes are
needed.
regards, tom lane
On Thu, Jun 11, 2009 at 2:13 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > Merlin Moncure <mmoncure@gmail.com> writes: >> I confirmed the aix problem on 4.3.3. Installed the patches and >> updated postgres 8.4b2 removing the aix hack. Server starts fine: > >> $ LOG: could not bind IPv6 socket: Addr family not supported by protocol >> HINT: Is another postmaster already running on port 5432? If not, >> wait a few seconds and retry. >> LOG: database system was shut down at 2009-06-11 13:23:32 EDT >> LOG: autovacuum launcher started >> LOG: database system is ready to accept connection > >> psql logs in. I don't know if the problem we identified this sprint >> is still present...not too concered about that. pull the AIX >> getaddrinfo check and go with it... no problems as best I can tell. >> (out of time for this today). > > OK, we'll go with that for RC1. I'll put a nonspecific note into the > docs suggesting a system update if trouble is encountered, but it would > be nice if we could add some information about exactly which fixes are > needed. I just pulled the aix rollup pack...and would expect others to do the same. It hasn't been updated since 2004-ish. merlin
On Wednesday 10 June 2009 23:54:41 Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > >>> I think it usually does that already ... > > > > Um, attached you will find a bunch of counterexamples. > > At a quick look, I'm not sure that any of these are in code that hasn't > been edited since the 8.3 pgindent run. So what does that mean then? Surely pgindent doesn't keep track of what has been edited when?
Peter Eisentraut wrote: > On Wednesday 10 June 2009 23:54:41 Tom Lane wrote: > >> Peter Eisentraut <peter_e@gmx.net> writes: >> >>>>> I think it usually does that already ... >>>>> >>> Um, attached you will find a bunch of counterexamples. >>> >> At a quick look, I'm not sure that any of these are in code that hasn't >> been edited since the 8.3 pgindent run. >> > > So what does that mean then? Surely pgindent doesn't keep track of what has > been edited when? > > If the code has been edited since the last pgindent run, then pgindent hasn't had a chance to adjust it, no? cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Peter Eisentraut wrote:
>> On Wednesday 10 June 2009 23:54:41 Tom Lane wrote:
>>> At a quick look, I'm not sure that any of these are in code that hasn't
>>> been edited since the 8.3 pgindent run.
>>
>> So what does that mean then? Surely pgindent doesn't keep track of what has
>> been edited when?
> If the code has been edited since the last pgindent run, then pgindent
> hasn't had a chance to adjust it, no?
Right. Those extra spaces all represent manual editing sloppiness.
I have not done a complete check, but I looked at the first couple of
examples you cited and verified that pgindent did remove those spaces.
regards, tom lane