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

PATCH In review Christian Hesse 4 participants 12 messages Oct 7, 2018 – Oct 18, 2018

lore

Share & tools

lore

Christian Hesse

author

2018-10-07 07:39 UTC #0 lore raw

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

From: Christian Hesse <[email protected]>

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

Signed-off-by Christian Hesse <[email protected]>

---
 contrib/subtree/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

contrib/subtree/Makefile +4 -0

index 5c6cc4ab2c..6906aae441 100644
@@ -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

2018-10-10 02:26 UTC #1 lore raw

Review

Quoted 9 lines · Christian Hesse

From: Christian Hesse <[email protected]>
We have targets 'install-man' and 'install-html', let's add build targets as well.
...
(quoted diff) +4 -0
+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?

contrib/subtree/Makefile +7 -3

index 5c6cc4ab2c..4a10a020a0 100644
@@ -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

2018-10-12 08:08 UTC #2 lore raw

Quoted 4 lines · Junio C Hamano

Quoted 2 lines · Christian Hesse
From: Christian Hesse <[email protected]>

Quoted 11 lines

contrib/subtree/Makefile $(GIT_SUBTREE): $(GIT_SUBTREE_SH) · line 65
Quoted 7 lines
We have targets 'install-man' and 'install-html', let's add build targets as well.
...
contrib/subtree/Makefile +4 -0
+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

Quoted 38 lines

contrib/subtree/Makefile +7 -3

index 5c6cc4ab2c..4a10a020a0 100644
@@ -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

author

2018-10-15 21:24 UTC #3 lore raw

In reply to Junio C Hamano

Junio C Hamano <[email protected]> on Wed, 2018/10/10 11:26:

Quoted 3 lines

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?

Signature · 3 lines

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

2018-10-16 02:05 UTC #4 lore raw

Quoted 8 lines · Christian Hesse

Junio C Hamano <[email protected]> on Wed, 2018/10/10 11:26:
Quoted 3 lines
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

2018-10-16 05:01 UTC #5 lore raw

Quoted 7 lines · Junio C Hamano

Quoted 5 lines · Christian Hesse
Junio C Hamano <[email protected]> on Wed, 2018/10/10 11:26:
Quoted 3 lines
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?

Quoted 6 lines

Quoted 3 lines
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

author

2018-10-16 07:57 UTC #6 lore raw

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

From: Christian Hesse <[email protected]>

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

Signed-off-by Christian Hesse <[email protected]>

---
 contrib/subtree/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

contrib/subtree/Makefile +3 -3

index 6906aae441..4a10a020a0 100644
@@ -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

2018-10-16 08:00 UTC #7 lore raw

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

Quoted 9 lines · Christian Hesse

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

Thanks, will queue.

Quoted 24 lines

contrib/subtree/Makefile +3 -3

index 6906aae441..4a10a020a0 100644
@@ -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

2018-10-17 02:45 UTC #8 lore raw

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

In reply to Christian Hesse

Hi,

Quoted 3 lines · Christian Hesse

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

[...]

contrib/subtree/Makefile install: $(GIT_SUBTREE) · line 76

Quoted 2 lines

contrib/subtree/Makefile +1 -1
-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

2018-10-18 02:05 UTC #9 lore raw

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

Quoted 10 lines · Jonathan Nieder

contrib/subtree/Makefile install: $(GIT_SUBTREE) · line 76
Quoted 2 lines
contrib/subtree/Makefile +1 -1
-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

Quoted 8 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. How was this patch tested?

Gah, that was silly of me.

Junio C Hamano

2018-10-18 02:09 UTC #10 lore raw

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

In reply to Jonathan Nieder

Quoted 7 lines · Jonathan Nieder

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 <[email protected]>
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(-)

contrib/subtree/Makefile +3 -3

index 4a10a020a0..6906aae441 100644
@@ -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

Signature · 1 line

2.19.1-450-ga4b8ab5363

Christian Hesse

author

2018-10-18 08:05 UTC #11 lore raw

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

Junio C Hamano <[email protected]> on Thu, 2018/10/18 11:09:

Quoted 26 lines

Quoted 7 lines · Jonathan Nieder
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 <[email protected]>
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 clean: · line 101

Quoted 5 lines

contrib/subtree/Makefile +1 -1

@@ -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...

Signature · 3 lines

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);}