Git development
 help / color / mirror / Atom feed
* Re: [patch] git: fix memory leak #2 in read-cache.c
From: Martin Schlemmer @ 2005-04-14 14:25 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: git, Linus Torvalds
In-Reply-To: <20050414132550.GA25496@elte.hu>

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

On Thu, 2005-04-14 at 15:25 +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > this patch fixes a memory leak in read-cache.c: when there's cache 
> > entry collision we should free the previous one.
> 
> > +		free(active_cache[pos]);
> >  		active_cache[pos] = ce;
> 
> i'm having second thoughs about this one: active_cache entries are not 
> always malloc()-ed - e.g. read_cache() will construct them from the 
> mmap() of the index file. Which must not be free()d!
> 
> one safe solution would be to malloc() all these entries and copy them 
> over from the index file? Slightly slower but safer and free()-able when 
> update-cache.c notices a collision. The (tested) patch below does this.
> 
> this would also make Martin Schlemmer's update-cache.c fix safe.
> 

Ok, bad me it seems - assumed it was malloc'd.


-- 
Martin Schlemmer


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [patch] git: fix memory leak #2 in read-cache.c
From: Linus Torvalds @ 2005-04-14 15:11 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: git
In-Reply-To: <20050414123934.GA15420@elte.hu>



On Thu, 14 Apr 2005, Ingo Molnar wrote:
> 
> this patch fixes a memory leak in read-cache.c: when there's cache entry 
> collision we should free the previous one.

As you already noticed "read_cache()" normally just populates the
active-cache with pointers to the mmap'ed "active" file.

Whether that is a good idea or not, I do not know. But I do know that the 
active file is the single biggest file we work with (ie 1.6MB in size), so 
since _most_ tools just read it and modify a very small number of entries, 
it seemed like a good idea.

In other words, if the common case is that we update a couple of entries
in the active cache, we actually saved 1.6MB (+ malloc overhead for the 17
_thousand_ allocations) by my approach.

And the leak? There's none. We never actually update an existing entry 
that was allocated with malloc(), unless the user does something stupid. 
In other words, the only case where there is a "leak" is when the user 
does something like

	update-cache file file file file file file .. 

with the same file listed several times.

And dammit, the whole point of doing stuff in user space is that the 
kernel takes care of business. Unlike kernel work, leaking is ok. You just 
have to make sure that it is limited enough to to not be a problem. I'm 
saying that in this case we're _better_ off leaking, because the mmap() 
trick saves us more memory than the leak can ever leak.

(The command line is limited to 128kB or so, which means that the most 
files you _can_ add with a single update-cache is _less_ than the mmap 
win).

It was _such_ a relief to program in user mode for a change. Not having to 
care about the small stuff is wonderful.

		Linus

^ permalink raw reply

* Re: [patch] git: fix memory leak #2 in read-cache.c
From: Ingo Molnar @ 2005-04-14 15:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504140804480.7211@ppc970.osdl.org>


* Linus Torvalds <torvalds@osdl.org> wrote:

> In other words, if the common case is that we update a couple of 
> entries in the active cache, we actually saved 1.6MB (+ malloc 
> overhead for the 17 _thousand_ allocations) by my approach.
> 
> And the leak? There's none. We never actually update an existing entry 
> that was allocated with malloc(), unless the user does something 
> stupid.  In other words, the only case where there is a "leak" is when 
> the user does something like
> 
> 	update-cache file file file file file file .. 
> 
> with the same file listed several times.

fair enough - as long as this is only used in a scripted environment, 
and not via some library and not within a repository server, web 
backend, etc.

	Ingo

^ permalink raw reply

* Re: [PATCH] Reorganize common code
From: Daniel Barkalow @ 2005-04-14 16:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <Pine.LNX.4.21.0504131755550.30848-100000@iabervon.org>

I forgot to include util.c. On my to-do list is something to make this
more obvious.

Signed-Off-By: Daniel Barkalow <barkalow@iabervon.org>

Index: util.c
===================================================================
--- /dev/null  (tree:e8194c62bfc68725972a6847fa2c6d529ca64137)
+++ 420a52c504bf712cd898a11746a54ea4349a9386/util.c  (mode:100644 sha1:d1d497c6709503f71138e816b2599d5105cc4915)
@@ -0,0 +1,37 @@
+#include "util.h"
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+void usage(const char *err)
+{
+	fprintf(stderr, "usage: %s\n", err);
+	exit(1);
+}
+
+static void report(const char *prefix, const char *err, va_list params)
+{
+	fputs(prefix, stderr);
+	vfprintf(stderr, err, params);
+	fputs("\n", stderr);
+}
+
+void die(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	report("fatal: ", err, params);
+	va_end(params);
+	exit(1);
+}
+
+int error(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	report("error: ", err, params);
+	va_end(params);
+	return -1;
+}


^ permalink raw reply

* Re: Yet another base64 patch
From: H. Peter Anvin @ 2005-04-14 16:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Christopher Li, git
In-Reply-To: <Pine.LNX.4.58.0504140038450.7211@ppc970.osdl.org>

Linus Torvalds wrote:
> 
> I'll tell you why a flat object directory format simply isn't an option.
> 
> Hint: maximum directory size. It's limited by n_link, and it's almost
> universally a 16-bit number on Linux (and generally artifically limited to
> 32000 entries).
> 
> In other words, if you ever expect to have more than 32000 objects, a flat 
> space simply isn't possible.
> 

Eh?!  n_link limits the number of *subdirectories* a directory can 
contain, not the number of *entries*.

	-hpa

	

^ permalink raw reply

* Re: Yet another base64 patch
From: H. Peter Anvin @ 2005-04-14 17:02 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504140114260.7211@ppc970.osdl.org>

Linus Torvalds wrote:
> 
> So why is "base64" worse than the stock one?
> 
> As mentioned, the "flat" version may be faster, but it really isn't an
> option. 32000 objects is peanuts. Any respectable source tree may hit that
> in a short time, and will break in horrible ways on many Linux
> filesystems.
> 

If it does, it's not because of n_link; see previous email.

I have used ext2 filesystems with hundreds of thousands of files per 
directory back in 1996.  It was slow but didn't break anything.

The only filesystem I know of which has a 2^16 entry limit is FAT.

> So you need at least a single level of subdirectory. 
> 
> What I don't get is why the stock hex version would be better than base64.
> 
> I like the result, I just don't _understand_ it.

