{"thread":{"id":"54466","subject":"SKIP_DASHED_BUILT_INS does not install git-*-pack","startedAt":"2020-10-20T01:27:17Z","lastAt":"2020-10-21T18:34:48Z","messageCount":7,"participants":["Michael Forney","Junio C Hamano","Taylor Blau","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"407943","messageId":"CAGw6cBsEjOnh-ZqXCPfFha=NYEdy7JDddha=UzAau0Z1tBrWKg@mail.gmail.com","threadId":"54466","inReplyTo":null,"subject":"SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Michael Forney","fromEmail":"mforney@mforney.org","sentAt":"2020-10-20T01:27:13Z","receivedAt":"2020-10-20T01:27:17Z","isPatch":false,"sender":{"key":"mforney@mforney.org","avatar":"https://avatars.githubusercontent.com/u/52851?v=4"},"body":"I saw that git 2.29.0 introduced a new make variable\nSKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it\nskips installation of bin/git-receive-pack bin/git-upload-archive and\nbin/git-upload-pack as well.\n\nThere is a comment that says these commands are special and expected\nto be in the bin/ directory in dashed form, so unless I'm missing\nsomething, I believe this is unintended.\n\nThis seems to be the offending hunk:\nhttps://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993\n\nReverting that hunk restores git-receive-pack, git-upload-archive, and\ngit-upload-pack (and only those).\n"},{"id":"407945","messageId":"xmqqtuupd5m2.fsf@gitster.c.googlers.com","threadId":"54466","inReplyTo":"CAGw6cBsEjOnh-ZqXCPfFha=NYEdy7JDddha=UzAau0Z1tBrWKg@mail.gmail.com","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-20T01:54:29Z","receivedAt":"2020-10-20T01:54:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Forney writes:\n\n> I saw that git 2.29.0 introduced a new make variable\n> SKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it\n> skips installation of bin/git-receive-pack bin/git-upload-archive and\n> bin/git-upload-pack as well.\n>\n> There is a comment that says these commands are special and expected\n> to be in the bin/ directory in dashed form, so unless I'm missing\n> something, I believe this is unintended.\n>\n> This seems to be the offending hunk:\n> https://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993\n>\n> Reverting that hunk restores git-receive-pack, git-upload-archive, and\n> git-upload-pack (and only those).\n\nThanks for a report. Dscho?\n"},{"id":"407952","messageId":"20201020025247.GF54484@nand.local","threadId":"54466","inReplyTo":"xmqqtuupd5m2.fsf@gitster.c.googlers.com","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-10-20T02:52:47Z","receivedAt":"2020-10-20T02:52:55Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 19, 2020 at 06:54:29PM -0700, Junio C Hamano wrote:\n> Michael Forney writes:\n>\n> > I saw that git 2.29.0 introduced a new make variable\n> > SKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it\n> > skips installation of bin/git-receive-pack bin/git-upload-archive and\n> > bin/git-upload-pack as well.\n> >\n> > There is a comment that says these commands are special and expected\n> > to be in the bin/ directory in dashed form, so unless I'm missing\n> > something, I believe this is unintended.\n> >\n> > This seems to be the offending hunk:\n> > https://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993\n> >\n> > Reverting that hunk restores git-receive-pack, git-upload-archive, and\n> > git-upload-pack (and only those).\n>\n> Thanks for a report. Dscho?\n\nI'm not Dscho, but I wonder if something as simple as the following\n(totally untested) patch might work. The rationale is that we already\nbuild 'ALL_COMMANDS_TO_INSTALL' to only contain the programs and\nbuiltins (iff SKIP_DASHED_BUILT_INS is unset) we want, so we could\ninstall each entry in that list unconditionally instead of iterating\nthrough BUILT_INS and checking SKIP_DASHED_BUILT_INS each time.\n\nIf Dscho or someone else wants to ack that this is a patch in the right\ndirections, I'd be happy to clean it up.\n\n--- 8< ---\n\ndiff --git a/Makefile b/Makefile\nindex 95571ee3fc..1b2b085765 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2991,17 +2991,14 @@ endif\n \t\t\t cp \"$$bindir/git$X\" \"$$bindir/$$p\" || exit; }; \\\n \t\tfi \\\n \tdone && \\\n-\tfor p in $(BUILT_INS); do \\\n+\tfor p in $(ALL_COMMANDS_TO_INSTALL); do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n-\t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n-\t\tthen \\\n-\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n-\t\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n-\t\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\t\t ln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t\t ln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t\t cp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n-\t\tfi \\\n+\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n+\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n+\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n+\t\t\tln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n+\t\t\tln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n+\t\t\tcp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n \tdone && \\\n \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n \tfor p in $$remote_curl_aliases; do \\\n"},{"id":"408060","messageId":"nycvar.QRO.7.76.6.2010211609520.56@tvgsbejvaqbjf.bet","threadId":"54466","inReplyTo":"xmqqtuupd5m2.fsf@gitster.c.googlers.com","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-21T14:10:52Z","receivedAt":"2020-10-21T14:11:02Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & Michael,\n\nOn Mon, 19 Oct 2020, Junio C Hamano wrote:\n\n> Michael Forney writes:\n>\n> > I saw that git 2.29.0 introduced a new make variable\n> > SKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it\n> > skips installation of bin/git-receive-pack bin/git-upload-archive and\n> > bin/git-upload-pack as well.\n> >\n> > There is a comment that says these commands are special and expected\n> > to be in the bin/ directory in dashed form, so unless I'm missing\n> > something, I believe this is unintended.\n> >\n> > This seems to be the offending hunk:\n> > https://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993\n> >\n> > Reverting that hunk restores git-receive-pack, git-upload-archive, and\n> > git-upload-pack (and only those).\n>\n> Thanks for a report. Dscho?\n\nHrm. We have this, specifically:\n\n```\nifeq (,$(SKIP_DASHED_BUILT_INS))\nALL_COMMANDS_TO_INSTALL += $(BUILT_INS)\nelse\n# git-upload-pack, git-receive-pack and git-upload-archive are special: they\n# are _expected_ to be present in the `bin/` directory in their dashed form.\nALL_COMMANDS_TO_INSTALL += git-receive-pack$(X)\nALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)\nALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)\nendif\n\n```\n\nWill have a look,\nDscho\n"},{"id":"408062","messageId":"nycvar.QRO.7.76.6.2010211614300.56@tvgsbejvaqbjf.bet","threadId":"54466","inReplyTo":"20201020025247.GF54484@nand.local","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-21T14:16:52Z","receivedAt":"2020-10-21T14:17:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Taylor,\n\nOn Mon, 19 Oct 2020, Taylor Blau wrote:\n\n> diff --git a/Makefile b/Makefile\n> index 95571ee3fc..1b2b085765 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2991,17 +2991,14 @@ endif\n> \t\t\t cp \"$$bindir/git$X\" \"$$bindir/$$p\" || exit; }; \\\n> \t\tfi \\\n> \tdone && \\\n> -\tfor p in $(BUILT_INS); do \\\n> +\tfor p in $(ALL_COMMANDS_TO_INSTALL); do \\\n> \t\t$(RM) \"$$execdir/$$p\" && \\\n> -\t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n> -\t\tthen \\\n> -\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n> -\t\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n> -\t\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n> -\t\t\t ln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> -\t\t\t ln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> -\t\t\t cp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n> -\t\tfi \\\n> +\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n> +\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n> +\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n> +\t\t\tln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> +\t\t\tln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> +\t\t\tcp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n\nThis code talks about `execdir`; I am fairly certain that we do not need\nanything specific in `libexec/git-core/`. However, for purposes of\nfetching/pushing, the mentioned executables should be present in the\n`bin/` directory in dashed form, no matter whether they are built-ins or\nnot.\n\nIn a not-so-distant future, we might even be able to teach Git to call for\nthe undashed form. But the problem there is that the Git doing the asking\nis on the client side, while the Git possibly missing the dashed forms is\non the remote side.\n\nCiao,\nDscho\n"},{"id":"408063","messageId":"nycvar.QRO.7.76.6.2010211626430.56@tvgsbejvaqbjf.bet","threadId":"54466","inReplyTo":"CAGw6cBsEjOnh-ZqXCPfFha=NYEdy7JDddha=UzAau0Z1tBrWKg@mail.gmail.com","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-21T14:27:35Z","receivedAt":"2020-10-21T14:27:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Mon, 19 Oct 2020, Michael Forney wrote:\n\n> I saw that git 2.29.0 introduced a new make variable\n> SKIP_DASHED_BUILT_INS. However, after testing it out I noticed that it\n> skips installation of bin/git-receive-pack bin/git-upload-archive and\n> bin/git-upload-pack as well.\n>\n> There is a comment that says these commands are special and expected\n> to be in the bin/ directory in dashed form, so unless I'm missing\n> something, I believe this is unintended.\n>\n> This seems to be the offending hunk:\n> https://github.com/git/git/commit/94de88c986712e79c20813ba54e797c4ca83137b#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L2979-R2993\n>\n> Reverting that hunk restores git-receive-pack, git-upload-archive, and\n> git-upload-pack (and only those).\n\nIndeed. I purposefully investigated without having a look at what you\nlinked at, and came to the exact same conclusion.\n\nAs soon as the build passed, I will send this to the list:\nhttps://github.com/gitgitgadget/git/pull/768\n\nThanks,\nDscho\n"},{"id":"408081","messageId":"xmqqd01b8m2q.fsf@gitster.c.googlers.com","threadId":"54466","inReplyTo":"nycvar.QRO.7.76.6.2010211626430.56@tvgsbejvaqbjf.bet","subject":"Re: SKIP_DASHED_BUILT_INS does not install git-*-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-21T18:34:37Z","receivedAt":"2020-10-21T18:34:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Reverting that hunk restores git-receive-pack, git-upload-archive, and\n>> git-upload-pack (and only those).\n>\n> Indeed. I purposefully investigated without having a look at what you\n> linked at, and came to the exact same conclusion.\n\nGood---that's sort-of \"independent verification\" ;-)\n\nThanks, let's fast-track the fix for 2.29.1.\n\n"}]}