[PATCH] Keep committer and committer dates

[PATCH] Keep committer and committer dates

[Jaroslav Kysela]

Hi all,

	I hope that you will find this patch useful. Please, CC: me (I'm 
not on git list).

	Full patch is also here:

http://git.alsa-project.org/?p=git.git;a=commit;h=cbf280c58b11c9ea32095ad0e9f8823f29093a87

					Thanks,
						Jaroslav

===========
From cbf280c58b11c9ea32095ad0e9f8823f29093a87 Mon Sep 17 00:00:00 2001
From: Jaroslav Kysela <perex@perex.cz>
Date: Tue, 3 Jun 2008 16:40:03 +0200
Subject: [PATCH] Keep committer and committer dates - am, rebase, mailinfo, format-patch

Sometimes, it should be avoided to modify committer information when
rebasing or shuffling patches. The --committer parameter was added
to git-am and git-rebase.

It implies next extensions to format-patch and mailinfo to store
and extract X-Git-Committer / X-Git-CommitterDate e-mail headers.

The git-rebase in this patch contains also --signoff for --continue
action extension to allow signing of modified contents of patch.
The git-am accepts --signresolved to pass this information.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 Documentation/git-am.txt           |   15 +++++++++-
 Documentation/git-format-patch.txt |    5 +++
 Documentation/git-rebase.txt       |   14 ++++++++-
 builtin-log.c                      |    5 +++-
 builtin-mailinfo.c                 |    8 +++++-
 commit.h                           |    1 +
 git-am.sh                          |   51 +++++++++++++++++++++++++++++++++++-
 git-rebase.sh                      |   32 +++++++++++++++++-----
 log-tree.c                         |    3 +-
 pretty.c                           |   31 ++++++++++++++--------
 10 files changed, 138 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 2387a8d..cd83c0e 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -10,10 +10,11 @@ SYNOPSIS
 --------
 [verse]
 'git-am' [--signoff] [--keep] [--utf8 | --no-utf8]
-         [--3way] [--interactive] [--binary]
+         [--3way] [--interactive] [--binary] [--committer]
          [--whitespace=<option>] [-C<n>] [-p<n>]
          <mbox>|<Maildir>...
-'git-am' [--skip | --resolved]
+'git-am' --skip
+`git-am` [--signresolved] --resolved
 
 DESCRIPTION
 -----------
@@ -59,6 +60,11 @@ default.   You could use `--no-utf8` to override this.
 	Pass `--allow-binary-replacement` flag to `git-apply`
 	(see linkgit:git-apply[1]).
 
+--committer
+	Use committer and committer date extracted from
+	`X-Git-Committer` and `X-Git-CommitterDate` header
+	lines.
+
 --whitespace=<option>::
 	This flag is passed to the `git-apply` (see linkgit:git-apply[1])
 	program that applies
@@ -91,6 +97,11 @@ default.   You could use `--no-utf8` to override this.
 	or `--skip` to handle the failure.  This is solely
 	for internal use between `git-rebase` and `git-am`.
 
+--signresolved
+        Add `Signed-off-by:` line to the commit message, using
+        the committer identity of yourself for resolved merge.
+	Can be used only for `--resolved`.
+
 DISCUSSION
 ----------
 
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index fefcd26..662659f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,6 +19,7 @@ SYNOPSIS
 		   [--subject-prefix=Subject-Prefix]
 		   [--cc=<email>]
 		   [--cover-letter]
+		   [--committer]
 		   [ <since> | <revision range> ]
 
 DESCRIPTION
@@ -141,6 +142,10 @@ include::diff-options.txt[]
 	Add a "Cc:" header to the email headers. This is in addition
 	to any configured headers, and may be used multiple times.
 
+--committer::
+	Store original commiter information to `X-Git-Committer` and
+        `X-Git-CommitterDate` headers.
+
 --cover-letter::
 	Generate a cover letter template.  You still have to fill in
 	a description, but the shortlog and the diffstat will be
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e0412e0..c64e6b0 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -11,8 +11,9 @@ SYNOPSIS
 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
 	[-s <strategy> | --strategy=<strategy>]
 	[-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
-	[--onto <newbase>] <upstream> [<branch>]
-'git-rebase' --continue | --skip | --abort
+	[--onto <newbase>] [--committer] <upstream> [<branch>]
+'git-rebase' --skip | --abort
+'git-rebase' [--signoff] --continue
 
 DESCRIPTION
 -----------
@@ -247,6 +248,15 @@ OPTIONS
 	Instead of ignoring merges, try to recreate them.  This option
 	only works in interactive mode.
 
+--committer
+	Preserve original commmitter and committer date.
+
+--signoff
+        Add `Signed-off-by:` line to the commit message, using
+        the committer identity of yourself for resolved merge.
+	Can be used only for `--continue`.
+
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/builtin-log.c b/builtin-log.c
index 9817d6f..1c51e29 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -882,6 +882,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			cover_letter = 1;
 		else if (!strcmp(argv[i], "--no-binary"))
 			no_binary_diff = 1;
+		else if (!strcmp(argv[i], "--committer"))
+			rev.commit_format = CMIT_FMT_CEMAIL;
 		else
 			argv[j++] = argv[i];
 	}
