threads / patch / 5761

patch, 5 partsfetch & co: misc output cleanup

Subject: [PATCH 0/5] fetch & co: misc output cleanup

## tl;dr

12 messages between Sep 29, 2006 and Sep 30, 2006. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Santi Béjar· Sep 29, 2006, 18:05 UTC · lore
Hi *,
this patchset includes:
      fetch: Reset remote refs list each time fetch_main is called
      fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"
      fetch & co: Use short sha1 in the output
      fetch: Add output for the not fast forward case
      fetch: Clean output

The first one is actually a bugfix (at least for me), but needed for the last one.

 git-fetch.sh   |   19 ++++++++++++++-----
 git-merge.sh   |    2 +-
 git-resolve.sh |    2 +-
 3 files changed, 16 insertions(+), 7 deletions(-)
Santi Béjar· Sep 29, 2006, 18:05 UTC · re: Santi Béjar · lore

[PATCH 1/5] fetch: Reset remote refs list each time fetch_main is called

This prevents the fetch of the heads again in the second call of fetch_main.
Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 git-fetch.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to git-fetch.sh +1 −0
diff --git a/git-fetch.sh b/git-fetch.sh
index bcc67ab..f1522bd 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -257,6 +257,7 @@ fi
 fetch_main () {
   reflist="$1"
   refs=
+  rref=
 
   for ref in $reflist
   do

-- 
1.4.2.1.g38049
Santi Béjar· Sep 29, 2006, 18:06 UTC · re: Santi Béjar · lore

[PATCH 2/5] fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"

I find it shorter, easier to copy&paste and cleaner.
Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 git-fetch.sh   |    2 +-
 git-merge.sh   |    2 +-
 git-resolve.sh |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Show changes to 3 files +3 −3

git-fetch.sh, git-merge.sh, git-resolve.sh

diff --git a/git-fetch.sh b/git-fetch.sh
index f1522bd..08d86cd 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -182,7 +182,7 @@ fast_forward_local () {
 		;;
 	    *,$local)
 		echo >&2 "* $1: fast forward to $3"
-		echo >&2 "  from $local to $2"
+		echo >&2 "  $local..$2"
 		git-update-ref -m "$rloga: fast-forward" "$1" "$2" "$local"
 		;;
 	    *)
diff --git a/git-merge.sh b/git-merge.sh
index 5b34b4d..fd587c5 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -197,7 +197,7 @@ f,*)
 	;;
 ?,1,"$head",*)
 	# Again the most common case of merging one remote.
-	echo "Updating from $head to $1"
+	echo "Updating $head..$1"
 	git-update-index --refresh 2>/dev/null
 	new_head=$(git-rev-parse --verify "$1^0") &&
 	git-read-tree -u -v -m $head "$new_head" &&
diff --git a/git-resolve.sh b/git-resolve.sh
index 729ec65..6e4fb02 100755
--- a/git-resolve.sh
+++ b/git-resolve.sh
@@ -46,7 +46,7 @@ case "$common" in
 	exit 0
 	;;
 "$head")
-	echo "Updating from $head to $merge"
+	echo "Updating $head..$merge"
 	git-read-tree -u -m $head $merge || exit 1
 	git-update-ref -m "resolve $merge_name: Fast forward" \
 		HEAD "$merge" "$head"

-- 
1.4.2.1.g38049
Santi Béjar· Sep 29, 2006, 18:06 UTC · re: Santi Béjar · lore

[PATCH 3/5] fetch & co: Use short sha1 in the output

... for brevity.
Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 git-fetch.sh   |    6 +++---
 git-merge.sh   |    2 +-
 git-resolve.sh |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
Show changes to 3 files +5 −5

git-fetch.sh, git-merge.sh, git-resolve.sh

diff --git a/git-fetch.sh b/git-fetch.sh
index 08d86cd..32553f8 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -129,11 +129,11 @@ append_fetch_head () {
     then
 	headc_=$(git-rev-parse --verify "$head_^0") || exit
 	echo "$headc_	$not_for_merge_	$note_" >>"$GIT_DIR/FETCH_HEAD"
-	[ "$verbose" ] && echo >&2 "* committish: $head_"
+	[ "$verbose" ] && echo >&2 "* committish: $(git-rev-parse --short $head_)"
 	[ "$verbose" ] && echo >&2 "  $note_"
     else
 	echo "$head_	not-for-merge	$note_" >>"$GIT_DIR/FETCH_HEAD"
-	[ "$verbose" ] && echo >&2 "* non-commit: $head_"
+	[ "$verbose" ] && echo >&2 "* non-commit: $(git-rev-parse --short $head_)"
 	[ "$verbose" ] && echo >&2 "  $note_"
     fi
     if test "$local_name_" != ""
@@ -182,7 +182,7 @@ fast_forward_local () {
 		;;
 	    *,$local)
 		echo >&2 "* $1: fast forward to $3"
-		echo >&2 "  $local..$2"
+		echo >&2 "  $(git-rev-parse --short $local)..$(git-rev-parse --short $2)"
 		git-update-ref -m "$rloga: fast-forward" "$1" "$2" "$local"
 		;;
 	    *)
