Git development
 help / color / mirror / Atom feed
* gitk on Windows: layout problem
From: Rutger Nijlunsing @ 2006-05-30 18:54 UTC (permalink / raw)
  To: git

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

Hi,

Is this a known problem? gitk-du-jour on Windows starts up with an
unusable layout. Screenshot attached.

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

[-- Attachment #2: gitk.PNG --]
[-- Type: image/png, Size: 11482 bytes --]

^ permalink raw reply

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Alex Riesen @ 2006-05-30 17:32 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: Christopher Faylor, git
In-Reply-To: <20060530170805.GC32457@h4x0r5.com>

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

On 5/30/06, Ryan Anderson <ryan@michonline.com> wrote:
> On Tue, May 30, 2006 at 06:00:20PM +0200, Alex Riesen wrote:
> > If you actually read the message, you'd probably notice ActiveState Perl.
> >
> > I have no idea why have you taken my post as an attempt to insult cygwin;
> > IF I had that in mind I'd dedicate a whole long post just to that.
>
> FWIW, it was probably this:
>         if test "$(uname -o)"= Cygwin; then
>
> (I only mention becuase I was about to apply this, then I saw that line,
> and now I'm confused, is this a fix for ActiveState, or Cygwin?)
>

Right. My bad. Should be "$(perl -e 'print $^O')" = MSWin32. That ($^O)
is actually how it is checked in git-annotate.perl (open_pipe).

Christopher, my apologies if that was that. I actually am hostile to
Windows and everything around it, and have my reasons for this.
Still, it does not justify the way how I did that patch.

[-- Attachment #2: send-mail-test.patch --]
[-- Type: text/x-patch, Size: 679 bytes --]

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index a61da1e..7afc358 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -25,6 +25,11 @@ test_expect_success \
      git add fake.sendmail
      GIT_AUTHOR_NAME="A" git commit -a -m "Second."'
 
+if test "$(perl -e 'print $^O')" = MSWin32; then
+    say "git-send-mail tests disabled on ActiveState Perl + Windows"
+    # because of windows being such a crap
+else
+
 test_expect_success \
     'Extract patches and send' \
     'git format-patch -n HEAD^1
@@ -38,4 +43,6 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+fi
+
 test_done



^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Ryan Anderson @ 2006-05-30 17:08 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Christopher Faylor, git
In-Reply-To: <81b0412b0605300900l7530792dqcea6d812602b9176@mail.gmail.com>

On Tue, May 30, 2006 at 06:00:20PM +0200, Alex Riesen wrote:
> If you actually read the message, you'd probably notice ActiveState Perl.
> 
> I have no idea why have you taken my post as an attempt to insult cygwin;
> IF I had that in mind I'd dedicate a whole long post just to that.

FWIW, it was probably this:
	if test "$(uname -o)"= Cygwin; then

(I only mention becuase I was about to apply this, then I saw that line,
and now I'm confused, is this a fix for ActiveState, or Cygwin?)

^ permalink raw reply

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Christopher Faylor @ 2006-05-30 17:03 UTC (permalink / raw)
  To: Alex Riesen, git
In-Reply-To: <81b0412b0605300900l7530792dqcea6d812602b9176@mail.gmail.com>

On Tue, May 30, 2006 at 06:00:20PM +0200, Alex Riesen wrote:
>On 5/30/06, Christopher Faylor <me@cgf.cx> wrote:
>>>froze afterwards anyway, as "wc" or "perl" did. Besides, it the
>>>command often freezes that poor imitation of xterm windows has.
>>
>>I assume that "the poor imitation of xterm" is referring to cygwin's
>>xterm here.  It's really too bad that you can't get into the mindset of
>>reporting problems to the cygwin mailing list when you notice them.
>
>Actually, I was referring to windows console. And no, I don't think you
>could do something about it.
>
>But honestly, I don't think it's worth supporting windows in general
>and cygwin in particular. And before anyone (again) asks why am
>_I_ doing it: I'd have to do my job with Perforce otherwise (as if
>windows wasn't bad enough...)

I think you've made your opinion on this issue pretty clear.

>>I can't comment on the proposed patch since, AFAIK, using cat, wc, and
>>(cygwin's) perl should all work just fine but I don't think it is ever
>>correct to complain about a platform in released software.
>
>If you actually read the message, you'd probably notice ActiveState Perl.

I actually did read the message.  Do you really want to employ this type
of hackneyed usenet technique here?

Referring to the Windows console as "xterm" (if that is what you were
actually doing) means that your bug report and patch were unclear.

You were also talking about "cat" and "wc" hanging.  Neither is a
Windows command.  Since you've previously complained that you had
to use Cygwin, I think it is safe to assume that you were talking
about Cygwin commands.

>I have no idea why have you taken my post as an attempt to insult cygwin;

There you go.  Kick it up a notch.  I asked you (and have been asking
you) to report problems with Cygwin in the Cygwin forums.  I don't think
that someone mentioning that software has bugs is an "insult".  You
obviously aren't insulting git by fixing bugs that you find.

>IF I had that in mind I'd dedicate a whole long post just to that.

I could write a really long essay about inappropriate mailing list
hostility, too.  Neither would be on-topic here, of course.

Anyway, I think that comments should be factual and not contain negative
opinions about Windows, Solaris, or whatever.

cgf

^ permalink raw reply

* tree_entry(): new tree-walking helper function
From: Linus Torvalds @ 2006-05-30 16:45 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


This adds a "tree_entry()" function that combines the common operation of 
doing a "tree_entry_extract()" + "update_tree_entry()".

It also has a simplified calling convention, designed for simple loops 
that traverse over a whole tree: the arguments are pointers to the tree 
descriptor and a name_entry structure to fill in, and it returns a boolean 
"true" if there was an entry left to be gotten in the tree.

This allows tree traversal with

	struct tree_desc desc;
	struct name_entry entry;

	desc.buf = tree->buffer;
	desc.size = tree->size;
	while (tree_entry(&desc, &entry) {
		... use "entry.{path, sha1, mode, pathlen}" ...
	}

which is not only shorter than writing it out in full, it's hopefully less 
error prone too.

[ It's actually a tad faster too - we don't need to recalculate the entry 
  pathlength in both extract and update, but need to do it only once.  
  Also, some callers can avoid doing a "strlen()" on the result, since 
  it's returned as part of the name_entry structure.

  However, by now we're talking just 1% speedup on "git-rev-list --objects 
  --all", and we're definitely at the point where tree walking is no 
  longer the issue any more. ]

NOTE! Not everybody wants to use this new helper function, since some of 
the tree walkers very much on purpose do the descriptor update separately 
from the entry extraction. So the "extract + update" sequence still 
remains as the core sequence, this is just a simplified interface.

We should probably add a silly two-line inline helper function for 
initializing the descriptor from the "struct tree" too, just to cut down 
on the noise from that common "desc" initializer.

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

This is another of my "I'm not going to push this patch very aggressively" 
series. I think it's a cleanup, and it removes more lines than it adds and 
seems to make things more readable, but is it worth it? You decide.

 builtin-grep.c      |   26 ++++++++++----------------
 builtin-read-tree.c |   36 +++++++++++++-----------------------
 builtin-rev-list.c  |   16 +++++-----------
 builtin-tar-tree.c  |   21 ++++++++-------------
 fetch.c             |   16 +++++-----------
 http-push.c         |   16 +++++-----------
 pack-objects.c      |   27 +++++++++++----------------
 revision.c          |   16 +++++-----------
 tree-walk.c         |   33 +++++++++++++++++++++++++++++++--
 tree-walk.h         |    5 ++++-
 tree.c              |   41 +++++++++++++++--------------------------
 11 files changed, 112 insertions(+), 141 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 53de8a8..acc4eea 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -578,11 +578,9 @@ static int grep_tree(struct grep_opt *op
 		     struct tree_desc *tree,
 		     const char *tree_name, const char *base)
 {
-	unsigned mode;
 	int len;
 	int hit = 0;
-	const char *path;
-	const unsigned char *sha1;
+	struct name_entry entry;
 	char *down;
 	char *path_buf = xmalloc(PATH_MAX + strlen(tree_name) + 100);
 
@@ -597,36 +595,32 @@ static int grep_tree(struct grep_opt *op
 	}
 	len = strlen(path_buf);
 
-	while (tree->size) {
-		int pathlen;
-		sha1 = tree_entry_extract(tree, &path, &mode);
-		pathlen = strlen(path);
-		strcpy(path_buf + len, path);
+	while (tree_entry(tree, &entry)) {
+		strcpy(path_buf + len, entry.path);
 
-		if (S_ISDIR(mode))
+		if (S_ISDIR(entry.mode))
 			/* Match "abc/" against pathspec to
 			 * decide if we want to descend into "abc"
 			 * directory.
 			 */
-			strcpy(path_buf + len + pathlen, "/");
+			strcpy(path_buf + len + entry.pathlen, "/");
 
 		if (!pathspec_matches(paths, down))
 			;
-		else if (S_ISREG(mode))
-			hit |= grep_sha1(opt, sha1, path_buf);
-		else if (S_ISDIR(mode)) {
+		else if (S_ISREG(entry.mode))
+			hit |= grep_sha1(opt, entry.sha1, path_buf);
+		else if (S_ISDIR(entry.mode)) {
 			char type[20];
 			struct tree_desc sub;
 			void *data;
-			data = read_sha1_file(sha1, type, &sub.size);
+			data = read_sha1_file(entry.sha1, type, &sub.size);
 			if (!data)
 				die("unable to read tree (%s)",
-				    sha1_to_hex(sha1));
+				    sha1_to_hex(entry.sha1));
 			sub.buf = data;
 			hit |= grep_tree(opt, paths, &sub, tree_name, down);
 			free(data);
 		}
-		update_tree_entry(tree);
 	}
 	return hit;
 }
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index d70411f..0c6ba3d 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -54,28 +54,23 @@ typedef int (*merge_fn_t)(struct cache_e
 static struct tree_entry_list *create_tree_entry_list(struct tree *tree)
 {
 	struct tree_desc desc;
+	struct name_entry one;
 	struct tree_entry_list *ret = NULL;
 	struct tree_entry_list **list_p = &ret;
 
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 
-	while (desc.size) {
-		unsigned mode;
-		const char *path;
-		const unsigned char *sha1;
+	while (tree_entry(&desc, &one)) {
 		struct tree_entry_list *entry;
 
-		sha1 = tree_entry_extract(&desc, &path, &mode);
-		update_tree_entry(&desc);
-
 		entry = xmalloc(sizeof(struct tree_entry_list));
-		entry->name = path;
-		entry->sha1 = sha1;
-		entry->mode = mode;
-		entry->directory = S_ISDIR(mode) != 0;
-		entry->executable = (mode & S_IXUSR) != 0;
-		entry->symlink = S_ISLNK(mode) != 0;
+		entry->name = one.path;
+		entry->sha1 = one.sha1;
+		entry->mode = one.mode;
+		entry->directory = S_ISDIR(one.mode) != 0;
+		entry->executable = (one.mode & S_IXUSR) != 0;
+		entry->symlink = S_ISLNK(one.mode) != 0;
 		entry->next = NULL;
 
 		*list_p = entry;
@@ -846,27 +841,22 @@ static int read_cache_unmerged(void)
 static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
 {
 	struct tree_desc desc;
+	struct name_entry entry;
 	int cnt;
 
 	memcpy(it->sha1, tree->object.sha1, 20);
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 	cnt = 0;
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-		if (!S_ISDIR(mode))
+	while (tree_entry(&desc, &entry)) {
+		if (!S_ISDIR(entry.mode))
 			cnt++;
 		else {
 			struct cache_tree_sub *sub;
-			struct tree *subtree = lookup_tree(sha1);
+			struct tree *subtree = lookup_tree(entry.sha1);
 			if (!subtree->object.parsed)
 				parse_tree(subtree);
-			sub = cache_tree_sub(it, name);
+			sub = cache_tree_sub(it, entry.path);
 			sub->cache_tree = cache_tree();
 			prime_cache_tree_rec(sub->cache_tree, subtree);
 			cnt += sub->cache_tree->entry_count;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 6e2b898..17c04b9 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -114,6 +114,7 @@ static struct object_list **process_tree
 {
 	struct object *obj = &tree->object;
 	struct tree_desc desc;
+	struct name_entry entry;
 	struct name_path me;
 
 	if (!revs.tree_objects)
@@ -132,18 +133,11 @@ static struct object_list **process_tree
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-
-		if (S_ISDIR(mode))
-			p = process_tree(lookup_tree(sha1), p, &me, name);
+	while (tree_entry(&desc, &entry)) {
+		if (S_ISDIR(entry.mode))
+			p = process_tree(lookup_tree(entry.sha1), p, &me, name);
 		else
-			p = process_blob(lookup_blob(sha1), p, &me, name);
+			p = process_blob(lookup_blob(entry.sha1), p, &me, name);
 	}
 	free(tree->buffer);
 	tree->buffer = NULL;
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 2d5e06f..5f740cf 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -271,30 +271,25 @@ static void write_global_extended_header
 static void traverse_tree(struct tree_desc *tree, struct strbuf *path)
 {
 	int pathlen = path->len;
+	struct name_entry entry;
 
-	while (tree->size) {
-		const char *name;
-		const unsigned char *sha1;
-		unsigned mode;
+	while (tree_entry(tree, &entry)) {
 		void *eltbuf;
 		char elttype[20];
 		unsigned long eltsize;
 
-		sha1 = tree_entry_extract(tree, &name, &mode);
-		update_tree_entry(tree);
-
-		eltbuf = read_sha1_file(sha1, elttype, &eltsize);
+		eltbuf = read_sha1_file(entry.sha1, elttype, &eltsize);
 		if (!eltbuf)
-			die("cannot read %s", sha1_to_hex(sha1));
+			die("cannot read %s", sha1_to_hex(entry.sha1));
 
 		path->len = pathlen;
-		strbuf_append_string(path, name);
-		if (S_ISDIR(mode))
+		strbuf_append_string(path, entry.path);
+		if (S_ISDIR(entry.mode))
 			strbuf_append_string(path, "/");
 
-		write_entry(sha1, path, mode, eltbuf, eltsize);
+		write_entry(entry.sha1, path, entry.mode, eltbuf, eltsize);
 
-		if (S_ISDIR(mode)) {
+		if (S_ISDIR(entry.mode)) {
 			struct tree_desc subtree;
 			subtree.buf = eltbuf;
 			subtree.size = eltsize;
diff --git a/fetch.c b/fetch.c
index b03c201..e9347ba 100644
--- a/fetch.c
+++ b/fetch.c
@@ -39,25 +39,19 @@ static int process(struct object *obj);
 static int process_tree(struct tree *tree)
 {
 	struct tree_desc desc;
+	struct name_entry entry;
 
 	if (parse_tree(tree))
 		return -1;
 
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-
-		if (S_ISDIR(mode)) {
-			struct tree *tree = lookup_tree(sha1);
+	while (tree_entry(&desc, &entry)) {
+		if (S_ISDIR(entry.mode)) {
+			struct tree *tree = lookup_tree(entry.sha1);
 			process_tree(tree);
 		} else {
-			struct blob *blob = lookup_blob(sha1);
+			struct blob *blob = lookup_blob(entry.sha1);
 			process(&blob->object);
 		}
 	}
diff --git a/http-push.c b/http-push.c
index 72ad89c..b1c018a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1715,6 +1715,7 @@ static struct object_list **process_tree
 {
 	struct object *obj = &tree->object;
 	struct tree_desc desc;
+	struct name_entry entry;
 	struct name_path me;
 
 	obj->flags |= LOCAL;
@@ -1734,18 +1735,11 @@ static struct object_list **process_tree
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-
-		if (S_ISDIR(mode))
-			p = process_tree(lookup_tree(sha1), p, &me, name);
+	while (tree_entry(&desc, &entry)) {
+		if (S_ISDIR(entry.mode))
+			p = process_tree(lookup_tree(entry.sha1), p, &me, name);
 		else
-			p = process_blob(lookup_blob(sha1), p, &me, name);
+			p = process_blob(lookup_blob(entry.sha1), p, &me, name);
 	}
 	free(tree->buffer);
 	tree->buffer = NULL;
diff --git a/pack-objects.c b/pack-objects.c
index 77284cf..3590cd5 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -690,25 +690,20 @@ static void add_pbase_object(struct tree
 			     const char *name,
 			     int cmplen)
 {
-	while (tree->size) {
-		const unsigned char *sha1;
-		const char *entry_name;
-		int entry_len;
-		unsigned mode;
+	struct name_entry entry;
+
+	while (tree_entry(tree,&entry)) {
 		unsigned long size;
 		char type[20];
 
-		sha1 = tree_entry_extract(tree, &entry_name, &mode);
-		update_tree_entry(tree);
-		entry_len = strlen(entry_name);
-		if (entry_len != cmplen ||
-		    memcmp(entry_name, name, cmplen) ||
-		    !has_sha1_file(sha1) ||
-		    sha1_object_info(sha1, type, &size))
+		if (entry.pathlen != cmplen ||
+		    memcmp(entry.path, name, cmplen) ||
+		    !has_sha1_file(entry.sha1) ||
+		    sha1_object_info(entry.sha1, type, &size))
 			continue;
 		if (name[cmplen] != '/') {
 			unsigned hash = name_hash(up, name);
-			add_object_entry(sha1, hash, 1);
+			add_object_entry(entry.sha1, hash, 1);
 			return;
 		}
 		if (!strcmp(type, tree_type)) {
@@ -718,15 +713,15 @@ static void add_pbase_object(struct tree
 			const char *down = name+cmplen+1;
 			int downlen = name_cmp_len(down);
 
-			tree = pbase_tree_get(sha1);
+			tree = pbase_tree_get(entry.sha1);
 			if (!tree)
 				return;
 			sub.buf = tree->tree_data;
 			sub.size = tree->tree_size;
 
 			me.up = up;
-			me.elem = entry_name;
-			me.len = entry_len;
+			me.elem = entry.path;
+			me.len = entry.pathlen;
 			add_pbase_object(&sub, &me, down, downlen);
 			pbase_tree_put(tree);
 		}
diff --git a/revision.c b/revision.c
index 8e93e40..6a6952c 100644
--- a/revision.c
+++ b/revision.c
@@ -54,6 +54,7 @@ static void mark_blob_uninteresting(stru
 void mark_tree_uninteresting(struct tree *tree)
 {
 	struct tree_desc desc;
+	struct name_entry entry;
 	struct object *obj = &tree->object;
 
 	if (obj->flags & UNINTERESTING)
@@ -66,18 +67,11 @@ void mark_tree_uninteresting(struct tree
 
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-
-		if (S_ISDIR(mode))
-			mark_tree_uninteresting(lookup_tree(sha1));
+	while (tree_entry(&desc, &entry)) {
+		if (S_ISDIR(entry.mode))
+			mark_tree_uninteresting(lookup_tree(entry.sha1));
 		else
-			mark_blob_uninteresting(lookup_blob(sha1));
+			mark_blob_uninteresting(lookup_blob(entry.sha1));
 	}
 
 	/*
diff --git a/tree-walk.c b/tree-walk.c
index 3922058..297c697 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -37,7 +37,7 @@ static void entry_extract(struct tree_de
 
 void update_tree_entry(struct tree_desc *desc)
 {
-	void *buf = desc->buf;
+	const void *buf = desc->buf;
 	unsigned long size = desc->size;
 	int len = strlen(buf) + 1 + 20;
 
@@ -63,7 +63,7 @@ static const char *get_mode(const char *
 
 const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)
 {
-	void *tree = desc->buf;
+	const void *tree = desc->buf;
 	unsigned long size = desc->size;
 	int len = strlen(tree)+1;
 	const unsigned char *sha1 = tree + len;
@@ -78,6 +78,35 @@ const unsigned char *tree_entry_extract(
 	return sha1;
 }
 
+int tree_entry(struct tree_desc *desc, struct name_entry *entry)
+{
+	const void *tree = desc->buf, *path;
+	unsigned long len, size = desc->size;
+
+	if (!size)
+		return 0;
+
+	path = get_mode(tree, &entry->mode);
+	if (!path)
+		die("corrupt tree file");
+
+	entry->path = path;
+	len = strlen(path);
+	entry->pathlen = len;
+
+	path += len + 1;
+	entry->sha1 = path;
+
+	path += 20;
+	len = path - tree;
+	if (len > size)
+		die("corrupt tree file");
+
+	desc->buf = path;
+	desc->size = size - len;
+	return 1;
+}
+
 void traverse_trees(int n, struct tree_desc *t, const char *base, traverse_callback_t callback)
 {
 	struct name_entry *entry = xmalloc(n*sizeof(*entry));
diff --git a/tree-walk.h b/tree-walk.h
index 47438fe..e57befa 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -2,7 +2,7 @@ #ifndef TREE_WALK_H
 #define TREE_WALK_H
 
 struct tree_desc {
-	void *buf;
+	const void *buf;
 	unsigned long size;
 };
 
@@ -16,6 +16,9 @@ struct name_entry {
 void update_tree_entry(struct tree_desc *);
 const unsigned char *tree_entry_extract(struct tree_desc *, const char **, unsigned int *);
 
+/* Helper function that does both of the above and returns true for success */
+int tree_entry(struct tree_desc *, struct name_entry *);
+
 void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1);
 
 typedef void (*traverse_callback_t)(int n, unsigned long mask, struct name_entry *entry, const char *base);
diff --git a/tree.c b/tree.c
index fb18724..9bbe2da 100644
--- a/tree.c
+++ b/tree.c
@@ -79,6 +79,7 @@ int read_tree_recursive(struct tree *tre
 			read_tree_fn_t fn)
 {
 	struct tree_desc desc;
+	struct name_entry entry;
 
 	if (parse_tree(tree))
 		return -1;
@@ -86,18 +87,11 @@ int read_tree_recursive(struct tree *tre
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
-
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-
-		if (!match_tree_entry(base, baselen, name, mode, match))
+	while (tree_entry(&desc, &entry)) {
+		if (!match_tree_entry(base, baselen, entry.path, entry.mode, match))
 			continue;
 
-		switch (fn(sha1, base, baselen, name, mode, stage)) {
+		switch (fn(entry.sha1, base, baselen, entry.path, entry.mode, stage)) {
 		case 0:
 			continue;
 		case READ_TREE_RECURSIVE:
@@ -105,18 +99,17 @@ int read_tree_recursive(struct tree *tre
 		default:
 			return -1;
 		}
-		if (S_ISDIR(mode)) {
+		if (S_ISDIR(entry.mode)) {
 			int retval;
-			int pathlen = strlen(name);
 			char *newbase;
 
-			newbase = xmalloc(baselen + 1 + pathlen);
+			newbase = xmalloc(baselen + 1 + entry.pathlen);
 			memcpy(newbase, base, baselen);
-			memcpy(newbase + baselen, name, pathlen);
-			newbase[baselen + pathlen] = '/';
-			retval = read_tree_recursive(lookup_tree(sha1),
+			memcpy(newbase + baselen, entry.path, entry.pathlen);
+			newbase[baselen + entry.pathlen] = '/';
+			retval = read_tree_recursive(lookup_tree(entry.sha1),
 						     newbase,
-						     baselen + pathlen + 1,
+						     baselen + entry.pathlen + 1,
 						     stage, match, fn);
 			free(newbase);
 			if (retval)
@@ -156,6 +149,7 @@ static int track_tree_refs(struct tree *
 	int n_refs = 0, i;
 	struct object_refs *refs;
 	struct tree_desc desc;
+	struct name_entry entry;
 
 	/* Count how many entries there are.. */
 	desc.buf = item->buffer;
@@ -170,18 +164,13 @@ static int track_tree_refs(struct tree *
 	refs = alloc_object_refs(n_refs);
 	desc.buf = item->buffer;
 	desc.size = item->size;
-	while (desc.size) {
-		unsigned mode;
-		const char *name;
-		const unsigned char *sha1;
+	while (tree_entry(&desc, &entry)) {
 		struct object *obj;
 
-		sha1 = tree_entry_extract(&desc, &name, &mode);
-		update_tree_entry(&desc);
-		if (S_ISDIR(mode))
-			obj = &lookup_tree(sha1)->object;
+		if (S_ISDIR(entry.mode))
+			obj = &lookup_tree(entry.sha1)->object;
 		else
-			obj = &lookup_blob(sha1)->object;
+			obj = &lookup_blob(entry.sha1)->object;
 		refs->ref[i++] = obj;
 	}
 	set_object_refs(&item->object, refs);

^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Alex Riesen @ 2006-05-30 16:00 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git
In-Reply-To: <20060530152103.GB8931@trixie.casa.cgf.cx>

On 5/30/06, Christopher Faylor <me@cgf.cx> wrote:
> >froze afterwards anyway, as "wc" or "perl" did. Besides, it the
> >command often freezes that poor imitation of xterm windows has.
>
> I assume that "the poor imitation of xterm" is referring to cygwin's
> xterm here.  It's really too bad that you can't get into the mindset of
> reporting problems to the cygwin mailing list when you notice them.

Actually, I was referring to windows console. And no, I don't think you
could do something about it.

But honestly, I don't think it's worth supporting windows in general
and cygwin in particular. And before anyone (again) asks why am
_I_ doing it: I'd have to do my job with Perforce otherwise (as if
windows wasn't bad enough...)

> I can't comment on the proposed patch since, AFAIK, using cat, wc, and
> (cygwin's) perl should all work just fine but I don't think it is ever
> correct to complain about a platform in released software.

If you actually read the message, you'd probably notice ActiveState Perl.

I have no idea why have you taken my post as an attempt to insult cygwin;
IF I had that in mind I'd dedicate a whole long post just to that.

^ permalink raw reply

* [PATCH] handle concurrent pruning of packed objects
From: Jeff King @ 2006-05-30 15:56 UTC (permalink / raw)
  To: junkio; +Cc: git

This patch causes read_sha1_file and sha1_object_info to re-examine the
list of packs if an object cannot be found. It works by re-running
prepare_packed_git() after an object fails to be found.

It does not attempt to clean up the old pack list. Old packs which are in
use can continue to be used (until unused by lru selection).  New packs
are placed at the front of the list and will thus be examined before old
packs.

Signed-off-by: Jeff King <peff@peff.net>
---

I tested this by making a simple repo with three commits: a, b, and c.
I ran git diff-tree --stdin, and then did the following:
  1. fed 'a b' to diff-tree
  2. ran git repack -a -d
  3. fed 'b c' to diff-tree
Vanilla git complains about the lack of object, whereas this version
provides the correct diff-tree output.

 sha1_file.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index f77c189..696e53f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -626,12 +626,12 @@ static void prepare_packed_git_one(char 
 	closedir(dir);
 }
 
+static int prepare_packed_git_run_once = 0;
 void prepare_packed_git(void)
 {
-	static int run_once = 0;
 	struct alternate_object_database *alt;
 
-	if (run_once)
+	if (prepare_packed_git_run_once)
 		return;
 	prepare_packed_git_one(get_object_directory(), 1);
 	prepare_alt_odb();
@@ -640,7 +640,13 @@ void prepare_packed_git(void)
 		prepare_packed_git_one(alt->base, 0);
 		alt->name[-1] = '/';
 	}
-	run_once = 1;
+	prepare_packed_git_run_once = 1;
+}
+
+static void reprepare_packed_git(void)
+{
+	prepare_packed_git_run_once = 0;
+	prepare_packed_git();
 }
 
 int check_sha1_signature(const unsigned char *sha1, void *map, unsigned long size, const char *type)
@@ -1212,9 +1218,12 @@ int sha1_object_info(const unsigned char
 	if (!map) {
 		struct pack_entry e;
 
-		if (!find_pack_entry(sha1, &e))
-			return error("unable to find %s", sha1_to_hex(sha1));
-		return packed_object_info(&e, type, sizep);
+		if (find_pack_entry(sha1, &e))
+			return packed_object_info(&e, type, sizep);
+		reprepare_packed_git();
+		if (find_pack_entry(sha1, &e))
+			return packed_object_info(&e, type, sizep);
+		return error("unable to find %s", sha1_to_hex(sha1));
 	}
 	if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
 		status = error("unable to unpack %s header",
@@ -1256,6 +1265,9 @@ void * read_sha1_file(const unsigned cha
 		munmap(map, mapsize);
 		return buf;
 	}
+	reprepare_packed_git();
+	if (find_pack_entry(sha1, &e))
+		return read_packed_sha1(sha1, type, size);
 	return NULL;
 }
 
-- 
1.3.3.g331f

^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Christopher Faylor @ 2006-05-30 15:21 UTC (permalink / raw)
  To: Junio C Hamano, Ryan Anderson, Alex Riesen, git
In-Reply-To: <81b0412b0605300623h4f915829yb388c8fdc062c009@mail.gmail.com>

On Tue, May 30, 2006 at 03:23:30PM +0200, Alex Riesen wrote:
>>--- a/git-send-email.perl
>>+++ b/git-send-email.perl
>>@@ -387,7 +387,9 @@ X-Mailer: git-send-email $gitversion
>>                my $pid = open my $sm, '|-';
>>                defined $pid or die $!;
>>                if (!$pid) {
>>-                       exec($smtp_server,'-i',@recipients) or die $!;
>
>This construction (perl pipe+fork) will not work on ActiveState Perl
>(it does not even parse the construct).
>Last time the problem arised it was suggested to replace readers
>with "qx{command}". Regretfully there were no writer case back
>then. I'd suggest using IPC::Open2 for portability. Like this:
>
> use IPC::Open2;
> my $fw;
> my $pid = open2(">&1", $fw, "perl", "-w");
> print $fw "exit 0\n";
> close($fw);'
>
>But I wont. It was never portable in windows, no matter how hard
>I tried. The best result was getting output from "cat -v", but "cat"
>froze afterwards anyway, as "wc" or "perl" did. Besides, it the
>command often freezes that poor imitation of xterm windows has.

I assume that "the poor imitation of xterm" is referring to cygwin's
xterm here.  It's really too bad that you can't get into the mindset of
reporting problems to the cygwin mailing list when you notice them.

I can't comment on the proposed patch since, AFAIK, using cat, wc, and
(cygwin's) perl should all work just fine but I don't think it is ever
correct to complain about a platform in released software.

cgf

>---
> t/t9001-send-email.sh |    7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
>diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>index a61da1e..c3a3737 100755
>--- a/t/t9001-send-email.sh
>+++ b/t/t9001-send-email.sh
>@@ -25,6 +25,11 @@ test_expect_success \
>      git add fake.sendmail
>      GIT_AUTHOR_NAME="A" git commit -a -m "Second."'
> 
>+if test "$(uname -o)" = Cygwin; then
>+    say "git-send-mail tests disabled on Windows"
>+    # because of windows being such a crap
>+else
>+

^ permalink raw reply

* Re: [RFC] git-fetch - repack in the background after fetching
From: Linus Torvalds @ 2006-05-30 14:53 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Martin Langhoff, git
In-Reply-To: <Pine.LNX.4.64.0605300207130.6713@iabervon.org>



On Tue, 30 May 2006, Daniel Barkalow wrote:
> 
> We should be able to fix this, right? If an object isn't found in packs or 
> unpacked, look for new packs; if there are any, look for the object in 
> them; if it's not there, then give up.

Yes. That sounds fine.

		Linus

^ permalink raw reply

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Alex Riesen @ 2006-05-30 13:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ryan Anderson, git
In-Reply-To: <7vu0782e33.fsf@assigned-by-dhcp.cox.net>

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

> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -387,7 +387,9 @@ X-Mailer: git-send-email $gitversion
>                 my $pid = open my $sm, '|-';
>                 defined $pid or die $!;
>                 if (!$pid) {
> -                       exec($smtp_server,'-i',@recipients) or die $!;

This construction (perl pipe+fork) will not work on ActiveState Perl
(it does not even parse the construct).
Last time the problem arised it was suggested to replace readers
with "qx{command}". Regretfully there were no writer case back
then. I'd suggest using IPC::Open2 for portability. Like this:

  use IPC::Open2;
  my $fw;
  my $pid = open2(">&1", $fw, "perl", "-w");
  print $fw "exit 0\n";
  close($fw);'

But I wont. It was never portable in windows, no matter how hard
I tried. The best result was getting output from "cat -v", but "cat"
froze afterwards anyway, as "wc" or "perl" did. Besides, it the
command often freezes that poor imitation of xterm windows has.

There is also 2-argument "open" with damn careful quoting,
if anyone cares (dont think anyone does, for windows).
How about disabling the test on cygwin? (patch attached).

[-- Attachment #2: 0001-disable-send-email-test-for-cygwin.txt --]
[-- Type: text/plain, Size: 731 bytes --]

---
 t/t9001-send-email.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index a61da1e..c3a3737 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -25,6 +25,11 @@ test_expect_success \
      git add fake.sendmail
      GIT_AUTHOR_NAME="A" git commit -a -m "Second."'
 
+if test "$(uname -o)" = Cygwin; then
+    say "git-send-mail tests disabled on Windows"
+    # because of windows being such a crap
+else
+
 test_expect_success \
     'Extract patches and send' \
     'git format-patch -n HEAD^1
@@ -38,4 +43,6 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+fi
+
 test_done
-- 
1.3.3.g7994


^ permalink raw reply related

* Re: [PATCH] git status: print files under untracked dir if -a is given
From: Yasushi SHOJI @ 2006-05-30 12:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfyir26v4.fsf@assigned-by-dhcp.cox.net>

Hi Junio,

At Tue, 30 May 2006 02:34:55 -0700,
Junio C Hamano wrote:
> 
> Yasushi SHOJI <yashi@atmark-techno.com> writes:
> 
> > git status: print files under untracked dir if -a is given
> >
> > git status (git-commit.sh) currently doesn't show files under
> > untracked directory.  this is inconvenient when adding many files
> > under new directory.
> >
> > this patch change its behavior to show files under untracked directory
> > if option --all is given.
> >
> > Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
> 
> I do not quite understand your rationale behind linking -a and
> "show untracked" behaviour.  In many cases, after modifying
> multiple files "commit -a" is the preferred way to make commits
> for people who keep their tree clean (meaning, they do not leave
> unrelated changes to their working tree files), and I suspect
> your change would clutter their commit log buffer with unrelated
> files they did not ask to see.

I assumed "--all" to mean "every single file under a working dir
except ignored". so I thought users of "commit -a" wouldn't mind to
see files under untracked dir. 

but I was wrong. man page clearly states that "... new files you have
not told git about are not affected."

# I admit I haven't used -a with commit because of my
# misunderstanding. it's nice to know the option is much safer than I
# expected.

> At least this would make things somewhat unpleasant for me to
> use, since I do "commit -a" often and I have my random notes
> files under ./+trash subdirectory of the main project (yes, I
> know I could add /+trash to .gitignore).

I wasn't expecting that usage.

> We have something different but perhaps related by Matthias
> Lederhofer to add "git status --untracked" since you did this
> patch.
> 
>         commit 443f8338b9e248353a7095a1096684f1ed106c66
>         Author: Matthias Lederhofer <matled@gmx.net>
>         Date:   Mon May 22 23:02:06 2006 +0200
> 
> Does it solve your problem?

yes, it perfectly does.

thanks,
--
       yashi

^ permalink raw reply

* [PATCH] cg-status: fix detection of dissappeared files
From: Jonas Fonseca @ 2006-05-30 12:48 UTC (permalink / raw)
  To: Petr Baudis, git

fatal: ambiguous argument 'manual.txt': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>

---
diff --git a/cg-status b/cg-status
index e2f97dd..aee5ff0 100755
--- a/cg-status
+++ b/cg-status
@@ -239,7 +239,7 @@ if [ "$workstatus" ]; then
 		git-diff-index HEAD -- "${basepath:-.}" | cut -f5- -d' ' | 
 		while IFS=$'\t' read -r mode file; do
 			if [ "$mode" = D ]; then
-				[ "$(git-diff-files "$file")" ] && mode=!
+				[ "$(git-diff-files -- "$file")" ] && mode=!
 			elif [ "$mode" = M ] && [ "$commitignore" ]; then
 				fgrep -qx "$file" "$_git/commit-ignore" && mode=m
 			fi


-- 
Jonas Fonseca

^ permalink raw reply related

* Re: [PATCH] git status: print files under untracked dir if -a is given
From: Junio C Hamano @ 2006-05-30  9:34 UTC (permalink / raw)
  To: Yasushi SHOJI; +Cc: git
In-Reply-To: <87slmrdhe6.wl@mail2.atmark-techno.com>

Yasushi SHOJI <yashi@atmark-techno.com> writes:

> git status: print files under untracked dir if -a is given
>
> git status (git-commit.sh) currently doesn't show files under
> untracked directory.  this is inconvenient when adding many files
> under new directory.
>
> this patch change its behavior to show files under untracked directory
> if option --all is given.
>
> Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>

I do not quite understand your rationale behind linking -a and
"show untracked" behaviour.  In many cases, after modifying
multiple files "commit -a" is the preferred way to make commits
for people who keep their tree clean (meaning, they do not leave
unrelated changes to their working tree files), and I suspect
your change would clutter their commit log buffer with unrelated
files they did not ask to see.

At least this would make things somewhat unpleasant for me to
use, since I do "commit -a" often and I have my random notes
files under ./+trash subdirectory of the main project (yes, I
know I could add /+trash to .gitignore).

We have something different but perhaps related by Matthias
Lederhofer to add "git status --untracked" since you did this
patch.

        commit 443f8338b9e248353a7095a1096684f1ed106c66
        Author: Matthias Lederhofer <matled@gmx.net>
        Date:   Mon May 22 23:02:06 2006 +0200

Does it solve your problem?

^ permalink raw reply

* Re: [RFC] git commit --branch
From: Junio C Hamano @ 2006-05-30  9:12 UTC (permalink / raw)
  To: Martin Waitz; +Cc: git
In-Reply-To: <20060529202851.GE14325@admingilde.org>

Martin Waitz <tali@admingilde.org> writes:

> Allow to commit to another branch and creating a merge in the current branch.
>
> Sometimes it is neccessary to have some local modifications in the tree
> in order to test it and work with it.  With this patch you can have
> one working tree which combines several topic branches in order to
> develop and test your changes.  When your changes are ready, you can
> commit them to the appropriate topic branch.
>
> With the --branch option, the commit will automatically be rebased to
> that branch.  The original tree will then be commited to your working
> branch as a merge.
>
> ---
>
> Perhaps something like this can even be integrated into the extended
> branch configuration which is currently under discussion.
> It may make sense to have one branch which always contains a merge
> of a selected set of other branches and having a default branch
> for new commits.

I think this was discussed in the past and I appreciate what you
are trying to do.  My understanding of the situation your patch
is trying to improve is this:

 - you have done a few topics and you are ready to test;

 - you pulled the topics into your test branch and have found
   problems;

 - you made changes while still on the test branch (otherwise
   you wouldn't be able to test the "fix") and it seems to work
   OK;

 - what now?  

And your approach is to backport the fix to its original topic
and then re-pull the topic onto the test branch.

While I think that is _one_ valid workflow, I am not convinced
that is _the_ best workflow.  What Johannes suggested would
equally work fine, and honestly speaking probably is a better
discipline.  You carry the fix in your working tree back to its
original topic and make a commit, without pulling the topic onto
the test branch immediately.  This has two advantages:

 - With your workflow, you will have a merge commit onto the
   testing branch immediately when you commit this fix to the
   original topic.  But often when I encounter this situation,
   after moving to the topic to backport the fix to it, I find
   myself reviewing what is in the topic and making other
   changes to the topic.  Johannes's workflow feels more natural
   to me from this aspect -- I take the fix I discovered while
   on the testing branch to the relevant topic to fix it.  I may
   or may not make the commit only with that fix (the first
   commit I make after switching the branches from testing to
   the topic may contain more than that fix), and after I make
   one commit, I may keep working on the topic a bit more before
   I decide it is a good time to test the whole thing again (to
   pull the topic into testing).  I do not necessarily want that
   extra merge immediately in the test branch.

 - A topic branch should be testable alone; if the changes near
   the tip of your topic depends on other topic (or more recent
   mainline than where the topic forked), then I think you
   shouldn't hesitate to pull in the other branch into the topic
   to keep it buildable and testable.  And your commit should be
   made after testing your changes; with your workflow, you have
   only tested the change in the context of the testing branch,
   not the topic branch your "primary" commit is on, even though
   that commit will be the source of eventual graduation to the
   mainline.  With Johannes's workflow, you first carry the
   change to the topic, so you have a chance to test it before
   making the commit (if you are not disciplined, you can make
   the commit without testing after switching branches, but the
   point is it gives people an option to test things before they
   make a commit if they wanted to).

^ permalink raw reply

* [PATCH] git status: print files under untracked dir if -a is given
From: Yasushi SHOJI @ 2006-05-30  8:46 UTC (permalink / raw)
  To: git; +Cc: tetsuya

git status: print files under untracked dir if -a is given

git status (git-commit.sh) currently doesn't show files under
untracked directory.  this is inconvenient when adding many files
under new directory.

this patch change its behavior to show files under untracked directory
if option --all is given.

Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>

---

3d3fa8f19c7d9b03b1a6e510970633ec8be7adac
 git-commit.sh |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

3d3fa8f19c7d9b03b1a6e510970633ec8be7adac
diff --git a/git-commit.sh b/git-commit.sh
index 6ef1a9d..0cde305 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -134,13 +134,18 @@ #'
 	report "Changed but not updated" \
 	    "use git-update-index to mark for commit"
 
+	if test -z "$all"
+	then
+	    directory_opt="--directory"
+	fi
+
 	if test -f "$GIT_DIR/info/exclude"
 	then
-	    git-ls-files -z --others --directory \
+	    git-ls-files -z --others $directory_opt \
 		--exclude-from="$GIT_DIR/info/exclude" \
 		--exclude-per-directory=.gitignore
 	else
-	    git-ls-files -z --others --directory \
+	    git-ls-files -z --others $directory_opt \
 		--exclude-per-directory=.gitignore
 	fi |
 	perl -e '$/ = "\0";
-- 
1.3.3.g70f7

^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Ryan Anderson @ 2006-05-30  8:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ryan Anderson, git
In-Reply-To: <7v8xok3vhj.fsf@assigned-by-dhcp.cox.net>

On Mon, May 29, 2006 at 10:57:44PM -0700, Junio C Hamano wrote:
> Ryan Anderson <rda@google.com> writes:
> 
> > Signed-off-by: Ryan Anderson <rda@google.com>
> >
> > ---
> >
> > 64ea8c0210c2e9d1711a870460eca326778a4ffc
> >  t/t9001-send-email.sh |   34 ++++++++++++++++++++++++++++++++++
> >  1 files changed, 34 insertions(+), 0 deletions(-)
> >  create mode 100755 t/t9001-send-email.sh
> 
> Adds test, alright, but I do not see the fix.  Is this a thinko?

I apparently screwed this patch up (and I think I lost it, in the
process.)

Let me reconstruct, I fixed the problems in a different way (I reworked
unique_email_address(@) into  unique_email_address($@), to pass a flag
stating whether to returned the cleaned email address or not, that
should come in a few minutes.)

^ permalink raw reply

* Re: [PATCH] git-clean fails on files beginning with a dash
From: Junio C Hamano @ 2006-05-30  8:49 UTC (permalink / raw)
  To: Dennis Stosberg; +Cc: git
In-Reply-To: <20060529150632.G6794bab6@leonov.stosberg.net>

Dennis Stosberg <dennis@stosberg.net> writes:

> Reproducible with:
>
> $ git init-db
> $ echo "some text" >-file
> $ git clean
> Removing -file
> rm: invalid option -- l
> Try `rm --help' for more information.

Thanks.

^ permalink raw reply

* Re: [PATCH] Automatically line wrap long commit messages.
From: Junio C Hamano @ 2006-05-30  8:38 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060529094605.GB27194@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> OK.  Ignore both patches then.  Two negative votes in such a short
> time suggests they are probably not generally accepted.  ;-)
>
>> We probably should allow "commit -F -" to read from the standard
>> input if we already don't, but that is about as far as I am
>> willing to go at this moment.
>
> We do.  So apparently the solution to my usage issue is:
>
> 	$ fmt -w 60 | git commit -F-
> 	This is my message.
>
> 	This is the body.  Etc....
> 	EOF
>
> I'm thinking that's too much work for me.

If we supported multiple -m (presumably each becomes a single line?)
with internal fmt, I do not see how it would become less work.

	$ git commit -w60 -m "This is my message." \
        	-m '' \
        	-m 'This is the body.  Etc....'

looks more typing to me, even without the second line to force
the empty line between the summary and the body.

^ permalink raw reply

* [PATCH] cvsimport: complete the cvsps run before starting the import
From: Martin Langhoff @ 2006-05-30  8:08 UTC (permalink / raw)
  To: junio, git; +Cc: Martin Langhoff

On 5/24/06, Linus Torvalds <torvalds@osdl.org> wrote:
> It's entirely possible that the fact that it now seems to work for me is
> purely timing-related, since I also ended up using "-P cvsps-output" to
> avoid having a huge cvsps binary in memory at the same time.

We now capture the output of cvsps to a tempfile, and then read it in.
cvsps 2.1 works quite a bit "in memory", and only prints its patchset info
once it has finished talking with cvs, but apparently retaining all that
memory allocation. With this patch, cvsps is finished and reaped before
cvsimport start working (and growing). So the footprint of the whole
process is much lower.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---

I don't particularly like the idea of switching from a safe system() call
to this ugly one. But this patch makes a huge difference importing gentoo's
repo, and I could not find a way to get system() to do redirection.

Of course, we could do the redirection in Perl. Ugly vs uglier?

---

 git-cvsimport.perl |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

5ce458e0883f39ae774ec67211e6565b65139b7f
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 60fc86a..2239c67 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -529,24 +529,36 @@ if ($opt_A) {
 	write_author_info("$git_dir/cvs-authors");
 }
 
-my $pid = open(CVS,"-|");
-die "Cannot fork: $!\n" unless defined $pid;
-unless($pid) {
+#
+# run cvsps into a file unless it's provided already
+#
+my $cvspsfile;
+if ($opt_P) {
+       $cvspsfile = $opt_P;
+} else {
+	my $cvspsfh;
+	($cvspsfh, $cvspsfile) = tempfile('gitXXXXXX', SUFFIX => '.cvsps',
+					  DIR => File::Spec->tmpdir());
+	close ($cvspsfh);
+	my ($cvspserrfh, $cvspserr)  = tempfile('gitXXXXXX', SUFFIX => '.err',
+						DIR => File::Spec->tmpdir());
+	close ($cvspserrfh);
+
 	my @opt;
 	@opt = split(/,/,$opt_p) if defined $opt_p;
 	unshift @opt, '-z', $opt_z if defined $opt_z;
-	unshift @opt, '-q'         unless defined $opt_v;
+	unshift @opt, '-q'	   unless defined $opt_v;
 	unless (defined($opt_p) && $opt_p =~ m/--no-cvs-direct/) {
 		push @opt, '--cvs-direct';
 	}
-	if ($opt_P) {
-	    exec("cat", $opt_P);
-	} else {
-	    exec("cvsps","--norc",@opt,"-u","-A",'--root',$opt_d,$cvs_tree);
-	    die "Could not start cvsps: $!\n";
-	}
+
+	print "Running cvsps\n"		  if $opt_v;
+	system(join(' ', "cvsps","--norc",@opt,"-u","-A",'--root',$opt_d,$cvs_tree, "1>$cvspsfile" ))
+		or die "Error in cvsps: $!\n";
 }
 
+open (CVS, "<$cvspsfile") 
+        or die "Cannot open cvsps output file $cvspsfile: $!\n";
 
 ## cvsps output:
 #---------------------
-- 
1.3.2.g82000

^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Junio C Hamano @ 2006-05-30  6:58 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: git
In-Reply-To: <7v1wuc3t9y.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> On top of yours, I think this covers the CC: trouble your test
> triggers.

Sorry, I did not look closely enough.  You are trying to keep
the address human friendly as long as possible so that you can
place them on the headers, so the previous one was bogus.

*BLUSH*

I think this is lower impact.  On the other hand, it appears
that at least whatever pretends to be /usr/lib/sendmail on my
box seems to grok 'A <author@example.com>' just fine, so maybe
the test was bogus (in which case you should just change the
expected command line parameters to include the human name).

I dunno.

-- >8 --
From c95682409346f7acc220ac64f453933d5a59ec3f Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 29 May 2006 23:53:13 -0700
Subject: [PATCH] send-email: do not pass bogus address to local sendmail binary

This makes t9001 test happy.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-send-email.perl   |    4 +++-
 t/t9001-send-email.sh |   19 +++++++++++++------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d418d6c..ac84553 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -387,7 +387,9 @@ X-Mailer: git-send-email $gitversion
 		my $pid = open my $sm, '|-';
 		defined $pid or die $!;
 		if (!$pid) {
-			exec($smtp_server,'-i',@recipients) or die $!;
+			exec($smtp_server,'-i',
+			     map { extract_valid_address($_) }
+			     @recipients) or die $!;
 		}
 		print $sm "$header\n$message";
 		close $sm or die $?;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 276cbac..a61da1e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -13,10 +13,14 @@ test_expect_success \
 
 test_expect_success \
     'Setup helper tool' \
-    'echo "#!/bin/sh" > fake.sendmail
-     echo "shift" >> fake.sendmail
-     echo "echo \"\$*\" > commandline" >> fake.sendmail
-     echo "cat > msgtxt" >> fake.sendmail
+    '(echo "#!/bin/sh"
+      echo shift
+      echo for a
+      echo do
+      echo "  echo \"!\$a!\""
+      echo "done >commandline"
+      echo "cat > msgtxt"
+      ) >fake.sendmail
      chmod +x ./fake.sendmail
      git add fake.sendmail
      GIT_AUTHOR_NAME="A" git commit -a -m "Second."'
@@ -26,9 +30,12 @@ test_expect_success \
     'git format-patch -n HEAD^1
      git send-email -from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" ./0001*txt'
 
+cat >expected <<\EOF
+!nobody@example.com!
+!author@example.com!
+EOF
 test_expect_success \
     'Verify commandline' \
-    'cline=$(cat commandline)
-     [ "$cline" == "nobody@example.com author@example.com" ]'
+    'diff commandline expected'
 
 test_done
-- 
1.3.3.g5029f

^ permalink raw reply related

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Junio C Hamano @ 2006-05-30  6:45 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: git
In-Reply-To: <7v8xok3vhj.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

>> 64ea8c0210c2e9d1711a870460eca326778a4ffc
>>  t/t9001-send-email.sh |   34 ++++++++++++++++++++++++++++++++++
>>  1 files changed, 34 insertions(+), 0 deletions(-)
>>  create mode 100755 t/t9001-send-email.sh
>
> Adds test, alright, but I do not see the fix.  Is this a thinko?

On top of yours, I think this covers the CC: trouble your test
triggers.

-- >8 -
send-email: fix cc address fed to underlying sendmail

---
diff --git a/git-send-email.perl b/git-send-email.perl
index d418d6c..d61ef8e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -448,9 +448,11 @@ foreach my $t (@files) {
 					else {
 						$author_not_sender = $2;
 					}
-					printf("(mbox) Adding cc: %s from line '%s'\n",
-						$2, $_) unless $quiet;
-					push @cc, $2;
+					my $cc = extract_valid_address($2);
+					printf("(mbox) Adding cc: %s from ".
+					       "line '%s'\n",
+						$cc, $_) unless $quiet;
+					push @cc, $cc;
 				}
 
 			} else {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 276cbac..a61da1e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -13,10 +13,14 @@ test_expect_success \
 
 test_expect_success \
     'Setup helper tool' \
-    'echo "#!/bin/sh" > fake.sendmail
-     echo "shift" >> fake.sendmail
-     echo "echo \"\$*\" > commandline" >> fake.sendmail
-     echo "cat > msgtxt" >> fake.sendmail
+    '(echo "#!/bin/sh"
+      echo shift
+      echo for a
+      echo do
+      echo "  echo \"!\$a!\""
+      echo "done >commandline"
+      echo "cat > msgtxt"
+      ) >fake.sendmail
      chmod +x ./fake.sendmail
      git add fake.sendmail
      GIT_AUTHOR_NAME="A" git commit -a -m "Second."'
@@ -26,9 +30,12 @@ test_expect_success \
     'git format-patch -n HEAD^1
      git send-email -from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" ./0001*txt'
 
+cat >expected <<\EOF
+!nobody@example.com!
+!author@example.com!
+EOF
 test_expect_success \
     'Verify commandline' \
-    'cline=$(cat commandline)
-     [ "$cline" == "nobody@example.com author@example.com" ]'
+    'diff commandline expected'
 
 test_done

^ permalink raw reply related

* Re: [RFC] git-fetch - repack in the background after fetching
From: Daniel Barkalow @ 2006-05-30  6:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Martin Langhoff, git
In-Reply-To: <Pine.LNX.4.64.0605292147010.5623@g5.osdl.org>

On Mon, 29 May 2006, Linus Torvalds wrote:

> Some long-running (in git terms) git programs will look up the pack-files 
> when they start, and if you repack after that, they won't see the new 
> pack-file, but they _will_ notice that the unpacked files are no longer 
> there, and will be very unhappy indeed.

We should be able to fix this, right? If an object isn't found in packs or 
unpacked, look for new packs; if there are any, look for the object in 
them; if it's not there, then give up. The only tricky thing is making it 
possible to scan through the available packs without installing any that 
are already installed. I think the failure case is only a critical path in 
the history-walking fetch code, which should probably disable this (or 
defer it to after trying to download the object).

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: irc usage..
From: Martin Langhoff @ 2006-05-30  6:01 UTC (permalink / raw)
  To: Donnie Berkholz
  Cc: Linus Torvalds, Yann Dirson, Git Mailing List, Matthias Urlichs,
	Johannes Schindelin
In-Reply-To: <447BD8C1.6090402@gentoo.org>

On 5/30/06, Donnie Berkholz <spyderous@gentoo.org> wrote:
> All I can think of is that I somehow OOM'd when I manually ran a repack
> and didn't notice it. But that should've at least made me unable to
> resume the cvsimport process, which happily kept chugging along later on.

Sounds likely -- and cvsimport restarts gracefully, though you might want to do

   git checkout HEAD

to get a usable checkout if the very first import failed. However, the
default head is master, and what you want to look at is origin or
whatever you passed as your -o parameter. I use cvshead normally, so I
do

   git log cvshead

> > My dmesg talks about an earlier cvs segfault. Nasty tree you have here
> > -- it's breaking all sorts of things... and teaching us a thing or two
> > about the import process.
> >
> >> Committed patch 249100 (origin 2005-08-20 05:05:58)
> >
> > Hmmm? How can you be at patch 249100 and still be a good year ahead of
> > me? Have you told cvsps to cut off old history?
>
> Nope. I ran the exact cvsps flags you posted earlier to create it.

Oh, that was an earlier PEBKAK at my end: I did git log HEAD instead
of git log cvshead. My import is now at  293145 (cvshead +0000
2005-12-25 12:24:42) which looks promising.

cheers,


martin

^ permalink raw reply

* Re: [PATCH 4/4] Add a basic test case for git send-email, and fix some real bugs discovered.
From: Junio C Hamano @ 2006-05-30  5:57 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: junkio, git
In-Reply-To: <11489310153617-git-send-email-1>

Ryan Anderson <rda@google.com> writes:

> Signed-off-by: Ryan Anderson <rda@google.com>
>
> ---
>
> 64ea8c0210c2e9d1711a870460eca326778a4ffc
>  t/t9001-send-email.sh |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9001-send-email.sh

Adds test, alright, but I do not see the fix.  Is this a thinko?

^ permalink raw reply

* Re: irc usage..
From: Donnie Berkholz @ 2006-05-30  5:31 UTC (permalink / raw)
  To: Martin Langhoff
  Cc: Linus Torvalds, Yann Dirson, Git Mailing List, Matthias Urlichs,
	Johannes Schindelin
In-Reply-To: <46a038f90605291719r292269bct61bf2817a9791e3d@mail.gmail.com>

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

Martin Langhoff wrote:
> On 5/30/06, Donnie Berkholz <spyderous@gentoo.org> wrote:
>> Looking closer, I see that the memory suckers do appear to be git, from
>> dmesg:
>>
>> Out of Memory: Kill process 17230 (git-repack) score 97207 and children.
>> Out of memory: Killed process 17231 (git-rev-list).
> 
> That would mean that you do have Linus' patch then. Grep cvsimport for
> repack and remove the -a -- and consider using his recent patch to
> rev-list.

You certainly would think so, and I did as well, but available evidence
indicates otherwise. I'm not sure how the repack got in there.

donnie@supernova ~ $ type git-cvsimport
git-cvsimport is /usr/bin/git-cvsimport
donnie@supernova ~ $ grep repack /usr/bin/git-cvsimport
donnie@supernova ~ $

All I can think of is that I somehow OOM'd when I manually ran a repack
and didn't notice it. But that should've at least made me unable to
resume the cvsimport process, which happily kept chugging along later on.

> My dmesg talks about an earlier cvs segfault. Nasty tree you have here
> -- it's breaking all sorts of things... and teaching us a thing or two
> about the import process.
> 
>> Committed patch 249100 (origin 2005-08-20 05:05:58)
> 
> Hmmm? How can you be at patch 249100 and still be a good year ahead of
> me? Have you told cvsps to cut off old history?

Nope. I ran the exact cvsps flags you posted earlier to create it.

Thanks,
Donnie


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

^ 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