The base64 version has 2^12 subdirectories instead of 2^8 (I just used 2 
characters as the hash key just like the hex version.)  So it ascerbates 
the performance penalty of subdirectory hashing.

	-hpa

^ permalink raw reply

* Re: [PATCH] merge-base
From: Daniel Barkalow @ 2005-04-14 17:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504132040020.7211@ppc970.osdl.org>

On Wed, 13 Apr 2005, Linus Torvalds wrote:

> I agree. But I did the silly "common revision tracking" part slightly
> differently and in particular I already made fsck and rev-tree use the 
> same exact code.

I think I only saw a cut-and-paste version, and I didn't want to follow
that pattern.

> Also, I don't see why you did the "common parent" thing as part of the
> "library", since that really does seem to be a very specific to this 
> problem, and neither fsck nor rev-tree really wants it. 

That was just silly; on the other hand, I think I'll eventually want to
have a "support-for-merging" library file, so that people can write
alternative merging programs which call library functions to pick out
history details. But that obviously shouldn't be the same file that
rev-tree and fsck share, and I'll probably do that by pulling main() out
of the program later.

> Also, the date parsing really is a separate issue from the revision 
> tracking (fsck does not want date parsing, but rev-tool does), so I think 
> you might want to do for date parsing what I just did for the revision.h 
> thing? No point in tying them together.

I think there is some value in having a library file that completely
parses "commit"-tagged files. Having the date field in struct revision
without the code to parse it in the file that defines the struct seems
poor to me.

> So could I ask you to re-factor it and base it on my current tree? Make 
> the "merge-base" program have that common parent thing in it, and factor 
> out the common date parsing into "parse-date.c" or something?

I'm not actually using the date in merge-base, either, so I'll just leave
that alone for now (merge-base is based on generations, not time,
currently).

	-Daniel
*This .sig left intentionally blank*


^ permalink raw reply

* [PATCH] merge-base-rebased
From: Daniel Barkalow @ 2005-04-14 17:20 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.21.0504141234490.30848-100000@iabervon.org>

Add a dumb-but-improvable program to find a not-too-bad commit to do a
three-way-merge of two commits.

Signed-Off-By: Daniel Barkalow <barkalow@iabervon.org>

Index: Makefile
===================================================================
--- 1756b578489f93999ded68ae347bef7d6063101c/Makefile  (mode:100644 sha1:b39b4ea37586693dd707d1d0750a9b580350ec50)
+++ b07fe1a7e0836e7d906c16c4f63b2e42b4a5193a/Makefile  (mode:100644 sha1:a2d920a36c1124fb0a031ae43fdd3a8106529c8f)
@@ -14,7 +14,7 @@
 
 PROG=   update-cache show-diff init-db write-tree read-tree commit-tree \
 	cat-file fsck-cache checkout-cache diff-tree rev-tree show-files \
-	check-files ls-tree merge-tree
+	check-files ls-tree merge-tree merge-base
 
 all: $(PROG)
 
@@ -67,6 +67,9 @@
 merge-tree: merge-tree.o read-cache.o
 	$(CC) $(CFLAGS) -o merge-tree merge-tree.o read-cache.o $(LIBS)
 
+merge-base: merge-base.o read-cache.o
+	$(CC) $(CFLAGS) -o merge-base merge-base.o read-cache.o $(LIBS)
+
 read-cache.o: cache.h
 show-diff.o: cache.h
 
Index: merge-base.c
===================================================================
--- /dev/null  (tree:1756b578489f93999ded68ae347bef7d6063101c)
+++ b07fe1a7e0836e7d906c16c4f63b2e42b4a5193a/merge-base.c  (mode:100644 sha1:a3e885ef8e2ae761d76c3217002467d19bf75a8d)
@@ -0,0 +1,132 @@
+#include <stdlib.h>
+#include "cache.h"
+#include "revision.h"
+
+static int parse_commit(struct revision *rev)
+{
+	if (!(rev->flags & SEEN)) {
+		void *buffer, *bufptr;
+		unsigned long size;
+		char type[20];
+		unsigned char parent[20];
+
+		rev->flags |= SEEN;
+		buffer = bufptr = read_sha1_file(rev->sha1, type, &size);
+		if (!buffer || strcmp(type, "commit"))
+			return -1;
+		bufptr += 46; /* "tree " + "hex sha1" + "\n" */
+		while (!memcmp(bufptr, "parent ", 7) && 
+		       !get_sha1_hex(bufptr+7, parent)) {
+			add_relationship(rev, parent);
+			bufptr += 48;   /* "parent " + "hex sha1" + "\n" */
+		}
+		//rev->date = parse_commit_date(bufptr);
+		free(buffer);
+	}
+	return 0;
+}
+
+struct revision *common_ancestor(struct revision *rev1, struct revision *rev2)
+{
+	struct parent *parent;
+
+	struct parent *rev1list = malloc(sizeof(struct parent));
+	struct parent *rev2list = malloc(sizeof(struct parent));
+        
+	struct parent *posn, *temp;
+
+	rev1list->parent = rev1;
+	rev1list->next = NULL;
+
+	rev2list->parent = rev2;
+	rev2list->next = NULL;
+
+	while (rev1list || rev2list) {
+		posn = rev1list;
+		rev1list = NULL;
+		while (posn) {
+			parse_commit(posn->parent);
+			if (posn->parent->flags & 0x0001) {
+				/*
+				printf("1 already seen %s %x\n",
+				       sha1_to_hex(posn->parent->sha1),
+				       posn->parent->flags);
+				*/
+                                // do nothing
+			} else if (posn->parent->flags & 0x0002) {
+                                // XXXX free lists
+				return posn->parent;
+			} else {
+				/*
+				printf("1 based on %s\n",
+				       sha1_to_hex(posn->parent->sha1));
+				*/
+				posn->parent->flags |= 0x0001;
+
+				parent = posn->parent->parent;
+				while (parent) {
+					temp = malloc(sizeof(struct parent));
+					temp->next = rev1list;
+					temp->parent = parent->parent;
+					rev1list = temp;
+					parent = parent->next;
+				}
+			}
+			posn = posn->next;
+		}
+		posn = rev2list;
+		rev2list = NULL;
+		while (posn) {
+			parse_commit(posn->parent);
+			if (posn->parent->flags & 0x0002) {
+				/*
+				printf("2 already seen %s\n",
+				       sha1_to_hex(posn->parent->sha1));
+				*/
+                                // do nothing
+			} else if (posn->parent->flags & 0x0001) {
+                                // XXXX free lists
+				return posn->parent;
+			} else {
+				/*
+				printf("2 based on %s\n",
+				       sha1_to_hex(posn->parent->sha1));
+				*/
+				posn->parent->flags |= 0x0002;
+
+				parent = posn->parent->parent;
+				while (parent) {
+					temp = malloc(sizeof(struct parent));
+					temp->next = rev2list;
+					temp->parent = parent->parent;
+					rev2list = temp;
+					parent = parent->next;
+				}
+			}
+			posn = posn->next;
+		}
+	}
+	return NULL;
+}
+
+int main(int argc, char **argv)
+{
+	struct revision *rev1, *rev2, *ret;
+	unsigned char rev1key[20], rev2key[20];
+	if (argc != 3 ||
+	    get_sha1_hex(argv[1], rev1key) ||
+	    get_sha1_hex(argv[2], rev2key)) {
+		usage("mergebase <commit-id> <commit-id>");
+	}
+	rev1 = lookup_rev(rev1key);
+	rev2 = lookup_rev(rev2key);
+	ret = common_ancestor(rev1, rev2);
+	if (ret) {
+		printf("%s\n", sha1_to_hex(ret->sha1));
+		return 0;
+	} else {
+		printf("Sorry.\n");
+		return 1;
+	}
+	
+}


