Git development
 help / color / mirror / Atom feed
* Re: Warning: cvsexportcommit considered dangerous
From: Robin Rosenberg @ 2007-11-04 23:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Steffen Prohaska, Git Mailing List, Alex Bennee
In-Reply-To: <Pine.LNX.4.64.0711042133330.4362@racer.site>

söndag 04 november 2007 skrev Johannes Schindelin:
> Hi,
> 
> On Sun, 4 Nov 2007, Steffen Prohaska wrote:
> 
> > On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
> > 
> > > ever since the up-to-date check was changed to use just one call to 
> > > "cvs status", a bug was present.  Now cvsexportcommit expects "cvs 
> > > status" to return the results in the same order as the file names were 
> > > passed.
> > > 
> > > This is not true, as I had to realise with one of my projects on 
> > > sourceforge.
> > > 
> > > Since time is so scarce on my side, I will not have time to fix this 
> > > bug, but will instead return to my old "commit by hand" procedure.
> > 
> > I introduced this 'optimization', which turned out to be a bug. So, I 
> > feel responsible. Sorry for the trouble.
> > 
> > In August this was already recognized and a patch submitted:
> > 
> > http://marc.info/?t=118718458000004&r=1&w=2
> > 
> > I do not know why it wasn't applied. I forgot re-checking after my 
> > vacation.
> 
> It slipped by me, because of holiday, too.  (I was on my well needed 
> holiday then.)
> 
> But that patch really seems like a step back to me.  The line "File: ... 
> Status: ..." should be parsable enough to fix the bug properly, instead of 
> undoing the optimisation.
Unfortunately it's not that easy to parse. It *can* be done by looking at the
repository path, and the CVS/Root etc, but it's not nice. 

> 
> AFAICS Robin replied with a "let's see if a proper fix materialises", and 
> I kind of hope that it will materialise soon.

Still hoping :). BTW, wouldn't this err on the right side anyway, i.e. if an
existing file was not up to date and was wrongly thought to not exist or a new 
file was thought to be up-to-date, I would get an error and would not be able
to commit. I've never seen it though and I always have a clean CVS checkout
so the potential bug seems unlikely to me.

The command I always use is.

	git cvsexportcommit -u -c -w /my/cvs/checkout

Never bitten me yet (touch wood).

My real worry is on the other side, with bad conversion from CVS to git.

-- robin

^ permalink raw reply

* Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.
From: Junio C Hamano @ 2007-11-04 22:58 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <1194172262-1563-2-git-send-email-madcoder@debian.org>

Two comments.

 * I have updated 1/10 with typo and indentation fixes.

 * I see you changed 2/10 to use OPTIONS_KEEPDASHDASH instead of
   PARSEOPT_OPTS, but the scripts that do not want the --keep
   behaviour do not set OPTIONS_KEEPDASHDASH to empty, so I do
   not see how this updatet would make _any_ difference.  The
   user can still screw up by having OPTIONS_KEEPDASHDASH in
   their environments by mistake, curiosity or just plain
   stupidity.

^ permalink raw reply

