From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org,
"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
"Jeff King" <peff@peff.net>,
"Jonathan Nieder" <jrnieder@gmail.com>
Subject: Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic
Date: Wed, 22 Sep 2021 09:08:00 -0700 [thread overview]
Message-ID: <xmqqmto48ufz.fsf@gitster.g> (raw)
In-Reply-To: <patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Wed, 22 Sep 2021 12:38:07 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
> use auto-detection in [2]. Then when -Wpedantic support was added to
> DEVOPTS in [3] we started passing -Wpedantic in combination with
> -Werror to the compiler here.
>
> This broke the auto-detection, but since we'd quieted it in [4] we
> didn't find out.
Are the references correct? I am not seeing "quiet"ing in [4]. The
redirection 2>&1 to cram error messages also to $(dep_check), hence
making it impossible to match '0', was done in [2].
We did make the pedantic mode the default and pass both -pedantic
and -Wpedantic after [4]. Before we had only -pedantic.
> It was emitting all of this on STDERR under GCC:
>
> /dev/null:1: error: ISO C forbids an empty translation unit
> [-Werror=pedantic]
> cc1: note: unrecognized command-line option
> ‘-Wno-pedantic-ms-format’ may have been intended to silence
> earlier diagnostics
> cc1: all warnings being treated as errors
>
> Let's fix that bug by maintaining a NON_DEVELOPER_CFLAGS, it's like
> ALL_CFLAGS but without anything we add in config.mak.dev, and
> furthermore stop redirecting STDERR to /dev/null, this means that
> someone whose compiler doesn't support this will see this output, but
> also this new message:
>
> Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
Hmmmmmph.
I recentaly saw many .depend directories (not necessarily empty)
left after "make distclean". After building on one branch, I often
check out a different branch then run distclean on the new branch,
so leftover build artifacts are not necessarily a bug in our
Makefile, but the bug you found may explain it?
While I agree with your analysis of the problem, I cannot shake this
nagging feeling that the proposed solution is barking up a wrong
tree. After all, -pedantic and any other option that lets the
compiler notice that it is being asked to compile an empty source
can come directly from the end user (e.g. CC="gcc -pedantic" or as
part of CFLAGS)---realization of which makes me wonder if it is
essential to compile /dev/null for this check, or any reasonably
syntactically correct program would do.
I wonder if the attached (with clean-up to remove the tracing cruft)
would show us a better direction. It feeds a single line
int dummy_for_dep_check;
C "program" from the standard input of the compiler to tackle the
"you are not supposed to be compiling an empty compilation unit"
problem in a more direct way.
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git c/Makefile w/Makefile
index 9df565f27b..0593ab7287 100644
--- c/Makefile
+++ w/Makefile
@@ -1277,9 +1277,9 @@ COMPUTE_HEADER_DEPENDENCIES = auto
endif
ifeq ($(COMPUTE_HEADER_DEPENDENCIES),auto)
-dep_check = $(shell $(CC) $(ALL_CFLAGS) \
+dep_check = $(shell echo >&2 doing dep check; echo int dummy_for_dep_check\; | $(CC) $(ALL_CFLAGS) \
-c -MF /dev/null -MQ /dev/null -MMD -MP \
- -x c /dev/null -o /dev/null 2>&1; \
+ -x c - -o /dev/null || echo >&2 oops; \
echo $$?)
ifeq ($(dep_check),0)
override COMPUTE_HEADER_DEPENDENCIES = yes
next prev parent reply other threads:[~2021-09-22 16:08 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-22 10:38 [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic Ævar Arnfjörð Bjarmason
2021-09-22 10:55 ` Carlo Arenas
2021-09-22 11:04 ` Ævar Arnfjörð Bjarmason
2021-09-22 16:08 ` Junio C Hamano [this message]
2021-09-22 17:04 ` Jeff King
2021-09-22 18:28 ` Junio C Hamano
2021-09-22 18:44 ` Carlo Arenas
2021-09-22 20:17 ` Jeff King
2021-09-22 20:36 ` Carlo Arenas
2021-09-22 22:40 ` Ævar Arnfjörð Bjarmason
2021-09-23 17:32 ` Jeff King
2021-09-23 0:03 ` Junio C Hamano
2021-09-23 16:20 ` Jeff King
2021-09-23 17:41 ` Junio C Hamano
2021-09-22 19:45 ` Ævar Arnfjörð Bjarmason
2021-09-22 22:08 ` [PATCH v2 0/2] Makefile: "pedantic" fallout on .depend and "compdb" Ævar Arnfjörð Bjarmason
2021-09-22 22:08 ` [PATCH v2 1/2] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic Ævar Arnfjörð Bjarmason
2021-09-22 22:08 ` [PATCH v2 2/2] Makefile: pass -Wno-pendantic under GENERATE_COMPILATION_DATABASE=yes Ævar Arnfjörð Bjarmason
2021-09-23 0:05 ` Carlo Arenas
2021-09-23 21:33 ` Junio C Hamano
2021-09-23 17:38 ` [PATCH v2 0/2] Makefile: "pedantic" fallout on .depend and "compdb" Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmto48ufz.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.