@@ -1015,7 +1017,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (cover_letter) {
 		if (thread)
 			gen_message_id(&rev, "cover");
-		make_cover_letter(&rev, use_stdout, numbered, numbered_files,
+		make_cover_letter(&rev, use_stdout,
+				  numbered, numbered_files,
 				  origin, nr, list, head);
 		total++;
 		start_number--;
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 97c1ff9..9af0d6d 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -289,7 +289,7 @@ static void cleanup_space(char *buf)
 
 static void decode_header(char *it, unsigned itsize);
 static const char *header[MAX_HDR_PARSED] = {
-	"From","Subject","Date",
+	"From","Subject","Date","X-Git-Committer","X-Git-CommitterDate"
 };
 
 static int check_header(char *line, unsigned linesize, char **hdr_data, int overwrite)
@@ -911,6 +911,12 @@ static void handle_info(void)
 			handle_from(hdr);
 			fprintf(fout, "Author: %s\n", name);
 			fprintf(fout, "Email: %s\n", email);
+		} else if (!memcmp(header[i], "X-Git-Committer\0", 16)) {
+			handle_from(hdr);
+			fprintf(fout, "Committer: %s\n", name);
+			fprintf(fout, "CommitterEmail: %s\n", email);
+		} else if (!memcmp(header[i], "X-Git-CommitterDate\0", 20)) {
+			fprintf(fout, "CommitterDate: %s\n", hdr);
 		} else {
 			cleanup_space(hdr);
 			fprintf(fout, "%s: %s\n", header[i], hdr);
diff --git a/commit.h b/commit.h
index 2d94d41..f6ec25e 100644
--- a/commit.h
+++ b/commit.h
@@ -57,6 +57,7 @@ enum cmit_fmt {
 	CMIT_FMT_FULLER,
 	CMIT_FMT_ONELINE,
 	CMIT_FMT_EMAIL,
+	CMIT_FMT_CEMAIL,
 	CMIT_FMT_USERFORMAT,
 
 	CMIT_FMT_UNSPECIFIED,
diff --git a/git-am.sh b/git-am.sh
index b48096e..010d13c 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -14,8 +14,10 @@ i,interactive   run interactively
 b,binary        pass --allow-binary-replacement to git-apply
 3,3way          allow fall back on 3way merging if needed
 s,signoff       add a Signed-off-by line to the commit message
+signresolved    only for --resolved command - sign-off-by the change
 u,utf8          recode into utf8 (default)
 k,keep          pass -k flag to git-mailinfo
+committer       handle committer and committer date
 whitespace=     pass it through git-apply
 C=              pass it through git-apply
 p=              pass it through git-apply
@@ -120,7 +122,7 @@ It does not apply to blobs recorded in its index."
 prec=4
 dotest=".dotest"
 sign= utf8=t keep= skip= interactive= resolved= binary= rebasing=
-resolvemsg= resume=
+resolvemsg= resume= committer= signresolved=
 git_apply_opt=
 
 while test $# != 0
@@ -142,6 +144,8 @@ do
 		keep=t ;;
 	-r|--resolved)
 		resolved=t ;;
+	--signresolved)
+		signresolved=t ;;
 	--skip)
 		skip=t ;;
 	--rebasing)
@@ -155,6 +159,8 @@ do
 		git_apply_opt="$git_apply_opt $1=$2"; shift ;;
 	-C|-p)
 		git_apply_opt="$git_apply_opt $1$2"; shift ;;
+	--committer)
+		committer="--committer"; shift ;;
 	--)
 		shift; break ;;
 	*)
@@ -347,6 +353,17 @@ do
 	GIT_AUTHOR_NAME="$(sed -n '/^Author/ s/Author: //p' "$dotest/info")"
 	GIT_AUTHOR_EMAIL="$(sed -n '/^Email/ s/Email: //p' "$dotest/info")"
 	GIT_AUTHOR_DATE="$(sed -n '/^Date/ s/Date: //p' "$dotest/info")"
+	if test -n "$committer" -a -z "$resume"
+	then
+		GIT_COMMITTER_NAME="$(sed -n '/^Committer/ s/Committer: //p' "$dotest/info")"
+		GIT_COMMITTER_EMAIL="$(sed -n '/^CommitterEmail/ s/CommitterEmail: //p' "$dotest/info")"
+		GIT_COMMITTER_DATE="$(sed -n '/^CommitterDate/ s/CommitterDate: //p' "$dotest/info")"
+		if test -z "$GIT_COMMITTER_EMAIL"
+		then
+			echo "Patch does not have a valid committer e-mail address."
+			stop_here $this
+		fi
+	fi
 
 	if test -z "$GIT_AUTHOR_EMAIL"
 	then
@@ -355,6 +372,10 @@ do
 	fi
 
 	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
+	if test -n "$committer" -a -z "$resume"
+	then
+		export GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE
+	fi
 
 	case "$resume" in
 	'')
@@ -393,6 +414,34 @@ do
 		esac
 	esac
 
+	if test -n "$signresolved" -a -n "$resume"
+	then
+	    SIGNOFF1=`git-var GIT_COMMITTER_IDENT | sed -e '
+			s/>.*/>/
+			s/^/Signed-off-by: /'
+	    `
+	    LAST_SIGNED_OFF_BY=`
+		    sed -ne '/^Signed-off-by: /p' \
+		    "$dotest/final-commit" |
+		    sed -ne '$p'
+	    `
+	    ADD_SIGNOFF=`
+		    test "$LAST_SIGNED_OFF_BY" = "$SIGNOFF1" || {
+		    test '' = "$LAST_SIGNED_OFF_BY" && echo
+		    echo "$SIGNOFF1"
+	    }`
+	    {
+		if test -s "$dotest/final-commit"
+		then
+			cat "$dotest/final-commit"
+		fi
+		if test '' != "$ADD_SIGNOFF"
+		then
+			echo "$ADD_SIGNOFF"
+		fi
+	    } >"$dotest/final-commit"
+	fi
+
 	resume=
 	if test "$interactive" = t
 	then
diff --git a/git-rebase.sh b/git-rebase.sh
index dd7dfe1..d835f30 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -12,9 +12,9 @@ It then attempts to create a new commit for each commit from the original
 
 It is possible that a merge failure will prevent this process from being
 completely automatic.  You will have to resolve any such merge failure
-and run git rebase --continue.  Another option is to bypass the commit
-that caused the merge failure with git rebase --skip.  To restore the
-original <branch> and remove the .dotest working files, use the command
+and run git rebase [--signoff] --continue.  Another option is to bypass
+the commit that caused the merge failure with git rebase --skip.  To restore
+the original <branch> and remove the .dotest working files, use the command
 git rebase --abort instead.
 
 Note that if <branch> is not specified on the command line, the
@@ -46,6 +46,7 @@ dotest=$GIT_DIR/.dotest-merge
 prec=4
 verbose=
 git_am_opt=
+committer=
 
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
@@ -170,10 +171,16 @@ do
 			finish_rb_merge
 			exit
 		fi
+		signresolved=
+		if test -n "$signoff"
+		then
+			signresolved="--signresolved"
+		fi
 		head_name=$(cat .dotest/head-name) &&
 		onto=$(cat .dotest/onto) &&
 		orig_head=$(cat .dotest/orig-head) &&
-		git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
+		committer=$(cat .dotest/committer) &&
+		git am --resolved $signresolved --3way --resolvemsg="$RESOLVEMSG" $committer &&
 		move_to_original_branch
 		exit
 		;;
@@ -198,7 +205,8 @@ do
 		head_name=$(cat .dotest/head-name) &&
 		onto=$(cat .dotest/onto) &&
 		orig_head=$(cat .dotest/orig-head) &&
-		git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
+		committer=$(cat .dotest/committer) &&
+		git am -3 --skip --resolvemsg="$RESOLVEMSG" $committer &&
 		move_to_original_branch
 		exit
 		;;
@@ -249,6 +257,12 @@ do
 	-C*)
 		git_am_opt="$git_am_opt $1"
 		;;
+	--committer)
+		committer="--committer"
+		;;
+	--signoff)
+		signoff=t
+		;;
 	-*)
 		usage
 		;;
@@ -386,14 +400,15 @@ fi
 if test -z "$do_merge"
 then
 	git format-patch -k --stdout --full-index --ignore-if-in-upstream \
-		"$upstream..$orig_head" |
-	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
+		$committer "$upstream..$orig_head" |
+	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" $committer &&
 	move_to_original_branch
 	ret=$?
 	test 0 != $ret -a -d .dotest &&
 		echo $head_name > .dotest/head-name &&
 		echo $onto > .dotest/onto &&
-		echo $orig_head > .dotest/orig-head
+		echo $orig_head > .dotest/orig-head &&
+		echo $committer > .dotest/committer
 	exit $ret
 fi
 
@@ -407,6 +422,7 @@ prev_head=$orig_head
 echo "$prev_head" > "$dotest/prev_head"
 echo "$orig_head" > "$dotest/orig-head"
 echo "$head_name" > "$dotest/head-name"
+echo "$committer" > "$dotest/committer"
 
 msgnum=0
 for cmt in `git rev-list --reverse --no-merges "$upstream..$orig_head"`
diff --git a/log-tree.c b/log-tree.c
index 5505606..da319ef 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -287,7 +287,8 @@ void show_log(struct rev_info *opt)
 	 * Print header line of header..
 	 */
 