* [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-04 22:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Symonds

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 Documentation/git-format-patch.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 92c0ab6..d8a89a1 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -9,10 +9,11 @@ git-format-patch - Prepare patches for e-mail submission
 SYNOPSIS
 --------
 [verse]
-'git-format-patch' [-n | -N | -k] [-o <dir> | --stdout] [--thread]
+'git-format-patch' [-k] [-o <dir> | --stdout] [--thread]
                    [--attach[=<boundary>] | --inline[=<boundary>]]
                    [-s | --signoff] [<common diff options>]
-                   [--start-number <n>] [--numbered-files]
+                   [-n | --numbered-files | -N | --no-numbered]
+                   [--start-number <n>]
                    [--in-reply-to=Message-Id] [--suffix=.<sfx>]
                    [--ignore-if-in-upstream]
                    [--subject-prefix=Subject-Prefix]
-- 
1.5.3.1

^ permalink raw reply related

* Re: [PATCH] errors: "strict subset" -> "ancestor"
From: J. Bruce Fields @ 2007-11-04 22:08 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <3F4A5458-AB2F-40C7-AA0E-9D26981BCE9D@zib.de>

On Sat, Nov 03, 2007 at 08:51:29AM +0100, Steffen Prohaska wrote:
>
> On Nov 3, 2007, at 3:39 AM, J. Bruce Fields wrote:
>
>> From: J. Bruce Fields <bfields@citi.umich.edu>
>>
>> The term "ancestor" is a bit more intuitive (and more consistent with
>> the documentation) than the term "strict subset".
>>
>> Also, remove superfluous "ref", capitalize, and add some carriage
>> returns, changing:
>>
>> 	error: remote 'refs/heads/master' is not a strict subset of local ref 
>> 'refs/heads/master'. maybe you are not up-to-date and need to pull first?
>> 	error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
>>
>> to:
>>
>> 	error: remote 'refs/heads/master' is not an ancestor of
>> 	 local 'refs/heads/master'.
>> 	 Maybe you are not up-to-date and need to pull first?
>> 	error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
>
>
> Junio suggested in [1] (see also earlier messages in that
> thread) to replace the recommendation to pull with a hint
> where to look in the user manual.
>
> [1] http://marc.info/?l=git&m=119398999317677&w=2
>
>
> The point is, there are various ways to resolve the problem.
> pull is not necessarily the right solution. At least, you should
> consider to rebase. Or maybe just something else went wrong.

Yeah, actually in my case I usually want to force....

So I think it's a good suggestion, but I'm putting it off for now as I'm
not sure yet where to refer people to, and don't like making the error a
lot longer.

Hm.  I wonder if extra "help" commandline flags would be a way to get
people extra guidance on particular situations without cluttering up the
default messages ("not sure what to try next?  Try -h notanancestor..."
Maybe not.)

>
> Nonetheless I think it could be a good idea to keep the most
> likely cases. So, how about
>
> "Are you up-to-date? Did you forget to pull or rebase? See User's Manual 
> for details."
>
> I put it as questions to avoid making a suggestion. The questions
> should give sufficient hints for searching in the User's Manual.
> I haven't found the single section that would explain exactly
> the situation we're dealing with.

Me neither.  And I don't think a reference to the whole thing is
helpful.

--b.

^ permalink raw reply

* [PATCH] errors: "strict subset" -> "ancestor"
From: J. Bruce Fields @ 2007-11-04 21:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Symonds, git, Steffen Prohaska
In-Reply-To: <2B1935E5-C490-4EB4-9BE6-0DD7F33FFE36@zib.de>

From: J. Bruce Fields <bfields@citi.umich.edu>

The term "ancestor" is a bit more intuitive (and more consistent with
the documentation) than the term "strict subset".

Also, remove superfluous "ref", and capitalize, changing:

	error: remote 'refs/heads/master' is not a strict subset of local ref 'refs/heads/master'. maybe you are not up-to-date and need to pull first?
	error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'

to:

	error: remote 'refs/heads/master' is not an ancestor of
	 local 'refs/heads/master'.
	 Maybe you are not up-to-date and need to pull first?
	error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 builtin-branch.c |    2 +-
 http-push.c      |    8 ++++----
 send-pack.c      |    6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

On Sat, Nov 03, 2007 at 08:24:29AM +0100, Steffen Prohaska wrote:
> They are not aligned. The second line is indented with one
> space. Look at examples in the commit message. The first line
> starts with "error:", which already destroys the alignment.

Yup, I think that's exactly what happened--I said "hey, maybe I should
try aligning this and see what it looks like?", then saw the problem,
then forgot to revert the extra space from everywhere.... Thanks for
noticing.  Here's an updated patch.--b.

diff --git a/builtin-branch.c b/builtin-branch.c
index 3da8b55..e8de27e 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -142,7 +142,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 
 		if (!force &&
 		    !in_merge_bases(rev, &head_rev, 1)) {
-			error("The branch '%s' is not a strict subset of "
+			error("The branch '%s' is not an ancestor of "
 				"your current HEAD.\n"
 				"If you are sure you want to delete it, "
 				"run 'git branch -D %s'.", argv[i], argv[i]);
diff --git a/http-push.c b/http-push.c
index c02a3af..5960d7c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2241,7 +2241,7 @@ static int delete_remote_branch(char *pattern, int force)
 
 		/* Remote branch must be an ancestor of remote HEAD */
 		if (!verify_merge_base(head_sha1, remote_ref->old_sha1)) {
-			return error("The branch '%s' is not a strict subset of your current HEAD.\nIf you are sure you want to delete it, run:\n\t'git http-push -D %s %s'", remote_ref->name, remote->url, pattern);
+			return error("The branch '%s' is not an ancestor of your current HEAD.\nIf you are sure you want to delete it, run:\n\t'git http-push -D %s %s'", remote_ref->name, remote->url, pattern);
 		}
 	}
 
@@ -2424,9 +2424,9 @@ int main(int argc, char **argv)
 				 * commits at the remote end and likely
 				 * we were not up to date to begin with.
 				 */
-				error("remote '%s' is not a strict "
-				      "subset of local ref '%s'. "
-				      "maybe you are not up-to-date and "
+				error("remote '%s' is not an ancestor of\n"
+				      " local '%s'.\n"
+				      " Maybe you are not up-to-date and "
 				      "need to pull first?",
 				      ref->name,
 				      ref->peer_ref->name);
diff --git a/send-pack.c b/send-pack.c
index 5e127a1..fbf2462 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -297,9 +297,9 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 				 * commits at the remote end and likely
 				 * we were not up to date to begin with.
 				 */
-				error("remote '%s' is not a strict "
-				      "subset of local ref '%s'. "
-				      "maybe you are not up-to-date and "
+				error("remote '%s' is not an ancestor of\n"
+				      " local '%s'.\n"
+				      " Maybe you are not up-to-date and "
 				      "need to pull first?",
 				      ref->name,
 				      ref->peer_ref->name);
-- 
1.5.3.5.475.g477d-dirty

^ permalink raw reply related

* Re: gitk graph routing problem
From: Paul Mackerras @ 2007-11-04 21:47 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071104104618.GA3078@steel.home>

Alex Riesen writes:

> To reproduce, try running in git repo:
> 
>     gitk 02f630448e5d48e..06ea6ba9cf46ef5

I can't reproduce it here, as my clone of the git repo doesn't have
02f630448e5d48e in it, even after updating...

Paul.

^ permalink raw reply

* Re: [PATCH] Fix an infinite loop in sq_quote_buf().
From: Pierre Habouzit @ 2007-11-04 21:37 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio Hamano, git
In-Reply-To: <200711042126.22512.johannes.sixt@telecom.at>

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

On Sun, Nov 04, 2007 at 08:26:22PM +0000, Johannes Sixt wrote:
> sq_quote_buf() treats single-quotes and exclamation marks specially, but
> it incorrectly parsed the input for single-quotes and backslashes.
> 
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
>  quote.c          |    2 +-
>  t/t5510-fetch.sh |    7 +++++++
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/quote.c b/quote.c
> index 482be05..919d092 100644
> --- a/quote.c
> +++ b/quote.c
> @@ -26,7 +26,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
>  
>  	strbuf_addch(dst, '\'');
>  	while (*src) {
> -		size_t len = strcspn(src, "'\\");
> +		size_t len = strcspn(src, "'!");
>  		strbuf_add(dst, src, len);
>  		src += len;
>  		while (need_bs_quote(*src)) {

  Ouch, good catch :/ sorry about that.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Warning: cvsexportcommit considered dangerous
From: Johannes Schindelin @ 2007-11-04 21:35 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Git Mailing List, Alex Bennee, Robin Rosenberg
In-Reply-To: <623F4AFA-FE43-4046-9D3F-435396BBE17D@zib.de>

Hi,

On Sun, 4 Nov 2007, Steffen Prohaska wrote:

> On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
> 
> > ever since the up-to-date check was changed to use just one call to 
> > "cvs status", a bug was present.  Now cvsexportcommit expects "cvs 
> > status" to return the results in the same order as the file names were 
> > passed.
> > 
> > This is not true, as I had to realise with one of my projects on 
> > sourceforge.
> > 
> > Since time is so scarce on my side, I will not have time to fix this 
> > bug, but will instead return to my old "commit by hand" procedure.
> 
> I introduced this 'optimization', which turned out to be a bug. So, I 
> feel responsible. Sorry for the trouble.
> 
> In August this was already recognized and a patch submitted:
> 
> http://marc.info/?t=118718458000004&r=1&w=2
> 
> I do not know why it wasn't applied. I forgot re-checking after my 
> vacation.

It slipped by me, because of holiday, too.  (I was on my well needed 
holiday then.)

But that patch really seems like a step back to me.  The line "File: ... 
Status: ..." should be parsable enough to fix the bug properly, instead of 
undoing the optimisation.

AFAICS Robin replied with a "let's see if a proper fix materialises", and 
I kind of hope that it will materialise soon.

Ciao,
Dscho

^ permalink raw reply

* [RFC PATCH] Reduce the number of connects when fetching
From: Daniel Barkalow @ 2007-11-04 21:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This shares the connection between getting the remote ref list and
getting objects in the first batch. (A second connection is still used
to follow tags)

It passes all of the tests, and should be fine, but I don't entirely 
understand the git protocol, so I'd like people who know it better to 
check this over. In particular, I don't know if there's a way to have the 
connection end up in a state where objects for more refs can be requested 
after some refs have been requested and the resulting objects read.

The idea is to keep the open connection in the data for the transport in 
between getting the list of refs and doing anything further. This 
therefore moves the connection-handling aspects outside of fetch-pack() 
and handles them primarily in transport.c.

This is on top of current next.
---
 builtin-fetch-pack.c |   74 ++++++++++++++++++++++++++-----------------------
 fetch-pack.h         |    2 +
 transport.c          |   41 +++++++++++++++++++--------
 3 files changed, 70 insertions(+), 47 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 862652b..7783c05 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -7,6 +7,7 @@
 #include "pack.h"
 #include "sideband.h"
 #include "fetch-pack.h"
+#include "remote.h"
 #include "run-command.h"
 
 static int transfer_unpack_limit = -1;
@@ -558,14 +559,14 @@ static int get_pack(int xd[2], char **pack_lockfile)
 }
 
 static struct ref *do_fetch_pack(int fd[2],
+		const struct ref *orig_ref,
 		int nr_match,
 		char **match,
 		char **pack_lockfile)
 {
-	struct ref *ref;
+	struct ref *ref = copy_ref_list(orig_ref);
 	unsigned char sha1[20];
 
-	get_remote_heads(fd[0], &ref, 0, NULL, 0);
 	if (is_repository_shallow() && !server_supports("shallow"))
 		die("Server does not support shallow clients");
 	if (server_supports("multi_ack")) {
@@ -583,10 +584,6 @@ static struct ref *do_fetch_pack(int fd[2],
 			fprintf(stderr, "Server supports side-band\n");
 		use_sideband = 1;
 	}
-	if (!ref) {
-		packet_flush(fd[1]);
-		die("no matching remote head");
-	}
 	if (everything_local(&ref, nr_match, match)) {
 		packet_flush(fd[1]);
 		goto all_done;
@@ -660,7 +657,7 @@ static void fetch_pack_setup(void)
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret, nr_heads;
-	struct ref *ref;
+	struct ref *ref = NULL;
 	char *dest = NULL, **heads;
 
 	nr_heads = 0;
@@ -716,9 +713,34 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	if (!dest)
 		usage(fetch_pack_usage);
 
-	ref = fetch_pack(&args, dest, nr_heads, heads, NULL);
+	int fd[2];
+	struct child_process *conn = git_connect(fd, (char *)dest, args.uploadpack,
+                          args.verbose ? CONNECT_VERBOSE : 0);
+	if (conn) {
+		get_remote_heads(fd[0], &ref, 0, NULL, 0);
+
+		ref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL);
+		close(fd[0]);
+		close(fd[1]);
+		if (finish_connect(conn))
+			ref = NULL;
+	} else {
+		ref = NULL;
+	}
 	ret = !ref;
 
+	if (!ret && nr_heads) {
+		/* If the heads to pull were given, we should have
+		 * consumed all of them by matching the remote.
+		 * Otherwise, 'git-fetch remote no-such-ref' would
+		 * silently succeed without issuing an error.
+		 */
+		for (i = 0; i < nr_heads; i++)
+			if (heads[i] && heads[i][0]) {
+				error("no such remote ref %s", heads[i]);
+				ret = 1;
+			}
+	}
 	while (ref) {
 		printf("%s %s\n",
 		       sha1_to_hex(ref->old_sha1), ref->name);
@@ -729,16 +751,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 }
 
 struct ref *fetch_pack(struct fetch_pack_args *my_args,
+		       int fd[], pid_t pid,
+		       const struct ref *ref,
 		const char *dest,
 		int nr_heads,
 		char **heads,
 		char **pack_lockfile)
 {
-	int i, ret;
-	int fd[2];
-	struct child_process *conn;
-	struct ref *ref;
 	struct stat st;
+	struct ref *ref_cpy;
 
 	fetch_pack_setup();
 	memcpy(&args, my_args, sizeof(args));
@@ -747,29 +768,15 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 			st.st_mtime = 0;
 	}
 
-	conn = git_connect(fd, (char *)dest, args.uploadpack,
-                          args.verbose ? CONNECT_VERBOSE : 0);
 	if (heads && nr_heads)
 		nr_heads = remove_duplicates(nr_heads, heads);
-	ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile);
-	close(fd[0]);
-	close(fd[1]);
-	ret = finish_connect(conn);
-
-	if (!ret && nr_heads) {
-		/* If the heads to pull were given, we should have
-		 * consumed all of them by matching the remote.
-		 * Otherwise, 'git-fetch remote no-such-ref' would
-		 * silently succeed without issuing an error.
-		 */
-		for (i = 0; i < nr_heads; i++)
-			if (heads[i] && heads[i][0]) {
-				error("no such remote ref %s", heads[i]);
-				ret = 1;
-			}
+	if (!ref) {
+		packet_flush(fd[1]);
+		die("no matching remote head");
 	}
+	ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
 
-	if (!ret && args.depth > 0) {
+	if (args.depth > 0) {
 		struct cache_time mtime;
 		char *shallow = git_path("shallow");
 		int fd;
@@ -798,8 +805,5 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		}
 	}
 
-	if (ret)
-		ref = NULL;
-
-	return ref;
+	return ref_cpy;
 }
diff --git a/fetch-pack.h b/fetch-pack.h
index a7888ea..8d35ef6 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -16,6 +16,8 @@ struct fetch_pack_args
 };
 
 struct ref *fetch_pack(struct fetch_pack_args *args,
+		int fd[], struct child_process *conn,
+		const struct ref *ref,
 		const char *dest,
 		int nr_heads,
 		char **heads,
diff --git a/transport.c b/transport.c
index f4577b7..175cf2c 100644
--- a/transport.c
+++ b/transport.c
@@ -567,6 +567,8 @@ struct git_transport_data {
 	unsigned thin : 1;
 	unsigned keep : 1;
 	int depth;
+	struct child_process *conn;
+	int fd[2];
 	const char *uploadpack;
 	const char *receivepack;
 };
@@ -597,20 +599,20 @@ static int set_git_option(struct transport *connection,
 	return 1;
 }
 
+static int connect_setup(struct transport *transport)
+{
+	struct git_transport_data *data = transport->data;
+	data->conn = git_connect(data->fd, transport->url, data->uploadpack, 0);
+	return 0;
+}
+
 static struct ref *get_refs_via_connect(struct transport *transport)
 {
 	struct git_transport_data *data = transport->data;
 	struct ref *refs;
-	int fd[2];
-	char *dest = xstrdup(transport->url);
-	struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0);
 
-	get_remote_heads(fd[0], &refs, 0, NULL, 0);
-	packet_flush(fd[1]);
-
-	finish_connect(conn);
-
-	free(dest);
+	connect_setup(transport);
+	get_remote_heads(data->fd[0], &refs, 0, NULL, 0);
 
 	return refs;
 }
@@ -621,7 +623,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	struct git_transport_data *data = transport->data;
 	char **heads = xmalloc(nr_heads * sizeof(*heads));
 	char **origh = xmalloc(nr_heads * sizeof(*origh));
-	struct ref *refs;
+	const struct ref *refs;
 	char *dest = xstrdup(transport->url);
 	struct fetch_pack_args args;
 	int i;
@@ -636,13 +638,27 @@ static int fetch_refs_via_pack(struct transport *transport,
 
 	for (i = 0; i < nr_heads; i++)
 		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
-	refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);
+
+	refs = transport_get_remote_refs(transport);
+	if (!data->conn) {
+		struct ref *refs_tmp;
+		connect_setup(transport);
+		get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0);
+		free_refs(refs_tmp);
+	}
+
+	refs = fetch_pack(&args, data->fd, data->conn, transport->remote_refs, 
+			  dest, nr_heads, heads, &transport->pack_lockfile);
+	close(data->fd[0]);
+	close(data->fd[1]);
+	if (finish_connect(data->conn))
+		refs = NULL;
+	data->conn = NULL;
 
 	for (i = 0; i < nr_heads; i++)
 		free(origh[i]);
 	free(origh);
 	free(heads);
-	free_refs(refs);
 	free(dest);
 	return 0;
 }
@@ -723,6 +739,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		ret->disconnect = disconnect_git;
 
 		data->thin = 1;
+		data->conn = NULL;
 		data->uploadpack = "git-upload-pack";
 		if (remote && remote->uploadpack)
 			data->uploadpack = remote->uploadpack;
-- 
1.5.3.4.1206.g5f96

^ permalink raw reply related

* Re: Warning: cvsexportcommit considered dangerous
From: Steffen Prohaska @ 2007-11-04 18:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Alex Bennee, Robin Rosenberg
In-Reply-To: <Pine.LNX.4.64.0711041638270.4362@racer.site>


On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:

> ever since the up-to-date check was changed to use just one call to  
> "cvs
> status", a bug was present.  Now cvsexportcommit expects "cvs  
> status" to
> return the results in the same order as the file names were passed.
>
> This is not true, as I had to realise with one of my projects on
> sourceforge.
>
> Since time is so scarce on my side, I will not have time to fix  
> this bug,
> but will instead return to my old "commit by hand" procedure.

I introduced this 'optimization', which turned out to be a bug.
So, I feel responsible. Sorry for the trouble.

In August this was already recognized and a patch submitted:

http://marc.info/?t=118718458000004&r=1&w=2

I do not know why it wasn't applied. I forgot re-checking after my
vacation.

I put all on CC who replied to the patch in August.

	Steffen

^ permalink raw reply

* Re: [PATCH] fix display overlap between remote and local progress
From: Nicolas Pitre @ 2007-11-04 21:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711041331520.4362@racer.site>

On Sun, 4 Nov 2007, Johannes Schindelin wrote:

> Hi,
> 
> On Sun, 4 Nov 2007, Nicolas Pitre wrote:
> 
> > +#define SUFFIX "\e[K"  /* change to "        " if ANSI sequences don't work */ 
> 
> I am almost certain (without even testing) that cmd.exe has problems with 
> that.  It does not even understand colorisation.

That's what I was expecting.  This is why I suggested an alternative in 
the comment.


Nicolas

^ permalink raw reply

* Re: [PATCH 3/2] Use parse-options in builtin-clean
From: Pierre Habouzit @ 2007-11-04 21:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <Pine.LNX.4.64.0711042023440.4362@racer.site>

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Sun, Nov 04, 2007 at 08:24:31PM +0000, Johannes Schindelin wrote:
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	On Sun, 4 Nov 2007, Pierre Habouzit wrote:
> 
> 	> On Sun, Nov 04, 2007 at 07:02:21PM +0000, Shawn Bohrer wrote:
> 	> 
> 	> > +	for (i = 1; i < argc; i++) {
> 	> > +		const char *arg = argv[i];
> 	> > [...]
> 	> 
> 	>   Please, parse-options.c is now in next, please use it.
> 
> 	Something like this?

  something like this works for me :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] Build in ls-remote
From: Daniel Barkalow @ 2007-11-04 20:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This actually replaces peek-remote with ls-remote, since peek-remote
now handles everything. peek-remote remains an a second name for
ls-remote, although its help message now gives the "ls-remote" name.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Keeping scripts in .../examples makes the diffstat much less compelling. 
This actually replaces a 142-line shell script with one line in git.c. I'm 
not actually sure if this one is even worth keeping as an example, since 
the git-specific portions just delegated to C code anyway.

 Makefile                                           |    3 +--
 builtin-peek-remote.c => builtin-ls-remote.c       |   10 +++++-----
 builtin.h                                          |    2 +-
 .../examples/git-ls-remote.sh                      |    0 
 git.c                                              |    3 ++-
 5 files changed, 9 insertions(+), 9 deletions(-)
 rename builtin-peek-remote.c => builtin-ls-remote.c (83%)
 rename git-ls-remote.sh => contrib/examples/git-ls-remote.sh (100%)

diff --git a/Makefile b/Makefile
index 3ec1876..470e54a 100644
--- a/Makefile
+++ b/Makefile
@@ -210,7 +210,6 @@ BASIC_LDFLAGS =
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
 	git-clean.sh git-clone.sh git-commit.sh \
-	git-ls-remote.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
 	git-repack.sh git-request-pull.sh \
@@ -345,6 +344,7 @@ BUILTIN_OBJS = \
 	builtin-log.o \
 	builtin-ls-files.o \
 	builtin-ls-tree.o \
+	builtin-ls-remote.o \
 	builtin-mailinfo.o \
 	builtin-mailsplit.o \
 	builtin-merge-base.o \
@@ -352,7 +352,6 @@ BUILTIN_OBJS = \
 	builtin-mv.o \
 	builtin-name-rev.o \
 	builtin-pack-objects.o \
-	builtin-peek-remote.o \
 	builtin-prune.o \
 	builtin-prune-packed.o \
 	builtin-push.o \
diff --git a/builtin-peek-remote.c b/builtin-ls-remote.c
similarity index 83%
rename from builtin-peek-remote.c
rename to builtin-ls-remote.c
index b4106f5..003580c 100644
--- a/builtin-peek-remote.c
+++ b/builtin-ls-remote.c
@@ -3,10 +3,10 @@
 #include "transport.h"
 #include "remote.h"
 
-static const char peek_remote_usage[] =
-"git-peek-remote [--upload-pack=<git-upload-pack>] [<host>:]<directory>";
+static const char ls_remote_usage[] =
+"git-ls-remote [--upload-pack=<git-upload-pack>] [<host>:]<directory>";
 
-int cmd_peek_remote(int argc, const char **argv, const char *prefix)
+int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *dest = NULL;
@@ -43,14 +43,14 @@ int cmd_peek_remote(int argc, const char **argv, const char *prefix)
 				flags |= REF_NORMAL;
 				continue;
 			}
-			usage(peek_remote_usage);
+			usage(ls_remote_usage);
 		}
 		dest = arg;
 		break;
 	}
 
 	if (!dest || i != argc - 1)
-		usage(peek_remote_usage);
+		usage(ls_remote_usage);
 
 	transport = transport_get(NULL, dest);
 	if (uploadpack != NULL)
diff --git a/builtin.h b/builtin.h
index 2335c01..525107f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -48,6 +48,7 @@ extern int cmd_log(int argc, const char **argv, const char *prefix);
 extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_ls_remote(int argc, const char **argv, const char *prefix);
 extern int cmd_mailinfo(int argc, const char **argv, const char *prefix);
 extern int cmd_mailsplit(int argc, const char **argv, const char *prefix);
 extern int cmd_merge_base(int argc, const char **argv, const char *prefix);