diff --git a/git-merge.sh b/git-merge.sh
index fd587c5..49c46d5 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -197,7 +197,7 @@ f,*)
 	;;
 ?,1,"$head",*)
 	# Again the most common case of merging one remote.
-	echo "Updating $head..$1"
+	echo "Updating $(git-rev-parse --short $head)..$(git-rev-parse --short $1)"
 	git-update-index --refresh 2>/dev/null
 	new_head=$(git-rev-parse --verify "$1^0") &&
 	git-read-tree -u -v -m $head "$new_head" &&
diff --git a/git-resolve.sh b/git-resolve.sh
index 6e4fb02..36b90e3 100755
--- a/git-resolve.sh
+++ b/git-resolve.sh
@@ -46,7 +46,7 @@ case "$common" in
 	exit 0
 	;;
 "$head")
-	echo "Updating $head..$merge"
+	echo "Updating $(git-rev-parse --short $head)..$(git-rev-parse --short $merge)"
 	git-read-tree -u -m $head $merge || exit 1
 	git-update-ref -m "resolve $merge_name: Fast forward" \
 		HEAD "$merge" "$head"

-- 
1.4.2.1.g38049
Santi Béjar· Sep 29, 2006, 18:07 UTC · re: Santi Béjar · lore

[PATCH 4/5] fetch: Add output for the not fast forward case

