Git development
 help / color / mirror / Atom feed
* [PATCH] dir: do all size checks before seeking back and fix file closing
From: Jonas Fonseca @ 2006-08-26 14:17 UTC (permalink / raw)
  To: git

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 dir.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index d53d48f..ff8a2fb 100644
--- a/dir.c
+++ b/dir.c
@@ -122,11 +122,11 @@ static int add_excludes_from_file_1(cons
 	size = lseek(fd, 0, SEEK_END);
 	if (size < 0)
 		goto err;
-	lseek(fd, 0, SEEK_SET);
 	if (size == 0) {
 		close(fd);
 		return 0;
 	}
+	lseek(fd, 0, SEEK_SET);
 	buf = xmalloc(size+1);
 	if (read(fd, buf, size) != size)
 		goto err;
@@ -146,7 +146,7 @@ static int add_excludes_from_file_1(cons
 	return 0;
 
  err:
-	if (0 <= fd)
+	if (0 >= fd)
 		close(fd);
 	return -1;
 }
-- 
1.4.2.GIT

-- 
Jonas Fonseca

^ permalink raw reply related

* [PATCH] Use xrealloc instead of realloc
From: Jonas Fonseca @ 2006-08-26 14:16 UTC (permalink / raw)
  To: git

Change places that use realloc, without a proper error path, to instead use
xrealloc. Drop an erroneous error path in the daemon code that used errno
in the die message in favour of the simpler xrealloc.

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

If this sort of cleanup is desired, I can make one for malloc -> xmalloc
too.

 builtin-fmt-merge-msg.c |    4 ++--
 builtin-log.c           |    4 ++--
 builtin-mv.c            |    6 +++---
 daemon.c                |    7 +------
 diff-delta.c            |    2 +-
 dir.c                   |    4 ++--
 git.c                   |    6 +++---
 sha1_file.c             |    2 +-
 xdiff-interface.c       |   12 ++++++------
 9 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 28b5dfd..a5ed8db 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -27,8 +27,8 @@ static void append_to_list(struct list *
 {
 	if (list->nr == list->alloc) {
 		list->alloc += 32;
-		list->list = realloc(list->list, sizeof(char *) * list->alloc);
-		list->payload = realloc(list->payload,
+		list->list = xrealloc(list->list, sizeof(char *) * list->alloc);
+		list->payload = xrealloc(list->payload,
 				sizeof(char *) * list->alloc);
 	}
 	list->payload[list->nr] = payload;
diff --git a/builtin-log.c b/builtin-log.c
index 691cf3a..fbc58bb 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -101,7 +101,7 @@ static int git_format_config(const char 
 	if (!strcmp(var, "format.headers")) {
 		int len = strlen(value);
 		extra_headers_size += len + 1;
-		extra_headers = realloc(extra_headers, extra_headers_size);
+		extra_headers = xrealloc(extra_headers, extra_headers_size);
 		extra_headers[extra_headers_size - len - 1] = 0;
 		strcat(extra_headers, value);
 		return 0;
@@ -381,7 +381,7 @@ int cmd_format_patch(int argc, const cha
 			continue;
 
 		nr++;
-		list = realloc(list, nr * sizeof(list[0]));
+		list = xrealloc(list, nr * sizeof(list[0]));
 		list[nr - 1] = commit;
 	}
 	total = nr;
diff --git a/builtin-mv.c b/builtin-mv.c
index fd1e520..4d21d88 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -168,13 +168,13 @@ int cmd_mv(int argc, const char **argv, 
 				int j, dst_len;
 
 				if (last - first > 0) {
-					source = realloc(source,
+					source = xrealloc(source,
 							(count + last - first)
 							* sizeof(char *));
-					destination = realloc(destination,
+					destination = xrealloc(destination,
 							(count + last - first)
 							* sizeof(char *));
-					modes = realloc(modes,
+					modes = xrealloc(modes,
 							(count + last - first)
 							* sizeof(enum update_mode));
 				}
diff --git a/daemon.c b/daemon.c
index dd3915a..66ec830 100644
--- a/daemon.c
+++ b/daemon.c
@@ -529,7 +529,6 @@ static int socksetup(int port, int **soc
 
 	for (ai = ai0; ai; ai = ai->ai_next) {
 		int sockfd;
-		int *newlist;
 
 		sockfd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
 		if (sockfd < 0)
@@ -563,11 +562,7 @@ #endif
 			continue;	/* not fatal */
 		}
 
-		newlist = realloc(socklist, sizeof(int) * (socknum + 1));
-		if (!newlist)
-			die("memory allocation failed: %s", strerror(errno));
-
-		socklist = newlist;
+		socklist = xrealloc(socklist, sizeof(int) * (socknum + 1));
 		socklist[socknum++] = sockfd;
 
 		if (maxfd < sockfd)
diff --git a/diff-delta.c b/diff-delta.c
index 51e2e56..fa16d06 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -392,7 +392,7 @@ create_delta(const struct delta_index *i
 				outsize = max_size + MAX_OP_SIZE + 1;
 			if (max_size && outpos > max_size)
 				break;
-			out = realloc(out, outsize);
+			out = xrealloc(out, outsize);
 			if (!out) {
 				free(tmp);
 				return NULL;
diff --git a/dir.c b/dir.c
index a686de6..d53d48f 100644
--- a/dir.c
+++ b/dir.c
@@ -101,8 +101,8 @@ void add_exclude(const char *string, con
 	x->baselen = baselen;
 	if (which->nr == which->alloc) {
 		which->alloc = alloc_nr(which->alloc);
-		which->excludes = realloc(which->excludes,
-					  which->alloc * sizeof(x));
+		which->excludes = xrealloc(which->excludes,
+					   which->alloc * sizeof(x));
 	}
 	which->excludes[which->nr++] = x;
 }
diff --git a/git.c b/git.c
index a01d195..3adf262 100644
--- a/git.c
+++ b/git.c
@@ -120,7 +120,7 @@ static int split_cmdline(char *cmdline, 
 				; /* skip */
 			if (count >= size) {
 				size += 16;
-				*argv = realloc(*argv, sizeof(char*) * size);
+				*argv = xrealloc(*argv, sizeof(char*) * size);
 			}
 			(*argv)[count++] = cmdline + dst;
 		} else if(!quoted && (c == '\'' || c == '"')) {
@@ -191,8 +191,8 @@ static int handle_alias(int *argcp, cons
 			fflush(stderr);
 		}
 
-		new_argv = realloc(new_argv, sizeof(char*) *
-				   (count + *argcp + 1));
+		new_argv = xrealloc(new_argv, sizeof(char*) *
+				    (count + *argcp + 1));
 		/* insert after command name */
 		memcpy(new_argv + count, *argv + 1, sizeof(char*) * *argcp);
 		new_argv[count+*argcp] = NULL;
diff --git a/sha1_file.c b/sha1_file.c
index 789deb7..879390a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1804,7 +1804,7 @@ int read_pipe(int fd, char** return_buf,
 			off += iret;
 			if (off == size) {
 				size *= 2;
-				buf = realloc(buf, size);
+				buf = xrealloc(buf, size);
 			}
 		}
 	} while (iret > 0);
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 6a82da7..08602f5 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -69,9 +69,9 @@ int xdiff_outf(void *priv_, mmbuffer_t *
 	for (i = 0; i < nbuf; i++) {
 		if (mb[i].ptr[mb[i].size-1] != '\n') {
 			/* Incomplete line */
-			priv->remainder = realloc(priv->remainder,
-						  priv->remainder_size +
-						  mb[i].size);
+			priv->remainder = xrealloc(priv->remainder,
+						   priv->remainder_size +
+						   mb[i].size);
 			memcpy(priv->remainder + priv->remainder_size,
 			       mb[i].ptr, mb[i].size);
 			priv->remainder_size += mb[i].size;
@@ -83,9 +83,9 @@ int xdiff_outf(void *priv_, mmbuffer_t *
 			consume_one(priv, mb[i].ptr, mb[i].size);
 			continue;
 		}
-		priv->remainder = realloc(priv->remainder,
-					  priv->remainder_size +
-					  mb[i].size);
+		priv->remainder = xrealloc(priv->remainder,
+					   priv->remainder_size +
+					   mb[i].size);
 		memcpy(priv->remainder + priv->remainder_size,
 		       mb[i].ptr, mb[i].size);
 		consume_one(priv, priv->remainder,
-- 
1.4.2.GIT

-- 
Jonas Fonseca

^ permalink raw reply related

* [PATCH] Use PATH_MAX instead of MAXPATHLEN
From: Jonas Fonseca @ 2006-08-26 14:09 UTC (permalink / raw)
  To: git

According to sys/paramh.h it's a "BSD name" for values defined in
<limits.h>. Besides PATH_MAX seems to be more commonly used.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 builtin-checkout-index.c |    2 +-
 dir.c                    |    2 +-
 entry.c                  |    4 +---
 git-compat-util.h        |    3 ---
 4 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 6b55f93..b097c88 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -45,7 +45,7 @@ #define CHECKOUT_ALL 4
 static int line_termination = '\n';
 static int checkout_stage; /* default to checkout stage0 */
 static int to_tempfile;
-static char topath[4][MAXPATHLEN+1];
+static char topath[4][PATH_MAX + 1];
 
 static struct checkout state;
 
diff --git a/dir.c b/dir.c
index 092d077..a686de6 100644
--- a/dir.c
+++ b/dir.c
@@ -293,7 +293,7 @@ static int read_directory_recursive(stru
 	if (fdir) {
 		int exclude_stk;
 		struct dirent *de;
-		char fullname[MAXPATHLEN + 1];
+		char fullname[PATH_MAX + 1];
 		memcpy(fullname, base, baselen);
 
 		exclude_stk = push_exclude_per_directory(dir, base, baselen);
diff --git a/entry.c b/entry.c
index 793724f..b2ea0ef 100644
--- a/entry.c
+++ b/entry.c
@@ -135,7 +135,7 @@ static int write_entry(struct cache_entr
 
 int checkout_entry(struct cache_entry *ce, struct checkout *state, char *topath)
 {
-	static char path[MAXPATHLEN+1];
+	static char path[PATH_MAX + 1];
 	struct stat st;
 	int len = state->base_dir_len;
 
@@ -172,5 +172,3 @@ int checkout_entry(struct cache_entry *c
 	create_directories(path, state);
 	return write_entry(ce, path, state, 0);
 }
-
-
diff --git a/git-compat-util.h b/git-compat-util.h
index b2e1895..91f2b0d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -172,7 +172,4 @@ static inline int sane_case(int x, int h
 	return x;
 }
 
-#ifndef MAXPATHLEN
-#define MAXPATHLEN 256
-#endif
 #endif
-- 
1.4.2.GIT

-- 
Jonas Fonseca

^ permalink raw reply related

* [PATCH 16a/19] gitweb: Remove workaround for git-diff bug fixed in f82cd3c
From: Jakub Narebski @ 2006-08-26 10:33 UTC (permalink / raw)
  To: git
In-Reply-To: <200608252113.34731.jnareb@gmail.com>

Remove workaround in git_blobdiff for error in git-diff (showing
reversed diff for diff of blobs), corrected in commit
 f82cd3c  Fix "git diff blob1 blob2" showing the diff in reverse.
which is post 1.4.2-rc2 commit.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e5a0db5..36a28a4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2841,8 +2841,7 @@ sub git_blobdiff {
 		}
 
 		# open patch output
-		#open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash
-		open $fd, "-|", $GIT, "diff", '-p', $hash, $hash_parent
+		open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash
 			or die_error(undef, "Open git-diff failed");
 	} else  {
 		die_error('404 Not Found', "Missing one of the blob diff parameters")
-- 
1.4.1.1

^ permalink raw reply related

* Re: Problem with pack
From: Junio C Hamano @ 2006-08-26 10:31 UTC (permalink / raw)
  To: git; +Cc: Sergio Callegari, Nicolas Pitre, Linus Torvalds
In-Reply-To: <7vu03z3j1y.fsf@assigned-by-dhcp.cox.net>

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

> Earlier you said "unpack-objects <$that-pack.pack" fails with
> "error code -3 in inflate..."  What exact error do you get?
> I am guessing that it is get_data() which says:
>
> 	"inflate returned %d\n"
>
> (side note: we should not say \n there).
> ...
> This pattern appears practically everywhere...
> ...  I've been
> wondering if it is possible for inflate to eat some input but
> that was not enough to produce one byte of output, and what [it]
> would return in such a case...

I do not think this fear does not apply to this particular case;
return value -3 is Z_DATA_ERROR, so the deflated stream is
corrupt.

> So there are only a few ways you can get that error message.
> ...

I just realized there is another not so inplausible explanation.

When the problematic pack was made on the mothership,
csum-file.c::sha1write_compressed() gave the data for the base
object to zlib, an alpha particle hit a memory cell that
contained zlib output buffer (resulting in a corrupt deflated
stream in variable "out"), and sha1write() wrote it out while
computing the right checksum.

Is the memory on your mothership reliable (I do not want to make
this message sound like one on the kernel list, but memtest86
might be in order)?

^ permalink raw reply

* Re: [PATCH 16/19] gitweb: Use git-diff-tree or git-diff patch output for blobdiff
From: Jakub Narebski @ 2006-08-26 10:17 UTC (permalink / raw)
  To: git
In-Reply-To: <7vpsen3itw.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Jakub Narebski wrote:
>>
>>> ATTENTION: The order of arguments (operands) to git-diff is reversed
>>> (sic!) to have correct diff in the legacy (no hash_parent_base) case.
>>> $hash_parent, $hash ordering is commented out, as it gives reversed
>>> patch (at least for git version 1.4.1.1) as compared to output in new
>>> scheme and output of older gitweb version.
>>
>> By the way, wa it corrected later? git version 1.4.1.1
> 
> I think you were involved in the thread that resulted in the 
> fix...
> 
> 53dd8a9 Show both blob names from "git diff blob1 blob2"
> f82cd3c Fix "git diff blob1 blob2" showing the diff in reverse.

Gaah. So I'd have to fix the gitweb, i.e. remove workaround
(and update git to 1.4.2). 

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 16/19] gitweb: Use git-diff-tree or git-diff patch output for blobdiff
From: Junio C Hamano @ 2006-08-26 10:14 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <ecp3uq$k1f$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Jakub Narebski wrote:
>
>> ATTENTION: The order of arguments (operands) to git-diff is reversed
>> (sic!) to have correct diff in the legacy (no hash_parent_base) case.
>> $hash_parent, $hash ordering is commented out, as it gives reversed
>> patch (at least for git version 1.4.1.1) as compared to output in new
>> scheme and output of older gitweb version.
>
> By the way, wa it corrected later? git version 1.4.1.1

I think you were involved in the thread that resulted in the 
fix...

53dd8a9 Show both blob names from "git diff blob1 blob2"
f82cd3c Fix "git diff blob1 blob2" showing the diff in reverse.

^ permalink raw reply

* Re: Problem with pack
From: Junio C Hamano @ 2006-08-26 10:09 UTC (permalink / raw)
  To: git; +Cc: Sergio Callegari, Nicolas Pitre, Linus Torvalds
In-Reply-To: <44EECBE2.7090801@arces.unibo.it>

Sergio Callegari <scallegari@arces.unibo.it> writes:

> I was not expecting this kind of problem, so I silly did a repack as
> the last thing, I went home, I attached the laptop to the net, I run
> unison, I started to work and I realized that there was a problem when
> I attempted a new repack which failed complaining about the corrupted
> pack...

Sorry about the mixed "intended audience" of this message,
asking Sergio for a bit more info as the end user who had
problems with git, and at the same time describing the code
level analysis of possible cause to ask for help from git
developers.  Nico CC'ed because he seems to be the person who
knows the best around this area including zlib.

- - -

Earlier you said that the mothership has 1.4.2, and the note has
1.4.0.  The sequence of events as I understand are:

	- repack -a -d on the mothership with 1.4.2; no problems
          observed.

        - transfer the results to note; this was done behind git so
          no problems observed.

        - tried to repack on note with 1.4.0; got "failed to
          read delta-pack base object" error.

Can you make the pack/idx available to the public for
postmortem?

Also I wonder if the pack can be read by 1.4.2.

Earlier you said "unpack-objects <$that-pack.pack" fails with
"error code -3 in inflate..."  What exact error do you get?
I am guessing that it is get_data() which says:

	"inflate returned %d\n"

(side note: we should not say \n there).

        static void *get_data(unsigned long size)
        {
                z_stream stream;
                void *buf = xmalloc(size);

                memset(&stream, 0, sizeof(stream));

                stream.next_out = buf;
                stream.avail_out = size;
                stream.next_in = fill(1);
                stream.avail_in = len;
                inflateInit(&stream);

                for (;;) {
                        int ret = inflate(&stream, 0);
                        use(len - stream.avail_in);
                        if (stream.total_out == size && ret == Z_STREAM_END)
                                break;
                        if (ret != Z_OK)
                                die("inflate returned %d\n", ret);
                        stream.next_in = fill(1);
                        stream.avail_in = len;
                }
                inflateEnd(&stream);
                return buf;
        }

This pattern appears practically everywhere.  When inflate()
returns, we expect its return value to be either Z_OK or
Z_STREAM_END and everything else is treated as an error.  Also
when we receive Z_STREAM_END the resulting length had better be
the size we expect.  I do not have any problem with the latter
but have always felt uneasy about the former but being no zlib
expert had been using the code as is.  zlib.h says Z_BUF_ERROR
is not fatal -- it just means this round of call with the given
input and output buffer did not make any progress.  I've been
wondering if it is possible for inflate to eat some input but
that was not enough to produce one byte of output, and what
would return in such a case...

About the error message you got while attempting to repack, the
exact same error message appears in two places, but the one that
is emitted is the one in sha1_file.c::unpack_delta_entry().  The
other one is in unpack-objects.c and is not run by repack.

This function is called after:

	- we find that we need to access an object;

	- we decided to use the .pack/.idx pair; when mmap'ing
          the .idx file, we validate that the pair is not
          corrupt by calling check_packed_git_idx().  This does
          the sha-1 checksum of both files;

	- we find the location of the deltified object in the
          .pack file by looking at the corresponding .idx;

	- we read from that location a handful bytes, find that
          it is deltified and learn its base object name.

So it is not likely that the .pack/.idx pair was corrupted after
it was written (i.e. not a bit rot).

Now, in unpack_delta_entry_function():

	- we find the location of its base object in the .pack
          file by looking at the .idx again; if this fails, we
          would die with a different error message "failed to
          find delta-pack base object";

	- we call unpack_entry_gently(); we would see the error
          message you saw only when this function returns NULL.

So unpack_entry_gently() while reading the base object returned
NULL.  Let's see how it can:

	- we read the data for the base object in the pack we
          identified earlier.  First we learn its type and size.

	- the base object could be also a deltified object, in
          which case it would recursively call unpack_delta_entry();
	  however, that function would never return NULL.  it
          either succeeds or die()s.  So the base must not have
          been OBJ_DELTA type.

        - the object type recorded there for the base object
          could have been something bogus, in which case we
          would return NULL.  But that would mean pack-object in
          1.4.2 generated a bogus pack on the mothership.

	- if the object type is not bogus, unpack_non_delta_entry()
          is called to extract the data for the base object.
          this decompresses the data stream, and if it does
          not inflate well it would return NULL.

So there are only a few ways you can get that error message.

	- pack-objects in 1.4.2 produced an invalid pack by
          recording:

          - bogus object type for the base object, or

          - incorrectly recording the offset of the base object in
            the pack file in .idx, or

	  - incorrectly recording the size of the base object in
            the pack file; 
 
 	  and checksumed the bogus resulting pack/idx pair as if
 	  nothing was wrong.

	- pack-objects in 1.4.2 produced a deflated stream that
          made unpack_non_delta_entry() unhappy.

Other changes between 1.4.0 and 1.4.2 that I do not think are
related are:

	- we slightly changed the way data is deflated with
          commit 12f6c30 to favour speed over compression, but
          this only affects loose objects and not packs.

	- we introduced a new file format for loose objects with
          commit 93821bd, but this needs to be explicitly
          enabled by .git/config option.  Even if the mothership
          1.4.2 had recorded loose objects in the new format,
          the process to create the pack by first expanding and
          then recompressing with the old-and-proven code, so
          this should not affect the resulting pack.  Even if
          such a loose object were copied to the note with 1.4.0
          together with the pack, object reading code always
          favours what's in the pack, so it should not even be
          touched.  Actually, the codepath that emits the error
          message does not read the base object from anywhere
          other than from the same pack.

	- we slightly changed the way data for the object to be
          deltified and the base object is read in pack-objects
          with commit 560b25a; I did not see anything obviously
          wrong with that change.

        - we slightly changed the way we pick the base object
          when making a delta with commits 8dbbd14 and 51d1e83;
          these should not change what happens after a pair is
          decided to be used as delta and its base.

^ permalink raw reply

* Re: [PATCH 16/19] gitweb: Use git-diff-tree or git-diff patch output for blobdiff
From: Jakub Narebski @ 2006-08-26  9:23 UTC (permalink / raw)
  To: git
In-Reply-To: <200608252113.34731.jnareb@gmail.com>

Jakub Narebski wrote:

> ATTENTION: The order of arguments (operands) to git-diff is reversed
> (sic!) to have correct diff in the legacy (no hash_parent_base) case.
> $hash_parent, $hash ordering is commented out, as it gives reversed
> patch (at least for git version 1.4.1.1) as compared to output in new
> scheme and output of older gitweb version.

By the way, wa it corrected later? git version 1.4.1.1


1010:jnareb@roke:~/git> git diff-tree 599f8d63140^ 599f8d63140 
:100644 100644 0bd517b2649af37d9980f85e784f9a00c3263922 8213ce240232a1dc8a0a498972323a33e8fcb7a0 M  builtin-grep.c


1011:jnareb@roke:~/git> git diff-tree -p 599f8d63140^ 599f8d63140
diff --git a/builtin-grep.c b/builtin-grep.c
index 0bd517b..8213ce2 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -293,9 +293,6 @@ static void compile_patterns(struct grep
         */
        p = opt->pattern_list;
        opt->pattern_expression = compile_pattern_expr(&p);
-#if DEBUG
-       dump_pattern_exp(opt->pattern_expression, 0);
-#endif
        if (p)
                die("incomplete pattern expression: %s", p->pattern);
 }


1012:jnareb@roke:~/git> git diff 0bd517b2649af37d9980f85e784f9a00c3263922 8213ce240232a1dc8a0a498972323a33e8fcb7a0
diff --git a/0bd517b2649af37d9980f85e784f9a00c3263922 b/0bd517b2649af37d9980f85e784f9a00c3263922
index 8213ce2..0bd517b 100644
--- a/0bd517b2649af37d9980f85e784f9a00c3263922
+++ b/0bd517b2649af37d9980f85e784f9a00c3263922
@@ -293,6 +293,9 @@ static void compile_patterns(struct grep
         */
        p = opt->pattern_list;
        opt->pattern_expression = compile_pattern_expr(&p);
+#if DEBUG
+       dump_pattern_exp(opt->pattern_expression, 0);
+#endif
        if (p)
                die("incomplete pattern expression: %s", p->pattern);
 }


-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply related

* [PATCH 5/5] Convert unpack_entry_gently and friends to use offsets.
From: Shawn Pearce @ 2006-08-26  8:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Change unpack_entry_gently and its helper functions to use offsets
rather than addresses and left counts to supply pack position
information.  In most cases this makes the code easier to follow,
and it reduces the number of local variables in a few functions.
It also better prepares this code for mapping partial segments of
packs and altering what regions of a pack are mapped while unpacking
an entry.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_file.c |   33 +++++++++++++++------------------
 1 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index e6d47c1..558ec4a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1041,9 +1041,9 @@ static int packed_object_info(struct pac
 	return 0;
 }
 
-static void *unpack_compressed_entry(unsigned char *data,
-				    unsigned long size,
-				    unsigned long left)
+static void *unpack_compressed_entry(struct packed_git *p,
+				    unsigned long offset,
+				    unsigned long size)
 {
 	int st;
 	z_stream stream;
@@ -1052,8 +1052,8 @@ static void *unpack_compressed_entry(uns
 	buffer = xmalloc(size + 1);
 	buffer[size] = 0;
 	memset(&stream, 0, sizeof(stream));
-	stream.next_in = data;
-	stream.avail_in = left;
+	stream.next_in = (unsigned char*)p->pack_base + offset;
+	stream.avail_in = p->pack_size - offset;
 	stream.next_out = buffer;
 	stream.avail_out = size;
 
@@ -1068,21 +1068,22 @@ static void *unpack_compressed_entry(uns
 	return buffer;
 }
 
-static void *unpack_delta_entry(unsigned char *base_sha1,
+static void *unpack_delta_entry(struct packed_git *p,
+				unsigned long offset,
 				unsigned long delta_size,
-				unsigned long left,
 				char *type,
-				unsigned long *sizep,
-				struct packed_git *p)
+				unsigned long *sizep)
 {
 	struct pack_entry base_ent;
 	void *delta_data, *result, *base;
 	unsigned long result_size, base_size;
+	unsigned char* base_sha1;
 
-	if (left < 20)
+	if ((offset + 20) >= p->pack_size)
 		die("truncated pack file");
 
 	/* The base entry _must_ be in the same pack */
+	base_sha1 = (unsigned char*)p->pack_base + offset;
 	if (!find_pack_entry_one(base_sha1, &base_ent, p))
 		die("failed to find delta-pack base object %s",
 		    sha1_to_hex(base_sha1));
@@ -1091,8 +1092,7 @@ static void *unpack_delta_entry(unsigned
 		die("failed to read delta-pack base object %s",
 		    sha1_to_hex(base_sha1));
 
-	delta_data = unpack_compressed_entry(base_sha1 + 20,
-			     delta_size, left - 20);
+	delta_data = unpack_compressed_entry(p, offset + 20, delta_size);
 	result = patch_delta(base, base_size,
 			     delta_data, delta_size,
 			     &result_size);
@@ -1124,23 +1124,20 @@ void *unpack_entry_gently(struct pack_en
 			  char *type, unsigned long *sizep)
 {
 	struct packed_git *p = entry->p;
-	unsigned long offset, size, left;
-	unsigned char *pack;
+	unsigned long offset, size;
 	enum object_type kind;
 
 	offset = unpack_object_header(p, entry->offset, &kind, &size);
-	pack = (unsigned char *) p->pack_base + offset;
-	left = p->pack_size - offset;
 	switch (kind) {
 	case OBJ_DELTA:
-		return unpack_delta_entry(pack, size, left, type, sizep, p);
+		return unpack_delta_entry(p, offset, size, type, sizep);
 	case OBJ_COMMIT:
 	case OBJ_TREE:
 	case OBJ_BLOB:
 	case OBJ_TAG:
 		strcpy(type, type_names[kind]);
 		*sizep = size;
-		return unpack_compressed_entry(pack, size, left);
+		return unpack_compressed_entry(p, offset, size);
 	default:
 		return NULL;
 	}
-- 
1.4.2.g6580

^ permalink raw reply related

* [PATCH 4/5] Cleanup unpack_object_header to use only offsets.
From: Shawn Pearce @ 2006-08-26  8:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If we're always incrementing both the offset and the pointer we
aren't gaining anything by keeping both.  Instead just use the
offset since that's what we were given and what we are expected
to return.  Also using offset is likely to make it easier to remap
the pack in the future should partial mapping of very large packs
get implemented.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_file.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index b580cee..e6d47c1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -917,23 +917,19 @@ static unsigned long unpack_object_heade
 	enum object_type *type, unsigned long *sizep)
 {
 	unsigned shift;
-	unsigned char *pack, c;
+	unsigned char c;
 	unsigned long size;
 
 	if (offset >= p->pack_size)
 		die("object offset outside of pack file");
-
-	pack =  (unsigned char *) p->pack_base + offset;
-	c = *pack++;
-	offset++;
+	c = *((unsigned char *)p->pack_base + offset++);
 	*type = (c >> 4) & 7;
 	size = c & 15;
 	shift = 4;
 	while (c & 0x80) {
 		if (offset >= p->pack_size)
 			die("object offset outside of pack file");
-		c = *pack++;
-		offset++;
+		c = *((unsigned char *)p->pack_base + offset++);
 		size += (c & 0x7f) << shift;
 		shift += 7;
 	}
-- 
1.4.2.g6580

^ permalink raw reply related

* [PATCH 3/5] Cleanup unpack_entry_gently and friends to use type_name array.
From: Shawn Pearce @ 2006-08-26  8:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[PATCH 3/5] Cleanup unpack_entry_gently and friends to use type_name array.

This change allows combining all of the non-delta entries into a
single case, as well as to remove an unnecessary local variable
in unpack_entry_gently.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_file.c |   34 ++++++----------------------------
 1 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index fd3e01b..b580cee 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -996,16 +996,10 @@ void packed_object_info_detail(struct pa
 	}
 	switch (kind) {
 	case OBJ_COMMIT:
-		strcpy(type, commit_type);
-		break;
 	case OBJ_TREE:
-		strcpy(type, tree_type);
-		break;
 	case OBJ_BLOB:
-		strcpy(type, blob_type);
-		break;
 	case OBJ_TAG:
-		strcpy(type, tag_type);
+		strcpy(type, type_names[kind]);
 		break;
 	default:
 		die("corrupted pack file %s containing object of kind %d",
@@ -1036,16 +1030,10 @@ static int packed_object_info(struct pac
 		unuse_packed_git(p);
 		return retval;
 	case OBJ_COMMIT:
-		strcpy(type, commit_type);
-		break;
 	case OBJ_TREE:
-		strcpy(type, tree_type);
-		break;
 	case OBJ_BLOB:
-		strcpy(type, blob_type);
-		break;
 	case OBJ_TAG:
-		strcpy(type, tag_type);
+		strcpy(type, type_names[kind]);
 		break;
 	default:
 		die("corrupted pack file %s containing object of kind %d",
@@ -1143,33 +1131,23 @@ void *unpack_entry_gently(struct pack_en
 	unsigned long offset, size, left;
 	unsigned char *pack;
 	enum object_type kind;
-	void *retval;
 
 	offset = unpack_object_header(p, entry->offset, &kind, &size);
 	pack = (unsigned char *) p->pack_base + offset;
 	left = p->pack_size - offset;
 	switch (kind) {
 	case OBJ_DELTA:
-		retval = unpack_delta_entry(pack, size, left, type, sizep, p);
-		return retval;
+		return unpack_delta_entry(pack, size, left, type, sizep, p);
 	case OBJ_COMMIT:
-		strcpy(type, commit_type);
-		break;
 	case OBJ_TREE:
-		strcpy(type, tree_type);
-		break;
 	case OBJ_BLOB:
-		strcpy(type, blob_type);
-		break;
 	case OBJ_TAG:
-		strcpy(type, tag_type);
-		break;
+		strcpy(type, type_names[kind]);
+		*sizep = size;
+		return unpack_compressed_entry(pack, size, left);
 	default:
 		return NULL;
 	}
-	*sizep = size;
-	retval = unpack_compressed_entry(pack, size, left);
-	return retval;
 }
 
 int num_packed_objects(const struct packed_git *p)
-- 
1.4.2.g6580

^ permalink raw reply related

* [PATCH 2/5] Reuse compression code in unpack_compressed_entry.
From: Shawn Pearce @ 2006-08-26  8:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[PATCH 2/5] Reuse compression code in unpack_compressed_entry.

This cleans up the code by reusing a perfectly good decompression
implementation at the expense of 1 extra byte of memory allocated in
temporary memory while the delta is being decompressed and applied
to the base.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_file.c |   25 ++++---------------------
 1 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 024b605..fd3e01b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1092,10 +1092,8 @@ static void *unpack_delta_entry(unsigned
 				struct packed_git *p)
 {
 	struct pack_entry base_ent;
-	void *data, *delta_data, *result, *base;
-	unsigned long data_size, result_size, base_size;
-	z_stream stream;
-	int st;
+	void *delta_data, *result, *base;
+	unsigned long result_size, base_size;
 
 	if (left < 20)
 		die("truncated pack file");
@@ -1109,23 +1107,8 @@ static void *unpack_delta_entry(unsigned
 		die("failed to read delta-pack base object %s",
 		    sha1_to_hex(base_sha1));
 
-	data = base_sha1 + 20;
-	data_size = left - 20;
-	delta_data = xmalloc(delta_size);
-
-	memset(&stream, 0, sizeof(stream));
-
-	stream.next_in = data;
-	stream.avail_in = data_size;
-	stream.next_out = delta_data;
-	stream.avail_out = delta_size;
-
-	inflateInit(&stream);
-	st = inflate(&stream, Z_FINISH);
-	inflateEnd(&stream);
-	if ((st != Z_STREAM_END) || stream.total_out != delta_size)
-		die("delta data unpack failed");
-
+	delta_data = unpack_compressed_entry(base_sha1 + 20,
+			     delta_size, left - 20);
 	result = patch_delta(base, base_size,
 			     delta_data, delta_size,
 			     &result_size);
-- 
1.4.2.g6580

^ permalink raw reply related

* [PATCH 1/5] Reorganize/rename unpack_non_delta_entry to unpack_compressed_entry.
From: Shawn Pearce @ 2006-08-26  8:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This function was moved above unpack_delta_entry so we can call it
from within unpack_delta_entry without a forward declaration.

This change looks worse than it is.  Its really just a relocation
of unpack_non_delta_entry to earlier in the file and renaming the
function to unpack_compressed_entry.  No other changes were made.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_file.c |   56 ++++++++++++++++++++++++++++----------------------------
 1 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 066cff1..024b605 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1057,6 +1057,33 @@ static int packed_object_info(struct pac
 	return 0;
 }
 
+static void *unpack_compressed_entry(unsigned char *data,
+				    unsigned long size,
+				    unsigned long left)
+{
+	int st;
+	z_stream stream;
+	unsigned char *buffer;
+
+	buffer = xmalloc(size + 1);
+	buffer[size] = 0;
+	memset(&stream, 0, sizeof(stream));
+	stream.next_in = data;
+	stream.avail_in = left;
+	stream.next_out = buffer;
+	stream.avail_out = size;
+
+	inflateInit(&stream);
+	st = inflate(&stream, Z_FINISH);
+	inflateEnd(&stream);
+	if ((st != Z_STREAM_END) || stream.total_out != size) {
+		free(buffer);
+		return NULL;
+	}
+
+	return buffer;
+}
+
 static void *unpack_delta_entry(unsigned char *base_sha1,
 				unsigned long delta_size,
 				unsigned long left,
@@ -1110,33 +1137,6 @@ static void *unpack_delta_entry(unsigned
 	return result;
 }
 
-static void *unpack_non_delta_entry(unsigned char *data,
-				    unsigned long size,
-				    unsigned long left)
-{
-	int st;
-	z_stream stream;
-	unsigned char *buffer;
-
-	buffer = xmalloc(size + 1);
-	buffer[size] = 0;
-	memset(&stream, 0, sizeof(stream));
-	stream.next_in = data;
-	stream.avail_in = left;
-	stream.next_out = buffer;
-	stream.avail_out = size;
-
-	inflateInit(&stream);
-	st = inflate(&stream, Z_FINISH);
-	inflateEnd(&stream);
-	if ((st != Z_STREAM_END) || stream.total_out != size) {
-		free(buffer);
-		return NULL;
-	}
-
-	return buffer;
-}
-
 static void *unpack_entry(struct pack_entry *entry,
 			  char *type, unsigned long *sizep)
 {
@@ -1185,7 +1185,7 @@ void *unpack_entry_gently(struct pack_en
 		return NULL;
 	}
 	*sizep = size;
-	retval = unpack_non_delta_entry(pack, size, left);
+	retval = unpack_compressed_entry(pack, size, left);
 	return retval;
 }
 
-- 
1.4.2.g6580

^ permalink raw reply related

* [PATCH 0/5] sha1_file.c pack reading cleanups
From: Shawn Pearce @ 2006-08-26  8:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This is a 5 patch series built on `master`.  I discovered these
cleanups when I lifted code from sha1_file.c for fast-import.c
(as I needed to read the pack I was writing and thus didn't have
a nice sorted index to work with).

I'll probably follow this series with another one which libifies
more of these functions to the point that I can actually use them
from within fast-import.c, without having an index file.  But I
might first send partial pack mapping as I'm also doing that in
fast-import.c right now.

In this series:

  1/5 Reorganize/rename unpack_non_delta_entry to unpack_compressed_entry.
  2/5 Reuse compression code in unpack_compressed_entry.
  3/5 Cleanup unpack_entry_gently and friends to use type_name array.
  4/5 Cleanup unpack_object_header to use only offsets.
  5/5 Convert unpack_entry_gently and friends to use offsets.

-- 
Shawn.

^ permalink raw reply

* Re: git-svn problem: unexpected files/diffs in commit
From: Eric Wong @ 2006-08-26  7:33 UTC (permalink / raw)
  To: Seth Falcon; +Cc: git
In-Reply-To: <m2lkpcfhml.fsf@ziti.local>

Seth Falcon <sethfalcon@gmail.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:

> > Outside of the SSL problems, the mis-commit isn't strictly user-error,
> > but git-svn is confusing in this case, as the documentation is unclear
> > about what git-svn should do in this case :x
> 
> > I usually check with git log remotes/git-svn..HEAD instead of git
> > diff.  Perhaps adding --no-merges would be more correct?

No, --no-merges is not correct (see below).

> I'm not sure how to reproduce the situation I was in, but what would
> git log have shown me that git diff didn't -- IOW, would it have been
> obvious that the commit op was going to add extra stuff and
> effectively undo a rev in svn?

> > I've been really slacking on the git-svn documentation the past few
> > months, help would be much appreciated.
> 
> I will try to send some doc patches.  But I may have a few questions ;-)

> I think commit-diff might be what I want to be using too, but I need
> to contribute some documentation for it before I can read the man page
> and start using it ;-)
> 
> An example call to git-svn commit-diff would be very helpful, I
> suspect.

git-svn commit-diff <a> <b>
(git diff-tree <a> <b> to check the result)

My new command, 'dcommit' wraps it.

'commit-diff' very low-level command that does exactly what you tell
it to do, compute the delta of two trees, and send it to the SVN repo.
The <a> tree doesn't even have to strictly match what's in the SVN repo
at the given moment if you're using the SVN:: libs.

'commit' was the original command, that meant: write this tree to SVN.
Period.  This was before SVN:: library support was available, so it had
to be stupidly simple.

foo..bar notation was supported, which in hindsight was a mistake
that has probably confused lots of people....

> I will have a look...
> 
> While I'm thinking of it, it would be really nice for git-svn's commit and
> commit-diff to have a dry-run type of flag.  Commits to your own git
> repos are easy to correct, but those made on some other public scm are
> less pretty.

I think everything is somewhat documented in the commit logs, but most
of those were back when git-svn was in contrib/

Otherwise, git-svn help attempts to be reasonably self-documenting, and
I've tried to keep the code to git-svn fairly readable (except the code
to generate the aforementioned help output :)

-- 
Eric Wong

^ permalink raw reply

* [PATCH] git-svn: add the 'dcommit' command
From: Eric Wong @ 2006-08-26  7:01 UTC (permalink / raw)
  To: Seth Falcon; +Cc: git
In-Reply-To: <20060825191516.GA8957@localdomain>

This is a high-level wrapper around the 'commit-diff' command
and used to produce cleaner history against the mirrored repository
through rebase/reset usage.

It's basically a more polished version of this:

for i in `git rev-list --no-merges remotes/git-svn..HEAD | tac`; do
	git-svn commit-diff $i~1 $i
done
git reset --hard remotes/git-svn

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-svn.txt |   25 +++++++++++++++++++++++++
 git-svn.perl              |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 9fce4d3..38e73ba 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -53,6 +53,15 @@ fetch::
 	See 'Additional Fetch Arguments' if you are interested in
 	manually joining branches on commit.
 
+dcommit::
+	Commit all diffs from the current HEAD directly to the SVN
+	repository, and then rebase or reset (depending on whether or
+	not there is a diff between SVN and HEAD).  It is recommended
+	that you run git-svn fetch and rebase (not pull) your commits
+	against the latest changes in the SVN repository.
+	This is advantageous over 'commit' (below) because it produces
+	cleaner, more linear history.
+
 commit::
 	Commit specified commit or tree objects to SVN.  This relies on
 	your imported fetch data being up-to-date.  This makes
@@ -146,6 +155,22 @@ loginname = Joe User <user@example.com>
 
 	repo-config key: svn.authors-file
 
+-m::
+--merge::
+-s<strategy>::
+--strategy=<strategy>::
+	These are only used with the 'dcommit' command.
+
+	Passed directly to git-rebase when using 'dcommit' if a
+	'git-reset' cannot be used (see dcommit).
+
+-n::
+--dry-run::
+	This is only used with the 'dcommit' command.
+
+	Print out the series of git arguments that would show
+	which diffs would be committed to SVN.
+
 ADVANCED OPTIONS
 ----------------
 -b<refname>::
diff --git a/git-svn.perl b/git-svn.perl
index b311c3d..9382a15 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -51,7 +51,8 @@ my ($_revision,$_stdin,$_no_ignore_ext,$
 	$_message, $_file, $_follow_parent, $_no_metadata,
 	$_template, $_shared, $_no_default_regex, $_no_graft_copy,
 	$_limit, $_verbose, $_incremental, $_oneline, $_l_fmt, $_show_commit,
-	$_version, $_upgrade, $_authors, $_branch_all_refs, @_opt_m);
+	$_version, $_upgrade, $_authors, $_branch_all_refs, @_opt_m,
+	$_merge, $_strategy, $_dry_run);
 my (@_branch_from, %tree_map, %users, %rusers, %equiv);
 my ($_svn_co_url_revs, $_svn_pg_peg_revs);
 my @repo_path_split_cache;
@@ -118,6 +119,11 @@ my %cmd = (
 			{ 'message|m=s' => \$_message,
 			  'file|F=s' => \$_file,
 			%cmt_opts } ],
+	dcommit => [ \&dcommit, 'Commit several diffs to merge with upstream',
+			{ 'merge|m|M' => \$_merge,
+			  'strategy|s=s' => \$_strategy,
+			  'dry-run|n' => \$_dry_run,
+			%cmt_opts } ],
 );
 
 my $cmd;
@@ -561,6 +567,33 @@ sub commit_lib {
 	unlink $commit_msg;
 }
 
+sub dcommit {
+	my $gs = "refs/remotes/$GIT_SVN";
+	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..HEAD"));
+	foreach my $d (reverse @refs) {
+		if ($_dry_run) {
+			print "diff-tree $d~1 $d\n";
+		} else {
+			commit_diff("$d~1", $d);
+		}
+	}
+	return if $_dry_run;
+	fetch();
+	my @diff = safe_qx(qw/git-diff-tree HEAD/, $gs);
+	my @finish;
+	if (@diff) {
+		@finish = qw/rebase/;
+		push @finish, qw/--merge/ if $_merge;
+		push @finish, "--strategy=$_strategy" if $_strategy;
+		print STDERR "W: HEAD and $gs differ, using @finish:\n", @diff;
+	} else {
+		print "No changes between current HEAD and $gs\n",
+		      "Hard resetting to the latest $gs\n";
+		@finish = qw/reset --hard/;
+	}
+	sys('git', @finish, $gs);
+}
+
 sub show_ignore {
 	$SVN_URL ||= file_to_s("$GIT_SVN_DIR/info/url");
 	$_use_lib ? show_ignore_lib() : show_ignore_cmd();
-- 
1.4.2.g7c9b

^ permalink raw reply related

* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Marco Costalba @ 2006-08-26  5:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <e5bfff550608252234u6d4e9efax8ca7e2225cef4cef@mail.gmail.com>

On 8/26/06, Marco Costalba <mcostalba@gmail.com> wrote:
> > Actually Linux git archive _is_ a special, odd-ball case.
>
> It's also our main customer ;-)
>

More seriously, do you foreseen reasons for an SCM to split old
histories (more then, say few years and some GB ago) in a separated
repository?

Now in Linux we have the historical repo and is splitted because it
was borne splitted, but do you think this could be also an acceptable
policy for a big project after years of git use: to backup old stuff
and keep recent one snappy?

With snappy I mean a long list of things: cloning, getting file
histories / blaming, packing, fsck-ing, etc...

The net effect could be a better browsing/handling experience in the
99% of cases where you need to check recent work (say last 2 years)
and in any case the possibility to do some archeologic research when
needed.

Marco

^ permalink raw reply

* Re: Mozilla import and large history
From: Shawn Pearce @ 2006-08-26  5:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejv43wq9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> People with long history stored in some SCM tend to want to
> migrate their history and then switch, and I expect that to be
> the norm, especially with the progress Mozilla import team is
> making with the CVS interface.

Not to hijack a thread or anything but we are definately making
some progress here.  Jon Smirl is able to import blob revisions,
create trees, commits and tags.  Import for Mozilla is down from
approx. a week to around 4 hours but right now we are finding that
the import isn't always correct.

Fortunately we've built a script to compare the result of the import
into GIT with the result of the import into SVN, as the cvs2svn
import is believed to be correct.  The script is not however very
fast, as it compares an SVN revision to the corresponding GIT tree
by checking out the SVN revision to a working directory, loading
the GIT tree into an index and using git-diff-files to compare them.

That simple script has caught a number of errors but has also
confirmed a number of cases as working correctly.  We're slowly
working through the errors.

We may also do a CVS checkout based script to verify branch heads
and labels all match, just as an extra check against what the native
SVN import had given.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Marco Costalba @ 2006-08-26  5:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejv43wq9.fsf@assigned-by-dhcp.cox.net>

> Actually Linux git archive _is_ a special, odd-ball case.

It's also our main customer ;-)

^ permalink raw reply

* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Junio C Hamano @ 2006-08-26  5:13 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550608252144p2d234a7cnf2ca2922c98d921b@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

>> > - Original code lines, ie. imported at the beginning and never
>> > modified, perhaps it is better to view without commit number, this
>> > could obfuscate the view and in any case is not an accurate info
>> > because the line was not modified during initial patch.
>>
>> That holds only true for very young projects, or ones that were
>> perfect from the beginning and did not see much action ;-).
>
> Or also for projects like Linux ;-)
>
> See blame output of something like kernel/dma.c or also kernel/exit.c,
> not exactly the most unknown files around.

Actually Linux git archive _is_ a special, odd-ball case.  Their
initial commit (2.6.12-rc2) contains all the fruits from their
10 years of evolution without git.

People with long history stored in some SCM tend to want to
migrate their history and then switch, and I expect that to be
the norm, especially with the progress Mozilla import team is
making with the CVS interface.

^ permalink raw reply

* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Marco Costalba @ 2006-08-26  4:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narebski
In-Reply-To: <7vveog45ff.fsf@assigned-by-dhcp.cox.net>

> > - Original code lines, ie. imported at the beginning and never
> > modified, perhaps it is better to view without commit number, this
> > could obfuscate the view and in any case is not an accurate info
> > because the line was not modified during initial patch.
>
> That holds only true for very young projects, or ones that were
> perfect from the beginning and did not see much action ;-).
>

Or also for projects like Linux ;-)

See blame output of something like kernel/dma.c or also kernel/exit.c,
not exactly the most unknown files around.

^ permalink raw reply

* [PATCH] Rewrite branch in C and make it a builtin.
From: Kristian  Høgsberg @ 2006-08-26  3:15 UTC (permalink / raw)
  To: git; +Cc: Kristian Høgsberg

From: =?utf-8?q?Kristian_H=C3=B8gsberg?= <krh@redhat.com>

A more or less straight port to C of the shell script version.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 Makefile         |    3 -
 builtin-branch.c |  227 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 builtin.h        |    1 
 git-branch.sh    |  130 -------------------------------
 git.c            |    1 
 5 files changed, 231 insertions(+), 131 deletions(-)

diff --git a/Makefile b/Makefile
index 23cd8a0..adf043e 100644
--- a/Makefile
+++ b/Makefile
@@ -149,7 +149,7 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powe
 ### --- END CONFIGURATION SECTION ---
 
 SCRIPT_SH = \
-	git-bisect.sh git-branch.sh git-checkout.sh \
+	git-bisect.sh git-checkout.sh \
 	git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
 	git-fetch.sh \
 	git-ls-remote.sh \
@@ -253,6 +253,7 @@ LIB_OBJS = \
 BUILTIN_OBJS = \
 	builtin-add.o \
 	builtin-apply.o \
+	builtin-branch.o \
 	builtin-cat-file.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
diff --git a/builtin-branch.c b/builtin-branch.c
new file mode 100644
index 0000000..c8d0275
--- /dev/null
+++ b/builtin-branch.c
@@ -0,0 +1,227 @@
+/*
+ * Builtin "git branch"
+ *
+ * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-branch.sh by Junio C Hamano.
+ */
+
+#include "cache.h"
+#include "refs.h"
+#include "commit.h"
+#include "builtin.h"
+
+static const char builtin_branch_usage[] =
+"git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
+
+
+static const char *head;
+static unsigned char head_sha1[20];
+
+static int in_merge_bases(const unsigned char *sha1,
+			  struct commit *rev1,
+			  struct commit *rev2)
+{
+	struct commit_list *bases, *b;
+	int ret = 0;
+
+	bases = get_merge_bases(rev1, rev2, 1);
+	for (b = bases; b; b = b->next) {
+		if (!hashcmp(sha1, b->item->object.sha1)) {
+			ret = 1;
+			break;
+		}
+	}
+
+	free_commit_list(bases);
+	return ret;
+}
+
+static void delete_branches(int argc, const char **argv, int force)
+{
+	struct commit *rev, *head_rev;
+	unsigned char sha1[20];
+	const char *name, *reflog;
+	int i;
+
+	head_rev = lookup_commit_reference(head_sha1);
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(head, argv[i]))
+			die("Cannot delete the branch you are currently on.");
+
+		name = git_path("refs/heads/%s", argv[i]);
+		if (!resolve_ref(name, sha1, 1))
+			die("Branch '%s' not found.", argv[i]);
+
+		rev = lookup_commit_reference(sha1);
+		if (!rev || !head_rev)
+			die("Couldn't look up commit objects.");
+
+		/* This checks whether the merge bases of branch and
+		 * HEAD contains branch -- which means that the HEAD
+		 * contains everything in both.
+		 */
+
+		if (!force &&
+		    !in_merge_bases(sha1, rev, head_rev)) {
+			fprintf(stderr,
+				"The branch '%s' is not a strict subset of your current HEAD.\n"
+				"If you are sure you want to delete it, run 'git branch -D %s'.\n",
+				argv[i], argv[i]);
+			exit(1);
+		}
+
+		unlink(name);
+
+		/* Unlink reflog if it exists. */
+		reflog = git_path("logs/refs/heads/%s", argv[i]);
+		unlink(reflog);
+
+		printf("Deleted branch %s.\n", argv[i]);
+	}
+}
+
+static int ref_index, ref_alloc;
+static char **ref_list;
+
+static int append_ref(const char *refname, const unsigned char *sha1)
+{
+	if (ref_index >= ref_alloc) {
+		ref_alloc = ref_alloc > 0 ? ref_alloc * 2 : 16;
+		ref_list = xrealloc(ref_list, ref_alloc * sizeof(char *));
+	}
+
+	ref_list[ref_index++] = strdup(refname);
+
+	return 0;
+}
+
+static int ref_cmp(const void *r1, const void *r2)
+{
+	return strcmp(*(char **)r1, *(char **)r2);
+}
+
+static void print_ref_list(int remote_only)
+{
+	int i;
+
+	if (remote_only)
+		for_each_remote_ref(append_ref);
+	else
+		for_each_branch_ref(append_ref);
+
+	qsort(ref_list, ref_index, sizeof(char *), ref_cmp);
+
+	for (i = 0; i < ref_index; i++) {
+		if (!strcmp(ref_list[i], head))
+			printf("* %s\n", ref_list[i]);
+		else
+			printf("  %s\n", ref_list[i]);
+	}
+}
+
+static void create_reflog(struct ref_lock *lock)
+{
+	struct stat stbuf;
+	int fd;
+
+	if (!stat(lock->log_file, &stbuf) && S_ISREG(stbuf.st_mode))
+		return;
+	if (safe_create_leading_directories(lock->log_file) < 0)
+		die("Unable to create directory for %s.", lock->log_file);
+	fd = open(lock->log_file, O_CREAT | O_TRUNC | O_WRONLY, 0666);
+	if (fd < 0)
+		die("Unable to create ref log %s: %s.",
+		    lock->log_file, strerror(errno));
+	close(fd);
+}
+
+static void create_branch(const char *name, const char *start,
+			  int force, int reflog)
+{
+	struct ref_lock *lock;
+	unsigned char sha1[20];
+	char ref[PATH_MAX], msg[PATH_MAX + 20];
+
+	snprintf(ref, sizeof ref, "refs/heads/%s", name);
+	if (check_ref_format(ref))
+		die("'%s' is not a valid branch name.", name);
+
+	if (resolve_ref(git_path(ref), sha1, 1)) {
+		if (!force)
+			die("A branch named '%s' already exists.", name);
+		else if (!strcmp(head, name))
+			die("Cannot force update the current branch.");
+	}
+
+	if (get_sha1(start, sha1))
+		die("Not a valid branch point: '%s'.", start);
+
+	lock = lock_any_ref_for_update(ref, NULL, 0);
+	if (!lock)
+		die("Failed to lock ref for update: %s.", strerror(errno));
+	if (reflog)
+		create_reflog(lock);
+	snprintf(msg, sizeof msg, "branch: Created from %s", start);
+	if (write_ref_sha1(lock, sha1, msg) < 0)
+		die("Failed to write ref: %s.", strerror(errno));
+}
+
+int cmd_branch(int argc, const char **argv, const char *prefix)
+{
+	int delete = 0, force_delete = 0, force_create = 0, remote_only = 0;
+	int reflog = 0;
+	int i, prefix_length;
+	const char *p;
+
+	git_config(git_default_config);
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (arg[0] != '-')
+			break;
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(arg, "-d")) {
+			delete = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-D")) {
+			delete = 1;
+			force_delete = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-f")) {
+			force_create = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-r")) {
+			remote_only = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-l")) {
+			reflog = 1;
+			continue;
+		}
+		usage(builtin_branch_usage);
+	}
+
+	prefix_length = strlen(git_path("refs/heads/"));
+	p = resolve_ref(git_path("HEAD"), head_sha1, 0);
+	if (!p)
+		die("Failed to resolve HEAD as a valid ref.");
+	head = strdup(p + prefix_length);
+
+	if (delete)
+		delete_branches(argc - i, argv + i, force_delete);
+	else if (i == argc)
+		print_ref_list(remote_only);
+	else if (argc - i == 1)
+		create_branch(argv[i], head, force_create, reflog);
+	else
+		create_branch(argv[i], argv[i + 1], force_create, reflog);
+
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index ade58c4..eb28986 100644
--- a/builtin.h
+++ b/builtin.h
@@ -15,6 +15,7 @@ extern int write_tree(unsigned char *sha
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
+extern int cmd_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
diff --git a/git-branch.sh b/git-branch.sh
deleted file mode 100755
index e0501ec..0000000
--- a/git-branch.sh
+++ /dev/null
@@ -1,130 +0,0 @@
-#!/bin/sh
-
-USAGE='[-l] [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r'
-LONG_USAGE='If no arguments, show available branches and mark current branch with a star.
-If one argument, create a new branch <branchname> based off of current HEAD.
-If two arguments, create a new branch <branchname> based off of <start-point>.'
-
-SUBDIRECTORY_OK='Yes'
-. git-sh-setup
-
-headref=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
-
-delete_branch () {
-    option="$1"
-    shift
-    for branch_name
-    do
-	case ",$headref," in
-	",$branch_name,")
-	    die "Cannot delete the branch you are on." ;;
-	,,)
-	    die "What branch are you on anyway?" ;;
-	esac
-	branch=$(cat "$GIT_DIR/refs/heads/$branch_name") &&
-	    branch=$(git-rev-parse --verify "$branch^0") ||
-		die "Seriously, what branch are you talking about?"
-	case "$option" in
-	-D)
-	    ;;
-	*)
-	    mbs=$(git-merge-base -a "$branch" HEAD | tr '\012' ' ')
-	    case " $mbs " in
-	    *' '$branch' '*)
-		# the merge base of branch and HEAD contains branch --
-		# which means that the HEAD contains everything in both.
-		;;
-	    *)
-		echo >&2 "The branch '$branch_name' is not a strict subset of your current HEAD.
-If you are sure you want to delete it, run 'git branch -D $branch_name'."
-		exit 1
-		;;
-	    esac
-	    ;;
-	esac
-	rm -f "$GIT_DIR/logs/refs/heads/$branch_name"
-	rm -f "$GIT_DIR/refs/heads/$branch_name"
-	echo "Deleted branch $branch_name."
-    done
-    exit 0
-}
-
-ls_remote_branches () {
-    git-rev-parse --symbolic --all |
-    sed -ne 's|^refs/\(remotes/\)|\1|p' |
-    sort
-}
-
-force=
-create_log=
-while case "$#,$1" in 0,*) break ;; *,-*) ;; *) break ;; esac
-do
-	case "$1" in
-	-d | -D)
-		delete_branch "$@"
-		exit
-		;;
-	-r)
-		ls_remote_branches
-		exit
-		;;
-	-f)
-		force="$1"
-		;;
-	-l)
-		create_log="yes"
-		;;
-	--)
-		shift
-		break
-		;;
-	-*)
-		usage
-		;;
-	esac
-	shift
-done
-
-case "$#" in
-0)
-	git-rev-parse --symbolic --branches |
-	sort |
-	while read ref
-	do
-		if test "$headref" = "$ref"
-		then
-			pfx='*'
-		else
-			pfx=' '
-		fi
-		echo "$pfx $ref"
-	done
-	exit 0 ;;
-1)
-	head=HEAD ;;
-2)
-	head="$2^0" ;;
-esac
-branchname="$1"
-
-rev=$(git-rev-parse --verify "$head") || exit
-
-git-check-ref-format "heads/$branchname" ||
-	die "we do not like '$branchname' as a branch name."
-
-if [ -e "$GIT_DIR/refs/heads/$branchname" ]
-then
-	if test '' = "$force"
-	then
-		die "$branchname already exists."
-	elif test "$branchname" = "$headref"
-	then
-		die "cannot force-update the current branch."
-	fi
-fi
-if test "$create_log" = 'yes'
-then
-	mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$branchname")
-	touch "$GIT_DIR/logs/refs/heads/$branchname"
-fi
-git update-ref -m "branch: Created from $head" "refs/heads/$branchname" $rev
diff --git a/git.c b/git.c
index 930998b..5738cb4 100644
--- a/git.c
+++ b/git.c
@@ -226,6 +226,7 @@ static void handle_internal_command(int 
 	} commands[] = {
 		{ "add", cmd_add, RUN_SETUP },
 		{ "apply", cmd_apply },
+		{ "branch", cmd_branch, RUN_SETUP },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
 		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
 		{ "check-ref-format", cmd_check_ref_format },
-- 
1.4.2.g8153-dirty

^ permalink raw reply related

* Re: reversible binary diff
From: Junio C Hamano @ 2006-08-26  2:14 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0608252050140.3683@localhost.localdomain>

Nicolas Pitre <nico@cam.org> writes:

> I just noticed that the binary diff format was augmented in order to 
> carry the reverse diff information.
>
> Why was this needed?
>
> I mean, if you want to reverse a binary diff you only need to retrieve 
> the original blob the forward diff was meant to apply against, and it is 
> certainly already available in the object store if the forward diff has 
> been previously applied.  Or has this assumption been wrong for some 
> work flow?

It's been wrong all along, but I do not think it practically
matters because I haven't seen anybody exchange binary diffs
back and forth.

As long as you use patch to switch between states inside the
repository the patch originates from, it is not needed.  The
change is just there for completeness.

But if you are sending a patch to somebody else, you would need
the reverse information.  t/t4116 test needs to be updated to
use separate repository that has only preimage and apply a patch
in reverse there to validate the correct operation.  The log
message for the commit that introduced the test mentions it, but
nobody noticed and took hint ;-).

^ permalink raw reply

* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Junio C Hamano @ 2006-08-26  2:05 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git, Jakub Narebski
In-Reply-To: <e5bfff550608251433o36713ee1na5544992320b5845@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

>> You can view new gitweb in work at
>>   http://front.fuw.edu.pl/cgi-bin/jnareb/gitweb.cgi
>
> Very nice job!
>
> Just a couple of suggestion regarding blame.
>
> - Instead of commit sha perhaps is more useful to show author name
> (linked to commit) and progressive number of revision.

Remember git history is not linear so progressive number does
not make much sense here.  I agree author and commit date would
be nice if they were readily available without cluttering.
A pop-up when hovered, perhaps?

> - Original code lines, ie. imported at the beginning and never
> modified, perhaps it is better to view without commit number, this
> could obfuscate the view and in any case is not an accurate info
> because the line was not modified during initial patch.

That holds only true for very young projects, or ones that were
perfect from the beginning and did not see much action ;-).

^ 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