Thread: Incorrect include file order in guc-file.l

Hi all,

While reviewing a different patch, I have noticed that guc-file.l
includes sys/stat.h in the middle of the PG internal headers.  The
usual practice is to have first postgres[_fe].h, followed by the
system headers and finally the internal headers.  That's a nit, but
all the other files do that.

{be,fe}-secure-openssl.c include some exceptions though, as documented
there.

Thoughts?
--
Michael

Hi,

On Wed, Nov 02, 2022 at 02:29:50PM +0900, Michael Paquier wrote:
>
> While reviewing a different patch, I have noticed that guc-file.l
> includes sys/stat.h in the middle of the PG internal headers.  The
> usual practice is to have first postgres[_fe].h, followed by the
> system headers and finally the internal headers.  That's a nit, but
> all the other files do that.
>
> {be,fe}-secure-openssl.c include some exceptions though, as documented
> there.

Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.

On Thu, Nov 03, 2022 at 12:40:19PM +0700, John Naylor wrote:
> On Wed, Nov 2, 2022 at 1:01 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>> Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.
>
> I've pushed this, thanks!

Thanks for the commit.  I've wanted to get it done yesterday but life
took over faster than that.  Before committing the change, there is
something I have noticed though: this header does not seem to be
necessary at all and it looks that there is nothing in guc-file.l that
needs it.  Why did you add it in dac048f to begin with?
--
Michael

On Thu, Nov 03, 2022 at 07:19:07PM +0700, John Naylor wrote:
> Because it wouldn't compile otherwise, obviously. :-)
>
> I must have been working on it before bfb9dfd93720

Hehe, my fault then ;p

The CI is able to complete without it.  Would you mind if it is
removed?  If you don't want us to poke more at the bear, that's a nit
so leaving things as they are is also fine by me.
--
Michael

On Fri, Nov 04, 2022 at 07:55:20AM +0700, John Naylor wrote:
> I've removed it.

Thanks.

Aha, there were three more of these, as of rewriteheap.c, copydir.c
and pgtz.c that I also forgot to clean up in bfb9dfd..
--
Michael