^ permalink raw reply

* Re: Date handling.
From: David Woodhouse @ 2005-04-14 17:38 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504140212100.7211@ppc970.osdl.org>

On Thu, 2005-04-14 at 02:12 -0700, Linus Torvalds wrote:
> I take that back. I'd be much happier with you doing and testing it, 
> because now I'm crashing.

OK. commit-tree now eats RFC2822 dates as AUTHOR_DATE because that's
what you're going to want to feed it. We store seconds since UTC epoch,
we add the author's or committer's timezone as auxiliary data so that
dates can be pretty-printed in the original timezone later if anyone
cares. I left the date parsing in rev-tree.c for backward compatibility
but it can be dropped when we change to base64 :)

Yes, glibc sucks and strptime is a pile of crap. We have to parse it
ourselves.

Index: commit-tree.c
--- 1756b578489f93999ded68ae347bef7d6063101c/commit-tree.c  (mode:100664 sha1:12196c79f31d004dff0df1f50dda67d8204f5568)
+++ 82ba574c85e9a2e4652419c88244e9dd1bfa8baa/commit-tree.c  (mode:100644 sha1:35cb09402c9868499bcaf6de42afbad9fdfebe05)
@@ -7,6 +7,9 @@
 
 #include <pwd.h>
 #include <time.h>
+#include <string.h>
+#include <ctype.h>
+#include <time.h>
 
 #define BLOCKING (1ul << 14)
 #define ORIG_OFFSET (40)
@@ -95,6 +98,148 @@
 	}
 }
 
