pgsql: Make pg_bsd_indent's .h files inclusion-order-safe.

Hi,

On February 12, 2023 10:06:55 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Make pg_bsd_indent's .h files inclusion-order-safe.

As-is, they failed headerscheck. Per buildfarm.

Cfbot didn't catch this, because it only triggered a warning, not a failure. We probably should add -Werror?

Greetings,

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Andres Freund <andres@anarazel.de> writes:

Cfbot didn't catch this, because it only triggered a warning, not a failure. We probably should add -Werror?

Yeah -- nobody is going to go digging through cfbot logs for warnings,
especially when they know that the Compiler Warnings task is supposed
to complain about those.

I don't think -Werror helps any though; it's a matter of whether the
cfbot is paying attention to headerscheck's exit status ...

regards, tom lane

Hi,

On February 12, 2023 1:11:08 PM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

Cfbot didn't catch this, because it only triggered a warning, not a failure. We probably should add -Werror?

Yeah -- nobody is going to go digging through cfbot logs for warnings,
especially when they know that the Compiler Warnings task is supposed
to complain about those.

I don't think -Werror helps any though; it's a matter of whether the
cfbot is paying attention to headerscheck's exit status ...

Thomas fixed that part a while ago, but it'll just return 0 if it just is a warning...

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Andres Freund <andres@anarazel.de> writes:

On February 12, 2023 1:11:08 PM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think -Werror helps any though; it's a matter of whether the
cfbot is paying attention to headerscheck's exit status ...

Thomas fixed that part a while ago, but it'll just return 0 if it just is a warning...

Hmm, but why'd the buildfarm complain then? ... oh, because it checks
for nonempty output as well as nonzero exit status.

regards, tom lane

Hi,

On 2023-02-12 16:25:28 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On February 12, 2023 1:11:08 PM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think -Werror helps any though; it's a matter of whether the
cfbot is paying attention to headerscheck's exit status ...

Thomas fixed that part a while ago, but it'll just return 0 if it just is a warning...

Hmm, but why'd the buildfarm complain then? ... oh, because it checks
for nonempty output as well as nonzero exit status.

Right.

Obviously that approach could be re-implemented for CI, but givent that it
additionally is somewhat required to make use of headerscheck/cpluspluscheck
locally, it feels better to solve it in headerscheck. And the easiest way for
that seems to be -Werror?

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

Obviously that approach could be re-implemented for CI, but givent that it
additionally is somewhat required to make use of headerscheck/cpluspluscheck
locally, it feels better to solve it in headerscheck. And the easiest way for
that seems to be -Werror?

Agreed --- although right now, headerscheck goes out of its way to be
agnostic about what compiler and cflags are used. Having it assume
that -Werror will work seems a bit ugly. Not sure that it's a problem
in practice though.

(BTW, we'll have to fix these scripts sometime to work in non-Make
builds.)

regards, tom lane