threads / patch / 49502

patchsubtree: add build targets 'man' and 'html'

Subject: [PATCH 1/1] subtree: add build targets 'man' and 'html'

## tl;dr

12 messages between Oct 7, 2018 and Oct 18, 2018. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Christian Hesse· Oct 7, 2018, 07:30 UTC · lore

From: Christian Hesse <mail@eworm.de>

We have targets 'install-man' and 'install-html', let's add build targets as well.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 contrib/subtree/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

Show changes to contrib/subtree/Makefile +4 −0

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 5c6cc4ab2c..6906aae441 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -59,6 +59,10 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 
 doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)
 
+man: $(GIT_SUBTREE_DOC)
+
+html: $(GIT_SUBTREE_HTML)
+
 install: $(GIT_SUBTREE)
 	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
 	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)

Junio C Hamano· Oct 10, 2018, 02:26 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] subtree: add build targets 'man' and 'html'

Christian Hesse <list@eworm.de> writes:

Show 9 quoted lines

> From: Christian Hesse <mail@eworm.de>
>
> We have targets 'install-man' and 'install-html', let's add build
> targets as well.
>   ...
> +man: $(GIT_SUBTREE_DOC)
> +
> +html: $(GIT_SUBTREE_HTML)
> +

As 'contrib' material without real maintenance, I do not care too deeply, but shouldn't this change be more like this to avoid duplicating the list of targets?

Show changes to contrib/subtree/Makefile +7 −3

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 5c6cc4ab2c..4a10a020a0 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -59,17 +59,21 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 
 doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)
 
+man: $(GIT_SUBTREE_DOC)
+
+html: $(GIT_SUBTREE_HTML)
+
 install: $(GIT_SUBTREE)
 	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
 	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)
 
 install-doc: install-man install-html
 
-install-man: $(GIT_SUBTREE_DOC)
+install-man: man
 	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
-install-html: $(GIT_SUBTREE_HTML)
+install-html: html
 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
 
@@ -94,4 +98,4 @@ clean:
 	$(RM) $(GIT_SUBTREE)
 	$(RM) *.xml *.html *.1
 
-.PHONY: FORCE
+.PHONY: FORCE man html install-man install-html

Johannes Schindelin· Oct 12, 2018, 08:08 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] subtree: add build targets 'man' and 'html'

On Wed, 10 Oct 2018, Junio C Hamano wrote:

Show 15 quoted lines

> Christian Hesse <list@eworm.de> writes:
> 
> > From: Christian Hesse <mail@eworm.de>
> >
> > We have targets 'install-man' and 'install-html', let's add build
> > targets as well.
> >   ...
> > +man: $(GIT_SUBTREE_DOC)
> > +
> > +html: $(GIT_SUBTREE_HTML)
> > +
> 
> As 'contrib' material without real maintenance, I do not care too
> deeply, but shouldn't this change be more like this to avoid
> duplicating the list of targets?

Ævar mentioned that he'd like this to graduate from contrib into core, and I had briefly looked at making subtree a built-in to that end. IOW please do not get too used to subtree being in contrib.

Ciao, Dscho

Show 38 quoted lines

> 
> 
> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
> index 5c6cc4ab2c..4a10a020a0 100644
> --- a/contrib/subtree/Makefile
> +++ b/contrib/subtree/Makefile
> @@ -59,17 +59,21 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
>  
>  doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)
>  
> +man: $(GIT_SUBTREE_DOC)
> +
> +html: $(GIT_SUBTREE_HTML)
> +
>  install: $(GIT_SUBTREE)
>  	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
>  	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)
>  
>  install-doc: install-man install-html
>  
> -install-man: $(GIT_SUBTREE_DOC)
> +install-man: man
>  	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
>  	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
>  
> -install-html: $(GIT_SUBTREE_HTML)
> +install-html: html
>  	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
>  	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
>  
> @@ -94,4 +98,4 @@ clean:
>  	$(RM) $(GIT_SUBTREE)
>  	$(RM) *.xml *.html *.1
>  
> -.PHONY: FORCE
> +.PHONY: FORCE man html install-man install-html
> 
>