+static const char *month_names[] = {
+        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+};
+
+static const char *weekday_names[] = {
+        "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+};
+
+
+static char *skipfws(char *str)
+{
+	while (isspace(*str))
+		str++;
+	return str;
+}
+
+	
+/* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
+   (i.e. English) day/month names, and it doesn't work correctly with %z. */
+static void parse_rfc2822_date(char *date, char *result, int maxlen)
+{
+	struct tm tm;
+	char *p;
+	int i, offset;
+	time_t then;
+
+	memset(&tm, 0, sizeof(tm));
+
+	/* Skip day-name */
+	p = skipfws(date);
+	if (!isdigit(*p)) {
+		for (i=0; i<7; i++) {
+			if (!strncmp(p,weekday_names[i],3) && p[3] == ',') {
+				p = skipfws(p+4);
+				goto day;
+			}
+		}
+		return;
+	}					
+
+	/* day */
+ day:
+	tm.tm_mday = strtoul(p, &p, 10);
+
+	if (tm.tm_mday < 1 || tm.tm_mday > 31)
+		return;
+
+	if (!isspace(*p))
+		return;
+
+	p = skipfws(p);
+
+	/* month */
+
+	for (i=0; i<12; i++) {
+		if (!strncmp(p, month_names[i], 3) && isspace(p[3])) {
+			tm.tm_mon = i;
+			p = skipfws(p+strlen(month_names[i]));
+			goto year;
+		}
+	}
+	return; /* Error -- bad month */
+
+	/* year */
+ year:	
+	tm.tm_year = strtoul(p, &p, 10);
+
+	if (!tm.tm_year && !isspace(*p))
+		return;
+
+	if (tm.tm_year > 1900)
+		tm.tm_year -= 1900;
+		
+	p=skipfws(p);
+
+	/* hour */
+	if (!isdigit(*p))
+		return;
+	tm.tm_hour = strtoul(p, &p, 10);
+	
+	if (!tm.tm_hour > 23)
+		return;
+
+	if (*p != ':')
+		return; /* Error -- bad time */
+	p++;
+
+	/* minute */
+	if (!isdigit(*p))
+		return;
+	tm.tm_min = strtoul(p, &p, 10);
+	
+	if (!tm.tm_min > 59)
+		return;
+
+	if (isspace(*p))
+		goto zone;
+
+	if (*p != ':')
+		return; /* Error -- bad time */
+	p++;
+
+	/* second */
+	if (!isdigit(*p))
+		return;
+	tm.tm_sec = strtoul(p, &p, 10);
+	
+	if (!tm.tm_sec > 59)
+		return;
+
+	if (!isspace(*p))
+		return;
+
+ zone:
+	p = skipfws(p);
+
+	if (*p == '-')
+		offset = -60;
+	else if (*p == '+')
+		offset = 60;
+	else
+	       return;
+
+	if (!isdigit(p[1]) || !isdigit(p[2]) || !isdigit(p[3]) || !isdigit(p[4]))
+		return;
+
+	i = strtoul(p+1, NULL, 10);
+	offset *= ((i % 100) + ((i / 100) * 60));
+
+	if (*(skipfws(p + 5)))
+		return;
+
+	then = mktime(&tm); /* mktime appears to ignore the GMT offset, stupidly */
+	if (then == -1)
+		return;
+
+	then -= offset;
+
+	snprintf(result, maxlen, "%lu %5.5s", then, p);
+}
+
 /*
  * Having more than two parents may be strange, but hey, there's
  * no conceptual reason why the file format couldn't accept multi-way
@@ -114,10 +259,12 @@
 	unsigned char commit_sha1[20];
 	char *gecos, *realgecos;
 	char *email, realemail[1000];
-	char *date, *realdate;
+	char date[20], realdate[20];
+	char *audate;
 	char comment[1000];
 	struct passwd *pw;
 	time_t now;
+	struct tm *tm;
 	char *buffer;
 	unsigned int size;
 
@@ -142,15 +289,19 @@
 	realemail[len] = '@';
 	gethostname(realemail+len+1, sizeof(realemail)-len-1);
 	time(&now);
-	realdate = ctime(&now);
+	tm = localtime(&now);
+
+	strftime(realdate, sizeof(realdate), "%s %z", tm);
+	strcpy(date, realdate);
 
 	gecos = getenv("AUTHOR_NAME") ? : realgecos;
 	email = getenv("AUTHOR_EMAIL") ? : realemail;
-	date = getenv("AUTHOR_DATE") ? : realdate;
+	audate = getenv("AUTHOR_DATE");
+	if (audate)
+		parse_rfc2822_date(audate, date, sizeof(date));
 
 	remove_special(gecos); remove_special(realgecos);
 	remove_special(email); remove_special(realemail);
-	remove_special(date); remove_special(realdate);
 
 	init_buffer(&buffer, &size);
 	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
Index: rev-tree.c
--- 1756b578489f93999ded68ae347bef7d6063101c/rev-tree.c  (mode:100664 sha1:7bf9e9a92f528485360f374239809714ce7a19f5)
+++ 82ba574c85e9a2e4652419c88244e9dd1bfa8baa/rev-tree.c  (mode:100644 sha1:9443f4560beedcf81f2f5e7e0664d169cb5c8527)
@@ -1,4 +1,5 @@
 #define _XOPEN_SOURCE /* glibc2 needs this */
+#define _BSD_SOURCE /* for tm.tm_gmtoff */
 #include <time.h>
 #include <ctype.h>
 
@@ -21,6 +22,7 @@
 	char buffer[100];
 	struct tm tm;
 	const char *formats[] = {
+		"%s",
 		"%c",
 		"%a %b %d %T %y",
 		NULL
@@ -30,7 +32,7 @@
 	p = buffer;
 	while (isspace(c = *buf))
 		buf++;
-	while ((c = *buf++) != '\n')
+	while ((c = *buf++) != '\n' && c)
 		*p++ = c;
 	*p++ = 0;
 	buf = buffer;
@@ -50,6 +52,8 @@
 
 static unsigned long parse_commit_date(const char *buf)
 {
+	unsigned long time;
+
 	if (memcmp(buf, "author", 6))
 		return 0;
 	while (*buf++ != '\n')
@@ -58,7 +62,11 @@
 		return 0;
 	while (*buf++ != '>')
 		/* nada */;
-	return parse_time(buf);
+
+	time = strtoul(buf, NULL, 10);
+	if (!time)
+		time = parse_time(buf);
+	return time;
 }
 
 static int parse_commit(unsigned char *sha1)


-- 
dwmw2


^ permalink raw reply

* Re: Yet another base64 patch
From: Linus Torvalds @ 2005-04-14 17:42 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Christopher Li, git
In-Reply-To: <425EA152.4090506@zytor.com>



On Thu, 14 Apr 2005, H. Peter Anvin wrote:
> 
> Eh?!  n_link limits the number of *subdirectories* a directory can 
> contain, not the number of *entries*.

Duh. I'm a git.

		Linus

^ permalink raw reply

* Handling renames.
From: David Woodhouse @ 2005-04-14 17:54 UTC (permalink / raw)
  To: git; +Cc: James Bottomley

I've been looking at tracking file revisions. One proposed solution was
to have a separate revision history for individual files, with a new
kind of 'filecommit' object which parallels the existing 'commit',
referencing a blob instead of a tree. Then trees would reference such
objects instead of referencing blobs directly.

I think that introduces a lot of redundancy though, because 99% of the
time, the revision history of the individual file is entirely
reproducible from the revision history of the tree. It's only when files
are renamed that we fall over -- and I think we can handle renames
fairly well if we just log them in the commit object. 

My 'gitfilelog.sh' script is already capable of tracking a given file
back through multiple tree commits, listing those commits where the file
in question was actually changed. It uses my patched version of diff-
tree which supports 'diff-tree <TREE_A> <TREE_B> <filename>' in order to
do this.

By storing rename information in the commit object, the script (or a
reimplementation of a similar algorithm) could know when to change the
filename it's looking for, as it goes back through the tree. That ought
to be perfectly sufficient.

So a commit involving a rename would look something like this...

	tree 82ba574c85e9a2e4652419c88244e9dd1bfa8baa
	parent bb95843a5a0f397270819462812735ee29796fb4
	rename foo.c bar.c
	author David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
	committer David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
	Rename foo.c to bar.c and s/foo_/bar_/g

Opinions? Dissent? We'd probably need to escape the filenames in some
way -- handwave over that for now.

-- 
dwmw2


^ permalink raw reply

* Re: Handling renames.
From: Linus Torvalds @ 2005-04-14 18:11 UTC (permalink / raw)
  To: David Woodhouse; +Cc: git, James Bottomley
In-Reply-To: <1113501260.27227.26.camel@hades.cambridge.redhat.com>



On Thu, 14 Apr 2005, David Woodhouse wrote:
>
> I've been looking at tracking file revisions. One proposed solution was
> to have a separate revision history for individual files, with a new
> kind of 'filecommit' object which parallels the existing 'commit',
> referencing a blob instead of a tree. Then trees would reference such
> objects instead of referencing blobs directly.

Please don't.  It's fundamentally the git notion of "content determines
objects".

It also has no relevance. A "rename" really doesn't exist in the git 
model. The git model really is about tracking data, not about tracking 
what happened to _create_ that data.

The one exception is the commit log. That's where you put the explanations 
of _why_ the data changed. And git itself doesn't care what the format is, 
apart from the git header.

So, you really need to think of git as a filesystem. You can then 
implement an SCM _on_top_of_it_, which means that your second suggestion 
is not only acceptable, it really is the _only_ way to handle this in git:

> So a commit involving a rename would look something like this...
> 
> 	tree 82ba574c85e9a2e4652419c88244e9dd1bfa8baa
> 	parent bb95843a5a0f397270819462812735ee29796fb4
> 	rename foo.c bar.c
> 	author David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
> 	committer David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
> 	Rename foo.c to bar.c and s/foo_/bar_/g

Except I want that empty line in there, and I want it in the "free-form"  
section. The "rename" part really isn't part of the git header. It's not 
what git tracks, it was tracked by an SCM system on top of git.

So the git header is an "inode" in the git filesystem, and like an inode 
it has a ctime and an mtime, and pointers to the data. So as far as git is 
concerned, this part:

	tree 82ba574c85e9a2e4652419c88244e9dd1bfa8baa
	parent bb95843a5a0f397270819462812735ee29796fb4
	author David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
	committer David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100

really is the filesystem "inode". The rest is whatever the filesystem user
puts into it, and git won't care.

> Opinions? Dissent? We'd probably need to escape the filenames in some
> way -- handwave over that for now.

The fact that git handles arbitrary filenames (stuff starting with "." 
excepted) doesn't mean that the SCM above it needs to. Quite frankly, I 
think an SCM that handles newlines in filenames is being silly. But a 
_filesystem_ needs to not care.

There are too many messy SCM's out there that do not hav ea "philosophy". 
Dammit, I'm not interested in creating another one. This thing has a 
mental model, and we keep to that model.

The reason UNIX is beautiful is that it has a mental model of processes 
and files. Git has a mental model of objects and certain very very limited 
relationships. The relationships git cares about are encoded in the C 
files, the "extra crap" (like rename info) is just that - stuff that 
random scripts wrote, and that is just informational and not central to 
the model.

		Linus

^ permalink raw reply

* Re: Handling renames.
From: Ingo Molnar @ 2005-04-14 18:12 UTC (permalink / raw)
  To: David Woodhouse; +Cc: git, James Bottomley
In-Reply-To: <1113501260.27227.26.camel@hades.cambridge.redhat.com>


* David Woodhouse <dwmw2@infradead.org> wrote:

> I've been looking at tracking file revisions. One proposed solution 
> was to have a separate revision history for individual files, with a 
> new kind of 'filecommit' object which parallels the existing 'commit', 
> referencing a blob instead of a tree. Then trees would reference such 
> objects instead of referencing blobs directly.
> 
> I think that introduces a lot of redundancy though, because 99% of the 
> time, the revision history of the individual file is entirely 
> reproducible from the revision history of the tree. It's only when 
> files are renamed that we fall over -- and I think we can handle 
> renames fairly well if we just log them in the commit object.

how about the following structure:

    - tree_new --->
    - tree_old ---> rename_commit -> blob

the rename_commit object just contains a pointer to the file content 
blob. If a rename happens then the old tree references the rename_commit 
object (instead of the blob), and the new tree references it too. This 
way there's no need to list the rename via namespace means: if a tree 
entry points to a rename_commit object then a rename happened and the 
rename_commit object is looked up in the old tree to get the old name.

there's no redundancy caused by this method: only renames (which are 
rare) go through the rename_commit redirection. (to speed up the lookup 
the rename_commit object could cache the offset of the two names within 
their tree objects.)

	Ingo

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Junio C Hamano @ 2005-04-14 18:12 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Linus Torvalds, git
In-Reply-To: <20050414121624.GZ25711@pasky.ji.cz>

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

PB> Bah, you outran me. ;-)

