patchsubtree/Makefile: Standardize (esp. for packagers)

From: James Denholm <nod.helm@gmail.com>

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.

To fix:

1: Scrap unused $(gitdir) assignment
    References were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5,
    but the assignment itself wasn't. Ergo, $(gitdir) hasn't actually
    been using it since then.

2: Use GIT-VERSION-FILE for version info, from $(gitver)
    GVF is already being used in most/all other makefiles in the
    project, and has been for _quite_ a while.

3: :%s/libexecdir/gitexecdir/g
    $(libexecdir) isn't used anywhere else in the project, while
    $(gitexecdir) is the standard.

On minor fixes, also fiddled with clean rule and a few calls/variables to improve congruency with other git makefiles.

Signed-off-by: James Denholm <nod.helm@gmail.com>
Based-on-patch-by: Dan McGee <dpmcgee@gmail.com>
---

Obligatory "first patch ever, yay, hello" exclamation goes here, random misspelt words and/or life story optional.

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.

Anyway, hopefully this might make some distros more inclined to package git-subtree in their canonical git packages. A very special thanks to Dan, who proposed the initial patch back in 2012-or-so that this is largely based off of.

 contrib/subtree/Makefile | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

On 24 Apr 2014 11:52, <nod.helm@gmail.com> wrote:

I hate to be that guy, but could I get an opinion on the proposed patch? Is git interested in purely makefile patches, or should I find further improvements to make in subtree and purpose this again with those?

Regards, James Denholm.

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

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

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.

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

Jeff King <peff@peff.net> wrote:

Yeah, I can see how that might become a bit of a problem. I was actually thinking of doing a bit of work on subtree beyond this specific patch, so hopefully that won't be a show-stopper. We'll see what happens, I guess.

subproject and mainline are actually made in contrib/subtree, but I'll look at perhaps "fixing" that when I split the proposal into a series as you suggest.

Thanks for the advice!

Regards, James Denholm.

On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:

Are they? I couldn't find any reference to them as directories except in the test script, and doing a "make" from contrib/subtree didn't create them. I'll leave it to you to investigate further whether the "clean" rules are cruft or not, but certainly if they are, cleaning up cruft is a good thing.

-Peff

Jeff King <peff@peff.net> wrote:

Yeah, I could be wrong as I don't have the code on-hand at the moment, but from memory they're made and populated by the test script as well. Either way I'll move all that into a subdir of t/ and rejigger the thing, if investigation reveals that as the Done Thing.

On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:

Agreed. It also doesn't help that when subtree patches are proposed (especially new features instead of obvious bugs), there often seems to be little or no feedback from anyone.

-------- Depending on how much time you have:

This may be outside the scope of work you were planning on, but it might be worth grepping through old mailing list archives for "subtree" patches that haven't been merged, and see if there is anything worth revisiting/resubmitting. I believe most of the following (at least) kind of languished and died, often with little or no real review and feedback:

http://marc.info/?l=git&m=138644067726844&w=2
http://marc.info/?l=git&m=138523794407181&w=2
  - My own series, plus another patch that has roughly the
    same description, but different semantics.

http://marc.info/?l=git&m=136321400525507&w=2
http://marc.info/?l=git&m=136321400525507&w=2
  - Some series from Paul Campbell.

http://marc.info/?l=git&m=136122107605036&w=2
http://marc.info/?l=git&m=135813589922554&w=2
http://marc.info/?l=git&m=136415434127550&w=2
http://marc.info/?l=git&m=136127692217856&w=2
  - Other series.

http://marc.info/?l=git&m=138557714926045&w=2
http://marc.info/?l=git&m=138129106613560&w=2
http://marc.info/?l=git&m=136415882128742&w=2
http://marc.info/?l=git&m=136415654228062&w=2
  - Miscellaneous

And probably others...

(I don't know if these are the latest or "best" versions of these, nor have I really looked at them closely to decide if they are worth including at all. Be sure to exameine not just the discussion around the specific patches, but also the other patches in each series...)

                     - Matthew Ogilvie

Matthew Ogilvie <mmogilvi_git@miniinfo.net> wrote:

While current, immediate focus is really just getting the makefile fixed up and hopefully then have more people package subtree by default, overall I'll very likely extend that to general work on subtree and such.

Yeah, certainly, I'll be sure to have a sticky-beak. Thanks for pointing those out!

Regards, James Denholm.

James Denholm wrote:

I think you should take a look at the Makefile of contrib/remote-helpers. I bet something simple like that would work just fine for subtree.

On 4 May 2014 05:22:48 GMT+10:00, Felipe Contreras <felipe.contreras@gmail.com> wrote:

The current makefile is simple enough, just quirky and likes to be a special snowflake. 'sall good, the v2 addresses most of my immediate concerns with it.

Regards, James Denholm.