Christian Hesse· Oct 15, 2018, 21:15 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] subtree: add build targets 'man' and 'html'

Junio C Hamano <gitster@pobox.com> on Wed, 2018/10/10 11:26:

> As 'contrib' material without real maintenance, I do not care too
> deeply, but shouldn't this change be more like this to avoid
> duplicating the list of targets?

Probably, yes. Do you want to add this yourself or do you want me to send an updated patch or one on top of the last change?

-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

Junio C Hamano· Oct 16, 2018, 02:05 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] subtree: add build targets 'man' and 'html'

Christian Hesse <list@eworm.de> writes:

Show 8 quoted lines

> Junio C Hamano <gitster@pobox.com> on Wed, 2018/10/10 11:26:
>> As 'contrib' material without real maintenance, I do not care too
>> deeply, but shouldn't this change be more like this to avoid
>> duplicating the list of targets?
>
> Probably, yes.
> Do you want to add this yourself or do you want me to send an updated patch
> or one on top of the last change?

In principle either is fine but keep in mind that I'll likely forget if you leave it up to me.

Junio C Hamano· Oct 16, 2018, 05:01 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] subtree: add build targets 'man' and 'html'

Junio C Hamano <gitster@pobox.com> writes:

Show 13 quoted lines

> Christian Hesse <list@eworm.de> writes:
>
>> Junio C Hamano <gitster@pobox.com> on Wed, 2018/10/10 11:26:
>>> As 'contrib' material without real maintenance, I do not care too
>>> deeply, but shouldn't this change be more like this to avoid
>>> duplicating the list of targets?
>>
>> Probably, yes.
>> Do you want to add this yourself or do you want me to send an updated patch
>> or one on top of the last change?
>
> In principle either is fine but keep in mind that I'll likely forget
> if you leave it up to me.

Actully, I take it back. The original patch is already in 'next', so an incremental on top of what you sent is the only valid improvement ;-)

Christian Hesse· Oct 16, 2018, 07:56 UTC · re: Junio C Hamano · lore

[PATCH 1/1] subtree: make install targets depend on build targets

From: Christian Hesse <mail@eworm.de>

Now that we have build targets let the install targets depend on them. Also make the targets phony.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 contrib/subtree/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Show changes to contrib/subtree/Makefile +3 −3

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 6906aae441..4a10a020a0 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -69,11 +69,11 @@ install: $(GIT_SUBTREE)
 
 install-doc: install-man install-html
 
-install-man: $(GIT_SUBTREE_DOC)
+install-man: man
 	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
-install-html: $(GIT_SUBTREE_HTML)
+install-html: html
 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
 
@@ -98,4 +98,4 @@ clean:
 	$(RM) $(GIT_SUBTREE)
 	$(RM) *.xml *.html *.1
 
-.PHONY: FORCE
+.PHONY: FORCE man html install-man install-html

Junio C Hamano· Oct 16, 2018, 08:00 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] subtree: make install targets depend on build targets

Christian Hesse <list@eworm.de> writes:

Show 9 quoted lines

> From: Christian Hesse <mail@eworm.de>
>
> Now that we have build targets let the install targets depend on them.
> Also make the targets phony.
>
> Signed-off-by: Christian Hesse <mail@eworm.de>
> ---
>  contrib/subtree/Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, will queue.

Show 24 quoted lines

> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
> index 6906aae441..4a10a020a0 100644
> --- a/contrib/subtree/Makefile
> +++ b/contrib/subtree/Makefile
> @@ -69,11 +69,11 @@ install: $(GIT_SUBTREE)
>  
>  install-doc: install-man install-html
>  
> -install-man: $(GIT_SUBTREE_DOC)
> +install-man: man
>  	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
>  	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
>  
> -install-html: $(GIT_SUBTREE_HTML)
> +install-html: html
>  	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
>  	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
>  
> @@ -98,4 +98,4 @@ clean:
>  	$(RM) $(GIT_SUBTREE)
>  	$(RM) *.xml *.html *.1
>  
> -.PHONY: FORCE
> +.PHONY: FORCE man html install-man install-html

Jonathan Nieder· Oct 17, 2018, 02:45 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] subtree: make install targets depend on build targets

