Skip to contentSkip to conversation

fetch & co: misc output cleanup

PATCH In review Santi Béjar 3 participants 12 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

cover fetch & co: misc output cleanup

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(-)
#1 lore raw
Quoted 7 lines · Santi Béjar

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.

#2 lore raw
Quoted 9 lines · A Large Angry SCM
Quoted 7 lines · Santi Béjar

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

Quoted 3 lines

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).

author
#3 lore raw
In reply to A Large Angry SCM

2006/9/29, A Large Angry SCM <[email protected]>:

Quoted 9 lines
Quoted 7 lines · Santi Béjar

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

Quoted 4 lines

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

#4 lore raw

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

In reply to Santi Béjar

This prevents the fetch of the heads again in the second call of fetch_main.

---
 git-fetch.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
git-fetch.sh +1 -0
index bcc67ab..f1522bd 100755
@@ -257,6 +257,7 @@ fi
 fetch_main () {
   reflist="$1"
   refs=
+  rref=
 
   for ref in $reflist
   do
Signature · 1 line
1.4.2.1.g38049
#5 lore raw

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

In reply to Santi Béjar

I find it shorter, easier to copy&paste and cleaner.

---
 git-fetch.sh   |    2 +-
 git-merge.sh   |    2 +-
 git-resolve.sh |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
git-fetch.sh +1 -1
index f1522bd..08d86cd 100755
@@ -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"
 		;;
 	    *)
git-merge.sh +1 -1
index 5b34b4d..fd587c5 100755
@@ -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" &&
git-resolve.sh +1 -1
index 729ec65..6e4fb02 100755
@@ -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"
Signature · 1 line
1.4.2.1.g38049
#6 lore raw

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

In reply to Santi Béjar

... for brevity.

---
 git-fetch.sh   |    6 +++---
 git-merge.sh   |    2 +-
 git-resolve.sh |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
git-fetch.sh +3 -3
index 08d86cd..32553f8 100755
@@ -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"
 		;;
 	    *)
git-merge.sh +1 -1
index fd587c5..49c46d5 100755
@@ -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" &&
git-resolve.sh +1 -1
index 6e4fb02..36b90e3 100755
@@ -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"
Signature · 1 line
1.4.2.1.g38049
#7 lore raw

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

In reply to Santi Béjar

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

---
 git-fetch.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
git-fetch.sh +1 -0
index 32553f8..ee4f5bd 100755
@@ -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."
Signature · 1 line
1.4.2.1.g38049
#8 lore raw

PATCH 5/5 fetch: Clean output

In reply to Santi Béjar

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/
---
 git-fetch.sh |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)
git-fetch.sh +11 -4
index ee4f5bd..cc71612 100755
@@ -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 () {
Signature · 1 line
1.4.2.1.g38049
#9 lore raw

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

Quoted 6 lines · Santi Béjar

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

Quoted 5 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

author
#10 lore raw

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

2006/9/30, Junio C Hamano <[email protected]>:

Quoted 9 lines
Quoted 6 lines · Santi Béjar

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
Quoted 4 lines

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).

Quoted 9 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.

Quoted 9 lines
Quoted 6 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.

Quoted 3 lines

* 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

#11 lore raw

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

Quoted 14 lines · Santi

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
Quoted 3 lines
$ 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.