Removal of useless include references

Started by Bruce Momjianover 14 years ago12 messageshackers
Jump to latest
#1Bruce Momjian
bruce@momjian.us

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

I am sure applying this patch will break builds on some platforms and
some option combinations so I will monitor the buildfarm when I apply it
and make adjustments.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Attachments:

/rtmp/includestext/x-diffDownload+116-770
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: Removal of useless include references

Bruce Momjian <bruce@momjian.us> writes:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

I am sure applying this patch will break builds on some platforms and
some option combinations so I will monitor the buildfarm when I apply it
and make adjustments.

The last time you did this was in July 2006. It took us two weeks to
mostly recover, but we were still dealing with some fallout in December,
cf
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00491.php

We had the buildfarm then, had had it for a couple years. The notion
that watching the buildfarm is enough is fully disproven by history.

Unless you have a better test plan than last time (which this isn't),
I don't think this should be done at all. The benefits are microscopic
and the pain real.

regards, tom lane

#3Christian Ullrich
chris@chrullrich.net
In reply to: Bruce Momjian (#1)
Re: Removal of useless include references

* Bruce Momjian wrote:

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

There are several unrelated changes to pg_upgrade in that patch, too.

--
Christian

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: Removal of useless include references

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

I am sure applying this patch will break builds on some platforms and
some option combinations so I will monitor the buildfarm when I apply it
and make adjustments.

The last time you did this was in July 2006. It took us two weeks to
mostly recover, but we were still dealing with some fallout in December,
cf
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00491.php

We had the buildfarm then, had had it for a couple years. The notion
that watching the buildfarm is enough is fully disproven by history.

Unless you have a better test plan than last time (which this isn't),
I don't think this should be done at all. The benefits are microscopic
and the pain real.

I don't have a better plan. There are #ifdef code blocks that often
don't get processed and therefore this can't be done better. I will
abandon the idea.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#1)
Re: Removal of useless include references

Excerpts from Bruce Momjian's message of vie ago 26 01:35:45 -0300 2011:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

In btree_gist I think you should remove #include "postgres.h" from the
.h file and put it in the .c files instead, as is customary. I think
that would make the other changes incorrect. ltree.h and pg_upgrade.h
also get this wrong.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#6Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#5)
Re: Removal of useless include references

Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

I am sure applying this patch will break builds on some platforms and
some option combinations so I will monitor the buildfarm when I apply it
and make adjustments.

The last time you did this was in July 2006. It took us two weeks to
mostly recover, but we were still dealing with some fallout in December,
cf
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00491.php

We had the buildfarm then, had had it for a couple years. The notion
that watching the buildfarm is enough is fully disproven by history.

Unless you have a better test plan than last time (which this isn't),
I don't think this should be done at all. The benefits are microscopic
and the pain real.

I don't have a better plan. There are #ifdef code blocks that often
don't get processed and therefore this can't be done better. I will
abandon the idea.

OK, try #2. I already had code that removed #if/#else/#endif code in
*.h files for better testing, so I extended that to all *.c files. This
reduces the size of the diff from 6.6k lines to 4.7k lines but it makes
it much less likely that there will be problems from running
pgrminclude.

The current patch is here:

http://momjian.us/expire/pgrminclude.diff

I tested the patch on BSD and Linux.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#7Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#5)
Re: Removal of useless include references

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of vie ago 26 01:35:45 -0300 2011:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

In btree_gist I think you should remove #include "postgres.h" from the
.h file and put it in the .c files instead, as is customary. I think
that would make the other changes incorrect. ltree.h and pg_upgrade.h
also get this wrong.

Thanks, done.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#8Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#6)
Re: Removal of useless include references

Bruce Momjian wrote:

Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

It has been years since I ran src/tools/pginclude/pgrminclude to remove
unnecessary include files. (I have already fixed things so include
files can be compiled on their own.)

The attached patch removes unneeded include references, and marks some
includes as needing to be skipped by pgrminclude.

I am sure applying this patch will break builds on some platforms and
some option combinations so I will monitor the buildfarm when I apply it
and make adjustments.

The last time you did this was in July 2006. It took us two weeks to
mostly recover, but we were still dealing with some fallout in December,
cf
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00491.php

We had the buildfarm then, had had it for a couple years. The notion
that watching the buildfarm is enough is fully disproven by history.

Unless you have a better test plan than last time (which this isn't),
I don't think this should be done at all. The benefits are microscopic
and the pain real.

I don't have a better plan. There are #ifdef code blocks that often
don't get processed and therefore this can't be done better. I will
abandon the idea.

OK, try #2. I already had code that removed #if/#else/#endif code in
*.h files for better testing, so I extended that to all *.c files. This
reduces the size of the diff from 6.6k lines to 4.7k lines but it makes
it much less likely that there will be problems from running
pgrminclude.

The current patch is here:

http://momjian.us/expire/pgrminclude.diff

I tested the patch on BSD and Linux.

I have re-run the script and applied the result, again tested on BSD and
Linux. I will monitor the buildfarm for possible failures.

This is not something we are going to do regularly, but probably every
five years like this time.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#9Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#8)
Re: Removal of useless include references

Excerpts from Bruce Momjian's message of jue sep 01 11:04:33 -0300 2011:

Bruce Momjian wrote:

OK, try #2. I already had code that removed #if/#else/#endif code in
*.h files for better testing, so I extended that to all *.c files. This
reduces the size of the diff from 6.6k lines to 4.7k lines but it makes
it much less likely that there will be problems from running
pgrminclude.

The current patch is here:

http://momjian.us/expire/pgrminclude.diff

I tested the patch on BSD and Linux.

I have re-run the script and applied the result, again tested on BSD and
Linux. I will monitor the buildfarm for possible failures.

I think anything of this sort should be tested on Windows too.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#9)
Re: Removal of useless include references

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Bruce Momjian's message of jue sep 01 11:04:33 -0300 2011:

I have re-run the script and applied the result, again tested on BSD and
Linux. I will monitor the buildfarm for possible failures.

I think anything of this sort should be tested on Windows too.

Well, if Windows is broken we'll find out soon enough from the
buildfarm. My recollection from the last go-round is that the pain
points were in non-default #define options that Bruce hadn't tested
and that no buildfarm critter exercised either, such as LOCK_DEBUG.

regards, tom lane

#11Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#9)
Re: Removal of useless include references

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of jue sep 01 11:04:33 -0300 2011:

Bruce Momjian wrote:

OK, try #2. I already had code that removed #if/#else/#endif code in
*.h files for better testing, so I extended that to all *.c files. This
reduces the size of the diff from 6.6k lines to 4.7k lines but it makes
it much less likely that there will be problems from running
pgrminclude.

The current patch is here:

http://momjian.us/expire/pgrminclude.diff

I tested the patch on BSD and Linux.

I have re-run the script and applied the result, again tested on BSD and
Linux. I will monitor the buildfarm for possible failures.

I think anything of this sort should be tested on Windows too.

Agreed, but we have so many Windows configurations I figured I would let
the buildfarm test them, no?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#12Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: Removal of useless include references

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Bruce Momjian's message of jue sep 01 11:04:33 -0300 2011:

I have re-run the script and applied the result, again tested on BSD and
Linux. I will monitor the buildfarm for possible failures.

I think anything of this sort should be tested on Windows too.

Well, if Windows is broken we'll find out soon enough from the
buildfarm. My recollection from the last go-round is that the pain
points were in non-default #define options that Bruce hadn't tested
and that no buildfarm critter exercised either, such as LOCK_DEBUG.

Ah, but this time I only removed includes for files I could compile with
all #if markers removed. It only got 5.8k diff lines out a possible
6.8k lines, but this seems like an acceptable cost for greater
reliability.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +