threads / discuss / 4839

git-update-ref (reflog) uses bogus author ident information

Subject: git-update-ref (reflog) uses bogus author ident information

## tl;dr

6 messages between Jul 10, 2006 and Jul 11, 2006.

replies: 5people: 2as markdown or json

Jakub Narebski· Jul 10, 2006, 20:52 UTC · lore
git-log reports 
  commit 059111c9381ce1444d17c8fc35606b0aa417ca42
  Author: Jakub Narebski <jnareb@gmail.com>
  Date:   Sat Jul 8 18:52:35 2006 +0200
  
      configure.ac vertical whitespace usage cleanup
 
git-var -l shows:
 GIT_COMMITTER_IDENT=Jakub Narebski <jnareb@gmail.com> 1152564452 +0200
 GIT_AUTHOR_IDENT=Jakub Narebski <jnareb@gmail.com> 1152564452 +0200
BUT in git/.git/logs/refs/heads/autoconf I have (broken into lines):
  fe7b45a419ae62ed96148d98f6aba8710a6f6245
  059111c9381ce1444d17c8fc35606b0aa417ca42 
  Jakub Narebski <jnareb@roke.D-201> 1152377555 +0200   
  commit: configure.ac vertical whitespace usage cleanu

where "roke.D-201" are results of "hostname -f" on my computer, and are suitable _only_ for my small private local network.

Bug or a feature?
I use git 1.4.0.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Jakub Narebski· Jul 10, 2006, 21:49 UTC · re: Jakub Narebski · lore

reflog doesn't note that commit was --amend-ed, and doesn't record pulls

Additionally, while reflog records git-reset invocations, it doesn't distinguish between an ordinary commit, and commit --amend (which I do a lot, most time because of forgotten update-index; yes, I know about commit -a option ;-). Well, you can extract this information looking at current and previous commit sha1, but it would be nice to have it noted somewhat in message part of reflog.

Reflog doesn't seem also to record pulls (e.g. master branch): pulls has empty reflog message part.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Shawn Pearce· Jul 11, 2006, 02:48 UTC · re: Jakub Narebski · lore

Re: reflog doesn't note that commit was --amend-ed, and doesn't record pulls

Jakub Narebski <jnareb@gmail.com> wrote:
Show 6 quoted lines
> Additionally, while reflog records git-reset invocations, it doesn't
> distinguish between an ordinary commit, and commit --amend (which I do
> a lot, most time because of forgotten update-index; yes, I know about commit
> -a option ;-). Well, you can extract this information looking at current
> and previous commit sha1, but it would be nice to have it noted somewhat in
> message part of reflog.
I'm attaching a patch below which fixes this.  It denotes the 4
major types of commits: initial, normal, amend, merge.
 
> Reflog doesn't seem also to record pulls (e.g. master branch): pulls has
> empty reflog message part.

Yea, that's a 'TODO' item. Now that someone is complaining about it I'll see if I can't get a fix done later tonight for it. Its probably going to be a few patches: one for git-merge and another for receive-pack.c.

-->-- Record the type of commit operation in the reflog.

If committing a merge (.git/MERGE_HEAD exists), an initial tree (no HEAD) or using --amend to amend the prior commit then denote the subtype of commit in the reflog. This helps to distinguish amended or merge commits from normal commits.

In the case of --amend the prior sha1 is probably the commit which is being thrown away in favor of the new commit. Since it is likely that the old commit doesn't have any ref pointing to it anymore it can be interesting to know why that the commit was replaced and orphaned.

In the case of a merge the prior sha1 is probably the first parent of the new merge commit. Consequently having its prior sha1 in the reflog is slightly less interesting but its still informative to know the commit was the result of a merge which had to be completed by hand.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 git-commit.sh         |    6 +++++-
 t/t1400-update-ref.sh |   19 ++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index 22c4ce8..05eccfe 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -635,9 +635,12 @@ fi
 PARENTS="-p HEAD"
 if test -z "$initial_commit"
 then
+	rloga='commit'
 	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