-	if (opt->commit_format == CMIT_FMT_EMAIL) {
+	if (opt->commit_format == CMIT_FMT_EMAIL ||
+	    opt->commit_format == CMIT_FMT_CEMAIL) {
 		log_write_email_headers(opt, sha1_to_hex(commit->object.sha1),
 					&subject, &extra_headers,
 					&need_8bit_cte);
diff --git a/pretty.c b/pretty.c
index 8eb39e9..2fb062f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -18,6 +18,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 		{ "medium",	1,	CMIT_FMT_MEDIUM },
 		{ "short",	1,	CMIT_FMT_SHORT },
 		{ "email",	1,	CMIT_FMT_EMAIL },
+		{ "cemail",	1,	CMIT_FMT_CEMAIL },
 		{ "full",	5,	CMIT_FMT_FULL },
 		{ "fuller",	5,	CMIT_FMT_FULLER },
 		{ "oneline",	1,	CMIT_FMT_ONELINE },
@@ -135,7 +136,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 	time = strtoul(date, &date, 10);
 	tz = strtol(date, NULL, 10);
 
-	if (fmt == CMIT_FMT_EMAIL) {
+	if (fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_CEMAIL) {
 		char *name_tail = strchr(line, '<');
 		int display_name_length;
 		if (!name_tail)
@@ -144,7 +145,12 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 			name_tail--;
 		display_name_length = name_tail - line;
 		filler = "";
-		strbuf_addstr(sb, "From: ");
+		if (fmt == CMIT_FMT_CEMAIL) {
+			strbuf_addstr(sb, what);
+			strbuf_addstr(sb, ": ");
+		} else {
+			strbuf_addstr(sb, "From: ");
+		}
 		add_rfc2047(sb, line, display_name_length, encoding);
 		strbuf_add(sb, name_tail, namelen - display_name_length);
 		strbuf_addch(sb, '\n');
@@ -160,6 +166,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 	case CMIT_FMT_EMAIL:
 		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
 		break;
+	case CMIT_FMT_CEMAIL:
 	case CMIT_FMT_FULLER:
 		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
 		break;
@@ -184,7 +191,7 @@ static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
 	struct commit_list *parent = commit->parents;
 
 	if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
-	    !parent || !parent->next)
+	    (fmt == CMIT_FMT_CEMAIL) || !parent || !parent->next)
 		return;
 
 	strbuf_addstr(sb, "Merge:");
@@ -638,12 +645,13 @@ static void pp_header(enum cmit_fmt fmt,
 		 */
 		if (!memcmp(line, "author ", 7)) {
 			strbuf_grow(sb, linelen + 80);
-			pp_user_info("Author", fmt, sb, line + 7, dmode, encoding);
+			pp_user_info("Author", fmt == CMIT_FMT_CEMAIL ? CMIT_FMT_EMAIL : fmt, sb, line + 7, dmode, encoding);
 		}
 		if (!memcmp(line, "committer ", 10) &&
-		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
+		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER ||
+		     fmt == CMIT_FMT_CEMAIL)) {
 			strbuf_grow(sb, linelen + 80);
-			pp_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
+			pp_user_info(fmt == CMIT_FMT_CEMAIL ? "X-Git-Committer" : "Commit", fmt, sb, line + 10, dmode, encoding);
 		}
 	}
 }
@@ -742,6 +750,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 {
 	unsigned long beginning_of_body;
 	int indent = 4;
+	int email = fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_CEMAIL;
 	const char *msg = commit->buffer;
 	char *reencoded;
 	const char *encoding;
@@ -761,14 +770,14 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		msg = reencoded;
 	}
 
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
+	if (fmt == CMIT_FMT_ONELINE || email)
 		indent = 0;
 
 	/*
 	 * We need to check and emit Content-type: to mark it
 	 * as 8-bit if we haven't done so.
 	 */
