diff-highlight: link to diff-highlight in DESTDIR #Makefile #diff-highlight
PATCH 1/1 diff-highlight: link to diff-highlight in DESTDIR #Makefile #diff-highlight
From: =?UTF-8?q?imme=C3=ABmosol?= <[email protected]>
Make git's diff-highlight program immediately available to the command-line. Create a link in DESTDIR that
refers to the generated/concatenated diff-highlight perl script
---
add symlinking diff-highlight into DESTDIR
In order to immediately make git's diff-highlight program available for command-line instructions; add a symlink to the generated/concatenated diff-highlight perl script(s) in DESTDIR
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-938%2Fimme-emosol%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-938/imme-emosol/patch-1-v1
Pull-Request: https://github.com/git/git/pull/938
contrib/diff-highlight/Makefile | 2 ++ 1 file changed, 2 insertions(+)
contrib/diff-highlight/Makefile +2 -0
@@ -9,6 +9,7 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl cat $^ >$@+ chmod +x $@+ mv $@+ $@ + ln --symbolic --target-directory=$(DESTDIR) $(abspath $@) shebang.perl: FORCE @echo '#!$(PERL_PATH_SQ)' >$@+
@@ -19,5 +20,6 @@ test: all clean: $(RM) diff-highlight + $(RM) --force $(DESTDIR)diff-highlight .PHONY: FORCE
base-commit: ef8ce8f3d4344fd3af049c17eeba5cd20d98b69f
Signature · 1 line
gitgitgadget
contrib/diff-highlight/Makefile diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl · line 12
Quoted 9 lines · immeëmosol via GitGitGadget
contrib/diff-highlight/Makefile +1
-0@@ -9,6 +9,7 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl cat $^ >$@+ chmod +x $@+ mv $@+ $@ + ln --symbolic --target-directory=$(DESTDIR) $(abspath $@)
Hmm. I am not opposed to having diff-highlight's Makefile be responsible for installing a symbolic link to the generated script, but I do not think that this Makefile recipe is the right place to do it.
This recipe is about building the executable, not installing it. If you want to introduce a separate .PHONY recipe for installing the script, I think that would be a better place to introduce this change.
Thanks,
Taylor
PATCH v2 1/1 diff-highlight: make install link into DESTDIR #Makefile
In reply to immeëmosol via GitGitGadgetFrom: =?UTF-8?q?imme=C3=ABmosol?= <[email protected]>
Make git's diff-highlight program immediately available to the command-line. Create a link in DESTDIR that
refers to the generated/concatenated diff-highlight perl script
---
add symlinking diff-highlight into DESTDIR
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-938%2Fimme-emosol%2Fpatch-1-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-938/imme-emosol/patch-1-v2
Pull-Request: https://github.com/git/git/pull/938
Range-diff vs v1:
1: e23aeb1a76c ! 1: ca76f336ded diff-highlight: link to diff-highlight in DESTDIR #Makefile #diff-highlight
@@ Metadata
Author: immeëmosol <[email protected]>
## Commit message ##
- diff-highlight: link to diff-highlight in DESTDIR #Makefile #diff-highlight
+ diff-highlight: make install link into DESTDIR #Makefile
Make git's diff-highlight program immediately available to the command-line. Create a link in DESTDIR that
@@ Commit message
## contrib/diff-highlight/Makefile ##
@@ contrib/diff-highlight/Makefile: diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl
- cat $^ >$@+
chmod +x $@+
mv $@+ $@
-+ ln --symbolic --target-directory=$(DESTDIR) $(abspath $@)
++install: diff-highlight
++ test -w $(DESTDIR) && \
++ ln --symbolic --target-directory=$(DESTDIR) $(abspath $<)
++
shebang.perl: FORCE
@echo '#!$(PERL_PATH_SQ)' >$@+
+ @cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@
@@ contrib/diff-highlight/Makefile: test: all
+ $(MAKE) -C t
clean:
++ test ! -L $(DESTDIR)/diff-highlight || \
++ $(RM) --force $(DESTDIR)/diff-highlight
$(RM) diff-highlight
-+ $(RM) --force $(DESTDIR)diff-highlight
.PHONY: FORCE
++.PHONY: install
contrib/diff-highlight/Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
contrib/diff-highlight/Makefile +7 -0
@@ -10,6 +10,10 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl chmod +x $@+ mv $@+ $@ +install: diff-highlight + test -w $(DESTDIR) && \ + ln --symbolic --target-directory=$(DESTDIR) $(abspath $<) + shebang.perl: FORCE @echo '#!$(PERL_PATH_SQ)' >$@+ @cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@
@@ -18,6 +22,9 @@ test: all $(MAKE) -C t clean: + test ! -L $(DESTDIR)/diff-highlight || \ + $(RM) --force $(DESTDIR)/diff-highlight $(RM) diff-highlight .PHONY: FORCE +.PHONY: install
base-commit: ef8ce8f3d4344fd3af049c17eeba5cd20d98b69f
Signature · 1 line
gitgitgadget
Re: [PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
Quoted 1 line · immeëmosol via GitGitGadget
[PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
What does #Makefile mean? Previous subject had two of these:
diff-highlight: link to diff-highlight in DESTDIR #Makefile #diff-highlight
Signature · 1 line
Kristoffer Haugsbakk
Re: [PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
Quoted 5 lines · On Sat, 12 Oct 2024, 20:35 Kristoffer Haugsbakk…
Quoted 1 line · immeëmosol via GitGitGadget
[PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
What does
#Makefilemean? […]
not much.
It is an attempt to signify what the commit relates to.
In this case it might hint at the commit not impacting the sources of what is built, but the build process itself.
Maybe #🏗️ would be more clear.
Re: [PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
Quoted 6 lines · immeëmosol
Quoted 4 lines · On Sat, 12 Oct 2024, 20:35 Kristoffer Haugsbakk…
Quoted 1 line · immeëmosol via GitGitGadget
[PATCH v2] diff-highlight: make install link into DESTDIR #Makefile
Quoted 7 lines
Quoted 1 line
What does
#Makefilemean? […]not much.
It is an attempt to signify what the commit relates to.
In this case it might hint at the commit not impacting the sources of what is built, but the build process itself.
Maybe #🏗️ would be more clear.
Drop all these. Study "git shortlog --no-merges" from the recent history and try to blend in. Don't try to be original where the originality does not matter.
Also, resist the temptation to go outside POSIX.1 needlessly.
https://pubs.opengroup.org/onlinepubs/9799919799/utilities/rm.html#tag_20_104
does not say "rm" takes "--force" and
https://pubs.opengroup.org/onlinepubs/9799919799/utilities/ln.html#tag_20_67
does not say "ln" takes "--symbolic" nor "--target-directory".
Be inclusive and help those whose userland tools are not tainted by GNUisms.
Thanks.
PATCH v3 1/1 diff-highlight: make install link into DESTDIR
In reply to immeëmosol via GitGitGadgetFrom: =?UTF-8?q?imme=C3=ABmosol?= <[email protected]>
Make git's diff-highlight program immediately available to the command-line. Create a link in DESTDIR that
refers to the generated/concatenated diff-highlight perl script
---
add symlinking diff-highlight into DESTDIR
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-938%2Fimme-emosol%2Fpatch-1-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-938/imme-emosol/patch-1-v3
Pull-Request: https://github.com/git/git/pull/938
Range-diff vs v2:
1: ca76f336ded ! 1: af4bea815fa diff-highlight: make install link into DESTDIR #Makefile
@@ Metadata
Author: immeëmosol <[email protected]>
## Commit message ##
- diff-highlight: make install link into DESTDIR #Makefile
+ diff-highlight: make install link into DESTDIR
Make git's diff-highlight program immediately available to the command-line.
Create a link in DESTDIR that
@@ contrib/diff-highlight/Makefile: diff-highlight: shebang.perl DiffHighlight.pm d
mv $@+ $@
+install: diff-highlight
-+ test -w $(DESTDIR) && \
-+ ln --symbolic --target-directory=$(DESTDIR) $(abspath $<)
++ test -w $(DESTDIR) && ln -s $(abspath $<) $(DESTDIR)
+
shebang.perl: FORCE
@echo '#!$(PERL_PATH_SQ)' >$@+
@@ contrib/diff-highlight/Makefile: test: all
clean:
+ test ! -L $(DESTDIR)/diff-highlight || \
-+ $(RM) --force $(DESTDIR)/diff-highlight
++ $(RM) -f $(DESTDIR)/diff-highlight
$(RM) diff-highlight
.PHONY: FORCEcontrib/diff-highlight/Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
contrib/diff-highlight/Makefile +6 -0
@@ -10,6 +10,9 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl chmod +x $@+ mv $@+ $@ +install: diff-highlight + test -w $(DESTDIR) && ln -s $(abspath $<) $(DESTDIR) + shebang.perl: FORCE @echo '#!$(PERL_PATH_SQ)' >$@+ @cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@
@@ -18,6 +21,9 @@ test: all $(MAKE) -C t clean: + test ! -L $(DESTDIR)/diff-highlight || \ + $(RM) -f $(DESTDIR)/diff-highlight $(RM) diff-highlight .PHONY: FORCE +.PHONY: install
base-commit: ef8ce8f3d4344fd3af049c17eeba5cd20d98b69f
Signature · 1 line
gitgitgadget
Re: [PATCH v3] diff-highlight: make install link into DESTDIR
contrib/diff-highlight/Makefile diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl · line 15
Quoted 11 lines · immeëmosol via GitGitGadget
contrib/diff-highlight/Makefile +3
-0@@ -10,6 +10,9 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl chmod +x $@+ mv $@+ $@ +install: diff-highlight + test -w $(DESTDIR) && ln -s $(abspath $<) $(DESTDIR) +
I'm not opposed to having an install target here, like we do in the main Makefile and in a few other contrib directories.
But in that case, I think it should behave more like those other targets:
1. Actually copy the program rather than making a symlink. Preferably
using $(INSTALL).
2. Respect $(prefix) in the usual way.
And also...
contrib/diff-highlight/Makefile test: all · line 26
Quoted 4 lines
contrib/diff-highlight/Makefile +2
-0clean: + test ! -L $(DESTDIR)/diff-highlight || \ + $(RM) -f $(DESTDIR)/diff-highlight $(RM) diff-highlight
3. It's unusual for "clean" to reach outside of the build directory. What you're doing here is more like an "uninstall" target, but we don't usually provide one.
There are a few different approaches other contrib/ items take to work like the rest of the Git:
- in contrib/contacts, we source config.mak from the top-level, and then define a default $(prefix). This gives some repeated boilerplate, but is pretty independent from the top-level Makefile.
- in contrib/credential/netrc, we piggy-back on the top-level Makefile's "install-perl-script", which knows where the user has asked us to install things. That might not be appropriate here, though, as I think it only puts things in libexec/, so "diff-highlight" wouldn't be generally available in the user's $PATH (though it would be enough to use as a pager within git).
-Peff
Re: [PATCH v3] diff-highlight: make install link into DESTDIR
Resending this mail to the list 'cause mail-client added html to previous mail.
Quoted 44 lines · Jeff King
contrib/diff-highlight/Makefile diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl · line 15
Quoted 11 lines · immeëmosol via GitGitGadget
contrib/diff-highlight/Makefile +3
-0@@ -10,6 +10,9 @@ diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl chmod +x $@+ mv $@+ $@ +install: diff-highlight + test -w $(DESTDIR) && ln -s $(abspath $<) $(DESTDIR) +I'm not opposed to having an install target here, like we do in the main
Makefileand in a few other contrib directories.But in that case, I think it should behave more like those other targets:
1. Actually copy the program rather than making a symlink. Preferably
using $(INSTALL).2. Respect $(prefix) in the usual way.
And also...
contrib/diff-highlight/Makefile diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl
Quoted 4 lines
contrib/diff-highlight/Makefile +2
-0clean: + test ! -L $(DESTDIR)/diff-highlight || \ + $(RM) -f $(DESTDIR)/diff-highlight $(RM) diff-highlight3. It's unusual for "clean" to reach outside of the build directory. What you're doing here is more like an "uninstall" target, but we don't usually provide one.
There are a few different approaches other contrib/ items take to work like the rest of the Git:
- in contrib/contacts, we source config.mak from the top-level, and then define a default $(prefix). This gives some repeated boilerplate, but is pretty independent from the top-level
Makefile.
Quoted 8 lines
- in contrib/credential/netrc, we piggy-back on the top-level Makefile's "install-perl-script", which knows where the user has asked us to install things. That might not be appropriate here, though, as I think it only puts things in libexec/, so "diff-highlight" wouldn't be generally available in the user's $PATH (though it would be enough to use as a pager within git).
-Peff
As mentioned, contrib/diff-highlight is less like other perl contribs like contrib/contacts and contrib/credential/netrc, those two seem to be git subcommands (git-*) where diff-highlight is more of a "standalone" command.
My usecase was to peek at what the command does by making it available in a $PATH writable by a non-root user. (Much like what is mentioned in contrib/diff-highlight/README#Use: git log -p --color | diff-highlight.= )
```sh
echo '# Given ~/.local/bin is in $PATH,'
( export DESTDIR=3D"${HOME?}/.local/bin/" ; make linked-in-destdir )
echo '# In another already open shell, try suggestion from readme.'
( export DESTDIR=3D"${HOME?}/.local/bin/" ; make clean )
```Thanks to all of you for the introduction into how contributions to git/git are handled.
Though it has been quite an informative introduction, and i can understand the suggestion of making it a install-target like other contrib-parts, i am currently not spending more time on this. Thanks again, and have a good day= .
---
Make git's diff-highlight program immediately available to the command-line= .
Create a link in DESTDIR that
refers to the generated/concatenated diff-highlight perl script
--- contrib/diff-highlight/Makefile | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/contrib/diff-highlight/Makefile
index f2be7cc9243719..84f6e65c730380 100644
contrib/diff-highlight/Makefile +0 -0
@@ -10,6 +10,11 @@ diff-highlight: shebang.perl DiffHighlight.pm
diff-highlight.perl
chmod +x $@+
mv $@+ $@+linked-in-destdir: diff-highlight
+ test -n "$(DESTDIR)" && \
+ test -w $(DESTDIR) && \
+ ln -s $(abspath $<) $(DESTDIR)
+
shebang.perl: FORCE
@echo '#!$(PERL_PATH_SQ)' >$@+
@cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@
@@ -17,7 +22,13 @@ shebang.perl: FORCE
test: all
$(MAKE) -C t-clean:
+unlink-from-destdir:
+ test -z "$(DESTDIR)" || \
+ test ! -L $(DESTDIR)/diff-highlight || \
+ $(RM) $(DESTDIR)/diff-highlight
+
+clean: unlink-from-destdir
$(RM) diff-highlight
.PHONY: FORCE
+.PHONY: linked-in-destdir unlink-from-destdir
Re: [PATCH v3] diff-highlight: make install link into DESTDIR
Quoted 10 lines · immeëmosol
As mentioned,
contrib/diff-highlightis less like other perl contribs likecontrib/contactsandcontrib/credential/netrc, those two seem to be git subcommands (git-*) where diff-highlight is more of a "standalone" command.My usecase was to peek at what the command does by making it available in a
$PATHwritable by a non-root user. (Much like what is mentioned incontrib/diff-highlight/README#Use:git log -p --color | diff-highlight.= )
Quoted 6 lines
```sh
echo '# Given ~/.local/bin is in $PATH,'
( export DESTDIR=3D"${HOME?}/.local/bin/" ; make linked-in-destdir )
echo '# In another already open shell, try suggestion from readme.'
( export DESTDIR=3D"${HOME?}/.local/bin/" ; make clean )
```Nah, it isn't DESTDIR's usage, it's prefix job!
make prefix=${HOME}/.local installQuoted 21 lines
---
Make git's diff-highlight program immediately available to the command-line= .
Create a link in DESTDIR that
refers to the generated/concatenated diff-highlight perl scriptSigned-off-by imme=C3=ABmosol <[email protected]>--- contrib/diff-highlight/Makefile | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)contrib/diff-highlight/Makefile +0
-0@@ -10,6 +10,11 @@ diff-highlight: shebang.perl DiffHighlight.pmdiff-highlight.perl
chmod +x $@+ mv $@+ $@
contrib/diff-highlight/Makefile
Quoted 4 lines
contrib/diff-highlight/Makefile +4
-0+linked-in-destdir: diff-highlight + test -n "$(DESTDIR)" && \ + test -w $(DESTDIR) && \ + ln -s $(abspath $<) $(DESTDIR)
So it would be something like this:
install: diff-highlight $(INSTALL) diff-highlight '$(DESTDIR)$(bindir_SQ)'
Quoted 20 lines
contrib/diff-highlight/Makefile +8
-1+ shebang.perl: FORCE @echo '#!$(PERL_PATH_SQ)' >$@+ @cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@@@ -17,7 +22,13 @@ shebang.perl: FORCE test: all $(MAKE) -C t -clean: +unlink-from-destdir: + test -z "$(DESTDIR)" || \ + test ! -L $(DESTDIR)/diff-highlight || \ + $(RM) $(DESTDIR)/diff-highlight + +clean: unlink-from-destdir $(RM) diff-highlight .PHONY: FORCE +.PHONY: linked-in-destdir unlink-from-destdir
Signature · 1 line
Danh
Re: [PATCH v3] diff-highlight: make install link into DESTDIR
In reply to immeëmosolQuoted 7 lines · immeëmosol
Thanks to all of you for the introduction into how contributions to git/git are handled.
Though it has been quite an informative introduction, and i can understand the suggestion of making it a install-target like other contrib-parts, i am currently not spending more time on this. Thanks again, and have a good day=
It is a bit sad to hear that you do not have time to bring this patch over the finish line, having received some useful pointers from others on the list.
Any takers who might want to pick this up instead?
Thanks,
Taylor