Git development
 help / color / mirror / Atom feed
* Re: [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0
From: Jeff King @ 2017-01-28  4:07 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <20170119163841.zhtpz6rxdieaywuy@sigill.intra.peff.net>

On Thu, Jan 19, 2017 at 11:38:41AM -0500, Jeff King wrote:

> On Thu, Jan 19, 2017 at 06:41:21PM +0700, Nguyễn Thái Ngọc Duy wrote:
> 
> > In this code we want to match the word "reset". If len is zero,
> > strncasecmp() will return zero and we incorrectly assume it's "reset" as
> > a result.
> 
> This is probably a good idea. This _is_ user-visible, so it's possible
> somebody was using empty config as a synonym for "reset". But since it
> was never documented, I feel like relying on that is somewhat crazy.

Hrm. This seems to break the add--interactive script if you do not have
color.diff.plain set:

  $ GIT_TRACE=1 git add -p
  ...
  22:58:12.568990 [pid=11401] git.c:387  trace: built-in: git 'config' '--get-color' 'color.diff.plain' ''
  fatal: unable to parse default color value
  config --get-color color.diff.plain : command returned error: 128

As you can see, the default value the empty string, which is now an
error.

The default in the C code for that value is GIT_COLOR_NORMAL, which
really is the empty string. So I think the old code was buggy to choose
"reset", but the new one is worse because it fails entirely. :)

We probably want something like this instead:

diff --git a/color.c b/color.c
index 190b2da96..dee61557e 100644
--- a/color.c
+++ b/color.c
@@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 		len--;
 	}
 
-	if (!len)
-		return -1;
+	if (!len) {
+		dst[0] = '\0';
+		return 0;
+	}
 
 	if (!strncasecmp(ptr, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);

The breakage is in 'next' (it looks like it went out a few days ago; I'm
surprised I didn't notice until now).

-Peff

^ permalink raw reply related

* Re: show all merge conflicts
From: G. Sylvie Davies @ 2017-01-28  5:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael Spiegel, git
In-Reply-To: <20170127175151.srhhczliqgvbzcre@sigill.intra.peff.net>

On Fri, Jan 27, 2017 at 9:51 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 27, 2017 at 11:56:08AM -0500, Michael Spiegel wrote:
>
>> I'm trying to determine whether a merge required a conflict to resolve
>> after the merge has occurred. The git book has some advice
>> (https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
>> `git show` on the merge commit or use `git log --cc -p -1`. These
>> strategies work when the merge conflict was resolved with a change
>> that is different from either parent. When the conflict is resolved
>> with a change that is the same as one of the parents, then these
>> commands are indistinguishable from a merge that did not conflict. Is
>> it possible to distinguish between a conflict-free merge and a merge
>> conflict that is resolved by with the changes from one the parents?
>
> No. You'd have to replay the merge to know if it would have had
> conflicts.
>


Aside from the usual "git log -cc", I think this should work (replace
HEAD with whichever commit you are analyzing):

git diff --name-only HEAD^2...HEAD^1 > m1
git diff --name-only HEAD^1...HEAD^2 > b1
git diff --name-only HEAD^1..HEAD    > m2
git diff --name-only HEAD^2..HEAD    > b2

If files listed between m1 and b2 differ, then the merge is dirty.
Similarly for m2 and b1.

More information here:

http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308


- Sylvie



> There was a patch series a few years ago that added a new diff-mode to
> do exactly that, and show the diff against what was resolved. It had a
> few issues (I don't remember exactly what) and never got merged.
>
> Certainly one complication is that you don't know exactly _how_ the
> merge was done in the first place (e.g., which merge strategy, which
> custom merge drivers were in effect, etc). But in general, replaying
> with a standard merge-recursive would get you most of what you want to
> know.
>
> I've done this manually sometimes when digging into erroneous merges
> (e.g., somebody accidentally runs "git reset -- <paths>" in the middle
> of a merge and throws away some changes.
>
> You should be able to do:
>
>   git checkout $merge^1
>   git merge $merge^2
>   git diff -R $merge
>
> to see what the original resolution did.
>
> -Peff

^ permalink raw reply

* Re: show all merge conflicts
From: Philip Oakley @ 2017-01-28 13:43 UTC (permalink / raw)
  To: G. Sylvie Davies, Jeff King; +Cc: Michael Spiegel, git
In-Reply-To: <CAAj3zPzO4+9t9_L2OXFmkihw-HwFvzybb7GXs4tTeFRyZHOaNQ@mail.gmail.com>

From: "G. Sylvie Davies" <sylvie@bit-booster.com>
> On Fri, Jan 27, 2017 at 9:51 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, Jan 27, 2017 at 11:56:08AM -0500, Michael Spiegel wrote:
>>
>>> I'm trying to determine whether a merge required a conflict to resolve
>>> after the merge has occurred. The git book has some advice
>>> (https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
>>> `git show` on the merge commit or use `git log --cc -p -1`. These
>>> strategies work when the merge conflict was resolved with a change
>>> that is different from either parent. When the conflict is resolved
>>> with a change that is the same as one of the parents, then these
>>> commands are indistinguishable from a merge that did not conflict. Is
>>> it possible to distinguish between a conflict-free merge and a merge
>>> conflict that is resolved by with the changes from one the parents?
>>
>> No. You'd have to replay the merge to know if it would have had
>> conflicts.
>>
>
>
> Aside from the usual "git log -cc", I think this should work (replace
> HEAD with whichever commit you are analyzing):
>
> git diff --name-only HEAD^2...HEAD^1 > m1
> git diff --name-only HEAD^1...HEAD^2 > b1
> git diff --name-only HEAD^1..HEAD    > m2
> git diff --name-only HEAD^2..HEAD    > b2
>
> If files listed between m1 and b2 differ, then the merge is dirty.
> Similarly for m2 and b1.
>
> More information here:
>
> http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308
>
>
> - Sylvie

This feels as though there ought to be some sort of --left-right option to 
get an indication of which side various changes came from

>
>> There was a patch series a few years ago that added a new diff-mode to
>> do exactly that, and show the diff against what was resolved. It had a
>> few issues (I don't remember exactly what) and never got merged.
>>
>> Certainly one complication is that you don't know exactly _how_ the
>> merge was done in the first place (e.g., which merge strategy, which
>> custom merge drivers were in effect, etc). But in general, replaying
>> with a standard merge-recursive would get you most of what you want to
>> know.
>>
>> I've done this manually sometimes when digging into erroneous merges
>> (e.g., somebody accidentally runs "git reset -- <paths>" in the middle
>> of a merge and throws away some changes.
>>
>> You should be able to do:
>>
>>   git checkout $merge^1
>>   git merge $merge^2
>>   git diff -R $merge
>>
>> to see what the original resolution did.
>>
>> -Peff
> 


^ permalink raw reply

* Re: show all merge conflicts
From: Jeff King @ 2017-01-28 14:28 UTC (permalink / raw)
  To: G. Sylvie Davies; +Cc: Michael Spiegel, git
In-Reply-To: <CAAj3zPzO4+9t9_L2OXFmkihw-HwFvzybb7GXs4tTeFRyZHOaNQ@mail.gmail.com>

On Fri, Jan 27, 2017 at 09:42:41PM -0800, G. Sylvie Davies wrote:

> Aside from the usual "git log -cc", I think this should work (replace
> HEAD with whichever commit you are analyzing):
> 
> git diff --name-only HEAD^2...HEAD^1 > m1
> git diff --name-only HEAD^1...HEAD^2 > b1
> git diff --name-only HEAD^1..HEAD    > m2
> git diff --name-only HEAD^2..HEAD    > b2
> 
> If files listed between m1 and b2 differ, then the merge is dirty.
> Similarly for m2 and b1.
> 
> More information here:
> 
> http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308

I don't think that can reliably find evil merges, since it looks at the
file level. If you had one hunk resolved for "theirs" and one hunk for
"ours" in a given file, then the file will be listed in each diff,
whether it has evil hunks or not.

I don't think this is just about evil merges, though. For instance,
try:

  seq 1 10 >file
  git add file
  git commit -m base

  sed s/4/master/ <file >tmp && mv tmp file
  git commit -am master

  git checkout -b other HEAD^
  sed s/4/other/ <file >tmp && mv tmp file
  git commit -am other

  git merge master
  git checkout --ours file
  git commit -am merged

  merge=$(git rev-parse HEAD)

The question is: were there conflicts in $merge, and how were they
resolved?

That isn't an evil merge, but there's still something interesting to
show that "git log --cc" won't display.

Replaying the merge like:

  git checkout $merge^1
  git merge $merge^2
  git diff -R $merge

shows you the patch to go from the conflict state to the final one.

-Peff

^ permalink raw reply

* Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-01-28 19:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, git, Stephan Beyer, Marc Strapetz, Johannes Schindelin
In-Reply-To: <xmqqfuk6alba.fsf@gitster.mtv.corp.google.com>

On 01/25, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Sat, Jan 21, 2017 at 08:08:02PM +0000, Thomas Gummerer wrote:
> >
> >> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> >> index 2e9cef06e6..0ad5335a3e 100644
> >> --- a/Documentation/git-stash.txt
> >> +++ b/Documentation/git-stash.txt
> >> @@ -47,8 +47,9 @@ OPTIONS
> >>  
> >>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
> >>  
> >> -	Save your local modifications to a new 'stash', and run `git reset
> >> -	--hard` to revert them.  The <message> part is optional and gives
> >> +	Save your local modifications to a new 'stash', and revert the
> >> +	the changes in the working tree to match the index.
> >> +	The <message> part is optional and gives
> >
> > Hrm. "git reset --hard" doesn't just make the working tree match the
> > index. It also resets the index to HEAD.  So either the original or your
> > new description is wrong.
> >
> > I think it's the latter. We really do reset the index unless
> > --keep-index is specified.
> 
> Correct.  So the patch is a net loss.  Perhaps not requiring readers
> to know "reset --hard" might be an improvement (I personally do not
> think so), but this loses crucial information from the description.
> 
> 	Save your local modifications (both in the working tree and
> 	to the index) to a new 'stash', and resets the index to HEAD
> 	and makes the working tree match the index (i.e. runs "git
> 	reset --hard").
> 
> That's one-and-a-half lines longer than the original, though.

Thanks all who chimed in here.  My new description is definitely not
right.  The reason I wanted to change it is part because it's an
implementation detail, and part because it's going to be not quite
right when the filename argument is introduced.

How about the following:

 	Save your local modifications to a new 'stash' and roll them back
 	both in the working tree and in the index.

As an added bonus this also matches what git stash save -p does.

^ permalink raw reply

* [PATCH] t0001: don't let a default ACL interfere with the umask test
From: Matt McCutchen @ 2017-01-28 20:25 UTC (permalink / raw)
  To: git

The "init creates a new deep directory (umask vs. shared)" test expects
the permissions of newly created files to be based on the umask, which
fails if a default ACL is inherited from the working tree for git.  So
attempt to remove a default ACL if there is one.  Same idea as
8ed0a740dd42bd0724aebed6e3b07c4ea2a2d5e8.  (I guess I'm the only one who
ever runs the test suite with a default ACL set.)

Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
 t/t0001-init.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index b8fc588..e424de5 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -258,6 +258,9 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar
 	(
 		# Leading directories should honor umask while
 		# the repository itself should follow "shared"
+		mkdir newdir &&
+		# Remove a default ACL if possible.
+		(setfacl -k newdir 2>/dev/null || true) &&
 		umask 002 &&
 		git init --bare --shared=0660 newdir/a/b/c &&
 		test_path_is_dir newdir/a/b/c/refs &&
-- 
2.9.3



^ permalink raw reply related

* [PATCH] merge-recursive: make "CONFLICT (rename/delete)" message show both paths
From: Matt McCutchen @ 2017-01-28 20:37 UTC (permalink / raw)
  To: git

The current message printed by "git merge-recursive" for a rename/delete
conflict is like this:

CONFLICT (rename/delete): new-path deleted in HEAD and renamed in
other-branch. Version other-branch of new-path left in tree.

To be more helpful, the message should show both paths of the rename and
state that the deletion occurred at the old path, not the new path.  So
change the message to the following format:

CONFLICT (rename/delete): old-path deleted in HEAD and renamed to
new-path in other-branch. Version other-branch of new-path left in tree.

Since this doubles the number of cases in handle_change_delete (modify vs.
rename), refactor the code to halve the number of cases again by merging the
cases where o->branch1 has the change and o->branch2 has the delete with the
cases that are the other way around.

Also add a simple test of the new conflict message.

Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---

This came up at:

https://github.com/cristibalan/braid/issues/41#issuecomment-275826716

Please check that my refactoring is indeed correct!  All the existing tests pass
for me, but the existing test coverage of these conflict messages looks poor.

 merge-recursive.c              | 117 ++++++++++++++++++++++-------------------
 t/t6045-merge-rename-delete.sh |  23 ++++++++
 2 files changed, 86 insertions(+), 54 deletions(-)
 create mode 100755 t/t6045-merge-rename-delete.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index d327209..e8fce10 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1061,16 +1061,20 @@ static int merge_file_one(struct merge_options *o,
 }
 
 static int handle_change_delete(struct merge_options *o,
-				 const char *path,
+				 const char *path, const char *old_path,
 				 const struct object_id *o_oid, int o_mode,
-				 const struct object_id *a_oid, int a_mode,
-				 const struct object_id *b_oid, int b_mode,
+				 const struct object_id *changed_oid,
+				 int changed_mode,
+				 const char *change_branch,
+				 const char *delete_branch,
 				 const char *change, const char *change_past)
 {
-	char *renamed = NULL;
+	char *alt_path = NULL;
+	const char *update_path = path;
 	int ret = 0;
+
 	if (dir_in_way(path, !o->call_depth, 0)) {
-		renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
+		update_path = alt_path = unique_path(o, path, change_branch);
 	}
 
 	if (o->call_depth) {
@@ -1081,43 +1085,43 @@ static int handle_change_delete(struct merge_options *o,
 		 */
 		ret = remove_file_from_cache(path);
 		if (!ret)
-			ret = update_file(o, 0, o_oid, o_mode,
-					  renamed ? renamed : path);
-	} else if (!a_oid) {
-		if (!renamed) {
-			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-			       "and %s in %s. Version %s of %s left in tree."),
-			       change, path, o->branch1, change_past,
-			       o->branch2, o->branch2, path);
-			ret = update_file(o, 0, b_oid, b_mode, path);
-		} else {
-			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-			       "and %s in %s. Version %s of %s left in tree at %s."),
-			       change, path, o->branch1, change_past,
-			       o->branch2, o->branch2, path, renamed);
-			ret = update_file(o, 0, b_oid, b_mode, renamed);
-		}
+			ret = update_file(o, 0, o_oid, o_mode, update_path);
 	} else {
-		if (!renamed) {
-			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-			       "and %s in %s. Version %s of %s left in tree."),
-			       change, path, o->branch2, change_past,
-			       o->branch1, o->branch1, path);
+		if (!alt_path) {
+			if (!old_path) {
+				output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+				       "and %s in %s. Version %s of %s left in tree."),
+				       change, path, delete_branch, change_past,
+				       change_branch, change_branch, path);
+			} else {
+				output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+				       "and %s to %s in %s. Version %s of %s left in tree."),
+				       change, old_path, delete_branch, change_past, path,
+				       change_branch, change_branch, path);
+			}
 		} else {
-			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-			       "and %s in %s. Version %s of %s left in tree at %s."),
-			       change, path, o->branch2, change_past,
-			       o->branch1, o->branch1, path, renamed);
-			ret = update_file(o, 0, a_oid, a_mode, renamed);
+			if (!old_path) {
+				output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+				       "and %s in %s. Version %s of %s left in tree at %s."),
+				       change, path, delete_branch, change_past,
+				       change_branch, change_branch, path, alt_path);
+			} else {
+				output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+				       "and %s to %s in %s. Version %s of %s left in tree at %s."),
+				       change, old_path, delete_branch, change_past, path,
+				       change_branch, change_branch, path, alt_path);
+			}
 		}
 		/*
-		 * No need to call update_file() on path when !renamed, since
-		 * that would needlessly touch path.  We could call
-		 * update_file_flags() with update_cache=0 and update_wd=0,
-		 * but that's a no-op.
+		 * No need to call update_file() on path when change_branch ==
+		 * o->branch1 && !alt_path, since that would needlessly touch
+		 * path.  We could call update_file_flags() with update_cache=0
+		 * and update_wd=0, but that's a no-op.
 		 */
+		if (change_branch != o->branch1 || alt_path)
+			ret = update_file(o, 0, changed_oid, changed_mode, update_path);
 	}
-	free(renamed);
+	free(alt_path);
 
 	return ret;
 }
@@ -1125,28 +1129,17 @@ static int handle_change_delete(struct merge_options *o,
 static int conflict_rename_delete(struct merge_options *o,
 				   struct diff_filepair *pair,
 				   const char *rename_branch,
-				   const char *other_branch)
+				   const char *delete_branch)
 {
 	const struct diff_filespec *orig = pair->one;
 	const struct diff_filespec *dest = pair->two;
-	const struct object_id *a_oid = NULL;
-	const struct object_id *b_oid = NULL;
-	int a_mode = 0;
-	int b_mode = 0;
-
-	if (rename_branch == o->branch1) {
-		a_oid = &dest->oid;
-		a_mode = dest->mode;
-	} else {
-		b_oid = &dest->oid;
-		b_mode = dest->mode;
-	}
 
 	if (handle_change_delete(o,
 				 o->call_depth ? orig->path : dest->path,
+				 o->call_depth ? NULL : orig->path,
 				 &orig->oid, orig->mode,
-				 a_oid, a_mode,
-				 b_oid, b_mode,
+				 &dest->oid, dest->mode,
+				 rename_branch, delete_branch,
 				 _("rename"), _("renamed")))
 		return -1;
 
@@ -1665,11 +1658,27 @@ static int handle_modify_delete(struct merge_options *o,
 				 struct object_id *a_oid, int a_mode,
 				 struct object_id *b_oid, int b_mode)
 {
+	const char *modify_branch, *delete_branch;
+	struct object_id *changed_oid;
+	int changed_mode;
+
+	if (a_oid) {
+		modify_branch = o->branch1;
+		delete_branch = o->branch2;
+		changed_oid = a_oid;
+		changed_mode = a_mode;
+	} else {
+		modify_branch = o->branch2;
+		delete_branch = o->branch1;
+		changed_oid = b_oid;
+		changed_mode = b_mode;
+	}
+
 	return handle_change_delete(o,
-				    path,
+				    path, NULL,
 				    o_oid, o_mode,
-				    a_oid, a_mode,
-				    b_oid, b_mode,
+				    changed_oid, changed_mode,
+				    modify_branch, delete_branch,
 				    _("modify"), _("modified"));
 }
 
diff --git a/t/t6045-merge-rename-delete.sh b/t/t6045-merge-rename-delete.sh
new file mode 100755
index 0000000..8f33244
--- /dev/null
+++ b/t/t6045-merge-rename-delete.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description='Merge-recursive rename/delete conflict message'
+. ./test-lib.sh
+
+test_expect_success 'rename/delete' '
+echo foo >A &&
+git add A &&
+git commit -m "initial" &&
+
+git checkout -b rename &&
+git mv A B &&
+git commit -m "rename" &&
+
+git checkout master &&
+git rm A &&
+git commit -m "delete" &&
+
+test_must_fail git merge --strategy=recursive rename >output &&
+test_i18ngrep "CONFLICT (rename/delete): A deleted in HEAD and renamed to B in rename. Version rename of B left in tree." output
+'
+
+test_done
-- 
2.9.3



^ permalink raw reply related

* [PATCH 0/5] introduce SWAP macro
From: René Scharfe @ 2017-01-28 21:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin

Exchanging the value of two variables requires declaring a temporary
variable and repeating their names.  The swap macro in apply.c
simplifies this for its callers without changing the compiled binary.
Polish this macro and export it, then use it throughout the code to
reduce repetition and hide the declaration of temporary variables.

  add SWAP macro
  apply: use SWAP macro
  use SWAP macro
  diff: use SWAP macro
  graph: use SWAP macro

 apply.c                       | 23 +++++++----------------
 builtin/diff-tree.c           |  4 +---
 builtin/diff.c                |  9 +++------
 contrib/coccinelle/swap.cocci | 28 ++++++++++++++++++++++++++++
 diff-no-index.c               |  6 ++----
 diff.c                        | 12 ++++--------
 git-compat-util.h             | 10 ++++++++++
 graph.c                       | 11 ++---------
 line-range.c                  |  3 +--
 merge-recursive.c             |  5 +----
 object.c                      |  4 +---
 pack-revindex.c               |  5 +----
 prio-queue.c                  |  4 +---
 strbuf.h                      |  4 +---
 14 files changed, 63 insertions(+), 65 deletions(-)
 create mode 100644 contrib/coccinelle/swap.cocci

-- 
2.11.0


^ permalink raw reply

* [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-28 21:38 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>

Add a macro for exchanging the values of variables.  It allows users
to avoid repetition and takes care of the temporary variable for them.
It also makes sure that the storage sizes of its two parameters are the
same.  Its memcpy(1) calls are optimized away by current compilers.

Also add a conservative semantic patch for transforming only swaps of
variables of the same type.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 contrib/coccinelle/swap.cocci | 28 ++++++++++++++++++++++++++++
 git-compat-util.h             | 10 ++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 contrib/coccinelle/swap.cocci

diff --git a/contrib/coccinelle/swap.cocci b/contrib/coccinelle/swap.cocci
new file mode 100644
index 0000000000..a0934d1fda
--- /dev/null
+++ b/contrib/coccinelle/swap.cocci
@@ -0,0 +1,28 @@
+@ swap_with_declaration @
+type T;
+identifier tmp;
+T a, b;
+@@
+- T tmp = a;
++ T tmp;
++ tmp = a;
+  a = b;
+  b = tmp;
+
+@ swap @
+type T;
+T tmp, a, b;
+@@
+- tmp = a;
+- a = b;
+- b = tmp;
++ SWAP(a, b);
+
+@ extends swap @
+identifier unused;
+@@
+  {
+  ...
+- T unused;
+  ... when != unused
+  }
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092b..66cd466eea 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -527,6 +527,16 @@ static inline int ends_with(const char *str, const char *suffix)
 	return strip_suffix(str, suffix, &len);
 }
 
+#define SWAP(a, b) do {						\
+	void *_swap_a_ptr = &(a);				\
+	void *_swap_b_ptr = &(b);				\
+	unsigned char _swap_buffer[sizeof(a)];			\
+	memcpy(_swap_buffer, _swap_a_ptr, sizeof(a));		\
+	memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) +		\
+	       BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b)));	\
+	memcpy(_swap_b_ptr, _swap_buffer, sizeof(a));		\
+} while (0)
+
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
 
 #ifndef PROT_READ
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/5] apply: use SWAP macro
From: René Scharfe @ 2017-01-28 21:40 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>

Use the exported macro SWAP instead of the file-scoped macro swap and
remove the latter's definition.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 apply.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/apply.c b/apply.c
index 2ed808d429..0e2caeab9c 100644
--- a/apply.c
+++ b/apply.c
@@ -2187,29 +2187,20 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
 	return offset + hdrsize + patchsize;
 }
 
-#define swap(a,b) myswap((a),(b),sizeof(a))
-
-#define myswap(a, b, size) do {		\
-	unsigned char mytmp[size];	\
-	memcpy(mytmp, &a, size);		\
-	memcpy(&a, &b, size);		\
-	memcpy(&b, mytmp, size);		\
-} while (0)
-
 static void reverse_patches(struct patch *p)
 {
 	for (; p; p = p->next) {
 		struct fragment *frag = p->fragments;
 
-		swap(p->new_name, p->old_name);
-		swap(p->new_mode, p->old_mode);
-		swap(p->is_new, p->is_delete);
-		swap(p->lines_added, p->lines_deleted);
-		swap(p->old_sha1_prefix, p->new_sha1_prefix);
+		SWAP(p->new_name, p->old_name);
+		SWAP(p->new_mode, p->old_mode);
+		SWAP(p->is_new, p->is_delete);
+		SWAP(p->lines_added, p->lines_deleted);
+		SWAP(p->old_sha1_prefix, p->new_sha1_prefix);
 
 		for (; frag; frag = frag->next) {
-			swap(frag->newpos, frag->oldpos);
-			swap(frag->newlines, frag->oldlines);
+			SWAP(frag->newpos, frag->oldpos);
+			SWAP(frag->newlines, frag->oldlines);
 		}
 	}
 }
