pgsql: Allow more include files to be compiled in their own by adding m

Started by Bruce Momjianalmost 15 years ago8 messagescomitters
Jump to latest
#1Bruce Momjian
bruce@momjian.us

Allow more include files to be compiled in their own by adding missing
include dependencies.

Modify pgcompinclude to skip a common fcinfo error.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/4bd7333b14786a2d757195e907709d2aee116809

Modified Files
--------------
contrib/cube/cubedata.h | 2 ++
src/backend/replication/repl_gram.y | 1 +
src/include/access/gin_private.h | 1 +
src/include/access/hash.h | 1 +
src/include/access/htup.h | 1 +
src/include/access/nbtree.h | 2 +-
src/include/access/xlog.h | 2 +-
src/tools/pginclude/pgcompinclude | 3 +++
8 files changed, 11 insertions(+), 2 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Bruce Momjian <bruce@momjian.us> writes:

Allow more include files to be compiled in their own by adding missing
include dependencies.

This patch broke the build on buildfarm member mongoose.

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: pgsql: Allow more include files to be compiled in their own by adding m

On 08/28/2011 04:55 PM, Tom Lane wrote:

Bruce Momjian<bruce@momjian.us> writes:

Allow more include files to be compiled in their own by adding missing
include dependencies.

This patch broke the build on buildfarm member mongoose.

I'm not sure I understand the point of it anyway.

cheers

andrew

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Allow more include files to be compiled in their own by adding missing
include dependencies.

This patch broke the build on buildfarm member mongoose.

This is the change I made to contrib/cube. I see other contrib modules
including fmgr.h too:

	diff --git a/contrib/cube/cubedata.h b/contrib/cube/cubedata.h
	new file mode 100644
	index fd0c26a..9760162
	*** a/contrib/cube/cubedata.h
	--- b/contrib/cube/cubedata.h
	***************
	*** 1,5 ****
	--- 1,7 ----
	  /* contrib/cube/cubedata.h */
	+ #include "fmgr.h"
	+
	  #define CUBE_MAX_DIM (100)

typedef struct NDBOX

I can remove it and we can see if it fixes things. Should I?

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

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

#5Bruce Momjian
bruce@momjian.us
In reply to: Andrew Dunstan (#3)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Andrew Dunstan wrote:

On 08/28/2011 04:55 PM, Tom Lane wrote:

Bruce Momjian<bruce@momjian.us> writes:

Allow more include files to be compiled in their own by adding missing
include dependencies.

This patch broke the build on buildfarm member mongoose.

I'm not sure I understand the point of it anyway.

The idea is that include files should include all needed includes so
they don't spill dependencies into files that use them.

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

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Bruce Momjian <bruce@momjian.us> writes:

Andrew Dunstan wrote:

I'm not sure I understand the point of it anyway.

The idea is that include files should include all needed includes so
they don't spill dependencies into files that use them.

I think that's an unwarranted expansion of the charter of what you're
doing. There are places where we intentionally do not pull in include
files that will be required if (and only if) one attempts to make use of
macros provided by a given header file. The first one that comes to
mind is in postgres_ext.h:

#define OID_MAX UINT_MAX
/* you will need to include <limits.h> to use the above #define */

but I believe there are others, and I don't want some mechanical process
second-guessing those decisions. I think the rule of "should compile on
its own" is okay, but I don't want that expanded to "every macro
provided by the file must be expandable with no further includes".

In the end, the point of what you are doing is to ensure that header
files can be included in any order. It is not to ensure some sort of
unclearly-defined closure rule about how many headers need be included
to make use of a particular facility.

regards, tom lane

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Andrew Dunstan wrote:

I'm not sure I understand the point of it anyway.

The idea is that include files should include all needed includes so
they don't spill dependencies into files that use them.

I think that's an unwarranted expansion of the charter of what you're
doing. There are places where we intentionally do not pull in include
files that will be required if (and only if) one attempts to make use of
macros provided by a given header file. The first one that comes to
mind is in postgres_ext.h:

#define OID_MAX UINT_MAX
/* you will need to include <limits.h> to use the above #define */

but I believe there are others, and I don't want some mechanical process
second-guessing those decisions. I think the rule of "should compile on
its own" is okay, but I don't want that expanded to "every macro
provided by the file must be expandable with no further includes".

Agreed.

In the end, the point of what you are doing is to ensure that header
files can be included in any order. It is not to ensure some sort of
unclearly-defined closure rule about how many headers need be included
to make use of a particular facility.

OK.

--
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 (#4)
Re: pgsql: Allow more include files to be compiled in their own by adding m

Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Allow more include files to be compiled in their own by adding missing
include dependencies.

This patch broke the build on buildfarm member mongoose.

This is the change I made to contrib/cube. I see other contrib modules
including fmgr.h too:

diff --git a/contrib/cube/cubedata.h b/contrib/cube/cubedata.h
new file mode 100644
index fd0c26a..9760162
*** a/contrib/cube/cubedata.h
--- b/contrib/cube/cubedata.h
***************
*** 1,5 ****
--- 1,7 ----
/* contrib/cube/cubedata.h */
+ #include "fmgr.h"
+
#define CUBE_MAX_DIM (100)

typedef struct NDBOX

I can remove it and we can see if it fixes things. Should I?

I am reverting this include change.

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

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