Git development
 help / color / mirror / Atom feed
* Re: .gitignore, .gitattributes, .gitmodules, .gitprecious?,.gitacls? etc.
From: Jan Hudec @ 2007-09-04 20:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Dmitry Kakurin, Petr Baudis, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0708280945350.28586@racer.site>

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

On Tue, Aug 28, 2007 at 09:49:47 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 27 Aug 2007, Dmitry Kakurin wrote:
> 
> > Here is the problem: we need to apply crlf attributes to a file. We
> > could have .gitattributes both in the index and in the worktree.
> > Which one do we use?
> > In general .gitattributes file could be (U)nchanged, (C)hanged, (NP)
> > NotPresent in each place.
> 
> I do not see these cases.  You can have these cases, basically:
> 
> - .gitattributes in worktree (then it does not matter what else we have),
> - .gitattributes not in the worktree, but in the index (then that is taken)
> 
> In the latter case, there could be conflicts _in_ .gitattributes, in which 
> case those .gitattributes are ignored.
> 
> I do not see any problem with that.

I do.

IMNSHO it should be the other way around:
 .gitattributes in index, than index version is used.
 .gitattributes not in index, but in worktree, than that tree version is used.

Why? Because when you check out another version, the .gitattributes commited
in that version need to be applied, since it might be different from whatever
is currently in the tree.

In the other direction, the tree version seems to make more sense, but in
reality it does not. If you do a partial commit of a single file, than the
index version gets into the commit, so that should better be the version used
to store the file. On the other hand if you change .gitattributes, the normal
commit rules are that you need to add it for commit, so needing to add it to
make it effective goes well together with it. update-index would need to
always handle .git* before other entries to make add . and commit -a work
correctly.

The case for worktree only is for cases when you for some reason want to have
local .gitattributes. Though I am not sure that should actually work, because
you couldn't have local .gitattributes if there is versioned version.
.git/info/attributes would be better for that. Which gets us back to "always
use .gitattributes from *index*" (and read it from tree before other files
when adding it).

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

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

^ permalink raw reply

* Re: [PATCH] Add post-merge hook.
From: Josh England @ 2007-09-04 20:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodgimc53.fsf@gitster.siamese.dyndns.org>

On Tue, 2007-09-04 at 13:03 -0700, Junio C Hamano wrote:
> >> >>  * Do you want to run the post-merge hook even for a squash
> >> >>    merge?
> >> >
> >> > Yes.  I'd like to run it at any time that the working tree might be
> >> > updated.
> >> 
> >> If that is the case, perhaps your hook may want to get a
> >> parameter to tell it what kind of "git-merge" invocation it was?
> >> Squash merge does not even advance the HEAD and is of a very
> >> different nature from a normal merge.
> >
> > OK.  Should it just pass in a flag (squash or normal), or are there
> > other merge types it should need to know about.
> 
> I suspect you have thought abuot the issues involved longer than
> I have ;-), so you should take whatever I say with grain of
> salt, but I think you would also want to know fast-forwards and
> up-to-dates if the hook wants to be generic, not "for Josh's
> workflow only".

Generic is great, I'm just trying to figure out when/why someone would
need to know the exact type of merge operation used.  The hook should be
generic, yet not require an end user to know any git internals not
explicitly mentioned in the git-merge man page.  I'm thinking that it
will be sufficient to pass a flag indicating whether the working tree
has been modified or not.  The flag can be set for normal merge and
fast-forward merges, and unset for up-to-dates and squash merges.  I
don't really know git internals myself.  Am I missing anything?

-JE

^ permalink raw reply

* Re: .gitignore, .gitattributes, .gitmodules, .gitprecious?, .gitacls? etc.
From: Jan Hudec @ 2007-09-04 20:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Sam Vilain, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0708290007020.28586@racer.site>

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

On Wed, Aug 29, 2007 at 00:07:55 +0100, Johannes Schindelin wrote:
> On Wed, 29 Aug 2007, Sam Vilain wrote:
> > Johannes Schindelin wrote:
> > >> Ok, but let's say for a moment that file properties are allowed, and 
> > >> that they are stored in the Reiser4 fashion. On filesystems that did not 
> > >> support this, it would be the only way to get at them - to go through 
> > >> the index. Unless they were also mapped to regular files, or 
> > >> filesystem-specific features somehow.
> > > 
> > > Happily, file properties as well hidden as these have _no_ _place_ in 
> > > source code that needs to be tracked.
> > 
> > But you're restricting your statements to tidy, sane code bases.  Are
> > there any particular reasons that git shouldn't be able to track insane
> > code bases, with attributes etc?  It sure would shut up a whole load of
> > people.
> 
> To the contrary.  People having those insane setups seem to be unable to 
> admit it.  And I'm sure you saw some on this very list, like me.  They 
> never shut up, they only get louder.

Whether it's insane depends on whether you want to keep git purely for
source control -- in which case they are definitely insane -- or want to
allow git to grow to a tool useful for other cases, like tracking content of
/etc, whole filesystem images and similar stuff -- in which case most of
those setups are not insane at all.

Personally I would vote for a middle ground. To keep core git simple, but to
provide enough hooks to build the other tools on top of it. Enough hooks in
this case would mean (at least the way I can imagine as workable) hooks to
run when transfering files between worktree and index and place for the hooks
to store the extra tracked information. This place could be some of special
named file (.gitxattrs or anything) which would be updated in index by the
hook, special index entries somehow related to individual filenames and/or
headers prepended to the blobs and trees. Any of those methods can be used
for storing permissions, acl, extended attributes and other random stuff and
different ones could be used by different hooks at the same time.

Each of those methods has it's advantages and disadvantages. The special
files are easiest to store if the hook is not available or can't represent
the data in given worktree -- simply make the special files regular. The
entries attached to files (which are equivalent of extended attributes) would
work with standard merge on them (because they are handled individually) and
the headers to files would probably require least changes to git (I believe
they are implementable for files with git as is using the hook provided for
keyword expansion (or is it not there?)).

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

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

^ permalink raw reply