+		rloga='commit (merge)'
 		PARENTS="-p HEAD "`sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD"`
 	elif test -n "$amend"; then
+		rloga='commit (amend)'
 		PARENTS=$(git-cat-file commit HEAD |
 			sed -n -e '/^$/q' -e 's/^parent /-p /p')
 	fi
@@ -649,6 +652,7 @@ else
 	fi
 	PARENTS=""
 	current=
+	rloga='commit (initial)'
 fi
 
 if test -z "$no_edit"
@@ -724,7 +728,7 @@ then
 	fi &&
 	commit=$(cat "$GIT_DIR"/COMMIT_MSG | git-commit-tree $tree $PARENTS) &&
 	rlogm=$(sed -e 1q "$GIT_DIR"/COMMIT_MSG) &&
-	git-update-ref -m "commit: $rlogm" HEAD $commit $current &&
+	git-update-ref -m "$rloga: $rlogm" HEAD $commit $current &&
 	rm -f -- "$GIT_DIR/MERGE_HEAD" &&
 	if test -f "$NEXT_INDEX"
 	then
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index df3e993..6a3515d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -188,17 +188,30 @@ test_expect_success \
 	 echo OTHER >F &&
 	 GIT_AUTHOR_DATE="2005-05-26 23:41" \
 	 GIT_COMMITTER_DATE="2005-05-26 23:41" git-commit -F M -a &&
-	 h_OTHER=$(git-rev-parse --verify HEAD)
+	 h_OTHER=$(git-rev-parse --verify HEAD) &&
+	 echo FIXED >F &&
+	 EDITOR=true \
+	 GIT_AUTHOR_DATE="2005-05-26 23:44" \
+	 GIT_COMMITTER_DATE="2005-05-26 23:44" git-commit --amend &&
+	 h_FIXED=$(git-rev-parse --verify HEAD) &&
+	 echo TEST+FIXED >F &&
+	 echo Merged initial commit and a later commit. >M &&
+	 echo $h_TEST >.git/MERGE_HEAD &&
+	 GIT_AUTHOR_DATE="2005-05-26 23:45" \
+	 GIT_COMMITTER_DATE="2005-05-26 23:45" git-commit -F M &&
+	 h_MERGED=$(git-rev-parse --verify HEAD)
 	 rm -f M'
 
 cat >expect <<EOF
-$Z $h_TEST $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	commit: add
+$Z $h_TEST $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	commit (initial): add
 $h_TEST $h_OTHER $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000	commit: The other day this did not work.
+$h_OTHER $h_FIXED $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117151040 +0000	commit (amend): The other day this did not work.
+$h_FIXED $h_MERGED $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117151100 +0000	commit (merge): Merged initial commit and a later commit.
 EOF
 test_expect_success \
 	'git-commit logged updates' \
 	'diff expect .git/logs/$m'
-unset h_TEST h_OTHER
+unset h_TEST h_OTHER h_FIXED h_MERGED
 
 test_expect_success \
 	'git-cat-file blob master:F (expect OTHER)' \
-- 
1.4.1.gc48f
Shawn Pearce· Jul 11, 2006, 03:38 UTC · re: Jakub Narebski · lore

Re: reflog doesn't note that commit was --amend-ed, and doesn't record pulls

Jakub Narebski <jnareb@gmail.com> wrote:
> Reflog doesn't seem also to record pulls (e.g. master branch): pulls has
> empty reflog message part.

Fixed with the patch below. git-merge doesn't record the merges yet, so some changes made by git-pull still aren't logged as nicely as one would like. But don't fear, that will be coming soon. :-)

-->-- Log ref changes made by git-fetch and git-pull.

When git-fetch updates a reference record in the associated reflog what type of update took place and who caused it (git-fetch or git-pull, by invoking git-fetch).

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 git-fetch.sh |   20 +++++++++++++++-----
 git-pull.sh  |    2 +-
 2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/git-fetch.sh b/git-fetch.sh
index 48818f8..5e4c4d6 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -11,6 +11,7 @@ LF='
 '
 IFS="$LF"
 
+rloga=fetch
 no_tags=
 tags=
 append=
