Обсуждение: pgsql: Make pg_bsd_indent's .h files inclusion-order-safe.
Make pg_bsd_indent's .h files inclusion-order-safe. As-is, they failed headerscheck. Per buildfarm. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/6ded4a5a3de1e8d5f3ec50efd4a2d7a39d2550a6 Modified Files -------------- src/tools/pg_bsd_indent/indent.h | 2 ++ 1 file changed, 2 insertions(+)
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