git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/1] Avoid calling find in the Makefile, if possible
Date: Mon, 04 Mar 2019 05:47:05 -0800 (PST)	[thread overview]
Message-ID: <pull.130.v2.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.130.git.gitgitgadget@gmail.com>

I noticed this quite a bit of time ago, but did not get a chance to look
into it in detail: all of a sudden, make started really slowly over here.

The culprit turned out to be a find call, which was in the Makefile for
ages, so I was puzzled why it only caused problems recently.

After some digging, I found out that the hdr-check target is the culprit:
before it was introduced, $(LIB_H) did not need to be expanded, and as a
consequence the find call was not executed. Once hdr-check made it into 
master, though, $(LIB_H) must be expanded to define that rule.

Since I have tons of worktrees as subdirectories of my principal Git clone,
and since I also have a 3rdparty/ directory with tons of repositories I use
for various testing/contributing purposes, this find is quite a little slow
over here.

So here is my suggested fix. It is based on similar code we already had in
the Makefile, obviously also intended to avoid an expensive find invocation.

Changes since v1:

 * Since LIB_H's lazy evaluation kicks in all the time anyway, changed the = 
   to := to avoid evaluating it three times.
 * Clarified in the commit message that the existing sites using $(LIB_H) 
   are not affected by this change (or at least not affected as long as no
   untracked header files are included in .c files).

Johannes Schindelin (1):
  Makefile: use `git ls-files` to list header files, if possible

 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


base-commit: c65a2884eae159bad540135479bc8afe20ff62d1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-130%2Fdscho%2Favoid-find-in-makefile-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-130/dscho/avoid-find-in-makefile-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/130

Range-diff vs v1:

 1:  0b5529406b ! 1:  cb253bd0cf Makefile: use `git ls-files` to list header files, if possible
     @@ -29,7 +29,11 @@
          than from a clone of Git's sources).
      
          This has one notable consequence: we no longer include `command-list.h`
     -    in `LIB_H`, as it is a generated file, not a tracked one.
     +    in `LIB_H`, as it is a generated file, not a tracked one, but that is
     +    easily worked around. Of the three sites that use `LIB_H`, two
     +    (`LOCALIZED_C` and `CHK_HDRS`) already handle generated headers
     +    separately. In the third, the computed-dependency fallback, we can just
     +    add in a reference to $(GENERATED_H).
      
          Likewise, we no longer include not-yet-tracked header files in `LIB_H`.
      
     @@ -46,7 +50,7 @@
       GENERATED_H += command-list.h
       
      -LIB_H = $(shell $(FIND) . \
     -+LIB_H = $(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
     ++LIB_H := $(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
      +	$(FIND) . \
       	-name .git -prune -o \
       	-name t -prune -o \

-- 
gitgitgadget

  parent reply	other threads:[~2019-03-04 13:47 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-01 19:57 [PATCH 0/1] Avoid calling find in the Makefile, if possible Johannes Schindelin via GitGitGadget
2019-03-01 19:57 ` [PATCH 1/1] Makefile: use `git ls-files` to list header files, " Johannes Schindelin via GitGitGadget
2019-03-01 21:36   ` Jeff King
2019-03-01 21:54     ` Jeff King
2019-03-01 22:01       ` Jeff King
2019-03-02 19:54         ` Johannes Schindelin
2019-03-03 17:08           ` Jeff King
2019-03-02 19:57       ` Johannes Schindelin
2019-03-03 17:11         ` Jeff King
2019-03-02 20:05     ` Johannes Schindelin
2019-03-03 17:19       ` Jeff King
2019-03-03 21:30         ` Ramsay Jones
2019-03-04 12:38           ` Johannes Schindelin
2019-03-04 20:31             ` Ramsay Jones
2019-03-04 21:37             ` Jeff King
2019-03-04 11:08         ` Johannes Schindelin
2019-03-04 21:41           ` Jeff King
2019-03-05  5:50             ` Junio C Hamano
2019-03-05 15:28               ` Johannes Schindelin
2019-03-05 22:26                 ` Junio C Hamano
2019-03-05 23:07               ` Jeff King
2019-03-06  0:23                 ` Ramsay Jones
2019-03-06  4:40                   ` Jeff King
2019-03-06  5:28                     ` Junio C Hamano
2019-03-06 19:05                       ` [PATCH] compat/bswap: add include header guards Jeff King
2019-03-06 22:42                         ` Junio C Hamano
2019-03-04 13:47 ` Johannes Schindelin via GitGitGadget [this message]
2019-03-04 13:47   ` [PATCH v2 1/1] Makefile: use `git ls-files` to list header files, if possible Johannes Schindelin via GitGitGadget
2019-03-04 20:38     ` Ramsay Jones
2019-03-04 21:01       ` Ramsay Jones
2019-03-04 21:43     ` 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=pull.130.v2.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).