-- 
2.11.0


^ permalink raw reply related

* [PATCH 3/5] use SWAP macro
From: René Scharfe @ 2017-01-28 21:40 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>

Apply the semantic patch swap.cocci to convert hand-rolled swaps to use
the macro SWAP.  The resulting code is shorter and easier to read, the
object code is effectively unchanged.

The patch for object.c had to be hand-edited in order to preserve the
comment before the change; Coccinelle tried to eat it for some reason.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 builtin/diff-tree.c | 4 +---
 builtin/diff.c      | 9 +++------
 diff-no-index.c     | 3 +--
 diff.c              | 8 +++-----
 graph.c             | 5 +----
 line-range.c        | 3 +--
 merge-recursive.c   | 5 +----
 object.c            | 4 +---
 pack-revindex.c     | 5 +----
 prio-queue.c        | 4 +---
 strbuf.h            | 4 +---
 11 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 806dd7a885..8ce00480cd 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -147,9 +147,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 		tree1 = opt->pending.objects[0].item;
 		tree2 = opt->pending.objects[1].item;
 		if (tree2->flags & UNINTERESTING) {
-			struct object *tmp = tree2;
-			tree2 = tree1;
-			tree1 = tmp;
+			SWAP(tree2, tree1);
 		}
 		diff_tree_sha1(tree1->oid.hash,
 			       tree2->oid.hash,
diff --git a/builtin/diff.c b/builtin/diff.c
index 7f91f6d226..3d64b85337 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -45,12 +45,9 @@ static void stuff_change(struct diff_options *opt,
 		return;
 
 	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
-		unsigned tmp;
-		const unsigned char *tmp_u;
-		const char *tmp_c;
-		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
-		tmp_u = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_u;
-		tmp_c = old_name; old_name = new_name; new_name = tmp_c;
+		SWAP(old_mode, new_mode);
+		SWAP(old_sha1, new_sha1);
+		SWAP(old_name, new_name);
 	}
 
 	if (opt->prefix &&
diff --git a/diff-no-index.c b/diff-no-index.c
index f420786039..1ae09894d7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o,
 
 		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 			unsigned tmp;
-			const char *tmp_c;
 			tmp = mode1; mode1 = mode2; mode2 = tmp;
-			tmp_c = name1; name1 = name2; name2 = tmp_c;
+			SWAP(name1, name2);
 		}
 
 		d1 = noindex_filespec(name1, mode1);
diff --git a/diff.c b/diff.c
index f08cd8e033..9de1ba264f 100644
--- a/diff.c
+++ b/diff.c
@@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options,
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		unsigned tmp;
-		const unsigned char *tmp_c;
-		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
-		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
+		SWAP(old_mode, new_mode);
+		SWAP(old_sha1, new_sha1);
 		tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
 			new_sha1_valid = tmp;
-		tmp = old_dirty_submodule; old_dirty_submodule = new_dirty_submodule;
-			new_dirty_submodule = tmp;
+		SWAP(old_dirty_submodule, new_dirty_submodule);
 	}
 
 	if (options->prefix &&
diff --git a/graph.c b/graph.c
index d4e8519c90..4c722303d2 100644
--- a/graph.c
+++ b/graph.c
@@ -997,7 +997,6 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf
 static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf *sb)
 {
 	int i;
-	int *tmp_mapping;
 	short used_horizontal = 0;
 	int horizontal_edge = -1;
 	int horizontal_edge_target = -1;
@@ -1132,9 +1131,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf
 	/*
 	 * Swap mapping and new_mapping
 	 */
-	tmp_mapping = graph->mapping;
-	graph->mapping = graph->new_mapping;
-	graph->new_mapping = tmp_mapping;
+	SWAP(graph->mapping, graph->new_mapping);
 
 	/*
 	 * If graph->mapping indicates that all of the branch lines
diff --git a/line-range.c b/line-range.c
index de4e32f942..323399d16c 100644
--- a/line-range.c
+++ b/line-range.c
@@ -269,8 +269,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
 		return -1;
 
 	if (*begin && *end && *end < *begin) {
-		long tmp;
-		tmp = *end; *end = *begin; *begin = tmp;
+		SWAP(*end, *begin);
 	}
 
 	return 0;
diff --git a/merge-recursive.c b/merge-recursive.c
index d327209443..b7ff1ada3c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1390,14 +1390,11 @@ static int process_renames(struct merge_options *o,
 			branch1 = o->branch1;
 			branch2 = o->branch2;
 		} else {
-			struct rename *tmp;
 			renames1 = b_renames;
 			renames2Dst = &a_by_dst;
 			branch1 = o->branch2;
 			branch2 = o->branch1;
-			tmp = ren2;
-			ren2 = ren1;
-			ren1 = tmp;
+			SWAP(ren2, ren1);
 		}
 
 		if (ren1->processed)
diff --git a/object.c b/object.c
index 67d9a9e221..e680d881a4 100644
--- a/object.c
+++ b/object.c
@@ -104,9 +104,7 @@ struct object *lookup_object(const unsigned char *sha1)
 		 * that we do not need to walk the hash table the next
 		 * time we look for it.
 		 */
-		struct object *tmp = obj_hash[i];
-		obj_hash[i] = obj_hash[first];
-		obj_hash[first] = tmp;
+		SWAP(obj_hash[i], obj_hash[first]);
 	}
 	return obj;
 }
diff --git a/pack-revindex.c b/pack-revindex.c
index 6bc7c94033..1b7ebd8d7e 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -59,7 +59,6 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
 	 * be a no-op, as everybody lands in the same zero-th bucket.
 	 */
 	for (bits = 0; max >> bits; bits += DIGIT_SIZE) {
-		struct revindex_entry *swap;
 		unsigned i;
 
 		memset(pos, 0, BUCKETS * sizeof(*pos));
@@ -97,9 +96,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
 		 * Now "to" contains the most sorted list, so we swap "from" and
 		 * "to" for the next iteration.
 		 */
-		swap = from;
-		from = to;
-		to = swap;
+		SWAP(from, to);
 	}
 
 	/*
diff --git a/prio-queue.c b/prio-queue.c
index e4365b00d6..17252d231b 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -12,9 +12,7 @@ static inline int compare(struct prio_queue *queue, int i, int j)
 
 static inline void swap(struct prio_queue *queue, int i, int j)
 {
-	struct prio_queue_entry tmp = queue->array[i];
-	queue->array[i] = queue->array[j];
-	queue->array[j] = tmp;
+	SWAP(queue->array[i], queue->array[j]);
 }
 
 void prio_queue_reverse(struct prio_queue *queue)
diff --git a/strbuf.h b/strbuf.h
index 2262b12683..cf1b5409e7 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -109,9 +109,7 @@ extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);
  */
 static inline void strbuf_swap(struct strbuf *a, struct strbuf *b)
 {
-	struct strbuf tmp = *a;
-	*a = *b;
-	*b = tmp;
+	SWAP(*a, *b);
 }
 
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 4/5] diff: use SWAP macro
From: René Scharfe @ 2017-01-28 21:41 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>

Use the macro SWAP to exchange the value of pairs of variables instead
of swapping them manually with the help of a temporary variable.  The
resulting code is shorter and easier to read.

The two cases were not transformed by the semantic patch swap.cocci
because it's extra careful and handles only cases where the types of all
variables are the same -- and here we swap two ints and use an unsigned
temporary variable for that.  Nevertheless the conversion is safe, as
the value range is preserved with and without the patch.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 diff-no-index.c | 3 +--
 diff.c          | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 1ae09894d7..df762fd0f7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -185,8 +185,7 @@ static int queue_diff(struct diff_options *o,
 		struct diff_filespec *d1, *d2;
 
 		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
-			unsigned tmp;
-			tmp = mode1; mode1 = mode2; mode2 = tmp;
+			SWAP(mode1, mode2);
 			SWAP(name1, name2);
 		}
 
diff --git a/diff.c b/diff.c
index 9de1ba264f..6c4f3f6b72 100644
--- a/diff.c
+++ b/diff.c
@@ -5117,11 +5117,9 @@ void diff_change(struct diff_options *options,
 		return;
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
-		unsigned tmp;
 		SWAP(old_mode, new_mode);
 		SWAP(old_sha1, new_sha1);
-		tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
-			new_sha1_valid = tmp;
+		SWAP(old_sha1_valid, new_sha1_valid);
 		SWAP(old_dirty_submodule, new_dirty_submodule);
 	}
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 5/5] graph: use SWAP macro
From: René Scharfe @ 2017-01-28 21:42 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>

Exchange the values of graph->columns and graph->new_columns using the
macro SWAP instead of hand-rolled code.  The result is shorter and
easier to read.

This transformation was not done by the semantic patch swap.cocci
because there's an unrelated statement between the second and the last
step of the exchange, so it didn't match the expected pattern.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 graph.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/graph.c b/graph.c
index 4c722303d2..29b0f51dc5 100644
--- a/graph.c
+++ b/graph.c
@@ -463,7 +463,6 @@ static void graph_update_width(struct git_graph *graph,
 static void graph_update_columns(struct git_graph *graph)
 {
 	struct commit_list *parent;
-	struct column *tmp_columns;
 	int max_new_columns;
 	int mapping_idx;
 	int i, seen_this, is_commit_in_columns;
@@ -476,11 +475,8 @@ static void graph_update_columns(struct git_graph *graph)
 	 * We'll re-use the old columns array as storage to compute the new
 	 * columns list for the commit after this one.
 	 */
-	tmp_columns = graph->columns;
-	graph->columns = graph->new_columns;
+	SWAP(graph->columns, graph->new_columns);
 	graph->num_columns = graph->num_new_columns;
-
-	graph->new_columns = tmp_columns;
 	graph->num_new_columns = 0;
 
 	/*
-- 
2.11.0


^ permalink raw reply related

* [PATCH] use oidcpy() for copying hashes between instances of struct object_id
From: René Scharfe @ 2017-01-28 22:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, brian m. carlson

Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 refs/files-backend.c | 2 +-
 wt-status.c          | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f9023939d5..8ee2aba39f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -697,7 +697,7 @@ static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,
 
 	if (peel_entry(entry, 0))
 		return -1;
-	hashcpy(peeled->hash, entry->u.value.peeled.hash);
+	oidcpy(peeled, &entry->u.value.peeled);
 	return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index a715e71906..a05328dc48 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -628,7 +628,7 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
 			d->index_status = DIFF_STATUS_ADDED;
 			/* Leave {mode,oid}_head zero for adds. */
 			d->mode_index = ce->ce_mode;
-			hashcpy(d->oid_index.hash, ce->oid.hash);
+			oidcpy(&d->oid_index, &ce->oid);
 		}
 	}
 }
@@ -2096,7 +2096,7 @@ static void wt_porcelain_v2_print_unmerged_entry(
 		if (strcmp(ce->name, it->string) || !stage)
 			break;
 		stages[stage - 1].mode = ce->ce_mode;
-		hashcpy(stages[stage - 1].oid.hash, ce->oid.hash);
+		oidcpy(&stages[stage - 1].oid, &ce->oid);
 		sum |= (1 << (stage - 1));
 	}
 	if (sum != d->stagemask)
-- 
2.11.0


^ permalink raw reply related

