Re: SKIP_DASHED_BUILT_INS does not install git-*-pack

[Taylor Blau <me@ttaylorr.com>]

On Mon, Oct 19, 2020 at 06:54:29PM -0700, Junio C Hamano wrote:
> 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 \