git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: nod.helm@gmail.com
To: git@vger.kernel.org
Cc: James Denholm <nod.helm@gmail.com>,
	greened@obbligato.org, apenwarr@gmail.com, dpmcgee@gmail.com
Subject: [PATCH] subtree/Makefile: Standardize (esp. for packagers)
Date: Thu, 24 Apr 2014 11:52:16 +1000	[thread overview]
Message-ID: <1398304336-1879-1-git-send-email-nod.helm@gmail.com> (raw)

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(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 4030a16..e1956b8 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -3,17 +3,23 @@
 
 prefix ?= /usr/local
 mandir ?= $(prefix)/share/man
-libexecdir ?= $(prefix)/libexec/git-core
-gitdir ?= $(shell git --exec-path)
+gitexecdir ?= $(prefix)/libexec/git-core
 man1dir ?= $(mandir)/man1
 
-gitver ?= $(word 3,$(shell git --version))
+../../GIT-VERSION-FILE: FORCE
+	$(MAKE) -C ../../ GIT-VERSION-FILE
 
-# this should be set to a 'standard' bsd-type install program
-INSTALL ?= install
+-include ../../GIT-VERSION-FILE
 
-ASCIIDOC_CONF      = ../../Documentation/asciidoc.conf
-MANPAGE_NORMAL_XSL =  ../../Documentation/manpage-normal.xsl
+# These should be set to 'standard' bsd-type programs
+INSTALL  ?= install
+RM       ?= rm -f
+
+ASCIIDOC ?= asciidoc
+XMLTO    ?= xmlto
+
+ASCIIDOC_CONF = ../../Documentation/asciidoc.conf
+MANPAGE_XSL   = ../../Documentation/manpage-normal.xsl
 
 GIT_SUBTREE_SH := git-subtree.sh
 GIT_SUBTREE    := git-subtree
@@ -31,8 +37,8 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)
 
 install: $(GIT_SUBTREE)
-	$(INSTALL) -d -m 755 $(DESTDIR)$(libexecdir)
-	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
+	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
+	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)
 
 install-doc: install-man
 
@@ -41,19 +47,22 @@ install-man: $(GIT_SUBTREE_DOC)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-	xmlto -m $(MANPAGE_NORMAL_XSL)  man $^
+	$(XMLTO) -m $(MANPAGE_XSL) man $^
 
 $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT)
-	asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \
-		-agit_version=$(gitver) $^
+	$(ASCIIDOC) -b docbook -d manpage -f $(ASCIIDOC_CONF) \
+		-agit_version=$(GIT_VERSION) $^
 
 $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT)
-	asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \
-		-agit_version=$(gitver) $^
+	$(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \
+		-agit_version=$(GIT_VERSION) $^
 
 test:
 	$(MAKE) -C t/ test
 
 clean:
-	rm -f *~ *.xml *.html *.1
-	rm -rf subproj mainline
+	$(RM) $(GIT_SUBTREE)
+	$(RM) *.xml *.html *.1
+	$(RM) -r subproj mainline
+
+.PHONY: FORCE
-- 
1.9.2

             reply	other threads:[~2014-04-24  1:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-24  1:52 nod.helm [this message]
     [not found] ` <CAHYYfeGNDLVxzP6zMyJnSi8GxpQaUKGAkqaLfXbZ=8B1k7vvyQ@mail.gmail.com>
2014-04-26  4:56   ` [PATCH] subtree/Makefile: Standardize (esp. for packagers) nod.helm
2014-04-26  7:25     ` Jeff King
2014-04-27  2:35       ` James Denholm
2014-04-27  2:51         ` Jeff King
2014-04-27  3:01           ` James Denholm
2014-04-30  3:20         ` Matthew Ogilvie
2014-05-03 12:56           ` James Denholm
2014-05-03 19:22             ` Felipe Contreras
2014-05-03 22:12               ` James Denholm

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

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

  git send-email \
    --in-reply-to=1398304336-1879-1-git-send-email-nod.helm@gmail.com \
    --to=nod.helm@gmail.com \
    --cc=apenwarr@gmail.com \
    --cc=dpmcgee@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=greened@obbligato.org \
    /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.
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 NNTP newsgroup(s).