Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.
From: Jakub Narebski @ 2006-09-06  9:12 UTC (permalink / raw)
  To: git
In-Reply-To: <7v64g1a9f7.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Note that you still have to give all the flags from the command
> line; only rev arguments (including A..B, A...B, and A^@ notations)
> can be give from the standard input.

Does this include ^A notation?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: stgit bisect feature request
From: Catalin Marinas @ 2006-09-06  8:36 UTC (permalink / raw)
  To: Torgil Svensson; +Cc: git
In-Reply-To: <e7bda7770609051649j56a92085i8d8a73ab12004c43@mail.gmail.com>

"Torgil Svensson" <torgil.svensson@gmail.com> wrote:
> I think i'd be nice if stgit wraps bisect and you could write
> $ stg bisect start
> $ stg bisect bad
> $ stg bisect good ORIG_HEAD
>
> and after bisect checks out the files it pushes the current stack as
> if you had used stg pull at this point

It would be nice but there could be conflicts when pushing patches,
especially if you do the bisect over a larger range of commits. It
gets difficult if some StGIT patches rely on features not present some
of the commits you are checking.

Have you tried this idea manually (or with some scripts) and found it
feasible?

-- 
Catalin

^ permalink raw reply

* Re: stgit bisect feature request
From: Karl Hasselström @ 2006-09-06  8:31 UTC (permalink / raw)
  To: Torgil Svensson; +Cc: git
In-Reply-To: <e7bda7770609051649j56a92085i8d8a73ab12004c43@mail.gmail.com>

On 2006-09-06 01:49:15 +0200, Torgil Svensson wrote:

> I think i'd be nice if stgit wraps bisect and you could write
>
> $ stg bisect start
> $ stg bisect bad
> $ stg bisect good ORIG_HEAD
>
> and after bisect checks out the files it pushes the current stack as
> if you had used stg pull at this point

Yes, this sounds very useful for bisecting when you can't test the old
revisions as-is, e.g. because you have to patch for an unrelated bug,
or because you need a patch to trigger the problem.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH 3/5] autoconf: Preliminary check for working mmap
From: Junio C Hamano @ 2006-09-06  8:15 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060906034054.GB30598@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Shawn Pearce <spearce@spearce.org> writes:
>> 
>> > I don't know if I've made this more complex than I really need to
>> > but I've permitted multiple windows per pack.  There is just one
>> > LRU of all windows across all packs and a maximum amount of address
>> > space to use for pack mappings.  Least recently used window gets
>> > tossed when we need a different window.  This permits us to keep
>> > say a window active on the front of a pack (near the commits) and
>> > another different active window closer to the back (near the blobs).
>> 
>> Sounds good. That is exactly what I was expecting it to be done.
>
> So I have this implemented but its against the 32 bit index.
> It passes the full test suite and appears to be working as intended.

Well, if you are talking about git.git t/ directory, I bet that
"the full test suite" easily fits in a single window so much of
the new code is not exercised, unless you have a runtime option
to make use of very tiny window (32kB would be nice) for testing
;-).

> I am going to rebase the changes to the 64 bit index in `pu`
> and clean up my history.  Its currently one massive commit with
> lots of changes that should be broken down into slightly more
> digestable chunks.

Breaking things up would be nice.  Do you have a good testcase
out of Mozilla repository that _needs_ 64-bit index?  The code
in "pu" uses 64-bit index only 32-bit offset cannot describe the
pack, so I only tested with an artificial case with the kernel
archive with .idx converted with convert-idx to 64-bit.

^ permalink raw reply

* Re: [PATCH] Include local config before platform tweaks
From: Junio C Hamano @ 2006-09-06  7:18 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: git, Shawn Pearce
In-Reply-To: <20060906033151.GB30540@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Brian Gernhardt <benji@silverinsanity.com> writes:
>> 
>> > Having config.mak included after the platform tweaks ignores NO_FINK
>> > or NO_DARWIN_PORTS in that file.  Simply including the config earlier
>> > fixes that.
>> 
>> I vaguely recall that this was brought up before, and the
>> conclusion was that the include location is correct but the way
>> darwin bits were done was wrong.  I do not recall the details
>> but does anybody on the list know?
>
> I think we just need to move the NO_FINK stuff below the include;
> like this:

Thanks Shawn.  Brian does Shawn's patch work for you?

^ permalink raw reply

* Re: [PATCH 3/5] autoconf: Preliminary check for working mmap
From: Junio C Hamano @ 2006-09-06  7:17 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060906034054.GB30598@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Shawn Pearce <spearce@spearce.org> writes:
>> 
>> > I don't know if I've made this more complex than I really need to
>> > but I've permitted multiple windows per pack.  There is just one
>> > LRU of all windows across all packs and a maximum amount of address
>> > space to use for pack mappings.  Least recently used window gets
>> > tossed when we need a different window.  This permits us to keep
>> > say a window active on the front of a pack (near the commits) and
>> > another different active window closer to the back (near the blobs).
>> 
>> Sounds good. That is exactly what I was expecting it to be done.
>
> So I have this implemented but its against the 32 bit index.
> It passes the full test suite and appears to be working as intended.
>
> I am going to rebase the changes to the 64 bit index in `pu`
> and clean up my history.  Its currently one massive commit with
> lots of changes that should be broken down into slightly more
> digestable chunks.
>
> I'll try to do that tonight and get a patch series out.  My email
> is currently proving to be unstable so you may not get the series
> until later on Wed.

