threads / patch / 50390

v3git-submodule.sh: shorten submodule SHA-1s using rev-parse

Subject: [PATCH v3] git-submodule.sh: shorten submodule SHA-1s using rev-parse

## tl;dr

4 messages between Feb 3, 2019 and Feb 4, 2019. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Sven van Haastregt· Feb 3, 2019, 21:00 UTC · lore

Until now, `git submodule summary` was always emitting 7-character SHA-1s that have a higher chance of being ambiguous for larger repositories. Use `git rev-parse --short` instead, which will determine suitable short SHA-1 lengths.

We cannot always rely on successfully invoking `git rev-parse` in the submodule directory. Keep the old method using `cut` as a fallback.

Signed-off-by: Sven van Haastregt <svenvh@gmail.com>
---

Differences since v2: Simplify code as suggested by Eric Sunshine <sunshine@sunshineco.com> and suppress stderr.

 git-submodule.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Show changes to git-submodule.sh +5 −2
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..e26146e721 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -850,8 +850,11 @@ cmd_summary() {
 			;;
 		esac
 
-		sha1_abbr_src=$(echo $sha1_src | cut -c1-7)
-		sha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)
+		sha1_abbr_src=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_src 2>/dev/null ||
+			echo $sha1_src | cut -c1-7)
+		sha1_abbr_dst=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_dst 2>/dev/null ||
+			echo $sha1_dst | cut -c1-7)
+
 		if test $status = T
 		then
 			blob="$(gettext "blob")"

-- 
2.20.1.dirty
Eric Sunshine· Feb 3, 2019, 21:28 UTC · re: Sven van Haastregt · lore

Re: [PATCH v3] git-submodule.sh: shorten submodule SHA-1s using rev-parse

On Sun, Feb 3, 2019 at 4:01 PM Sven van Haastregt <svenvh@gmail.com> wrote:
Show 7 quoted lines
> Until now, `git submodule summary` was always emitting 7-character
> SHA-1s that have a higher chance of being ambiguous for larger
> repositories.  Use `git rev-parse --short` instead, which will
> determine suitable short SHA-1 lengths.
>
> We cannot always rely on successfully invoking `git rev-parse` in the
> submodule directory.  Keep the old method using `cut` as a fallback.

Reviewers and future readers of this patch are left in the dark regarding the circumstances in which git-rev-parse may fail in the submodule directory. It would be helpful for the commit message to explain this potential failure in enough detail for someone working in this area in the future to understand any implications of changes to this code.

> Signed-off-by: Sven van Haastregt <svenvh@gmail.com>
> ---
> Differences since v2: Simplify code as suggested by Eric
> Sunshine <sunshine@sunshineco.com> and suppress stderr.
This version looks better. Thanks.
Junio C Hamano· Feb 4, 2019, 18:47 UTC · re: Eric Sunshine · lore

Re: [PATCH v3] git-submodule.sh: shorten submodule SHA-1s using rev-parse

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 15 quoted lines
> On Sun, Feb 3, 2019 at 4:01 PM Sven van Haastregt <svenvh@gmail.com> wrote:
>> Until now, `git submodule summary` was always emitting 7-character
>> SHA-1s that have a higher chance of being ambiguous for larger
>> repositories.  Use `git rev-parse --short` instead, which will
>> determine suitable short SHA-1 lengths.
>>
>> We cannot always rely on successfully invoking `git rev-parse` in the
>> submodule directory.  Keep the old method using `cut` as a fallback.
>
> Reviewers and future readers of this patch are left in the dark
> regarding the circumstances in which git-rev-parse may fail in the
> submodule directory. It would be helpful for the commit message to
> explain this potential failure in enough detail for someone working in
> this area in the future to understand any implications of changes to
> this code.
Replace that problematic sentence with something like
	As a submodule may not be initialized with "submodule init"
	or not cloned, `git rev-parse` may not work in it yet; as a
	fallback, use the original method of cutting at 7 hexdigits.
perhaps?
Show 7 quoted lines
>
>> Signed-off-by: Sven van Haastregt <svenvh@gmail.com>
>> ---
>> Differences since v2: Simplify code as suggested by Eric
>> Sunshine <sunshine@sunshineco.com> and suppress stderr.
>
> This version looks better. Thanks.
Eric Sunshine· Feb 4, 2019, 19:01 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] git-submodule.sh: shorten submodule SHA-1s using rev-parse

On Mon, Feb 4, 2019 at 1:47 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Sun, Feb 3, 2019 at 4:01 PM Sven van Haastregt <svenvh@gmail.com> wrote:
> >> We cannot always rely on successfully invoking `git rev-parse` in the
> >> submodule directory.  Keep the old method using `cut` as a fallback.
> >
> > Reviewers and future readers of this patch are left in the dark
> > regarding the circumstances in which git-rev-parse may fail in the
> > submodule directory. It would be helpful for the commit message to
> > explain this potential failure in enough detail for someone working in
> > this area in the future to understand any implications of changes to
> > this code.
>
> Replace that problematic sentence with something like
>
>         As a submodule may not be initialized with "submodule init"
>         or not cloned, `git rev-parse` may not work in it yet; as a
>         fallback, use the original method of cutting at 7 hexdigits.
>
> perhaps?
That's better.

← back to recent threads