Re: [HACKERS] pgindent run coming
On Wednesday 10 June 2009 23:54:41 Tom Lane wrote: Peter Eisentraut pete...@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? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Peter Eisentraut wrote: On Wednesday 10 June 2009 23:54:41 Tom Lane wrote: Peter Eisentraut pete...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Andrew Dunstan and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Merlin Moncure mmonc...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
On Thu, Jun 11, 2009 at 2:13 PM, Tom Lanet...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://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;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Simon Riggs si...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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.origWed 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;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://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;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 specified more 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't be formatted as nicely as it should. This sounds like a painful thing to have to do, but it's really a symptom of a problem in C: typedef causes a syntactic change in the language and 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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Andrew Dunstan wrote:
Tom Lane wrote:
Bruce Momjian br...@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 */
...
boolvisible;/* 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 br...@momjian.ushttp://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
Re: [HACKERS] pgindent run coming
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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 */
...
boolvisible;/* 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=dungbeetledt=2009-06-10%20064401stg=typedefs
So we need to look and see what tag that symbol has in the objdump output.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Greg Stark st...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Peter Eisentraut pete...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 */
...
boolvisible;/* 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=dungbeetledt=2009-06-10%20064401stg=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 br...@momjian.ushttp://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
18dbc: 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
1147073: 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
120dd15: 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
12144c1: 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
1273c9b: 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
1348c44: 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
14b0f05: 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
14b9979: 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
14c28e8: 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
15d8c9d: 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
15e42f5: Abbrev Number: 26
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Greg Stark st...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Peter Eisentraut pete...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
On Wednesday 10 June 2009 22:50:15 Bruce Momjian wrote:
Tom Lane wrote:
Peter Eisentraut pete...@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.
Index: src/bin/initdb/initdb.c
===
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.171
diff -u -3 -p -r1.171 initdb.c
--- src/bin/initdb/initdb.c 3 Jun 2009 16:17:49 - 1.171
+++ src/bin/initdb/initdb.c 10 Jun 2009 20:42:32 -
@@ -1356,9 +1356,9 @@ bootstrap_template1(char *short_version)
bki_lines = replace_token(bki_lines, ENCODING, encodingid);
bki_lines = replace_token(bki_lines, LC_COLLATE, lc_collate);
-
+
bki_lines = replace_token(bki_lines, LC_CTYPE, lc_ctype);
-
+
/*
* Pass correct LC_xxx environment to bootstrap.
*
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.110
diff -u -3 -p -r1.110 pg_ctl.c
--- src/bin/pg_ctl/pg_ctl.c 5 Apr 2009 04:19:58 - 1.110
+++ src/bin/pg_ctl/pg_ctl.c 10 Jun 2009 20:42:32 -
@@ -1486,9 +1486,9 @@ CreateRestrictedProcess(char *cmd, PROCE
}
#ifndef __CYGWIN__
-AddUserToDacl(processInfo-hProcess);
+ AddUserToDacl(processInfo-hProcess);
#endif
-
+
CloseHandle(restrictedToken);
ResumeThread(processInfo-hThread);
Index: src/bin/pg_dump/dumputils.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.45
diff -u -3 -p -r1.45 dumputils.c
--- src/bin/pg_dump/dumputils.c 11 Mar 2009 03:33:29 - 1.45
+++ src/bin/pg_dump/dumputils.c 10 Jun 2009 20:42:33 -
@@ -59,7 +59,7 @@ init_parallel_dump_utils(void)
const char *
fmtId(const char *rawid)
{
- /*
+ /*
* The Tls code goes awry if we use a static var, so we provide for both
* static and auto, and omit any use of the static var when using Tls.
*/
@@ -87,7 +87,7 @@ fmtId(const char *rawid)
{
/* new buffer */
id_return = createPQExpBuffer();
-#ifdef WIN32
+#ifdef WIN32
if (parallel_init_done)
TlsSetValue(tls_index,id_return);
else
@@ -95,7 +95,7 @@ fmtId(const char *rawid)
#else
s_id_return = id_return;
#endif
-
+
}
/*
Index: src/bin/pg_dump/keywords.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/keywords.c,v
retrieving revision 1.2
diff -u -3 -p -r1.2 keywords.c
--- src/bin/pg_dump/keywords.c 8 Mar 2009 16:53:30 - 1.2
+++ src/bin/pg_dump/keywords.c 10 Jun 2009 20:42:33 -
@@ -18,7 +18,7 @@
#include parser/keywords.h
/*
- * We don't need the token number, so leave it out to avoid requiring other
+ * We don't need the token number, so leave it out to avoid requiring other
* backend headers.
*/
#define PG_KEYWORD(a,b,c) {a,0,c},
Index: src/bin/pg_dump/pg_backup_archiver.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.171
diff -u -3 -p -r1.171 pg_backup_archiver.c
--- src/bin/pg_dump/pg_backup_archiver.c 4 Jun 2009 19:16:48 - 1.171
+++ src/bin/pg_dump/pg_backup_archiver.c 10 Jun 2009 20:42:33 -
@@ -3487,12 +3487,12 @@ parallel_restore(RestoreArgs *args)
/*
* Close and reopen the input file so we have a private file pointer
- * that doesn't stomp on anyone else's file pointer, if we're actually
+ * that doesn't stomp on anyone else's file pointer, if we're actually
* going to need to read from the file. Otherwise, just close it
* except on Windows, where it will possibly be needed by other threads.
*
* Note: on Windows, since we are using threads not processes, the
- * reopen call *doesn't* close the original file pointer but just open
+ * reopen call *doesn't* close the original file pointer but just open
* a new one.
*/
if (te-section == SECTION_DATA )
@@ -3870,7 +3870,7 @@ CloneArchive(ArchiveHandle *AH)
ArchiveHandle *clone;
/* Make a flat copy */
- clone = (ArchiveHandle *) malloc(sizeof(ArchiveHandle));
+ clone = (ArchiveHandle *) malloc(sizeof(ArchiveHandle));
if (clone == NULL)
die_horribly(AH, modulename, out of memory\n);
memcpy(clone, AH, sizeof(ArchiveHandle));
Index: src/bin/pg_dump/pg_backup_custom.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_custom.c,v
retrieving revision 1.41
diff -u -3 -p -r1.41 pg_backup_custom.c
--- src/bin/pg_dump/pg_backup_custom.c 2 Feb 2009 20:07:37 - 1.41
+++ src/bin/pg_dump/pg_backup_custom.c 10 Jun 2009 20:42:33 -
@@ -133,8 +133,8 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
AH-StartBlobPtr = _StartBlob;
AH-EndBlobPtr = _EndBlob;
AH-EndBlobsPtr = _EndBlobs;
- AH-ClonePtr =
Re: [HACKERS] pgindent run coming
Peter Eisentraut pete...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
On Wed, Jun 10, 2009 at 9:54 PM, Tom Lanet...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Andrew Dunstan and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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.old 2123 typedefs.new and the diffs generates are a similar number of lines: 133657 diff.old_typedefs 135042 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 3e64 97648 /usr/include/time.h 31358 EXCL 0 0 7638 97624 /usr/include/sys/time.h 31359 EXCL 0 0 97648 /usr/include/time.h 31360 EXCL 0 0 1dff 25540 /usr/include/fcntl.h 31361 BINCL 0 0 000144be 449667 /usr/include/sys/stat.h 31362 LSYM 0 0 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_u id:(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:(4 8,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 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_s ize:(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),5 76,32;st_lspare:(0,3),608,32;st_qspare:(51,3)=ar(0,1);0;3;(0,3),640,128;; -- 31364 LSYM 0 0 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_u id:(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_siz e:(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)=a r(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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian br...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
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 21`;
@err = () if `uname -s 21` =~ /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 @sy...@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;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgindent run coming
Bruce Momjian [EMAIL PROTECTED] writes: It is about time to run pgindent before we enter beta testing. Is this weekend good for everyone? I think we should wait until the fate of the GUC patch is determined --- if we want to apply it, a pgindent run is going to cause some unnecessary work, plus we'd need to re-indent afterwards. I've never been a fan of waiting till the last minute to run pgindent, but I don't think we have to do it before first beta. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pgindent run coming
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: It is about time to run pgindent before we enter beta testing. Is this weekend good for everyone? I think we should wait until the fate of the GUC patch is determined --- if we want to apply it, a pgindent run is going to cause some unnecessary work, plus we'd need to re-indent afterwards. I've never been a fan of waiting till the last minute to run pgindent, but I don't think we have to do it before first beta. OK. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