* [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Lukas Sandström @ 2007-09-04 20:50 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

Hi.

This is an attempt to use "The Better String Library"[1] in builtin-mailinfo.c

The patch doesn't pass all the tests in the testsuit yet, but I thought I'd
send it out so people can decide if they like how the code looks.

I'm not sending a patch to add the library files at this time. I'll send
that patch when this patch is working.

The changes required to make it pass the tests shouldn't be very large.

/Lukas

[1] http://bstring.sourceforge.net/

---
 builtin-mailinfo.c |  795 ++++++++++++++++++++++++++--------------------------
 1 files changed, 392 insertions(+), 403 deletions(-)

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index d7cb11d..2ddc15d 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -5,14 +5,14 @@
 #include "cache.h"
 #include "builtin.h"
 #include "utf8.h"
+#include "bstring/bstrlib.h"
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
 static int keep_subject;
-static const char *metainfo_charset;
-static char line[1000];
-static char name[1000];
-static char email[1000];
+static bstring metainfo_charset;
+static bstring name;
+static bstring email;
 
 static enum  {
 	TE_DONTCARE, TE_QP, TE_BASE64,
@@ -21,321 +21,291 @@ static enum  {
 	TYPE_TEXT, TYPE_OTHER,
 } message_type;
 
-static char charset[256];
+static bstring charset;
 static int patch_lines;
-static char **p_hdr_data, **s_hdr_data;
+static bstring *p_hdr_data, *s_hdr_data;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
-static char *sanity_check(char *name, char *email)
+static bstring sanity_check(bstring name, bstring email)
 {
-	int len = strlen(name);
-	if (len < 3 || len > 60)
+	static struct tagbstring email_ind = bsStatic("<@>");
+	if (blength(name) < 3 || blength(name) > 60)
 		return email;
-	if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
+	if (binchr(name, 0, &email_ind) != BSTR_ERR)
 		return email;
 	return name;
 }
 
-static int bogus_from(char *line)
+static int bogus_from(const_bstring line)
 {
 	/* John Doe <johndoe> */
-	char *bra, *ket, *dst, *cp;
-
+	int bra, ket;
 	/* This is fallback, so do not bother if we already have an
 	 * e-mail address.
 	 */
-	if (*email)
+	if (blength(email))
 		return 0;
 
-	bra = strchr(line, '<');
-	if (!bra)
+	bra = bstrchr(line, '<');
+	if (bra == BSTR_ERR)
 		return 0;
-	ket = strchr(bra, '>');
-	if (!ket)
+	ket = bstrchrp(line, bra, '>');
+	if (ket == BSTR_ERR)
 		return 0;
 
-	for (dst = email, cp = bra+1; cp < ket; )
-		*dst++ = *cp++;
-	*dst = 0;
-	for (cp = line; isspace(*cp); cp++)
-		;
-	for (bra--; isspace(*bra); bra--)
-		*bra = 0;
-	cp = sanity_check(cp, email);
-	strcpy(name, cp);
+	bdestroy(email);
+	email = bmidstr(line, bra + 1, ket - bra - 1);
+
+	name = bmidstr(line, 0, bra);
+	btrimws(name);
+	bassign(name, sanity_check(name, email));
 	return 1;
 }
 
-static int handle_from(char *in_line)
+static int handle_from(const_bstring line)
 {
-	char line[1000];
-	char *at;
-	char *dst;
+	int at, es, ee;
+	static struct tagbstring email_delim = bsStatic(" \n\t\r\v\f<>");
 
-	strcpy(line, in_line);
-	at = strchr(line, '@');
-	if (!at)
+	at = bstrchr(line, '@');
+	if (at == BSTR_ERR)
 		return bogus_from(line);
 
 	/*
 	 * If we already have one email, don't take any confusing lines
 	 */
-	if (*email && strchr(at+1, '@'))
+	if (blength(email) && bstrchrp(line, '@', at + 1) != BSTR_ERR)
 		return 0;
 
 	/* Pick up the string around '@', possibly delimited with <>
-	 * pair; that is the email part.  White them out while copying.
+	 * pair; that is the email part.
 	 */
-	while (at > line) {
-		char c = at[-1];
-		if (isspace(c))
-			break;
-		if (c == '<') {
-			at[-1] = ' ';
-			break;
-		}
-		at--;
-	}
-	dst = email;
-	for (;;) {
-		unsigned char c = *at;
-		if (!c || c == '>' || isspace(c)) {
-			if (c == '>')
-				*at = ' ';
-			break;
-		}
-		*at++ = ' ';
-		*dst++ = c;
-	}
-	*dst++ = 0;
+
+	es = binchrr(line, at, &email_delim);
+	ee = binchr(line, at, &email_delim);
+	bdestroy(email);
+	email = bmidstr(line, es + 1, ee - es - 1);
 
 	/* The remainder is name.  It could be "John Doe <john.doe@xz>"
 	 * or "john.doe@xz (John Doe)", but we have whited out the
 	 * email part, so trim from both ends, possibly removing
 	 * the () pair at the end.
 	 */
-	at = line + strlen(line);
-	while (at > line) {
-		unsigned char c = *--at;
-		if (!isspace(c)) {
-			at[(c == ')') ? 0 : 1] = 0;
-			break;
-		}
-	}
 
-	at = line;
-	for (;;) {
-		unsigned char c = *at;
-		if (!c || !isspace(c)) {
-			if (c == '(')
-				at++;
-			break;
-		}
-		at++;
-	}
-	at = sanity_check(at, email);
-	strcpy(name, at);
+	bdestroy(name);
+	name = bstrcpy(line);
+	bdelete(name, es, ee - es + 1);
+	btrimws(name);
+	if (bchar(name, 0) == '(')
+		bdelete(name, 0, 1);
+	if (bchar(name, blength(name) - 1) == ')')
+		btrunc(name, blength(name) - 1);
+	
+	bassign(name, sanity_check(name, email));
 	return 1;
 }
 
-static int handle_header(char *line, char *data, int ofs)
-{
-	if (!line || !data)
-		return 1;
-
-	strcpy(data, line+ofs);
-
-	return 0;
-}
-
 /* NOTE NOTE NOTE.  We do not claim we do full MIME.  We just attempt
  * to have enough heuristics to grok MIME encoded patches often found
  * on our mailing lists.  For example, we do not even treat header lines
  * case insensitively.
  */
 
-static int slurp_attr(const char *line, const char *name, char *attr)
+static bstring slurp_attr(const_bstring line, const char *name)
 {
-	const char *ends, *ap = strcasestr(line, name);
-	size_t sz;
+	int end, start;
+	static struct tagbstring endchars = bsStatic("; \t");
+	struct tagbstring bname;
 
-	if (!ap) {
-		*attr = 0;
-		return 0;
-	}
-	ap += strlen(name);
-	if (*ap == '"') {
-		ap++;
-		ends = "\"";
+	btfromcstr(bname, name);
+	start =  binstrcaseless(line, 0, &bname);
+	if (start == BSTR_ERR)
+		return NULL;
+	
+	start += blength(&bname);
+	if (blength(line) > start && bchar(line, start) == '"') {
+		start++;
+		if ((end = bstrchrp(line, start, '"')) == BSTR_ERR)
+			end = blength(line);
+		return bmidstr(line, start, end - start);
 	}
-	else
-		ends = "; \t";
-	sz = strcspn(ap, ends);
-	memcpy(attr, ap, sz);
-	attr[sz] = 0;
-	return 1;
+	if ((end = binchr(line, start, &endchars)) == BSTR_ERR)
+		end = blength(line);
+	return bmidstr(line, start, end - start);
 }
 
 struct content_type {
-	char *boundary;
-	int boundary_len;
+	bstring boundary;
 };
 
 static struct content_type content[MAX_BOUNDARIES];
 
 static struct content_type *content_top = content;
 
-static int handle_content_type(char *line)
+static int handle_content_type(const_bstring line)
 {
-	char boundary[256];
-
-	if (strcasestr(line, "text/") == NULL)
+	static struct tagbstring cmp_text = bsStatic("text/");
+	bstring attr, boundary;
+	
+	if (binstrcaseless(line, 0, &cmp_text) == BSTR_ERR)
 		 message_type = TYPE_OTHER;
-	if (slurp_attr(line, "boundary=", boundary + 2)) {
-		memcpy(boundary, "--", 2);
+
+	if ((attr = slurp_attr(line, "boundary="))) {
+		boundary = bfromcstr("--");
+		bconcat(boundary, attr);
+		bdestroy(attr);
 		if (content_top++ >= &content[MAX_BOUNDARIES]) {
 			fprintf(stderr, "Too many boundaries to handle\n");
 			exit(1);
 		}
-		content_top->boundary_len = strlen(boundary);
-		content_top->boundary = xmalloc(content_top->boundary_len+1);
-		strcpy(content_top->boundary, boundary);
+		content_top->boundary = boundary;
+		return 0;
 	}
-	if (slurp_attr(line, "charset=", charset)) {
-		int i, c;
-		for (i = 0; (c = charset[i]) != 0; i++)
-			charset[i] = tolower(c);
+	if ((attr = slurp_attr(line, "charset="))) {
+		if (btolower(attr) == BSTR_ERR)
+			die("Couldn't convert %s to lowercase.\n", attr->data);
+		charset = attr;
 	}
 	return 0;
 }
 
-static int handle_content_transfer_encoding(char *line)
+static int handle_content_transfer_encoding(const_bstring line)
 {
-	if (strcasestr(line, "base64"))
+	static struct tagbstring cmp_base64 = bsStatic("base64");
+	static struct tagbstring cmp_qp = bsStatic("quoted-printable");
+
+	if (binstrcaseless(line, 0, &cmp_base64) != BSTR_ERR)
 		transfer_encoding = TE_BASE64;
-	else if (strcasestr(line, "quoted-printable"))
+	else if (binstrcaseless(line, 0, &cmp_qp))
 		transfer_encoding = TE_QP;
 	else
 		transfer_encoding = TE_DONTCARE;
 	return 0;
 }
 
-static int is_multipart_boundary(const char *line)
-{
-	return (!memcmp(line, content_top->boundary, content_top->boundary_len));
-}
-
-static int eatspace(char *line)
+static int is_multipart_boundary(const_bstring line)
 {
-	int len = strlen(line);
-	while (len > 0 && isspace(line[len-1]))
-		line[--len] = 0;
-	return len;
+	return !bstrncmp(line, content_top->boundary, blength(content_top->boundary));
 }
 
-static char *cleanup_subject(char *subject)
+/*
+ * Removes (Re:|[ \[\t:]|\[.*\])* if prefixed and
+ * trims trailing whitespace.
+ */
+static void cleanup_subject(bstring subject)
 {
-	for (;;) {
-		char *p;
-		int len, remove;
-		switch (*subject) {
+	int pos;
+	while (blength(subject)) {
+		switch (bchar(subject, 0)) {
 		case 'r': case 'R':
-			if (!memcmp("e:", subject+1, 2)) {
-				subject += 3;
+			if (blength(subject) <= 3)
+				break;
+			if (!memcmp(bdata(subject) + 1, "e:", 2)) {
+				bdelete(subject, 0, 3);
 				continue;
 			}
 			break;
-		case ' ': case '\t': case ':':
-			subject++;
-			continue;
-
 		case '[':
-			p = strchr(subject, ']');
-			if (!p) {
-				subject++;
-				continue;
-			}
-			len = strlen(p);
-			remove = p - subject;
-			if (remove <= len *2) {
-				subject = p+1;
-				continue;
+			if ((pos = bstrchr(subject, ']')) != BSTR_ERR) {
+				/* Don't remove more than a third of the subject. */
+				if (pos <= blength(subject)/3) {
+					bdelete(subject, 0, pos + 1);
+					continue;
+				}
+				break;
 			}
-			break;
+		/* fall through */
+		case ' ': case '\t': case ':':
+			bdelete(subject, 0, 1);
+			continue;
 		}
-		eatspace(subject);
-		return subject;
+
+		btrimws(subject);
+		return;
 	}
 }
 
-static void cleanup_space(char *buf)
+static void cleanup_space(bstring buf)
 {
-	unsigned char c;
-	while ((c = *buf) != 0) {
-		buf++;
-		if (isspace(c)) {
-			buf[-1] = ' ';
-			c = *buf;
-			while (isspace(c)) {
-				int len = strlen(buf);
-				memmove(buf, buf+1, len);
-				c = *buf;
-			}
+	struct bstrList *tok;
+	static struct tagbstring whitespace = bsStatic(" \n\t\r\f\v");
+	int i;
+
+	tok = bsplitstr(buf, &whitespace);
+	btrunc(buf, 0);
+	for (i = 0; i < tok->qty; i++) {
+		if (blength(tok->entry[i])) {
+			bconcat(buf, tok->entry[i]);
+			bconchar(buf, ' ');
 		}
 	}
+	/* Remove the last ' ' */
+	btrunc(buf, blength(buf) - 1);
+	bstrListDestroy(tok);
+}
+
+static int handle_header(bstring line, bstring *data, int ofs)
+{
+	if (!line)
+		return 1;
+
+	bdestroy(*data);
+	*data = bmidstr(line, ofs, blength(line) - ofs);
+
+	return 0;
 }
 
-static void decode_header(char *it, unsigned itsize);
+static void decode_header(bstring line);
 static char *header[MAX_HDR_PARSED] = {
 	"From","Subject","Date",
 };
 
-static int check_header(char *line, unsigned linesize, char **hdr_data, int overwrite)
+static int check_header(bstring line, bstring hdr_data[], int overwrite)
 {
 	int i;
 
 	/* search for the interesting parts */
 	for (i = 0; header[i]; i++) {
 		int len = strlen(header[i]);
+
 		if ((!hdr_data[i] || overwrite) &&
-		    !strncasecmp(line, header[i], len) &&
-		    line[len] == ':' && isspace(line[len + 1])) {
+		    bisstemeqcaselessblk(line, header[i], len) &&
+		    bchar(line, len) == ':' && isspace(bchar(line, len + 1))) {
 			/* Unwrap inline B and Q encoding, and optionally
 			 * normalize the meta information to utf8.
 			 */
-			decode_header(line + len + 2, linesize - len - 2);
-			hdr_data[i] = xmalloc(1000 * sizeof(char));
-			if (! handle_header(line, hdr_data[i], len + 2)) {
+			decode_header(line);
+			if (!handle_header(line, &hdr_data[i], len + 2)) {
 				return 1;
 			}
 		}
 	}
 
 	/* Content stuff */
-	if (!strncasecmp(line, "Content-Type", 12) &&
-		line[12] == ':' && isspace(line[12 + 1])) {
-		decode_header(line + 12 + 2, linesize - 12 - 2);
+	if (!bisstemeqcaselessblk(line, bsStaticBlkParms("Content-Type")) &&
+		bchar(line, 12) == ':' && isspace(bchar(line, 12 + 1))) {
+		decode_header(line);
 		if (! handle_content_type(line)) {
 			return 1;
 		}
 	}
-	if (!strncasecmp(line, "Content-Transfer-Encoding", 25) &&
-		line[25] == ':' && isspace(line[25 + 1])) {
-		decode_header(line + 25 + 2, linesize - 25 - 2);
+	if (!bisstemeqcaselessblk(line, bsStaticBlkParms("Content-Transfer-Encoding")) &&
+		bchar(line, 25) == ':' && isspace(bchar(line, 25 + 1))) {
+		decode_header(line);
 		if (! handle_content_transfer_encoding(line)) {
 			return 1;
 		}
 	}
 
 	/* for inbody stuff */
-	if (!memcmp(">From", line, 5) && isspace(line[5]))
+	if (bisstemeqblk(line, bsStaticBlkParms(">From")) && isspace(bchar(line, 5)))
 		return 1;
-	if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
+	if (bisstemeqblk(line, bsStaticBlkParms("[PATCH]")) && isspace(bchar(line, 7))) {
 		for (i = 0; header[i]; i++) {
-			if (!memcmp("Subject: ", header[i], 9)) {
-				if (! handle_header(line, hdr_data[i], 0)) {
+			if (!memcmp("Subject", header[i], 7)) {
+				if (!handle_header(line, &hdr_data[i], 0)) {
 					return 1;
 				}
 			}
@@ -346,7 +316,7 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
 	return 0;
 }
 
-static int is_rfc2822_header(char *line)
+static int is_rfc2822_header(const_bstring line)
 {
 	/*
 	 * The section that defines the loosest possible
@@ -357,15 +327,15 @@ static int is_rfc2822_header(char *line)
 	 * ftext = %d33-57 / %59-126
 	 */
 	int ch;
-	char *cp = line;
+	char *cp = bdata(line);
 
 	/* Count mbox From headers as headers */
-	if (!memcmp(line, "From ", 5) || !memcmp(line, ">From ", 6))
+	if (blength(line) >= 6 && (!memcmp(cp, "From ", 5) || !memcmp(cp, ">From ", 6)))
 		return 1;
 
 	while ((ch = *cp++)) {
 		if (ch == ':')
-			return cp != line;
+			return cp != bdata(line);
 		if ((33 <= ch && ch <= 57) ||
 		    (59 <= ch && ch <= 126))
 			continue;
@@ -375,34 +345,23 @@ static int is_rfc2822_header(char *line)
 }
 
 /*
- * sz is size of 'line' buffer in bytes.  Must be reasonably
- * long enough to hold one physical real-world e-mail line.
+ * 'line' must be a valid bstring
  */
-static int read_one_header_line(char *line, int sz, FILE *in)
+static int read_one_header_line(struct bStream *in, bstring line)
 {
-	int len;
-
-	/*
-	 * We will read at most (sz-1) bytes and then potentially
-	 * re-add NUL after it.  Accessing line[sz] after this is safe
-	 * and we can allow len to grow up to and including sz.
-	 */
-	sz--;
-
 	/* Get the first part of the line. */
-	if (!fgets(line, sz, in))
-		return 0;
+	if (bsreadln(line, in, '\n') != BSTR_OK)
+		goto unread_line;
 
 	/*
 	 * Is it an empty line or not a valid rfc2822 header?
 	 * If so, stop here, and return false ("not a header")
 	 */
-	len = eatspace(line);
-	if (!len || !is_rfc2822_header(line)) {
+	brtrimws(line);
+	if (!blength(line) || !is_rfc2822_header(line)) {
 		/* Re-add the newline */
-		line[len] = '\n';
-		line[len + 1] = '\0';
-		return 0;
+		bconchar(line, '\n');
+		goto unread_line;
 	}
 
 	/*
@@ -410,63 +369,57 @@ static int read_one_header_line(char *line, int sz, FILE *in)
 	 * Yuck, 2822 header "folding"
 	 */
 	for (;;) {
-		int peek, addlen;
-		static char continuation[1000];
+		bstring continuation;
+		continuation = bfromcstr("");
 
-		peek = fgetc(in); ungetc(peek, in);
-		if (peek != ' ' && peek != '\t')
+		if (bsreadln(continuation, in, '\n') != BSTR_OK)
 			break;
-		if (!fgets(continuation, sizeof(continuation), in))
+		if (bchar(continuation, 0) != ' ' && bchar(continuation, 0) != '\t') {
+			bsunread(in, continuation);
 			break;
-		addlen = eatspace(continuation);
-		if (len < sz - 1) {
-			if (addlen >= sz - len)
-				addlen = sz - len - 1;
-			memcpy(line + len, continuation, addlen);
-			line[len] = '\n';
-			len += addlen;
 		}
+
+		continuation->data[0] = '\n';
+		brtrimws(continuation);
+		bconcat(line, continuation);
 	}
-	line[len] = 0;
 
 	return 1;
+unread_line:
+	bsunread(in, line);
+	return 0;
 }
 
-static int decode_q_segment(char *in, char *ot, unsigned otsize, char *ep, int rfc2047)
+static bstring decode_q_segment(bstring line, int rfc2047)
 {
-	char *otend = ot + otsize;
+	char *in = bdata(line);
 	int c;
-	while ((c = *in++) != 0 && (in <= ep)) {
-		if (ot == otend) {
-			*--ot = '\0';
-			return -1;
-		}
+	bstring out = bfromcstralloc(blength(line), "");
+
+	while ((c = *in++) != 0) {
 		if (c == '=') {
 			int d = *in++;
 			if (d == '\n' || !d)
 				break; /* drop trailing newline */
-			*ot++ = ((hexval(d) << 4) | hexval(*in++));
+			bconchar(out, (hexval(d) << 4) | hexval(*in++));
 			continue;
 		}
 		if (rfc2047 && c == '_') /* rfc2047 4.2 (2) */
 			c = 0x20;
-		*ot++ = c;
+		bconchar(out, c);
 	}
-	*ot = 0;
-	return 0;
+	return out;
 }
 
-static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
+static bstring decode_b_segment(bstring line)
 {
 	/* Decode in..ep, possibly in-place to ot */
 	int c, pos = 0, acc = 0;
-	char *otend = ot + otsize;
+	char *in = bdata(line);
+	bstring out;
 
-	while ((c = *in++) != 0 && (in <= ep)) {
-		if (ot == otend) {
-			*--ot = '\0';
-			return -1;
-		}
+	out = bfromcstralloc(blength(line), "");
+	while ((c = *in++) != 0) {
 		if (c == '+')
 			c = 62;
 		else if (c == '/')
@@ -491,21 +444,20 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
 			acc = (c << 2);
 			break;
 		case 1:
-			*ot++ = (acc | (c >> 4));
+			bconchar(out, (acc | (c >> 4)));
 			acc = (c & 15) << 4;
 			break;
 		case 2:
-			*ot++ = (acc | (c >> 2));
+			bconchar(out, (acc | (c >> 2)));
 			acc = (c & 3) << 6;
 			break;
 		case 3:
-			*ot++ = (acc | c);
+			bconchar(out, (acc | c));
 			acc = pos = 0;
 			break;
 		}
 	}
-	*ot = 0;
-	return 0;
+	return out;
 }
 
 /*
@@ -519,147 +471,174 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
  * Otherwise, we default to assuming it is Latin1 for historical
  * reasons.
  */
-static const char *guess_charset(const char *line, const char *target_charset)
+static bstring guess_charset(bstring line, bstring target_charset)
 {
-	if (is_encoding_utf8(target_charset)) {
-		if (is_utf8(line))
+	//FIXME: convert utf8.c to bstring
+	if (is_encoding_utf8(bdata(target_charset))) {
+		if (is_utf8(bdata(line)))
 			return NULL;
 	}
-	return "latin1";
+	return bfromcstr("latin1");
 }
 
-static void convert_to_utf8(char *line, unsigned linesize, const char *charset)
+static void convert_to_utf8(bstring line, bstring charset)
 {
-	char *out;
+	bstring out;
+	char *cout;
 
-	if (!charset || !*charset) {
-		charset = guess_charset(line, metainfo_charset);
-		if (!charset)
+	if (blength(charset) == 0) {
+		out = guess_charset(line, metainfo_charset);
+		if (!out) {
+			bdestroy(charset);
+			charset = NULL;
 			return;
+		}
+		bassign(charset, out);
+		bdestroy(out);
 	}
 
-	if (!strcmp(metainfo_charset, charset))
+	if (!bstrcmp(metainfo_charset, charset))
 		return;
-	out = reencode_string(line, metainfo_charset, charset);
-	if (!out)
+	//FIXME: convert utf8.c to use bstring
+	cout = reencode_string(bdata(line), bdata(metainfo_charset), bdata(charset));
+	if (!cout)
 		die("cannot convert from %s to %s\n",
-		    charset, metainfo_charset);
-	strlcpy(line, out, linesize);
-	free(out);
+		    bdata(charset), bdata(metainfo_charset));
+	bassigncstr(line, cout);
 }
 
-static int decode_header_bq(char *it, unsigned itsize)
+static int decode_header_bq(bstring line)
 {
-	char *in, *out, *ep, *cp, *sp;
-	char outbuf[1000];
+	bstring out;
+	bstring decoded = NULL, charset_q = NULL, tmp;
 	int rfc2047 = 0;
+	int in = 0, cp, ep;
 
-	in = it;
-	out = outbuf;
-	while ((ep = strstr(in, "=?")) != NULL) {
-		int sz, encoding;
-		char charset_q[256], piecebuf[256];
+	struct tagbstring cmp_eq_qst = bsStatic("=?");
+	struct tagbstring cmp_qst_eq = bsStatic("?=");
+
+	out = bfromcstralloc(blength(line), "");
+
+	while ((ep = binstr(line, 0, &cmp_eq_qst)) != BSTR_ERR) {
+		int encoding;
 		rfc2047 = 1;
 
-		if (in != ep) {
-			sz = ep - in;
-			memcpy(out, in, sz);
-			out += sz;
-			in += sz;
-		}
+		bcatblk(out, bdataofs(line, in), ep - in);
+		in += ep - in + 2;
 		/* E.g.
 		 * ep : "=?iso-2022-jp?B?GyR...?= foo"
 		 * ep : "=?ISO-8859-1?Q?Foo=FCbar?= baz"
 		 */
-		ep += 2;
-		cp = strchr(ep, '?');
-		if (!cp)
-			return rfc2047; /* no munging */
-		for (sp = ep; sp < cp; sp++)
-			charset_q[sp - ep] = tolower(*sp);
-		charset_q[cp - ep] = 0;
-		encoding = cp[1];
-		if (!encoding || cp[2] != '?')
-			return rfc2047; /* no munging */
-		ep = strstr(cp + 3, "?=");
-		if (!ep)
-			return rfc2047; /* no munging */
+
+		cp = bstrchrp(line, in, '?');
+		if (cp == BSTR_ERR)
+			goto out0; /* no munging */
+
+		charset_q = bmidstr(line, in, cp - in);
+		if (charset_q->slen)
+			btolower(charset_q);
+
+		if (line->slen < cp + 2)
+			goto out1;
+			//die("Bad header: %s,", line->data);
+
+		encoding = bchar(line, cp + 1);
+		if (!encoding || bchar(line, cp + 2) != '?')
+			goto out1; /* no munging */
+		ep = binstr(line, cp + 3, &cmp_qst_eq);
+		if (ep == BSTR_ERR)
+			goto out1; /* no munging */
 		switch (tolower(encoding)) {
 		default:
-			return rfc2047; /* no munging */
+			goto out1; /* no munging */
 		case 'b':
-			sz = decode_b_segment(cp + 3, piecebuf, sizeof(piecebuf), ep);
+			//FIXME: use bmid2tbstr ?
+			// Needs to change the decode function to not look for null
+			tmp = bmidstr(line, cp + 3, ep - cp -3);
+			decoded = decode_b_segment(tmp);
 			break;
 		case 'q':
-			sz = decode_q_segment(cp + 3, piecebuf, sizeof(piecebuf), ep, 1);
+			tmp = bmidstr(line, cp + 3, ep - cp -3);
+			decoded = decode_q_segment(tmp, 1);
 			break;
 		}
-		if (sz < 0)
-			return rfc2047;
+		bdestroy(tmp);
+		if (decoded == NULL)
+			goto out1;
 		if (metainfo_charset)
-			convert_to_utf8(piecebuf, sizeof(piecebuf), charset_q);
+			convert_to_utf8(decoded, charset_q);
 
-		sz = strlen(piecebuf);
-		if (outbuf + sizeof(outbuf) <= out + sz)
-			return rfc2047; /* no munging */
-		strcpy(out, piecebuf);
-		out += sz;
+		bconcat(out, decoded);
 		in = ep + 2;
+
+		bdestroy(decoded);
+		bdestroy(charset_q);
 	}
-	strcpy(out, in);
-	strlcpy(it, outbuf, itsize);
+	/* Add the remainder of the line. */
+	bcatblk(out, bdataofs(line, in), blength(line) - in);
+
+	bassign(line, out);
+
+	bdestroy(decoded);
+out1:
+	bdestroy(charset_q);
+out0:
+	bdestroy(out);
 	return rfc2047;
 }
 
-static void decode_header(char *it, unsigned itsize)
+static void decode_header(bstring line)
 {
-
-	if (decode_header_bq(it, itsize))
+	if (decode_header_bq(line))
 		return;
 	/* otherwise "it" is a straight copy of the input.
 	 * This can be binary guck but there is no charset specified.
 	 */
 	if (metainfo_charset)
-		convert_to_utf8(it, itsize, "");
+		convert_to_utf8(line, NULL);
 }
 
-static void decode_transfer_encoding(char *line, unsigned linesize)
+static void decode_transfer_encoding(bstring line)
 {
-	char *ep;
+	bstring ret = NULL;
 
 	switch (transfer_encoding) {
 	case TE_QP:
-		ep = line + strlen(line);
-		decode_q_segment(line, line, linesize, ep, 0);
+		ret = decode_q_segment(line, 0);
 		break;
 	case TE_BASE64:
-		ep = line + strlen(line);
-		decode_b_segment(line, line, linesize, ep);
+		ret = decode_b_segment(line);
 		break;
 	case TE_DONTCARE:
 		break;
 	}
+	if (ret)
+		bassign(line, ret);
+	bdestroy(ret);
 }
 
-static int handle_filter(char *line, unsigned linesize);
+static int handle_filter(bstring line);
 
-static int find_boundary(void)
+static int find_boundary(struct bStream *in, bstring line)
 {
-	while(fgets(line, sizeof(line), fin) != NULL) {
+	while(bsreadln(line, in, '\n') != BSTR_ERR) {
 		if (is_multipart_boundary(line))
 			return 1;
 	}
 	return 0;
 }
 
-static int handle_boundary(void)
+static int handle_boundary(struct bStream *in, bstring line)
 {
-	char newline[]="\n";
+	struct tagbstring newline = bsStatic("\n");
+	char *c;
 again:
-	if (!memcmp(line+content_top->boundary_len, "--", 2)) {
+	if (blength(line) >= blength(content_top->boundary) + 2 &&
+	    (c = bdataofs(line, blength(content_top->boundary))) &&
+	    !memcmp(c, "--", 2)) {
 		/* we hit an end boundary */
 		/* pop the current boundary off the stack */
-		free(content_top->boundary);
+		bdestroy(content_top->boundary);
 
 		/* technically won't happen as is_multipart_boundary()
 		   will fail first.  But just in case..
@@ -669,49 +648,52 @@ again:
 					"can't recover\n");
 			exit(1);
 		}
-		handle_filter(newline, sizeof(newline));
+		handle_filter(&newline);
 
 		/* skip to the next boundary */
-		if (!find_boundary())
+		if (!find_boundary(in, line)) {
+			bsunread(in, line);
 			return 0;
+		}
 		goto again;
 	}
 
 	/* set some defaults */
 	transfer_encoding = TE_DONTCARE;
-	charset[0] = 0;
+	bassigncstr(charset, "");
 	message_type = TYPE_TEXT;
 
 	/* slurp in this section's info */
-	while (read_one_header_line(line, sizeof(line), fin))
-		check_header(line, sizeof(line), p_hdr_data, 0);
+	while (read_one_header_line(in, line))
+		check_header(line, p_hdr_data, 0);
 
 	/* eat the blank line after section info */
-	return (fgets(line, sizeof(line), fin) != NULL);
+	return (bsreadln(line, in, '\n') != BSTR_ERR);
 }
 
-static inline int patchbreak(const char *line)
+static inline int patchbreak(const_bstring line)
 {
+	int i;
+
 	/* Beginning of a "diff -" header? */
-	if (!memcmp("diff -", line, 6))
+	if (!bisstemeqblk(line, bsStaticBlkParms("diff -")))
 		return 1;
 
 	/* CVS "Index: " line? */
-	if (!memcmp("Index: ", line, 7))
+	if (!bisstemeqblk(line, bsStaticBlkParms("Index: ")))
 		return 1;
 
 	/*
 	 * "--- <filename>" starts patches without headers
 	 * "---<sp>*" is a manual separator
 	 */
-	if (!memcmp("---", line, 3)) {
-		line += 3;
+	if (!bisstemeqblk(line, bsStaticBlkParms("---"))) {
 		/* space followed by a filename? */
-		if (line[0] == ' ' && !isspace(line[1]))
+		if (bchar(line, 3) == ' ' && !isspace(bchar(line, 4)))
 			return 1;
 		/* Just whitespace? */
-		for (;;) {
-			unsigned char c = *line++;
+		for (i = 3; i < blength(line); i++) {
+			unsigned char c = bchar(line, i);
 			if (c == '\n')
 				return 1;
 			if (!isspace(c))
@@ -723,31 +705,25 @@ static inline int patchbreak(const char *line)
 }
 
 
-static int handle_commit_msg(char *line, unsigned linesize)
+static int handle_commit_msg(bstring line)
 {
 	static int still_looking = 1;
-	char *endline = line + linesize;
+	char *c;
 
 	if (!cmitmsg)
 		return 0;
 
 	if (still_looking) {
-		char *cp = line;
-		if (isspace(*line)) {
-			for (cp = line + 1; *cp; cp++) {
-				if (!isspace(*cp))
-					break;
-			}
-			if (!*cp)
-				return 0;
-		}
-		if ((still_looking = check_header(cp, endline - cp, s_hdr_data, 0)) != 0)
+		brtrimws(line);
+		if (blength(line) == 0)
+			return 0;
+		if ((still_looking = check_header(line, s_hdr_data, 0)) != 0)
 			return 0;
 	}
 
 	/* normalize the log message to UTF-8. */
 	if (metainfo_charset)
-		convert_to_utf8(line, endline - line, charset);
+		convert_to_utf8(line, charset);
 
 	if (patchbreak(line)) {
 		fclose(cmitmsg);
@@ -755,18 +731,24 @@ static int handle_commit_msg(char *line, unsigned linesize)
 		return 1;
 	}
 
-	fputs(line, cmitmsg);
+	if ((c = bdata(line)) == NULL)
+		die("Programming error: line had no data\n");
+	fputs(c, cmitmsg);
 	return 0;
 }
 
-static int handle_patch(char *line)
+static int handle_patch(const_bstring line)
 {
-	fputs(line, patchfile);
+	char *c;
+
+	if ((c = bdata(line)) == NULL)
+		die("Programming error: patch line had no data\n");
+	fputs(c, patchfile);
 	patch_lines++;
 	return 0;
 }
 
-static int handle_filter(char *line, unsigned linesize)
+static int handle_filter(bstring line)
 {
 	static int filter = 0;
 
@@ -775,7 +757,7 @@ static int handle_filter(char *line, unsigned linesize)
 	 */
 	switch (filter) {
 	case 0:
-		if (!handle_commit_msg(line, linesize))
+		if (!handle_commit_msg(line))
 			break;
 		filter++;
 	case 1:
@@ -789,16 +771,19 @@ static int handle_filter(char *line, unsigned linesize)
 	return 0;
 }
 
-static void handle_body(void)
+static void handle_body(struct bStream *in)
 {
-	int rc = 0;
-	static char newline[2000];
-	static char *np = newline;
+	//FIXME: bdestroy line. unread line in more places?
+	bstring line;
+	int rc = 0, i, end;
 
+	line = bfromcstr("");
 	/* Skip up to the first boundary */
-	if (content_top->boundary) {
-		if (!find_boundary())
+	if (content_top->boundary) {//FIXME: ?
+		if (!find_boundary(in, line)) {
+			bsunread(in, line);
 			return;
+		}
 	}
 
 	do {
@@ -806,24 +791,24 @@ static void handle_body(void)
 		if (content_top->boundary && is_multipart_boundary(line)) {
 			/* flush any leftover */
 			if ((transfer_encoding == TE_BASE64)  &&
-			    (np != newline)) {
-				handle_filter(newline, sizeof(newline));
+			    (blength(line))) {
+				handle_filter(line);
 			}
-			if (!handle_boundary())
+			if (!handle_boundary(in, line))
 				return;
 		}
 
 		/* Unwrap transfer encoding */
-		decode_transfer_encoding(line, sizeof(line));
+		decode_transfer_encoding(line);
 
 		switch (transfer_encoding) {
 		case TE_BASE64:
 		{
-			char *op = line;
+			struct bstrList *lines;
 
 			/* binary data most likely doesn't have newlines */
 			if (message_type != TYPE_TEXT) {
-				rc = handle_filter(line, sizeof(newline));
+				rc = handle_filter(line);
 				break;
 			}
 
@@ -832,54 +817,55 @@ static void handle_body(void)
 			 * at a time to handle_filter()
 			 */
 
-			do {
-				while (*op != '\n' && *op != 0)
-					*np++ = *op++;
-				*np = *op;
-				if (*np != 0) {
-					/* should be sitting on a new line */
-					*(++np) = 0;
-					op++;
-					rc = handle_filter(newline, sizeof(newline));
-					np = newline;
-				}
-			} while (*op != 0);
-			/* the partial chunk is saved in newline and
+			lines = bsplit(line, '\n');
+			end = lines->qty - 1;
+			/* the partial chunk is saved in line and
 			 * will be appended by the next iteration of fgets
 			 */
+			if (bchar(line, blength(line) - 1) != '\n') {
+				bassign(line, lines->entry[end]);
+				end--;
+			} else
+				btrunc(line, 0);
+			for (i = 0; i <= end; i++)
+				rc = handle_filter(lines->entry[i]);
+
+			bstrListDestroy(lines);
 			break;
 		}
 		default:
-			rc = handle_filter(line, sizeof(newline));
+			rc = handle_filter(line);
+			btrunc(line, 0);
 		}
 		if (rc)
 			/* nothing left to filter */
 			break;
-	} while (fgets(line, sizeof(line), fin));
+	} while (bsreadlna(line, in, '\n') != BSTR_ERR);
 
 	return;
 }
 
-static void output_header_lines(FILE *fout, const char *hdr, char *data)
+static void output_header_lines(FILE *fout, const char *hdr, const_bstring data)
 {
+	char *sp;
+	sp = bdata(data);
 	while (1) {
-		char *ep = strchr(data, '\n');
+		char *ep = strchr(sp, '\n');
 		int len;
 		if (!ep)
-			len = strlen(data);
+			len = strlen(sp);
 		else
-			len = ep - data;
-		fprintf(fout, "%s: %.*s\n", hdr, len, data);
+			len = ep - sp;
+		fprintf(fout, "%s: %.*s\n", hdr, len, sp);
 		if (!ep)
 			break;
-		data = ep + 1;
+		sp = ep + 1;
 	}
 }
 
 static void handle_info(void)
 {
-	char *sub;
-	char *hdr;
+	bstring hdr;
 	int i;
 
 	for (i = 0; header[i]; i++) {
@@ -893,32 +879,32 @@ static void handle_info(void)
 			continue;
 
 		if (!memcmp(header[i], "Subject", 7)) {
-			if (keep_subject)
-				sub = hdr;
-			else {
-				sub = cleanup_subject(hdr);
-				cleanup_space(sub);
+			if (!keep_subject) {
+				cleanup_subject(hdr);
+				cleanup_space(hdr);
 			}
-			output_header_lines(fout, "Subject", sub);
+			output_header_lines(fout, "Subject", hdr);
 		} else if (!memcmp(header[i], "From", 4)) {
 			handle_from(hdr);
-			fprintf(fout, "Author: %s\n", name);
-			fprintf(fout, "Email: %s\n", email);
+			fprintf(fout, "Author: %s\n", bdata(name));
+			fprintf(fout, "Email: %s\n", bdata(email));
 		} else {
 			cleanup_space(hdr);
-			fprintf(fout, "%s: %s\n", header[i], hdr);
+			fprintf(fout, "%s: %s\n", header[i], bdata(hdr));
 		}
 	}
 	fprintf(fout, "\n");
 }
 
-static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
+static int mailinfo(FILE *in, FILE *out, int ks, const_bstring encoding,
 		    const char *msg, const char *patch)
 {
 	keep_subject = ks;
-	metainfo_charset = encoding;
+	metainfo_charset = bstrcpy(encoding);
 	fin = in;
 	fout = out;
+	bstring line;
+	struct bStream *in_stream = bsopen((bNread) fread, in);
 
 	cmitmsg = fopen(msg, "w");
 	if (!cmitmsg) {
@@ -932,14 +918,17 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
 		return -1;
 	}
 
-	p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
-	s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
+	p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
+	s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
 
 	/* process the email header */
-	while (read_one_header_line(line, sizeof(line), fin))
-		check_header(line, sizeof(line), p_hdr_data, 1);
+	line = bfromcstr("");
+	while (read_one_header_line(in_stream, line))
+		check_header(line, p_hdr_data, 1);
 
-	handle_body();
+	bsunread(in_stream, line);
+	
+	handle_body(in_stream);
 	handle_info();
 
 	return 0;
@@ -958,17 +947,17 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config);
 
 	def_charset = (git_commit_encoding ? git_commit_encoding : "utf-8");
-	metainfo_charset = def_charset;
+	metainfo_charset = bfromcstr(def_charset);
 
 	while (1 < argc && argv[1][0] == '-') {
 		if (!strcmp(argv[1], "-k"))
 			keep_subject = 1;
 		else if (!strcmp(argv[1], "-u"))
-			metainfo_charset = def_charset;
+			bassigncstr(metainfo_charset, def_charset);
 		else if (!strcmp(argv[1], "-n"))
 			metainfo_charset = NULL;
 		else if (!prefixcmp(argv[1], "--encoding="))
-			metainfo_charset = argv[1] + 11;
+			bassigncstr(metainfo_charset, argv[1] + 11);
 		else
 			usage(mailinfo_usage);
 		argc--; argv++;
-- 
1.5.3.rc7

^ permalink raw reply related

* Re: [PATCH] Add post-merge hook.
From: Steven Grimm @ 2007-09-04 20:52 UTC (permalink / raw)
  To: Josh England; +Cc: Junio C Hamano, git
In-Reply-To: <1188937975.6192.59.camel@beauty>

Josh England wrote:
> I'm thinking that it
> will be sufficient to pass a flag indicating whether the working tree
> has been modified or not.  The flag can be set for normal merge and
> fast-forward merges, and unset for up-to-dates and squash merges.
>   

Squash merges modify the working tree. In fact, that's *all* they do -- 
they don't commit anything.

-Steve

^ permalink raw reply

* Re: Calculating tree nodes
From: Martin Langhoff @ 2007-09-04 21:02 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Jon Smirl, Johannes Schindelin, Shawn O. Pearce, Git Mailing List
In-Reply-To: <46DD7660.2030109@op5.se>

On 9/5/07, Andreas Ericsson <ae@op5.se> wrote:
> Jon Smirl wrote:
> > The
> > essential part of the commit is the SHA1 list. The path names belong
> > to the file objects and should be stored there.
>
> Sorry, but it'll take code and benchmarks to convince me this is a
> good idea.

Same here. But I do want to note that adding the pathname to the blob
(and storing them on the combined SHA1 I assume) is broken, broken,
broken. It won't support any of the great semantics that git has
today.

Jon: I think you need to think carefully about the key operations like
diff and log on paths on projects like linux or mozilla. Even better,
craft a proof-of-concept patch that shows how that'd work, and why
it's faster/smaller.

cheers,



m

^ permalink raw reply

* Re: .gitignore, .gitattributes, .gitmodules, .gitprecious?, .gitacls? etc.
From: Jan Hudec @ 2007-09-04 21:03 UTC (permalink / raw)
  To: Sergio Callegari; +Cc: git
In-Reply-To: <loom.20070827T185519-641@post.gmane.org>

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

On Mon, Aug 27, 2007 at 17:07:34 +0000, Sergio Callegari wrote:
> David Kastrup <dak <at> gnu.org> writes:
> 
> > 
> > Sergio Callegari <scallegari <at> arces.unibo.it> writes:
> > 
> > > Couldn't all this directory/ownership/permission tracing be easily
> > > done by using hooks?  E.g. Having a pre-status and pre-commit hook
> > > one could fire up a program/script to collect all the extra info he
> > > wants to trace and store it somewhere (typically in some traced
> > > file).  The other way round one could have a post-checkout hook and
> > > he could arrange it to fire up some program to look into the
> > > extra-info file to set up all the meta-data he wants.
> > >
> > > This would be very flexible and would permit to manage absolutely
> > > /any/ kind of the metadata leaving absolute freedom about how to do
> > > so.
> > >
> > > Am I missing something here?
> > 
> > Merging.
> > 
> 
> Sorry, maybe I am really missing something, since merging does not look to me
> as an issue.
> 
> Why cannot git simply do the merging in the working tree as it normally
> does, including merging of the traced metadata file generated by the metadata
> helpers invoked via the hooks?
> Only, again more hooks are needed and likely a post-merge hook, so that at
> the end of the merge, the metadata can be applied.
> 
> Only, to have things going on smoothly, one should be so wise to assure that
> the metadata helpers save metadata as nice, sorted text files in order to
> minimize the burden of manual intervention if there are conflicts in
> metadata merging.

The post-checkout (no need for post-merge -- after in-index merge is done,
the files are checked out to worktree, so post-checkout would run anyway)
could actually apply any custom merge strategy required to avoid/clean up
spurious conflicts in the metadata file (eg. adding two files that go after
each other would be a textual conflict). The relevant versions are stored in
index stages at that point.

> BTW.  Having a post-checkout hook could also help getting rid of unwanted
> empty directories, couldn't it?

Probably not. I would imagine it would actually only run for the files being
checked out -- and there is nothing checked out in empty directories. (Well,
it would run once or once per directory with list of checked out files on
standard input).

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

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

^ permalink raw reply

* Re: [PATCH] Add post-merge hook.
From: Josh England @ 2007-09-04 21:23 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Junio C Hamano, git
In-Reply-To: <46DDC576.3080708@midwinter.com>

On Tue, 2007-09-04 at 13:52 -0700, Steven Grimm wrote:
> Josh England wrote:
> > I'm thinking that it
> > will be sufficient to pass a flag indicating whether the working tree
> > has been modified or not.  The flag can be set for normal merge and
> > fast-forward merges, and unset for up-to-dates and squash merges.
> >   
> Squash merges modify the working tree. In fact, that's *all* they do -- 
> they don't commit anything.

OK.  Looking at it closer, the post-merge hook should only run when
there has been a real merge (not run for up-to-dates), which seems
alright with me.  With the immediate data at hand I could pass in a flag
indicating a squash merge or not, or could simply pass in the current
HEAD.  I think the first scenario is more appropriate.

-JE

^ permalink raw reply

* Re: Git's database structure
From: Theodore Tso @ 2007-09-04 21:25 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <9e4733910709041044r71264346n341d178565dd0521@mail.gmail.com>

On Tue, Sep 04, 2007 at 01:44:47PM -0400, Jon Smirl wrote:
> The current data store design is not very flexible. Databases solved
> the flexibility problem long ago. I'm just wondering if we should
> steal some good ideas out of the database world and apply them to git.
> Ten years from now we may have 100GB git databases and really wish we
> had more flexible ways of querying them.

Databases solved the flexibility problem, at the cost of performance.
And if you use full normalized form in your database scheme, it costs
you even more in performance, because of all of the joins that you
need in order get the information you need to do, you know, useful
work as opposed to database wanking.

If you take a look at the really big databases with super high
performance requirements, say like those used to managed airline
tickets/reservation/fares, you will find that they are not normalized,
and they are not relational; they can't afford to be.  And if you take
a look at some of git competition that use relational databases to
store their SCM data, and take a look at how loooooong they they take
to do even basic operations, I would say that the onus is on you to
prove that normalization is actually a win in terms of real (not
theoretical) advantages, and that it doesn't cause performance to go
into the toilet.

I think the fundamental disconnect here is that no one is buying your
claim that just because the data design is "more flexible" that this
is automatically a good thing in and of itself, and we should even for
a moment, "put performance aside".  

I also don't think that attempting to force git's data structures into
database terms makes sense; it is much closer to an filesystem using
an object based store --- and very few people except for folks like
Hans Resiers believes that Filesystems and Database should be
unified....

						- Ted

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Rutger Nijlunsing @ 2007-09-04 21:02 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Johannes Sixt, Marius Storm-Olsen, Johannes Sixt,
	Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709041356070.28586@racer.site>

On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 4 Sep 2007, Johannes Schindelin wrote:
> 
> > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > 
> > > Johannes Schindelin schrieb:
> > > > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > > > > Therefore, I've pushed out a fixup patch at the top of mingw.git's 
> > > > > devel branch that converts mtime to local time
> > > > 
> > > > On Linux, we compare to UTC to begin with, right?  We should do that 
> > > > here, too...  So if time(NULL) does not return UTC on MinGW, we have 
> > > > to wrap that function, too.
> > > 
> > > According to MSDN, time(NULL) returns "the number of seconds elapsed 
> > > since [epoch] according to the system clock". Please don't ask me what 
> > > "the system clock" is.
> > 
> > I think I know.  From my QEmu adventures I know that DOS/Windows expects 
> > the system clock to be set to local time, in contrast to _all_ other 
> > operating systems.
> 
> Now I am utterly confused.  MSDN says
> 
> 	FILETIME
> 
> 	Contains a 64-bit value representing the number of 100-nanosecond 
> 	intervals since January 1, 1601 (UTC).
> 
> Hmm.


[Warning: war stories ahead...]

If you really, really want to know more:

http://search.cpan.org/~shay/Win32-UTCFileTime-1.45/lib/Win32/UTCFileTime.pm


-- 
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: David Kastrup @ 2007-09-04 21:31 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Marius Storm-Olsen, Johannes Schindelin, Johannes Sixt,
	Git Mailing List
In-Reply-To: <46DD3943.8040403@eudaptics.com>

Johannes Sixt <j.sixt@eudaptics.com> writes:

> Marius Storm-Olsen schrieb:
>
>> Ok, I can give it a performance test, but I tend to agree with
>> David Kastrup there. It would be better if we rather fix the places
>> where we check with the local timestamp instead; depending of
>> course on how many places we actually do this.  We'll see how much
>> the timezone conversion in the custom stat functions actually hurt
>> us performance wise.
>
> I'd make the decision on the grounds of a perfomance test. If it
> turns out that the penalty is bearable, we should keep this stuff
> private to the MinGW build. Otherwise, we would need MinGW specific
> code at the call sites (unless we can hide the opposite conversion
> in some other wrapper function).
>
> ... time passes ...
>
> Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I
> can run it 100,000,000 times per second. So I'm confident that there
> won't be any noticable degradation with my proposed change.

The problem is that it will break in the hour when day light saving
time ends.  Or when you travel between timezones.

Local time is not continuous or unique.  That is why it is a bad idea
to make decisions based on it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Alex Riesen @ 2007-09-04 21:38 UTC (permalink / raw)
  To: Lukas Sandström; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <46DDC500.5000606@etek.chalmers.se>

Lukas Sandström, Tue, Sep 04, 2007 22:50:08 +0200:
> Hi.
> 
> This is an attempt to use "The Better String Library"[1] in builtin-mailinfo.c
> 
> The patch doesn't pass all the tests in the testsuit yet, but I thought I'd
> send it out so people can decide if they like how the code looks.

It looks uglier, but what are measurable merits? Object code size,
perfomance hit/improvement, valgrind logs?

> -static int read_one_header_line(char *line, int sz, FILE *in)
> +static int read_one_header_line(struct bStream *in, bstring line)

Every coder has a time in his life when he writes a string library...
and a stream support for it.

^ permalink raw reply

* Re: Git's database structure
From: Jon Smirl @ 2007-09-04 21:54 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20070904212507.GA24434@thunk.org>

On 9/4/07, Theodore Tso <tytso@mit.edu> wrote:
> On Tue, Sep 04, 2007 at 01:44:47PM -0400, Jon Smirl wrote:
> > The current data store design is not very flexible. Databases solved
> > the flexibility problem long ago. I'm just wondering if we should
> > steal some good ideas out of the database world and apply them to git.
> > Ten years from now we may have 100GB git databases and really wish we
> > had more flexible ways of querying them.
>
> Databases solved the flexibility problem, at the cost of performance.
> And if you use full normalized form in your database scheme, it costs
> you even more in performance, because of all of the joins that you
> need in order get the information you need to do, you know, useful
> work as opposed to database wanking.
>
> If you take a look at the really big databases with super high
> performance requirements, say like those used to managed airline
> tickets/reservation/fares, you will find that they are not normalized,
> and they are not relational; they can't afford to be.  And if you take
> a look at some of git competition that use relational databases to
> store their SCM data, and take a look at how loooooong they they take
> to do even basic operations, I would say that the onus is on you to
> prove that normalization is actually a win in terms of real (not
> theoretical) advantages, and that it doesn't cause performance to go
> into the toilet.
>
> I think the fundamental disconnect here is that no one is buying your
> claim that just because the data design is "more flexible" that this
> is automatically a good thing in and of itself, and we should even for
> a moment, "put performance aside".

It is very easy to get bogged down in performance arguments on
database design when the correct answer is that there are always lots
of different ways to achieve the same goal. I wanted to defer debating
performance until we closely looked at the relationships between the
data at an abstract level.

Since git hasn't stored all of the fields in the object table (the
path is encoded in the index) we are never going to be able to build
an alternative way of indexing the object table. Not being able to
build alternative indexes is likely to cause problems when the
database starts getting really big. Without an index every query that
can't use the path name index is reduced to doing full table scans.

A few things that could benefit from alternative indexing, blame,
full-text search, automating the Maintainers file, etc.

I'm just asking if we really want to make full table scans the only
possible way to implement these types of queries. If the answer is no,
then let's first explore how to fix things at an abstract level before
diving into the performance arguments.

An obvious parallel from the file system world is the locate database
and how it is forced to continuously rescan the file system and store
full path names.


>
> I also don't think that attempting to force git's data structures into
> database terms makes sense; it is much closer to an filesystem using
> an object based store --- and very few people except for folks like
> Hans Resiers believes that Filesystems and Database should be
> unified....
>
>                                                 - Ted
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Reece Dunn @ 2007-09-04 21:54 UTC (permalink / raw)
  To: git, Johannes Schindelin, Johannes Sixt, Marius Storm-Olsen,
	"Johannes Sixt" <johan
In-Reply-To: <20070904210200.GA32472@nospam.com>

On 04/09/07, Rutger Nijlunsing <rutger@nospam.com> wrote:
> On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
> > Hi,
> >
> > On Tue, 4 Sep 2007, Johannes Schindelin wrote:
> >
> > > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > >
> > > > Johannes Schindelin schrieb:
> > > > > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > > > > > Therefore, I've pushed out a fixup patch at the top of mingw.git's
> > > > > > devel branch that converts mtime to local time
> > > > >
> > > > > On Linux, we compare to UTC to begin with, right?  We should do that
> > > > > here, too...  So if time(NULL) does not return UTC on MinGW, we have
> > > > > to wrap that function, too.
> > > >
> > > > According to MSDN, time(NULL) returns "the number of seconds elapsed
> > > > since [epoch] according to the system clock". Please don't ask me what
> > > > "the system clock" is.
> > >
> > > I think I know.  From my QEmu adventures I know that DOS/Windows expects
> > > the system clock to be set to local time, in contrast to _all_ other
> > > operating systems.
> >
> > Now I am utterly confused.  MSDN says
> >
> >       FILETIME
> >
> >       Contains a 64-bit value representing the number of 100-nanosecond
> >       intervals since January 1, 1601 (UTC).
> >
> > Hmm.
>
>
> [Warning: war stories ahead...]
>
> If you really, really want to know more:
>
> http://search.cpan.org/~shay/Win32-UTCFileTime-1.45/lib/Win32/UTCFileTime.pm

Hmm, this may explain something that I have been observing on
Windows+cygwin. When I run `git diff`, I sometimes get it reporting
that all (from what I can tell) files have changed, like a `find .
-type f -exec touch {} \;` command was run.

I was going to report this on a new thread, but this looks like a more
relevant place to do so.

My Windows machine is currently in Daylight Savings Time mode, and
from my observations, I have only seen this repeat first thing the
next day. I am not sure why, but every 24hrs, it looks as if the file
time reported by and checked from git is different to that reported by
stat.

I have not had time yet to play around with the mingw port and this
new stat implementation to see if it addresses this issue.

- Reece

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Pierre Habouzit @ 2007-09-04 23:01 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Lukas Sandström, Git Mailing List, Junio C Hamano
In-Reply-To: <20070904213857.GA21351@steel.home>

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

On mar, sep 04, 2007 at 09:38:57 +0000, Alex Riesen wrote:
> Lukas Sandström, Tue, Sep 04, 2007 22:50:08 +0200:
> > Hi.
> > 
> > This is an attempt to use "The Better String Library"[1] in builtin-mailinfo.c
> > 
> > The patch doesn't pass all the tests in the testsuit yet, but I thought I'd
> > send it out so people can decide if they like how the code looks.
> 
> It looks uglier, but what are measurable merits? Object code size,
> perfomance hit/improvement, valgrind logs?

  Well I honestly believe that putting strbufs/bstrings in mailinfo.c
adds no value. I was going to give it a try to see how strbufs
performed, but it's just useless.

  The main problem mailinfo has, it's according to Junio that it may
sometimes truncate some things in buffers at 1000 octets, without dying
loudly. That is bad.

  _but_ there is no point in using arbitrary long string buffers to
parse a mail. Remember, a mail goes through SMTP, and SMTP is supposed
to limit its lines at 512 characters (without use of extensions at
least). Not to mention that an email address cannot be more than 64+256
chars long (or sth around that). So using variable lengths buffers is
just a waste.

  string buffers are not really (IMHO) supposed to help in parsing
tasks, and when you need to do some serious parsing, either do it by
hand or use lex, but nothing in between makes sense to me.

  OTOH, string buffers can be used in many places where git has (at
least 4 different to my current count, growing) many implementations of
always slightly different kind of buffers. I've some more patches
pending here than the one I already sent, and well, here is the
diffstat:

$ git diff --stat origin/master.. ^strbuf*
 archive-tar.c         |   67 ++++++++++++------------------------------------
 builtin-apply.c       |   29 ++++++---------------
 builtin-blame.c       |   34 ++++++++-----------------
 builtin-commit-tree.c |   59 +++++++++---------------------------------
 builtin-rerere.c      |   53 +++++++++++---------------------------
 cache-tree.c          |   57 ++++++++++++++---------------------------
 diff.c                |   25 ++++++------------
 fast-import.c         |   38 +++++++++++----------------
 mktree.c              |   26 ++++++-------------
 9 files changed, 116 insertions(+), 272 deletions(-)

  I mean, there is not even a need to show the diff to understand what
the gain is. And that was possible, because strbufs are straightforward,
and gives you the kind of controls git needs (tweaking how memory will
be allocated to avoid reallocs is part of the answer).


  A French author once said: “Il semble que la perfection soit atteinte
non quand il n'y a plus rien à ajouter, mais quand il n'y a plus rien à
retrancher.” -- Antoine de St Éxupéry[0]. IMHO git will never need any
of the bstring splits, streaming functions, tokenization or whatever,
and supporting those has necessarily led the bstring library to make
some choices that may not fit git needs. I don't really like reinventing
the wheel, but OTOH buffers and strings are often of the critical path,
and having a nice fitting buffer API is priceless.


  [0] Perfection is achieved, not when there is nothing more to add, but
      when there is nothing left to take away.
-- 
·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: [PATCH 2/3] archive: specfile support (--pretty=format: in   archive files)
From: René Scharfe @ 2007-09-04 23:13 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Andreas Ericsson, Junio C Hamano, Git Mailing List,
	Michael Gernoth, Thomas Glanzmann
In-Reply-To: <Pine.LNX.4.64.0709041139140.28586@racer.site>

Johannes Schindelin schrieb:
> Hi,
> 
> On Tue, 4 Sep 2007, Andreas Ericsson wrote:
> 
>> Junio C Hamano wrote:
>>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>>>
>>>> The attribute is useful for creating auto-updating specfiles.  It is 
>>>> limited by the underlying function format_commit_message(), though. 
>>>> E.g. currently there is no placeholder for git-describe like output, 
>>>> and expanded specfiles can't contain NUL bytes.  That can be fixed 
>>>> in format_commit_message() later and will then benefit users of 
>>>> git-log, too.
>>> Interesting. I however wonder if "specfile" is a good name for this 
>>> attribute, although I admit I do not think of anything better offhand.
>> "releasefile", perhaps?
> 
> Maybe we should not so much name it by purpose, but by function.  How 
> about "substformat" for the attribute name, and replacing any 
> $Format:blablub$ inside those files with something a la 
> --pretty=format:blablub?

I like the $Format:...$ notation.  How about naming the attribute
"template", as that's what a thus marked file is?

René

^ permalink raw reply

* Re: [PATCH 2/3] archive: specfile support (--pretty=format: in archive files)
From: René Scharfe @ 2007-09-04 23:13 UTC (permalink / raw)
  To: David Kastrup
  Cc: Johannes Schindelin, Junio C Hamano, Git Mailing List,
	Michael Gernoth, Thomas Glanzmann
In-Reply-To: <85ps0zmrhs.fsf@lola.goethe.zz>

David Kastrup schrieb:
> I think a bit more layering would be helpful: when using git-svn, one
> would want to have things like $Id$ and $Date$ expanded, so maybe
> attribute specs like
> 
> somefile: expandmarkers="$Date: %aD$ $Id: ....$"
> 
> would be nice having.  In the case of git-svn, I would expect them to
> be generated from git-svn from the respective svn properties, so that
> the user is not bothered with figuring out the awful $Id$ and whatever
> strings.

Eek!  I'd rather shove this into a new layer below specfile/template
expansion, which would do something like that:

   s/\$Date:.*\$/$Format:$$Date: %aD$/

This way A) the complexity and ugliness affects only the users of these
fields, while $Format:...$ users are not adversely affected, and B) we
can first implement $Format:...$ handling and add the rest later.

In order to expand $Id$, git-archive would need to run some kind of
callback, right? :-/  Let's first see if such a thing is really needed.

René

^ permalink raw reply

* Re: [PATCH 2/3] archive: specfile support (--pretty=format: in archive files)
From: René Scharfe @ 2007-09-04 23:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Michael Gernoth, Thomas Glanzmann
In-Reply-To: <7vtzqb8fw2.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
>> Why did it take me that long to come up with such a simple patch?
>> There was a vacation and a feature freeze in between, but above all
>> I was only recently able to convince myself (using ugly code) that
>> format_commit_message() can indeed be made to expand placeholders
>> to git-describe strings..
> 
> Thanks.  Will take a look.

Here's that ugly code, by the way.  It adds two placeholders, %d for
description and %D for description depth.  Shortcomings of this code:
it adds three members to struct commit, it unconditionally computes
the description when format_commit_message() -- even if the format
string doesn't contain %d and %D, the patch is not nicely split up.
But it convinced me that describe *can* indeed be librarified. :-)

René


 Makefile           |    1 +
 builtin-describe.c |  219 +++------------------------------------------------
 cache.h            |    5 +
 commit.c           |   31 ++++++++
 commit.h           |    5 +
 describe.c         |  170 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 225 insertions(+), 206 deletions(-)

diff --git a/Makefile b/Makefile
index 51af531..7ec95f8 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,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 \
+	describe.o \
 	interpolate.o \
 	lockfile.o \
 	patch-ids.o \
diff --git a/builtin-describe.c b/builtin-describe.c
index 669110c..50ec08f 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -17,124 +17,13 @@ static int tags;	/* But allow any tags if --tags is specified */
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
 
-struct commit_name {
-	int prio; /* annotated tag = 2, tag = 1, head = 0 */
-	char path[FLEX_ARRAY]; /* more */
-};
-static const char *prio_names[] = {
-	"head", "lightweight", "annotated",
-};
-
-static void add_to_known_names(const char *path,
-			       struct commit *commit,
-			       int prio)
-{
-	struct commit_name *e = commit->util;
-	if (!e || e->prio < prio) {
-		size_t len = strlen(path)+1;
-		free(e);
-		e = xmalloc(sizeof(struct commit_name) + len);
-		e->prio = prio;
-		memcpy(e->path, path, len);
-		commit->util = e;
-	}
-}
-
-static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
-{
-	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
-	struct object *object;
-	int prio;
-
-	if (!commit)
-		return 0;
-	object = parse_object(sha1);
-	/* If --all, then any refs are used.
-	 * If --tags, then any tags are used.
-	 * Otherwise only annotated tags are used.
-	 */
-	if (!prefixcmp(path, "refs/tags/")) {
-		if (object->type == OBJ_TAG)
-			prio = 2;
-		else
-			prio = 1;
-	}
-	else
-		prio = 0;
-
-	if (!all) {
-		if (!prio)
-			return 0;
-		if (!tags && prio < 2)
-			return 0;
-	}
-	add_to_known_names(all ? path + 5 : path + 10, commit, prio);
-	return 0;
-}
-
-struct possible_tag {
-	struct commit_name *name;
-	int depth;
-	int found_order;
-	unsigned flag_within;
-};
-
-static int compare_pt(const void *a_, const void *b_)
-{
-	struct possible_tag *a = (struct possible_tag *)a_;
-	struct possible_tag *b = (struct possible_tag *)b_;
-	if (a->name->prio != b->name->prio)
-		return b->name->prio - a->name->prio;
-	if (a->depth != b->depth)
-		return a->depth - b->depth;
-	if (a->found_order != b->found_order)
-		return a->found_order - b->found_order;
-	return 0;
-}
-
-static unsigned long finish_depth_computation(
-	struct commit_list **list,
-	struct possible_tag *best)
-{
-	unsigned long seen_commits = 0;
-	while (*list) {
-		struct commit *c = pop_commit(list);
-		struct commit_list *parents = c->parents;
-		seen_commits++;
-		if (c->object.flags & best->flag_within) {
-			struct commit_list *a = *list;
-			while (a) {
-				struct commit *i = a->item;
-				if (!(i->object.flags & best->flag_within))
-					break;
-				a = a->next;
-			}
-			if (!a)
-				break;
-		} else
-			best->depth++;
-		while (parents) {
-			struct commit *p = parents->item;
-			parse_commit(p);
-			if (!(p->object.flags & SEEN))
-				insert_by_date(p, list);
-			p->object.flags |= c->object.flags;
-			parents = parents->next;
-		}
-	}
-	return seen_commits;
-}
-
 static void describe(const char *arg, int last_one)
 {
 	unsigned char sha1[20];
-	struct commit *cmit, *gave_up_on = NULL;
-	struct commit_list *list;
+	struct commit *cmit;
 	static int initialized = 0;
-	struct commit_name *n;
-	struct possible_tag all_matches[MAX_TAGS];
-	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
-	unsigned long seen_commits = 0;
+	char *name;
+	int depth = 0;
 
 	if (get_sha1(arg, sha1))
 		die("Not a valid object name %s", arg);
@@ -144,100 +33,23 @@ static void describe(const char *arg, int last_one)
 
 	if (!initialized) {
 		initialized = 1;
-		for_each_ref(get_name, NULL);
-	}
-
-	n = cmit->util;
-	if (n) {
-		printf("%s\n", n->path);
-		return;
+		load_commit_names(all ? 0 : (tags ? 1 : 2));
 	}
 
-	if (debug)
-		fprintf(stderr, "searching to describe %s\n", arg);
-
-	list = NULL;
-	cmit->object.flags = SEEN;
-	commit_list_insert(cmit, &list);
-	while (list) {
-		struct commit *c = pop_commit(&list);
-		struct commit_list *parents = c->parents;
-		seen_commits++;
-		n = c->util;
-		if (n) {
-			if (match_cnt < max_candidates) {
-				struct possible_tag *t = &all_matches[match_cnt++];
-				t->name = n;
-				t->depth = seen_commits - 1;
-				t->flag_within = 1u << match_cnt;
-				t->found_order = match_cnt;
-				c->object.flags |= t->flag_within;
-				if (n->prio == 2)
-					annotated_cnt++;
-			}
-			else {
-				gave_up_on = c;
-				break;
-			}
-		}
-		for (cur_match = 0; cur_match < match_cnt; cur_match++) {
-			struct possible_tag *t = &all_matches[cur_match];
-			if (!(c->object.flags & t->flag_within))
-				t->depth++;
-		}
-		if (annotated_cnt && !list) {
-			if (debug)
-				fprintf(stderr, "finished search at %s\n",
-					sha1_to_hex(c->object.sha1));
-			break;
-		}
-		while (parents) {
-			struct commit *p = parents->item;
-			parse_commit(p);
-			if (!(p->object.flags & SEEN))
-				insert_by_date(p, &list);
-			p->object.flags |= c->object.flags;
-			parents = parents->next;
-		}
-	}
-
-	if (!match_cnt)
+	name = describe_commit(cmit, max_candidates, &depth);
+	if (!name)
 		die("cannot describe '%s'", sha1_to_hex(cmit->object.sha1));
+	if (!all)
+		name += 5;
 
-	qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
-
-	if (gave_up_on) {
-		insert_by_date(gave_up_on, &list);
-		seen_commits--;
-	}
-	seen_commits += finish_depth_computation(&list, &all_matches[0]);
-	free_commit_list(list);
-
-	if (debug) {
-		for (cur_match = 0; cur_match < match_cnt; cur_match++) {
-			struct possible_tag *t = &all_matches[cur_match];
-			fprintf(stderr, " %-11s %8d %s\n",
-				prio_names[t->name->prio],
-				t->depth, t->name->path);
-		}
-		fprintf(stderr, "traversed %lu commits\n", seen_commits);
-		if (gave_up_on) {
-			fprintf(stderr,
-				"more than %i tags found; listed %i most recent\n"
-				"gave up search at %s\n",
-				max_candidates, max_candidates,
-				sha1_to_hex(gave_up_on->object.sha1));
-		}
-	}
-	if (abbrev == 0)
-		printf("%s\n", all_matches[0].name->path );
+	if (abbrev == 0 || depth == 0)
+		printf("%s\n", name);
 	else
-		printf("%s-%d-g%s\n", all_matches[0].name->path,
-		       all_matches[0].depth,
+		printf("%s-%d-g%s\n", name, depth,
 		       find_unique_abbrev(cmit->object.sha1, abbrev));
 
 	if (!last_one)
-		clear_commit_marks(cmit, -1);
+		clear_commit_name_flags(cmit);
 }
 
 int cmd_describe(int argc, const char **argv, const char *prefix)
@@ -263,13 +75,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			if (abbrev != 0 && (abbrev < MINIMUM_ABBREV || 40 < abbrev))
 				abbrev = DEFAULT_ABBREV;
 		}
-		else if (!prefixcmp(arg, "--candidates=")) {
+		else if (!prefixcmp(arg, "--candidates="))
 			max_candidates = strtoul(arg + 13, NULL, 10);
-			if (max_candidates < 1)
-				max_candidates = 1;
-			else if (max_candidates > MAX_TAGS)
-				max_candidates = MAX_TAGS;
-		}
 		else
 			usage(describe_usage);
 	}
diff --git a/cache.h b/cache.h
index 70abbd5..07ee149 100644
--- a/cache.h
+++ b/cache.h
@@ -600,4 +600,9 @@ extern int diff_auto_refresh_index;
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
 
+/* describe.c */
+struct commit;
+extern void load_commit_names(int min_prio);
+extern char *describe_commit(struct commit *cmit, int max_candidates, int *depthp);
+
 #endif /* CACHE_H */
diff --git a/commit.c b/commit.c
index dc5a064..d2343f2 100644
--- a/commit.c
+++ b/commit.c
@@ -455,6 +455,19 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
 	}
 }
 
+void clear_commit_name_flags(struct commit *commit)
+{
+	struct commit_list *parents;
+
+	commit->name_flags = 0;
+
+	for (parents = commit->parents; parents; parents = parents->next) {
+		/* Have we already cleared this? */
+		if (parents->item->name_flags)
+			clear_commit_name_flags(parents->item);
+	}
+}
+
 /*
  * Generic support for pretty-printing the header
  */
@@ -819,6 +832,8 @@ static long format_commit_message(const struct commit *commit,
 		{ "%Cblue" },	/* blue */
 		{ "%Creset" },	/* reset color */
 		{ "%n" },	/* newline */
+		{ "%d" },	/* description */
+		{ "%D" },	/* description depth */
 		{ "%m" },	/* left/right/bottom */
 	};
 	enum interp_index {
@@ -837,6 +852,7 @@ static long format_commit_message(const struct commit *commit,
 		IBODY,
 		IRED, IGREEN, IBLUE, IRESET_COLOR,
 		INEWLINE,
+		IDESC, IDESC_DEPTH,
 		ILEFT_RIGHT,
 	};
 	struct commit_list *p;
@@ -920,6 +936,21 @@ static long format_commit_message(const struct commit *commit,
 		if (!table[i].value)
 			interp_set_entry(table, i, "<unknown>");
 
+	{
+	struct commit *cmit = (struct commit *)commit;
+	char *name;
+	char tmp[20];
+	int depth;
+	load_commit_names(2);
+	name = describe_commit(cmit, 10, &depth);
+	if (!name)
+		name = "";
+	sprintf(tmp, "%d", depth);
+	interp_set_entry(table, IDESC, name);
+	interp_set_entry(table, IDESC_DEPTH, tmp);
+	clear_commit_name_flags(cmit);
+	}
+
 	do {
 		char *buf = *buf_p;
 		unsigned long space = *space_p;
diff --git a/commit.h b/commit.h
index 467872e..69b6d67 100644
--- a/commit.h
+++ b/commit.h
@@ -17,6 +17,9 @@ struct commit {
 	struct commit_list *parents;
 	struct tree *tree;
 	char *buffer;
+	char *name;
+	unsigned int name_flags;
+	char name_prio;
 };
 
 extern int save_commit_buffer;
@@ -72,6 +75,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
 struct commit *pop_commit(struct commit_list **stack);
 
 void clear_commit_marks(struct commit *commit, unsigned int mark);
+void clear_commit_name_flags(struct commit *commit);
+
 
 /*
  * Performs an in-place topological sort of list supplied.
diff --git a/describe.c b/describe.c
new file mode 100644
index 0000000..acd4517
--- /dev/null
+++ b/describe.c
@@ -0,0 +1,170 @@
+#include "cache.h"
+#include "commit.h"
+#include "tag.h"
+#include "refs.h"
+
+#define SEEN		(1u<<0)
+#define MAX_TAGS	(FLAG_BITS - 1)
+
+struct possible_tag {
+	char *name;
+	int name_prio;
+	int depth;
+	int found_order;
+	unsigned flag_within;
+};
+
+static int get_name(const char *path, const unsigned char *sha1, int flag,
+                    void *cb_data)
+{
+	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit) {
+		struct object *object = parse_object(sha1);
+		int min_prio = *((int *)cb_data);
+		int prio = 0;
+
+		if (!prefixcmp(path, "refs/tags/")) {
+			if (object->type == OBJ_TAG)
+				prio = 2;
+			else
+				prio = 1;
+		}
+
+		if (prio >= min_prio &&
+		    (!commit->name || commit->name_prio < prio)) {
+			free(commit->name);
+			commit->name = xstrdup(path + 5);
+			commit->name_prio = prio;
+		}
+	}
+	return 0;
+}
+
+void load_commit_names(int min_prio)
+{
+	for_each_ref(get_name, &min_prio);
+}
+
+static int compare_pt(const void *a_, const void *b_)
+{
+	struct possible_tag *a = (struct possible_tag *)a_;
+	struct possible_tag *b = (struct possible_tag *)b_;
+	if (a->name_prio != b->name_prio)
+		return b->name_prio - a->name_prio;
+	if (a->depth != b->depth)
+		return a->depth - b->depth;
+	if (a->found_order != b->found_order)
+		return a->found_order - b->found_order;
+	return 0;
+}
+
+static unsigned long finish_depth_computation(
+	struct commit_list **list,
+	struct possible_tag *best)
+{
+	unsigned long seen_commits = 0;
+	while (*list) {
+		struct commit *c = pop_commit(list);
+		struct commit_list *parents = c->parents;
+		seen_commits++;
+		if (c->name_flags & best->flag_within) {
+			struct commit_list *a = *list;
+			while (a) {
+				struct commit *i = a->item;
+				if (!(i->name_flags & best->flag_within))
+					break;
+				a = a->next;
+			}
+			if (!a)
+				break;
+		} else
+			best->depth++;
+		while (parents) {
+			struct commit *p = parents->item;
+			parse_commit(p);
+			if (!(p->name_flags & SEEN))
+				insert_by_date(p, list);
+			p->name_flags |= c->name_flags;
+			parents = parents->next;
+		}
+	}
+	return seen_commits;
+}
+
+char *describe_commit(struct commit *cmit, int max_candidates, int *depthp)
+{
+	struct commit *gave_up_on = NULL;
+	struct commit_list *list;
+	struct possible_tag all_matches[MAX_TAGS];
+	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
+	unsigned long seen_commits = 0;
+
+	if (cmit->name) {
+		*depthp = 0;
+		return cmit->name;
+	}
+
+	if (max_candidates < 1)
+		max_candidates = 1;
+	else if (max_candidates > MAX_TAGS)
+		max_candidates = MAX_TAGS;
+
+	list = NULL;
+	cmit->name_flags = SEEN;
+	commit_list_insert(cmit, &list);
+	while (list) {
+		struct commit *c = pop_commit(&list);
+		struct commit_list *parents = c->parents;
+		seen_commits++;
+		if (c->name) {
+			if (match_cnt < max_candidates) {
+				struct possible_tag *t = &all_matches[match_cnt++];
+				t->name = c->name;
+				t->name_prio = c->name_prio;
+				t->depth = seen_commits - 1;
+				t->flag_within = 1u << match_cnt;
+				t->found_order = match_cnt;
+				c->name_flags |= t->flag_within;
+				if (c->name_prio == 2)
+					annotated_cnt++;
+			}
+			else {
+				gave_up_on = c;
+				break;
+			}
+		}
+		for (cur_match = 0; cur_match < match_cnt; cur_match++) {
+			struct possible_tag *t = &all_matches[cur_match];
+			if (!(c->name_flags & t->flag_within))
+				t->depth++;
+		}
+		if (annotated_cnt && !list)
+			break;
+		while (parents) {
+			struct commit *p = parents->item;
+			parse_commit(p);
+			if (!(p->name_flags & SEEN))
+				insert_by_date(p, &list);
+			p->name_flags |= c->name_flags;
+			parents = parents->next;
+		}
+	}
+
+	if (!match_cnt) {
+		free_commit_list(list);
+		*depthp = -1;
+		return NULL;
+	}
+
+	qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
+
+	if (gave_up_on) {
+		insert_by_date(gave_up_on, &list);
+		seen_commits--;
+	}
+	seen_commits += finish_depth_computation(&list, &all_matches[0]);
+	free_commit_list(list);
+
+	*depthp = all_matches[0].depth;
+	return all_matches[0].name;
+}

^ permalink raw reply related

* Re: [PATCH] Functions for updating refs.
From: Carlos Rica @ 2007-09-04 23:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7v642qnwr7.fsf@gitster.siamese.dyndns.org>

2007/9/4, Junio C Hamano <gitster@pobox.com>:
> Carlos Rica <jasampler@gmail.com> writes:
>
> > diff --git a/refs.c b/refs.c
> > index 09a2c87..4fd5065 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -1455,3 +1455,35 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
> >  {
> >       return do_for_each_reflog("", fn, cb_data);
> >  }
> > +
> > +int update_ref_or_die(const char *action, const char *refname,
> > +                             const unsigned char *sha1,
> > +                             const unsigned char *oldval, int flags)
> > +{
> > +     static struct ref_lock *lock;
> > +     lock = lock_any_ref_for_update(refname, oldval, flags);
> > +     if (!lock)
> > +             die("Cannot lock the ref '%s'.", refname);
> > +     if (write_ref_sha1(lock, sha1, action) < 0)
> > +             die("Cannot update the ref '%s'.", refname);
> > +     return 0;
> > +}
> > +
> > +int update_ref_or_error(const char *action, const char *refname,
> > +                             const unsigned char *sha1,
> > +                             const unsigned char *oldval, int quiet)
> > +{
> > +     static struct ref_lock *lock;
> > +     lock = lock_any_ref_for_update(refname, oldval, 0);
> > +     if (!lock) {
> > +             if (!quiet)
> > +                     error("Cannot lock the ref '%s'.", refname);
> > +             return 1;
> > +     }
> > +     if (write_ref_sha1(lock, sha1, action) < 0) {
> > +             if (!quiet)
> > +                     error("Cannot update the ref '%s'.", refname);
> > +             return 1;
> > +     }
> > +     return 0;
> > +}
>
> This makes me wonder three things:
>
>  - Why doesn't "or_error" side allow "flags" as "or_die" one?
>    Could the 'quiet' option become part of "flags" perhaps?

I saw that the only code that needed the flags was the
builtin-update-ref.c, and it also needed to die(). The
others I saw only want that parameter set to 0.
builtin-tag.c was doing die() also, not using flags, though.

>  - They look quite similar.  Is it a good idea to refactor them
>    further, or they are so small it does not matter?

I would like to know how to refactor it, however the code I saw
sometimes needs to call die(), others to error() and others
need to get only a value of success or not.

The function die() returns 128 and terminates the program,
prepending "fatal: " in the message, while error() doesn't exit
and prepend "error: ", so they were very different and I
resolved to separate them.

Another option is returning different error codes, so
the caller could decide what to output in each case, but
I thought that these functions were only useful to unify those
instructions with those error messages for this common
operation that many builtins use, specially when they come
from scripts who call to "git update-ref".

>
>  - Why isn't lock released with unlock_ref()?

I inspected this some weeks ago, and I finally came to think
that it is released in the write_ref_sha1 call after the lock.
But the code was complex, can someone confirm this?

^ permalink raw reply

* Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.
From: René Scharfe @ 2007-09-04 23:46 UTC (permalink / raw)
  To: git; +Cc: git
In-Reply-To: <11889144743483-git-send-email-madcoder@debian.org>

Pierre Habouzit schrieb:
>   This is just cleaner way to deal with strbufs, using its API rather than
> reinventing it in the module (e.g. strbuf_append_string is just the plain
> strbuf_addstr function, and it was used to perform what strbuf_addch does
> anyways).
> ---
>  archive-tar.c |   65 ++++++++++++++-------------------------------------------
>  1 files changed, 16 insertions(+), 49 deletions(-)

Apart from the wrong subject I really like this patch. :-D

Thanks!
René

^ permalink raw reply

* Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.
From: René Scharfe @ 2007-09-04 23:46 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <11889144743483-git-send-email-madcoder@debian.org>

Pierre Habouzit schrieb:
>   This is just cleaner way to deal with strbufs, using its API rather than
> reinventing it in the module (e.g. strbuf_append_string is just the plain
> strbuf_addstr function, and it was used to perform what strbuf_addch does
> anyways).
> ---
>  archive-tar.c |   65 ++++++++++++++-------------------------------------------
>  1 files changed, 16 insertions(+), 49 deletions(-)

Apart from the wrong subject line I really like this patch. :-D

Thanks!
René

^ permalink raw reply

* Re: git-svn and a nested branches folder
From: Eric Wong @ 2007-09-05  0:15 UTC (permalink / raw)
  To: Russ Brown; +Cc: git
In-Reply-To: <46DD6EEA.9010304@gmail.com>

Russ Brown <pickscrape@gmail.com> wrote:
> Hi,

Hi Russ,

> I'm having some trouble with using git-svn to fetch a repository, and I
> think it's because the repository doesn't store branches as a flat list
> directly under the 'branches' directory.
> 
> Basically, we have a structure like this:
> 
> |
> +-trunk
> +-tags
> +-branches
>   + category-a
>     + branch-a
>     + branch-b
>   + category-b
>     + branch-c
>     + branch-d
> 
> etc. category-a and category-b are simple directories created using svn
> mkdir. The branches are created using svn cp.
> 
> It helps us to organise the branches better, but the rationale is
> besides the point. The problem is that git-svn seems to want to treat
> category-a and category-b as branches, which isn't right at all. As a
> result, git-svn seems to skip most (if not all) revisions that occur in
> these directories and creates a lot of entries in unhandled.log.

This is a known problem with the way git-svn handles branches and tags.
Nested branch (and tags) structures aren't supported with globbing and
so you can't have more than one branches/tags specification in your
config.

> I've also encountered an index corruption in one of the
> .git/svn/<branch>/index files which I think it probably related.

Not sure about that.  git-svn should detect and attempt to fix index
corruptions (which happened for me since I interrupt git-svn quite
often when working on it).

> I've had a quick look at the source myself, but perl isn't my strong
> point. What I think it should do is something like recurse down the tree
> from the directory given looking for folders that copy from trunk (or
> some other branch that already exists). That would work perfectly well
> for the default flat branch storage method as well as the one we use.

The globbing functionality of branches in git-svn is some of the ugliest
code I've ever written in my life.  Somehow I got it working for the
simple general cases but I've been afraid to touch it for a while...

> The only other problem is in branch naming, which could clash if you
> only use the outer-most directory name, so I'd suggest something that
> involves concatenating the folders in the path relative to 'branches' to
> keep them unique (if git can handle slashes in branch names then all the
> better).

As Peter suggested, disable globbing for branches and use explicit
fetch refspecs for now...

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] Functions for updating refs.
From: Junio C Hamano @ 2007-09-05  0:16 UTC (permalink / raw)
  To: Carlos Rica; +Cc: git, Johannes Schindelin
In-Reply-To: <1b46aba20709041632x60ee4d3eweae9f5217d2f3b86@mail.gmail.com>

"Carlos Rica" <jasampler@gmail.com> writes:
>>  - Why doesn't "or_error" side allow "flags" as "or_die" one?
>>    Could the 'quiet' option become part of "flags" perhaps?
>
> I saw that the only code that needed the flags was the
> builtin-update-ref.c, and it also needed to die(). The
> others I saw only want that parameter set to 0.
> builtin-tag.c was doing die() also, not using flags, though.

Ok, when other built-ins start using these functions, they might
want to pass different flags, but it is easy enough for us to
extend the interface later.

>>  - They look quite similar.  Is it a good idea to refactor them
>>    further, or they are so small it does not matter?
>
> The function die() returns 128 and terminates the program,
> prepending "fatal: " in the message, while error() doesn't exit
> and prepend "error: ", so they were very different and I
> resolved to separate them.

Fair enough.

>>  - Why isn't lock released with unlock_ref()?
>
> I inspected this some weeks ago, and I finally came to think
> that it is released in the write_ref_sha1 call after the lock.

Ah, that's right!

^ permalink raw reply

* Re: [PATCH 2/3] archive: specfile support (--pretty=format: in archive files)
From: Junio C Hamano @ 2007-09-05  0:19 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List, Michael Gernoth, Thomas Glanzmann
In-Reply-To: <46DDE6B3.8010507@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Here's that ugly code, by the way.  It adds two placeholders, %d for
> description and %D for description depth.  Shortcomings of this code:
> it adds three members to struct commit, it unconditionally computes
> the description when format_commit_message() -- even if the format
> string doesn't contain %d and %D, the patch is not nicely split up.
> But it convinced me that describe *can* indeed be librarified. :-)

Yeah, unconditional computation for %d/%D is really a killer.
Otherwise the change itself does not necessarily look too ugly,
though.

^ permalink raw reply

* Re: [PATCH 2/3] archive: specfile support (--pretty=format: in   archive files)
From: Junio C Hamano @ 2007-09-05  0:23 UTC (permalink / raw)
  To: René Scharfe
  Cc: Johannes Schindelin, Andreas Ericsson, Git Mailing List,
	Michael Gernoth, Thomas Glanzmann
In-Reply-To: <46DDE69C.1080908@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

>> Maybe we should not so much name it by purpose, but by function.  How 
>> about "substformat" for the attribute name, and replacing any 
>> $Format:blablub$ inside those files with something a la 
>> --pretty=format:blablub?
>
> I like the $Format:...$ notation.  How about naming the attribute
> "template", as that's what a thus marked file is?

Sounds good, although I suspect "template" might confuse newbies
that checkout may apply the substitution as well.  How about
something with "export" in it?  export-subst, perhaps?

^ 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