* [PATCH] checkout: convert post_checkout_hook() to struct object_id
From: René Scharfe @ 2017-01-28 22:14 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, brian m. carlson

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 builtin/checkout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c198..80d5e38981 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -56,8 +56,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
 			      int changed)
 {
 	return run_hook_le(NULL, "post-checkout",
-			   sha1_to_hex(old ? old->object.oid.hash : null_sha1),
-			   sha1_to_hex(new ? new->object.oid.hash : null_sha1),
+			   oid_to_hex(old ? &old->object.oid : &null_oid),
+			   oid_to_hex(new ? &new->object.oid : &null_oid),
 			   changed ? "1" : "0", NULL);
 	/* "new" can be NULL when checking out from the index before
 	   a commit exists. */
-- 
2.11.0


^ permalink raw reply related

* [PATCH] use oid_to_hex_r() for converting struct object_id hashes to hex strings
From: René Scharfe @ 2017-01-28 22:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, brian m. carlson

Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 builtin/blame.c       | 4 ++--
 builtin/merge-index.c | 2 +-
 builtin/rev-list.c    | 2 +-
 diff.c                | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5b..cffc626540 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1901,7 +1901,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 	struct origin *suspect = ent->suspect;
 	char hex[GIT_SHA1_HEXSZ + 1];
 
-	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+	oid_to_hex_r(hex, &suspect->commit->object.oid);
 	printf("%s %d %d %d\n",
 	       hex,
 	       ent->s_lno + 1,
@@ -1941,7 +1941,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 
 	get_commit_info(suspect->commit, &ci, 1);
-	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+	oid_to_hex_r(hex, &suspect->commit->object.oid);
 
 	cp = nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index ce356b1da1..2d1b6db6bd 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -22,7 +22,7 @@ static int merge_entry(int pos, const char *path)
 		if (strcmp(ce->name, path))
 			break;
 		found++;
-		sha1_to_hex_r(hexbuf[stage], ce->oid.hash);
+		oid_to_hex_r(hexbuf[stage], &ce->oid);
 		xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
 		arguments[stage] = hexbuf[stage];
 		arguments[stage + 4] = ownbuf[stage];
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index c43decda70..0aa93d5891 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -237,7 +237,7 @@ static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
 		cnt = reaches;
 
 	if (revs->commits)
-		sha1_to_hex_r(hex, revs->commits->item->object.oid.hash);
+		oid_to_hex_r(hex, &revs->commits->item->object.oid);
 
 	if (flags & BISECT_SHOW_ALL) {
 		traverse_commit_list(revs, show_commit, show_object, info);
diff --git a/diff.c b/diff.c
index f08cd8e033..d91a344908 100644
--- a/diff.c
+++ b/diff.c
@@ -3015,7 +3015,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
 			if (!one->oid_valid)
 				sha1_to_hex_r(temp->hex, null_sha1);
 			else
-				sha1_to_hex_r(temp->hex, one->oid.hash);
+				oid_to_hex_r(temp->hex, &one->oid);
 			/* Even though we may sometimes borrow the
 			 * contents from the work tree, we always want
 			 * one->mode.  mode is trustworthy even when
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Jeff King @ 2017-01-28 23:54 UTC (permalink / raw)
  To: Thomas Gummerer
  Cc: Junio C Hamano, git, Stephan Beyer, Marc Strapetz,
	Johannes Schindelin
In-Reply-To: <20170128192958.GB31189@hank>

On Sat, Jan 28, 2017 at 07:30:28PM +0000, Thomas Gummerer wrote:

> Thanks all who chimed in here.  My new description is definitely not
> right.  The reason I wanted to change it is part because it's an
> implementation detail, and part because it's going to be not quite
> right when the filename argument is introduced.
> 
> How about the following:
> 
>  	Save your local modifications to a new 'stash' and roll them back
>  	both in the working tree and in the index.
> 
> As an added bonus this also matches what git stash save -p does.

IMHO that is both informative and accurate. You could add:
 
  (unless --keep-index was used)

at the end of the sentence, though I am not sure it is necessary.

-Peff

^ permalink raw reply

* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Stefan Beller @ 2017-01-28 23:50 UTC (permalink / raw)
  To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170128020207.179015-17-bmwill@google.com>

On Fri, Jan 27, 2017 at 6:01 PM, Brandon Williams <bmwill@google.com> wrote:
> From: Junio C Hamano <gitster@pobox.com>
>
> This updates the other two ways the attribute check is done via an
> array of "struct attr_check_item" elements.  These two niches
> appear only in "git check-attr".
>
>  * The caller does not know offhand what attributes it wants to ask
>    about and cannot use attr_check_initl() to prepare the
>    attr_check structure.
>
>  * The caller may not know what attributes it wants to ask at all,
>    and instead wants to learn everything that the given path has.
>
> Such a caller can call attr_check_alloc() to allocate an empty
> attr_check, and then call attr_check_append() to add attribute names
> one by one.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  attr.c               | 168 ++++++++++++++++++++++++---------------------------
>  attr.h               |   9 +--
>  builtin/check-attr.c |  60 +++++++++---------
>  3 files changed, 112 insertions(+), 125 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index de8bf35a3..40818246f 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -132,75 +132,6 @@ struct git_attr *git_attr(const char *name)
>         return git_attr_internal(name, strlen(name));
>  }
>
> -struct attr_check *attr_check_alloc(void)
> -{
> -       return xcalloc(1, sizeof(struct attr_check));
> -}
> -
> -struct attr_check *attr_check_initl(const char *one, ...)
> -{
> -       struct attr_check *check;
> -       int cnt;
> -       va_list params;
> -       const char *param;
> -
> -       va_start(params, one);
> -       for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> -               ;
> -       va_end(params);
> -
> -       check = attr_check_alloc();
> -       check->nr = cnt;
> -       check->alloc = cnt;
> -       check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> -
> -       check->items[0].attr = git_attr(one);
> -       va_start(params, one);
> -       for (cnt = 1; cnt < check->nr; cnt++) {
> -               const struct git_attr *attr;
> -               param = va_arg(params, const char *);
> -               if (!param)
> -                       die("BUG: counted %d != ended at %d",
> -                           check->nr, cnt);
> -               attr = git_attr(param);
> -               if (!attr)
> -                       die("BUG: %s: not a valid attribute name", param);
> -               check->items[cnt].attr = attr;
> -       }
> -       va_end(params);
> -       return check;
> -}

This being moved down to below (being review churn) sounds like a
rebase mistake. ;)

> -
> -struct attr_check_item *attr_check_append(struct attr_check *check,
> -                                         const struct git_attr *attr)
> -{
> -       struct attr_check_item *item;
> -
> -       ALLOC_GROW(check->items, check->nr + 1, check->alloc);
> -       item = &check->items[check->nr++];
> -       item->attr = attr;
> -       return item;
> -}
> -
> -void attr_check_reset(struct attr_check *check)
> -{
> -       check->nr = 0;
> -}
> -
> -void attr_check_clear(struct attr_check *check)
> -{
> -       free(check->items);
> -       check->items = NULL;
> -       check->alloc = 0;
> -       check->nr = 0;
> -}
> -
> -void attr_check_free(struct attr_check *check)
> -{
> -       attr_check_clear(check);
> -       free(check);
> -}
> -
>  /* What does a matched pattern decide? */
>  struct attr_state {
>         struct git_attr *attr;
> @@ -439,6 +370,75 @@ static void free_attr_elem(struct attr_stack *e)
>         free(e);
>  }
>
> +struct attr_check *attr_check_alloc(void)
> +{
> +       return xcalloc(1, sizeof(struct attr_check));
> +}
> +
> +struct attr_check *attr_check_initl(const char *one, ...)
> +{
> +       struct attr_check *check;
> +       int cnt;
> +       va_list params;
> +       const char *param;
> +
> +       va_start(params, one);
> +       for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> +               ;
> +       va_end(params);
> +
> +       check = attr_check_alloc();
> +       check->nr = cnt;
> +       check->alloc = cnt;
> +       check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> +
> +       check->items[0].attr = git_attr(one);
> +       va_start(params, one);
> +       for (cnt = 1; cnt < check->nr; cnt++) {
> +               const struct git_attr *attr;
> +               param = va_arg(params, const char *);
> +               if (!param)
> +                       die("BUG: counted %d != ended at %d",
> +                           check->nr, cnt);
> +               attr = git_attr(param);
> +               if (!attr)
> +                       die("BUG: %s: not a valid attribute name", param);
> +               check->items[cnt].attr = attr;
> +       }
> +       va_end(params);
> +       return check;
> +}
> +
> +struct attr_check_item *attr_check_append(struct attr_check *check,
> +                                         const struct git_attr *attr)
> +{
> +       struct attr_check_item *item;
> +
> +       ALLOC_GROW(check->items, check->nr + 1, check->alloc);
> +       item = &check->items[check->nr++];
> +       item->attr = attr;
> +       return item;
> +}
> +
> +void attr_check_reset(struct attr_check *check)
> +{
> +       check->nr = 0;
> +}
> +
> +void attr_check_clear(struct attr_check *check)
> +{
> +       free(check->items);
> +       check->items = NULL;
> +       check->alloc = 0;
> +       check->nr = 0;
> +}
> +
> +void attr_check_free(struct attr_check *check)
> +{
> +       attr_check_clear(check);
> +       free(check);
> +}
> +
>  static const char *builtin_attr[] = {
>         "[attr]binary -diff -merge -text",
>         NULL,
> @@ -906,32 +906,22 @@ int git_check_attrs(const char *path, int num, struct attr_check_item *check)
>         return 0;
>  }

Below is where the actual change for this patch starts?

>
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check)
> +void git_all_attrs(const char *path, struct attr_check *check)
>  {
> -       int i, count, j;
> +       int i;
>
> -       collect_some_attrs(path, 0, NULL);
> +       attr_check_reset(check);
> +       collect_some_attrs(path, check->nr, check->items);
>
> -       /* Count the number of attributes that are set. */
> -       count = 0;
> -       for (i = 0; i < attr_nr; i++) {
> -               const char *value = check_all_attr[i].value;
> -               if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
> -                       ++count;
> -       }
> -       *num = count;
> -       ALLOC_ARRAY(*check, count);
> -       j = 0;
>         for (i = 0; i < attr_nr; i++) {
> +               const char *name = check_all_attr[i].attr->name;
>                 const char *value = check_all_attr[i].value;
> -               if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
> -                       (*check)[j].attr = check_all_attr[i].attr;
> -                       (*check)[j].value = value;
> -                       ++j;
> -               }
> +               struct attr_check_item *item;
> +               if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
> +                       continue;
> +               item = attr_check_append(check, git_attr(name));
> +               item->value = value;
>         }
> -
> -       return 0;
>  }
>
>  int git_check_attr(const char *path, struct attr_check *check)
> diff --git a/attr.h b/attr.h
> index e611b139a..9f2729842 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -56,13 +56,10 @@ int git_check_attrs(const char *path, int, struct attr_check_item *);
>  extern int git_check_attr(const char *path, struct attr_check *check);
>
>  /*
> - * Retrieve all attributes that apply to the specified path.  *num
> - * will be set to the number of attributes on the path; **check will
> - * be set to point at a newly-allocated array of git_attr_check
> - * objects describing the attributes and their values.  *check must be
> - * free()ed by the caller.
> + * Retrieve all attributes that apply to the specified path.
> + * check holds the attributes and their values.
>   */
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check);
> +extern void git_all_attrs(const char *path, struct attr_check *check);
>
>  enum git_attr_direction {
>         GIT_ATTR_CHECKIN,
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index 889264a5b..40cdff13e 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
>         OPT_END()
>  };
>
> -static void output_attr(int cnt, struct attr_check_item *check,
> -                       const char *file)
> +static void output_attr(struct attr_check *check, const char *file)
>  {
>         int j;
> +       int cnt = check->nr;
> +
>         for (j = 0; j < cnt; j++) {
> -               const char *value = check[j].value;
> +               const char *value = check->items[j].value;
>
>                 if (ATTR_TRUE(value))
>                         value = "set";
> @@ -42,36 +43,38 @@ static void output_attr(int cnt, struct attr_check_item *check,
>                         printf("%s%c" /* path */
>                                "%s%c" /* attrname */
>                                "%s%c" /* attrvalue */,
> -                              file, 0, git_attr_name(check[j].attr), 0, value, 0);
> +                              file, 0,
> +                              git_attr_name(check->items[j].attr), 0, value, 0);
>                 } else {
>                         quote_c_style(file, NULL, stdout, 0);
> -                       printf(": %s: %s\n", git_attr_name(check[j].attr), value);
> +                       printf(": %s: %s\n",
> +                              git_attr_name(check->items[j].attr), value);
>                 }
> -
>         }
>  }
>
>  static void check_attr(const char *prefix,
> -                      int cnt, struct attr_check_item *check,
> +                      struct attr_check *check,
> +                      int collect_all,
>                        const char *file)
>  {
>         char *full_path =
>                 prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
> -       if (check != NULL) {
> -               if (git_check_attrs(full_path, cnt, check))
> -                       die("git_check_attrs died");
> -               output_attr(cnt, check, file);
> +
> +       if (collect_all) {
> +               git_all_attrs(full_path, check);
>         } else {
> -               if (git_all_attrs(full_path, &cnt, &check))
> -                       die("git_all_attrs died");
> -               output_attr(cnt, check, file);
> -               free(check);
> +               if (git_check_attr(full_path, check))
> +                       die("git_check_attr died");
>         }
> +       output_attr(check, file);
> +
>         free(full_path);
>  }
>
>  static void check_attr_stdin_paths(const char *prefix,
> -                                  int cnt, struct attr_check_item *check)
> +                                  struct attr_check *check,
> +                                  int collect_all)
>  {
>         struct strbuf buf = STRBUF_INIT;
>         struct strbuf unquoted = STRBUF_INIT;
> @@ -85,7 +88,7 @@ static void check_attr_stdin_paths(const char *prefix,
>                                 die("line is badly quoted");
>                         strbuf_swap(&buf, &unquoted);
>                 }
> -               check_attr(prefix, cnt, check, buf.buf);
> +               check_attr(prefix, check, collect_all, buf.buf);
>                 maybe_flush_or_die(stdout, "attribute to stdout");
>         }
>         strbuf_release(&buf);
> @@ -100,7 +103,7 @@ static NORETURN void error_with_usage(const char *msg)
>
>  int cmd_check_attr(int argc, const char **argv, const char *prefix)
>  {
> -       struct attr_check_item *check;
> +       struct attr_check *check;
>         int cnt, i, doubledash, filei;
>
>         if (!is_bare_repository())
> @@ -160,28 +163,25 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
>                         error_with_usage("No file specified");
>         }
>
> -       if (all_attrs) {
> -               check = NULL;
> -       } else {
> -               check = xcalloc(cnt, sizeof(*check));
> +       check = attr_check_alloc();
> +       if (!all_attrs) {
>                 for (i = 0; i < cnt; i++) {
> -                       const char *name;
> -                       struct git_attr *a;
> -                       name = argv[i];
> -                       a = git_attr(name);
> +                       struct git_attr *a = git_attr(argv[i]);
>                         if (!a)
>                                 return error("%s: not a valid attribute name",
> -                                       name);
> -                       check[i].attr = a;
> +                                            argv[i]);
> +                       attr_check_append(check, a);
>                 }
>         }
>
>         if (stdin_paths)
> -               check_attr_stdin_paths(prefix, cnt, check);
> +               check_attr_stdin_paths(prefix, check, all_attrs);
>         else {
>                 for (i = filei; i < argc; i++)
> -                       check_attr(prefix, cnt, check, argv[i]);
> +                       check_attr(prefix, check, all_attrs, argv[i]);
>                 maybe_flush_or_die(stdout, "attribute to stdout");
>         }
> +
> +       attr_check_free(check);
>         return 0;
>  }
> --
> 2.11.0.483.g087da7b7c-goog
>