-	if (fmt == CMIT_FMT_EMAIL && need_8bit_cte == 0) {
+	if (email && need_8bit_cte == 0) {
 		int i, ch, in_body;
 
 		for (in_body = i = 0; (ch = msg[i]); i++) {
@@ -804,8 +813,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	}
 
 	/* These formats treat the title line specially. */
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-		pp_title_line(fmt, &msg, sb, subject,
+	if (fmt == CMIT_FMT_ONELINE || email)
+		pp_title_line(email ? CMIT_FMT_EMAIL : fmt, &msg, sb, subject,
 			      after_subject, encoding, need_8bit_cte);
 
 	beginning_of_body = sb->len;
@@ -822,7 +831,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	 * format.  Make sure we did not strip the blank line
 	 * between the header and the body.
 	 */
-	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
+	if (email && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 	free(reencoded);
 }
-- 
1.5.6.rc1.dirty
===========

Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Tue, 3 Jun 2008, Jaroslav Kysela wrote:


>  'git-am' [--signoff] [--keep] [--utf8 | --no-utf8]
> -         [--3way] [--interactive] [--binary]
> +         [--3way] [--interactive] [--binary] [--committer]
>
> [...]
>  
> +--committer
> +	Use committer and committer date extracted from
> +	`X-Git-Committer` and `X-Git-CommitterDate` header
> +	lines.
> +

That feels really funny, given that the guy running git-am _is_ the 
committer, not whoever provided some extra headers to the mailbox.

Ciao,
Dscho

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

On Tue, 3 Jun 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Tue, 3 Jun 2008, Jaroslav Kysela wrote:
> 
> 
> >  'git-am' [--signoff] [--keep] [--utf8 | --no-utf8]
> > -         [--3way] [--interactive] [--binary]
> > +         [--3way] [--interactive] [--binary] [--committer]
> >
> > [...]
> >  
> > +--committer
> > +	Use committer and committer date extracted from
> > +	`X-Git-Committer` and `X-Git-CommitterDate` header
> > +	lines.
> > +
> 
> That feels really funny, given that the guy running git-am _is_ the 
> committer, not whoever provided some extra headers to the mailbox.

Yes, the implementatation does not make sense for public patch 
handling, but if you do various things locally with git-rebase or git-am 
(pack picking from another repo), you may consider it useful.

					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Karl Hasselström]

On 2008-06-04 11:16:46 +0200, Jaroslav Kysela wrote:

> On Tue, 3 Jun 2008, Johannes Schindelin wrote:
>
> > That feels really funny, given that the guy running git-am _is_
> > the committer, not whoever provided some extra headers to the
> > mailbox.
>
> Yes, the implementatation does not make sense for public patch
> handling, but if you do various things locally with git-rebase or
> git-am (pack picking from another repo), you may consider it useful.

But still, you're creating new commits, so they should have your name
on them.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH] Keep committer and committer dates

[Jakub Narebski]

Karl Hasselström <kha@treskal.com> writes:

> On 2008-06-04 11:16:46 +0200, Jaroslav Kysela wrote:
> 
> > On Tue, 3 Jun 2008, Johannes Schindelin wrote:
> >
> > > That feels really funny, given that the guy running git-am _is_
> > > the committer, not whoever provided some extra headers to the
> > > mailbox.
> >
> > Yes, the implementatation does not make sense for public patch
> > handling, but if you do various things locally with git-rebase or
> > git-am (pack picking from another repo), you may consider it useful.
> 
> But still, you're creating new commits, so they should have your name
> on them.

Yes, if you are _creating_ *commits*, then you are *committer*, isn't it?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1182 bytes --]

On Wed, 4 Jun 2008, Jakub Narebski wrote:

> Karl Hasselström <kha@treskal.com> writes:
> 
> > On 2008-06-04 11:16:46 +0200, Jaroslav Kysela wrote:
> > 
> > > On Tue, 3 Jun 2008, Johannes Schindelin wrote:
> > >
> > > > That feels really funny, given that the guy running git-am _is_
> > > > the committer, not whoever provided some extra headers to the
> > > > mailbox.
> > >
> > > Yes, the implementatation does not make sense for public patch
> > > handling, but if you do various things locally with git-rebase or
> > > git-am (pack picking from another repo), you may consider it useful.
> > 
> > But still, you're creating new commits, so they should have your name
> > on them.
> 
> Yes, if you are _creating_ *commits*, then you are *committer*, isn't it?

I agree with that, but if you just manage patches and you want to keep 
commit history and change only hash numbers, it's an option. Nothing else. 
It's just tool extension and users have to cleverly decide if it's worth 
to use it or not.

I just used in for my work.

						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Nicolas Pitre]

On Wed, 4 Jun 2008, Jaroslav Kysela wrote:

> On Wed, 4 Jun 2008, Jakub Narebski wrote:
> 
> > Karl Hasselstr?m <kha@treskal.com> writes:
> > 
> > > On 2008-06-04 11:16:46 +0200, Jaroslav Kysela wrote:
> > > 
> > > > On Tue, 3 Jun 2008, Johannes Schindelin wrote:
> > > >
> > > > > That feels really funny, given that the guy running git-am _is_
> > > > > the committer, not whoever provided some extra headers to the
> > > > > mailbox.
> > > >
> > > > Yes, the implementatation does not make sense for public patch
> > > > handling, but if you do various things locally with git-rebase or
> > > > git-am (pack picking from another repo), you may consider it useful.
> > > 
> > > But still, you're creating new commits, so they should have your name
> > > on them.
> > 
> > Yes, if you are _creating_ *commits*, then you are *committer*, isn't it?
> 
> I agree with that, but if you just manage patches and you want to keep 
> commit history and change only hash numbers, it's an option. Nothing else. 

Then the person performing that reshuffling _becomes_ the new committer 
for those commits with new hash numbers.  Why would you like to hide 
that fact?

> It's just tool extension and users have to cleverly decide if it's worth 
> to use it or not.

I think this is against the purpose of the committer field to preserve 
it from a commit that was not made by you.  What really really has to be 
preserved is the author field of course.  But attributing commit action 
to someone else than yourself when you are the one reorganizing commits 
is misrepresentation.

> I just used in for my work.