Just being in a different timezone, I guess.

PB> I'll change it to use the cool git-pasky stuff (commit-id etc) and its
PB> style of committing - that is, it will merely record the update-caches
PB> to be done upon commit, and it will read-tree the branch we are merging
PB> to instead of the ancestor. (So that git diff gives useful output.)

Sorry, I have not seen what you have been doing since pasky 0.3,
and I have not even started to understand the mental model of
the world your tool is building.  That said, my gut feeling is
that telling this script about git-pasky's world model might be
a mistake.  I'd rather see you consider the script as mere "part
of the plumbing".  Maybe adding an extra parameter to the script
to let the user explicitly specify the common ancestor to use
would be needed, but I would prefer git-pasky-merge to do its
own magic (converting symbolic commit names into raw commit
names and such) before calling this low level script.

That way people like me who have not migrated to your framework
can still keep using it.  All the script currently needs is a
bare git object database; i.e., nothing other than what is in
.git/objects and a couple of commit record SHA1s as its
parameters.  No .git/heads/, no .git/HEAD.local, no .git/tags,
are involved for it to work, and I would prefer to keep things
that way if possible.

>> * show-diff updates to add -r flag to squelch diffs for files not in
>> the working directory.  This is mainly useful when verifying the
>> result of an automated merge.

PB> -r traditionally means recursive - what's the reasoning behind the
PB> choice of this letter?

Well, '-r' is not necessarily recursive. "ls -r" is reverse, "sort
-r" is reverse.  "less -r" is raw.  "cat -r" is reversible.
"nethack -r" is race ;-).  You are thinking as an SCM person so
it may look that way.  "diff -r" is recursive.  "darcs add -r"
is recursive.  But even in the SCM world, "cvs add -r" is not
(it means read-only) neither "co -r" (explicit revision) ;-).

I would rather pick '-q' if I were doing the patch today, but I
was too tired and did not think of a letter when I wrote it.  I
guess '-r' stood for removed, but I agree it is a bad choice.
Any objections to '-q'?

PB> use strict?

Not in this iteration but eventually yes.

PB> It'd be simpler to do just

PB> 	my @common = (map { $common{$_} }
PB> 	              sort { $b <=> $a }
PB> 	              keys %common)

Well, actually you spotted a bug between the implementation and
what I wanted to do.  It should have been:

map { $_->[0] }
    sort { $b->[1] <=> $a->[1] }
        map { [ $common{$_} => $_ ] } keys %common

That is, sort [anscestor => number of times it appears] tuple by
the "number of times it appears" in decreasing order, and
project the resulting list to a list of ancestors.  It is trying
to deal with the following pattern in rev-tree output:

TIMESTAMP1 EDGE1:1 ANCESTOR1:3 ANCESTOR2:3
TIMESTAMP2 EDGE2:2 ANCESTOR1:3

and when the above happens I wanted to pick up ANCESTOR1, but
that was without no sound reason.