@@ -51,6 +52,9 @@ do
 	-k|--k|--ke|--kee|--keep)
 		keep=--keep
 		;;
+	--reflog-action=*)
+		rloga=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+		;;
 	-*)
 		usage
 		;;
@@ -75,6 +79,9 @@ refs=
 rref=
 rsync_slurped_objects=
 
+rloga="$rloga $remote_nick"
+test "$remote_nick" == "$remote" || rloga="$rloga $remote"
+
 if test "" = "$append"
 then
 	: >"$GIT_DIR/FETCH_HEAD"
@@ -149,11 +156,12 @@ fast_forward_local () {
 			[ "$verbose" ] && echo >&2 "* $1: same as $3"
 		else
 			echo >&2 "* $1: updating with $3"
+			git-update-ref -m "$rloga: updating tag" "$1" "$2"
 		fi
 	else
 		echo >&2 "* $1: storing $3"
+		git-update-ref -m "$rloga: storing tag" "$1" "$2"
 	fi
-	git-update-ref "$1" "$2"
 	;;
 
     refs/heads/* | refs/remotes/*)
@@ -174,7 +182,7 @@ fast_forward_local () {
 	    *,$local)
 		echo >&2 "* $1: fast forward to $3"
 		echo >&2 "  from $local to $2"
-		git-update-ref "$1" "$2" "$local"
+		git-update-ref -m "$rloga: fast-forward" "$1" "$2" "$local"
 		;;
 	    *)
 		false
@@ -184,7 +192,7 @@ fast_forward_local () {
 		case ",$force,$single_force," in
 		*,t,*)
 			echo >&2 "  forcing update."
-			git-update-ref "$1" "$2" "$local"
+			git-update-ref -m "$rloga: forced-update" "$1" "$2" "$local"
 			;;
 		*)
 			echo >&2 "  not updating."
@@ -194,7 +202,7 @@ fast_forward_local () {
 	    }
 	else
 	    echo >&2 "* $1: storing $3"
-	    git-update-ref "$1" "$2"
+	    git-update-ref -m "$rloga: storing head" "$1" "$2"
 	fi
 	;;
     esac
@@ -422,7 +430,9 @@ case ",$update_head_ok,$orig_head," in
 	curr_head=$(git-rev-parse --verify HEAD 2>/dev/null)
 	if test "$curr_head" != "$orig_head"
 	then
-	    	git-update-ref HEAD "$orig_head"
+	    git-update-ref \
+			-m "$rloga: Undoing incorrectly fetched HEAD." \
+			HEAD "$orig_head"
 		die "Cannot fetch into the current branch."
 	fi
 	;;
diff --git a/git-pull.sh b/git-pull.sh
index 076785c..d337bf4 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -45,7 +45,7 @@ do
 done
 
 orig_head=$(git-rev-parse --verify HEAD) || die "Pulling into a black hole?"
-git-fetch --update-head-ok "$@" || exit 1
+git-fetch --update-head-ok --reflog-action=pull "$@" || exit 1
 
 curr_head=$(git-rev-parse --verify HEAD)
 if test "$curr_head" != "$orig_head"
-- 
1.4.1.gc48f
Shawn Pearce· Jul 11, 2006, 00:27 UTC · re: Jakub Narebski · lore

Re: git-update-ref (reflog) uses bogus author ident information

Jakub Narebski <jnareb@gmail.com> wrote:
Show 23 quoted lines
> git-log reports 
> 
>   commit 059111c9381ce1444d17c8fc35606b0aa417ca42
>   Author: Jakub Narebski <jnareb@gmail.com>
>   Date:   Sat Jul 8 18:52:35 2006 +0200
>   
>       configure.ac vertical whitespace usage cleanup
>  
> git-var -l shows:
> 
>  GIT_COMMITTER_IDENT=Jakub Narebski <jnareb@gmail.com> 1152564452 +0200
>  GIT_AUTHOR_IDENT=Jakub Narebski <jnareb@gmail.com> 1152564452 +0200
> 
> BUT in git/.git/logs/refs/heads/autoconf I have (broken into lines):
>   fe7b45a419ae62ed96148d98f6aba8710a6f6245
>   059111c9381ce1444d17c8fc35606b0aa417ca42 
>   Jakub Narebski <jnareb@roke.D-201> 1152377555 +0200   
>   commit: configure.ac vertical whitespace usage cleanu
> 
> where "roke.D-201" are results of "hostname -f" on my computer, and are
> suitable _only_ for my small private local network.
> 
> Bug or a feature?

This is definately a bug. The reflog uses the same ident code that commit-tree uses, with the idea that GIT_COMMITTER_IDENT would drive the entry in the reflog, as you expected it to.

Looking in refs.c at log_ref_write its invoking setup_ident() then git_committer_info(1). git_committer_info should be using the environment variables (GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL, GIT_COMMITTER_DATE) with defaults back to the gecos values. Based on that it would seem that the user data in .git/config is being ignored... Hm.

So you must be expecting .git/config to be supplying your data but its getting overrwritten by setup_ident() in log_ref_write.

Here's the bug fix. Sorry about that one. That bug has been in there since the reflog code was first put into GIT. :-)

-->-- Allow user.name and user.email to drive reflog entry.

Apparently calling setup_ident() after git_config causes the user.name and user.email values read from the config file to be replaced with the data obtained from the host. This means that users who have setup their email address in user.email will instead be writing reflog entries with their hostname.

Moving setup_ident() to before git_config in update-ref resolves
this ordering problem.
---
 builtin-update-ref.c |    1 +
 refs.c               |    1 -
 2 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 00333c7..83094ab 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -12,6 +12,7 @@ int cmd_update_ref(int argc, const char 
 	unsigned char sha1[20], oldsha1[20];
 	int i;
 
+	setup_ident();
 	setup_git_directory();
 	git_config(git_default_config);
 
diff --git a/refs.c b/refs.c
index 2d9c1dc..56db394 100644
--- a/refs.c
+++ b/refs.c
@@ -379,7 +379,6 @@ static int log_ref_write(struct ref_lock
 			lock->log_file, strerror(errno));
 	}
 
-	setup_ident();
 	committer = git_committer_info(1);
 	if (msg) {
 		maxlen = strlen(committer) + strlen(msg) + 2*40 + 5;
-- 
1.4.1.gc48f
Shawn Pearce· Jul 11, 2006, 00:32 UTC · re: Shawn Pearce · lore

Re: git-update-ref (reflog) uses bogus author ident information

Shawn Pearce <spearce@spearce.org> wrote:
Show 10 quoted lines
> Allow user.name and user.email to drive reflog entry.
> 
> Apparently calling setup_ident() after git_config causes the
> user.name and user.email values read from the config file to be
> replaced with the data obtained from the host.  This means that
> users who have setup their email address in user.email will instead
> be writing reflog entries with their hostname.
> 
> Moving setup_ident() to before git_config in update-ref resolves
> this ordering problem.
Whoops, I forgot this line:  :-)
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Show 31 quoted lines
> ---
>  builtin-update-ref.c |    1 +
>  refs.c               |    1 -
>  2 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/builtin-update-ref.c b/builtin-update-ref.c
> index 00333c7..83094ab 100644
> --- a/builtin-update-ref.c
> +++ b/builtin-update-ref.c
> @@ -12,6 +12,7 @@ int cmd_update_ref(int argc, const char 
>  	unsigned char sha1[20], oldsha1[20];
>  	int i;
>  
> +	setup_ident();
>  	setup_git_directory();
>  	git_config(git_default_config);
>  
> diff --git a/refs.c b/refs.c
> index 2d9c1dc..56db394 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -379,7 +379,6 @@ static int log_ref_write(struct ref_lock
>  			lock->log_file, strerror(errno));
>  	}
>  
> -	setup_ident();
>  	committer = git_committer_info(1);
>  	if (msg) {
>  		maxlen = strlen(committer) + strlen(msg) + 2*40 + 5;
> -- 
> 1.4.1.gc48f

← back to recent threads