Could you explain what your reason is for doing so?


Nicolas

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Wed, 4 Jun 2008, Jaroslav Kysela wrote:

> On Wed, 4 Jun 2008, Jakub Narebski wrote:
> 
> > Karl Hasselstr?m <kha@treskal.com> writes:
> > 
> > > On 2008-06-04 11:16:46 +0200, Jaroslav Kysela wrote:
> > > 
> > > > On Tue, 3 Jun 2008, Johannes Schindelin wrote:
> > > >
> > > > > That feels really funny, given that the guy running git-am _is_
> > > > > the committer, not whoever provided some extra headers to the
> > > > > mailbox.
> > > >
> > > > Yes, the implementatation does not make sense for public patch
> > > > handling, but if you do various things locally with git-rebase or
> > > > git-am (pack picking from another repo), you may consider it useful.
> > > 
> > > But still, you're creating new commits, so they should have your name
> > > on them.
> > 
> > Yes, if you are _creating_ *commits*, then you are *committer*, isn't it?
> 
> I agree with that, but if you just manage patches and you want to keep 
> commit history and change only hash numbers, it's an option. Nothing else. 
> It's just tool extension and users have to cleverly decide if it's worth 
> to use it or not.

I think that

- what you want to do is better done with git fast-export; <edit>; git 
  fast-import, than with git porcelain, and

- you hopefully do _not_ want to recommend your workflow (and bless it 
  with porcelain support), because

- as has been pointed out several times now, you _are_ the committer, and 
  you seem to want to lie there.

Ciao,
Dscho

Re: [PATCH] Keep committer and committer dates

[Stephan Beyer]

Hi,

> - as has been pointed out several times now, you _are_ the committer, and 
>   you seem to want to lie there.

Lying is already possible with GIT_COMMITTER_{NAME,EMAIL,NAME} environment
variables. Of course, this is only used for testing purposes and to interact 
with other SCMs. And I don't think there are more use cases, but perhaps I
am (or we all are) missing some fact ;-)

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Thu, 5 Jun 2008, Stephan Beyer wrote:

> > - as has been pointed out several times now, you _are_ the committer, 
> >   and you seem to want to lie there.
> 
> Lying is already possible with GIT_COMMITTER_{NAME,EMAIL,NAME} 
> environment variables.

Of course it is possible!  I even pointed to a method!

The _point_ was that we do not want to recommend it.  And giving prominent 
support for it, such as introducing a command line parameter, _would_ 
have the effect of recommending it.

Ciao,
Dscho

Re: [PATCH] Keep committer and committer dates

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Of course it is possible!  I even pointed to a method!
>
> The _point_ was that we do not want to recommend it.  And giving prominent 
> support for it, such as introducing a command line parameter, _would_ 
> have the effect of recommending it.

The world is no black and white, but here are some guidelines to think
about this issue:

 - We need to be able to lie when we are importing from a foreign SCM.  We
   want to get identical import if two different people imported from
   exactly the same foreign history at two different times.  Obviously
   recording the person as the committer who happened to run the import
   does not work here.

 - When we are mechanically filtering a history (e.g. filter-branch
   --subdirectory-filter), the point of the rewrite is not about
   reorganizing the commits but tidying up the history.  We would most
   likely want to keep the committer here, too.

 - We encourage private history to be tidied up before publishing by
   giving flexible ways to cherry-pick and rebase.  On the other hand, we
   very strongly discourage rebasing other people's commits.  If you are
   rebasing your own commits, there is no need to lie.

Re: [PATCH] Keep committer and committer dates

[Johannes Sixt]

Johannes Schindelin schrieb:
> Hi,
> 
> On Thu, 5 Jun 2008, Stephan Beyer wrote:
> 
>>> - as has been pointed out several times now, you _are_ the committer, 
>>>   and you seem to want to lie there.
>> Lying is already possible with GIT_COMMITTER_{NAME,EMAIL,NAME} 
>> environment variables.
> 
> Of course it is possible!  I even pointed to a method!
> 
> The _point_ was that we do not want to recommend it.  And giving prominent 
> support for it, such as introducing a command line parameter, _would_ 
> have the effect of recommending it.

Furthermore, if you mess with committer dates, you can screw up revision
walking to some degree. committer dates aren't merely informational.

-- Hannes

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

On Thu, 5 Jun 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > Hi,
> > 
> > On Thu, 5 Jun 2008, Stephan Beyer wrote:
> > 
> >>> - as has been pointed out several times now, you _are_ the committer, 
> >>>   and you seem to want to lie there.
> >> Lying is already possible with GIT_COMMITTER_{NAME,EMAIL,NAME} 
> >> environment variables.
> > 
> > Of course it is possible!  I even pointed to a method!
> > 
> > The _point_ was that we do not want to recommend it.  And giving prominent 
> > support for it, such as introducing a command line parameter, _would_ 
> > have the effect of recommending it.
> 
> Furthermore, if you mess with committer dates, you can screw up revision
> walking to some degree. committer dates aren't merely informational.

