Re: SKIP_DASHED_BUILT_INS does not install git-*-pack
On Mon, Oct 19, 2020 at 06:54:29PM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Michael Forney <mforney@mforney.org> writes:
>
> > I saw that git 2.29.0 introduced a new make variable
> > SKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it
> > skips installation of bin/git-receive-pack bin/git-upload-archive and
> > bin/git-upload-pack as well.
> >
> > There is a comment that says these commands are special and expected
> > to be in the bin/ directory in dashed form, so unless I'm missing
> > something, I believe this is unintended.
> >
> > This seems to be the offending hunk:
> > https://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993
> >
> > Reverting that hunk restores git-receive-pack, git-upload-archive, and
> > git-upload-pack (and only those).
>
> Thanks for a report. Dscho?
I'm not Dscho, but I wonder if something as simple as the following (totally untested) patch might work. The rationale is that we already build 'ALL_COMMANDS_TO_INSTALL' to only contain the programs and builtins (iff SKIP_DASHED_BUILT_INS is unset) we want, so we could install each entry in that list unconditionally instead of iterating through BUILT_INS and checking SKIP_DASHED_BUILT_INS each time.
If Dscho or someone else wants to ack that this is a patch in the right directions, I'd be happy to clean it up.
--- 8< ---
diff --git a/Makefile b/Makefile
index 95571ee3fc..1b2b085765 100644
--- a/Makefile
+++ b/Makefile
@@ -2991,17 +2991,14 @@ endif
cp "$$bindir/git$X" "$$bindir/$$p" || exit; }; \
fi \
done && \
- for p in $(BUILT_INS); do \
+ for p in $(ALL_COMMANDS_TO_INSTALL); do \
$(RM) "$$execdir/$$p" && \
- if test -z "$(SKIP_DASHED_BUILT_INS)"; \
- then \
- test -n "$(INSTALL_SYMLINKS)" && \
- ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \
- { test -z "$(NO_INSTALL_HARDLINKS)" && \
- ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \
- ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
- cp "$$execdir/git$X" "$$execdir/$$p" || exit; }; \
- fi \
+ test -n "$(INSTALL_SYMLINKS)" && \
+ ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \
+ { test -z "$(NO_INSTALL_HARDLINKS)" && \
+ ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \
+ ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
+ cp "$$execdir/git$X" "$$execdir/$$p" || exit; }; \
done && \
remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
for p in $$remote_curl_aliases; do \
Michael Forney
