Re: SKIP_DASHED_BUILT_INS does not install git-*-pack
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 20, 2020, 02:52 UTC
- Message-ID
- <20201020025247.GF54484@nand.local>
- In-Reply-To
- <xmqqtuupd5m2.fsf@gitster.c.googlers.com>
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 \