{"thread":{"id":"50271","subject":"[PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","startedAt":"2019-01-21T08:01:55Z","lastAt":"2019-01-25T13:06:00Z","messageCount":7,"participants":["Sven van Haastregt","Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"367216","messageId":"20190120204653.3224-1-svenvh@gmail.com","threadId":"50271","inReplyTo":null,"subject":"[PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Sven van Haastregt","fromEmail":"svenvh@gmail.com","sentAt":"2019-01-20T20:46:53Z","receivedAt":"2019-01-21T08:01:55Z","isPatch":true,"sender":{"key":"svenvh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1358906?v=4"},"body":"Until now, `git submodule summary` was always emitting 7-character\nSHA-1s that have a higher chance of being ambiguous for larger\nrepositories.  Use `git rev-parse --short` instead, which will\ndetermine suitable short SHA-1 lengths.\n\nSigned-off-by: Sven van Haastregt <svenvh@gmail.com>\n---\n git-submodule.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 5e608f8bad..a422b0728d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -850,8 +850,8 @@ cmd_summary() {\n \t\t\t;;\n \t\tesac\n \n-\t\tsha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n-\t\tsha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n+\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n+\t\tsha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst)\n \t\tif test $status = T\n \t\tthen\n \t\t\tblob=\"$(gettext \"blob\")\"\n-- \n2.20.1.dirty\n\n"},{"id":"367318","messageId":"nycvar.QRO.7.76.6.1901221623370.41@tvgsbejvaqbjf.bet","threadId":"50271","inReplyTo":"20190120204653.3224-1-svenvh@gmail.com","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-01-22T15:24:26Z","receivedAt":"2019-01-22T15:24:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Sven,\n\nOn Sun, 20 Jan 2019, Sven van Haastregt wrote:\n\n> Until now, `git submodule summary` was always emitting 7-character\n> SHA-1s that have a higher chance of being ambiguous for larger\n> repositories.  Use `git rev-parse --short` instead, which will\n> determine suitable short SHA-1 lengths.\n\nGood point. Just one suggestion:\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 5e608f8bad..a422b0728d 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -850,8 +850,8 @@ cmd_summary() {\n>  \t\t\t;;\n>  \t\tesac\n>  \n> -\t\tsha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n> -\t\tsha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n\nHow about `git -C \"$name\" rev-parse --short`? That would less likely run\nover 80 columns/line, either.\n\nCiao,\nJohannes\n\n> +\t\tsha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst)\n>  \t\tif test $status = T\n>  \t\tthen\n>  \t\t\tblob=\"$(gettext \"blob\")\"\n> -- \n> 2.20.1.dirty\n> \n> \n"},{"id":"367351","messageId":"xmqqfttkfnmy.fsf@gitster-ct.c.googlers.com","threadId":"50271","inReplyTo":"20190120204653.3224-1-svenvh@gmail.com","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-22T20:22:45Z","receivedAt":"2019-01-22T20:22:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven van Haastregt <svenvh@gmail.com> writes:\n\n> Until now, `git submodule summary` was always emitting 7-character\n> SHA-1s that have a higher chance of being ambiguous for larger\n> repositories.  Use `git rev-parse --short` instead, which will\n> determine suitable short SHA-1 lengths.\n\nIn general I think it is a good idea to scale as the repository\ngrows by asking \"rev-parse --short\" to do the job.\n\nOne thing it is not clear to me is that this codepath is prepared to\nhandle sha1_src and sha1_dst referring to an object that does not\nexist (i.e. $missing_(src|dst)=t); the original code will still give\nus 7 hexdigit to show on the headline, but does the updated code\nthat uses \"rev-parse --short\" give us a reasonable output?\n\n>\n> Signed-off-by: Sven van Haastregt <svenvh@gmail.com>\n> ---\n>  git-submodule.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 5e608f8bad..a422b0728d 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -850,8 +850,8 @@ cmd_summary() {\n>  \t\t\t;;\n>  \t\tesac\n>  \n> -\t\tsha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n> -\t\tsha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n> +\t\tsha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst)\n>  \t\tif test $status = T\n>  \t\tthen\n>  \t\t\tblob=\"$(gettext \"blob\")\"\n"},{"id":"367352","messageId":"xmqqbm48fnl0.fsf@gitster-ct.c.googlers.com","threadId":"50271","inReplyTo":"nycvar.QRO.7.76.6.1901221623370.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-22T20:23:55Z","receivedAt":"2019-01-22T20:24:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n>\n> How about `git -C \"$name\" rev-parse --short`? That would less likely run\n> over 80 columns/line, either.\n\nThat would be a separate patch, either as a preliminary or a\nfollow-up.  The existing code has too many of the same construct.\n"},{"id":"367378","messageId":"5C478D44.2070405@gmail.com","threadId":"50271","inReplyTo":"xmqqfttkfnmy.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Sven van Haastregt","fromEmail":"svenvh@gmail.com","sentAt":"2019-01-22T21:38:12Z","receivedAt":"2019-01-22T21:38:26Z","isPatch":true,"sender":{"key":"svenvh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1358906?v=4"},"body":"On 22/01/19 20:22, Junio C Hamano wrote:\n> One thing it is not clear to me is that this codepath is prepared to\n> handle sha1_src and sha1_dst referring to an object that does not\n> exist (i.e. $missing_(src|dst)=t); the original code will still give\n> us 7 hexdigit to show on the headline, but does the updated code\n> that uses \"rev-parse --short\" give us a reasonable output?\n\nGood point, I expect it will output an error message in that case.  I \nsuppose we can fallback to the old echo and cut if the object does not \nexist; I'll update the patch.\n\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 5e608f8bad..a422b0728d 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -850,8 +850,8 @@ cmd_summary() {\n>>   \t\t\t;;\n>>   \t\tesac\n>>\n>> -\t\tsha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n>> -\t\tsha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n>> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n>> +\t\tsha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst)\n>>   \t\tif test $status = T\n>>   \t\tthen\n>>   \t\t\tblob=\"$(gettext \"blob\")\"\n>\n\n"},{"id":"367609","messageId":"20190124214953.GA19668@sigill.intra.peff.net","threadId":"50271","inReplyTo":"xmqqbm48fnl0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-01-24T21:49:53Z","receivedAt":"2019-01-24T21:49:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 22, 2019 at 12:23:55PM -0800, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n> >\n> > How about `git -C \"$name\" rev-parse --short`? That would less likely run\n> > over 80 columns/line, either.\n> \n> That would be a separate patch, either as a preliminary or a\n> follow-up.  The existing code has too many of the same construct.\n\nIt's also not the same; if $GIT_DIR is already set in the environment,\nthat will override auto-discovery done after the chdir() triggered by\n\"-C\". It would need to be \"git --git-dir\".\n\nSincerely,\nSomebody who has been bit by the distinction many times\n"},{"id":"367655","messageId":"nycvar.QRO.7.76.6.1901251405010.41@tvgsbejvaqbjf.bet","threadId":"50271","inReplyTo":"20190124214953.GA19668@sigill.intra.peff.net","subject":"Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-01-25T13:05:23Z","receivedAt":"2019-01-25T13:06:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 24 Jan 2019, Jeff King wrote:\n\n> On Tue, Jan 22, 2019 at 12:23:55PM -0800, Junio C Hamano wrote:\n> \n> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> > \n> > >> +\t\tsha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src)\n> > >\n> > > How about `git -C \"$name\" rev-parse --short`? That would less likely run\n> > > over 80 columns/line, either.\n> > \n> > That would be a separate patch, either as a preliminary or a\n> > follow-up.  The existing code has too many of the same construct.\n> \n> It's also not the same; if $GIT_DIR is already set in the environment,\n> that will override auto-discovery done after the chdir() triggered by\n> \"-C\". It would need to be \"git --git-dir\".\n> \n> Sincerely,\n> Somebody who has been bit by the distinction many times\n\nOuch. Good point.\n\nThanks,\nDscho\n"}]}