From: Felipe Contreras <felipe.contreras@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Emily Shaffer" <emilyshaffer@google.com>,
"Jeff Hostetler" <jeffhost@microsoft.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: RE: [PATCH 3/3] Makefile: remove an out-of-date comment
Date: Thu, 17 Jun 2021 15:55:24 -0500 [thread overview]
Message-ID: <60cbb6bc37daf_9bf20865@natae.notmuch> (raw)
In-Reply-To: <patch-3.3-ddae86802e-20210617T095827Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason wrote:
> This comment added in dfea575017 (Makefile: lazily compute header
> dependencies, 2010-01-26) has been out of date since
> 92b88eba9f (Makefile: use `git ls-files` to list header files, if
> possible, 2019-03-04), when we did exactly what it tells us not to do
> and added $(GENERATED_H) to $(OBJECTS) dependencies.
Very true.
> The rest of it was also somewhere between inaccurate and outdated,
> since as of b8ba629264 (Makefile: fold MISC_H into LIB_H, 2012-06-20)
> it's not followed by a list of header files, that got moved earlier in
> the file into LIB_H in b8ba629264 (Makefile: fold MISC_H into LIB_H,
> 2012-06-20).
I don't see the point in mentioning b8ba629264 twice, perhaps you meant:
60d24dd255 (Makefile: fold XDIFF_H and VCSSVN_H into LIB_H, 2012-07-06)
This commit also removed part of the comment:
# XXX. Please check occasionally that these include all dependencies
# gcc detects!
I have tried to understand what was the purpose of that comment in the
past, but I don't get it, it says:
Dependencies on automatically generated headers such as command-list.h
should _not_ be included here, since they are necessary even when
building an object for the first time.
Why would it matter if we are building the object for the first time, or
rebuilding it? If we need command-list.h to build help.o once, we need
it always. What am I missing?
It seems to me this comment never made sense.
> --- a/Makefile
> +++ b/Makefile
> @@ -2503,12 +2503,6 @@ ifneq ($(dep_files_present),)
> include $(dep_files_present)
> endif
> else
> -# Dependencies on header files, for platforms that do not support
> -# the gcc -MMD option.
> -#
> -# Dependencies on automatically generated headers such as command-list.h
> -# should _not_ be included here, since they are necessary even when
> -# building an object for the first time.
>
Can we remove that extra space once we are at it?
> $(OBJECTS): $(LIB_H) $(GENERATED_H)
> endif
The change itself looks good to me.
Cheers.
--
Felipe Contreras
next prev parent reply other threads:[~2021-06-17 20:55 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-17 10:01 [PATCH 0/3] Makefile: misc trivial fixes Ævar Arnfjörð Bjarmason
2021-06-17 10:01 ` [PATCH 1/3] Makefile: mark "check" target as .PHONY Ævar Arnfjörð Bjarmason
2021-06-17 20:04 ` Felipe Contreras
2021-06-17 10:01 ` [PATCH 2/3] Makefile: stop hardcoding {command,config}-list.h Ævar Arnfjörð Bjarmason
2021-06-17 20:14 ` Felipe Contreras
2021-06-17 20:58 ` Ævar Arnfjörð Bjarmason
2021-06-17 21:58 ` Felipe Contreras
2021-06-18 8:05 ` Ævar Arnfjörð Bjarmason
2021-06-17 10:01 ` [PATCH 3/3] Makefile: remove an out-of-date comment Ævar Arnfjörð Bjarmason
2021-06-17 20:55 ` Felipe Contreras [this message]
2021-06-29 19:03 ` [PATCH v2 0/3] Makefile: misc trivial fixes Ævar Arnfjörð Bjarmason
2021-06-29 19:03 ` [PATCH v2 1/3] Makefile: mark "check" target as .PHONY Ævar Arnfjörð Bjarmason
2021-06-29 19:03 ` [PATCH v2 2/3] Makefile: stop hardcoding {command,config}-list.h Ævar Arnfjörð Bjarmason
2021-06-29 19:03 ` [PATCH v2 3/3] Makefile: remove an out-of-date comment Ævar Arnfjörð Bjarmason
2021-06-29 19:25 ` [PATCH v2 0/3] Makefile: misc trivial fixes Felipe Contreras
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=60cbb6bc37daf_9bf20865@natae.notmuch \
--to=felipe.contreras@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jeffhost@microsoft.com \
/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.