Hi,

Christian Hesse wrote:

> --- a/contrib/subtree/Makefile
> +++ b/contrib/subtree/Makefile
> @@ -69,11 +69,11 @@ install: $(GIT_SUBTREE)

[...]

> -install-html: $(GIT_SUBTREE_HTML)
> +install-html: html

This broke the build for me:

 make[2]: Entering directory '/build/git-2.19.1+next.20181016/contrib/subtree'
 install -m 644 html /build/git-2.19.1+next.20181016/debian/tmp/usr/share/doc/git/html
 install: cannot stat 'html': No such file or directory
 make[2]: *** [Makefile:78: install-html] Error 1

The rule says

 install-html: html
	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)

and $^ substitutes to "html" after this change. How was this patch tested?

Thanks, Jonathan

Junio C Hamano· Oct 18, 2018, 02:05 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/1] subtree: make install targets depend on build targets

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 18 quoted lines

>> -install-html: $(GIT_SUBTREE_HTML)
>> +install-html: html
>
> This broke the build for me:
>
>  make[2]: Entering directory '/build/git-2.19.1+next.20181016/contrib/subtree'
>  install -m 644 html /build/git-2.19.1+next.20181016/debian/tmp/usr/share/doc/git/html
>  install: cannot stat 'html': No such file or directory
>  make[2]: *** [Makefile:78: install-html] Error 1
>
> The rule says
>
>  install-html: html
> 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
> 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
>
> and $^ substitutes to "html" after this change.  How was this patch
> tested?

Gah, that was silly of me.

Junio C Hamano· Oct 18, 2018, 02:09 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/1] subtree: make install targets depend on build targets

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 7 quoted lines

> The rule says
>
>  install-html: html
> 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
> 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
>
> and $^ substitutes to "html" after this change.

Sorry about that.

From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 18 Oct 2018 11:07:17 +0900
Subject: [PATCH] Revert "subtree: make install targets depend on build targets"

This reverts commit 744f7c4c314dc0e7816ac05520e8358c8318187a.

These targets do depend on the fact that each prereq is explicitly
listed via their use of $^, which I failed to notice, and broke the
build.
---
 contrib/subtree/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Show changes to contrib/subtree/Makefile +3 −3

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 4a10a020a0..6906aae441 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -69,11 +69,11 @@ install: $(GIT_SUBTREE)
 
 install-doc: install-man install-html
 
-install-man: man
+install-man: $(GIT_SUBTREE_DOC)
 	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
-install-html: html
+install-html: $(GIT_SUBTREE_HTML)
 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
 
@@ -98,4 +98,4 @@ clean:
 	$(RM) $(GIT_SUBTREE)
 	$(RM) *.xml *.html *.1
 
-.PHONY: FORCE man html install-man install-html
+.PHONY: FORCE

-- 
2.19.1-450-ga4b8ab5363

Christian Hesse· Oct 18, 2018, 08:05 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] subtree: make install targets depend on build targets

Junio C Hamano <gitster@pobox.com> on Thu, 2018/10/18 11:09:

Show 31 quoted lines

> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > The rule says
> >
> >  install-html: html
> > 	$(INSTALL) -d -m 755 $(DESTDIR)$(htmldir)
> > 	$(INSTALL) -m 644 $^ $(DESTDIR)$(htmldir)
> >
> > and $^ substitutes to "html" after this change.  
> 
> Sorry about that.
> 
> From: Junio C Hamano <gitster@pobox.com>
> Date: Thu, 18 Oct 2018 11:07:17 +0900
> Subject: [PATCH] Revert "subtree: make install targets depend on build
> targets"
> 
> This reverts commit 744f7c4c314dc0e7816ac05520e8358c8318187a.
> 
> These targets do depend on the fact that each prereq is explicitly
> listed via their use of $^, which I failed to notice, and broke the
> build.
>
> [...]
>
> @@ -98,4 +98,4 @@ clean:
>  	$(RM) $(GIT_SUBTREE)
>  	$(RM) *.xml *.html *.1
>  
> -.PHONY: FORCE man html install-man install-html
> +.PHONY: FORCE

We could keep the phony part at least...

-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}