^ permalink raw reply

* git-daemon shallow checkout fail
From: Bob Proulx @ 2017-01-29  0:29 UTC (permalink / raw)
  To: git

I am trying to understand a problem with shallow checkouts through the
git-daemon.  The server side fails trying to create a shallow_XXXXXX
file in the repository.  But of course it can't due to no permissions
from the git-daemon user.

However the problem driving me crazy is that this only fails this way
on one machine.  Unfortunately failing on the machine I need to use.
If I try this same setup on any other machine I try then there is no
failure and it works okay.  Therefore I conclude that in the failing
case it is trying to write a shallow_XXXXXX file in the repository but
in all of the passing cases it does not.  I browsed through the
git-daemon source but couldn't deduce the flow yet.

Does anyone know why one system would try to create shallow_XXXXXX
files in the repository while another one would not?

Trying to be unambiguous here is my test case:

  mkdir /run/git
  chmod 755 /run/git
  chown nobody:root /run/git
  cd /run/git && env -i HOME=/run/git PATH=/usr/local/bin:/usr/bin:/bin su -s /bin/sh -c "git-daemon --export-all --base-path=/srv/git --verbose" nobody
  [18340] Ready to rumble

That sets up the test case.  Have any bare git repository in /srv/git
for use for cloning.

  git clone --depth 1 git://localhost/test-project.git
  Cloning into 'test-project'...
  fatal: The remote end hung up unexpectedly
  fatal: early EOF
  fatal: index-pack failed

And the server side says:

  [26071] Request upload-pack for '/test-project.git'
  [26071] fatal: Unable to create temporary file '/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
  [26055] [26071] Disconnected (with error)

Of course git-daemon running as nobody can't create a temporary file
shallow_XXXXXX in the /srv/git/test-project.git because it has no
permissions by design.  But why does this work on other systems and
not work on my target system?

  git --version  # from today's git clone build
  git version 2.11.0.485.g4e59582

Thanks!
Bob

^ permalink raw reply

* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Brandon Williams @ 2017-01-29  2:44 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <CAGZ79kax8Q6bD1Xm-kGAK=yTG6TYWcR3SCOjN9BeX9rzwkyaig@mail.gmail.com>

On 01/28, Stefan Beller wrote:
> 
> This being moved down to below (being review churn) sounds like a
> rebase mistake. ;)
> 

Yep, thanks for catching that.  I'll need to fix that up.

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH 2/3] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-29 12:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqziihskbi.fsf@gitster.mtv.corp.google.com>

On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > +	stash_msg="$*"
> > +
> > +	if test -z stash_msg
> 
> A dollar-sign is missing here, I think.

Yes, thanks.

> > +	then
> > +		push_stash $push_options
> > +	else
> > +		push_stash $push_options -m "$stash_msg"
> > +	fi
> > +}

-- 
Thomas

^ permalink raw reply

* [PATCH] receive-pack: call string_list_clear() unconditionally
From: René Scharfe @ 2017-01-29 13:09 UTC (permalink / raw)
  To: Git List; +Cc: Stefan Beller, Junio C Hamano

string_list_clear() handles empty lists just fine, so remove the
redundant check.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 builtin/receive-pack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6b97cbdbe..1dbb8a069 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1942,8 +1942,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		run_receive_hook(commands, "post-receive", 1,
 				 &push_options);
 		run_update_post_hook(commands);
-		if (push_options.nr)
-			string_list_clear(&push_options, 0);
+		string_list_clear(&push_options, 0);
 		if (auto_gc) {
 			const char *argv_gc_auto[] = {
 				"gc", "--auto", "--quiet", NULL,
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 3/3] stash: support filename argument
From: Thomas Gummerer @ 2017-01-29 13:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqinp5sj98.fsf@gitster.mtv.corp.google.com>

On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > diff --git a/git-stash.sh b/git-stash.sh
> > index d6b4ae3290..7dcce629bd 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -41,7 +41,7 @@ no_changes () {
> >  untracked_files () {
> >  	excl_opt=--exclude-standard
> >  	test "$untracked" = "all" && excl_opt=
> > -	git ls-files -o -z $excl_opt
> > +	git ls-files -o -z $excl_opt -- $1
> 
> Does $1 need to be quoted to prevent it from split at $IFS?

Not sure, I'll check and add a test for the re-roll.

> > @@ -56,6 +56,23 @@ clear_stash () {
> >  }
> >  
> >  create_stash () {
> > +	files=
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		--)
> > +			shift
> > +			break
> > +			;;
> > +		--files)
> > +			;;
> > +		*)
> > +			files="$1 $files"
> > +			;;
> 
> Hmph.  What is this "no-op" option about?  Did you mean to say
> something like this instead?
> 
> 	case "$1" in
> 	...
> 	--file)
> 		case $# in
> 		1)
> 			die "--file needs a pathspec" ;;
> 		*)
> 			shift
> 			files="$files$1 " ;;
> 		esac
> 		;;
>

Hmm that would require multiple --file arguments to create_stash,
which I wanted to avoid.  But probably the correct solution is to
introduce a new format for create_stash, which allows a -m before the
message, and uses -- to disambiguate before the file name arguments.

This would be similar to what Johannes suggested in [1], deprecating
the old syntax in git stash create.  While this didn't work in git
stash save, it would work in git stash create, as "--" isn't used to
disambiguate anything there yet.

> Another thing I noticed.  We won't support filenames with embedded
> $IFS characters at all?
> 
> I somehow had an impression that the script was carefully done
> (e.g. by using -z option where appropriate) to add such a
> limitation.
> 
> Perhaps we have broken it over time and it no longer matters
> (i.e. there already may be existing breakages), but this troubles
> me somehow.

Good point, I didn't think about $IFS characters.  Fill fix in the
next round.

> By the way, in addition to "push" thing that corrects the argument
> convention by requiring "-m" before the message, we need to correct
> create_stash that is used internally from "stash push" somehow?

Yeah, I think that would make sense, see above.

[1]: http://public-inbox.org/git/alpine.DEB.2.20.1701241148300.3469@virtualbox/

^ permalink raw reply

* Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows
From: Luke Diamand @ 2017-01-29 17:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lars Schneider, Git Users, George Vanburgh
In-Reply-To: <xmqqa8ac1k7o.fsf@gitster.mtv.corp.google.com>

On 27 January 2017 at 17:33, Junio C Hamano <gitster@pobox.com> wrote:
> George Vanburgh <george@vanburgh.me> writes:
>
>> From: George Vanburgh <gvanburgh@bloomberg.net>
>>
>> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
>> git config - no user mappings are applied to the generated repository.
>> ...
>> Using splitlines solves this issue, by splitting config on all
>> typical delimiters ('\n', '\r\n' etc.)
>
> Luke, Lars, this version seems to be in line with the conclusion of
> your earlier reviews, e.g.
>
> <CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>
>
> Even though it looks OK to my eyes, I'll wait for Acks or further
> refinement suggestions from either of you two before acting on this
> patch.

It looks good to me. The tests all pass, and the change looks correct.

Ack.

Luke


>
> Thanks.
>
>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
>> ---
>>  git-p4.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index f427bf6..b66f68b 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -656,7 +656,7 @@ def gitConfigInt(key):
>>  def gitConfigList(key):
>>      if not _gitConfig.has_key(key):
>>          s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
>> -        _gitConfig[key] = s.strip().split(os.linesep)
>> +        _gitConfig[key] = s.strip().splitlines()
>>          if _gitConfig[key] == ['']:
>>              _gitConfig[key] = []
>>      return _gitConfig[key]
>>
>> --
>> https://github.com/git/git/pull/319

^ permalink raw reply

* [RFC] Proof of concept: Support multiple authors
From: Cornelius Schumacher @ 2017-01-29 18:06 UTC (permalink / raw)
  To: git; +Cc: josh

This patch is a proof of concept implementation of support for
multiple authors. It adds an optional `authors` header to commits
which is set when there are authors configured in the git config.

A new command `git-authors` is used to manage the authors settings.
Authors are identified by initials and their names and emails are
set in a `.git_authors_map` file.

Signed-off-by: Cornelius Schumacher <schumacher@kde.org>
---

When doing pair programming we have to work around the limitation that
git can only have a single author in each commit. There are some tools
which help with that such as [git-duet] [1], but there are still some
limits, because the information about multiple authors is not reflected
in the native git data model.

Here is a proposal how to change that and implement native support for
multiple authors in git. It comes with a patch as a proof of concept.
The patch by no means is finished, it doesn't cover all cases and needs
more tests and error handling. It's meant as an illustration of the
concept.

The basic idea is to introduce a new optional `authors` header in
commits which contains a list of authors. The header is set in each new
commit when there is an entry `authors.current` in the git config listing
the current authors. When this config is not there the behavior falls
back to the current standard behavior.

When the header is there it is treated in the same way as the author
header. It's preserved on merges and similar operations, is displayed in
git show, and used to create a list of `From` addresses in `format-patch`.
Email supports multiple `From` addresses as specified in section 3.6.2 of
RFC 5322.

When multiple authors are configured, they still write the standard author
header to keep backwards compatibility. The first author is used as author
and committer. In the future it might be good to implement something like
automatic rotation of the order of authors to give credit in a fair way.

To make it easier to work with the authors there is a new command
`git-authors`. It sets the list of authors using initials as shortcut for
the full configuration with name and email. The mapping of initials to
names and email addresses is taken from a file `.git_authors_map` in the
home directory of the users. This way it's possible to quickly set a list
of authors by running a command such as `git authors ab cd`. This is
useful when doing pair programming because the people working together
usually switch quite frequently and using the command with the intials is
quicker and less error-prone than editing the configuration with full
names and emails.