Take your time.  My usual git day is Wednesday but this week it
is shot and I won't be doing much gitting until later this week.

^ permalink raw reply

* Re: [PATCH 3/5] autoconf: Preliminary check for working mmap
From: Shawn Pearce @ 2006-09-06  3:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wqqkbbv.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > I don't know if I've made this more complex than I really need to
> > but I've permitted multiple windows per pack.  There is just one
> > LRU of all windows across all packs and a maximum amount of address
> > space to use for pack mappings.  Least recently used window gets
> > tossed when we need a different window.  This permits us to keep
> > say a window active on the front of a pack (near the commits) and
> > another different active window closer to the back (near the blobs).
> 
> Sounds good. That is exactly what I was expecting it to be done.

So I have this implemented but its against the 32 bit index.
It passes the full test suite and appears to be working as intended.

I am going to rebase the changes to the 64 bit index in `pu`
and clean up my history.  Its currently one massive commit with
lots of changes that should be broken down into slightly more
digestable chunks.

I'll try to do that tonight and get a patch series out.  My email
is currently proving to be unstable so you may not get the series
until later on Wed.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Include local config before platform tweaks
From: Shawn Pearce @ 2006-09-06  3:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brian Gernhardt, git
In-Reply-To: <7vlkoyarnx.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Brian Gernhardt <benji@silverinsanity.com> writes:
> 
> > Having config.mak included after the platform tweaks ignores NO_FINK
> > or NO_DARWIN_PORTS in that file.  Simply including the config earlier
> > fixes that.
> 
> I vaguely recall that this was brought up before, and the
> conclusion was that the include location is correct but the way
> darwin bits were done was wrong.  I do not recall the details
> but does anybody on the list know?

I think we just need to move the NO_FINK stuff below the include;
like this:


diff --git a/Makefile b/Makefile
index 164dbcf..748907b 100644
--- a/Makefile
+++ b/Makefile
@@ -328,18 +333,6 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_STRLCPY = YesPlease
-	ifndef NO_FINK
-		ifeq ($(shell test -d /sw/lib && echo y),y)
-			BASIC_CFLAGS += -I/sw/include
-			BASIC_LDFLAGS += -L/sw/lib
-		endif
-	endif
-	ifndef NO_DARWIN_PORTS
-		ifeq ($(shell test -d /opt/local/lib && echo y),y)
-			BASIC_CFLAGS += -I/opt/local/include
-			BASIC_LDFLAGS += -L/opt/local/lib
-		endif
-	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -433,6 +426,21 @@ else
 	endif
 endif
 
+ifeq ($(uname_S),Darwin)
+	ifndef NO_FINK
+		ifeq ($(shell test -d /sw/lib && echo y),y)
+			BASIC_CFLAGS += -I/sw/include
+			BASIC_LDFLAGS += -L/sw/lib
+		endif
+	endif
+	ifndef NO_DARWIN_PORTS
+		ifeq ($(shell test -d /opt/local/lib && echo y),y)
+			BASIC_CFLAGS += -I/opt/local/include
+			BASIC_LDFLAGS += -L/opt/local/lib
+		endif
+	endif
+endif
+
 ifndef NO_CURL
 	ifdef CURLDIR
 		# This is still problematic -- gcc does not always want -R.

^ permalink raw reply related

* [PATCH 1/2] revision.c: allow injecting revision parameters after setup_revisions().
From: Junio C Hamano @ 2006-09-06  4:47 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git

setup_revisions() wants to get all the parameters at once and
then postprocesses the resulting revs structure after it is done
with them.  This code structure is a bit cumbersome to deal with
efficiently when we want to inject revision parameters from the
side (e.g. read from standard input).

Fortunately, the nature of this postprocessing is not affected by
revision parameters; they are affected only by flags.  So it is
Ok to do add_object() after the it returns.

This splits out the code that deals with the revision parameter
out of the main loop of setup_revisions(), so that we can later
call it from elsewhere after it returns.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c |  159 ++++++++++++++++++++++++++++++++----------------------------
 revision.h |    2 +
 2 files changed, 87 insertions(+), 74 deletions(-)

diff --git a/revision.c b/revision.c
index b588f74..db01682 100644
--- a/revision.c
+++ b/revision.c
@@ -592,6 +592,85 @@ static void prepare_show_merge(struct re
 	revs->prune_data = prune;
 }
 
