* [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass @ 2014-05-06 12:41 James Denholm 2014-05-06 12:41 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm 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. The main issues are that calls are made to git itself in the build process, and that a subtree-exclusive variable is used for specifying the exec path. Patches 1/5 through 3/5 resolve these. The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other makefiles across the project. One problem is foreseen: 3/5 will necessitate that package maintainers who already have git-subtree included in their packages update their build-scripts. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> Based-on-patch-by: Dan McGee <dpmcgee@gmail.com> James Denholm (5): contrib/subtree/Makefile: scrap unused $(gitdir) contrib/subtree/Makefile: Use GIT-VERSION-FILE contrib/subtree/Makefile: s/libexecdir/gitexecdir contrib/subtree/Makefile: Doc-gen rules cleanup contrib/subtree/Makefile: clean rule cleanup contrib/subtree/Makefile | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) -- 1.9.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm @ 2014-05-06 12:41 ` James Denholm 2014-05-06 12:41 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm ` (4 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm In 7ff8463dba0d74fc07a766bed457ae7afcc902b5, the references to gitdir were removed but the assignment itself wasn't. Hence, drop the gitdir assignment. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 4030a16..87797ed 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -4,7 +4,6 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core -gitdir ?= $(shell git --exec-path) man1dir ?= $(mandir)/man1 gitver ?= $(word 3,$(shell git --version)) -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm 2014-05-06 12:41 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm @ 2014-05-06 12:41 ` James Denholm 2014-05-06 12:41 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm ` (3 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm GVF is already being used in most/all other makefiles in the project, and has been for _quite_ a while. Hence, drop file-unique gitver and replace with GIT_VERSION. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 87797ed..f63334b 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -6,7 +6,10 @@ mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 -gitver ?= $(word 3,$(shell git --version)) +../../GIT-VERSION-FILE: FORCE + $(MAKE) -C ../../ GIT-VERSION-FILE + +-include ../../GIT-VERSION-FILE # this should be set to a 'standard' bsd-type install program INSTALL ?= install @@ -44,11 +47,11 @@ $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML) $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT) asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT) asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ test: $(MAKE) -C t/ test @@ -56,3 +59,5 @@ test: clean: rm -f *~ *.xml *.html *.1 rm -rf subproj mainline + +.PHONY: FORCE -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm 2014-05-06 12:41 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm 2014-05-06 12:41 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm @ 2014-05-06 12:41 ` James Denholm 2014-05-06 12:41 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm ` (2 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm $(libexecdir) isn't used anywhere else in the project, while $(gitexecdir) is the standard in the other appropriate makefiles. Hence, replace the former with the latter. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f63334b..579bb51 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -3,7 +3,7 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man -libexecdir ?= $(prefix)/libexec/git-core +gitexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 ../../GIT-VERSION-FILE: FORCE @@ -33,8 +33,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 -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm ` (2 preceding siblings ...) 2014-05-06 12:41 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm @ 2014-05-06 12:41 ` James Denholm 2014-05-06 12:41 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm 2014-05-06 21:17 ` [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass Junio C Hamano 5 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm git:Documentation/Makefile establishes asciidoc/xmlto calls as being handled through their appropriate variables, Hence, change to bring into congruency with. Similarly, MANPAGE_XSL exists in git:Documentation/Makefile, while MANPAGE_NORMAL_XSL does not outside contrib/subtree. Hence, replace MANPAGE_NORMAL_XSL with MANPAGE_XSL. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 579bb51..f3834b5 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -14,8 +14,11 @@ man1dir ?= $(mandir)/man1 # this should be set to a 'standard' bsd-type install program INSTALL ?= install -ASCIIDOC_CONF = ../../Documentation/asciidoc.conf -MANPAGE_NORMAL_XSL = ../../Documentation/manpage-normal.xsl +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 @@ -43,14 +46,14 @@ 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) \ + $(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) \ + $(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ -agit_version=$(GIT_VERSION) $^ test: -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm ` (3 preceding siblings ...) 2014-05-06 12:41 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm @ 2014-05-06 12:41 ` James Denholm 2014-05-06 21:17 ` [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass Junio C Hamano 5 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, James Denholm git:Documentation/Makefile and others establish "RM ?= rm -f" as a convention for rm calls in clean rules, hence follow this convention instead of simply forcing clean to use rm. subproj and mainline no longer need to be removed in clean, as they are no longer created in git:contrib/subtree by "make test". Hence, remove the rm call for those folders. Other makefiles don't remove "*~" files, remove the rm call to prevent unexpected behaviour in the future. Similarly, clean doesn't remove the installable file, so rectify this. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f3834b5..d888d45 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -12,7 +12,8 @@ man1dir ?= $(mandir)/man1 -include ../../GIT-VERSION-FILE # this should be set to a 'standard' bsd-type install program -INSTALL ?= install +INSTALL ?= install +RM ?= rm -f ASCIIDOC = asciidoc XMLTO = xmlto @@ -60,7 +61,7 @@ test: $(MAKE) -C t/ test clean: - rm -f *~ *.xml *.html *.1 - rm -rf subproj mainline + $(RM) $(GIT_SUBTREE) + $(RM) *.xml *.html *.1 .PHONY: FORCE -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm ` (4 preceding siblings ...) 2014-05-06 12:41 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm @ 2014-05-06 21:17 ` Junio C Hamano 2014-05-06 21:46 ` James Denholm 5 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2014-05-06 21:17 UTC (permalink / raw) To: James Denholm; +Cc: git James Denholm <nod.helm@gmail.com> writes: > 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. > > The main issues are that calls are made to git itself in the build > process, and that a subtree-exclusive variable is used for specifying > the exec path. Patches 1/5 through 3/5 resolve these. > > The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other > makefiles across the project. > > One problem is foreseen: 3/5 will necessitate that package maintainers > who already have git-subtree included in their packages update their > build-scripts. > > Reviewed-by: Jeff King <peff@peff.net> > Signed-off-by: James Denholm <nod.helm@gmail.com> > Based-on-patch-by: Dan McGee <dpmcgee@gmail.com> It is funny to see sign-off on 0/5 ;-) By the way, this is v3, not v2, no? It was somewhat confusing to see Peff saying "filfre to add my reviewed-by" on v2, noticing you posted something new, and not finding v3. Will queue. Thanks. > > James Denholm (5): > contrib/subtree/Makefile: scrap unused $(gitdir) > contrib/subtree/Makefile: Use GIT-VERSION-FILE > contrib/subtree/Makefile: s/libexecdir/gitexecdir > contrib/subtree/Makefile: Doc-gen rules cleanup > contrib/subtree/Makefile: clean rule cleanup > > contrib/subtree/Makefile | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass 2014-05-06 21:17 ` [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass Junio C Hamano @ 2014-05-06 21:46 ` James Denholm 0 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-06 21:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano <gitster@pobox.com> wrote: >It is funny to see sign-off on 0/5 ;-) Yeah, I wasn't quite sure of exact protocol, and sort-of defaulted to sign-all-the-things mode. >By the way, this is v3, not v2, no? It was somewhat confusing to >see Peff saying "filfre to add my reviewed-by" on v2, noticing you >posted something new, and not finding v3. Ah, right. I thought that resending a post-discussion patch was the done thing, given Documentation/SubmittingPatches, but that a comment line might not have been worth a version bump. >Will queue. Thanks. Awesome, thanks. Regards, James Denholm. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass @ 2014-05-03 12:49 James Denholm 2014-05-03 12:49 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm 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. The main issues are that calls are made to git itself in the build process, and that a subtree-exclusive variable is used for specifying the exec path. Patches 1/5 through 3/5 resolve these. The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other makefiles across the project. One problem is foreseen: 3/5 will necessitate that package maintainers who already have git-subtree included in their packages update their build-scripts. Signed-off-by: James Denholm <nod.helm@gmail.com> Based-on-patch-by: Dan McGee <dpmcgee@gmail.com> James Denholm (5): contrib/subtree/Makefile: scrap unused $(gitdir) contrib/subtree/Makefile: Use GIT-VERSION-FILE contrib/subtree/Makefile: s/libexecdir/gitexecdir contrib/subtree/Makefile: Doc-gen rules cleanup contrib/subtree/Makefile: clean rule cleanup contrib/subtree/Makefile | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) -- 1.9.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) 2014-05-03 12:49 James Denholm @ 2014-05-03 12:49 ` James Denholm 2014-05-03 12:49 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm All references were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5, but the assignment itself wasn't. Hence, drop gitdir assignment. Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 4030a16..87797ed 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -4,7 +4,6 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core -gitdir ?= $(shell git --exec-path) man1dir ?= $(mandir)/man1 gitver ?= $(word 3,$(shell git --version)) -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE 2014-05-03 12:49 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm @ 2014-05-03 12:49 ` James Denholm 2014-05-03 12:49 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm GVF is already being used in most/all other makefiles in the project, and has been for _quite_ a while. Hence, drop file-unique gitver and replace with GIT_VERSION. Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 87797ed..f63334b 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -6,7 +6,10 @@ mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 -gitver ?= $(word 3,$(shell git --version)) +../../GIT-VERSION-FILE: FORCE + $(MAKE) -C ../../ GIT-VERSION-FILE + +-include ../../GIT-VERSION-FILE # this should be set to a 'standard' bsd-type install program INSTALL ?= install @@ -44,11 +47,11 @@ $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML) $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT) asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT) asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ test: $(MAKE) -C t/ test @@ -56,3 +59,5 @@ test: clean: rm -f *~ *.xml *.html *.1 rm -rf subproj mainline + +.PHONY: FORCE -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir 2014-05-03 12:49 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm @ 2014-05-03 12:49 ` James Denholm 2014-05-03 12:49 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm $(libexecdir) isn't used anywhere else in the project, while $(gitexecdir) is the standard in the other appropriate makefiles. Hence, replace the former with the latter. Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f63334b..579bb51 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -3,7 +3,7 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man -libexecdir ?= $(prefix)/libexec/git-core +gitexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 ../../GIT-VERSION-FILE: FORCE @@ -33,8 +33,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 -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup 2014-05-03 12:49 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm @ 2014-05-03 12:49 ` James Denholm 2014-05-03 12:49 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm git:Documentation/Makefile establishes asciidoc/xmlto calls as being handled through their appropriate variables, Hence, change to bring into congruency with. Similarly, MANPAGE_XSL exists in git:Documentation/Makefile, while MANPAGE_NORMAL_XSL does not outside contrib/subtree. Hence, replace MANPAGE_NORMAL_XSL with MANPAGE_XSL. Signed-off-by: James Denholm <nod.helm@gmail.com> --- contrib/subtree/Makefile | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 579bb51..f3834b5 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -14,8 +14,11 @@ man1dir ?= $(mandir)/man1 # this should be set to a 'standard' bsd-type install program INSTALL ?= install -ASCIIDOC_CONF = ../../Documentation/asciidoc.conf -MANPAGE_NORMAL_XSL = ../../Documentation/manpage-normal.xsl +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 @@ -43,14 +46,14 @@ 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) \ + $(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) \ + $(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ -agit_version=$(GIT_VERSION) $^ test: -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-03 12:49 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm @ 2014-05-03 12:49 ` James Denholm 2014-05-05 5:09 ` Jeff King 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-03 12:49 UTC (permalink / raw) To: git; +Cc: greend, apenwarr, gpmcgee, peff, mmogilvi_git, James Denholm git:Documentation/Makefile and others establish "RM ?= rm -f" as a convention for rm calls in clean rules, hence follow this convention instead of simply forcing clean to use rm. subproj and mainline no longer need to be removed in clean, as they are no longer created in git:contrib/subtree by "make test". Hence, remove the rm call for those folders. Other makefiles don't remove "*~" files, remove the rm call to prevent unexpected behaviour in the future. Similarly, clean doesn't remove the installable file, so rectify this. Signed-off-by: James Denholm <nod.helm@gmail.com> --- Admittedly, git:Makefile does not itself follow the "RM ?= rm -f" setup, instead using "RM = rm -f", but I felt that there were probably $ARCANE_REASONS for this. Also, Peff, you were right about the dirs. contrib/subtree/Makefile | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f3834b5..4f96a24 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1 -include ../../GIT-VERSION-FILE -# this should be set to a 'standard' bsd-type install program -INSTALL ?= install +# These should be set to 'standard' bsd-type programs +INSTALL ?= install +RM ?= rm -f ASCIIDOC = asciidoc XMLTO = xmlto @@ -60,7 +61,7 @@ test: $(MAKE) -C t/ test clean: - rm -f *~ *.xml *.html *.1 - rm -rf subproj mainline + $(RM) $(GIT_SUBTREE) + $(RM) *.xml *.html *.1 .PHONY: FORCE -- 1.9.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-03 12:49 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm @ 2014-05-05 5:09 ` Jeff King 2014-05-05 21:41 ` James Denholm 0 siblings, 1 reply; 13+ messages in thread From: Jeff King @ 2014-05-05 5:09 UTC (permalink / raw) To: James Denholm; +Cc: git, greend, apenwarr, gpmcgee, mmogilvi_git On Sat, May 03, 2014 at 10:49:35PM +1000, James Denholm wrote: > diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile > index f3834b5..4f96a24 100644 > --- a/contrib/subtree/Makefile > +++ b/contrib/subtree/Makefile > @@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1 > > -include ../../GIT-VERSION-FILE > > -# this should be set to a 'standard' bsd-type install program > -INSTALL ?= install > +# These should be set to 'standard' bsd-type programs > +INSTALL ?= install > +RM ?= rm -f I do not think BSD-ism matters for "rm", as it works pretty much the same everywhere. "install", on the other hand, is a bit weirder between systems. So you might want to leave that comment as-is. OTOH, we do not even bother with such a comment in the main Makefile. -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-05 5:09 ` Jeff King @ 2014-05-05 21:41 ` James Denholm 2014-05-05 21:49 ` Jeff King 0 siblings, 1 reply; 13+ messages in thread From: James Denholm @ 2014-05-05 21:41 UTC (permalink / raw) To: Jeff King; +Cc: git, greend, apenwarr, gpmcgee, mmogilvi_git On 5 May 2014 15:09:39 GMT+10:00, Jeff King <peff@peff.net> wrote: >On Sat, May 03, 2014 at 10:49:35PM +1000, James Denholm wrote: > >> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile >> index f3834b5..4f96a24 100644 >> --- a/contrib/subtree/Makefile >> +++ b/contrib/subtree/Makefile >> @@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1 >> >> -include ../../GIT-VERSION-FILE >> >> -# this should be set to a 'standard' bsd-type install program >> -INSTALL ?= install >> +# These should be set to 'standard' bsd-type programs >> +INSTALL ?= install >> +RM ?= rm -f > >I do not think BSD-ism matters for "rm", as it works pretty much the >same everywhere. "install", on the other hand, is a bit weirder between >systems. So you might want to leave that comment as-is. True. I might just buff that out when sending the patch to Junio, unless protocol dictates otherwise - a reroll for a single comment line seems a bit excessive to me at the moment. Regards, James Denholm. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-05 21:41 ` James Denholm @ 2014-05-05 21:49 ` Jeff King 2014-05-05 21:59 ` James Denholm 0 siblings, 1 reply; 13+ messages in thread From: Jeff King @ 2014-05-05 21:49 UTC (permalink / raw) To: James Denholm; +Cc: git, greend, apenwarr, gpmcgee, mmogilvi_git On Tue, May 06, 2014 at 07:41:29AM +1000, James Denholm wrote: > >I do not think BSD-ism matters for "rm", as it works pretty much the > >same everywhere. "install", on the other hand, is a bit weirder between > >systems. So you might want to leave that comment as-is. > > True. I might just buff that out when sending the patch to Junio, unless > protocol dictates otherwise - a reroll for a single comment line seems > a bit excessive to me at the moment. I don't think it is that big a deal either way. It's fine to tweak when you send re-roll the final for Junio. Sometimes for trivial fixups like this, Junio can just tweak it as he applies, but I do not know if he is even paying attention to this thread, so you may want to re-post anyway to get his attention. Either way, feel free to add my: Reviewed-by: Jeff King <peff@peff.net> -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup 2014-05-05 21:49 ` Jeff King @ 2014-05-05 21:59 ` James Denholm 0 siblings, 0 replies; 13+ messages in thread From: James Denholm @ 2014-05-05 21:59 UTC (permalink / raw) To: Jeff King; +Cc: git, greend, apenwarr, gpmcgee, mmogilvi_git On 6 May 2014 07:49:30 GMT+10:00, Jeff King <peff@peff.net> wrote: >On Tue, May 06, 2014 at 07:41:29AM +1000, James Denholm wrote: > >> >I do not think BSD-ism matters for "rm", as it works pretty much the >> >same everywhere. "install", on the other hand, is a bit weirder >between >> >systems. So you might want to leave that comment as-is. >> >> True. I might just buff that out when sending the patch to Junio, >unless >> protocol dictates otherwise - a reroll for a single comment line >seems >> a bit excessive to me at the moment. > >I don't think it is that big a deal either way. > >It's fine to tweak when you send re-roll the final for Junio. Sometimes >for trivial fixups like this, Junio can just tweak it as he applies, >but >I do not know if he is even paying attention to this thread, so you may >want to re-post anyway to get his attention. Sure, sounds good and will do. >Either way, feel free to add my: > > Reviewed-by: Jeff King <peff@peff.net> Awesome, thanks again. Regards, James Denholm. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-05-06 21:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-06 12:41 [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass James Denholm 2014-05-06 12:41 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm 2014-05-06 12:41 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm 2014-05-06 12:41 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm 2014-05-06 12:41 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm 2014-05-06 12:41 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm 2014-05-06 21:17 ` [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass Junio C Hamano 2014-05-06 21:46 ` James Denholm -- strict thread matches above, loose matches on Subject: below -- 2014-05-03 12:49 James Denholm 2014-05-03 12:49 ` [PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir) James Denholm 2014-05-03 12:49 ` [PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE James Denholm 2014-05-03 12:49 ` [PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir James Denholm 2014-05-03 12:49 ` [PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup James Denholm 2014-05-03 12:49 ` [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup James Denholm 2014-05-05 5:09 ` Jeff King 2014-05-05 21:41 ` James Denholm 2014-05-05 21:49 ` Jeff King 2014-05-05 21:59 ` James Denholm
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).