PB> But I really think this is a horrible heuristic. I believe you should
PB> take the latest commit in the --edges output, and from that choose the
PB> base whose rev-tree --edges the_base merged_branch has the least lines
PB> on output. (That is, the path to it is shortest - ideally it's already
PB> part of the merged_branch.)

I'll try something along that line.  Honestly the ancestor
selection part was what I had most trouble with.  Thanks.

PB> What about

PB> sub OLDMODE { 0 }
PB> sub NEWMODE { 1 }
PB> sub NEWSHA { 2 }

PB> and then using that when accessing the tuple? Would make the code
PB> much more readable.

Totally agreed; readability cleanup is needed, just as "use
strict" you mentioned, before it is ready for public
consumption.  Remember, however, the primary purpose of the
message was to share it with Linus so that I can ask his opinion
while the script was still slushy; the contents that array
contained was still changing then and was too early for symbolic
constants.  I'll do that in the next round.

PB> It is a good idea to check merge's exit code and give a notice at the
PB> end if there were any conflicts.

In principle yes, but I noticed that merge already gave me a
nice warning message when it found conflicts, so there was no
need to do so myself in this case.  See sample output:

    $ perl ./git-merge.perl \
        71796686221a0a56ccc25b02386ed8ea648da14d \
        bb95843a5a0f397270819462812735ee29796fb4 
    Common ancestor: 9f02d4d233223462d3f6217b5837b786e6286ba4
    O - COPYING
    O - README
    ...
    O - write-tree.c
    A M write-blob.c
    A M show-diff.c
    ...
    A M update-cache.c
    A M git-merge.perl
    B M merge-tree.c
    MRG Makefile
    merge: warning: conflicts during merge
    $ 

>> +# Create a temporary directory and go there.
>> +system 'rm', '-rf', ',,merge-temp';

PB> Can't we call it just ,,merge?

I'd rather have a command line option '-o' (scrapping the
current '-o' and renaming it to something else; as you can see I
am terrible at picking option names ;-)) to mean "output to this
directory".  I am not really an Arch person so I do not
particulary care about /^,,/.  How about "git~merge~$$"?

