Обсуждение: pgsql: Allow more include files to be compiled in their own by adding m
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(-)
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
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
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. +
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. +
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
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. +
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. +