Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)

From: Jeff King <peff@peff.net>
Date: Apr 26, 2014, 07:25 UTC
Message-ID: <20140426072520.GB7558@sigill.intra.peff.net>
In-Reply-To: <3cb4338e-de68-404d-86dc-70cac7e13606@email.android.com>

On Sat, Apr 26, 2014 at 02:56:15PM +1000, nod.helm@gmail.com wrote:

Show 15 quoted lines

> > contrib/subtree/Makefile is a shambles in regards to it's consistency
> > with other makefiles, which makes subtree overly painful to include in
> > build scripts.
> >
> > Two major issues are present:
> >
> > Firstly, calls to git itself (for $(gitdir) and $(gitver)), making
> > building difficult on systems that don't have git.
> >
> > Secondly, the Makefile uses the variable $(libexecdir) for defining the
> > exec path.
> >
> > (...)
> 
> I hate to be that guy, but could I get an opinion on the proposed patch?

It's OK to be that guy; prompting or reposting when a patch has been overlooked is normal here.

> Is git interested in purely makefile patches, or should I find further
> improvements to make in subtree and purpose this again with those?

Makefile improvements are fine on their own. I think the problem is that contrib/subtree does not really have an active dedicated area maintainer.

Your changes look fine to me from a cursory examination. It would probably be more readable as four patches (the 3 "fix" points from your list, plus the "minor fixes" mentioned at the end). Then each patch stands on its own, can say what problem it's fixing, and how.

Show 6 quoted lines

> I've left `rm -f -r subproj mainline` in the clean rule for now,
> however I'd suggest those actually belong in
> contrib/subtree/t/Makefile:clean, given that they are only ever
> generated by `make test`. But given that there aren't any other
> comparable setups in contrib/, I'm somewhat apprehensive to move them
> without opinion.

Do we even make those directories anymore? It looks like they are part of the tests, but the whole test script runs inside its own trash directory. I wonder if they are vestiges from the time when subtree was its own repository outside of contrib/. If so, they can be dropped here (and from .gitignore).

-Peff