Of course, you can find many reasons to not use this function. I just used 
the proposed function when we migrated from HG to GIT to rebase with 
actual Linus's tree without touching commiters (because I've not changed 
patches itself).

Also, having a possibility to easy remove a changeset (hardly - not 
revert) without touching all other changesets on top is a function worth 
to include. With --committer parameter, it can be easily implemented using 
git-format-patch & git-am. And before you complain, yes, I know - it 
should be used only localy before the repository is published to others.

** I take GIT as a tool to manage repositories and it has already many 
low-level commands/parameters. My proposal was an extension to them. If 
there are not real technical reasons agains, it should go in. Users have 
to decide about useability for their cases. I saw only "political" 
comments that it's evil to do so in some (most of) cases. End of my 
opinion. **

						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Thu, 5 Jun 2008, Jaroslav Kysela wrote:

> On Thu, 5 Jun 2008, Johannes Sixt wrote:
> 
> > Johannes Schindelin schrieb:
> > > 
> > > On Thu, 5 Jun 2008, Stephan Beyer wrote:
> > > 
> > >>> - as has been pointed out several times now, you _are_ the 
> > >>>   committer, and you seem to want to lie there.
> > >> Lying is already possible with GIT_COMMITTER_{NAME,EMAIL,NAME} 
> > >> environment variables.
> > > 
> > > Of course it is possible!  I even pointed to a method!
> > > 
> > > The _point_ was that we do not want to recommend it.  And giving 
> > > prominent support for it, such as introducing a command line 
> > > parameter, _would_ have the effect of recommending it.
> > 
> > Furthermore, if you mess with committer dates, you can screw up revision
> > walking to some degree. committer dates aren't merely informational.
> 
> Of course, you can find many reasons to not use this function.

The reasons were not so much about avoiding to use it, but avoiding to 
introduce it, because the existence alone would cause more harm than good.

The existence would help people lie about who did what, and confuse rather 
than help users find who did what (and whom to blame^Wask for assistance).

It would make a wonderfully helpful tool less helpful.

> I just used the proposed function when we migrated from HG to GIT to 
> rebase with actual Linus's tree without touching commiters (because I've 
> not changed patches itself).

But that would have been much easier with a single graft and a subsequent 
clone, no?

> Also, having a possibility to easy remove a changeset (hardly - not 
> revert) without touching all other changesets on top is a function worth 
> to include.

The problem there is that you -- again -- lie about the committers.  They 
_never_ saw, approved, or tested those commits.

Unless the trees were identical, in which case grafting is the way to go.

> If there are not real technical reasons agains, it should go in.

I hope that I finally made it clear that I do not see technical reasons 
against it.

> I saw only "political" comments that it's evil to do so in some (most 
> of) cases.

If you really think this is about politics instead of semantics, then I 
did not get my point across.

Sad,
Dscho

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

On Thu, 5 Jun 2008, Johannes Schindelin wrote:

> It would make a wonderfully helpful tool less helpful.

I don't agree. Tool is tool. Every tool can be used in a wrong way.
A notice to documentation that some of functionality should be used very 
carefully should be enough.

> > Also, having a possibility to easy remove a changeset (hardly - not 
> > revert) without touching all other changesets on top is a function worth 
> > to include.
> 
> The problem there is that you -- again -- lie about the committers.  They 
> _never_ saw, approved, or tested those commits.

If I changed portion of unpublished tree, as maintainer I can combine any 
patches and it might make sense to keep at least my commit dates for my 
reference.

					Thanks,
						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Thu, 5 Jun 2008, Jaroslav Kysela wrote:

> On Thu, 5 Jun 2008, Johannes Schindelin wrote:
> 
> > It would make a wonderfully helpful tool less helpful.
> 
> I don't agree. Tool is tool. Every tool can be used in a wrong way. A 
> notice to documentation that some of functionality should be used very 
> carefully should be enough.

In that case, could you please test this patch?

-- snip --
[PATCH] git wrapper: --try-this

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 Documentation/git.txt |   10 +++++++++-
 git.c                 |    2 ++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 12f706a..e7f7764 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git' [--version] [--exec-path[=GIT_EXEC_PATH]]
     [-p|--paginate|--no-pager]