@@ -55,7 +56,6 @@ extern int cmd_merge_file(int argc, const char **argv, const char *prefix);
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
-extern int cmd_peek_remote(int argc, const char **argv, const char *prefix);
 extern int cmd_pickaxe(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
diff --git a/git-ls-remote.sh b/contrib/examples/git-ls-remote.sh
similarity index 100%
rename from git-ls-remote.sh
rename to contrib/examples/git-ls-remote.sh
diff --git a/git.c b/git.c
index 19a2172..b173f22 100644
--- a/git.c
+++ b/git.c
@@ -326,6 +326,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
 		{ "ls-files", cmd_ls_files, RUN_SETUP },
 		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+		{ "ls-remote", cmd_ls_remote },
 		{ "mailinfo", cmd_mailinfo },
 		{ "mailsplit", cmd_mailsplit },
 		{ "merge-base", cmd_merge_base, RUN_SETUP },
@@ -333,7 +334,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 		{ "name-rev", cmd_name_rev, RUN_SETUP },
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-		{ "peek-remote", cmd_peek_remote },
+		{ "peek-remote", cmd_ls_remote },
 		{ "pickaxe", cmd_blame, RUN_SETUP },
 		{ "prune", cmd_prune, RUN_SETUP },
 		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-- 
1.5.3.4.1206.g5f96

^ permalink raw reply related

* [PATCH qgit] Update to latest --early-output git log patch
From: Marco Costalba @ 2007-11-04 20:50 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Linus Torvalds

Fix broken implementation after Linus updated his
early output patch to version with improve output format.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 src/git.h           |    2 +-
 src/git_startup.cpp |   10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/git.h b/src/git.h
index 92879fb..789fea4 100644
--- a/src/git.h
+++ b/src/git.h
@@ -251,7 +251,7 @@ private:
 	bool startParseProc(SCList initCmd, FileHistory* fh, SCRef buf);
 	bool tryFollowRenames(FileHistory* fh);
 	bool populateRenamedPatches(SCRef sha, SCList nn, FileHistory* fh,
QStringList* on, bool bt);
-	void doEarlyOutput(Rev* rev, int* start);
+	void doEarlyOutput(Rev* rev, const QByteArray& ba, int* start);
 	int addChunk(FileHistory* fh, const QByteArray& ba, int ofs);
 	void parseDiffFormat(RevFile& rf, SCRef buf);
 	void parseDiffFormatLine(RevFile& rf, SCRef line, int parNum);
diff --git a/src/git_startup.cpp b/src/git_startup.cpp
index df272fc..090d5f9 100644
--- a/src/git_startup.cpp
+++ b/src/git_startup.cpp
@@ -841,10 +841,10 @@ void Git::loadFileNames() {
 	indexTree();
 }

-void Git::doEarlyOutput(Rev* rev, int* start) {
+void Git::doEarlyOutput(Rev* rev, const QByteArray& ba, int* start) {

 	delete rev;
-	*start += QString("Final output:\n").length();
+	*start = ba.indexOf('\n', *start) + 1;

 	Rev* cl = NULL;
 	const Rev* r = revLookup(ZERO_SHA);
@@ -870,7 +870,7 @@ int Git::addChunk(FileHistory* fh, const
QByteArray& ba, int start) {
 		rev = new Rev(ba, start, fh->revOrder.count(), &nextStart,
!isMainHistory(fh));

 		if (nextStart == -2)
-			doEarlyOutput(rev, &start);
+			doEarlyOutput(rev, ba, &start);

 	} while (nextStart == -2);

@@ -1377,8 +1377,8 @@ int Rev::indexData(bool quick, bool withDiff) const {
 	if (start > last) // offset 'start' points to the char after "commit "
 		return -1;

-	if (uint(ba.at(start) == 'u'))
-		return -2; // "Final output:", let caller handle this
+	if (uint(ba.at(start) == 'u')) // "Final output:", let caller handle this
+		return (ba.indexOf('\n', start) != -1 ? -2 : -1);

 	// take in account --boundary and --left-right options
 	startOfs = uint(ba.at(start) == '-' || ba.at(start) == '<' ||
ba.at(start) == '>');
-- 
1.5.3.5.565.g985b6

^ permalink raw reply related

* [PATCH] Fix an infinite loop in sq_quote_buf().
From: Johannes Sixt @ 2007-11-04 20:26 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

sq_quote_buf() treats single-quotes and exclamation marks specially, but
it incorrectly parsed the input for single-quotes and backslashes.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 quote.c          |    2 +-
 t/t5510-fetch.sh |    7 +++++++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/quote.c b/quote.c
index 482be05..919d092 100644
--- a/quote.c
+++ b/quote.c
@@ -26,7 +26,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
 
 	strbuf_addch(dst, '\'');
 	while (*src) {
-		size_t len = strcspn(src, "'\\");
+		size_t len = strcspn(src, "'!");
 		strbuf_add(dst, src, len);
 		src += len;
 		while (need_bs_quote(*src)) {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d217657..aad863d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -208,4 +208,11 @@ test_expect_success 'fetch with a non-applying branch.<name>.merge' '
 	git fetch blub
 '
 
+# the strange name is: a\!'b
+test_expect_success 'quoting of a strangely named repo' '
+	! git fetch "a\\!'\''b" > result 2>&1 &&
+	cat result &&
+	grep "fatal: '\''a\\\\!'\''b'\''" result
+'
+
 test_done
-- 
1.5.3.4.315.g2ce38

^ permalink raw reply related

* [PATCH 3/2] Use parse-options in builtin-clean
From: Johannes Schindelin @ 2007-11-04 20:24 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <20071104194129.GA4207@artemis.corp>


Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sun, 4 Nov 2007, Pierre Habouzit wrote:

	> On Sun, Nov 04, 2007 at 07:02:21PM +0000, Shawn Bohrer wrote:
	> 
	> > +	for (i = 1; i < argc; i++) {
	> > +		const char *arg = argv[i];
	> > [...]
	> 
	>   Please, parse-options.c is now in next, please use it.

	Something like this?

 builtin-clean.c |   71 ++++++++++++++++++++----------------------------------
 1 files changed, 26 insertions(+), 45 deletions(-)

diff --git a/builtin-clean.c b/builtin-clean.c
index 4141eb4..d6fc2ad 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -9,81 +9,62 @@
 #include "builtin.h"
 #include "cache.h"
 #include "dir.h"
+#include "parse-options.h"
 
-static int disabled = 1;
+static int force = 0;
 static int show_only = 0;
 static int remove_directories = 0;
 static int quiet = 0;
 static int ignored = 0;
 static int ignored_only = 0;
 
-static const char builtin_clean_usage[] =
-"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+static const char *const builtin_clean_usage[] = {
+	"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...",
+	NULL
+};
 
 static int git_clean_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "clean.requireforce")) {
-		disabled = git_config_bool(var, value);
+		force = !git_config_bool(var, value);
 	}
 	return 0;
 }
 
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
-	int i, j;
+	int j;
 	struct strbuf directory;
 	struct dir_struct dir;
 	const char *path = ".";
 	const char *base = "";
 	int baselen = 0;
 	static const char **pathspec;
+	struct option options[] = {
+		OPT__QUIET(&quiet),
+		OPT__DRY_RUN(&show_only),
+		OPT_BOOLEAN('f', NULL, &force, "force"),
+		OPT_BOOLEAN('d', NULL, &remove_directories,
+				"remove whole directories"),
+		OPT_BOOLEAN('x', NULL, &ignored, "remove ignored files, too"),
+		OPT_BOOLEAN('X', NULL, &ignored_only,
+				"remove only ignored files"),
+		OPT_END()
+	};
 
-	memset(&dir, 0, sizeof(dir));
 	git_config(git_clean_config);
+	argc = parse_options(argc, argv, options, builtin_clean_usage, 0);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (arg[0] != '-')
-			break;
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-n")) {
-			show_only = 1;
-			disabled = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-f")) {
-			disabled = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-d")) {
-			remove_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-q")) {
-			quiet = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x")) {
-			ignored = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-X")) {
-			ignored_only = 1;
-			dir.show_ignored =1;
-			dir.exclude_per_dir = ".gitignore";
-			continue;
-		}
-		usage(builtin_clean_usage);
+	memset(&dir, 0, sizeof(dir));
+	if (ignored_only) {
+		dir.show_ignored =1;
+		dir.exclude_per_dir = ".gitignore";
 	}
 
 	if (ignored && ignored_only)
 		die("-x and -X cannot be used together");
 
-	if (disabled)
+	if (!show_only && !force)
 		die("clean.requireForce set and -n or -f not given; refusing to clean");
 
 	dir.show_other_directories = 1;
@@ -96,7 +77,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 	read_cache();
 	read_directory(&dir, path, base, baselen, pathspec);
 	strbuf_init(&directory, 0);
-- 
1.5.3.5.1549.g91a3

^ permalink raw reply related

* [PATCH 3/2] Enhance --early-output format
From: Linus Torvalds @ 2007-11-04 20:12 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Marco Costalba, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0711041004220.15101@woody.linux-foundation.org>


This makes --early-output a bit more advanced, and actually makes it 
generate multiple "Final output:" headers as it updates things 
asynchronously. I realize that the "Final output:" line is now illogical, 
since it's not really final until it also says "done", but 

It now _always_ generates a "Final output:" header in front of any commit 
list, and that output header gives you a *guess* at the maximum number of 
commits available. However, it should be noted that the guess can be 
completely off: I do a reasonable job estimating it, but it is not meant 
to be exact. 

So what happens is that you may get output like this:

 - at 0.1 seconds:

	Final output: 2 incomplete
	.. 2 commits listed ..

 - half a second later:

	Final output: 33 incomplete
	.. 33 commits listed ..

 - another half a second after that:	

	Final output: 71 incomplete
	.. 71 commits listed ..

 - another half second later:

	Final output: 136 incomplete
	.. 100 commits listed: we hit the --early-output limit, and
	.. will only output 100 commits, and after this you'll not
	.. see an "incomplete" report any more since you got as much
	.. early output as you asked for!

 - .. and then finally:

	Final output: 73106 done
	.. all the commits ..

The above is a real-life scenario on my current kernel tree after having 
flushed all the caches.

Tested with the experimental gitk patch that Paul sent out, and by looking 
at the actual log output (and verifying that my commit count guesses 
actually match real life fairly well).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

On Sun, 4 Nov 2007, Linus Torvalds wrote:
> 
> I'm looking at it now, I'll have to think about this a bit more. It might 
> be trivial to fix, but this thing has real potential for being subtle.

It wasn't totally trivial, but it doesn't seem to be excessively subtle 
either. About half the patch is moving around some code to look at whether 
the commit is interesting or not and rewriting the parents, so that it can 
be shared with the revision walker.

 builtin-log.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 revision.c    |   63 +++++++++++++++++++++++-----------------
 revision.h    |    8 +++++
 3 files changed, 119 insertions(+), 40 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 707add2..268a7af 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -77,17 +77,85 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	}
 }
 
+/*
+ * This gives a rough estimate for how many commits we
+ * will print out in the list.
+ */
+static int estimate_commit_count(struct rev_info *rev, struct commit_list *list)
+{
+	int n = 0;
+
+	while (list) {
+		struct commit *commit = list->item;
+		unsigned int flags = commit->object.flags;
+
+		list = list->next;
+		if (flags & UNINTERESTING)
+			continue;
+		if (rev->prune_fn && rev->dense && !(flags & TREECHANGE)) {
+			if (commit->parents && !commit->parents->next)
+				continue;
+		}
+		n++;
+	}
+	return n;
+}
+
+static void show_early_header(struct rev_info *rev, const char *stage, int nr)
+{
+	if (rev->shown_one) {
+		rev->shown_one = 0;
+		if (rev->commit_format != CMIT_FMT_ONELINE)
+			putchar(rev->diffopt.line_termination);
+	}
+	printf("Final output: %d %s\n", nr, stage);
+}
+
+struct itimerval early_output_timer;
+
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
 	int i = revs->early_output;
+	int show_header = 1;
 
 	sort_in_topological_order(&list, revs->lifo);
 	while (list && i) {
 		struct commit *commit = list->item;
-		log_tree_commit(revs, commit);
+		switch (simplify_commit(revs, commit)) {
+		case commit_show:
+			if (show_header) {
+				int n = estimate_commit_count(revs, list);
+				show_early_header(revs, "incomplete", n);
+				show_header = 0;
+			}
+			log_tree_commit(revs, commit);
+			i--;
+			break;
+		case commit_ignore:
+			break;
+		case commit_error:
+			return;
+		}
 		list = list->next;
-		i--;
 	}
+
+	/* Did we already get enough commits for the early output? */
+	if (!i)
+		return;
+
+	/*
+	 * ..if no, then repeat it twice a second until we
+	 * do.
+	 *
+	 * NOTE! We don't use "it_interval", because if the
+	 * reader isn't listening, we want our output to be
+	 * throttled by the writing, and not have the timer
+	 * trigger every second even if we're blocked on a
+	 * reader!
+	 */
+	early_output_timer.it_value.tv_sec = 0;
+	early_output_timer.it_value.tv_usec = 500000;
+	setitimer(ITIMER_REAL, &early_output_timer, NULL);
 }
 
 static void early_output(int signal)
@@ -98,7 +166,6 @@ static void early_output(int signal)
 static void setup_early_output(struct rev_info *rev)
 {
 	struct sigaction sa;
-	struct itimerval v;
 
 	/*
 	 * Set up the signal handler, minimally intrusively:
@@ -120,21 +187,16 @@ static void setup_early_output(struct rev_info *rev)
 	 *
 	 * This is a one-time-only trigger.
 	 */
-	memset(&v, 0, sizeof(v));
-	v.it_value.tv_sec = 0;
-	v.it_value.tv_usec = 100000;
-	setitimer(ITIMER_REAL, &v, NULL);
+	early_output_timer.it_value.tv_sec = 0;
+	early_output_timer.it_value.tv_usec = 100000;
+	setitimer(ITIMER_REAL, &early_output_timer, NULL);
 }
 
 static void finish_early_output(struct rev_info *rev)
 {
+	int n = estimate_commit_count(rev, rev->commits);
 	signal(SIGALRM, SIG_IGN);
-	if (rev->shown_one) {
-		rev->shown_one = 0;
-		if (rev->commit_format != CMIT_FMT_ONELINE)
-			putchar(rev->diffopt.line_termination);
-	}
-	printf("Final output:\n");
+	show_early_header(rev, "done", n);
 }
 
 static int cmd_log_walk(struct rev_info *rev)
diff --git a/revision.c b/revision.c
index 26610bb..5d6f208 100644
--- a/revision.c
+++ b/revision.c
@@ -1398,6 +1398,36 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
 			   commit->buffer, strlen(commit->buffer));
 }
 
+enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
+{
+	if (commit->object.flags & SHOWN)
+		return commit_ignore;
+	if (revs->unpacked && has_sha1_pack(commit->object.sha1, revs->ignore_packed))
+		return commit_ignore;
+	if (commit->object.flags & UNINTERESTING)
+		return commit_ignore;
+	if (revs->min_age != -1 && (commit->date > revs->min_age))
+		return commit_ignore;
+	if (revs->no_merges && commit->parents && commit->parents->next)
+		return commit_ignore;
+	if (!commit_match(commit, revs))
+		return commit_ignore;
+	if (revs->prune_fn && revs->dense) {
+		/* Commit without changes? */
+		if (!(commit->object.flags & TREECHANGE)) {
+			/* drop merges unless we want parenthood */
+			if (!revs->parents)
+				return commit_ignore;
+			/* non-merge - always ignore it */
+			if (!commit->parents || !commit->parents->next)
+				return commit_ignore;
+		}
+		if (revs->parents && rewrite_parents(revs, commit) < 0)
+			return commit_error;
+	}
+	return commit_show;
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
 	if (!revs->commits)
@@ -1425,36 +1455,15 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			if (add_parents_to_list(revs, commit, &revs->commits) < 0)
 				return NULL;
 		}