The command also supports setting a single author, setting more than two
authors or clearing the configuration for multiple authors to go back to
the standard behavior without the new authors header.

The concept of the command and the mappings file is similar to what
git-duet does, so that it should be familiar to many people doing pair
programming. The behavior of git doesn't change when the new feature is
not used and when it's used it should be backwards compatible so that it
doesn't break existing functionality. This should make a smooth transition
for users who choose to make use of it.

Adding support for multiple authors would make the life of developers doing
pair programming easier. It would be useful in itself, but it would also
need support by other tools around git to use its full potential. This
might take a while, but I think it's worth the effort.

I'm willing to continue to work on this and create a patch which is suitable
for inclusion in git.

What do you think?

[1]: https://github.com/git-duet/git-duet

 .gitignore              |   1 +
 Makefile                |   3 +
 authors.c               | 205 ++++++++++++++++++++++++++++++++++++++++++++++++
 authors.h               |  29 +++++++
 builtin.h               |   1 +
 builtin/am.c            |  18 ++++-
 builtin/authors.c       |  82 +++++++++++++++++++
 builtin/commit-tree.c   |   2 +-
 builtin/commit.c        |  56 +++++++++++--
 builtin/merge.c         |   4 +-
 cache.h                 |   1 +
 commit.c                |  17 ++--
 commit.h                |   6 +-
 git.c                   |   1 +
 ident.c                 |   2 +-
 mailinfo.c              |  17 ++++
 mailinfo.h              |   2 +
 notes-cache.c           |   2 +-
 notes-utils.c           |   2 +-
 pretty.c                |  79 ++++++++++++++++++-
 t/helper/.gitignore     |   1 +
 t/helper/test-authors.c |  42 ++++++++++
 t/t9904-authors.sh      |  32 ++++++++
 23 files changed, 581 insertions(+), 24 deletions(-)
 create mode 100644 authors.c
 create mode 100644 authors.h
 create mode 100644 builtin/authors.c
 create mode 100644 t/helper/test-authors.c
 create mode 100755 t/t9904-authors.sh

diff --git a/.gitignore b/.gitignore
index 6722f78..1323907 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,6 +16,7 @@
 /git-apply
 /git-archimport
 /git-archive
+/git-authors
 /git-bisect
 /git-bisect--helper
 /git-blame
diff --git a/Makefile b/Makefile
index 27afd0f..22f4b28 100644
--- a/Makefile
+++ b/Makefile
@@ -597,6 +597,7 @@ X =

 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))

+TEST_PROGRAMS_NEED_X += test-authors
 TEST_PROGRAMS_NEED_X += test-chmtime
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-config
@@ -700,6 +701,7 @@ LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += argv-array.o
 LIB_OBJS += attr.o
+LIB_OBJS += authors.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
 LIB_OBJS += blob.o
@@ -859,6 +861,7 @@ BUILTIN_OBJS += builtin/am.o
 BUILTIN_OBJS += builtin/annotate.o
 BUILTIN_OBJS += builtin/apply.o
 BUILTIN_OBJS += builtin/archive.o
+BUILTIN_OBJS += builtin/authors.o
 BUILTIN_OBJS += builtin/bisect--helper.o
 BUILTIN_OBJS += builtin/blame.o
 BUILTIN_OBJS += builtin/branch.o
diff --git a/authors.c b/authors.c
new file mode 100644
index 0000000..ba782a0
--- /dev/null
+++ b/authors.c
@@ -0,0 +1,205 @@
+#include "cache.h"
+#include "authors.h"
+#include "string.h"
+#include "strbuf.h"
+
+/*
+ * given an authors line, split the fields
+ * to allow the caller to parse it.
+ * Signal a success by returning 0.
+ */
+int split_authors_line(struct authors_split *split, const char *line, int len)
+{
+	const char *cp;
+
+	memset(split, 0, sizeof(*split));
+
+	split->begin = line;
+
+	for (cp = line + len - 1; *cp != '>'; cp--)
+		if (cp == line) return -1;
+
+	split->end = cp + 1;
+	return 0;
+}
+
+void read_authors_map_line(struct string_list *map, char *buffer)
+{
+	int len = strlen(buffer);
+
+	if (len && buffer[len - 1] == '\n')
+		buffer[--len] = 0;
+
+	string_list_insert(map, xstrdup(buffer));
+}
+
+void read_authors_map_file(struct string_list *map)
+{
+	char buffer[1024];
+	FILE *f;
+	const char *filename;
+	const char *home;
+
+	home = getenv("HOME");
+	if (!home)
+		die("HOME not set");
+
+	filename = mkpathdup("%s/.git_authors_map", home);
+
+	f = fopen(filename, "r");
+	if (!f) {
+		if (errno == ENOENT) {
+			warning("~/.git_authors_map does not exist");
+			return;
+		}
+		die_errno("unable to open authors map at %s", filename);
+	}
+
+	while (fgets(buffer, sizeof(buffer), f) != NULL)
+		read_authors_map_line(map, buffer);
+	fclose(f);
+}
+
+char *lookup_author(struct string_list *map, const char *author_abbr)
+{
+	struct string_list_item *author_item = NULL;
+	struct string_list_item *item;
+
+	for_each_string_list_item(item, map) {
+		if (strncmp(item->string, author_abbr, strlen(author_abbr)) == 0 &&
+		    strlen(item->string) > strlen(author_abbr) &&
+		    *(item->string + strlen(author_abbr)) == ' ') {
+			author_item = item;
+			break;
+		}
+	}
+
+	if (!author_item)
+		return NULL;
+
+	return xstrdup(author_item->string + strlen(author_abbr) + 1);
+}
+
+const char *expand_authors(struct string_list *map, const char *author_shorts)
+{
+	int i;
+	const char *author_start = author_shorts;
+	const char *author_end;
+	char *author_short, *expanded_author;
+	static struct strbuf expanded_authors = STRBUF_INIT;
+
+	strbuf_reset(&expanded_authors);
+
+	for (i = 0; i <= strlen(author_shorts); i++) {
+		author_end = author_shorts + i;
+		if (*author_end == ' ' || *author_end == '\0') {
+			author_short = xstrndup(author_start, author_end - author_start);
+			expanded_author = lookup_author(map, author_short);
+			if (!expanded_author)
+				die("Could not expand author '%s'. Add it to the file ~/.git_authors_map.", author_short);
+			else {
+				if (expanded_authors.len > 0)
+					strbuf_addch(&expanded_authors, ',');
+				strbuf_addstr(&expanded_authors, expanded_author);
+				free(expanded_author);
+			}
+			free(author_short);
+
+			author_start = author_end + 1;
+		}
+	}
+
+	return expanded_authors.buf;
+}
+
+const char *git_authors_info(void)
+{
+	static struct strbuf authors_info = STRBUF_INIT;
+	const char *authors_config = NULL;
+	const char *date_str = NULL;
+	struct string_list authors_map = STRING_LIST_INIT_NODUP;
+
+	if (git_config_get_string_const("authors.current", &authors_config))
+		return NULL;
+
+	read_authors_map_file(&authors_map);
+
+	strbuf_reset(&authors_info);
+	strbuf_addstr(&authors_info, expand_authors(&authors_map, authors_config));
+
+	strbuf_addch(&authors_info, ' ');
+	date_str = getenv("GIT_AUTHOR_DATE");
+	if (date_str && date_str[0]) {
+		if (parse_date(date_str, &authors_info) < 0)
+			die("invalid date format: %s", date_str);
+	}
+	else
+		strbuf_addstr(&authors_info, ident_default_date());
+
+	return authors_info.buf;
+}
+
+const char *git_authors_first_info(const char *authors)
+{
+	static struct strbuf authors_first_info = STRBUF_INIT;
+	struct authors_split split;
+	const char *cp;
+
+	if (split_authors_line(&split, authors, strlen(authors)) < 0)
+		die("invalid authors format: %s", authors);
+
+	for (cp = split.begin; cp < split.end; cp++)
+		if (*cp == ',')
+			break;
+	strbuf_add(&authors_first_info, split.begin, cp - split.begin);
+	strbuf_add(&authors_first_info, split.end, strlen(authors));
+
+	return authors_first_info.buf;
+}
+
+const char *authors_split_to_email_froms(const struct authors_split *authors)
+{
+	static struct strbuf email_froms = STRBUF_INIT;
+	const char *cp;
+
+	strbuf_reset(&email_froms);
+
+	strbuf_addstr(&email_froms, "From: ");
+	for(cp = authors->begin; cp < authors->end; cp++)
+		if (*cp == ',')
+			strbuf_addstr(&email_froms, "\nFrom: ");
+		else
+			strbuf_addch(&email_froms, *cp);
+	strbuf_addch(&email_froms, '\n');
+
+	return email_froms.buf;
+}
+
+int has_multiple_authors(const char *authors)
+{
+	const char *cp = authors;
+
+	while (*cp != '\0')
+		if (*cp++ == ',')
+			return 1;
+	return 0;
+}
+
+const char *fmt_authors(const char *authors, const char *date_str)
+{
+	static struct strbuf authors_info = STRBUF_INIT;
+
+	strbuf_reset(&authors_info);
+
+	strbuf_addstr(&authors_info, authors);
+
+	strbuf_addch(&authors_info, ' ');
+	if (date_str && date_str[0]) {
+		if (parse_date(date_str, &authors_info) < 0)
+			die("invalid date format: %s", date_str);
+	}
+	else
+		strbuf_addstr(&authors_info, ident_default_date());
+
+	return authors_info.buf;
+}
diff --git a/authors.h b/authors.h
new file mode 100644
index 0000000..aa96674
--- /dev/null
+++ b/authors.h
@@ -0,0 +1,29 @@
+#ifndef AUTHORS_H
+#define AUTHORS_H
+
+#include "string-list.h"
+
+struct authors_split {
+	const char *begin;
+	const char *end;
+};
+/*
+ * Signals an success with 0
+ */
+extern int split_authors_line(struct authors_split *, const char *, int);
+
+extern const char *git_authors_info(void);
+extern const char *git_authors_first_info(const char *);
+
+extern void read_authors_map_file(struct string_list *);
+
+extern char *lookup_author(struct string_list *, const char *);
+extern const char *expand_authors(struct string_list *, const char *);
+
+extern const char *authors_split_to_email_froms(const struct authors_split *);
+
+extern int has_multiple_authors(const char *);
+
+extern const char *fmt_authors(const char *, const char *);
+
+#endif /* AUTHORS_H */
diff --git a/builtin.h b/builtin.h
index b9122bc..fec370e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -34,6 +34,7 @@ extern int cmd_am(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
 extern int cmd_archive(int argc, const char **argv, const char *prefix);
+extern int cmd_authors(int argc, const char **argv, const char *prefix);
 extern int cmd_bisect__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_blame(int argc, const char **argv, const char *prefix);
 extern int cmd_branch(int argc, const char **argv, const char *prefix);
diff --git a/builtin/am.c b/builtin/am.c
index 31fb605..cc131d4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -30,6 +30,7 @@
 #include "mailinfo.h"
 #include "apply.h"
 #include "string-list.h"
+#include "authors.h"

 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -106,6 +107,7 @@ struct am_state {
 	char *author_name;
 	char *author_email;
 	char *author_date;
+	char *authors;
 	char *msg;
 	size_t msg_len;

@@ -171,6 +173,7 @@ static void am_state_release(struct am_state *state)
 	free(state->author_name);
 	free(state->author_email);
 	free(state->author_date);
+	free(state->authors);
 	free(state->msg);
 	argv_array_clear(&state->git_apply_opts);
 }
@@ -1237,6 +1240,7 @@ static int parse_mail(struct am_state *state, const char *mail)
 	struct strbuf author_name = STRBUF_INIT;
 	struct strbuf author_date = STRBUF_INIT;
 	struct strbuf author_email = STRBUF_INIT;
+	struct strbuf authors = STRBUF_INIT;
 	int ret = 0;
 	struct mailinfo mi;

@@ -1303,6 +1307,8 @@ static int parse_mail(struct am_state *state, const char *mail)
 			strbuf_addstr(&author_email, x);
 		else if (skip_prefix(sb.buf, "Date: ", &x))
 			strbuf_addstr(&author_date, x);
+		else if (skip_prefix(sb.buf, "Authors: ", &x))
+			strbuf_addstr(&authors, x);
 	}
 	fclose(fp);