Use the ... notation to be able to use this directly in "git log" to see how the two non-fast-forward heads have diverged.

Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 git-fetch.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to git-fetch.sh +1 −0
diff --git a/git-fetch.sh b/git-fetch.sh
index 32553f8..ee4f5bd 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -190,6 +190,7 @@ fast_forward_local () {
 		;;
 	    esac || {
 		echo >&2 "* $1: does not fast forward to $3;"
+		echo >&2 "  $(git-rev-parse --short $local)...$(git-rev-parse --short $2)"
 		case ",$force,$single_force," in
 		*,t,*)
 			echo >&2 "  forcing update."

-- 
1.4.2.1.g38049
Santi Béjar· Sep 29, 2006, 18:08 UTC · re: Santi Béjar · lore

[PATCH 5/5] fetch: Clean output

Do not show duplicated remote branch information and reformat the output as:
$ git fetch -v   # the committish lines for the -v.
* refs/heads/origin: fast forward to remote branch 'master' of ../git/
  1ad7a06..bc1a580
  committish: bc1a580
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
  7c733a8...5faa935
  not updating.
  forcing update.
  committish: 5faa935
* refs/heads/next: same as remote branch 'origin/next' of ../git/
  committish: ce47b9f
...
* refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/
  committish: 8c7a107
$ git fetch -v origin refs/heads/master
* committish: 695dffe
  branch 'master' of ../git/
Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 git-fetch.sh |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)
Show changes to git-fetch.sh +11 −4
diff --git a/git-fetch.sh b/git-fetch.sh
index ee4f5bd..cc71612 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -129,19 +129,26 @@ append_fetch_head () {
     then
 	headc_=$(git-rev-parse --verify "$head_^0") || exit
 	echo "$headc_	$not_for_merge_	$note_" >>"$GIT_DIR/FETCH_HEAD"
-	[ "$verbose" ] && echo >&2 "* committish: $(git-rev-parse --short $head_)"
-	[ "$verbose" ] && echo >&2 "  $note_"
+	committish=committish
     else
 	echo "$head_	not-for-merge	$note_" >>"$GIT_DIR/FETCH_HEAD"
-	[ "$verbose" ] && echo >&2 "* non-commit: $(git-rev-parse --short $head_)"
-	[ "$verbose" ] && echo >&2 "  $note_"
+	committish=non-commit
     fi
+    star_prefix="*"
     if test "$local_name_" != ""
     then
 	# We are storing the head locally.  Make sure that it is
 	# a fast forward (aka "reverse push").
 	fast_forward_local "$local_name_" "$head_" "$note_"
+	exit=$?
+	star_prefix=" "
     fi
+    if test "$verbose"
+    then
+	echo >&2 "$star_prefix $committish: $(git-rev-parse --short $head_)"
+	[ -z "$local_name_" ] && echo >&2 "  $note_"
+    fi
+    return ${exit:-0}
 }
 
 fast_forward_local () {

-- 
1.4.2.1.g38049
Junio C Hamano· Sep 30, 2006, 02:17 UTC · re: Santi Béjar · lore

Re: [PATCH 5/5] fetch: Clean output

Santi Béjar <sbejar@gmail.com> writes:
Show 6 quoted lines
> Do not show duplicated remote branch information and reformat the output as:
>
> $ git fetch -v   # the committish lines for the -v.
> * refs/heads/origin: fast forward to remote branch 'master' of ../git/
>   1ad7a06..bc1a580
>   committish: bc1a580

I am not quite sure about this --- it is not obvious what these two numbers represent anymore. Also I think the last line outlived its usefulness (99% of the time refs are committish, so noting exception is good but otherwise it is not interesting).

I know you opted for minimum patch, but it might be a good time to polish the wording a bit while we are touching the general vicinity of the code.

How about saying something like:
 * refs/heads/origin: fast forward to remote branch 'master' of ../git/
   old..new = 1ad7a06..bc1a580
Show 5 quoted lines
> * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
>   7c733a8...5faa935
>   not updating.
>   forcing update.
>   committish: 5faa935

This is even more confusing. Perhaps we would want to have two cases, depending on --force (and +).

 * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
   but forcing update anyway.  old...new = 7c733a8...5faa935
 * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
   not updating.  old...new = 7c733a8...5faa935
Santi· Sep 30, 2006, 09:42 UTC · re: Junio C Hamano · lore

Re: [PATCH 5/5] fetch: Clean output

2006/9/30, Junio C Hamano <junkio@cox.net>:
Show 13 quoted lines
> Santi Béjar <sbejar@gmail.com> writes:
>
> > Do not show duplicated remote branch information and reformat the output as:
> >
> > $ git fetch -v   # the committish lines for the -v.
> > * refs/heads/origin: fast forward to remote branch 'master' of ../git/
> >   1ad7a06..bc1a580
> >   committish: bc1a580
>
> I am not quite sure about this --- it is not obvious what these
> two numbers represent anymore.  Also I think the last line
> outlived its usefulness (99% of the time refs are committish, so
> noting exception is good but otherwise it is not interesting).

I agree in the case of updating a ref, but I would keep it otherwise (and even in this case only when -v is given).

Show 9 quoted lines
>
> I know you opted for minimum patch, but it might be a good time
> to polish the wording a bit while we are touching the general
> vicinity of the code.
>
> How about saying something like:
>
>  * refs/heads/origin: fast forward to remote branch 'master' of ../git/
>    old..new = 1ad7a06..bc1a580
I like it.
Show 9 quoted lines
>
> > * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
> >   7c733a8...5faa935
> >   not updating.
> >   forcing update.
> >   committish: 5faa935
>
> This is even more confusing.  Perhaps we would want to have two
> cases, depending on --force (and +).

Sorry, it was from two copy'n'paste. I did it in a new repository (without the +), then it stoped at this point (I suppose to force the user to solve the problem). Then I put the + and did the copy'n'paste again without deleting this extra line.

>
>  * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/;
>    but forcing update anyway.  old...new = 7c733a8...5faa935
With the old...new in a new line?
So at the end, something like this output?
$ git fetch -v   # the committish lines for the -v.
* refs/heads/origin: fast forward to remote branch 'master' of ../git/
 old..new: 1ad7a06..bc1a580
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/
 old...new: 7c733a8...5faa935
 forcing update.
* refs/heads/next: same as remote branch 'origin/next' of ../git/
 committish: ce47b9f
...
* refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/
 committish: 8c7a107
$ git fetch -v origin refs/heads/master
* committish: 695dffe
 branch 'master' of ../git/
Santi
Junio C Hamano· Sep 30, 2006, 10:37 UTC · re: Santi · lore

Re: [PATCH 5/5] fetch: Clean output

Santi <sbejar@gmail.com> writes:
Show 17 quoted lines
> So at the end, something like this output?
>
> $ git fetch -v   # the committish lines for the -v.
> * refs/heads/origin: fast forward to remote branch 'master' of ../git/
> old..new: 1ad7a06..bc1a580
> * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/
> old...new: 7c733a8...5faa935
> forcing update.
> * refs/heads/next: same as remote branch 'origin/next' of ../git/
> committish: ce47b9f
> ...
> * refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/
> committish: 8c7a107
>
> $ git fetch -v origin refs/heads/master
> * committish: 695dffe
> branch 'master' of ../git/
For the last one, the second and third line feel backwards.

Most cases in your summary (i.e. storing on a tracking branch or tag) in general look like:

        * <local ref name>: <what happened and where it came from>
	<label>: <object names>

where <label> (e.g. old..new) describes what <object names> mean. So to be consistent, the <label> for "same as" case (i.e. storing what is fetched in a local ref, but it was already the same) and "storing anew" case, we might want to say their true object types, not "committish".

Also we probably would want to "s/remote branch/branch" in the "where from" part, since we always say "remote" in fetch anyway.

Then the last case where we do not use local tracking branch would simply be something like:

	* fetched branch 'master' of ../git
        commit: 695dffe

How about this? Essentially the same as the above but with minor tweaks.

1. the ones we store in our local ref (either branches or tags):
 1a) fast-forward
  * refs/heads/origin: fast forward to branch 'master' of ../git/
    old..new: 1ad7a06..bc1a580
 1b) same (only shown under -v)
  * refs/heads/next: same as branch 'origin/next' of ../git/
    commit: ce47b9f
 1c) non-fast-forward, forced
  * refs/heads/pu: forcing update to branch 'pu' of ../git/
    old...new: 7c733a8...5faa935
 1d) non-fast-forward, did not update because not forced
  * refs/heads/po: does not fast-forward to branch 'po' of ../git/
    old...new: 7c733a8...5faa935
 1e) creating a new local ref to store
  * refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/
    tag: 8c7a107
