Re: [PATCH 1/3] Makefile: use $(ALL_CFLAGS) in $(HCO) target

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 26, 2019, 19:47 UTC
Message-ID: <nycvar.QRO.7.76.6.1909262144060.15067@tvgsbejvaqbjf.bet>
In-Reply-To: <20190926173801.GA42456@generichostname>

Hi,

On Thu, 26 Sep 2019, Denton Liu wrote:

Show 34 quoted lines

> On Thu, Sep 26, 2019 at 02:49:55PM +0200, Johannes Schindelin wrote:
> >
> > On Mon, 23 Sep 2019, Denton Liu wrote:
> >
> > > On platforms that can run `make hdr-check` but require custom flags,
> > > this target was failing because none of them were being passed to the
> > > compiler. For example, on MacOS, the NO_OPENSSL flag was being set but
> > > it was not being passed into compiler so the check was failing.
> > >
> > > Pass $(ALL_CFLAGS) into the compiler for the $(HCO) target so that these
> > > custom flags can be used.
> > >
> > > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > > ---
> > >  Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index f879697ea3..d8df4e316b 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -2773,7 +2773,7 @@ CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
> > >  HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
> > >
> > >  $(HCO): %.hco: %.h FORCE
> > > -	$(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
> > > +	$(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $(ALL_CFLAGS) $<
> >
> > Isn't this essentially identical to what I sent in
> > https://public-inbox.org/git/pull.129.git.gitgitgadget@gmail.com/
> > ?
>
> Whoops, didn't notice this. I heard there's some quote about great minds
> or something ;).

Hahaha! ;-)

Show 5 quoted lines

> Anyway, this patch is superceded by v2 4/4 which should emulate the
> compilation process even better, as suggested by Peff. The only
> problem with that patch is that it splashes temporary *.hcc files
> everywhere but I think that if it's only a developer-run target, it
> should be fine.

I agree that Peff's improvement is nice.

The only problem is that my patch made it into `next` already, according to https://github.com/gitgitgadget/git/pull/129 (look for the labels on the right side, or for the comments on the bottom), and https://github.com/gitster/git/commit/a3f332f4fb10 agrees (look for names of the branches containing that commit, under the commit message).

So we should explicitly as for my patch to be backed out from `next` and be stopped from advancing to `master`.

Junio, would you kindly do that? According to the PR, you gave this branch the name `js/honor-cflags-in-hdr-check`.

Thanks, Dscho