{"thread":{"id":"36486","subject":"[PATCH] subtree/Makefile: Standardize (esp. for packagers)","startedAt":"2014-04-24T01:52:16Z","lastAt":"2014-05-03T22:12:54Z","messageCount":10,"participants":["nod.helm@gmail.com","Jeff King","James Denholm","Matthew Ogilvie","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"239512","messageId":"1398304336-1879-1-git-send-email-nod.helm@gmail.com","threadId":"36486","inReplyTo":null,"subject":"[PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"","fromEmail":"nod.helm@gmail.com","sentAt":"2014-04-24T01:52:16Z","receivedAt":"2014-04-24T01:52:16Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"From: James Denholm \n\ncontrib/subtree/Makefile is a shambles in regards to it's consistency\nwith other makefiles, which makes subtree overly painful to include in\nbuild scripts.\n\nTwo major issues are present:\n\nFirstly, calls to git itself (for $(gitdir) and $(gitver)), making\nbuilding difficult on systems that don't have git.\n\nSecondly, the Makefile uses the variable $(libexecdir) for defining the\nexec path.\n\nTo fix:\n\n1: Scrap unused $(gitdir) assignment\n References were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5,\n but the assignment itself wasn't. Ergo, $(gitdir) hasn't actually\n been using it since then.\n\n2: Use GIT-VERSION-FILE for version info, from $(gitver)\n GVF is already being used in most/all other makefiles in the\n project, and has been for _quite_ a while.\n\n3: :%s/libexecdir/gitexecdir/g\n $(libexecdir) isn't used anywhere else in the project, while\n $(gitexecdir) is the standard.\n\nOn minor fixes, also fiddled with clean rule and a few calls/variables\nto improve congruency with other git makefiles.\n\nSigned-off-by: James Denholm \nBased-on-patch-by: Dan McGee \n---\n\nObligatory \"first patch ever, yay, hello\" exclamation goes here,\nrandom misspelt words and/or life story optional.\n\nI've left `rm -f -r subproj mainline` in the clean rule for now, however\nI'd suggest those actually belong in contrib/subtree/t/Makefile:clean,\ngiven that they are only ever generated by `make test`. But given that\nthere aren't any other comparable setups in contrib/, I'm somewhat\napprehensive to move them without opinion.\n\nAnyway, hopefully this might make some distros more inclined to package\ngit-subtree in their canonical git packages. A very special thanks to\nDan, who proposed the initial patch back in 2012-or-so that this is\nlargely based off of.\n\n contrib/subtree/Makefile | 41 +++++++++++++++++++++++++----------------\n 1 file changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile\nindex 4030a16..e1956b8 100644\n--- a/contrib/subtree/Makefile\n+++ b/contrib/subtree/Makefile\n@@ -3,17 +3,23 @@\n \n prefix ?= /usr/local\n mandir ?= $(prefix)/share/man\n-libexecdir ?= $(prefix)/libexec/git-core\n-gitdir ?= $(shell git --exec-path)\n+gitexecdir ?= $(prefix)/libexec/git-core\n man1dir ?= $(mandir)/man1\n \n-gitver ?= $(word 3,$(shell git --version))\n+../../GIT-VERSION-FILE: FORCE\n+\t$(MAKE) -C ../../ GIT-VERSION-FILE\n \n-# this should be set to a 'standard' bsd-type install program\n-INSTALL ?= install\n+-include ../../GIT-VERSION-FILE\n \n-ASCIIDOC_CONF = ../../Documentation/asciidoc.conf\n-MANPAGE_NORMAL_XSL = ../../Documentation/manpage-normal.xsl\n+# These should be set to 'standard' bsd-type programs\n+INSTALL ?= install\n+RM ?= rm -f\n+\n+ASCIIDOC ?= asciidoc\n+XMLTO ?= xmlto\n+\n+ASCIIDOC_CONF = ../../Documentation/asciidoc.conf\n+MANPAGE_XSL = ../../Documentation/manpage-normal.xsl\n \n GIT_SUBTREE_SH := git-subtree.sh\n GIT_SUBTREE := git-subtree\n@@ -31,8 +37,8 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)\n doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)\n \n install: $(GIT_SUBTREE)\n-\t$(INSTALL) -d -m 755 $(DESTDIR)$(libexecdir)\n-\t$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)\n+\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n+\t$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)\n \n install-doc: install-man\n \n@@ -41,19 +47,22 @@ install-man: $(GIT_SUBTREE_DOC)\n \t$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)\n \n $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)\n-\txmlto -m $(MANPAGE_NORMAL_XSL) man $^\n+\t$(XMLTO) -m $(MANPAGE_XSL) man $^\n \n $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT)\n-\tasciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \\\n-\t\t-agit_version=$(gitver) $^\n+\t$(ASCIIDOC) -b docbook -d manpage -f $(ASCIIDOC_CONF) \\\n+\t\t-agit_version=$(GIT_VERSION) $^\n \n $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT)\n-\tasciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \\\n-\t\t-agit_version=$(gitver) $^\n+\t$(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \\\n+\t\t-agit_version=$(GIT_VERSION) $^\n \n test:\n \t$(MAKE) -C t/ test\n \n clean:\n-\trm -f *~ *.xml *.html *.1\n-\trm -rf subproj mainline\n+\t$(RM) $(GIT_SUBTREE)\n+\t$(RM) *.xml *.html *.1\n+\t$(RM) -r subproj mainline\n+\n+.PHONY: FORCE\n-- \n1.9.2\n"},{"id":"239741","messageId":"3cb4338e-de68-404d-86dc-70cac7e13606@email.android.com","threadId":"36486","inReplyTo":"CAHYYfeGNDLVxzP6zMyJnSi8GxpQaUKGAkqaLfXbZ=8B1k7vvyQ@mail.gmail.com","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"","fromEmail":"nod.helm@gmail.com","sentAt":"2014-04-26T04:56:15Z","receivedAt":"2014-04-26T04:56:15Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"\n\nOn 24 Apr 2014 11:52, wrote:\n>\n> From: James Denholm \n>\n> contrib/subtree/Makefile is a shambles in regards to it's consistency\n> with other makefiles, which makes subtree overly painful to include in\n> build scripts.\n>\n> Two major issues are present:\n>\n> Firstly, calls to git itself (for $(gitdir) and $(gitver)), making\n> building difficult on systems that don't have git.\n>\n> Secondly, the Makefile uses the variable $(libexecdir) for defining the\n> exec path.\n>\n> (...)\n\nI hate to be that guy, but could I get an opinion on the proposed patch? Is\ngit\ninterested in purely makefile patches,\nor should I find further improvements\nto make in subtree and purpose this again with those?\n\nRegards,\nJames Denholm.\n"},{"id":"239745","messageId":"20140426072520.GB7558@sigill.intra.peff.net","threadId":"36486","inReplyTo":"3cb4338e-de68-404d-86dc-70cac7e13606@email.android.com","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-26T07:25:20Z","receivedAt":"2014-04-26T07:25:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 26, 2014 at 02:56:15PM +1000, nod.helm@gmail.com wrote:\n\n> > contrib/subtree/Makefile is a shambles in regards to it's consistency\n> > with other makefiles, which makes subtree overly painful to include in\n> > build scripts.\n> >\n> > Two major issues are present:\n> >\n> > Firstly, calls to git itself (for $(gitdir) and $(gitver)), making\n> > building difficult on systems that don't have git.\n> >\n> > Secondly, the Makefile uses the variable $(libexecdir) for defining the\n> > exec path.\n> >\n> > (...)\n> \n> I hate to be that guy, but could I get an opinion on the proposed patch?\n\nIt's OK to be that guy; prompting or reposting when a patch has been\noverlooked is normal here.\n\n> Is git interested in purely makefile patches, or should I find further\n> improvements to make in subtree and purpose this again with those?\n\nMakefile improvements are fine on their own. I think the problem is that\ncontrib/subtree does not really have an active dedicated area\nmaintainer.\n\nYour changes look fine to me from a cursory examination. It would\nprobably be more readable as four patches (the 3 \"fix\" points from your\nlist, plus the \"minor fixes\" mentioned at the end). Then each patch\nstands on its own, can say what problem it's fixing, and how.\n\n> I've left `rm -f -r subproj mainline` in the clean rule for now,\n> however I'd suggest those actually belong in\n> contrib/subtree/t/Makefile:clean, given that they are only ever\n> generated by `make test`. But given that there aren't any other\n> comparable setups in contrib/, I'm somewhat apprehensive to move them\n> without opinion.\n\nDo we even make those directories anymore? It looks like they are part\nof the tests, but the whole test script runs inside its own trash\ndirectory. I wonder if they are vestiges from the time when subtree was\nits own repository outside of contrib/. If so, they can be dropped here\n(and from .gitignore).\n\n-Peff\n"},{"id":"239787","messageId":"6a7bcc79-d9c3-4cf8-8f3b-a6a16298c221@email.android.com","threadId":"36486","inReplyTo":"20140426072520.GB7558@sigill.intra.peff.net","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-04-27T02:35:13Z","receivedAt":"2014-04-27T02:35:13Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"Jeff King wrote:\n>I think the problem is that\n>contrib/subtree does not really have an active dedicated area\n>maintainer.\n\nYeah, I can see how that might become a bit of a problem. I was\nactually thinking of doing a bit of work on subtree beyond this\nspecific patch, so hopefully that won't be a show-stopper. We'll\nsee what happens, I guess.\n\n>Your changes look fine to me from a cursory examination. It would\n>probably be more readable as four patches (the 3 \"fix\" points from your\n>list, plus the \"minor fixes\" mentioned at the end). Then each patch\n>stands on its own, can say what problem it's fixing, and how.\n>\n> (...)\n>\n>Do we even make [subproject and mainline] anymore? It looks like they are part\n>of the tests, but the whole test script runs inside its own trash\n>directory.\n\nsubproject and mainline are actually made in contrib/subtree,\nbut I'll look at perhaps \"fixing\" that when I split the proposal\ninto a series as you suggest.\n\nThanks for the advice!\n\nRegards,\nJames Denholm.\n"},{"id":"239788","messageId":"20140427025150.GA26382@sigill.intra.peff.net","threadId":"36486","inReplyTo":"6a7bcc79-d9c3-4cf8-8f3b-a6a16298c221@email.android.com","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-27T02:51:50Z","receivedAt":"2014-04-27T02:51:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:\n\n> >Do we even make [subproject and mainline] anymore? It looks like they are part\n> >of the tests, but the whole test script runs inside its own trash\n> >directory.\n> \n> subproject and mainline are actually made in contrib/subtree,\n> but I'll look at perhaps \"fixing\" that when I split the proposal\n> into a series as you suggest.\n\nAre they? I couldn't find any reference to them as directories except in\nthe test script, and doing a \"make\" from contrib/subtree didn't create\nthem. I'll leave it to you to investigate further whether the \"clean\"\nrules are cruft or not, but certainly if they are, cleaning up cruft is\na good thing.\n\n-Peff\n"},{"id":"239789","messageId":"b883baa0-740f-4d2a-b7b1-4cc9294f88ed@email.android.com","threadId":"36486","inReplyTo":"20140427025150.GA26382@sigill.intra.peff.net","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-04-27T03:01:42Z","receivedAt":"2014-04-27T03:01:42Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"Jeff King wrote:\n>On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:\n>\n>> >Do we even make [subproject and mainline] anymore? It looks like\n>they are part\n>> >of the tests, but the whole test script runs inside its own trash\n>> >directory.\n>> \n>> subproject and mainline are actually made in contrib/subtree,\n>> but I'll look at perhaps \"fixing\" that when I split the proposal\n>> into a series as you suggest.\n>\n>Are they? I couldn't find any reference to them as directories except\n>in\n>the test script, and doing a \"make\" from contrib/subtree didn't create\n>them.\n\nYeah, I could be wrong as I don't have the code on-hand at the moment,\nbut from memory they're made and populated by the test script as\nwell. Either way I'll move all that into a subdir of t/ and rejigger the thing, if\ninvestigation reveals that as the Done Thing.\n\n>I'll leave it to you to investigate further whether the \"clean\"\n>rules are cruft or not, but certainly if they are, cleaning up cruft is\n>a good thing.\n"},{"id":"240264","messageId":"20140430032045.GA4613@comcast.net","threadId":"36486","inReplyTo":"6a7bcc79-d9c3-4cf8-8f3b-a6a16298c221@email.android.com","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2014-04-30T03:20:45Z","receivedAt":"2014-04-30T03:20:45Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:\n> Jeff King wrote:\n> >I think the problem is that\n> >contrib/subtree does not really have an active dedicated area\n> >maintainer.\n> \n> Yeah, I can see how that might become a bit of a problem. I was\n> actually thinking of doing a bit of work on subtree beyond this\n> specific patch, so hopefully that won't be a show-stopper. We'll\n> see what happens, I guess.\n\nAgreed. It also doesn't help that when subtree patches are proposed\n(especially new features instead of obvious bugs), there often seems\nto be little or no feedback from anyone.\n\n--------\nDepending on how much time you have:\n\nThis may be outside the scope of work you were planning on, but\nit might be worth grepping through old mailing list archives for\n\"subtree\" patches that haven't been merged, and see if there is\nanything worth revisiting/resubmitting. I believe most of the\nfollowing (at least) kind of languished and died, often with little\nor no real review and feedback:\n\nhttp://marc.info/?l=git&m=138644067726844&w=2\nhttp://marc.info/?l=git&m=138523794407181&w=2\n - My own series, plus another patch that has roughly the\n same description, but different semantics.\n\nhttp://marc.info/?l=git&m=136321400525507&w=2\nhttp://marc.info/?l=git&m=136321400525507&w=2\n - Some series from Paul Campbell.\n\nhttp://marc.info/?l=git&m=136122107605036&w=2\nhttp://marc.info/?l=git&m=135813589922554&w=2\nhttp://marc.info/?l=git&m=136415434127550&w=2\nhttp://marc.info/?l=git&m=136127692217856&w=2\n - Other series.\n\nhttp://marc.info/?l=git&m=138557714926045&w=2\nhttp://marc.info/?l=git&m=138129106613560&w=2\nhttp://marc.info/?l=git&m=136415882128742&w=2\nhttp://marc.info/?l=git&m=136415654228062&w=2\n - Miscellaneous\n\nAnd probably others...\n\n(I don't know if these are the latest or \"best\" versions of these, nor\nhave I really looked at them closely to decide if they are worth\nincluding at all. Be sure to exameine not just the discussion around\nthe specific patches, but also the other patches in each series...)\n\n - Matthew Ogilvie\n"},{"id":"240633","messageId":"CAHYYfeGNO5QknoKkZfYy3XLNRZsVmf0WjeNGkDxH3QwPF-RsUQ@mail.gmail.com","threadId":"36486","inReplyTo":"20140430032045.GA4613@comcast.net","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-05-03T12:56:25Z","receivedAt":"2014-05-03T12:56:25Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"Matthew Ogilvie wrote:\n> On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:\n>> Jeff King wrote:\n> Agreed. It also doesn't help that when subtree patches are proposed\n> (especially new features instead of obvious bugs), there often seems\n> to be little or no feedback from anyone.\n>\n> --------\n> Depending on how much time you have:\n>\n> This may be outside the scope of work you were planning on,\n\nWhile current, immediate focus is really just getting the makefile fixed\nup and hopefully then have more people package subtree by default,\noverall I'll very likely extend that to general work on subtree and such.\n\n> but\n> it might be worth grepping through old mailing list archives for\n> \"subtree\" patches that haven't been merged, and see if there is\n> anything worth revisiting/resubmitting. I believe most of the\n> following (at least) kind of languished and died, often with little\n> or no real review and feedback:\n>\n> (...)\n>\n> (I don't know if these are the latest or \"best\" versions of these, nor\n> have I really looked at them closely to decide if they are worth\n> including at all. Be sure to exameine not just the discussion around\n> the specific patches, but also the other patches in each series...)\n\nYeah, certainly, I'll be sure to have a sticky-beak. Thanks for pointing\nthose out!\n\nRegards,\nJames Denholm.\n"},{"id":"240635","messageId":"5365420874947_27397d32f016@nysa.notmuch","threadId":"36486","inReplyTo":"CAHYYfeGNO5QknoKkZfYy3XLNRZsVmf0WjeNGkDxH3QwPF-RsUQ@mail.gmail.com","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-03T19:22:48Z","receivedAt":"2014-05-03T19:22:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"James Denholm wrote:\n> Matthew Ogilvie wrote:\n> > On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:\n> >> Jeff King wrote:\n> > Agreed. It also doesn't help that when subtree patches are proposed\n> > (especially new features instead of obvious bugs), there often seems\n> > to be little or no feedback from anyone.\n> >\n> > --------\n> > Depending on how much time you have:\n> >\n> > This may be outside the scope of work you were planning on,\n> \n> While current, immediate focus is really just getting the makefile fixed\n> up and hopefully then have more people package subtree by default,\n> overall I'll very likely extend that to general work on subtree and such.\n\nI think you should take a look at the Makefile of\ncontrib/remote-helpers. I bet something simple like that would work just\nfine for subtree.\n\n-- \nFelipe Contreras\n"},{"id":"240649","messageId":"6e64be78-58d0-42c5-97de-aaf9310f014d@email.android.com","threadId":"36486","inReplyTo":"5365420874947_27397d32f016@nysa.notmuch","subject":"Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-05-03T22:12:54Z","receivedAt":"2014-05-03T22:12:54Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"On 4 May 2014 05:22:48 GMT+10:00, Felipe Contreras wrote:\n>I think you should take a look at the Makefile of\n>contrib/remote-helpers. I bet something simple like that would work\n>just\n>fine for subtree.\n\nThe current makefile is simple enough, just quirky and likes\nto be a special snowflake. 'sall good, the v2 addresses most\nof my immediate concerns with it.\n\nRegards,\nJames Denholm.\n"}]}