Thread: Add missing includes
Hi, Some files were missing information from the c.h header. -- Tristan Partin Neon (https://neon.tech)
Attachment
Hi, On 2023-May-22, Tristan Partin wrote: > Some files were missing information from the c.h header. Actually, these omissions are intentional, and we have bespoke handling for this in our own header-checking scripts (src/tools/pginclude). I imagine this is documented somewhere, but ATM I can't remember where. (And if not, maybe that's something we should do.) -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
On Mon May 22, 2023 at 10:16 AM CDT, Alvaro Herrera wrote: > > Some files were missing information from the c.h header. > > Actually, these omissions are intentional, and we have bespoke handling > for this in our own header-checking scripts (src/tools/pginclude). I > imagine this is documented somewhere, but ATM I can't remember where. > (And if not, maybe that's something we should do.) Interesting. Thanks for the information. Thought I had seen in a different email thread that header files were supposed to include all that they needed to. Anyways, ignore the patch :). -- Tristan Partin Neon (https://neon.tech)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2023-May-22, Tristan Partin wrote:
>> Some files were missing information from the c.h header.
> Actually, these omissions are intentional, and we have bespoke handling
> for this in our own header-checking scripts (src/tools/pginclude). I
> imagine this is documented somewhere, but ATM I can't remember where.
> (And if not, maybe that's something we should do.)
Yeah, the general policy is that .h files should not explicitly include
c.h (nor postgres.h nor postgres_fe.h). Instead, .c files should include
the appropriate one of those three files first. This allows sharing of
.h files more easily across frontend/backend/common environments.
I'm not sure where this is documented either.
regards, tom lane
On Mon May 22, 2023 at 10:28 AM CDT, Tom Lane wrote: > >> Some files were missing information from the c.h header. > > > Actually, these omissions are intentional, and we have bespoke handling > > for this in our own header-checking scripts (src/tools/pginclude). I > > imagine this is documented somewhere, but ATM I can't remember where. > > (And if not, maybe that's something we should do.) > > Yeah, the general policy is that .h files should not explicitly include > c.h (nor postgres.h nor postgres_fe.h). Instead, .c files should include > the appropriate one of those three files first. This allows sharing of > .h files more easily across frontend/backend/common environments. > > I'm not sure where this is documented either. Thanks for the added context. I'll try to keep this in mind going forward. -- Tristan Partin Neon (https://neon.tech)
"Tristan Partin" <tristan@neon.tech> writes:
> Interesting. Thanks for the information. Thought I had seen in a
> different email thread that header files were supposed to include all
> that they needed to. Anyways, ignore the patch :).
There is such a policy, but not with respect to those particular
headers. You might find src/tools/pginclude/headerscheck and
src/tools/pginclude/cpluspluscheck to be interesting reading,
as those are the tests we run to verify this policy.
regards, tom lane
On Mon, May 22, 2023 at 11:34:14AM -0400, Tom Lane wrote: > There is such a policy, but not with respect to those particular > headers. You might find src/tools/pginclude/headerscheck and > src/tools/pginclude/cpluspluscheck to be interesting reading, > as those are the tests we run to verify this policy. The standalone compile policy is documented in src/tools/pginclude/README, AFAIK, so close enough :p -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> The standalone compile policy is documented in
> src/tools/pginclude/README, AFAIK, so close enough :p
Hmm, that needs an update --- the scripts are slightly smarter
now than that text gives them credit for. I'll see to it after
the release freeze lifts.
regards, tom lane