@@ -1333,6 +1339,10 @@ static int parse_mail(struct am_state *state, const char *mail)
 	assert(!state->author_date);
 	state->author_date = strbuf_detach(&author_date, NULL);

+	assert(!state->authors);
+	if (authors.len > 0)
+		state->authors = strbuf_detach(&authors, NULL);
+
 	assert(!state->msg);
 	state->msg = strbuf_detach(&msg, &state->msg_len);

@@ -1341,6 +1351,7 @@ static int parse_mail(struct am_state *state, const char *mail)
 	strbuf_release(&author_date);
 	strbuf_release(&author_email);
 	strbuf_release(&author_name);
+	strbuf_release(&authors);
 	strbuf_release(&sb);
 	clear_mailinfo(&mi);
 	return ret;
@@ -1678,6 +1689,7 @@ static void do_commit(const struct am_state *state)
 	struct commit_list *parents = NULL;
 	const char *reflog_msg, *author;
 	struct strbuf sb = STRBUF_INIT;
+	const char *authors_info = NULL;

 	if (run_hook_le(NULL, "pre-applypatch", NULL))
 		exit(1);
@@ -1701,8 +1713,12 @@ static void do_commit(const struct am_state *state)
 		setenv("GIT_COMMITTER_DATE",
 			state->ignore_date ? "" : state->author_date, 1);

+	if (state->authors)
+		authors_info = fmt_authors(state->authors,
+				state->ignore_date ? NULL : state->author_date);
+
 	if (commit_tree(state->msg, state->msg_len, tree.hash, parents, commit.hash,
-				author, state->sign_commit))
+				author, authors_info, state->sign_commit))
 		die(_("failed to write commit object"));

 	reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/authors.c b/builtin/authors.c
new file mode 100644
index 0000000..3342b91
--- /dev/null
+++ b/builtin/authors.c
@@ -0,0 +1,82 @@
+#include "builtin.h"
+#include "authors.h"
+#include "parse-options.h"
+
+static const char *const builtin_authors_usage[] = {
+	N_("git authors [<options>]"),
+	NULL
+};
+
+static int actions;
+
+#define ACTION_LIST (1<<0)
+#define ACTION_GET (1<<1)
+#define ACTION_SET (1<<2)
+#define ACTION_CLEAR (1<<3)
+
+static struct option builtin_authors_options[] = {
+	OPT_BIT('c', "clear", &actions, N_("clear current authors"), ACTION_CLEAR),
+	OPT_BIT('l', "list", &actions, N_("list all available authors"), ACTION_LIST)
+};
+
+static struct string_list authors_map = STRING_LIST_INIT_NODUP;
+
+int cmd_authors(int argc, const char **argv, const char *prefix)
+{
+	struct string_list_item *item;
+
+	argc = parse_options(argc, argv, prefix, builtin_authors_options,
+			     builtin_authors_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	if (actions == 0) {
+	if (argc == 0)
+		actions = ACTION_GET;
+	else
+	actions = ACTION_SET;
+	}
+
+	read_authors_map_file(&authors_map);
+
+	if (actions == ACTION_LIST)
+		for_each_string_list_item(item, &authors_map) {
+		printf("%s\n", item->string);
+	}
+	else if (actions == ACTION_GET) {
+		const char *authors_config = NULL;
+		const char *expanded_authors;
+
+		if (git_config_get_string_const("authors.current", &authors_config))
+			die("No current authors set. Use `git authors <initials> <initials> to set authors.");
+
+		printf("Short:    %s\n", authors_config);
+
+		expanded_authors = expand_authors(&authors_map, authors_config);
+
+		printf("Expanded: %s\n", expanded_authors);
+	}
+	else if (actions == ACTION_SET) {
+		int i;
+		static struct strbuf authors_info = STRBUF_INIT;
+		int lookup_error = 0;
+
+		for (i = 0; i < argc; ++i) {
+			if (!lookup_author(&authors_map, argv[i])) {
+				lookup_error--;
+				error("Couldn't find author '%s'", argv[i]);
+			}
+			if (i > 0)
+				strbuf_addch(&authors_info, ' ');
+				strbuf_addstr(&authors_info, argv[i]);
+		}
+		if (lookup_error < 0)
+			die("Add missing authors to ~/.git_authors_map");
+
+		git_config_set("authors.current", authors_info.buf);
+	}
+	else if (actions == ACTION_CLEAR) {
+		git_config_set("authors.current", NULL);
+	}
+
+	return 0;
+}
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 6050172..ecc2ce4 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -118,7 +118,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	}

 	if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-			commit_oid.hash, NULL, sign_commit)) {
+			commit_oid.hash, NULL, NULL, sign_commit)) {
 		strbuf_release(&buffer);
 		return 1;
 	}
diff --git a/builtin/commit.c b/builtin/commit.c
index 711f96c..88038ce 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -33,6 +33,7 @@
 #include "notes-utils.h"
 #include "mailmap.h"
 #include "sigchain.h"
+#include "authors.h"

 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -622,6 +623,29 @@ static void determine_author_info(struct strbuf *author_ident)
 	free(date);
 }

+static void determine_authors_info(struct strbuf *authors_info)
+{
+	const char *current_authors_info;
+
+	if (author_message) {
+		struct authors_split authors;
+		size_t len;
+		const char *a;
+
+		a = find_commit_header(author_message_buffer, "authors", &len);
+		if (a) {
+			if (split_authors_line(&authors, a, len) < 0)
+				die(_("commit '%s' has malformed authors line"), author_message);
+
+			strbuf_add(authors_info, a, len);
+		}
+	} else {
+		current_authors_info = git_authors_info();
+		if (current_authors_info)
+			strbuf_addstr(authors_info, current_authors_info);
+	}
+}
+
 static int author_date_is_interesting(void)
 {
 	return author_message || force_date;
@@ -660,7 +684,8 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct commit *current_head,
 			     struct wt_status *s,
-			     struct strbuf *author_ident)
+			     struct strbuf *author_ident,
+			     struct strbuf *authors_info)
 {
 	struct stat statbuf;
 	struct strbuf committer_ident = STRBUF_INIT;
@@ -671,8 +696,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
 	int old_display_comment_prefix;

-	/* This checks and barfs if author is badly specified */
-	determine_author_info(author_ident);
+	determine_authors_info(authors_info);
+	if (authors_info->len > 0) {
+		strbuf_addstr(author_ident, git_authors_first_info(authors_info->buf));
+		strbuf_addstr(&committer_ident, git_authors_first_info(authors_info->buf));
+	} else {
+		/* This checks and barfs if author is badly specified */
+		determine_author_info(author_ident);
+		strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
+	}

 	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
 		return 0;
@@ -800,11 +832,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	strbuf_release(&sb);

 	/* This checks if committer ident is explicitly given */
-	strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
 	if (use_editor && include_status) {
 		int ident_shown = 0;
 		int saved_color_setting;
 		struct ident_split ci, ai;
+		struct authors_split authors_split;

 		if (whence != FROM_COMMIT) {
 			if (cleanup_mode == CLEANUP_SCISSORS)
@@ -854,6 +886,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		assert_split_ident(&ai, author_ident);
 		assert_split_ident(&ci, &committer_ident);

+		if (authors_info->len > 0) {
+			split_authors_line(&authors_split, authors_info->buf, authors_info->len);
+			status_printf_ln(s, GIT_COLOR_NORMAL,
+				_("%s"
+				"Authors:   %.*s"),
+				ident_shown++ ? "" : "\n",
+				(int)(authors_split.end - authors_split.begin), authors_split.begin);
+		}
 		if (ident_cmp(&ai, &ci))
 			status_printf_ln(s, GIT_COLOR_NORMAL,
 				_("%s"
@@ -1637,6 +1677,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)

 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf author_ident = STRBUF_INIT;
+	struct strbuf authors_info = STRBUF_INIT;
 	const char *index_file, *reflog_msg;
 	char *nl;
 	unsigned char sha1[20];
@@ -1675,7 +1716,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	/* Set up everything for writing the commit object.  This includes
 	   running hooks, writing the trees, and interacting with the user.  */
 	if (!prepare_to_commit(index_file, prefix,
-			       current_head, &s, &author_ident)) {
+			       current_head, &s, &author_ident, &authors_info)) {
 		rollback_index_files();
 		return 1;
 	}
@@ -1762,11 +1803,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	}

 	if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
-			 parents, sha1, author_ident.buf, sign_commit, extra)) {
+			 parents, sha1, author_ident.buf,
+			 authors_info.len > 0 ? authors_info.buf : NULL,
+			 sign_commit, extra)) {
 		rollback_index_files();
 		die(_("failed to write commit object"));
 	}
 	strbuf_release(&author_ident);
+	strbuf_release(&authors_info);
 	free_commit_extra_headers(extra);

 	nl = strchr(sb.buf, '\n');
diff --git a/builtin/merge.c b/builtin/merge.c
index a96d4fb..a23599c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -798,7 +798,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 	pptr = commit_list_append(remoteheads->item, pptr);
 	prepare_to_commit(remoteheads);
 	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+			result_commit, NULL, NULL, sign_commit))
 		die(_("failed to write commit object"));
 	finish(head, remoteheads, result_commit, "In-index merge");
 	drop_save();
@@ -823,7 +823,7 @@ static int finish_automerge(struct commit *head,
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
 	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+			result_commit, NULL, NULL, sign_commit))
 		die(_("failed to write commit object"));
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
 	finish(head, remoteheads, result_commit, buf.buf);
diff --git a/cache.h b/cache.h
index 40f7ddd..3cb4a84 100644
--- a/cache.h
+++ b/cache.h
@@ -1327,6 +1327,7 @@ extern const char *fmt_ident(const char *name, const char *email, const char *da
 extern const char *fmt_name(const char *name, const char *email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
+extern const char *ident_default_date(void);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 extern int git_ident_config(const char *, const char *, void *);
diff --git a/commit.c b/commit.c
index 2cf8515..40eb1c4 100644
--- a/commit.c
+++ b/commit.c
@@ -11,6 +11,7 @@
 #include "commit-slab.h"
 #include "prio-queue.h"
 #include "sha1-lookup.h"
+#include "authors.h"

 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);

@@ -1312,7 +1313,8 @@ static inline int standard_header_field(const char *field, size_t len)
 		(len == 6 && !memcmp(field, "parent ", 7)) ||
 		(len == 6 && !memcmp(field, "author ", 7)) ||
 		(len == 9 && !memcmp(field, "committer ", 10)) ||
-		(len == 8 && !memcmp(field, "encoding ", 9)));
+		(len == 8 && !memcmp(field, "encoding ", 9)) ||
+		(len == 7 && !memcmp(field, "authors ", 8)));
 }

 static int excluded_header_field(const char *field, size_t len, const char **exclude)
@@ -1388,14 +1390,14 @@ void free_commit_extra_headers(struct commit_extra_header *extra)
 int commit_tree(const char *msg, size_t msg_len,
 		const unsigned char *tree,
 		struct commit_list *parents, unsigned char *ret,
-		const char *author, const char *sign_commit)
+		const char *author, const char *authors, const char *sign_commit)
 {
 	struct commit_extra_header *extra = NULL, **tail = &extra;
 	int result;

 	append_merge_tag_headers(parents, &tail);
 	result = commit_tree_extended(msg, msg_len, tree, parents, ret,
-				      author, sign_commit, extra);
+				      author, authors, sign_commit, extra);
 	free_commit_extra_headers(extra);
 	return result;
 }
