git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Jeff King <peff@peff.net>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/1] Makefile: use `git ls-files` to list header files, if possible
Date: Wed, 06 Mar 2019 07:26:00 +0900	[thread overview]
Message-ID: <xmqqlg1tt0uf.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1903051627050.41@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Tue, 5 Mar 2019 16:28:45 +0100 (STD)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> After a normal build, with dynamic dependency checking on, we would
>> have sufficient information to figure it out.  Would that help?
>
> Yes, *if* the dynamic dependency checking is in effect (read: if we are
> compiling with GCC).
>
> However, I think that one of the benefits of the current approach is that
> hdr-check finds issues also in headers that are *not* part of the current
> build. Read: once hdr-check is run as part of the CI build, it is harder
> for some random contributor to break hdr-check for somebody else with
> different compile time options.

Sure.  That is exactly why we have multiple configurations at the
CI, hopefully more than each individual contributor has at hand so
that CI can give more coverage than any single individual can test.

What I was assuming was that at least one of these multiple
configurations would cover the case where sha256/gcrypt.h truly is
used, because that particular build is configured to use the system
header and library.  And it would be a far better approach to make
that build check also for the header, if we can cover all the
problematic (in the same sense as the sha256/gcrypt.h) header files,
than unconditionally "checking" without knowing the prerequisite and
causing a false positive.

Having said that, the optional use of ls-files introduced by the
patch in question is a valid incremental improvement over the
current Makefile, so it is a pretty much independent/orthogonal
issue.

  reply	other threads:[~2019-03-05 22:26 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 [this message]
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 ` [PATCH v2 0/1] Avoid calling find in the Makefile, if possible Johannes Schindelin via GitGitGadget
2019-03-04 13:47   ` [PATCH v2 1/1] Makefile: use `git ls-files` to list header files, " 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlg1tt0uf.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).