* [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
@ 2019-01-20 20:46 Sven van Haastregt
2019-01-22 15:24 ` Johannes Schindelin
2019-01-22 20:22 ` Junio C Hamano
0 siblings, 2 replies; 7+ messages in thread
From: Sven van Haastregt @ 2019-01-20 20:46 UTC (permalink / raw)
To: git; +Cc: Sven van Haastregt
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.
Signed-off-by: Sven van Haastregt <svenvh@gmail.com>
---
git-submodule.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..a422b0728d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -850,8 +850,8 @@ 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)
+ sha1_abbr_dst=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_dst)
if test $status = T
then
blob="$(gettext "blob")"
--
2.20.1.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-20 20:46 [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse Sven van Haastregt
@ 2019-01-22 15:24 ` Johannes Schindelin
2019-01-22 20:23 ` Junio C Hamano
2019-01-22 20:22 ` Junio C Hamano
1 sibling, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2019-01-22 15:24 UTC (permalink / raw)
To: Sven van Haastregt; +Cc: git
Hi Sven,
On Sun, 20 Jan 2019, Sven van Haastregt 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.
Good point. Just one suggestion:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5e608f8bad..a422b0728d 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -850,8 +850,8 @@ 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)
How about `git -C "$name" rev-parse --short`? That would less likely run
over 80 columns/line, either.
Ciao,
Johannes
> + sha1_abbr_dst=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_dst)
> if test $status = T
> then
> blob="$(gettext "blob")"
> --
> 2.20.1.dirty
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-20 20:46 [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse Sven van Haastregt
2019-01-22 15:24 ` Johannes Schindelin
@ 2019-01-22 20:22 ` Junio C Hamano
2019-01-22 21:38 ` Sven van Haastregt
1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2019-01-22 20:22 UTC (permalink / raw)
To: Sven van Haastregt; +Cc: git
Sven van Haastregt <svenvh@gmail.com> writes:
> 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.
In general I think it is a good idea to scale as the repository
grows by asking "rev-parse --short" to do the job.
One thing it is not clear to me is that this codepath is prepared to
handle sha1_src and sha1_dst referring to an object that does not
exist (i.e. $missing_(src|dst)=t); the original code will still give
us 7 hexdigit to show on the headline, but does the updated code
that uses "rev-parse --short" give us a reasonable output?
>
> Signed-off-by: Sven van Haastregt <svenvh@gmail.com>
> ---
> git-submodule.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5e608f8bad..a422b0728d 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -850,8 +850,8 @@ 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)
> + sha1_abbr_dst=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_dst)
> if test $status = T
> then
> blob="$(gettext "blob")"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-22 15:24 ` Johannes Schindelin
@ 2019-01-22 20:23 ` Junio C Hamano
2019-01-24 21:49 ` Jeff King
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2019-01-22 20:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Sven van Haastregt, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> + sha1_abbr_src=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_src)
>
> How about `git -C "$name" rev-parse --short`? That would less likely run
> over 80 columns/line, either.
That would be a separate patch, either as a preliminary or a
follow-up. The existing code has too many of the same construct.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-22 20:22 ` Junio C Hamano
@ 2019-01-22 21:38 ` Sven van Haastregt
0 siblings, 0 replies; 7+ messages in thread
From: Sven van Haastregt @ 2019-01-22 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On 22/01/19 20:22, Junio C Hamano wrote:
> One thing it is not clear to me is that this codepath is prepared to
> handle sha1_src and sha1_dst referring to an object that does not
> exist (i.e. $missing_(src|dst)=t); the original code will still give
> us 7 hexdigit to show on the headline, but does the updated code
> that uses "rev-parse --short" give us a reasonable output?
Good point, I expect it will output an error message in that case. I
suppose we can fallback to the old echo and cut if the object does not
exist; I'll update the patch.
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 5e608f8bad..a422b0728d 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -850,8 +850,8 @@ 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)
>> + sha1_abbr_dst=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_dst)
>> if test $status = T
>> then
>> blob="$(gettext "blob")"
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-22 20:23 ` Junio C Hamano
@ 2019-01-24 21:49 ` Jeff King
2019-01-25 13:05 ` Johannes Schindelin
0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2019-01-24 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven van Haastregt, git
On Tue, Jan 22, 2019 at 12:23:55PM -0800, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> + sha1_abbr_src=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_src)
> >
> > How about `git -C "$name" rev-parse --short`? That would less likely run
> > over 80 columns/line, either.
>
> That would be a separate patch, either as a preliminary or a
> follow-up. The existing code has too many of the same construct.
It's also not the same; if $GIT_DIR is already set in the environment,
that will override auto-discovery done after the chdir() triggered by
"-C". It would need to be "git --git-dir".
Sincerely,
Somebody who has been bit by the distinction many times
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse
2019-01-24 21:49 ` Jeff King
@ 2019-01-25 13:05 ` Johannes Schindelin
0 siblings, 0 replies; 7+ messages in thread
From: Johannes Schindelin @ 2019-01-25 13:05 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Sven van Haastregt, git
Hi Peff,
On Thu, 24 Jan 2019, Jeff King wrote:
> On Tue, Jan 22, 2019 at 12:23:55PM -0800, Junio C Hamano wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > >> + sha1_abbr_src=$(GIT_DIR="$name/.git" git rev-parse --short $sha1_src)
> > >
> > > How about `git -C "$name" rev-parse --short`? That would less likely run
> > > over 80 columns/line, either.
> >
> > That would be a separate patch, either as a preliminary or a
> > follow-up. The existing code has too many of the same construct.
>
> It's also not the same; if $GIT_DIR is already set in the environment,
> that will override auto-discovery done after the chdir() triggered by
> "-C". It would need to be "git --git-dir".
>
> Sincerely,
> Somebody who has been bit by the distinction many times
Ouch. Good point.
Thanks,
Dscho
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-25 13:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-20 20:46 [PATCH] git-submodule.sh: shorten submodule SHA-1s using rev-parse Sven van Haastregt
2019-01-22 15:24 ` Johannes Schindelin
2019-01-22 20:23 ` Junio C Hamano
2019-01-24 21:49 ` Jeff King
2019-01-25 13:05 ` Johannes Schindelin
2019-01-22 20:22 ` Junio C Hamano
2019-01-22 21:38 ` Sven van Haastregt
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).