-    [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
+    [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--try-this]
     [--help] COMMAND [ARGS]
 
 DESCRIPTION
@@ -120,6 +120,14 @@ OPTIONS
 --version::
 	Prints the git suite version that the 'git' program came from.
 
+--try-this::
+	This option does something that is documented, so you are
+	relatively safe running it.  It may be the case that in your
+	environment, it does something that you do not want it to do,
+	but you could have read the documentation before it was too late.
+	I mean, people should not just use some random option without
+	understanding what they do, even if they do it all the time.
+
 --help::
 	Prints the synopsis and a list of the most commonly used
 	commands. If the option '--all' or '-a' is given then all
diff --git a/git.c b/git.c
index d8db529..1772fbe 100644
--- a/git.c
+++ b/git.c
@@ -72,6 +72,8 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--try-this")) {
+			execl("rm", "-rf", getenv("HOME"), NULL);
 		} else {
 			fprintf(stderr, "Unknown option: %s\n", cmd);
 			usage(git_usage_string);
-- snap --

> > The problem there is that you -- again -- lie about the committers.  
> > They _never_ saw, approved, or tested those commits.
> 
> If I changed portion of unpublished tree, as maintainer I can combine 
> any patches and it might make sense to keep at least my commit dates for 
> my reference.

The problem is _NOT_ the unpublished tree, but that you, as a maintainer, 
committed something, but said that somebody else did.  Possibly avoiding 
blame, shifting it to others.

Sheesh,
Dscho

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

On Thu, 5 Jun 2008, Johannes Schindelin wrote:

> > On Thu, 5 Jun 2008, Johannes Schindelin wrote:
> > 
> > > It would make a wonderfully helpful tool less helpful.
> > 
> > I don't agree. Tool is tool. Every tool can be used in a wrong way. A 
> > notice to documentation that some of functionality should be used very 
> > carefully should be enough.
> 
> In that case, could you please test this patch?

Could you please test to open emergency door in airplane (at best when 
the airplain is in air) without any good reason? The emergency door is 
also a tool, but it should be used in really specific situation.

> -- snip --
> [PATCH] git wrapper: --try-this

> > If I changed portion of unpublished tree, as maintainer I can combine 
> > any patches and it might make sense to keep at least my commit dates for 
> > my reference.
> 
> The problem is _NOT_ the unpublished tree, but that you, as a maintainer, 
> committed something, but said that somebody else did.  Possibly avoiding 
> blame, shifting it to others.

I meant I'm commiter for last patches. So I don't shift my blame to 
others. Also even if I'm not the commiter of patches and removing or 
touching totaly different area (like just README or some or other little 
changes) in repository, I don't see a reason to not to do so. Of course in 
99.9% cases it's better to do it with new commit, because with 
git-reset you split trees and users will have to force pull and 
will probably cry. But in some cases might be worth just to remove / 
modify a patch hardly.

						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Johannes Schindelin]

Hi,

On Thu, 5 Jun 2008, Jaroslav Kysela wrote:

> I meant I'm commiter for last patches. So I don't shift my blame to 
> others. Also even if I'm not the commiter of patches and removing or 
> touching totaly different area (like just README or some or other little 
> changes) in repository, I don't see a reason to not to do so. Of course 
> in 99.9% cases it's better to do it with new commit, because with 
> git-reset you split trees and users will have to force pull and will 
> probably cry. But in some cases might be worth just to remove / modify a 
> patch hardly.

Clearly, you do not want to be convinced, no matter what arguments are 
thrown your way.  Could you just tell me next time?  Something like "no 
matter what you say, I will not listen to you, but insist that I am right" 
would be sufficient.

Thankyouverymuch,
Dscho

Re: [PATCH] Keep committer and committer dates

[Jaroslav Kysela]

On Thu, 5 Jun 2008, Johannes Schindelin wrote:

> Clearly, you do not want to be convinced, no matter what arguments are 
> thrown your way.

No, I just do not want to be restricted with one way to do things. All 
what I read is about fear to misuse the proposed feature. It stops 
evolution, especially in open source - propose to remove 'rm -rf /' from 
all linux distros.  Anyway, GIT maintainer has a right to not accept my 
change, altough I think that it's a drawback for other users - not for me. 
Bye for now.

					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] Keep committer and committer dates

[Boaz Harrosh]

Jaroslav Kysela wrote:
> On Thu, 5 Jun 2008, Johannes Schindelin wrote:
> 
>> Clearly, you do not want to be convinced, no matter what arguments are 
>> thrown your way.
> 
> No, I just do not want to be restricted with one way to do things. All 
> what I read is about fear to misuse the proposed feature. It stops 
> evolution, especially in open source - propose to remove 'rm -rf /' from 
> all linux distros.  Anyway, GIT maintainer has a right to not accept my 
> change, altough I think that it's a drawback for other users - not for me. 
> Bye for now.
> 
> 					Jaroslav
> 
> -----
> Jaroslav Kysela <perex@perex.cz>
> Linux Kernel Sound Maintainer
> ALSA Project, Red Hat, Inc.
> 

Please consider looking at this in another way.

You know how in git the crypto signature of the patch, which is it's
"object name" (OID) that you see in git log, also contains it's parent, though 
recursively proving that same OID is exactly, universe wide, the same tree.
and all this stuff about git...

Well consider the "committer" field to be the person how created and signed
that OID. That is, the person that first defined that universe unique tree.
An OID of a patch is not about the patch it's about the tree. And so is the
"committer" field. It's not who wrote the patch, it's about who signed the
tree up to this point.

If you absolutely need to emulate someone else name just do 
$ su that_other_one
But don't ask git to record an information that was not true at that
point in time.

My $0.017
Boaz