Re: [PATCH v3] diff-highlight: make install link into DESTDIR
Resending this mail to the list 'cause mail-client added html to previous mail.
On Sat, 12 Oct 2024 at 22:55, Jeff King <peff@peff.net> wrote:
Show 52 quoted lines
>
> On Sat, Oct 12, 2024 at 08:23:33PM +0000, immeëmosol via GitGitGadget wrote:
>
> > diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
> > index f2be7cc9243..a53e09e0bdd 100644
> > --- a/contrib/diff-highlight/Makefile
> > +++ b/contrib/diff-highlight/Makefile
> > @@ -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...
>
> > clean:
> > + 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
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
Signed-off-by: imme=C3=ABmosol <will+developer@willfris.nl>
---
contrib/diff-highlight/Makefile | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
Show changes to contrib/diff-highlight/Makefile +12 −1
diff --git a/contrib/diff-highlight/Makefile
b/contrib/diff-highlight/Makefile
index f2be7cc9243719..84f6e65c730380 100644
--- a/contrib/diff-highlight/Makefile
+++ b/contrib/diff-highlight/Makefile
@@ -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
immeëmosol via GitGitGadget