>> +for ((',,merge-temp', '.git')) { mkdir $_; chdir $_; }
>> +symlink "../../.git/objects", "objects";
>> +chdir '..';
>> +
>> +my $ancestor_tree = read_commit_tree($common);
>> +system 'read-tree', $ancestor_tree;
>> +
>> +my %tree0 = read_diff_tree($ancestor_tree, read_commit_tree($ARGV[0]));
>> +my %tree1 = read_diff_tree($ancestor_tree, read_commit_tree($ARGV[1]));
>> +
>> +my @ancestor_file = read_show_files();
>> +my %ancestor_file = map { $_ => 1 } @ancestor_file;
>> +
>> +for (@ancestor_file) {
>> +    if (! exists $tree0{$_} && ! exists $tree1{$_}) {
>> +	if ($full_checkout) {
>> +	    system 'checkout-cache', $_;
>> +	}
>> +	print STDERR "O - $_\n";

PB> Huh, what are you trying to do here? I think you should just record
PB> remove, no? (And I wouldn't do anything with my read-tree. ;-)

At this moment in the script, we have run "read-tree" the
ancestor so the dircache has the original.  %tree0 and %tree1
both did not touch the path ($_ here) so it is the same as
ancestor.  When '-f' is specified we are populating the output
working tree with the merge result so that is what that
'checkout-cache' is about.  "O - $path" means "we took the
original".

The idea is to populate the dircache of merge-temp with the
merge result and leave uncertain stuff as in the common ancestor
state, so that the user can fix them starting from there.

Maybe it is a good time for me to summarize the output somewhere
in a document.

    O - $path	Tree-A and tree-B did not touch this; the result
                is taken from the ancestor (O for original).

    A D $path	Only tree-A (or tree-B) deleted this and the other
    B D $path   branch did not touch this; the result is to delete.

    A M $path	Only tree-A (or tree-B) modified this and the other
    B M $path   branch did not touch this; the result is to use one
                from tree-A (or tree-B).  This includes file
                creation case.

    *DD $path	Both tree-A and tree-B deleted this; the result
                is to delete.

    *DM $path   Tree-A deleted while tree-B modified this (or
    *MD $path   vice versa), and manual conflict resolution is
                needed; dircache is left as in the ancestor, and
                the modified file is saved as $path~A~ in the
                working directory.  The user can rename it to $path
                and run show-diff to see what Tree-A wanted to do
                and decide before running update-cache.

    *MM $path   Tree-A and tree-B did the exact same
                modification; the result is to use that.

    MRG $path   Tree-A and tree-B have different modifications;
                run "merge" and the merge result is left as
                $path in the working directory.

In cases other than *DM, *MD, and MRG, the result is trivial and
is recorded in the dircache.  Without '-o' (to be renamed ;-)
nor '-f' there will not be a file checked out in the working
directory for them.  The three merge cases need human attention.
The dircache is not touched in these cases and left as the
ancestor version, and the working directory gets some file as
described above.

NOTE NOTE NOTE: I am not dealing with a case where both branches
create the same file but with different contents.  In such a
case the current code falls into MRG path without having a
common ancestor, which is nonsense---I can use /dev/null as the
common ancestor, I guess.  Also NOTE NOTE NOTE I need to detect
the case where one branch creates a directory while the other
creates a file.  There is nothing an automated tool can do in
that case but it needs to be detected and be told the user
loudly.


^ permalink raw reply

* Re: Handling renames.
From: H. Peter Anvin @ 2005-04-14 18:21 UTC (permalink / raw)
  To: David Woodhouse; +Cc: git, James Bottomley
In-Reply-To: <1113501260.27227.26.camel@hades.cambridge.redhat.com>

David Woodhouse wrote:
> 
> Opinions? Dissent? We'd probably need to escape the filenames in some
> way -- handwave over that for now.
> 

For readability and simplicity I'd suggest using either URL-style %XX 
escapes or octal \xxx escapes for anything bytes < 33, minus the escape 
character.

Although Linus is correct in that an SCM doesn't *have* to handle this, 
it really feels like shooting for mediocracy to me.  We might as well 
design it right from the beginning.

	-hpa

^ permalink raw reply

* [PATCH 1/1] git: remove redundant getenv() in init-db
From: tony.luck @ 2005-04-14 18:30 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

From: Tony Luck <tony.luck@intel.com>

init-db calls getenv(DB_ENVIRONMENT) twice.  Once should be enough.

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

 init-db.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

--- init-db.c
+++ init-db.c	2005-04-14 11:01:52.000000000 -0700
@@ -7,7 +7,7 @@
 
 int main(int argc, char **argv)
 {
-	char *sha1_dir = getenv(DB_ENVIRONMENT), *path;
+	char *sha1_dir, *path;
 	int len, i;
 
 	if (mkdir(".git", 0755) < 0) {

^ permalink raw reply

* Re: Handling renames.
From: Linus Torvalds @ 2005-04-14 18:32 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: David Woodhouse, git, James Bottomley
In-Reply-To: <20050414181224.GA16126@elte.hu>



On Thu, 14 Apr 2005, Ingo Molnar wrote:
> 
> there's no redundancy caused by this method: only renames (which are 
> rare) go through the rename_commit redirection. (to speed up the lookup 
> the rename_commit object could cache the offset of the two names within 
> their tree objects.)

Bzzt. Wrong.

ANYTHING you do with games like this will cause the "same directory 
creates different object" crap.

Git doesn't do that. There fundamentally is no history in objects, 
_except_ for the commit object. Two objects with the same name are 
identical, and that means that they are easy to share. 

Any time you break that model, you break the whole point of git. Don't do 
it. You'll be very very sorry if you ever do, because it breaks the clean 
separation of "time" and "space". I guarantee you that your merges will 
become _harder_ rather than easier.

What you can do at an SCM level, is that if you want to track renames, you
track them as a separate commit altogether. Ie if you notice a rename, you
first commit the rename (and you can _see_ it's a rename, since the object
didn't change, and the sha1 stayed the same, which in git-speak means that
it is the same object, ie that _is_ a rename as far as git is concerned),
and then you create the "this is the data that changed" as a _second_
commit.

But don't make it a new kind of commit. It's just a regular commit, 
dammit. No new abstractions. 

Trust me, it's worth it to follow the rules. You don't start making up new 
concepts for every new thing you track. Next you'll want "tag objects". 
That's a totally idiotic idea. What you do is you tag things at a higher 
level than git ever is, and git will _never_ have to know about tag 
objects. 

Some "higher level" thing can add its own rules _on_top_ of git rules. The
same way we have normal applications having their _own_ rules on top of
the kernel. You do abstraction in layers, but for this to work, the base 
you build on top of had better be damn solid, and not have any ugly 
special cases.

		Linus

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Linus Torvalds @ 2005-04-14 18:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Petr Baudis, git
In-Reply-To: <7vll7lqlbg.fsf@assigned-by-dhcp.cox.net>



On Thu, 14 Apr 2005, Junio C Hamano wrote:
> 
> Sorry, I have not seen what you have been doing since pasky 0.3,
> and I have not even started to understand the mental model of
> the world your tool is building.  That said, my gut feeling is
> that telling this script about git-pasky's world model might be
> a mistake.  I'd rather see you consider the script as mere "part
> of the plumbing". 

I agree. Having separate abstraction layers is good.  I'm actually very 
happy with Pasky's cleaned-up-tree, exactly because unlike the first one, 
Pasky did a great job of maintaining the abstraction between "plumbing" 
and user interfaces.

The plumbing should take user interface needs into account, but the more
conceptually separate it is ("does it makes sense on its own?") the better
off we'll be. And "merge these two trees" (which works on a _tree_ level)
or "find the common commit" (which works on a _commit_ level) look like 
plumbing to me - the kind of things I should have written, if I weren't 
such a lazy slob.

		Linus

^ permalink raw reply

* Re: Handling renames.
From: Linus Torvalds @ 2005-04-14 18:48 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: David Woodhouse, git, James Bottomley
In-Reply-To: <425EB4AE.4010700@zytor.com>



On Thu, 14 Apr 2005, H. Peter Anvin wrote:
> 
> Although Linus is correct in that an SCM doesn't *have* to handle this, 
> it really feels like shooting for mediocracy to me.  We might as well 
> design it right from the beginning.

No. git is not an SCM. it's a filesystem designed to _host_ an SCM, and 
that _is_ doing it right from the beginning.

Keep the abstractions clean. Do _not_ get confused into thinking that git 
is an SCM. If you think of it that way, you'll end up with crap you can't 
think about.

And at a filesystem layer, "rename" already exists. It's moving an object 
to a new name in a tree. git already does that very well, thank you very 
much.

But a filesystem rename is _not_ the same thing as an SCM rename.  An SCM 
rename is built on top of a filesystem rename, but it has its own issues 
that may or may not make sense for the filesystem.

		Linus

^ permalink raw reply

* Re: Handling renames.
From: H. Peter Anvin @ 2005-04-14 18:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: David Woodhouse, git, James Bottomley
In-Reply-To: <Pine.LNX.4.58.0504141145220.7211@ppc970.osdl.org>

Linus Torvalds wrote:
> 
> On Thu, 14 Apr 2005, H. Peter Anvin wrote:
> 
>>Although Linus is correct in that an SCM doesn't *have* to handle this, 
>>it really feels like shooting for mediocracy to me.  We might as well 
>>design it right from the beginning.
> 
> No. git is not an SCM. it's a filesystem designed to _host_ an SCM, and 
> that _is_ doing it right from the beginning.
> 
> Keep the abstractions clean. Do _not_ get confused into thinking that git 
> is an SCM. If you think of it that way, you'll end up with crap you can't 
> think about.
> 
> And at a filesystem layer, "rename" already exists. It's moving an object 
> to a new name in a tree. git already does that very well, thank you very 
> much.
> 
> But a filesystem rename is _not_ the same thing as an SCM rename.  An SCM 
> rename is built on top of a filesystem rename, but it has its own issues 
> that may or may not make sense for the filesystem.
> 

I wasn't referring to git per se, I was referring to the hosted SCM.

	-hpa

^ permalink raw reply

* Re: Handling renames.
From: Ingo Molnar @ 2005-04-14 18:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: David Woodhouse, git, James Bottomley
In-Reply-To: <Pine.LNX.4.58.0504141124220.7211@ppc970.osdl.org>


* Linus Torvalds <torvalds@osdl.org> wrote:

> [...] Ie if you notice a rename, you first commit the rename (and you 
> can _see_ it's a rename, since the object didn't change, and the sha1 
> stayed the same, which in git-speak means that it is the same object, 
> ie that _is_ a rename as far as git is concerned), and then you create 
> the "this is the data that changed" as a _second_ commit.

ok, i accept your point of not putting this into such a low level as the 
object abstraction. Was a bad idea.

but i dont think the above would be enough: there can be renames of 
objects that have the same sha1 hash as other objects in the same tree, 
and developers want to track individual objects, regardless of whether 
other files share the same content. So some formal operation would be 
needed to signal renames - e.g. to embedd it in the commit object, per 
David's suggestion.

The thing i tried to avoid was to list long filenames in the commit 
(because of the tree hierarchy we'd need to do tree-absolute pathnames 
or something like that, and escape things, and do lookups - duplicating 
a VFS which is quite bad) - it would be better to identify the rename 
source and target via its tree object hash and its offset within that 
tree. Such information could be embedded in the commit object just fine.  
Something like:

me bb95843a5a0f397270819462812735ee29796fb4
tree 1756b578489f93999ded68ae347bef7d6063101c
parent 9f02d4d233223462d3f6217b5837b786e6286ba4
author
committer
rename 39021759c903a943a33a28cfbd5070d36d851581 15234 9f02d4d233223462d3f6217b5837b786e6286ba4 16163

?

	Ingo

^ permalink raw reply

* trying to figure out this git thing - some questions
From: Ehud Shabtai @ 2005-04-14 19:02 UTC (permalink / raw)
  To: git

I'm trying to understand how it works and I'll appreciate if someone could help.

1. git uses object abstraction for the different types and so
everything is in one directory (objects). From what I've seen in the
implementation, the different kind of objects are not of the same type
(there aren't any operations which work on two different types) and
thus in each step when an object is used its type is verified.
What's the benefit of having them all in the same tree? An alternative
would be to separate the different object types into different
directories which trivially allows getting a list of all commits, or
trees or blobs.

2. A commit can have more than one parent. Can anyone draw an example
of such a case? When do we get a commit graph which is not linear?

3. How does git handle binary files? I guess it doesn't really care if
it's binary or text, but how would the diff and merge scripts handle
them?

^ permalink raw reply

* Re: Handling renames.
From: David Woodhouse @ 2005-04-14 19:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, James Bottomley
In-Reply-To: <Pine.LNX.4.58.0504141102430.7211@ppc970.osdl.org>

On Thu, 2005-04-14 at 11:11 -0700, Linus Torvalds wrote:
> So, you really need to think of git as a filesystem. You can then 
> implement an SCM _on_top_of_it_, which means that your second suggestion 
> is not only acceptable, it really is the _only_ way to handle this in git:
> 
> > So a commit involving a rename would look something like this...
> > 
> > 	tree 82ba574c85e9a2e4652419c88244e9dd1bfa8baa
> > 	parent bb95843a5a0f397270819462812735ee29796fb4
> > 	rename foo.c bar.c
> > 	author David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
> > 	committer David Woodhouse <dwmw2@hades.cambridge.redhat.com> 1113499881 +0100
> > 	Rename foo.c to bar.c and s/foo_/bar_/g
> 
> Except I want that empty line in there, and I want it in the "free-form"  
> section. The "rename" part really isn't part of the git header. It's not 
> what git tracks, it was tracked by an SCM system on top of git.

Note that not only may you have a _set_ of renames, but you'll also have
a _different_ set of renames for each parent. Consider the
representation of a merge where a file was called 'foo' in one parent,
'bar' in the other, and we called it 'foobar' in the resulting tree.

That's the main reason I wanted the renames in with the parent
information -- so it's <parent><rename><rename><...><parent><rename>...

I see your point though and I can't be bothered to argue for the sake of
the slight efficiency benefit we might gain from doing it that way. The
implementation details really aren't that interesting right now.

Let us assume, however, that we have this information somehow stored in
each commit object. It's perfectly sufficient from the POV of the 
'git revtool' which I've been poking at; is it good enough for merges?

Consider a simple case: A branch is taken, file foo.c is renamed to
bar.c, and now we're trying to merge that branch back into the head,
which has moved on. 

We can't just take 'bar.c' as a new file -- we have to track it all the
way back to its inception, and notice that it actually shares a common
ancestor with 'foo.c' in the other parent of the merge.

How feasible, and how computationally expensive, is that task going to
be? Especially given that there may be _many_ new files that we need to
attempt to tie up with their partners, across many potential renames. 

One option for optimising this, if we really need to, might be to track
the file back to its _first_ ancestor and use that as an identification.
The SCM could store that identifier in the blob itself, or we could
consider it an 'inode number' and store it in git's tree objects.

If we can avoid that, however, it would be nice. How feasible is the
merge going to be without it?

-- 
dwmw2



^ permalink raw reply

* Re: Yet another base64 patch
From: bert hubert @ 2005-04-14 19:11 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: H. Peter Anvin, Christopher Li, git
In-Reply-To: <Pine.LNX.4.58.0504141042450.7211@ppc970.osdl.org>

On Thu, Apr 14, 2005 at 10:42:56AM -0700, Linus Torvalds wrote:
> > Eh?!  n_link limits the number of *subdirectories* a directory can 
> > contain, not the number of *entries*.
> 
> Duh. I'm a git.

That may be true :-), but from the "front lines" I can report that
directories with > 32000 or > 65000 entries is *asking* for trouble. There
is a whole chain of systems that need to get things right for huge
directories to work well, and it often is not that way.

Even though it should be.

So the question is, should git be the harbringer of improvements in this
area, or should it go with the flow.

Bert.

-- 
http://www.PowerDNS.com      Open source, database driven DNS Software 
http://netherlabs.nl              Open and Closed source services

^ permalink raw reply

* Re: Date handling.
From: tony.luck @ 2005-04-14 19:19 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Linus Torvalds, git
In-Reply-To: <1113500316.27227.8.camel@hades.cambridge.redhat.com>

> OK. commit-tree now eats RFC2822 dates as AUTHOR_DATE because that's
> what you're going to want to feed it. We store seconds since UTC epoch,
> we add the author's or committer's timezone as auxiliary data so that
> dates can be pretty-printed in the original timezone later if anyone
> cares.

With a UTC date, why would anyone care in which timezone the commit was
made?  Any pretty printing would most likely be prettiest if it is done
relative to the timezone of the person looking at the commit record, not
the person who created the record.

If we do need the timezone, then I think we also need the latitude of the
committer too, so that we know whether to interpret "July" as summer or
winter :-)

-Tony

^ 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