@@ -1518,7 +1520,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
 int commit_tree_extended(const char *msg, size_t msg_len,
 			 const unsigned char *tree,
 			 struct commit_list *parents, unsigned char *ret,
-			 const char *author, const char *sign_commit,
+			 const char *author, const char *authors,
+			 const char *sign_commit,
 			 struct commit_extra_header *extra)
 {
 	int result;
@@ -1549,11 +1552,13 @@ int commit_tree_extended(const char *msg, size_t msg_len,

 	/* Person/date information */
 	if (!author)
-		author = git_author_info(IDENT_STRICT);
+		author = authors ? git_authors_first_info(authors) : git_author_info(IDENT_STRICT);
 	strbuf_addf(&buffer, "author %s\n", author);
-	strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
+	strbuf_addf(&buffer, "committer %s\n", authors ? git_authors_first_info(authors) : git_committer_info(IDENT_STRICT));
 	if (!encoding_is_utf8)
 		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
+	if (authors)
+		strbuf_addf(&buffer, "authors %s\n", authors);

 	while (extra) {
 		add_extra_header(&buffer, extra);
diff --git a/commit.h b/commit.h
index 9c12abb..4880460 100644
--- a/commit.h
+++ b/commit.h
@@ -331,12 +331,14 @@ extern void append_merge_tag_headers(struct commit_list *parents,
 extern int commit_tree(const char *msg, size_t msg_len,
 		       const unsigned char *tree,
 		       struct commit_list *parents, unsigned char *ret,
-		       const char *author, const char *sign_commit);
+		       const char *author, const char *authors,
+		       const char *sign_commit);

 extern int commit_tree_extended(const char *msg, size_t msg_len,
 				const unsigned char *tree,
 				struct commit_list *parents, unsigned char *ret,
-				const char *author, const char *sign_commit,
+				const char *author, const char *authors,
+				const char *sign_commit,
 				struct commit_extra_header *);

 extern struct commit_extra_header *read_commit_extra_headers(struct commit *, const char **);
diff --git a/git.c b/git.c
index b367cf6..b6fb852 100644
--- a/git.c
+++ b/git.c
@@ -397,6 +397,7 @@ static struct cmd_struct commands[] = {
 	{ "annotate", cmd_annotate, RUN_SETUP },
 	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
 	{ "archive", cmd_archive, RUN_SETUP_GENTLY },
+	{ "authors", cmd_authors, RUN_SETUP },
 	{ "bisect--helper", cmd_bisect__helper, RUN_SETUP },
 	{ "blame", cmd_blame, RUN_SETUP },
 	{ "branch", cmd_branch, RUN_SETUP },
diff --git a/ident.c b/ident.c
index ac4ae02..b538abc 100644
--- a/ident.c
+++ b/ident.c
@@ -177,7 +177,7 @@ const char *ident_default_email(void)
 	return git_default_email.buf;
 }

-static const char *ident_default_date(void)
+const char *ident_default_date(void)
 {
 	if (!git_default_date.len)
 		datestamp(&git_default_date);
diff --git a/mailinfo.c b/mailinfo.c
index a489d9d..43c85de 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "utf8.h"
 #include "strbuf.h"
+#include "authors.h"
 #include "mailinfo.h"

 static void cleanup_space(struct strbuf *sb)
@@ -533,6 +534,18 @@ static int check_header(struct mailinfo *mi,
 {
 	int i, ret = 0, len;
 	struct strbuf sb = STRBUF_INIT;
+	struct strbuf from = STRBUF_INIT;
+
+	if (overwrite && cmp_header(line, "From")) {
+		len = strlen("From: ");
+		strbuf_add(&from, line->buf + len, line->len - len);
+		decode_header(mi, &from);
+
+		if (mi->authors.len > 0)
+			strbuf_addch(&mi->authors,',');
+		strbuf_addstr(&mi->authors, from.buf);
+	}
+	strbuf_release(&from);

 	/* search for the interesting parts */
 	for (i = 0; header[i]; i++) {
@@ -1068,6 +1081,8 @@ static void handle_info(struct mailinfo *mi)
 			fprintf(mi->output, "%s: %s\n", header[i], hdr->buf);
 		}
 	}
+	if (has_multiple_authors(mi->authors.buf))
+		fprintf(mi->output, "Authors: %s\n", mi->authors.buf);
 	fprintf(mi->output, "\n");
 }

@@ -1133,6 +1148,7 @@ void setup_mailinfo(struct mailinfo *mi)
 	strbuf_init(&mi->charset, 0);
 	strbuf_init(&mi->log_message, 0);
 	strbuf_init(&mi->inbody_header_accum, 0);
+	strbuf_init(&mi->authors, 0);
 	mi->header_stage = 1;
 	mi->use_inbody_headers = 1;
 	mi->content_top = mi->content;
@@ -1147,6 +1163,7 @@ void clear_mailinfo(struct mailinfo *mi)
 	strbuf_release(&mi->email);
 	strbuf_release(&mi->charset);
 	strbuf_release(&mi->inbody_header_accum);
+	strbuf_release(&mi->authors);
 	free(mi->message_id);

 	for (i = 0; mi->p_hdr_data[i]; i++)
diff --git a/mailinfo.h b/mailinfo.h
index 04a2535..d264d0d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -33,6 +33,8 @@ struct mailinfo {

 	struct strbuf log_message;
 	int input_error;
+
+	struct strbuf authors;
 };

 extern void setup_mailinfo(struct mailinfo *);
diff --git a/notes-cache.c b/notes-cache.c
index 5dfc5cb..2b58add 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -58,7 +58,7 @@ int notes_cache_write(struct notes_cache *c)
 	if (write_notes_tree(&c->tree, tree_sha1))
 		return -1;
 	if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,
-			commit_sha1, NULL, NULL) < 0)
+			commit_sha1, NULL, NULL, NULL) < 0)
 		return -1;
 	if (update_ref("update notes cache", c->tree.update_ref, commit_sha1,
 		       NULL, 0, UPDATE_REFS_QUIET_ON_ERR) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 24a3361..8adf381 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -26,7 +26,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
 		/* else: t->ref points to nothing, assume root/orphan commit */
 	}

-	if (commit_tree(msg, msg_len, tree_sha1, parents, result_sha1, NULL, NULL))
+	if (commit_tree(msg, msg_len, tree_sha1, parents, result_sha1, NULL, NULL, NULL))
 		die("Failed to commit notes tree to database");
 }

diff --git a/pretty.c b/pretty.c
index 5e68383..d505f2a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -11,6 +11,7 @@
 #include "reflog-walk.h"
 #include "gpg-interface.h"
 #include "trailer.h"
+#include "authors.h"

 static char *user_format;
 static struct cmt_fmt_map {
@@ -510,6 +511,55 @@ void pp_user_info(struct pretty_print_context *pp,
 	}
 }

+void pp_authors_info(struct pretty_print_context *pp,
+		     struct strbuf *sb,
+		     const char *line, const char *encoding)
+{
+	struct ident_split ident;
+	char *line_end;
+	struct authors_split authors;
+
+	if (pp->fmt == CMIT_FMT_ONELINE)
+		return;
+
+	line_end = strchrnul(line, '\n');
+
+	if (split_authors_line(&authors, line, line_end - line))
+		return;
+
+	if (cmit_fmt_is_mail(pp->fmt)) {
+		strbuf_addstr(sb, authors_split_to_email_froms(&authors));
+	} else {
+		strbuf_addstr(sb, "Authors: ");
+		if (pp->fmt == CMIT_FMT_FULLER)
+			strbuf_addstr(sb, "    ");
+		strbuf_add(sb, authors.begin, authors.end - authors.begin);
+		strbuf_addch(sb, '\n');
+	}
+
+	/* Use the ident split for parsing the date part */
+	if (split_ident_line(&ident, line, line_end - line))
+		return;
+
+	switch (pp->fmt) {
+	case CMIT_FMT_MEDIUM:
+		strbuf_addf(sb, "Date:    %s\n",
+			    show_ident_date(&ident, &pp->date_mode));
+		break;
+	case CMIT_FMT_EMAIL:
+	case CMIT_FMT_MBOXRD:
+		strbuf_addf(sb, "Date: %s\n",
+			    show_ident_date(&ident, DATE_MODE(RFC2822)));
+		break;
+	case CMIT_FMT_FULLER:
+		strbuf_addf(sb, "AuthorsDate: %s\n",
+			    show_ident_date(&ident, &pp->date_mode));
+		break;
+	default:
+		break;
+	}
+}
+
 static int is_blank_line(const char *line, int *len_p)
 {
 	int len = *len_p;
@@ -1544,6 +1594,23 @@ static void pp_header(struct pretty_print_context *pp,
 		      struct strbuf *sb)
 {
 	int parents_shown = 0;
+	int has_authors = 0;
+	const char *msg = *msg_p;
+
+	/* Check if header has `authors` header */
+	for(;;) {
+		const char *line = msg;
+		int linelen = get_one_line(msg);
+		if (!linelen)
+			break;
+		msg += linelen;
+		if (linelen == 1)
+			break;
+		if (starts_with(line, "authors ")) {
+			has_authors = 1;
+			break;
+		}
+	}

 	for (;;) {
 		const char *name, *line = *msg_p;
@@ -1582,13 +1649,19 @@ static void pp_header(struct pretty_print_context *pp,
 		 * FULLER shows both authors and dates.
 		 */
 		if (skip_prefix(line, "author ", &name)) {
-			strbuf_grow(sb, linelen + 80);
-			pp_user_info(pp, "Author", sb, name, encoding);
+			if (!has_authors) {
+				strbuf_grow(sb, linelen + 80);
+				pp_user_info(pp, "Author", sb, name, encoding);
+			}
 		}
 		if (skip_prefix(line, "committer ", &name) &&
 		    (pp->fmt == CMIT_FMT_FULL || pp->fmt == CMIT_FMT_FULLER)) {
 			strbuf_grow(sb, linelen + 80);
-			pp_user_info(pp, "Commit", sb, name, encoding);
+			pp_user_info(pp, has_authors ? "Commit " : "Commit", sb, name, encoding);
+		}
+		if (skip_prefix(line, "authors ", &name)) {
+			strbuf_grow(sb, linelen + 80);
+			pp_authors_info(pp, sb, name, encoding);
 		}
 	}
 }
diff --git a/t/helper/.gitignore b/t/helper/.gitignore
index d6e8b36..9ad4bbc 100644
--- a/t/helper/.gitignore
+++ b/t/helper/.gitignore
@@ -1,3 +1,4 @@
+/test-authors
 /test-chmtime
 /test-ctype
 /test-config
diff --git a/t/helper/test-authors.c b/t/helper/test-authors.c
new file mode 100644
index 0000000..ab93f9f
--- /dev/null
+++ b/t/helper/test-authors.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "authors.h"
+
+static const char *usage_msg = "\n"
+"  test-authors split [authors_info]\n"
+"  test-authors has-multiple-authors [authors]\n";
+
+static void test_split_authors(const char **argv)
+{
+	struct authors_split split;
+	int result;
+	struct strbuf splitted = STRBUF_INIT;
+
+	printf("%s -> ",*argv);
+	result = split_authors_line(&split, *argv, strlen(*argv));
+	if (result)
+		printf("error");
+	else {
+		strbuf_add(&splitted, split.begin, split.end - split.begin);
+		printf(splitted.buf);
+	}
+	printf("\n");
+}
+
+static void test_has_multiple_authors(const char **argv)
+{
+	printf("%s -> %s\n", *argv, has_multiple_authors(*argv) ? "yes" : "no");
+}
+
+int cmd_main(int argc, const char **argv)
+{
+	argv++;
+	if (argc != 3)
+		usage(usage_msg);
+	if (!strcmp(*argv, "split"))
+		test_split_authors(argv+1);
+	else if (!strcmp(*argv, "has-multiple"))
+		test_has_multiple_authors(argv+1);
+	else
+		usage(usage_msg);
+	return 0;
+}
diff --git a/t/t9904-authors.sh b/t/t9904-authors.sh
new file mode 100755
index 0000000..eb7fffa
--- /dev/null
+++ b/t/t9904-authors.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description='test authors'
+
+. ./test-lib.sh
+
+check_split() {
+	echo "$1 -> $2" >expect
+	test_expect_success "split '$1'" "
+	test-authors split '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_split 'xxx' 'error'
+check_split 'Some Guy <sg@example.com> 1484387401 +0100' 'Some Guy <sg@example.com>'
+check_split 'Some Guy <sg@example.com>,Another Pal <ap@example.com> 1484387401 +0100' 'Some Guy <sg@example.com>,Another Pal <ap@example.com>'
+check_split 'Some Guy <sg@example.com>,Another Pal <ap@example.com>' 'Some Guy <sg@example.com>,Another Pal <ap@example.com>'
+
+check_has_multiple() {
+	echo "$1 -> $2" >expect
+	test_expect_success "has multiple authors '$1'" "
+	test-authors has-multiple '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_has_multiple 'abc' 'no'
+check_has_multiple 'Some Guy <sg@example.com>' 'no'
+check_has_multiple 'Some Guy <sg@example.com>,Another Pal <ap@example.com>' 'yes'
+
+test_done
--
2.6.6


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox