All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Denton Liu <liu.denton@gmail.com>
Subject: Re: [PATCH v2] Makefile: make the "sparse" target non-.PHONY
Date: Fri, 24 Sep 2021 03:30:10 +0200	[thread overview]
Message-ID: <875yuq4u4u.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqtuib199x.fsf@gitster.g>


On Thu, Sep 23 2021, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
>
>> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>>> We ensure that the recursive dependencies are correct by depending on
>>> the *.o file, which in turn will have correct dependencies by either
>>> depending on all header files, or under
>>> "COMPUTE_HEADER_DEPENDENCIES=yes" the headers it needs.
>>> 
>>> This means that a plain "make sparse" is much slower, as we'll now
>>> need to make the *.o files just to create the *.sp files, but
>>> incrementally creating the *.sp files is *much* faster and less
>>> verbose, it thus becomes viable to run "sparse" along with "all" as
>>> e.g. "git rebase --exec 'make all sparse'".
>>
>> OK. I think this solves the dependency issues sufficiently. It is a
>> tradeoff that you must do the normal build in order to do the sparse
>> check now. That is certainly fine for my workflow (I am building Git all
>> the time, and only occasionally run "make sparse"). I don't know if
>> others would like it less (e.g., if Ramsay is frequently running sparse
>> checks without having just built).
>>
>> (I'd say "I do not care that much either way", but then I do not care
>> all that much either way about incremental sparse checks either, so I'm
>> not sure my opinion really matters).
>
> My build procedure runs "make sparse" before the primary build,
> simply because the former tends to be much faster to fail when there
> is an issue in the code.  I can understand that depending on .o is a
> cheap way to piggyback on the dependencies it has, but my latency
> will get much slower if this goes in _and_ I keep trying to pick up
> potentially problematic patches from the list.

Would you be OK with just running something like this instead?:

    make -j $(nproc) objects CC=cgcc CFLAGS="-no-compile -Wsparse-error -Wno-pointer-arith -Wno-memcpy-max-count"

It gives you almost exactly the same thing as the old "sparse" target. I
think the way it worked is really not something we needed a special
target for in the first place, or perhaps just a .PHONY alias.

The "almost" is because those -Wno-* are now per-file via
SP_EXTRA_FLAGS.

I.e. if we have a $(CC) that's willing to accept CC-like options but
just won't create output files we can use "objects" (or other targets I
added in 029bac01a87 (Makefile: add {program,xdiff,test,git,fuzz}-objs &
objects targets, 2021-02-23)). Even if the tool itself created broken
*.o files a change of $(CC) to the "real" compiler would ensure that
they'd get regenerated.

  parent reply	other threads:[~2021-09-24  1:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 22:55 [PATCH 0/3] Makefile: make "sparse" and "hdr-check" non-.PHONY Ævar Arnfjörð Bjarmason
2021-09-21 22:55 ` [PATCH 1/3] Makefile: make the "sparse" target non-.PHONY Ævar Arnfjörð Bjarmason
2021-09-22  2:24   ` Jeff King
2021-09-21 22:55 ` [PATCH 2/3] Makefile: do one append in %.hcc rule Ævar Arnfjörð Bjarmason
2021-09-21 22:55 ` [PATCH 3/3] Makefile: make the "hdr-check" target non-.PHONY Ævar Arnfjörð Bjarmason
2021-09-22  2:11 ` [PATCH 0/3] Makefile: make "sparse" and "hdr-check" non-.PHONY Jeff King
2021-09-22 16:58   ` Ramsay Jones
2021-09-22 17:53     ` Jeff King
2021-09-22 19:17       ` Ramsay Jones
2021-09-22 23:28         ` Junio C Hamano
2021-09-23  1:07           ` Ævar Arnfjörð Bjarmason
2021-09-23  1:23             ` Junio C Hamano
2021-09-23  2:17               ` Ævar Arnfjörð Bjarmason
2021-09-22 19:24     ` Junio C Hamano
2021-09-23  0:07 ` [PATCH v2] Makefile: make the "sparse" target non-.PHONY Ævar Arnfjörð Bjarmason
2021-09-23 16:24   ` Jeff King
2021-09-23 17:06     ` Ævar Arnfjörð Bjarmason
2021-09-23 17:17       ` Jeff King
2021-09-23 17:39     ` Junio C Hamano
2021-09-23 23:28       ` Ramsay Jones
2021-09-24  1:16         ` Ævar Arnfjörð Bjarmason
2021-09-24 16:38           ` Ramsay Jones
2021-09-24  1:30       ` Ævar Arnfjörð Bjarmason [this message]
2021-09-24 19:37         ` Junio C Hamano
2021-09-28  1:15   ` [PATCH v3] Makefile: add a non-.PHONY "sparse-incr" target Ævar Arnfjörð Bjarmason
2021-09-28  1:43     ` [PATCH v4] " Ævar Arnfjörð Bjarmason
2021-09-28 17:44       ` Junio C Hamano
2021-09-28 19:45         ` Ævar Arnfjörð Bjarmason

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=875yuq4u4u.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.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.