2. the ones we do not store in our local ref (only shown under -v):
  * fetched branch 'master' of ../git
    commit: 695dffe

I think we should reword the "what happened" for 1d to clarify that we did not touch refs/heads/po, but other than that I am reasonably happy about how they look consistent.

But please don't take the above too seriously. As everybody should be very well aware by now, I am very bad at UI designs.

A Large Angry SCM· Sep 29, 2006, 18:54 UTC · re: Santi Béjar · lore

Re: [PATCH 0/5] fetch & co: misc output cleanup

Santi Béjar wrote:
Show 7 quoted lines
> this patchset includes:
> 
>       fetch: Reset remote refs list each time fetch_main is called
>       fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"
>       fetch & co: Use short sha1 in the output
>       fetch: Add output for the not fast forward case
>       fetch: Clean output

Please do not make short ID output the default. That is, unless you can somehow guarantee that the short ID reported today will _always_ be unambiguous even with future additions to the repository.

Santi· Sep 29, 2006, 20:48 UTC · re: A Large Angry SCM · lore

Re: [PATCH 0/5] fetch & co: misc output cleanup

2006/9/29, A Large Angry SCM <gitzilla@gmail.com>:
Show 13 quoted lines
> Santi Béjar wrote:
> > this patchset includes:
> >
> >       fetch: Reset remote refs list each time fetch_main is called
> >       fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"
> >       fetch & co: Use short sha1 in the output
> >       fetch: Add output for the not fast forward case
> >       fetch: Clean output
>
> Please do not make short ID output the default. That is, unless you can
> somehow guarantee that the short ID reported today will _always_ be
> unambiguous even with future additions to the repository.
>

There is not such guarantee. But the fetch output is to be use immediately in git-log, etc. If you want these "guarantees" you must know that you have to keep the long version.

By the way, is there a way to show all the commits,tags,... that abbreviate to a short hash? This way instead of just saying "bad revision" we could show the list of possible revisions.

Also, git-log & co also output abbreviated hashes by default. So why not for git-fetch?

Santi
Junio C Hamano· Sep 29, 2006, 21:27 UTC · re: A Large Angry SCM · lore

Re: [PATCH 0/5] fetch & co: misc output cleanup

A Large Angry SCM <gitzilla@gmail.com> writes:
Show 12 quoted lines
> Santi Béjar wrote:
>> this patchset includes:
>>
>>       fetch: Reset remote refs list each time fetch_main is called
>>       fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"
>>       fetch & co: Use short sha1 in the output
>>       fetch: Add output for the not fast forward case
>>       fetch: Clean output
>
> Please do not make short ID output the default. That is, unless you
> can somehow guarantee that the short ID reported today will _always_
> be unambiguous even with future additions to the repository.

I would understand your worries of spitting out only abbreviated object names if they appear in a document to be preserved for a long term. But we are talking about output to stderr to help people see what happened in the operation they initiated a few moments ago, and to help them use the output for cut&paste while it is still in the scrollbuffer of the terminal.

Shortening them would help A..B avoid crossing the line boundaries, which terminal emulators have trouble with cutting sometimes. That benefit may outweigh the longer term ambiguity concerns.

The only real-life situation I can imagine the program output is preserved in any longer term than immediate consumption for cut&paste is to paste it into a piece of e-mail (which is archived), to show exactly what the program said to the user. But then it carries the timestamp and you should be able to figure out what the abbreviation uniquely stood for back then if you really cared. If you are producing a document for longer term storage, the program that produces such a document should be doing its own rev-parse anyway (or maybe pass whatever it is piping through git-name-rev --stdin).

I think it is Ok to always use the abbreviated form in the places Santi's patch touched, as long as people understand these messages are not for long term storage. We might want to use a bit longer abbreviation than the default (7 hexdigits).

← back to recent threads