Add support for --wrt-author, --author and --exclude-author switches to git-rev-list

[PATCH] Add support for --wrt-author, --author and --exclude-author switches to git-rev-list

This patch is a complete replacement for:
[PATCH] Add support for --wrt flag to git-rev-list

This patch adds support for --wrt-author, --exclude-author and --author switches to git-rev-list.

When --wrt-author is specified, --merge-order is implied but
the linearisation of non-linear epochs is adjusted so that branches contributed to by the author sort after branches contributed to by others. This has the effect of presenting a linear history of changes as seen by the author's own workspace.

For example, given this commit history:

a4 ---

| \   \
|  b4 |
|/ |  |
a3 |  |
|  |  |
a2 |  |
|  |  c3
|  |  |
|  |  c2
|  b3 |
|  | /|
|  b2 |
|  |  c1
|  | /
|  b1    <-- commit authored by author@domain
a1 |
|  |
a0 |
| /

root

--merge-order alone would sort it as follows:

= a4

| c3
| c2
| c1
^ b4
| b3
| b2
| b1
^ a3
| a2
| a1
| a0

= root

however, with --wrt-author --author=author@domain, git-rev-list sorts as follows:

= a4

| c3
| c2
| c1
^ b4
| a3
| a2
| a1
| a0

^ b3
| b2
| b1
= root

If --exclude-author is specified, then git-rev-list behaves as if a ^argument had been added for each commit authored by the author.

The --exclude-author option allows an author to see a brief summary of changes since they last made a contribution to the commit graph. For example:

      git-rev-list --exclude-author --pretty HEAD 

To configure which author git-rev-list uses when processing the --wrt-author and --exclude-author switches, use the --author=author@domain option. If this option is not specified, git-rev-list uses GIT_AUTHOR_EMAIL or a value derived from the local user, host and domain names (per git-commit-tree's algorithm)

To support this change, some additional functions were added to commit.c to allow the extraction of author from the commit header.

Also, --show-breaks, like --wrt-author now implies --merge-order rather than requires it.

This change also updates Documentation/git-rev-list.txt to include documentation for --wrt-author and other git-rev-list options not currently documented.

A test case has been included in t/t6000-rev-list.sh to verify this change works as expected.

Use of --wrt-author implies a small overhead (linear with respect to nodes printed) compared with --merge-order.

This change extracts real email id formatting functions from commit-tree.c into user.c and user.h.

NOTE TO REVIEWERS: please pay special attention to get_real_identity()
which was extracted from commit-tree.c and placed into user.c. It was essentially a cut and paste, but since this has the potential to affect the operation of git-commit-tree, it would be worth being very careful about checking this change.

Diverged from ed37b5b2b94398f3ab8312dfdf23cfd25549e3ec by Linus Torvalds <[email protected]>
---

@@ -9,13 +9,25 @@ git-rev-list - Lists commit objects in r
 
 SYNOPSIS
 --------
-'git-rev-list' [ *--max-count*=number ] [ *--max-age*=timestamp ] [ *--min-age*=timestamp ] [ *--merge-order* [ *--show-breaks* ] ] <commit>
+'git-rev-list' [ *--max-count*=number ] [ *--max-age*=timestamp ] [ *--min-age*=timestamp ] [ *--pretty* ] [ *--parents* ] [ *--header* ] [ *--merge-order* ] [ *--show-breaks* ] [ *--wrt-author* ] [ *--exclude-author* ] [ *--author=author@domain* ]  <head-to-include> <head-to-include> ... 
+^<base-to-exclude> 
+^<base-to-exclude> ...
 
 DESCRIPTION
 -----------
-Lists commit objects in reverse chronological order starting at the
-given commit, taking ancestry relationship into account.  This is
-useful to produce human-readable log output.
+
+Lists commit objects in reverse chronological order starting at the commits specified by head-to-include values, 
+taking ancestry relationship into account.  git-rev-list will not print any commits that are reachable 
+by any of the base-to-exclude values. 
+
+If *--pretty* is specified, git-rev-list prints a short summary of each commit on stdout. This can be usefully
+parsed by git-shortlog to produce a short log of changes.
+
+If *--parents* is specified, git-rev-list prints a space separated list of parents on the same line as the
+commit identifier.
+
+If *--header* is specified, git-rev-list prints to stdout, on line after the identifier of each commit, 
+the contents of the commit's header followed by a trailing NUL.
 
 If *--merge-order* is specified, the commit history is decomposed into a unique sequence of minimal, non-linear
 epochs and maximal, linear epochs. Non-linear epochs are then linearised by sorting them into merge order, which 

@@ -42,17 +54,30 @@ Commits marked with (=) represent the bo
 Commits marked with (|) are direct parents of commits immediately preceding the marked commit in the list.
 Commits marked with (^) are not parents of the immediately preceding commit. These "breaks" represent necessary discontinuities implied by trying to represent an arbtirary DAG in a linear form.
 
-*--show-breaks* is only valid if *--merge-order* is also specified.
+The *--author* option is used to specify the author used by the *--wrt-author* and *--exclude-author* switches. If 
+an *--author* option is not specified, the author used is derived from GIT_AUTHOR_EMAIL or, ultimately, 
+from the local user, host and domain names as per the logic used by git-commit-tree.
+
+If *--wrt=author* is specified, then git-rev-list sorts commits so that contemporaneously 
+developed commits appear to occur after (that is, sort before) any commits authored by the 
+author specified by *--author*. This reflects the order in which that author 
+sees contemporaneously developed commits appear in her own workspace.
+
+If *--exclude-author* is specified, then any commit authored by the author is marked as uninteresting as if it 
+had been specified with ^ on the command line. This option restricts git-rev-list's to just the changes since 
+the author contributed a change. 
+
+Use of *--show-breaks*, *--exclude-author* or *--wrt-author* implies *--merge-order*.
 
 Author
 ------
 Written by Linus Torvalds <[email protected]>
 
-Original *--merge-order* logic by Jon Seymour <[email protected]>
+Original *--merge-order*, *--wrt-author* and *--exclude-author* logic by Jon Seymour <[email protected]>
 
 Documentation
 --------------
-Documentation by David Greaves, Junio C Hamano and the git-list <[email protected]>.
+Documentation by David Greaves, Junio C Hamano, Jon Seymour and the git-list <[email protected]>.
 
 GIT
 ---

@@ -41,9 +41,9 @@ install: $(PROG) $(SCRIPTS)
 
 LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \
 	 tag.o delta.o date.o index.o diff-delta.o patch-delta.o entry.o \
-	 epoch.o refs.o
+	 epoch.o refs.o user.o
 LIB_FILE=libgit.a
-LIB_H=cache.h object.h blob.h tree.h commit.h tag.h delta.h epoch.h
+LIB_H=cache.h object.h blob.h tree.h commit.h tag.h delta.h epoch.h user.h
 
 LIB_H += strbuf.h
 LIB_OBJS += strbuf.o

@@ -138,6 +138,7 @@ diffcore-pickaxe.o : $(LIB_H) diffcore.h
 diffcore-break.o : $(LIB_H) diffcore.h
 diffcore-order.o : $(LIB_H) diffcore.h
 epoch.o: $(LIB_H)
+user.o: $(LIB_H)
 
 test: all
 	$(MAKE) -C t/ all

@@ -4,10 +4,11 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 #include "cache.h"
+#include "user.h"
 
-#include <pwd.h>
 #include <time.h>
 #include <ctype.h>
+#include <pwd.h>
 
 #define BLOCKING (1ul << 14)
 

@@ -100,17 +101,16 @@ static char *commit_tree_usage = "git-co
 
 int main(int argc, char **argv)
 {
-	int i, len;
+	int i;
 	int parents = 0;
 	unsigned char tree_sha1[20];
 	unsigned char parent_sha1[MAXPARENT][20];
 	unsigned char commit_sha1[20];
 	char *gecos, *realgecos, *commitgecos;
-	char *email, *commitemail, realemail[1000];
+	char *email, *commitemail, *realemail;
 	char date[50], realdate[50];
 	char *audate, *cmdate;
 	char comment[1000];
-	struct passwd *pw;
 	char *buffer;
 	unsigned int size;
 

@@ -128,18 +128,8 @@ int main(int argc, char **argv)
 	}
 	if (!parents)
 		fprintf(stderr, "Committing initial tree %s\n", argv[1]);
-	pw = getpwuid(getuid());
-	if (!pw)
-		die("You don't exist. Go away!");
-	realgecos = pw->pw_gecos;
-	len = strlen(pw->pw_name);
-	memcpy(realemail, pw->pw_name, len);
-	realemail[len] = '@';
-	gethostname(realemail+len+1, sizeof(realemail)-len-1);
-	if (!strchr(realemail+len+1, '.')) {
-		strcat(realemail, ".");
-		getdomainname(realemail+strlen(realemail), sizeof(realemail)-strlen(realemail)-1);
-	}
+
+        get_real_identity(&realemail, &realgecos);
 
 	datestamp(realdate, sizeof(realdate));
 	strcpy(date, realdate);

@@ -53,4 +53,17 @@ struct commit *pop_most_recent_commit(st
 struct commit *pop_commit(struct commit_list **stack);
 
 int count_parents(struct commit * commit);
+
+//
+// Copies the value of the (index+1)'th commit header matching the name specified
+// into the buffer supplied and append a trailing NUL.
+//
+// Returns n<0 if the buffer was too short (-n is the required length)
+// Returns 0 if the header doesn't exist.
+// Returns n>0 where n is the length of the copied zero-terminated value, including the terminating zero.
+//
+int copy_commit_header(struct commit * commit, char * header, int index, char * buffer, int len);
+
+// Copies the commit's author value into the buffer supplied. Return values as per copy_commit_header.
+int copy_author(struct commit * commit, char * buffer, int len);
 #endif /* COMMIT_H */

@@ -8,13 +8,19 @@
 #define DO       2
 typedef int (*emitter_func) (struct commit *); 
 
-int sort_list_in_merge_order(struct commit_list *list, emitter_func emitter);
+// functions of this type return non-zero if the commit is "local" by some private assessment of
+// what it is for a commit to be local.
+typedef int (*local_test_func) (struct commit *); 
+typedef void (*marker_func) (struct commit *); 
+
+extern int sort_list_in_merge_order(struct commit_list *list, emitter_func emitter, local_test_func local_test, marker_func marker);
 
 #define UNINTERESTING  (1u<<2)
 #define BOUNDARY       (1u<<3)
 #define VISITED        (1u<<4)
 #define DISCONTINUITY  (1u<<5)
 #define DUPCHECK       (1u<<6)
+#define LOCAL          (1u<<7)
 
 
 #endif				/* EPOCH_H */

new file mode 100644
@@ -0,0 +1,32 @@
+#include "cache.h"
+#include "user.h"
+#include <string.h>
+#include <pwd.h>
+
+void get_real_identity(char **email, char **gecos)
+{
+	static char buffer[1000];
+	struct passwd *pw;
+	int len;
+
+	pw = getpwuid(getuid());
+	if (!pw)
+		die("You don't exist. Go away!");
+
+	len = strlen(pw->pw_name);
+	memcpy(buffer, pw->pw_name, len);
+	buffer[len] = '@';
+	gethostname(buffer + len + 1, sizeof(buffer) - len - 1);
+	if (!strchr(buffer + len + 1, '.')) {
+		strcat(buffer, ".");
+		getdomainname(buffer + strlen(buffer), sizeof(buffer) - strlen(buffer) - 1);
+	}
+
+	if (gecos) {
+		*gecos = strdup(pw->pw_gecos);
+	}
+
+	if (email) {
+		*email = strdup(buffer);
+	}
+}

new file mode 100644
@@ -0,0 +1,8 @@
+#ifndef USER_H
+#define USER_H
+//
+// Allocates two new strings to contain the real email and
+// name of the current user.
+//
+extern void get_real_identity(char **email, char **gecos);
+#endif

Dear diary, on Tue, Jun 07, 2005 at 11:15:23AM CEST, I got a letter where Jon Seymour <[email protected]> told me that...

I'd prefer just --wrt-author and --exclude-author to take an argument on their own.

(Note that I don't endorse this patch and the --wrt-author behaviour in particular seems strange. I don't have enough time to comment on it sensibly now, though. I'm just focusing on style here since I'd like to still be able to read git's source code few weeks from now on.)

				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Dear diary, on Tue, Jun 07, 2005 at 11:59:36AM CEST, I got a letter where Jon Seymour <[email protected]> told me that...

Hmm, then why not make it --wrt-author[=AUTHOR] ? Similar to the --pretty option. BTW, can it do multiple author excludes now? The commandline would look especially horrifying in that case now, I guess.

I'd prefer --stop-at-author from the choices you offer in your other mail.

Yes, such motivation makes sense. But is the author field the right one? If you are integrating a lot of other people's patches in particular, I think it makes no sense whatsoever - you already reviewed and consciously applied them, but your option will regard them as something alien and merged from outside, right?

And, after all, the other branches might be _quite_ long-lived. I think it would be confusing for the user if the commit graph looked like

a1 -- a2 -- a3 -- a4 -- a5 -- a6 -- a7 -- a8 -- a9 -- a10
\ / / \ /
- b1 -- b2 -- b3 -- b4 -- b5 -- b6 -- b7 -- b8 -- b9

If your patch first chooses b1, it then shows all of it, completely ignoring a2, right? I can't see how that would be right - the subsequent merges from a should be shown.

				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

No, it doesn't. I can see a use for multiple ^ arguments, but the use case for multiple --stop-at-author arguments doesn't strike me as being particularly useful. Multiple ^'s helps you prune the graph at places you already know about --stop-at-author allows you to prune the tree with respect to one author. It is particularly useful if you are the author concerned because you have some recollection of being of what you did last. I am not sure it is that useful to prune the tree with respect to multiple authors - the results would be somewhat unpredictable, I would think.

Ok, unless I Linus or the list disagree I will modify my patch to be --stop-at-author.

No, that's not the intent of --wrt-author when specified without --stop-at-author.

--wrt-author doesn't cause any commits to be ignored ... it simply changes the order
in which they are displayed by altering the order in which parents are visited.

I'll get it wrong if I do it by hand so I'll create a test case and show you what the default merge order would be, the --wrt-author --author=a and the --wrt-author --author=b cases.

Stay tuned...

jon.

Ok, this is how --wrt-author sorts your example:

(Original example transposed so that variable width fonts don't play havoc with the layout)

a10 b9

| \  |
a9  b8
|    |
a8  b7
|    | 
a7  b6
| /  |
a6  b5
| \  |
a5  b4
|    |
a4  b3
| \  |
a3  b2
|    |
a2  b1
| /

a1

with respect to a and b respectively

A B

| b9      | b9
^ a10     ^ a10
| b8      | a9
| b7      | a8          
| b6      | a7
| b5      ^ b8
^ a9      | b7
| a8      | b6
| a7      | a6
| a6      | a5
| b4      | a4
| b3      | a3
^ a5      | a2
| a4      ^ b5
| b2      | b4
| b1      | b3
^ a3      | b2
| a2      | b1
= a1      = a1

In otherwords, B doesn't see any of A's changes after a1 until b6, whereas A sees some of B's changes at a4 and some at a6, then not again until a10.

jon.

Pasky,

Hey, I was thinking, instead of stop-at-author, how about we call it prune-at-author instead?

Regards,

jon.

G'day Linus,

I'd like to withdraw this patch until the --merge-order bug fix of my earlier post is applied. The patch as it stands won't operate correctly once that fix is applied, so I'll have to modify it. Also, I want to rename the --exclude-author switch to --prune-at-author.

Regards,

jon.

Correction:

--wrt-author helps to reconstruct the merge-history from the perspective of each individual author.

jon.

| not sure why, but my first attempt to post this to the list didn't seem to make it

| I've also modified my response slightly to suggest an alternate name for the
| --exclude-author swtich which may partly address pasky's concerns. amended
| response marked with ***

The reason I don't want to do this is that it doesn't really make sense in the context of the change to specify one author for --wrt-author and another for --exclude-author. In normal use --author defaults to GIT_AUTHOR_EMAIL or the locally derived [email protected]. The intention is simply to override this default derivation.

*** That said, it might better if --exclude-author was called something like --following-author --after-author or --stop-at-author, since its meaning is to stop traversal rather than simply omit match entries which is the meaning that a reasonable user might attach to use of the word "exclude"

My preference is --following-author.

Thoughts anyone?

The rationale for the change is as follows:

During parallel development, one is aware of ones own changes...everyone else changes haven't happened yet as far as you are concerned. Only when they appear in a future merge that incorporates one's own changes do the other changes appear in your own workspace.

As far as you are concerned, these changes occurred after you made your own - your changes were not dependent on those changes, only on those that came before. So the linearisation reflects that perceived ordering of changes.

--wrt-author helps to reconstruct the merge-history from the perspective of each individual committer.

Let me know if this doesn't explain the rationale completely enough for your understanding.

jon.

A small update to the patch which corrects a couple of documentation typos and a slight correction to git-rev-list options processing has been posted here. The update has been tested with a complete apply+build from the base commit.

http://blackcubes.dyndns.org/epoch/wrt-patch.patch

Regards,

jon.