-		if (commit->object.flags & SHOWN)
-			continue;
-
-		if (revs->unpacked && has_sha1_pack(commit->object.sha1,
-						    revs->ignore_packed))
-		    continue;
 
-		if (commit->object.flags & UNINTERESTING)
-			continue;
-		if (revs->min_age != -1 && (commit->date > revs->min_age))
-			continue;
-		if (revs->no_merges &&
-		    commit->parents && commit->parents->next)
-			continue;
-		if (!commit_match(commit, revs))
+		switch (simplify_commit(revs, commit)) {
+		case commit_ignore:
 			continue;
-		if (revs->prune_fn && revs->dense) {
-			/* Commit without changes? */
-			if (!(commit->object.flags & TREECHANGE)) {
-				/* drop merges unless we want parenthood */
-				if (!revs->parents)
-					continue;
-				/* non-merge - always ignore it */
-				if (!commit->parents || !commit->parents->next)
-					continue;
-			}
-			if (revs->parents && rewrite_parents(revs, commit) < 0)
-				return NULL;
+		case commit_error:
+			return -1;
+		default:
+			return commit;
 		}
-		return commit;
 	} while (revs->commits);
 	return NULL;
 }
diff --git a/revision.h b/revision.h
index d8a5a50..2232247 100644
--- a/revision.h
+++ b/revision.h
@@ -133,4 +133,12 @@ extern void add_object(struct object *obj,
 
 extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);
 
+enum commit_action {
+	commit_ignore,
+	commit_show,
+	commit_error
+};
+
+extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit);
+
 #endif

^ permalink raw reply related

* [PATCH] upload-pack: Use finish_{command,async}() instead of waitpid().
From: Johannes Sixt @ 2007-11-04 19:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

upload-pack spawns two processes, rev-list and pack-objects, and carefully
monitors their status so that it can report failure to the remote end.
This change removes the complicated procedures on the grounds of the
following observations:

- If everything is OK, rev-list closes its output pipe end, upon which
  pack-objects (which reads from the pipe) sees EOF and terminates itself,
  closing its output (and error) pipes. upload-pack reads from both until
  it sees EOF in both. It collects the exit codes of the child processes
  (which indicate success) and terminates successfully.

- If rev-list sees an error, it closes its output and terminates with
  failure. pack-objects sees EOF in its input and terminates successfully.
  Again upload-pack reads its inputs until EOF. When it now collects
  the exit codes of its child processes, it notices the failure of rev-list
  and signals failure to the remote end.

- If pack-objects sees an error, it terminates with failure. Since this
  breaks the pipe to rev-list, rev-list is killed with SIGPIPE.
  upload-pack reads its input until EOF, then collects the exit codes of
  the child processes, notices their failures, and signals failure to the
  remote end.

- If upload-pack itself dies unexpectedly, pack-objects is killed with
  SIGPIPE, and subsequently also rev-list.

The upshot of this is that precise monitoring of child processes is not
required because both terminate if either one of them dies unexpectedly.
This allows us to use finish_command() and finish_async() instead of
an explicit waitpid(2) call.

The change is smaller than it looks because most of it only reduces the
indentation of a large part of the inner loop.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
	This patch allows us to reduce the differences to the
	MinGW port even further. It goes on top of js/forkexec
	(which meanwhile is in master).

	The test case checks for failures in rev-list (a missing
	object). Any hints how to trigger a failure in pack-objects
	that does not also trigger in rev-list would be welcome.

	BTW, I don't know what it means to process zombies if the
	parent does not waitpid(), but just terminates. Does this
	work as expected, ie. no zombies are left behind?

	-- Hannes

 t/t5530-upload-pack-error.sh |   49 +++++++++++
 upload-pack.c                |  192 +++++++++++++++++-------------------------
 2 files changed, 126 insertions(+), 115 deletions(-)
 create mode 100755 t/t5530-upload-pack-error.sh

diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh
new file mode 100755
index 0000000..9923ba0
--- /dev/null
+++ b/t/t5530-upload-pack-error.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='errors in upload-pack'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+test_expect_success 'setup and corrupt repository' '
+
+	echo file >file &&
+	git add file &&
+	git rev-parse :file &&
+	git commit -a -m original &&
+	test_tick &&
+	echo changed >file &&
+	git commit -a -m changed &&
+	rm -f .git/objects/f7/3f3093ff865c514c6c51f867e35f693487d0d3
+
+'
+
+test_expect_failure 'fsck fails' '
+
+	git fsck
+'
+
+test_expect_failure 'upload pack fails due to error in rev-list' '
+
+	echo "0032want $(git rev-parse HEAD)
+00000009done
+0000" | git-upload-pack . > /dev/null
+
+'
+
+test_expect_success 'create empty repository' '
+
+	mkdir foo &&
+	cd foo &&
+	git init
+
+'
+
+test_expect_failure 'fetch fails' '
+
+	git fetch .. master
+
+'
+
+test_done
diff --git a/upload-pack.c b/upload-pack.c
index 6799468..7e04311 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -144,6 +144,7 @@ static void create_pack_file(void)
 	char abort_msg[] = "aborting due to possible repository "
 		"corruption on the remote side.";
 	int buffered = -1;
+	ssize_t sz;
 	const char *argv[10];
 	int arg = 0;
 
@@ -168,22 +169,15 @@ static void create_pack_file(void)
 	pack_objects.git_cmd = 1;
 	pack_objects.argv = argv;
 
-	if (start_command(&pack_objects)) {
-		/* daemon sets things up to ignore TERM */
-		kill(rev_list.pid, SIGKILL);
+	if (start_command(&pack_objects))
 		die("git-upload-pack: unable to fork git-pack-objects");
-	}
 
 	/* We read from pack_objects.err to capture stderr output for
 	 * progress bar, and pack_objects.out to capture the pack data.
 	 */
 
 	while (1) {
-		const char *who;
 		struct pollfd pfd[2];
-		pid_t pid;
-		int status;
-		ssize_t sz;
 		int pe, pu, pollsize;
 
 		reset_timeout();
@@ -204,123 +198,91 @@ static void create_pack_file(void)
 			pollsize++;
 		}
 
-		if (pollsize) {
-			if (poll(pfd, pollsize, -1) < 0) {
-				if (errno != EINTR) {
-					error("poll failed, resuming: %s",
-					      strerror(errno));
-					sleep(1);
-				}
-				continue;
-			}
-			if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
-				/* Data ready; we keep the last byte
-				 * to ourselves in case we detect
-				 * broken rev-list, so that we can
-				 * leave the stream corrupted.  This
-				 * is unfortunate -- unpack-objects
-				 * would happily accept a valid pack
-				 * data with trailing garbage, so
-				 * appending garbage after we pass all
-				 * the pack data is not good enough to
-				 * signal breakage to downstream.
-				 */
-				char *cp = data;
-				ssize_t outsz = 0;
-				if (0 <= buffered) {
-					*cp++ = buffered;
-					outsz++;
-				}
-				sz = xread(pack_objects.out, cp,
-					  sizeof(data) - outsz);
-				if (0 < sz)
-						;
-				else if (sz == 0) {
-					close(pack_objects.out);
-					pack_objects.out = -1;
-				}
-				else
-					goto fail;
-				sz += outsz;
-				if (1 < sz) {
-					buffered = data[sz-1] & 0xFF;
-					sz--;
-				}
-				else
-					buffered = -1;
-				sz = send_client_data(1, data, sz);
-				if (sz < 0)
-					goto fail;
-			}
-			if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) {
-				/* Status ready; we ship that in the side-band
-				 * or dump to the standard error.
-				 */
-				sz = xread(pack_objects.err, progress,
-					  sizeof(progress));
-				if (0 < sz)
-					send_client_data(2, progress, sz);
-				else if (sz == 0) {
-					close(pack_objects.err);
-					pack_objects.err = -1;
-				}
-				else
-					goto fail;
+		if (!pollsize)
+			break;
+
+		if (poll(pfd, pollsize, -1) < 0) {
+			if (errno != EINTR) {
+				error("poll failed, resuming: %s",
+				      strerror(errno));
+				sleep(1);
 			}
+			continue;
 		}
-
-		/* See if the children are still there */
-		if (rev_list.pid || pack_objects.pid) {
-			pid = waitpid(-1, &status, WNOHANG);
-			if (!pid)
-				continue;
-			who = ((pid == rev_list.pid) ? "git-rev-list" :
-			       (pid == pack_objects.pid) ? "git-pack-objects" :
-			       NULL);
-			if (!who) {
-				if (pid < 0) {
-					error("git-upload-pack: %s",
-					      strerror(errno));
-					goto fail;
-				}
-				error("git-upload-pack: we weren't "
-				      "waiting for %d", pid);
-				continue;
+		if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
+			/* Data ready; we keep the last byte to ourselves
+			 * in case we detect broken rev-list, so that we
+			 * can leave the stream corrupted.  This is
+			 * unfortunate -- unpack-objects would happily
+			 * accept a valid packdata with trailing garbage,
+			 * so appending garbage after we pass all the
+			 * pack data is not good enough to signal
+			 * breakage to downstream.
+			 */
+			char *cp = data;
+			ssize_t outsz = 0;
+			if (0 <= buffered) {
+				*cp++ = buffered;
+				outsz++;
+			}
+			sz = xread(pack_objects.out, cp,
+				  sizeof(data) - outsz);
+			if (0 < sz)
+					;
+			else if (sz == 0) {
+				close(pack_objects.out);
+				pack_objects.out = -1;
 			}
-			if (!WIFEXITED(status) || WEXITSTATUS(status) > 0) {
-				error("git-upload-pack: %s died with error.",
-				      who);
+			else
 				goto fail;
+			sz += outsz;
+			if (1 < sz) {
+				buffered = data[sz-1] & 0xFF;
+				sz--;
 			}
-			if (pid == rev_list.pid)
-				rev_list.pid = 0;
-			if (pid == pack_objects.pid)
-				pack_objects.pid = 0;
-			if (rev_list.pid || pack_objects.pid)
-				continue;
-		}
-
-		/* both died happily */
-		if (pollsize)
-			continue;
-
-		/* flush the data */
-		if (0 <= buffered) {
-			data[0] = buffered;
-			sz = send_client_data(1, data, 1);
+			else
+				buffered = -1;
+			sz = send_client_data(1, data, sz);
 			if (sz < 0)
 				goto fail;
-			fprintf(stderr, "flushed.\n");
 		}
-		if (use_sideband)
-			packet_flush(1);
-		return;
+		if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) {
+			/* Status ready; we ship that in the side-band
+			 * or dump to the standard error.
+			 */
+			sz = xread(pack_objects.err, progress,
+				  sizeof(progress));
+			if (0 < sz)
+				send_client_data(2, progress, sz);
+			else if (sz == 0) {
+				close(pack_objects.err);
+				pack_objects.err = -1;
+			}
+			else
+				goto fail;
+		}
+	}
+
+	if (finish_command(&pack_objects)) {
+		error("git-upload-pack: git-pack-objects died with error.");
+		goto fail;
+	}
+	if (finish_async(&rev_list))
+		goto fail;	/* error was already reported */
+
+	/* flush the data */
+	if (0 <= buffered) {
+		data[0] = buffered;
+		sz = send_client_data(1, data, 1);
+		if (sz < 0)
+			goto fail;
+		fprintf(stderr, "flushed.\n");
 	}
+	if (use_sideband)
+		packet_flush(1);
+	return;
+
  fail:
-	if (pack_objects.pid)
-		kill(pack_objects.pid, SIGKILL);
-	if (rev_list.pid)
-		kill(rev_list.pid, SIGKILL);
 	send_client_data(3, abort_msg, sizeof(abort_msg));
 	die("git-upload-pack: %s", abort_msg);
 }
-- 
1.5.3.4.315.g2ce38

^ permalink raw reply related

* Re: [PATCH] Make git-clean a builtin
From: Pierre Habouzit @ 2007-11-04 19:41 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <11942029474058-git-send-email-shawn.bohrer@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

On Sun, Nov 04, 2007 at 07:02:21PM +0000, Shawn Bohrer wrote:

> +	for (i = 1; i < argc; i++) {
> +		const char *arg = argv[i];
> +
> +		if (arg[0] != '-')
> +			break;
> +		if (!strcmp(arg, "--")) {
> +			i++;
> +			break;
> +		}
> +		if (!strcmp(arg, "-n")) {
> +			show_only = 1;
> +			disabled = 0;
> +			continue;
> +		}
> +		if (!strcmp(arg, "-f")) {
> +			disabled = 0;
> +			continue;
> +		}
> +		if (!strcmp(arg, "-d")) {
> +			remove_directories = 1;
> +			continue;
> +		}
> +		if (!strcmp(arg, "-q")) {
> +			quiet = 1;
> +			continue;
> +		}
> +		if (!strcmp(arg, "-x")) {
> +			ignored = 1;
> +			continue;
> +		}
> +		if (!strcmp(arg, "-X")) {
> +			ignored_only = 1;
> +			dir.show_ignored =1;
> +			dir.exclude_per_dir = ".gitignore";
> +			continue;
> +		}
> +		usage(builtin_clean_usage);

  Please, parse-options.c is now in next, please use it.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: Johannes Schindelin @ 2007-11-04 19:15 UTC (permalink / raw)
  To: git, Rene Scharfe, gitster
In-Reply-To: <Pine.LNX.4.64.0711041912190.4362@racer.site>


Use the new function interp_find_active() to avoid calculating the
unique hash names, and other things, when they are not even asked for.

Unfortunately, we cannot reuse the result of that function, which
would be cleaner: there are more users than just git log.  Most
notably, git-archive with "$Format:...$" substitution.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	So I found another reason why the function has to be called
	everytime.  But this reason appeals to me much more.

	Originally, I wanted to do this differently, by providing a
	function which generates the substitutions, but the header
	parsing makes that infeasible.

 pretty.c |   55 ++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/pretty.c b/pretty.c
index 490cede..241e91c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -393,6 +393,7 @@ void format_commit_message(const struct commit *commit,
 	int i;
 	enum { HEADER, SUBJECT, BODY } state;
 	const char *msg = commit->buffer;
+	char *active = interp_find_active(format, table, ARRAY_SIZE(table));
 
 	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
 		die("invalid interp table!");
@@ -407,12 +408,18 @@ void format_commit_message(const struct commit *commit,
 	/* these depend on the commit */
 	if (!commit->object.parsed)
 		parse_object(commit->object.sha1);
-	interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
-	interp_set_entry(table, IHASH_ABBREV,
+	if (active[IHASH])
+		interp_set_entry(table, IHASH,
+				sha1_to_hex(commit->object.sha1));
+	if (active[IHASH_ABBREV])
+		interp_set_entry(table, IHASH_ABBREV,
 			find_unique_abbrev(commit->object.sha1,
 				DEFAULT_ABBREV));
-	interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
-	interp_set_entry(table, ITREE_ABBREV,
+	if (active[ITREE])
+		interp_set_entry(table, ITREE,
+				sha1_to_hex(commit->tree->object.sha1));
+	if (active[ITREE_ABBREV])
+		interp_set_entry(table, ITREE_ABBREV,
 			find_unique_abbrev(commit->tree->object.sha1,
 				DEFAULT_ABBREV));
 	interp_set_entry(table, ILEFT_RIGHT,
@@ -422,22 +429,27 @@ void format_commit_message(const struct commit *commit,
 			 ? "<"
 			 : ">");
 
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			sha1_to_hex(p->item->object.sha1));
-	interp_set_entry(table, IPARENTS, parents + 1);
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			find_unique_abbrev(p->item->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+	if (active[IPARENTS]) {
+		parents[1] = 0;
+		for (i = 0, p = commit->parents;
+				p && i < sizeof(parents) - 1;
+				p = p->next)
+			i += snprintf(parents + i, sizeof(parents) - i - 1,
+				" %s", sha1_to_hex(p->item->object.sha1));
+		interp_set_entry(table, IPARENTS, parents + 1);
+	}
+
+	if (active[IPARENTS_ABBREV]) {
+		parents[1] = 0;
+		for (i = 0, p = commit->parents;
+				p && i < sizeof(parents) - 1;
+				p = p->next)
+			i += snprintf(parents + i, sizeof(parents) - i - 1,
+				" %s",
+				find_unique_abbrev(p->item->object.sha1,
+					DEFAULT_ABBREV));
+		interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+	}
 
 	for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
 		int eol;
@@ -464,7 +476,7 @@ void format_commit_message(const struct commit *commit,
 				xmemdupz(msg + i + 9, eol - i - 9);
 		i = eol;
 	}
-	if (msg[i])
+	if (active[IBODY] && msg[i])
 		table[IBODY].value = xstrdup(msg + i);
 
 	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
@@ -476,6 +488,7 @@ void format_commit_message(const struct commit *commit,
 	}
 	strbuf_setlen(sb, sb->len + len);
 	interp_clear_table(table, ARRAY_SIZE(table));
+	free(active);
 }
 
 static void pp_header(enum cmit_fmt fmt,
-- 
1.5.3.5.1549.g91a3

^ permalink raw reply related

* [PATCH 2/3] interpolate.[ch]: Add a function to find which interpolations are active.
From: Johannes Schindelin @ 2007-11-04 19:15 UTC (permalink / raw)
  To: git, Rene Scharfe, gitster
In-Reply-To: <Pine.LNX.4.64.0711041912190.4362@racer.site>


Some substitutions require pretty expensive operations.  So it make
sense to find out which are needed to begin with.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 interpolate.c |   20 ++++++++++++++++++++
 interpolate.h |    2 ++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/interpolate.c b/interpolate.c
index 6ef53f2..80eeb36 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -102,3 +102,23 @@ unsigned long interpolate(char *result, unsigned long reslen,
 		*dest = '\0';
 	return newlen;
 }
+
+char *interp_find_active(const char *orig,
+		const struct interp *interps, int ninterps)
+{
+	char *result = xcalloc(1, ninterps);
+	char c;
+	int i;
+
+	while ((c = *(orig++)))
+		if (c == '%')
+			/* Try to match an interpolation string. */
+			for (i = 0; i < ninterps; i++)
+				if (!prefixcmp(orig, interps[i].name + 1)) {
+					result[i] = 1;
+					orig += strlen(interps[i].name + 1);
+					break;
+				}
+
+	return result;
+}
diff --git a/interpolate.h b/interpolate.h
index 77407e6..2d197c5 100644
--- a/interpolate.h
+++ b/interpolate.h
@@ -22,5 +22,7 @@ extern void interp_clear_table(struct interp *table, int ninterps);
 extern unsigned long interpolate(char *result, unsigned long reslen,
 				 const char *orig,
 				 const struct interp *interps, int ninterps);
+extern char *interp_find_active(const char *orig,
+				const struct interp *interps, int ninterps);
 
 #endif /* INTERPOLATE_H */
-- 
1.5.3.5.1549.g91a3

^ permalink raw reply related

* [PATCH 1/3] Split off the pretty print stuff into its own file
From: Johannes Schindelin @ 2007-11-04 19:15 UTC (permalink / raw)
  To: git, Rene Scharfe, gitster
In-Reply-To: <Pine.LNX.4.64.0711041912190.4362@racer.site>


The file commit.c got quite large, but it does not have to be: the
code concerning pretty printing is pretty well contained.  In fact,
this commit just splits it off into pretty.c, leaving commit.c with
just 672 lines.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I know, I suggested using "format-patch -C -C" for this case, but
	the response was correct in that it seems funny.

	AFAICT this is a verbatim move of two hunks from commit.c to
	pretty.c, and the usual #include mantra in front of the latter to 
	make it	compile.

 Makefile |    2 +-
 commit.c |  718 -------------------------------------------------------------
 pretty.c |  723 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 724 insertions(+), 719 deletions(-)
 create mode 100644 pretty.c

diff --git a/Makefile b/Makefile
index 19a48f5..4c5f864 100644
--- a/Makefile
+++ b/Makefile
@@ -299,7 +299,7 @@ DIFF_OBJS = \
 LIB_OBJS = \
 	blob.o commit.o connect.o csum-file.o cache-tree.o base85.o \
 	date.o diff-delta.o entry.o exec_cmd.o ident.o \
-	interpolate.o hash.o \
+	pretty.o interpolate.o hash.o \
 	lockfile.o \
 	patch-ids.o \
 	object.o pack-check.o pack-write.o patch-delta.o path.o pkt-line.o \
diff --git a/commit.c b/commit.c
index ab4eb8b..f074811 100644
--- a/commit.c
+++ b/commit.c
@@ -3,7 +3,6 @@
 #include "commit.h"
 #include "pkt-line.h"
 #include "utf8.h"
-#include "interpolate.h"
 #include "diff.h"
 #include "revision.h"
 
@@ -11,46 +10,6 @@ int save_commit_buffer = 1;
 
 const char *commit_type = "commit";
 
-static struct cmt_fmt_map {
-	const char *n;
-	size_t cmp_len;
-	enum cmit_fmt v;
-} cmt_fmts[] = {
-	{ "raw",	1,	CMIT_FMT_RAW },
-	{ "medium",	1,	CMIT_FMT_MEDIUM },
-	{ "short",	1,	CMIT_FMT_SHORT },
-	{ "email",	1,	CMIT_FMT_EMAIL },
-	{ "full",	5,	CMIT_FMT_FULL },
-	{ "fuller",	5,	CMIT_FMT_FULLER },
-	{ "oneline",	1,	CMIT_FMT_ONELINE },
-	{ "format:",	7,	CMIT_FMT_USERFORMAT},
-};
-
-static char *user_format;
-
-enum cmit_fmt get_commit_format(const char *arg)
-{
-	int i;
-
-	if (!arg || !*arg)
-		return CMIT_FMT_DEFAULT;
-	if (*arg == '=')
-		arg++;
-	if (!prefixcmp(arg, "format:")) {
-		if (user_format)
-			free(user_format);
-		user_format = xstrdup(arg + 7);
-		return CMIT_FMT_USERFORMAT;
-	}
-	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
-		if (!strncmp(arg, cmt_fmts[i].n, cmt_fmts[i].cmp_len) &&
-		    !strncmp(arg, cmt_fmts[i].n, strlen(arg)))
-			return cmt_fmts[i].v;
-	}
-
-	die("invalid --pretty format: %s", arg);
-}
-
 static struct commit *check_commit(struct object *obj,
 				   const unsigned char *sha1,
 				   int quiet)
@@ -444,683 +403,6 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
 	}
 }
 
-/*
- * Generic support for pretty-printing the header
- */
-static int get_one_line(const char *msg)
-{
-	int ret = 0;
-
-	for (;;) {
-		char c = *msg++;
-		if (!c)
-			break;
-		ret++;
-		if (c == '\n')
-			break;
-	}
-	return ret;
-}
-
-/* High bit set, or ISO-2022-INT */
-int non_ascii(int ch)
-{
-	ch = (ch & 0xff);
-	return ((ch & 0x80) || (ch == 0x1b));
-}
-
-static int is_rfc2047_special(char ch)
-{
-	return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
-}
-
-static void add_rfc2047(struct strbuf *sb, const char *line, int len,
-		       const char *encoding)
-{
-	int i, last;
-
-	for (i = 0; i < len; i++) {
-		int ch = line[i];
-		if (non_ascii(ch))
-			goto needquote;
-		if ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))
-			goto needquote;
-	}
-	strbuf_add(sb, line, len);
-	return;
-
-needquote:
-	strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
-	strbuf_addf(sb, "=?%s?q?", encoding);
-	for (i = last = 0; i < len; i++) {
-		unsigned ch = line[i] & 0xFF;
-		/*
-		 * We encode ' ' using '=20' even though rfc2047
-		 * allows using '_' for readability.  Unfortunately,
-		 * many programs do not understand this and just
-		 * leave the underscore in place.
-		 */
-		if (is_rfc2047_special(ch) || ch == ' ') {
-			strbuf_add(sb, line + last, i - last);
-			strbuf_addf(sb, "=%02X", ch);
-			last = i + 1;
-		}
-	}
-	strbuf_add(sb, line + last, len - last);
-	strbuf_addstr(sb, "?=");
-}
-
-static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-			 const char *line, enum date_mode dmode,
-			 const char *encoding)
-{
-	char *date;
-	int namelen;
-	unsigned long time;
-	int tz;
-	const char *filler = "    ";
-
-	if (fmt == CMIT_FMT_ONELINE)
-		return;
-	date = strchr(line, '>');
-	if (!date)
-		return;
-	namelen = ++date - line;
-	time = strtoul(date, &date, 10);
-	tz = strtol(date, NULL, 10);
-
-	if (fmt == CMIT_FMT_EMAIL) {
-		char *name_tail = strchr(line, '<');
-		int display_name_length;
-		if (!name_tail)
-			return;
-		while (line < name_tail && isspace(name_tail[-1]))
-			name_tail--;
-		display_name_length = name_tail - line;
-		filler = "";
-		strbuf_addstr(sb, "From: ");
-		add_rfc2047(sb, line, display_name_length, encoding);
-		strbuf_add(sb, name_tail, namelen - display_name_length);
-		strbuf_addch(sb, '\n');
-	} else {
-		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
-			      (fmt == CMIT_FMT_FULLER) ? 4 : 0,
-			      filler, namelen, line);
-	}
-	switch (fmt) {
-	case CMIT_FMT_MEDIUM:
-		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, dmode));
-		break;
-	case CMIT_FMT_EMAIL:
-		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
-		break;
-	case CMIT_FMT_FULLER:
-		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
-		break;
-	default:
-		/* notin' */
-		break;
-	}
-}
-
-static int is_empty_line(const char *line, int *len_p)
-{
-	int len = *len_p;
-	while (len && isspace(line[len-1]))
-		len--;
-	*len_p = len;
-	return !len;
-}
-
-static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
-			const struct commit *commit, int abbrev)
-{
-	struct commit_list *parent = commit->parents;
-
-	if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
-	    !parent || !parent->next)
-		return;
-
-	strbuf_addstr(sb, "Merge:");
-
-	while (parent) {
-		struct commit *p = parent->item;
-		const char *hex = NULL;
-		const char *dots;
-		if (abbrev)
-			hex = find_unique_abbrev(p->object.sha1, abbrev);
-		if (!hex)
-			hex = sha1_to_hex(p->object.sha1);
-		dots = (abbrev && strlen(hex) != 40) ?  "..." : "";
-		parent = parent->next;
-
-		strbuf_addf(sb, " %s%s", hex, dots);
-	}
-	strbuf_addch(sb, '\n');
-}
-
-static char *get_header(const struct commit *commit, const char *key)
-{
-	int key_len = strlen(key);
-	const char *line = commit->buffer;
-
-	for (;;) {
-		const char *eol = strchr(line, '\n'), *next;
-
-		if (line == eol)
-			return NULL;
-		if (!eol) {
-			eol = line + strlen(line);
-			next = NULL;
-		} else
-			next = eol + 1;
-		if (eol - line > key_len &&
-		    !strncmp(line, key, key_len) &&
-		    line[key_len] == ' ') {
-			return xmemdupz(line + key_len + 1, eol - line - key_len - 1);
-		}
-		line = next;
-	}
-}
-
-static char *replace_encoding_header(char *buf, const char *encoding)
-{
-	struct strbuf tmp;
-	size_t start, len;
-	char *cp = buf;
-
-	/* guess if there is an encoding header before a \n\n */
-	while (strncmp(cp, "encoding ", strlen("encoding "))) {
-		cp = strchr(cp, '\n');
-		if (!cp || *++cp == '\n')
-			return buf;
-	}
-	start = cp - buf;
-	cp = strchr(cp, '\n');
-	if (!cp)
-		return buf; /* should not happen but be defensive */
-	len = cp + 1 - (buf + start);
-
-	strbuf_init(&tmp, 0);
-	strbuf_attach(&tmp, buf, strlen(buf), strlen(buf) + 1);
-	if (is_encoding_utf8(encoding)) {
-		/* we have re-coded to UTF-8; drop the header */
-		strbuf_remove(&tmp, start, len);
-	} else {
-		/* just replaces XXXX in 'encoding XXXX\n' */
-		strbuf_splice(&tmp, start + strlen("encoding "),
-					  len - strlen("encoding \n"),
-					  encoding, strlen(encoding));
-	}
-	return strbuf_detach(&tmp, NULL);
-}
-
-static char *logmsg_reencode(const struct commit *commit,
-			     const char *output_encoding)
-{
-	static const char *utf8 = "utf-8";
-	const char *use_encoding;
-	char *encoding;
-	char *out;
-
-	if (!*output_encoding)
-		return NULL;
-	encoding = get_header(commit, "encoding");
-	use_encoding = encoding ? encoding : utf8;
-	if (!strcmp(use_encoding, output_encoding))
-		if (encoding) /* we'll strip encoding header later */
-			out = xstrdup(commit->buffer);
-		else
-			return NULL; /* nothing to do */
-	else
-		out = reencode_string(commit->buffer,
-				      output_encoding, use_encoding);
-	if (out)
-		out = replace_encoding_header(out, output_encoding);
-
-	free(encoding);
-	return out;
-}
-
-static void fill_person(struct interp *table, const char *msg, int len)
-{
-	int start, end, tz = 0;
-	unsigned long date;
-	char *ep;
-
-	/* parse name */
-	for (end = 0; end < len && msg[end] != '<'; end++)
-		; /* do nothing */
-	start = end + 1;
-	while (end > 0 && isspace(msg[end - 1]))
-		end--;
-	table[0].value = xmemdupz(msg, end);
-
-	if (start >= len)
-		return;
-
-	/* parse email */
-	for (end = start + 1; end < len && msg[end] != '>'; end++)
-		; /* do nothing */
-
-	if (end >= len)
-		return;
-
-	table[1].value = xmemdupz(msg + start, end - start);
-
-	/* parse date */
-	for (start = end + 1; start < len && isspace(msg[start]); start++)
-		; /* do nothing */
-	if (start >= len)
-		return;
-	date = strtoul(msg + start, &ep, 10);
-	if (msg + start == ep)
-		return;
-
-	table[5].value = xmemdupz(msg + start, ep - (msg + start));
-
-	/* parse tz */
-	for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
-		; /* do nothing */
-	if (start + 1 < len) {
-		tz = strtoul(msg + start + 1, NULL, 10);
-		if (msg[start] == '-')
-			tz = -tz;
-	}
-
-	interp_set_entry(table, 2, show_date(date, tz, DATE_NORMAL));
-	interp_set_entry(table, 3, show_date(date, tz, DATE_RFC2822));
-	interp_set_entry(table, 4, show_date(date, tz, DATE_RELATIVE));
-	interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
-}
-
-void format_commit_message(const struct commit *commit,
-                           const void *format, struct strbuf *sb)
-{
-	struct interp table[] = {
-		{ "%H" },	/* commit hash */
-		{ "%h" },	/* abbreviated commit hash */
-		{ "%T" },	/* tree hash */
-		{ "%t" },	/* abbreviated tree hash */
-		{ "%P" },	/* parent hashes */
-		{ "%p" },	/* abbreviated parent hashes */
-		{ "%an" },	/* author name */
-		{ "%ae" },	/* author email */
-		{ "%ad" },	/* author date */
-		{ "%aD" },	/* author date, RFC2822 style */
-		{ "%ar" },	/* author date, relative */
-		{ "%at" },	/* author date, UNIX timestamp */
-		{ "%ai" },	/* author date, ISO 8601 */
-		{ "%cn" },	/* committer name */
-		{ "%ce" },	/* committer email */
-		{ "%cd" },	/* committer date */
-		{ "%cD" },	/* committer date, RFC2822 style */
-		{ "%cr" },	/* committer date, relative */
-		{ "%ct" },	/* committer date, UNIX timestamp */
-		{ "%ci" },	/* committer date, ISO 8601 */
-		{ "%e" },	/* encoding */
-		{ "%s" },	/* subject */
-		{ "%b" },	/* body */
-		{ "%Cred" },	/* red */
-		{ "%Cgreen" },	/* green */
-		{ "%Cblue" },	/* blue */
-		{ "%Creset" },	/* reset color */
-		{ "%n" },	/* newline */
-		{ "%m" },	/* left/right/bottom */
-	};
-	enum interp_index {
-		IHASH = 0, IHASH_ABBREV,
-		ITREE, ITREE_ABBREV,
-		IPARENTS, IPARENTS_ABBREV,
-		IAUTHOR_NAME, IAUTHOR_EMAIL,
-		IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
-		IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
-		ICOMMITTER_NAME, ICOMMITTER_EMAIL,
-		ICOMMITTER_DATE, ICOMMITTER_DATE_RFC2822,
-		ICOMMITTER_DATE_RELATIVE, ICOMMITTER_TIMESTAMP,
-		ICOMMITTER_ISO8601,
-		IENCODING,
-		ISUBJECT,
-		IBODY,
-		IRED, IGREEN, IBLUE, IRESET_COLOR,
-		INEWLINE,
-		ILEFT_RIGHT,
-	};
-	struct commit_list *p;
-	char parents[1024];
-	unsigned long len;
-	int i;
-	enum { HEADER, SUBJECT, BODY } state;
-	const char *msg = commit->buffer;
-
-	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
-		die("invalid interp table!");
-
-	/* these are independent of the commit */
-	interp_set_entry(table, IRED, "\033[31m");
-	interp_set_entry(table, IGREEN, "\033[32m");
-	interp_set_entry(table, IBLUE, "\033[34m");
-	interp_set_entry(table, IRESET_COLOR, "\033[m");
-	interp_set_entry(table, INEWLINE, "\n");
-
-	/* these depend on the commit */
-	if (!commit->object.parsed)
-		parse_object(commit->object.sha1);
-	interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
-	interp_set_entry(table, IHASH_ABBREV,
-			find_unique_abbrev(commit->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
-	interp_set_entry(table, ITREE_ABBREV,
-			find_unique_abbrev(commit->tree->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, ILEFT_RIGHT,
-			 (commit->object.flags & BOUNDARY)
-			 ? "-"
-			 : (commit->object.flags & SYMMETRIC_LEFT)
-			 ? "<"
-			 : ">");
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			sha1_to_hex(p->item->object.sha1));
-	interp_set_entry(table, IPARENTS, parents + 1);
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			find_unique_abbrev(p->item->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
-
-	for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
-		int eol;
-		for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
-			; /* do nothing */
-
-		if (state == SUBJECT) {
-			table[ISUBJECT].value = xmemdupz(msg + i, eol - i);
-			i = eol;
-		}
-		if (i == eol) {
-			state++;
-			/* strip empty lines */
-			while (msg[eol + 1] == '\n')
-				eol++;
-		} else if (!prefixcmp(msg + i, "author "))
-			fill_person(table + IAUTHOR_NAME,
-					msg + i + 7, eol - i - 7);
-		else if (!prefixcmp(msg + i, "committer "))
-			fill_person(table + ICOMMITTER_NAME,
-					msg + i + 10, eol - i - 10);
-		else if (!prefixcmp(msg + i, "encoding "))
-			table[IENCODING].value =
-				xmemdupz(msg + i + 9, eol - i - 9);
-		i = eol;
-	}
-	if (msg[i])
-		table[IBODY].value = xstrdup(msg + i);
-
-	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
-				format, table, ARRAY_SIZE(table));
-	if (len > strbuf_avail(sb)) {
-		strbuf_grow(sb, len);
-		interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
-					format, table, ARRAY_SIZE(table));
-	}
-	strbuf_setlen(sb, sb->len + len);
-	interp_clear_table(table, ARRAY_SIZE(table));
-}
-
-static void pp_header(enum cmit_fmt fmt,
-		      int abbrev,
-		      enum date_mode dmode,
-		      const char *encoding,
-		      const struct commit *commit,
-		      const char **msg_p,
-		      struct strbuf *sb)
-{
-	int parents_shown = 0;
-
-	for (;;) {
-		const char *line = *msg_p;
-		int linelen = get_one_line(*msg_p);
-
-		if (!linelen)
-			return;
-		*msg_p += linelen;
-
-		if (linelen == 1)
-			/* End of header */
-			return;
-
-		if (fmt == CMIT_FMT_RAW) {
-			strbuf_add(sb, line, linelen);
-			continue;
-		}
-
-		if (!memcmp(line, "parent ", 7)) {
-			if (linelen != 48)
-				die("bad parent line in commit");
-			continue;
-		}
-
-		if (!parents_shown) {
-			struct commit_list *parent;
-			int num;
-			for (parent = commit->parents, num = 0;
-			     parent;
-			     parent = parent->next, num++)
-				;
-			/* with enough slop */
-			strbuf_grow(sb, num * 50 + 20);
-			add_merge_info(fmt, sb, commit, abbrev);
-			parents_shown = 1;
-		}
-
-		/*
-		 * MEDIUM == DEFAULT shows only author with dates.
-		 * FULL shows both authors but not dates.
-		 * FULLER shows both authors and dates.
-		 */
-		if (!memcmp(line, "author ", 7)) {
-			strbuf_grow(sb, linelen + 80);
-			add_user_info("Author", fmt, sb, line + 7, dmode, encoding);
-		}
-		if (!memcmp(line, "committer ", 10) &&
-		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
-			strbuf_grow(sb, linelen + 80);
-			add_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
-		}
-	}
-}
-
-static void pp_title_line(enum cmit_fmt fmt,
-			  const char **msg_p,
-			  struct strbuf *sb,
-			  const char *subject,
-			  const char *after_subject,
-			  const char *encoding,
-			  int plain_non_ascii)
-{
-	struct strbuf title;
-
-	strbuf_init(&title, 80);
-
-	for (;;) {
-		const char *line = *msg_p;
-		int linelen = get_one_line(line);
-
-		*msg_p += linelen;
-		if (!linelen || is_empty_line(line, &linelen))
-			break;
-
-		strbuf_grow(&title, linelen + 2);
-		if (title.len) {
-			if (fmt == CMIT_FMT_EMAIL) {
-				strbuf_addch(&title, '\n');
-			}
-			strbuf_addch(&title, ' ');
-		}
-		strbuf_add(&title, line, linelen);
-	}
-
-	strbuf_grow(sb, title.len + 1024);
-	if (subject) {
-		strbuf_addstr(sb, subject);
-		add_rfc2047(sb, title.buf, title.len, encoding);
-	} else {
-		strbuf_addbuf(sb, &title);
-	}
-	strbuf_addch(sb, '\n');
-
-	if (plain_non_ascii) {
-		const char *header_fmt =
-			"MIME-Version: 1.0\n"
-			"Content-Type: text/plain; charset=%s\n"
-			"Content-Transfer-Encoding: 8bit\n";
-		strbuf_addf(sb, header_fmt, encoding);
-	}
-	if (after_subject) {
-		strbuf_addstr(sb, after_subject);
-	}
-	if (fmt == CMIT_FMT_EMAIL) {
-		strbuf_addch(sb, '\n');
-	}
-	strbuf_release(&title);
-}
-
-static void pp_remainder(enum cmit_fmt fmt,
-			 const char **msg_p,
-			 struct strbuf *sb,
-			 int indent)
-{
-	int first = 1;
-	for (;;) {
-		const char *line = *msg_p;
-		int linelen = get_one_line(line);
-		*msg_p += linelen;
-
-		if (!linelen)
-			break;
-
-		if (is_empty_line(line, &linelen)) {
-			if (first)
-				continue;
-			if (fmt == CMIT_FMT_SHORT)
-				break;
-		}
-		first = 0;
-
-		strbuf_grow(sb, linelen + indent + 20);
-		if (indent) {
-			memset(sb->buf + sb->len, ' ', indent);
-			strbuf_setlen(sb, sb->len + indent);
-		}
-		strbuf_add(sb, line, linelen);
-		strbuf_addch(sb, '\n');
-	}
-}
-
-void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-				  struct strbuf *sb, int abbrev,
-				  const char *subject, const char *after_subject,
-				  enum date_mode dmode, int plain_non_ascii)
-{
-	unsigned long beginning_of_body;
-	int indent = 4;
-	const char *msg = commit->buffer;
-	char *reencoded;
-	const char *encoding;
-
-	if (fmt == CMIT_FMT_USERFORMAT) {
-		format_commit_message(commit, user_format, sb);
-		return;
-	}
-
-	encoding = (git_log_output_encoding
-		    ? git_log_output_encoding
-		    : git_commit_encoding);
-	if (!encoding)
-		encoding = "utf-8";
-	reencoded = logmsg_reencode(commit, encoding);
-	if (reencoded) {
-		msg = reencoded;
-	}
-
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-		indent = 0;
-
-	/* After-subject is used to pass in Content-Type: multipart
-	 * MIME header; in that case we do not have to do the
-	 * plaintext content type even if the commit message has
-	 * non 7-bit ASCII character.  Otherwise, check if we need
-	 * to say this is not a 7-bit ASCII.
-	 */
-	if (fmt == CMIT_FMT_EMAIL && !after_subject) {
-		int i, ch, in_body;
-
-		for (in_body = i = 0; (ch = msg[i]); i++) {
-			if (!in_body) {
-				/* author could be non 7-bit ASCII but
-				 * the log may be so; skip over the
-				 * header part first.
-				 */
-				if (ch == '\n' && msg[i+1] == '\n')
-					in_body = 1;
-			}
-			else if (non_ascii(ch)) {
-				plain_non_ascii = 1;
-				break;
-			}
-		}
-	}
-
-	pp_header(fmt, abbrev, dmode, encoding, commit, &msg, sb);
-	if (fmt != CMIT_FMT_ONELINE && !subject) {
-		strbuf_addch(sb, '\n');
-	}
-
-	/* Skip excess blank lines at the beginning of body, if any... */
-	for (;;) {
-		int linelen = get_one_line(msg);
-		int ll = linelen;
-		if (!linelen)
-			break;
-		if (!is_empty_line(msg, &ll))
-			break;
-		msg += linelen;
-	}
-
-	/* These formats treat the title line specially. */
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-		pp_title_line(fmt, &msg, sb, subject,
-			      after_subject, encoding, plain_non_ascii);
-
-	beginning_of_body = sb->len;
-	if (fmt != CMIT_FMT_ONELINE)
-		pp_remainder(fmt, &msg, sb, indent);
-	strbuf_rtrim(sb);
-
-	/* Make sure there is an EOLN for the non-oneline case */
-	if (fmt != CMIT_FMT_ONELINE)
-		strbuf_addch(sb, '\n');
-
-	/*
-	 * The caller may append additional body text in e-mail
-	 * format.  Make sure we did not strip the blank line
-	 * between the header and the body.
-	 */
-	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
-		strbuf_addch(sb, '\n');
-	free(reencoded);
-}
-
 struct commit *pop_commit(struct commit_list **stack)
 {
 	struct commit_list *top = *stack;
diff --git a/pretty.c b/pretty.c
new file mode 100644
index 0000000..490cede
--- /dev/null
+++ b/pretty.c
@@ -0,0 +1,723 @@
+#include "cache.h"
+#include "commit.h"
+#include "interpolate.h"
+#include "utf8.h"
+#include "diff.h"
+#include "revision.h"
+
+static struct cmt_fmt_map {
+	const char *n;
+	size_t cmp_len;
+	enum cmit_fmt v;
+} cmt_fmts[] = {
+	{ "raw",	1,	CMIT_FMT_RAW },
+	{ "medium",	1,	CMIT_FMT_MEDIUM },
+	{ "short",	1,	CMIT_FMT_SHORT },
+	{ "email",	1,	CMIT_FMT_EMAIL },
+	{ "full",	5,	CMIT_FMT_FULL },
+	{ "fuller",	5,	CMIT_FMT_FULLER },
+	{ "oneline",	1,	CMIT_FMT_ONELINE },
+	{ "format:",	7,	CMIT_FMT_USERFORMAT},
+};
+
+static char *user_format;
+
+enum cmit_fmt get_commit_format(const char *arg)
+{
+	int i;
+
+	if (!arg || !*arg)
+		return CMIT_FMT_DEFAULT;
+	if (*arg == '=')
+		arg++;
+	if (!prefixcmp(arg, "format:")) {
+		if (user_format)
+			free(user_format);
+		user_format = xstrdup(arg + 7);
+		return CMIT_FMT_USERFORMAT;
+	}
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strncmp(arg, cmt_fmts[i].n, cmt_fmts[i].cmp_len) &&
+		    !strncmp(arg, cmt_fmts[i].n, strlen(arg)))
+			return cmt_fmts[i].v;
+	}
+
+	die("invalid --pretty format: %s", arg);
+}
+
+/*
+ * Generic support for pretty-printing the header
+ */
+static int get_one_line(const char *msg)
+{
+	int ret = 0;
+
+	for (;;) {
+		char c = *msg++;
+		if (!c)
+			break;
+		ret++;
+		if (c == '\n')
+			break;
+	}
+	return ret;
+}
+
+/* High bit set, or ISO-2022-INT */
+int non_ascii(int ch)
+{
+	ch = (ch & 0xff);
+	return ((ch & 0x80) || (ch == 0x1b));
+}
+
+static int is_rfc2047_special(char ch)
+{
+	return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
+}
+
+static void add_rfc2047(struct strbuf *sb, const char *line, int len,
+		       const char *encoding)
+{
+	int i, last;
+
+	for (i = 0; i < len; i++) {
+		int ch = line[i];
+		if (non_ascii(ch))
+			goto needquote;
+		if ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))
+			goto needquote;
+	}
+	strbuf_add(sb, line, len);
+	return;
+
+needquote:
+	strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
+	strbuf_addf(sb, "=?%s?q?", encoding);
+	for (i = last = 0; i < len; i++) {
+		unsigned ch = line[i] & 0xFF;
+		/*
+		 * We encode ' ' using '=20' even though rfc2047
+		 * allows using '_' for readability.  Unfortunately,
+		 * many programs do not understand this and just
+		 * leave the underscore in place.
+		 */
+		if (is_rfc2047_special(ch) || ch == ' ') {
+			strbuf_add(sb, line + last, i - last);
+			strbuf_addf(sb, "=%02X", ch);
+			last = i + 1;
+		}
+	}
+	strbuf_add(sb, line + last, len - last);
+	strbuf_addstr(sb, "?=");
+}
+
+static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
+			 const char *line, enum date_mode dmode,
+			 const char *encoding)
+{
+	char *date;
+	int namelen;
+	unsigned long time;
+	int tz;
+	const char *filler = "    ";
+
+	if (fmt == CMIT_FMT_ONELINE)
+		return;
+	date = strchr(line, '>');
+	if (!date)
+		return;
+	namelen = ++date - line;
+	time = strtoul(date, &date, 10);
+	tz = strtol(date, NULL, 10);
+
+	if (fmt == CMIT_FMT_EMAIL) {
+		char *name_tail = strchr(line, '<');
+		int display_name_length;
+		if (!name_tail)
+			return;
+		while (line < name_tail && isspace(name_tail[-1]))
+			name_tail--;
+		display_name_length = name_tail - line;
+		filler = "";
+		strbuf_addstr(sb, "From: ");
+		add_rfc2047(sb, line, display_name_length, encoding);
+		strbuf_add(sb, name_tail, namelen - display_name_length);
+		strbuf_addch(sb, '\n');
+	} else {
+		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
+			      (fmt == CMIT_FMT_FULLER) ? 4 : 0,
+			      filler, namelen, line);
+	}
+	switch (fmt) {
+	case CMIT_FMT_MEDIUM:
+		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, dmode));
+		break;
+	case CMIT_FMT_EMAIL:
+		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
+		break;
+	case CMIT_FMT_FULLER:
+		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
+		break;
+	default:
+		/* notin' */
+		break;
+	}
+}
+
+static int is_empty_line(const char *line, int *len_p)
+{
+	int len = *len_p;
+	while (len && isspace(line[len-1]))
+		len--;
+	*len_p = len;
+	return !len;
+}
+
+static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
+			const struct commit *commit, int abbrev)
+{
+	struct commit_list *parent = commit->parents;
+
+	if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
+	    !parent || !parent->next)
+		return;
+
+	strbuf_addstr(sb, "Merge:");
+
+	while (parent) {
+		struct commit *p = parent->item;
+		const char *hex = NULL;
+		const char *dots;
+		if (abbrev)
+			hex = find_unique_abbrev(p->object.sha1, abbrev);
+		if (!hex)
+			hex = sha1_to_hex(p->object.sha1);
+		dots = (abbrev && strlen(hex) != 40) ?  "..." : "";
+		parent = parent->next;
+
+		strbuf_addf(sb, " %s%s", hex, dots);
+	}
+	strbuf_addch(sb, '\n');
+}
+
+static char *get_header(const struct commit *commit, const char *key)
+{
+	int key_len = strlen(key);
+	const char *line = commit->buffer;
+
+	for (;;) {
+		const char *eol = strchr(line, '\n'), *next;
+
+		if (line == eol)
+			return NULL;
+		if (!eol) {
+			eol = line + strlen(line);
+			next = NULL;
+		} else
+			next = eol + 1;
+		if (eol - line > key_len &&
+		    !strncmp(line, key, key_len) &&
+		    line[key_len] == ' ') {
+			return xmemdupz(line + key_len + 1, eol - line - key_len - 1);
+		}
+		line = next;
+	}
+}
+
+static char *replace_encoding_header(char *buf, const char *encoding)
+{
+	struct strbuf tmp;
+	size_t start, len;
+	char *cp = buf;
+
+	/* guess if there is an encoding header before a \n\n */
+	while (strncmp(cp, "encoding ", strlen("encoding "))) {
+		cp = strchr(cp, '\n');
+		if (!cp || *++cp == '\n')
+			return buf;
+	}
+	start = cp - buf;
+	cp = strchr(cp, '\n');
+	if (!cp)
+		return buf; /* should not happen but be defensive */
+	len = cp + 1 - (buf + start);
+
+	strbuf_init(&tmp, 0);
+	strbuf_attach(&tmp, buf, strlen(buf), strlen(buf) + 1);
+	if (is_encoding_utf8(encoding)) {
+		/* we have re-coded to UTF-8; drop the header */
+		strbuf_remove(&tmp, start, len);
+	} else {
+		/* just replaces XXXX in 'encoding XXXX\n' */
+		strbuf_splice(&tmp, start + strlen("encoding "),
+					  len - strlen("encoding \n"),
+					  encoding, strlen(encoding));
+	}
+	return strbuf_detach(&tmp, NULL);
+}
+
+static char *logmsg_reencode(const struct commit *commit,
+			     const char *output_encoding)
+{
+	static const char *utf8 = "utf-8";
+	const char *use_encoding;
+	char *encoding;
+	char *out;
+
+	if (!*output_encoding)
+		return NULL;
+	encoding = get_header(commit, "encoding");
+	use_encoding = encoding ? encoding : utf8;
+	if (!strcmp(use_encoding, output_encoding))
+		if (encoding) /* we'll strip encoding header later */
+			out = xstrdup(commit->buffer);
+		else
+			return NULL; /* nothing to do */
+	else
+		out = reencode_string(commit->buffer,
+				      output_encoding, use_encoding);
+	if (out)
+		out = replace_encoding_header(out, output_encoding);
+
+	free(encoding);
+	return out;
+}
+
+static void fill_person(struct interp *table, const char *msg, int len)
+{
+	int start, end, tz = 0;
+	unsigned long date;
+	char *ep;
+
+	/* parse name */
+	for (end = 0; end < len && msg[end] != '<'; end++)
+		; /* do nothing */
+	start = end + 1;
+	while (end > 0 && isspace(msg[end - 1]))
+		end--;
+	table[0].value = xmemdupz(msg, end);
+
+	if (start >= len)
+		return;
+
+	/* parse email */
+	for (end = start + 1; end < len && msg[end] != '>'; end++)
+		; /* do nothing */
+
+	if (end >= len)
+		return;
+
+	table[1].value = xmemdupz(msg + start, end - start);
+
+	/* parse date */
+	for (start = end + 1; start < len && isspace(msg[start]); start++)
+		; /* do nothing */
+	if (start >= len)
+		return;
+	date = strtoul(msg + start, &ep, 10);
+	if (msg + start == ep)
+		return;
+
+	table[5].value = xmemdupz(msg + start, ep - (msg + start));
+
+	/* parse tz */
+	for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
+		; /* do nothing */
+	if (start + 1 < len) {
+		tz = strtoul(msg + start + 1, NULL, 10);
+		if (msg[start] == '-')
+			tz = -tz;
+	}
+
+	interp_set_entry(table, 2, show_date(date, tz, DATE_NORMAL));
+	interp_set_entry(table, 3, show_date(date, tz, DATE_RFC2822));
+	interp_set_entry(table, 4, show_date(date, tz, DATE_RELATIVE));
+	interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
+}
+
+void format_commit_message(const struct commit *commit,
+                           const void *format, struct strbuf *sb)
+{
+	struct interp table[] = {
+		{ "%H" },	/* commit hash */
+		{ "%h" },	/* abbreviated commit hash */
+		{ "%T" },	/* tree hash */
+		{ "%t" },	/* abbreviated tree hash */
+		{ "%P" },	/* parent hashes */
+		{ "%p" },	/* abbreviated parent hashes */
+		{ "%an" },	/* author name */
+		{ "%ae" },	/* author email */
+		{ "%ad" },	/* author date */
+		{ "%aD" },	/* author date, RFC2822 style */
+		{ "%ar" },	/* author date, relative */
+		{ "%at" },	/* author date, UNIX timestamp */
+		{ "%ai" },	/* author date, ISO 8601 */
+		{ "%cn" },	/* committer name */
+		{ "%ce" },	/* committer email */
+		{ "%cd" },	/* committer date */
+		{ "%cD" },	/* committer date, RFC2822 style */
+		{ "%cr" },	/* committer date, relative */
+		{ "%ct" },	/* committer date, UNIX timestamp */
+		{ "%ci" },	/* committer date, ISO 8601 */
+		{ "%e" },	/* encoding */
+		{ "%s" },	/* subject */
+		{ "%b" },	/* body */
+		{ "%Cred" },	/* red */
+		{ "%Cgreen" },	/* green */
+		{ "%Cblue" },	/* blue */
+		{ "%Creset" },	/* reset color */
+		{ "%n" },	/* newline */
+		{ "%m" },	/* left/right/bottom */
+	};
+	enum interp_index {
+		IHASH = 0, IHASH_ABBREV,
+		ITREE, ITREE_ABBREV,
+		IPARENTS, IPARENTS_ABBREV,
+		IAUTHOR_NAME, IAUTHOR_EMAIL,
+		IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
+		IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
+		ICOMMITTER_NAME, ICOMMITTER_EMAIL,
+		ICOMMITTER_DATE, ICOMMITTER_DATE_RFC2822,
+		ICOMMITTER_DATE_RELATIVE, ICOMMITTER_TIMESTAMP,
+		ICOMMITTER_ISO8601,
+		IENCODING,
+		ISUBJECT,
+		IBODY,
+		IRED, IGREEN, IBLUE, IRESET_COLOR,
+		INEWLINE,
+		ILEFT_RIGHT,
+	};
+	struct commit_list *p;
+	char parents[1024];
+	unsigned long len;
+	int i;
+	enum { HEADER, SUBJECT, BODY } state;
+	const char *msg = commit->buffer;
+
+	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
+		die("invalid interp table!");
+
+	/* these are independent of the commit */
+	interp_set_entry(table, IRED, "\033[31m");
+	interp_set_entry(table, IGREEN, "\033[32m");
+	interp_set_entry(table, IBLUE, "\033[34m");
+	interp_set_entry(table, IRESET_COLOR, "\033[m");
+	interp_set_entry(table, INEWLINE, "\n");
+
+	/* these depend on the commit */
+	if (!commit->object.parsed)
+		parse_object(commit->object.sha1);
+	interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
+	interp_set_entry(table, IHASH_ABBREV,
+			find_unique_abbrev(commit->object.sha1,
+				DEFAULT_ABBREV));
+	interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
+	interp_set_entry(table, ITREE_ABBREV,
+			find_unique_abbrev(commit->tree->object.sha1,
+				DEFAULT_ABBREV));
+	interp_set_entry(table, ILEFT_RIGHT,
+			 (commit->object.flags & BOUNDARY)
+			 ? "-"
+			 : (commit->object.flags & SYMMETRIC_LEFT)
+			 ? "<"
+			 : ">");
+
+	parents[1] = 0;
+	for (i = 0, p = commit->parents;
+			p && i < sizeof(parents) - 1;
+			p = p->next)
+		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
+			sha1_to_hex(p->item->object.sha1));
+	interp_set_entry(table, IPARENTS, parents + 1);
+
+	parents[1] = 0;
+	for (i = 0, p = commit->parents;
+			p && i < sizeof(parents) - 1;
+			p = p->next)
+		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
+			find_unique_abbrev(p->item->object.sha1,
+				DEFAULT_ABBREV));
+	interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+
+	for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
+		int eol;
+		for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
+			; /* do nothing */
+
+		if (state == SUBJECT) {
+			table[ISUBJECT].value = xmemdupz(msg + i, eol - i);
+			i = eol;
+		}
+		if (i == eol) {
+			state++;
+			/* strip empty lines */
+			while (msg[eol + 1] == '\n')
+				eol++;
+		} else if (!prefixcmp(msg + i, "author "))
+			fill_person(table + IAUTHOR_NAME,
+					msg + i + 7, eol - i - 7);
+		else if (!prefixcmp(msg + i, "committer "))
+			fill_person(table + ICOMMITTER_NAME,
+					msg + i + 10, eol - i - 10);
+		else if (!prefixcmp(msg + i, "encoding "))
+			table[IENCODING].value =
+				xmemdupz(msg + i + 9, eol - i - 9);
+		i = eol;
+	}
+	if (msg[i])
+		table[IBODY].value = xstrdup(msg + i);
+
+	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
+				format, table, ARRAY_SIZE(table));
+	if (len > strbuf_avail(sb)) {
+		strbuf_grow(sb, len);
+		interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
+					format, table, ARRAY_SIZE(table));
+	}
+	strbuf_setlen(sb, sb->len + len);
+	interp_clear_table(table, ARRAY_SIZE(table));
+}
+
+static void pp_header(enum cmit_fmt fmt,
+		      int abbrev,
+		      enum date_mode dmode,
+		      const char *encoding,
+		      const struct commit *commit,
+		      const char **msg_p,
+		      struct strbuf *sb)
+{
+	int parents_shown = 0;
+
+	for (;;) {
+		const char *line = *msg_p;
+		int linelen = get_one_line(*msg_p);
+
+		if (!linelen)
+			return;
+		*msg_p += linelen;
+
+		if (linelen == 1)
+			/* End of header */
+			return;
+
+		if (fmt == CMIT_FMT_RAW) {
+			strbuf_add(sb, line, linelen);
+			continue;
+		}
+
+		if (!memcmp(line, "parent ", 7)) {
+			if (linelen != 48)
+				die("bad parent line in commit");
+			continue;
+		}
+
+		if (!parents_shown) {
+			struct commit_list *parent;
+			int num;
+			for (parent = commit->parents, num = 0;
+			     parent;
+			     parent = parent->next, num++)
+				;
+			/* with enough slop */
+			strbuf_grow(sb, num * 50 + 20);
+			add_merge_info(fmt, sb, commit, abbrev);
+			parents_shown = 1;
+		}
+
+		/*
+		 * MEDIUM == DEFAULT shows only author with dates.
+		 * FULL shows both authors but not dates.
+		 * FULLER shows both authors and dates.
+		 */
+		if (!memcmp(line, "author ", 7)) {
+			strbuf_grow(sb, linelen + 80);
+			add_user_info("Author", fmt, sb, line + 7, dmode, encoding);
+		}
+		if (!memcmp(line, "committer ", 10) &&
+		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
+			strbuf_grow(sb, linelen + 80);
+			add_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
+		}
+	}
+}
+
+static void pp_title_line(enum cmit_fmt fmt,
+			  const char **msg_p,
+			  struct strbuf *sb,
+			  const char *subject,
+			  const char *after_subject,
+			  const char *encoding,
+			  int plain_non_ascii)
+{
+	struct strbuf title;
+
+	strbuf_init(&title, 80);
+
+	for (;;) {
+		const char *line = *msg_p;
+		int linelen = get_one_line(line);
+
+		*msg_p += linelen;
+		if (!linelen || is_empty_line(line, &linelen))
+			break;
+
+		strbuf_grow(&title, linelen + 2);
+		if (title.len) {
+			if (fmt == CMIT_FMT_EMAIL) {
+				strbuf_addch(&title, '\n');
+			}
+			strbuf_addch(&title, ' ');
+		}
+		strbuf_add(&title, line, linelen);
+	}
+
+	strbuf_grow(sb, title.len + 1024);
+	if (subject) {
+		strbuf_addstr(sb, subject);
+		add_rfc2047(sb, title.buf, title.len, encoding);
+	} else {
+		strbuf_addbuf(sb, &title);
+	}
+	strbuf_addch(sb, '\n');
+
+	if (plain_non_ascii) {
+		const char *header_fmt =
+			"MIME-Version: 1.0\n"
+			"Content-Type: text/plain; charset=%s\n"
+			"Content-Transfer-Encoding: 8bit\n";
+		strbuf_addf(sb, header_fmt, encoding);
+	}
+	if (after_subject) {
+		strbuf_addstr(sb, after_subject);
+	}
+	if (fmt == CMIT_FMT_EMAIL) {
+		strbuf_addch(sb, '\n');
+	}
+	strbuf_release(&title);
+}
+
+static void pp_remainder(enum cmit_fmt fmt,
+			 const char **msg_p,
+			 struct strbuf *sb,
+			 int indent)
+{
+	int first = 1;
+	for (;;) {
+		const char *line = *msg_p;
+		int linelen = get_one_line(line);
+		*msg_p += linelen;
+
+		if (!linelen)
+			break;
+
+		if (is_empty_line(line, &linelen)) {
+			if (first)
+				continue;
+			if (fmt == CMIT_FMT_SHORT)
+				break;
+		}
+		first = 0;
+
+		strbuf_grow(sb, linelen + indent + 20);
+		if (indent) {
+			memset(sb->buf + sb->len, ' ', indent);
+			strbuf_setlen(sb, sb->len + indent);
+		}
+		strbuf_add(sb, line, linelen);
+		strbuf_addch(sb, '\n');
+	}
+}
+
+void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
+				  struct strbuf *sb, int abbrev,
+				  const char *subject, const char *after_subject,
+				  enum date_mode dmode, int plain_non_ascii)
+{
+	unsigned long beginning_of_body;
+	int indent = 4;
+	const char *msg = commit->buffer;
+	char *reencoded;
+	const char *encoding;
+
+	if (fmt == CMIT_FMT_USERFORMAT) {
+		format_commit_message(commit, user_format, sb);
+		return;
+	}
+
+	encoding = (git_log_output_encoding
+		    ? git_log_output_encoding
+		    : git_commit_encoding);
+	if (!encoding)
+		encoding = "utf-8";
+	reencoded = logmsg_reencode(commit, encoding);
+	if (reencoded) {
+		msg = reencoded;
+	}
+
+	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
+		indent = 0;
+
+	/* After-subject is used to pass in Content-Type: multipart
+	 * MIME header; in that case we do not have to do the
+	 * plaintext content type even if the commit message has
+	 * non 7-bit ASCII character.  Otherwise, check if we need
+	 * to say this is not a 7-bit ASCII.
+	 */
+	if (fmt == CMIT_FMT_EMAIL && !after_subject) {
+		int i, ch, in_body;
+
+		for (in_body = i = 0; (ch = msg[i]); i++) {
+			if (!in_body) {
+				/* author could be non 7-bit ASCII but
+				 * the log may be so; skip over the
+				 * header part first.
+				 */
+				if (ch == '\n' && msg[i+1] == '\n')
+					in_body = 1;
+			}
+			else if (non_ascii(ch)) {
+				plain_non_ascii = 1;
+				break;
+			}
+		}
+	}
+
+	pp_header(fmt, abbrev, dmode, encoding, commit, &msg, sb);
+	if (fmt != CMIT_FMT_ONELINE && !subject) {
+		strbuf_addch(sb, '\n');
+	}
+
+	/* Skip excess blank lines at the beginning of body, if any... */
+	for (;;) {
+		int linelen = get_one_line(msg);
+		int ll = linelen;
+		if (!linelen)
+			break;
+		if (!is_empty_line(msg, &ll))
+			break;
+		msg += linelen;
+	}
+
+	/* These formats treat the title line specially. */
+	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
+		pp_title_line(fmt, &msg, sb, subject,
+			      after_subject, encoding, plain_non_ascii);
+
+	beginning_of_body = sb->len;
+	if (fmt != CMIT_FMT_ONELINE)
+		pp_remainder(fmt, &msg, sb, indent);
+	strbuf_rtrim(sb);
+
+	/* Make sure there is an EOLN for the non-oneline case */
+	if (fmt != CMIT_FMT_ONELINE)
+		strbuf_addch(sb, '\n');
+
+	/*
+	 * The caller may append additional body text in e-mail
+	 * format.  Make sure we did not strip the blank line
+	 * between the header and the body.
+	 */
+	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
+		strbuf_addch(sb, '\n');
+	free(reencoded);
+}
-- 
1.5.3.5.1549.g91a3