+int handle_revision_arg(const char *arg, struct rev_info *revs,
+			int flags,
+			int cant_be_filename)
+{
+	char *dotdot;
+	struct object *object;
+	unsigned char sha1[20];
+	int local_flags;
+
+	dotdot = strstr(arg, "..");
+	if (dotdot) {
+		unsigned char from_sha1[20];
+		const char *next = dotdot + 2;
+		const char *this = arg;
+		int symmetric = *next == '.';
+		unsigned int flags_exclude = flags ^ UNINTERESTING;
+
+		*dotdot = 0;
+		next += symmetric;
+
+		if (!*next)
+			next = "HEAD";
+		if (dotdot == arg)
+			this = "HEAD";
+		if (!get_sha1(this, from_sha1) &&
+		    !get_sha1(next, sha1)) {
+			struct commit *a, *b;
+			struct commit_list *exclude;
+
+			a = lookup_commit_reference(from_sha1);
+			b = lookup_commit_reference(sha1);
+			if (!a || !b) {
+				die(symmetric ?
+				    "Invalid symmetric difference expression %s...%s" :
+				    "Invalid revision range %s..%s",
+				    arg, next);
+			}
+
+			if (!cant_be_filename) {
+				*dotdot = '.';
+				verify_non_filename(revs->prefix, arg);
+			}
+
+			if (symmetric) {
+				exclude = get_merge_bases(a, b, 1);
+				add_pending_commit_list(revs, exclude,
+							flags_exclude);
+				free_commit_list(exclude);
+				a->object.flags |= flags;
+			} else
+				a->object.flags |= flags_exclude;
+			b->object.flags |= flags;
+			add_pending_object(revs, &a->object, this);
+			add_pending_object(revs, &b->object, next);
+			return 0;
+		}
+		*dotdot = '.';
+	}
+	dotdot = strstr(arg, "^@");
+	if (dotdot && !dotdot[2]) {
+		*dotdot = 0;
+		if (add_parents_only(revs, arg, flags))
+			return 0;
+		*dotdot = '^';
+	}
+	local_flags = 0;
+	if (*arg == '^') {
+		local_flags = UNINTERESTING;
+		arg++;
+	}
+	if (get_sha1(arg, sha1))
+		return -1;
+	if (!cant_be_filename)
+		verify_non_filename(revs->prefix, arg);
+	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	add_pending_object(revs, object, arg);
+	return 0;
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
@@ -620,12 +699,7 @@ int setup_revisions(int argc, const char
 
 	flags = show_merge = 0;
 	for (i = 1; i < argc; i++) {
-		struct object *object;
 		const char *arg = argv[i];
-		unsigned char sha1[20];
-		char *dotdot;
-		int local_flags;
-
 		if (*arg == '-') {
 			int opts;
 			if (!strncmp(arg, "--max-count=", 12)) {
@@ -830,71 +904,10 @@ int setup_revisions(int argc, const char
 			left++;
 			continue;
 		}
-		dotdot = strstr(arg, "..");
-		if (dotdot) {
-			unsigned char from_sha1[20];
-			const char *next = dotdot + 2;
-			const char *this = arg;
-			int symmetric = *next == '.';
-			unsigned int flags_exclude = flags ^ UNINTERESTING;
-
-			*dotdot = 0;
-			next += symmetric;
-
-			if (!*next)
-				next = "HEAD";
-			if (dotdot == arg)
-				this = "HEAD";
-			if (!get_sha1(this, from_sha1) &&
-			    !get_sha1(next, sha1)) {
-				struct commit *a, *b;
-				struct commit_list *exclude;
-
-				a = lookup_commit_reference(from_sha1);
-				b = lookup_commit_reference(sha1);
-				if (!a || !b) {
-					die(symmetric ?
-					    "Invalid symmetric difference expression %s...%s" :
-					    "Invalid revision range %s..%s",
-					    arg, next);
-				}
-
-				if (!seen_dashdash) {
-					*dotdot = '.';
-					verify_non_filename(revs->prefix, arg);
-				}
-
-				if (symmetric) {
-					exclude = get_merge_bases(a, b, 1);
-					add_pending_commit_list(revs, exclude,
-					                        flags_exclude);
-					free_commit_list(exclude);
-					a->object.flags |= flags;
-				} else
-					a->object.flags |= flags_exclude;
-				b->object.flags |= flags;
-				add_pending_object(revs, &a->object, this);
-				add_pending_object(revs, &b->object, next);
-				continue;
-			}
-			*dotdot = '.';
-		}
-		dotdot = strstr(arg, "^@");
-		if (dotdot && !dotdot[2]) {
-			*dotdot = 0;
-			if (add_parents_only(revs, arg, flags))
-				continue;
-			*dotdot = '^';
-		}
-		local_flags = 0;
-		if (*arg == '^') {
-			local_flags = UNINTERESTING;
-			arg++;
-		}
-		if (get_sha1(arg, sha1)) {
-			int j;
 
-			if (seen_dashdash || local_flags)
+		if (handle_revision_arg(arg, revs, flags, seen_dashdash)) {
+			int j;
+			if (seen_dashdash || *arg == '^')
 				die("bad revision '%s'", arg);
 
 			/* If we didn't have a "--":
@@ -906,14 +919,12 @@ int setup_revisions(int argc, const char
 			for (j = i; j < argc; j++)
 				verify_filename(revs->prefix, argv[j]);
 
-			revs->prune_data = get_pathspec(revs->prefix, argv + i);
+			revs->prune_data = get_pathspec(revs->prefix,
+							argv + i);
 			break;
 		}
-		if (!seen_dashdash)
-			verify_non_filename(revs->prefix, arg);
-		object = get_reference(revs, arg, sha1, flags ^ local_flags);
-		add_pending_object(revs, object, arg);
 	}
+
 	if (show_merge)
 		prepare_show_merge(revs);
 	if (def && !revs->pending.nr) {
diff --git a/revision.h b/revision.h
index d289781..c1f71af 100644
--- a/revision.h
+++ b/revision.h
@@ -90,6 +90,8 @@ extern int rev_compare_tree(struct rev_i
 
 extern void init_revisions(struct rev_info *revs, const char *prefix);
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
+
 extern void prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
 
-- 
1.4.2.g980c3

^ permalink raw reply related

* [PATCH 2/2] Teach rev-list an option to read revs from the standard input.
From: Junio C Hamano @ 2006-09-06  4:47 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git

When --stdin option is given, in addition to the <rev>s listed
on the command line, the command can read one rev parameter per
line from the standard input.  The list of revs ends at the
first empty line or EOF.

Note that you still have to give all the flags from the command
line; only rev arguments (including A..B, A...B, and A^@ notations)
can be give from the standard input.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-rev-list.txt |    6 ++++++
 builtin-rev-list.c             |   25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 3c4c2fb..28966ad 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 	     [ \--remove-empty ]
 	     [ \--not ]
 	     [ \--all ]
+	     [ \--stdin ]
 	     [ \--topo-order ]
 	     [ \--parents ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
@@ -171,6 +172,11 @@ limiting may be applied.
 	Pretend as if all the refs in `$GIT_DIR/refs/` are listed on the
 	command line as '<commit>'.
 
+--stdin::
+
+	In addition to the '<commit>' listed on the command
+	line, read them from the standard input.
+
 --merge::
 
 	After a failed merge, show refs that touch files having a
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8437454..8fe8afb 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -23,6 +23,7 @@ static const char rev_list_usage[] =
 "    --no-merges\n"
 "    --remove-empty\n"
 "    --all\n"
+"    --stdin\n"
 "  ordering output:\n"
 "    --topo-order\n"
 "    --date-order\n"
@@ -304,10 +305,28 @@ static void mark_edges_uninteresting(str
 	}
 }
 
+static void read_revisions_from_stdin(struct rev_info *revs)
+{
+	char line[1000];
+
+	while (fgets(line, sizeof(line), stdin) != NULL) {
+		int len = strlen(line);
+		if (line[len - 1] == '\n')
+			line[--len] = 0;
+		if (!len)
+			break;
+		if (line[0] == '-')
+			die("options not supported in --stdin mode");
+		if (handle_revision_arg(line, revs, 0, 1))
+			die("bad revision '%s'", line);
+	}
+}
+
 int cmd_rev_list(int argc, const char **argv, const char *prefix)
 {
 	struct commit_list *list;
 	int i;
+	int read_from_stdin = 0;
 
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
@@ -329,6 +348,12 @@ int cmd_rev_list(int argc, const char **
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--stdin")) {
+			if (read_from_stdin++)
+				die("--stdin given twice?");
+			read_revisions_from_stdin(&revs);
+			continue;
+		}
 		usage(rev_list_usage);
 
 	}
-- 
1.4.2.g980c3

^ permalink raw reply related

* Re: [PATCH 1/2] rev list add option accepting revision constraints on standard input
From: Junio C Hamano @ 2006-09-06  4:46 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <7vpseaarrl.fsf@assigned-by-dhcp.cox.net>

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

> Is calling setup_revisions() on the same revs like this many
> times safe?  I do not think so, especially what is after the
> primary "for()" loop in the function.
>
> I was sort-of expecting that you would instead replace that
> primary for() loop in setup_revisions() with some sort of
> callback...

I take half of the above back.  Even after setup_revisions()
returns, adding more revisions via add_pending_object() is
safe.  However, the postprocessing done in setup_revisions()
after its main loop, while I do not think they would crash when
called twice, would be very wasteful.

And ``callback'' interface is usually very cumbersome to use, so
we probably would want to avoid it unless absolutely necessary.

I've outlined an alternative implementation in two patches;
I'll be sending them out shortly.

^ permalink raw reply

* Re: [PATCH 1/2] rev list add option accepting revision constraints on standard input
From: Andy Whitcroft @ 2006-09-06  1:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpseaarrl.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> Add a --stdin flag which causes rev-list to additionally read
>> its stdin stream and parse that for revision constraints.
> 
>> +/*
>> + * Parse revision information, filling in the "rev_info" structure,
>> + * revisions are taken from stream.
>> + */
>> +static void setup_revisions_stream(FILE *stream, struct rev_info *revs)
>> +{
>> +	char line[1000];
>> +	const char *args[] = { 0, line, 0 };
>> +
>> +	while (fgets(line, sizeof(line), stream) != NULL) {
>> +		line[strlen(line) - 1] = 0;
>> +
>> +		if (line[0] == '-')
>> +			die("options not supported in --stdin mode");
>> +
>> +		(void)setup_revisions(2, args, revs, NULL);
>> +	}
>> +}
> 
> Is calling setup_revisions() on the same revs like this many
> times safe?  I do not think so, especially what is after the
> primary "for()" loop in the function.
> 
> I was sort-of expecting that you would instead replace that
> primary for() loop in setup_revisions() with some sort of
> callback...

Heh, well I'll give it another poke tommorrow my time ...

Thanks for the feedback.  Its hard to get any context on something new
without trying and failing :).

-apw

^ permalink raw reply

* stgit bisect feature request
From: Torgil Svensson @ 2006-09-05 23:49 UTC (permalink / raw)
  To: git

I think i'd be nice if stgit wraps bisect and you could write
$ stg bisect start
$ stg bisect bad
$ stg bisect good ORIG_HEAD

and after bisect checks out the files it pushes the current stack as
if you had used stg pull at this point

//Torgil

^ permalink raw reply

* Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines
From: Junio C Hamano @ 2006-09-05 23:26 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: Matthias Urlichs, git
In-Reply-To: <20060905221754.GI14732@sashak.voltaire.com>

Sasha Khapyorsky <sashak@voltaire.com> writes:

> BTW, what about to importing subdirectories, like this:
>
>  <trunk>/path/to/subdir
>  <branches>/path/to/subdir
>
> Is this could be improvement?

I somehow had an impression that svnimport dealt with the
reversed layout already, although $project/{trunk,branches,tags}
layout is assumed by default; maybe I was mistaken.

If the tool can automatically detect the layout the remote
project employs, and adjust the default accordingly, I would
imagine that would be a useful addition.

^ permalink raw reply

* Re: [PATCH] Include local config before platform tweaks
From: Junio C Hamano @ 2006-09-05 22:13 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: git
In-Reply-To: <9434EEBD-57BE-46D7-A2FF-069BB960AA44@silverinsanity.com>

Brian Gernhardt <benji@silverinsanity.com> writes:

> Having config.mak included after the platform tweaks ignores NO_FINK
> or NO_DARWIN_PORTS in that file.  Simply including the config earlier
> fixes that.

I vaguely recall that this was brought up before, and the
conclusion was that the include location is correct but the way
darwin bits were done was wrong.  I do not recall the details
but does anybody on the list know?

^ permalink raw reply

* Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines
From: Sasha Khapyorsky @ 2006-09-05 22:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthias Urlichs, git
In-Reply-To: <7v1wqqc8dh.fsf@assigned-by-dhcp.cox.net>

On 14:26 Tue 05 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> > Hi,
> >
> > This feature was useful with importing https://openib.org/svn/gen2 .
> >
> > Sasha
> >
> > This add '-S' option. When specified svn-import will try to parse
> > commit message for 'Signed-off-by: ...' line, and if found will use
> > the name and email address extracted at first occurrence as this commit
> > author name and author email address. Committer name and email are
> > extracted in usual way.
> >
> > Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
> 
> Thanks.
> 
> I do not think the first signed-off-by is necessarily the author
> of the change, so we are risking miscrediting (or misblaming) a
> wrong person. 

Right, there is such risk, so this feature is optional and by default is
"off".

> Having said that, using the committer information
> has the same miscredit problem, so this change might be Ok, but
> I am not sure if it adds much improvement.
> 
> I will wait for an ack/nack from somebody who use svnimport and
> know it well.

Sure.

BTW, what about to importing subdirectories, like this:

 <trunk>/path/to/subdir
 <branches>/path/to/subdir

Is this could be improvement?

> 
> BTW, I do not appreciate the first six lines of your message
> being _before_ the proposed commit log message.  Please have it
> between "---\n" (that comes immediately after your own
> "Signed-off-by:") and the diffstat, like this:
> 
>         This add '-S' option. When specified svn-import will try to...
>         ... in usual way.
> 
>         Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
>         ---
> 
>           Hi,
> 
>           This feature was useful with importing https://openib.org/svn/gen2 .
> 
>           Sasha
> 
>          git-svnimport.perl |   31 ++++++++++++++++++++-----------
>          1 files changed, 20 insertions(+), 11 deletions(-)
> 
>          diff --git a/...

Ok.

Sasha

^ permalink raw reply

* Re: [PATCH 1/2] rev list add option accepting revision constraints on standard input
From: Junio C Hamano @ 2006-09-05 22:10 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <20060905215157.GA29172@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

> Add a --stdin flag which causes rev-list to additionally read
> its stdin stream and parse that for revision constraints.

> +/*
> + * Parse revision information, filling in the "rev_info" structure,
> + * revisions are taken from stream.
> + */
> +static void setup_revisions_stream(FILE *stream, struct rev_info *revs)
> +{
> +	char line[1000];
> +	const char *args[] = { 0, line, 0 };
> +
> +	while (fgets(line, sizeof(line), stream) != NULL) {
> +		line[strlen(line) - 1] = 0;
> +
> +		if (line[0] == '-')
> +			die("options not supported in --stdin mode");
> +
> +		(void)setup_revisions(2, args, revs, NULL);
> +	}
> +}

Is calling setup_revisions() on the same revs like this many
times safe?  I do not think so, especially what is after the
primary "for()" loop in the function.

I was sort-of expecting that you would instead replace that
primary for() loop in setup_revisions() with some sort of
callback...

^ permalink raw reply

* [PATCH] Include local config before platform tweaks
From: Brian Gernhardt @ 2006-09-05 22:00 UTC (permalink / raw)
  To: git

Having config.mak included after the platform tweaks ignores NO_FINK  
or NO_DARWIN_PORTS in that file.  Simply including the config earlier  
fixes that.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
Makefile |    6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7b3114f..81902d4 100644
--- a/Makefile
+++ b/Makefile
@@ -300,6 +300,9 @@ BUILTIN_OBJS = \
GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
LIBS = $(GITLIBS) -lz
+-include config.mak.autogen
+-include config.mak
+
#
# Platform specific tweaks
#
@@ -403,9 +406,6 @@ ifneq (,$(findstring arm,$(uname_M)))
	ARM_SHA1 = YesPlease
endif
--include config.mak.autogen
--include config.mak
-
ifdef WITH_OWN_SUBPROCESS_PY
	PYMODULES += compat/subprocess.py
else

^ permalink raw reply related

* Some issues with current qgit on exit
From: Pavel Roskin @ 2006-09-05 22:00 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git

Hi, Marco!

The current qgit reports assertion failure on exit:
ASSERT in tabWdg_currentChanged

That's how you can trigger it:

select one of the patches
double click on the entry to open the "patch" tab
double click on one of the modified files on the right
switch back to the "rev list" or "patch" tab
close qgit by Ctrl-Q

I've tried to debug the problem.  MainImpl::currentTabType() returns -1.
It's possible that the logic in that function is flawed, or maybe it
doesn't work properly on exit.

Another problem is segmentation fault when closing qgit with more that
one file tab and the "rev list" tab selected:

select one of the patches
select "view file" in the pop-up menu on one of the files
activate the "rev list" tab
select "view file in new tab" on the same file
activate the "rev list" tab
close qgit by Ctrl-Q

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 46998824734480 (LWP 3112)]
0x0000001400040038 in ?? ()
(gdb) where
#0  0x0000001400040038 in ?? ()
#1  0x000000000046c336 in MainImpl::tabWdg_currentChanged (this=0x794490, w=0x12098d0)
    at mainimpl.cpp:788
#2  0x000000000048b33b in MainBase::qt_invoke (this=0x794490, _id=93, _o=0x7fffe87794a0)
    at mainbase.moc.cc:240
#3  0x0000000000497f19 in MainImpl::qt_invoke (this=0x794490, _id=93, _o=0x7fffe87794a0)
    at mainimpl.moc.cc:350
#4  0x00002abec28a8929 in QObject::activate_signal () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#5  0x00002abec2bf77e4 in QTabWidget::currentChanged ()
   from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#6  0x00002abec2bf7741 in QTabWidget::qt_invoke () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#7  0x00002abec28a8929 in QObject::activate_signal () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#8  0x00002abec28a9578 in QObject::activate_signal () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#9  0x00002abec29cd85d in QTabBar::setTabEnabled () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#10 0x00002abec29d0bde in QTabWidget::removePage () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#11 0x00002abec29cffcc in QTabWidget::eventFilter () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#12 0x00002abec28a7fc2 in QObject::activate_filters () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#13 0x00002abec28a8037 in QObject::event () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#14 0x00002abec28dfaf8 in QWidget::event () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#15 0x00002abec2847da5 in QApplication::internalNotify ()
   from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#16 0x00002abec2849274 in QApplication::notify () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#17 0x00002abec28a97b8 in QObject::removeChild () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#18 0x00002abec28aa898 in QObject::~QObject$base () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#19 0x00002abec28e2720 in QWidget::~QWidget$base () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#20 0x00000000004a8a91 in ~TabFile (this=0xac9e40) at filebase.uic.cc:513
#21 0x0000000000440b14 in ~FileView (this=0xac9b80) at fileview.cpp:99
#22 0x00002abec28a8089 in QObject::event () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#23 0x00002abec2847da5 in QApplication::internalNotify ()
#24 0x00002abec2849274 in QApplication::notify () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#25 0x00002abec2848da9 in QApplication::sendPostedEvents ()
   from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#26 0x00002abec285f5fa in QEventLoop::enterLoop () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#27 0x00002abec285f442 in QEventLoop::exec () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#28 0x000000000047d134 in main (argc=1, argv=0x7fffe8779df8) at qgit.cpp:17


All this is observed on the current development Fedora Core 6 with Qt 
3.3.6 and gcc 4.1.1, architecture is x86_64.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* [PATCH 2/2] send pack switch to using git rev list stdin
From: Andy Whitcroft @ 2006-09-05 21:52 UTC (permalink / raw)
  To: git
In-Reply-To: <44FDECD1.2090909@shadowen.org>

send-pack: switch to using git-rev-list --stdin

When we are generating packs to update remote repositories we
want to supply as much information as possible about the revisions
that already exist to rev-list in order optimise the pack as much
as possible.  We need to pass two revisions for each branch we are
updating in the remote repository and one for each additional branch.
Where the remote repository has numerous branches we can run out
of command line space to pass them.

Utilise the git-rev-list --stdin mode to allow unlimited numbers
of revision constraints.  This allows us to move back to the much
simpler unordered revision selection code.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/send-pack.c b/send-pack.c
index ac4501d..f6e5eed 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -38,9 +38,8 @@ static void exec_pack_objects(void)
 
 static void exec_rev_list(struct ref *refs)
 {
-	struct ref *ref;
-	static const char *args[1000];
-	int i = 0, j;
+	static const char *args[4];
+	int i = 0;
 
 	args[i++] = "rev-list";	/* 0 */
 	if (use_thin_pack)	/* 1 */
@@ -48,43 +47,29 @@ static void exec_rev_list(struct ref *re
 	else
 		args[i++] = "--objects";
 
-	/* First send the ones we care about most */
-	for (ref = refs; ref; ref = ref->next) {
-		if (900 < i)
-			die("git-rev-list environment overflow");
-		if (!is_zero_sha1(ref->new_sha1)) {
-			char *buf = xmalloc(100);
-			args[i++] = buf;
-			snprintf(buf, 50, "%s", sha1_to_hex(ref->new_sha1));
-			buf += 50;
-			if (!is_zero_sha1(ref->old_sha1) &&
-			    has_sha1_file(ref->old_sha1)) {
-				args[i++] = buf;
-				snprintf(buf, 50, "^%s",
-					 sha1_to_hex(ref->old_sha1));
-			}
-		}
-	}
+	args[i++] = "--stdin";
 
-	/* Then a handful of the remainder
-	 * NEEDSWORK: we would be better off if used the newer ones first.
-	 */
-	for (ref = refs, j = i + 16;
-	     i < 900 && i < j && ref;
-	     ref = ref->next) {
-		if (is_zero_sha1(ref->new_sha1) &&
-		    !is_zero_sha1(ref->old_sha1) &&
-		    has_sha1_file(ref->old_sha1)) {
-			char *buf = xmalloc(42);
-			args[i++] = buf;
-			snprintf(buf, 42, "^%s", sha1_to_hex(ref->old_sha1));
-		}
-	}
 	args[i] = NULL;
 	execv_git_cmd(args);
 	die("git-rev-list exec failed (%s)", strerror(errno));
 }
 
+static void builtin_rev_list_generate(struct ref *refs)
+{
+        while (refs) {
+                if (!is_zero_sha1(refs->old_sha1) &&
+                    has_sha1_file(refs->old_sha1)) {
+                        printf("^%s", sha1_to_hex(refs->old_sha1));
+                }
+                if (!is_zero_sha1(refs->new_sha1)) {
+                        printf("%s", sha1_to_hex(refs->new_sha1));
+                }
+                refs = refs->next;
+        }
+
+	exit(0);
+}
+
 static void rev_list(int fd, struct ref *refs)
 {
 	int pipe_fd[2];
@@ -111,13 +96,38 @@ static void rev_list(int fd, struct ref 
 	exec_rev_list(refs);
 }
 
+static void rev_list_generate(int fd, struct ref *refs)
+{
+	int pipe_fd[2];
+	pid_t rev_list_generate_pid;
+
+	if (pipe(pipe_fd) < 0)
+		die("rev-list-generate setup: pipe failed");
+	rev_list_generate_pid = fork();
+	if (!rev_list_generate_pid) {
+		dup2(pipe_fd[0], 0);
+		dup2(fd, 1);
+		close(pipe_fd[0]);
+		close(pipe_fd[1]);
+		close(fd);
+		rev_list(fd, refs);
+		die("rev-list setup failed");
+	}
+	if (rev_list_generate_pid < 0)
+		die("rev-list-generate fork failed");
+	dup2(pipe_fd[1], 1);
+	close(pipe_fd[0]);
+	close(pipe_fd[1]);
+	close(fd);
+	builtin_rev_list_generate(refs);
+}
 static void pack_objects(int fd, struct ref *refs)
 {
 	pid_t rev_list_pid;
 
 	rev_list_pid = fork();
 	if (!rev_list_pid) {
-		rev_list(fd, refs);
+		rev_list_generate(fd, refs);
 		die("rev-list setup failed");
 	}
 	if (rev_list_pid < 0)

^ permalink raw reply related

* [PATCH 1/2] rev list add option accepting revision constraints on standard input
From: Andy Whitcroft @ 2006-09-05 21:51 UTC (permalink / raw)
  To: git
In-Reply-To: <44FDECD1.2090909@shadowen.org>

rev-list: add option accepting revision constraints on standard input

When we are generating packs to update remote repositories we
want to supply as much information as possible about the revisions
that already exist to rev-list in order optimise the pack as much
as possible.  We need to pass two revisions for each branch we are
updating in the remote repository and one for each additional branch.
Where the remote repository has numerous branches we can run out
of command line space to pass them.

Add a --stdin flag which causes rev-list to additionally read
its stdin stream and parse that for revision constraints.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8437454..0303909 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -304,10 +304,30 @@ static void mark_edges_uninteresting(str
 	}
 }
 
+/*
+ * Parse revision information, filling in the "rev_info" structure,
+ * revisions are taken from stream.
+ */
+static void setup_revisions_stream(FILE *stream, struct rev_info *revs)
+{
+	char line[1000];
+	const char *args[] = { 0, line, 0 };
+
+	while (fgets(line, sizeof(line), stream) != NULL) {
+		line[strlen(line) - 1] = 0;
+
+		if (line[0] == '-')
+			die("options not supported in --stdin mode");
+
+		(void)setup_revisions(2, args, revs, NULL);
+	}
+}
+
 int cmd_rev_list(int argc, const char **argv, const char *prefix)
 {
 	struct commit_list *list;
 	int i;
+	int read_stdin = 0;
 
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
@@ -329,9 +349,15 @@ int cmd_rev_list(int argc, const char **
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--stdin")) {
+			read_stdin = 1;
+			continue;
+		}
 		usage(rev_list_usage);
-
 	}
+	if (read_stdin)
+		setup_revisions_stream(stdin, &revs);
+
 	if (revs.commit_format != CMIT_FMT_UNSPECIFIED) {
 		/* The command line has a --pretty  */
 		hdr_termination = '\n';

^ permalink raw reply related

* Re: send-pack: limit on negative references
From: Andy Whitcroft @ 2006-09-05 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwt8igu4u.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> I've been having trouble with git push apparently resending the entire
>> commit trace for the branch each and every time I push; even with short
>> branch names.  This seems to be related to the changes made to handle
>> the case where the remote end has a large number of branches (>900).
> 
> I think the right fix is to do one or both of the following, and
> lift that 900 cut-off entirely.
> 
> One is to teach rev-list to read the information it is taking
> from the command line instead from its standard input.
> 
> Another is to teach pack-object the same trick on top of my
> patch last night.  This has an added benefit that we save one
> pipe+fork+exec there.
> 
> These are essentially suggestions from Linus made twice
> separately in the past, so they must be on the right track ;-).
> 
> If nobody does, I would do it myself, but the list is welcome to
> beat me to it.  Especially, the former (giving --stdin option to
> rev-list) should be trivial.

Ok, I'll have a go at this one.  As you say it looks pretty simple to
reuse the command line revision parser on an incoming stream.

-apw

^ permalink raw reply

* Re: [PATCH 2/5] autoconf: Add -liconv to LIBS when NEEDS_LIBICONV
From: Junio C Hamano @ 2006-09-05 21:27 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060905162526.GA5547@diku.dk>

Jonas Fonseca <fonseca@diku.dk> writes:

> Jakub Narebski <jnareb@gmail.com> wrote Tue, Sep 05, 2006:
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> Just in case; it could matter for testing if iconv is properly
>> supported (NO_ICONV test).
>> 
>>  configure.ac |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 36f9cd9..fc5b813 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -147,6 +147,7 @@ AC_CHECK_LIB([c], [iconv],
>>  [NEEDS_LIBICONV=],
>>  [NEEDS_LIBICONV=YesPlease])
>>  AC_SUBST(NEEDS_LIBICONV)
>> +test -n "$NEEDS_SOCKET" && LIBS="$LIBS -liconv"
>
> I see that this has entered already, but it looks like it needs a
> s/NEEDS_SOCKET/NEEDS_LIBICONV/

Eek, eye-ballo-o.  Thanks for noticing.

^ permalink raw reply

* Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines
From: Junio C Hamano @ 2006-09-05 21:26 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: Matthias Urlichs, git
In-Reply-To: <20060905184611.GB14732@sashak.voltaire.com>

Sasha Khapyorsky <sashak@voltaire.com> writes:

> Hi,
>
> This feature was useful with importing https://openib.org/svn/gen2 .
>
> Sasha
>
> This add '-S' option. When specified svn-import will try to parse
> commit message for 'Signed-off-by: ...' line, and if found will use
> the name and email address extracted at first occurrence as this commit
> author name and author email address. Committer name and email are
> extracted in usual way.
>
> Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>

Thanks.

I do not think the first signed-off-by is necessarily the author
of the change, so we are risking miscrediting (or misblaming) a
wrong person.  Having said that, using the committer information
has the same miscredit problem, so this change might be Ok, but
I am not sure if it adds much improvement.

I will wait for an ack/nack from somebody who use svnimport and
know it well.

BTW, I do not appreciate the first six lines of your message
being _before_ the proposed commit log message.  Please have it
between "---\n" (that comes immediately after your own
"Signed-off-by:") and the diffstat, like this:

        This add '-S' option. When specified svn-import will try to...
        ... in usual way.

        Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
        ---

          Hi,

          This feature was useful with importing https://openib.org/svn/gen2 .

          Sasha

         git-svnimport.perl |   31 ++++++++++++++++++++-----------
         1 files changed, 20 insertions(+), 11 deletions(-)

         diff --git a/...

^ permalink raw reply

* Re: Running gitweb under mod_perl
From: Jakub Narebski @ 2006-09-05 20:32 UTC (permalink / raw)
  To: git
In-Reply-To: <20060824140525.G638085b@leonov.stosberg.net>

Dennis Stosberg wrote:

> Jakub Narebski wrote:
> 
>> What should I put in Apache configuration (Apache 2.0.54 if this
>> matters, mod_perl 2.0.1) 
> 
> From my configuration:
> 
>   <Directory /home/dennis/public_html/perl>
>     Options -Indexes +ExecCGI
>     AllowOverride None
>     PerlSendHeader On
>     SetHandler perl-script
>     PerlHandler ModPerl::Registry
>   </Directory>

I use mod_perl 2.0 version

   Alias /perl /var/www/perl
   <Directory /var/www/perl>
       SetHandler perl-script
       PerlResponseHandler ModPerl::Registry
       PerlOptions +ParseHeaders
       Options +ExecCGI
   </Directory>

What is strange that ApacheBench is showing that mod_perl is _slower_ than
CGI version: 3003.305 [ms] (mean) CGI vs 3500.589 [ms] (mean) mod_perl
for summary page for git.git repository (my copy that is).

I wonder if I misconfigured something...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ 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