{"thread":{"id":"50336","subject":"[PATCH v2] git-submodule.sh: shorten submodule SHA-1s using rev-parse","startedAt":"2019-01-27T19:31:07Z","lastAt":"2019-01-28T02:34:34Z","messageCount":3,"participants":["Sven van Haastregt","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"367761","messageId":"20190127192740.8678-1-svenvh@gmail.com","threadId":"50336","inReplyTo":null,"subject":"[PATCH v2] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Sven van Haastregt","fromEmail":"svenvh@gmail.com","sentAt":"2019-01-27T19:27:40Z","receivedAt":"2019-01-27T19:31:07Z","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\nWe cannot rely on always successfully invoking `git rev-parse` in the\nsubmodule directory.  Keep the old method using `cut` as a fallback.\n\nSigned-off-by: Sven van Haastregt <svenvh@gmail.com>\n---\n\nDifferences since v1: Keep the old method using `cut` as a fallback.\n\n git-submodule.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 5e608f8bad..5bd724e63b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -850,8 +850,16 @@ cmd_summary() {\n \t\t\t;;\n \t\tesac\n \n+\t\t# Ensure we have crudely abbreviated SHA-1s as fallback in case\n+\t\t# rev-parse fails to shorten the SHA-1s.\n \t\tsha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n \t\tsha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n+\n+\t\tsha1_abbrev=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src) &&\n+\t\tsha1_abbr_src=$sha1_abbrev\n+\t\tsha1_abbrev=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst) &&\n+\t\tsha1_abbr_dst=$sha1_abbrev\n+\n \t\tif test $status = T\n \t\tthen\n \t\t\tblob=\"$(gettext \"blob\")\"\n-- \n2.20.1.dirty\n\n"},{"id":"367805","messageId":"CAPig+cRLLTYB=tVOY39dojLYZqoX=yxPRZY9fjo9z+5+P2BKKg@mail.gmail.com","threadId":"50336","inReplyTo":"20190127192740.8678-1-svenvh@gmail.com","subject":"Re: [PATCH v2] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-01-27T23:46:10Z","receivedAt":"2019-01-27T23:46:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 27, 2019 at 2:28 PM Sven van Haastregt <svenvh@gmail.com> wrote:\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>\n> We cannot rely on always successfully invoking `git rev-parse` in the\n> submodule directory.  Keep the old method using `cut` as a fallback.\n>\n> Signed-off-by: Sven van Haastregt <svenvh@gmail.com>\n> ---\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> @@ -850,8 +850,16 @@ cmd_summary() {\n> +               # Ensure we have crudely abbreviated SHA-1s as fallback in case\n> +               # rev-parse fails to shorten the SHA-1s.\n>                 sha1_abbr_src=$(echo $sha1_src | cut -c1-7)\n>                 sha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)\n> +\n> +               sha1_abbrev=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src) &&\n> +               sha1_abbr_src=$sha1_abbrev\n> +               sha1_abbrev=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst) &&\n> +               sha1_abbr_dst=$sha1_abbrev\n\nThis could be made a bit easier to follow by using indentation and ||\nrather than &&. For instance, rewriting the entire block as:\n\n    # Shorten with hard-coded fallback if rev-parse fails\n    sha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src ||\n        echo $sha1_src | cut -c1-7)\n    sha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst ||\n        echo $sha1_dst | cut -c1-7)\n\nIn fact, the code is clear enough that the comment isn't even needed.\n\nBy the way, if git-rev-parse does fail, is it going to produce an\nerror message on stderr that needs to be suppressed?\n"},{"id":"367812","messageId":"xmqqy375bjen.fsf@gitster-ct.c.googlers.com","threadId":"50336","inReplyTo":"CAPig+cRLLTYB=tVOY39dojLYZqoX=yxPRZY9fjo9z+5+P2BKKg@mail.gmail.com","subject":"Re: [PATCH v2] git-submodule.sh: shorten submodule SHA-1s using rev-parse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-28T02:33:36Z","receivedAt":"2019-01-28T02:34:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> This could be made a bit easier to follow by using indentation and ||\n> rather than &&. For instance, rewriting the entire block as:\n>\n>     # Shorten with hard-coded fallback if rev-parse fails\n>     sha1_abbr_src=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_src ||\n>         echo $sha1_src | cut -c1-7)\n>     sha1_abbr_dst=$(GIT_DIR=\"$name/.git\" git rev-parse --short $sha1_dst ||\n>         echo $sha1_dst | cut -c1-7)\n>\n> In fact, the code is clear enough that the comment isn't even needed.\n>\n> By the way, if git-rev-parse does fail, is it going to produce an\n> error message on stderr that needs to be suppressed?\n\nAll good points ;-).\n"}]}