^ permalink raw reply related

* [PATCH 0/3] Make user formatted commit listing less expensive
From: Johannes Schindelin @ 2007-11-04 19:14 UTC (permalink / raw)
  To: git, Rene Scharfe, gitster

Hi,

this series of three splits off the formatting code from commit.c, adds 
the function interp_find_active() to interpolate.[ch], and then uses it in 
the obvious way.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-11-04 19:02 UTC (permalink / raw)
  To: git; +Cc: gitster, Shawn Bohrer
In-Reply-To: <11942029442710-git-send-email-shawn.bohrer@gmail.com>

This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to
the examples.

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---
 Makefile                                      |    3 +-
 builtin-clean.c                               |  157 +++++++++++++++++++++++++
 builtin.h                                     |    1 +
 git-clean.sh => contrib/examples/git-clean.sh |    0 
 git.c                                         |    1 +
 5 files changed, 161 insertions(+), 1 deletions(-)
 create mode 100644 builtin-clean.c
 rename git-clean.sh => contrib/examples/git-clean.sh (100%)

diff --git a/Makefile b/Makefile
index 3ec1876..fad49b2 100644
--- a/Makefile
+++ b/Makefile
@@ -209,7 +209,7 @@ BASIC_LDFLAGS =
 
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
-	git-clean.sh git-clone.sh git-commit.sh \
+	git-clone.sh git-commit.sh \
 	git-ls-remote.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
@@ -326,6 +326,7 @@ BUILTIN_OBJS = \
 	builtin-check-attr.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
+	builtin-clean.o \
 	builtin-commit-tree.o \
 	builtin-count-objects.o \
 	builtin-describe.o \
diff --git a/builtin-clean.c b/builtin-clean.c
new file mode 100644
index 0000000..4141eb4
--- /dev/null
+++ b/builtin-clean.c
@@ -0,0 +1,157 @@
+/*
+ * "git clean" builtin command
+ *
+ * Copyright (C) 2007 Shawn Bohrer
+ *
+ * Based on git-clean.sh by Pavel Roskin
+ */
+
+#include "builtin.h"
+#include "cache.h"
+#include "dir.h"
+
+static int disabled = 1;
+static int show_only = 0;
+static int remove_directories = 0;
+static int quiet = 0;
+static int ignored = 0;
+static int ignored_only = 0;
+
+static const char builtin_clean_usage[] =
+"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+
+static int git_clean_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "clean.requireforce")) {
+		disabled = git_config_bool(var, value);
+	}
+	return 0;
+}
+
+int cmd_clean(int argc, const char **argv, const char *prefix)
+{
+	int i, j;
+	struct strbuf directory;
+	struct dir_struct dir;
+	const char *path = ".";
+	const char *base = "";
+	int baselen = 0;
+	static const char **pathspec;
+
+	memset(&dir, 0, sizeof(dir));
+	git_config(git_clean_config);
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (arg[0] != '-')
+			break;
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(arg, "-n")) {
+			show_only = 1;
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-f")) {
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-d")) {
+			remove_directories = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-q")) {
+			quiet = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-x")) {
+			ignored = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-X")) {
+			ignored_only = 1;
+			dir.show_ignored =1;
+			dir.exclude_per_dir = ".gitignore";
+			continue;
+		}
+		usage(builtin_clean_usage);
+	}
+
+	if (ignored && ignored_only)
+		die("-x and -X cannot be used together");
+
+	if (disabled)
+		die("clean.requireForce set and -n or -f not given; refusing to clean");
+
+	dir.show_other_directories = 1;
+
+	if (!ignored) {
+		dir.exclude_per_dir = ".gitignore";
+		if (!access(git_path("info/exclude"), F_OK)) {
+			char *exclude_path = git_path("info/exclude");
+			add_excludes_from_file(&dir, exclude_path);
+		}
+	}
+
+	pathspec = get_pathspec(prefix, argv + i);
+	read_cache();
+	read_directory(&dir, path, base, baselen, pathspec);
+	strbuf_init(&directory, 0);
+
+	for (j = 0; j < dir.nr; ++j) {
+		struct dir_entry *ent = dir.entries[j];
+		int len, pos;
+		struct cache_entry *ce;
+		struct stat st;
+
+		/*
+		 * Remove the '/' at the end that directory
+		 * walking adds for directory entries.
+		 */
+		len = ent->len;
+		if (len && ent->name[len-1] == '/')
+			len--;
+		pos = cache_name_pos(ent->name, len);
+		if (0 <= pos)
+			continue;	/* exact match */
+		pos = -pos - 1;
+		if (pos < active_nr) {
+			ce = active_cache[pos];
+			if (ce_namelen(ce) == len &&
+			    !memcmp(ce->name, ent->name, len))
+				continue; /* Yup, this one exists unmerged */
+		}
+
+		/* remove the files */
+		if (!lstat(ent->name, &st) && (S_ISDIR(st.st_mode))) {
+			strbuf_addstr(&directory, ent->name);
+			if (show_only && remove_directories) {
+				printf("Would remove %s\n", directory.buf);
+			} else if (quiet && remove_directories) {
+				remove_dir_recursively(&directory, 0);
+			} else if (remove_directories) {
+				printf("Removing %s\n", ent->name);
+				remove_dir_recursively(&directory, 0);
+			} else if (show_only) {
+				printf("Would not remove %s\n", directory.buf);
+			} else {
+				printf("Not removing %s\n", directory.buf);
+			}
+			strbuf_reset(&directory);
+		} else {
+			if (show_only) {
+				printf("Would remove %s\n", ent->name);
+				continue;
+			} else if (!quiet) {
+				printf("Removing %s\n", ent->name);
+			}
+			unlink(ent->name);
+		}
+	}
+
+	strbuf_release(&directory);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 2335c01..0cbd685 100644
--- a/builtin.h
+++ b/builtin.h
@@ -24,6 +24,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_clean(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
diff --git a/git-clean.sh b/contrib/examples/git-clean.sh
similarity index 100%
rename from git-clean.sh
rename to contrib/examples/git-clean.sh
diff --git a/git.c b/git.c
index 19a2172..30b7c22 100644
--- a/git.c
+++ b/git.c
@@ -298,6 +298,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
-- 
1.5.3.GIT

^ permalink raw reply related

* [RFC] Second attempt at making git-clean a builtin
From: Shawn Bohrer @ 2007-11-04 19:02 UTC (permalink / raw)
  To: git; +Cc: gitster


I've taken all of the comments I received from my previous attempt see:

http://marc.info/?l=git&m=119181975419521&w=2

With these new changes in place my new git-clean passes all of the
original tests as well as the new tests I've added.  While looking at
how git-ls-files walks the tree there were some things that didn't quite
understand, or thought might be unnecessary so there may be some things I
missed.  For example I'm still not quite sure what verify_pathspec()
does.

I did however notice what I would call a bug in the behavior of
git-ls-files and therefore the current git-clean.sh.  With the current
git-clean if you have two directories that contain only untracked files,
for example docs/ and examples/ running:

git clean docs/ examples/

will not remove either directory.  Instead you must use the -d
parameter.  To me this makes sense, however if you run:

git clean docs/

it will remove the docs directory without using the -d parameter.  My
patch is at least consistent in that it requires the -d in both cases.

^ permalink raw reply


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