Git development
 help / color / mirror / Atom feed
* Re: [osol-bugs] access() behaves strange when used as root
From: Stefan Pfetzing @ 2006-05-24 14:08 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <447460C1.6070305@sun.com>

Hi Alan,

2006/5/24, Alan Coopersmith <Alan.Coopersmith@sun.com>:

> Compilers also fall in the class of things I've never understood why
> people would ever run as root.   Far too complex and completely unnecessary.
> "make all" as a normal user, and then if you absolutely must, "make install"
> as root.  (After running "make -n install" first to see what it will do.)

Yes thats completely true, but it still leaves the point if you want
to manage some
of your config files with git.

bye

Stefan

-- 
       http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.

^ permalink raw reply

* Re: [osol-bugs] access() behaves strange when used as root
From: Stefan Pfetzing @ 2006-05-24 14:09 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <f3d7535d0605240708k7e55cc3fu8c2e8ad744f738c9@mail.gmail.com>

[snip]

oups - sorry, wrong recepient... :( *shrug*

bye

Stefan

-- 
       http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.

^ permalink raw reply

* Re: [PATCH] Add a test-case for git-apply trying to add an ending line
From: Linus Torvalds @ 2006-05-24 14:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Catalin Marinas, git
In-Reply-To: <7v8xosqaqm.fsf@assigned-by-dhcp.cox.net>



On Tue, 23 May 2006, Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > On Tue, 23 May 2006, Junio C Hamano wrote:
> >
> >> The issue is if we can reliably tell if there is such an EOF
> >> context by looking at the diff.  Not having the same number of
> >> lines that starts with ' ' in the hunk is not really a nice way
> >> of doing so (you could make a unified diff that does not have
> >> trailing context at all), and I do not offhand think of a good
> >> way to do so.
> >
> > We can. Something like this should do it.
> >
> > (The same thing could be done for "match_beginning", perhaps).
> 
> But this is exactly what I said I had trouble with in the above.

Well, not quite. You said "not the same number of lines", and I say "no 
ending context". Very different.

My patch actually is totally self-consistent: not having any context at 
the end of a unified diff really means that it is the end of the file (ie, 
the "end of file" there _is_ the context). And if you want to apply files 
without context, you should use "-Cx", and my patch does that too - if you 
asked for "relaxed context checking", it will re-try without the "only at 
end" check thanks to the

	if (match_end) {
		match_end = 0;
		continue;
	}

so it all should work.

Not that I _tested_ it, of course ;)

		Linus

^ permalink raw reply

* cg-clone -a
From: Belmar-Letelier @ 2006-05-24 15:12 UTC (permalink / raw)
  To: git

Hello

Any news about a a way to get all tags with cogito ?

Some kind of

$ cg-clone -a

-- 
Luis Belmar-Letelier

^ permalink raw reply

* Clean up sha1 file writing
From: Linus Torvalds @ 2006-05-24 15:30 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


This cleans up and future-proofs the sha1 file writing in sha1_file.c.

In particular, instead of doing a simple "write()" call and just verifying 
that it succeeds (or - as in one place - just assuming it does), it uses 
"write_buffer()" to write data to the file descriptor while correctly 
checking for partial writes, EINTR etc.

It also splits up write_sha1_to_fd() to be a lot more readable: if we need 
to re-create the compressed object, we do so in a separate helper 
function, making the logic a whole lot more modular and obvious.

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

This shouldn't change any behaviour, and it's obviously touching some core 
code, so maybe it's not worth it. On the other hand, from a longer-term 
maintenance standpoint and from a "be much more careful when doing file 
writes" standpoint, I think it's worth it.

The re-write is "obviously correct" (famous last words) and is mostly 
just moving code around and getting rid of a few temporaries that become 
unnecessary as a result.

The patch looks a bit messy: the changes aren't actually that big, but the 
split-up and the resulting re-indentation makes the patch fairly 
unreadable, so the cleanups are more obvious when you look at the 
before-and-after side by side rather than when looking at the unified 
diff..)

diff --git a/sha1_file.c b/sha1_file.c
index 2230010..c2fe7c2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1399,6 +1399,25 @@ int move_temp_to_file(const char *tmpfil
 	return 0;
 }
 
+static int write_buffer(int fd, const void *buf, size_t len)
+{
+	while (len) {
+		ssize_t size;
+
+		size = write(fd, buf, len);
+		if (!size)
+			return error("file write: disk full");
+		if (size < 0) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
+			return error("file write error (%s)", strerror(errno));
+		}
+		len -= size;
+		buf += size;
+	}
+	return 0;
+}
+
 int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
 {
 	int size;
@@ -1465,8 +1484,8 @@ int write_sha1_file(void *buf, unsigned 
 	deflateEnd(&stream);
 	size = stream.total_out;
 
-	if (write(fd, compressed, size) != size)
-		die("unable to write file");
+	if (write_buffer(fd, compressed, size) < 0)
+		die("unable to write sha1 file");
 	fchmod(fd, 0444);
 	close(fd);
 	free(compressed);
@@ -1474,73 +1493,70 @@ int write_sha1_file(void *buf, unsigned 
 	return move_temp_to_file(tmpfile, filename);
 }
 
-int write_sha1_to_fd(int fd, const unsigned char *sha1)
+/*
+ * We need to unpack and recompress the object for writing
+ * it out to a different file.
+ */
+static void *repack_object(const unsigned char *sha1, unsigned long *objsize)
 {
-	ssize_t size;
-	unsigned long objsize;
-	int posn = 0;
-	void *map = map_sha1_file_internal(sha1, &objsize);
-	void *buf = map;
-	void *temp_obj = NULL;
+	size_t size;
 	z_stream stream;
+	unsigned char *unpacked;
+	unsigned long len;
+	char type[20];
+	char hdr[50];
+	int hdrlen;
+	void *buf;
 
-	if (!buf) {
-		unsigned char *unpacked;
-		unsigned long len;
-		char type[20];
-		char hdr[50];
-		int hdrlen;
-		// need to unpack and recompress it by itself
-		unpacked = read_packed_sha1(sha1, type, &len);
+	// need to unpack and recompress it by itself
+	unpacked = read_packed_sha1(sha1, type, &len);
 
-		hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
+	hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
 
-		/* Set it up */
-		memset(&stream, 0, sizeof(stream));
-		deflateInit(&stream, Z_BEST_COMPRESSION);
-		size = deflateBound(&stream, len + hdrlen);
-		temp_obj = buf = xmalloc(size);
+	/* Set it up */
+	memset(&stream, 0, sizeof(stream));
+	deflateInit(&stream, Z_BEST_COMPRESSION);
+	size = deflateBound(&stream, len + hdrlen);
+	buf = xmalloc(size);
 
-		/* Compress it */
-		stream.next_out = buf;
-		stream.avail_out = size;
+	/* Compress it */
+	stream.next_out = buf;
+	stream.avail_out = size;
 		
-		/* First header.. */
-		stream.next_in = (void *)hdr;
-		stream.avail_in = hdrlen;
-		while (deflate(&stream, 0) == Z_OK)
-			/* nothing */;
+	/* First header.. */
+	stream.next_in = (void *)hdr;
+	stream.avail_in = hdrlen;
+	while (deflate(&stream, 0) == Z_OK)
+		/* nothing */;
 
-		/* Then the data itself.. */
-		stream.next_in = unpacked;
-		stream.avail_in = len;
-		while (deflate(&stream, Z_FINISH) == Z_OK)
-			/* nothing */;
-		deflateEnd(&stream);
-		free(unpacked);
-		
-		objsize = stream.total_out;
-	}
+	/* Then the data itself.. */
+	stream.next_in = unpacked;
+	stream.avail_in = len;
+	while (deflate(&stream, Z_FINISH) == Z_OK)
+		/* nothing */;
+	deflateEnd(&stream);
+	free(unpacked);
 
-	do {
-		size = write(fd, buf + posn, objsize - posn);
-		if (size <= 0) {
-			if (!size) {
-				fprintf(stderr, "write closed\n");
-			} else {
-				perror("write ");
-			}
-			return -1;
-		}
-		posn += size;
-	} while (posn < objsize);
+	*objsize = stream.total_out;
+	return buf;
+}
 
-	if (map)
-		munmap(map, objsize);
-	if (temp_obj)
-		free(temp_obj);
+int write_sha1_to_fd(int fd, const unsigned char *sha1)
+{
+	int retval;
+	unsigned long objsize;
+	void *buf = map_sha1_file_internal(sha1, &objsize);
 
-	return 0;
+	if (buf) {
+		retval = write_buffer(fd, buf, objsize);
+		munmap(buf, objsize);
+		return retval;
+	}
+
+	buf = repack_object(sha1, &objsize);
+	retval = write_buffer(fd, buf, objsize);    
+	free(buf);
+	return retval;
 }
 
 int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
@@ -1579,7 +1595,8 @@ int write_sha1_from_fd(const unsigned ch
 				SHA1_Update(&c, discard, sizeof(discard) -
 					    stream.avail_out);
 			} while (stream.avail_in && ret == Z_OK);
-			write(local, buffer, *bufposn - stream.avail_in);
+			if (write_buffer(local, buffer, *bufposn - stream.avail_in) < 0)
+				die("unable to write sha1 file");
 			memmove(buffer, buffer + *bufposn - stream.avail_in,
 				stream.avail_in);
 			*bufposn = stream.avail_in;

^ permalink raw reply related

* Re: Slow fetches of tags
From: Linus Torvalds @ 2006-05-24 16:45 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: git
In-Reply-To: <20060524131022.GA11449@linux-mips.org>



On Wed, 24 May 2006, Ralf Baechle wrote:
>
> I have a fairly large git tree (with a 320MB pack file containing some
> 700,000 objects).  A small fetch like
> 
>   git fetch git://www.kernel.org/pub/scm/linux/kernel/git/stable/\
>        linux-2.6.16.y.git master:v2.6.16-stable
> 
> which only fetches a handful of objects (v2.6.16.17 -> v2.6.16.18) will
> take on the order of 4-5 minutes.  Adding the "-n" option is will bring
> the operation down to under a second, so it really is just the tags
> that are slowing things down so much..

So this is a tree where you already _have_ most of the tags, no?

Can you add a printout to show what the "taglist" is for you in 
git-fetch.sh (just before the thing that does that

	fetch_main "$taglist"

thing?). It _should_ have pruned out all the tags you already have.

Or is it just the "git-ls-remote" that takes forever? (Or, if you run 
"top", is there something that is an obviously heavy operation on the 
client side?)

		Linus

^ permalink raw reply

* Re: Incremental cvsimports
From: Junio C Hamano @ 2006-05-24 17:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Martin Langhoff, git, geoff
In-Reply-To: <20060524135828.GA23934@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Odd. It's either a bug with importing tags in older versions, or there's
> some deep perl voodoo that I don't understand (either way, it is "fixed"
> in more recent versions).  Importing ENOENT directly is reasonable.

Sounds good.  Thanks for the back-and-forth helping others in
the community.  I appreciate it.

> Junio, can you apply the following fix?

Will do, but I would have preferred if you did the commit log
message and the stuff properly.  Less work for me ;-).

>
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index af331d9..76f6246 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -23,7 +23,7 @@ use File::Basename qw(basename dirname);
>  use Time::Local;
>  use IO::Socket;
>  use IO::Pipe;
> -use POSIX qw(strftime dup2 :errno_h);
> +use POSIX qw(strftime dup2 ENOENT);
>  use IPC::Open2;
>  
>  $SIG{'PIPE'}="IGNORE";

^ permalink raw reply

* Re: Slow fetches of tags
From: Linus Torvalds @ 2006-05-24 17:21 UTC (permalink / raw)
  To: Ralf Baechle, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605240931480.5623@g5.osdl.org>



On Wed, 24 May 2006, Linus Torvalds wrote:
> 
> Can you add a printout to show what the "taglist" is for you in 
> git-fetch.sh (just before the thing that does that
> 
> 	fetch_main "$taglist"
> 
> thing?). It _should_ have pruned out all the tags you already have.

Actually, looking at that tag-fetching logic, we already know that we have 
the objects that the tags point to (because those are the only kinds that 
we should auto-follow). I wonder if the slowness is because of all the 
have/want commit following, which walks the whole tree to say "I have 
this", when in this case we really should directly say "I have these" for 
the objects that the tags point to.

So the problem may be that we basically send a totally unnecessary list of 
all the objects we have, when the other end really only cares about the 
fact that we have the objects that the tags point to. Which we know we do, 
but we didn't say so, because "git-fetch" didn't really mark them that 
way.

And instead of sending the commits that we know we have, and that we know 
are the interesting ones and that will cut off the tag-object-walk, we 
start from all the local tips, and use the regular "parse commits in date 
order" thing and send "have" lines for everything we see that isn't 
common. Walking a lot of unnecessary crud.

Junio? Any ideas? I didn't want to do that tag-auto-following, and while I 
admit it's damn convenient, it's really quite broken, methinks. 

I almost suspect that we need to have a syntax where-by the local 
fetch-list ends up doing

	"$tagname:$tagname:$sha1wehave"

as the argument to fetch-pack, and then fetch-pack would be modified to 
send those "$sha1wehave" objects early as "have" objects. Ie start from 
something like

	diff --git a/git-fetch.sh b/git-fetch.sh
	index 280f62e..dce3812 100755
	--- a/git-fetch.sh
	+++ b/git-fetch.sh
	@@ -400,7 +400,7 @@ case "$no_tags$tags" in
	 			}
	 			git-cat-file -t "$sha1" >/dev/null 2>&1 || continue
	 			echo >&2 "Auto-following $name"
	-			echo ".${name}:${name}"
	+			echo ".${name}:${name}:${sha1}"
	 		done)
	 	esac
	 	case "$taglist" in

and then pass the info all the way up (the above patch will obviously 
result in a totally broken script, everything downstream from that point 
would have to be taught about the "already have this" part too).

Ralf, which repo is this, so that others (me, if I get the time and 
energy, Junio or some other hapless sucker^W^Whero if I'm lucky) can try 
things out?

		Linus

^ permalink raw reply

* Re: Slow fetches of tags
From: Ralf Baechle @ 2006-05-24 18:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605240931480.5623@g5.osdl.org>

On Wed, May 24, 2006 at 09:45:29AM -0700, Linus Torvalds wrote:

> So this is a tree where you already _have_ most of the tags, no?

Yes, git did end up only fetching v2.6.16.18 as the single tag.

> Can you add a printout to show what the "taglist" is for you in 
> git-fetch.sh (just before the thing that does that
> 
> 	fetch_main "$taglist"
> 
> thing?). It _should_ have pruned out all the tags you already have.

Right, it's just "refs/tags/v2.6.16.18:refs/tags/v2.6.16.18".

> Or is it just the "git-ls-remote" that takes forever?

git-ls-remote git://www.kernel.org/pub/scm/linux/kernel/git/stable/\
linux-2.6.16.y takes about 1.5s.

> (Or, if you run 
> "top", is there something that is an obviously heavy operation on the 
> client side?)

git-fetch-pack was burning some 6min CPU.  Nothing else even even shows
up on the "top" radar.

Another funny thing I noticed in top is that the git-fetch-pack arguments
got overwritten:

$ cat /proc/1702/cmdline | tr '\0' ' '
git-fetch-pack --thin git //www.kernel.org pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git  efs/heads/master  efs/tags/v2.6.16.18

Guess that doesn't matter.  Anyway, so I ran strace on this git-fetch-pack
invocation:

[...]
munmap(0xb7fe5000, 229)                 = 0
getdents(5, /* 0 entries */, 4096)      = 0
close(5)                                = 0
getdents(4, /* 0 entries */, 4096)      = 0
close(4)                                = 0
write(3, "0046want 9b549d8e1e2f16cffbb414a"..., 70) = 70
write(3, "0000", 4)                     = 4
write(3, "0032have 0bcf7932d0ea742e765a40b"..., 50) = 50
write(3, "0032have 54e938a80873e85f9c02ab4"..., 50) = 50
write(3, "0032have 2d0a9369c540519bab8018e"..., 50) = 50
write(3, "0032have bf3060065ef9f0a8274fc32"..., 50) = 50
write(3, "0032have 27602bd8de8456ac619b77c"..., 50) = 50
[... another 42,000+ similar lines chopped off ...]

9b549d8e1e2f16cffbb414a is Chris Wright's tag for v2.6.16.18.  So far,
as expected.

And this is where things are getting interesting:

$ git-name-rev 0bcf7932d0ea742e765a40b
0bcf7932d0ea742e765a40b master
$ git-name-rev 54e938a80873e85f9c02ab4
54e938a80873e85f9c02ab4 34k-2.6.16.18
$ git-name-rev 2d0a9369c540519bab8018e
2d0a9369c540519bab8018e 34k-2.6.16.18~1
$ git-name-rev bf3060065ef9f0a8274fc32
bf3060065ef9f0a8274fc32 34k-2.6.16.18~2
$ git-name-rev 27602bd8de8456ac619b77c
27602bd8de8456ac619b77c 34k-2.6.16.18~3

It's sending every object back to the start of history ...

  Ralf

^ permalink raw reply

* Re: Slow fetches of tags
From: Junio C Hamano @ 2006-05-24 18:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Ralf Baechle, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605240947580.5623@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> So the problem may be that we basically send a totally unnecessary list of 
> all the objects we have, when the other end really only cares about the 
> fact that we have the objects that the tags point to. Which we know we do, 
> but we didn't say so, because "git-fetch" didn't really mark them that 
> way.

I think this speculation is correct.  We should be able to do
better.

> I almost suspect that we need to have a syntax where-by the local 
> fetch-list ends up doing
>
> 	"$tagname:$tagname:$sha1wehave"
>
> as the argument to fetch-pack, and then fetch-pack would be modified to 
> send those "$sha1wehave" objects early as "have" objects.

But this logic has to be a bit more involved.

A "have" object is not just has_sha1_file(), but it needs to be
reachable from one of our tips we have already verified as
complete, so either the caller of fetch-pack does the
verification and give a verified $sha1wehave, or fetch-pack
takes $sha1weseemtohave and does its own verification and then
send it as one of the "have" objects (the issue is the same as
the one in my previous message to Eric W. Biederman -- we trust
only refs not just having a single object).

It might be useful to have a helper script you can give N object
names and M refs (and/or --all flag to mean "all of the refs"),
which returns the ones that are reachable from the given refs.
It would be even more useful if it were a helper function, but
given that the computation would involve walking the ancestry
chain, I suspect it would have a bad interaction with any user
of such a helper function that wants to do its own ancestry
walking, because many of them seem to assume an object that has
already been parsed are the ones they parsed for their own
purpose.

^ permalink raw reply

* Re: Clean up sha1 file writing
From: Matthias Lederhofer @ 2006-05-24 18:14 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605240820560.5623@g5.osdl.org>

> checking for partial writes
Just out of interest: is this to be safe on any OS or should this
be checked always?

> +		size = write(fd, buf, len);
> +		if (!size)
> +			return error("file write: disk full");
Shouldn't write to a full disk return -1 with ENOSPC?

^ permalink raw reply

* Re: Slow fetches of tags
From: Junio C Hamano @ 2006-05-24 18:41 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: git
In-Reply-To: <20060524180813.GA32519@linux-mips.org>

Ralf Baechle <ralf@linux-mips.org> writes:

>> Or is it just the "git-ls-remote" that takes forever?
>
> git-ls-remote git://www.kernel.org/pub/scm/linux/kernel/git/stable/\
> linux-2.6.16.y takes about 1.5s.

Good; that is as expected.  ls-remote over git protocol just
gets the initial "have" lines from the upload-pack and exits,
and there is no handshaking.

> Another funny thing I noticed in top is that the git-fetch-pack arguments
> got overwritten:
>
> $ cat /proc/1702/cmdline | tr '\0' ' '
> git-fetch-pack --thin git //www.kernel.org pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git  efs/heads/master  efs/tags/v2.6.16.18
>
> Guess that doesn't matter.

This is also expected - fetch-pack (connect.c::path_match(), actually) 
smudges the list of refs to remember which ones the caller asked
are going to be fulfilled and which ones are not.  Not the most
beautiful part of the code ;-).

> Guess that doesn't matter.  Anyway, so I ran strace on this git-fetch-pack
> invocation:
>
> [...]
> munmap(0xb7fe5000, 229)                 = 0
> getdents(5, /* 0 entries */, 4096)      = 0
> close(5)                                = 0
> getdents(4, /* 0 entries */, 4096)      = 0
> close(4)                                = 0
> write(3, "0046want 9b549d8e1e2f16cffbb414a"..., 70) = 70
> write(3, "0000", 4)                     = 4
> write(3, "0032have 0bcf7932d0ea742e765a40b"..., 50) = 50
> write(3, "0032have 54e938a80873e85f9c02ab4"..., 50) = 50
> write(3, "0032have 2d0a9369c540519bab8018e"..., 50) = 50
> write(3, "0032have bf3060065ef9f0a8274fc32"..., 50) = 50
> write(3, "0032have 27602bd8de8456ac619b77c"..., 50) = 50
> [... another 42,000+ similar lines chopped off ...]
>
> 9b549d8e1e2f16cffbb414a is Chris Wright's tag for v2.6.16.18.  So far,
> as expected.
>
> And this is where things are getting interesting:
>
> $ git-name-rev 0bcf7932d0ea742e765a40b
> 0bcf7932d0ea742e765a40b master
> $ git-name-rev 54e938a80873e85f9c02ab4
> 54e938a80873e85f9c02ab4 34k-2.6.16.18
> $ git-name-rev 2d0a9369c540519bab8018e
> 2d0a9369c540519bab8018e 34k-2.6.16.18~1
> $ git-name-rev bf3060065ef9f0a8274fc32
> bf3060065ef9f0a8274fc32 34k-2.6.16.18~2
> $ git-name-rev 27602bd8de8456ac619b77c
> 27602bd8de8456ac619b77c 34k-2.6.16.18~3
>
> It's sending every object back to the start of history ...

Is this "master" commit 0bcf79 part of v2.6.16.18 history?  If
not, how diverged are you?  That is, what does this command tell
you?

	git rev-list b7d0617..master | wc -l

Here, b7d0617 is the name of the commit object that is pointed
by v2.6.16.18 tag.

^ permalink raw reply

* Re: Clean up sha1 file writing
From: Linus Torvalds @ 2006-05-24 18:52 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: Git Mailing List
In-Reply-To: <E1Fixs4-0005pD-10@moooo.ath.cx>



On Wed, 24 May 2006, Matthias Lederhofer wrote:

> > checking for partial writes
>
> Just out of interest: is this to be safe on any OS or should this
> be checked always?

Any POSIX-conformant OS/filesystem should always do a full write for a 
regular file, unless a serious error happens.

HOWEVER. 

In practice, you can get partial writes at least over NFS (hey, it may not 
be posix, but it's _common_) when the filesystem has been mounted soft 
(and/or interruptible). And obviously if your file descriptor isn't a 
regular file, you can easily get partial writes.

Doing the loop is always safe, so it's worth doing it that way.

> > +		size = write(fd, buf, len);
> > +		if (!size)
> > +			return error("file write: disk full");
>
> Shouldn't write to a full disk return -1 with ENOSPC?

In that case, the "size < 0" check will catch it. The "return zero for 
full" case is an alternate error return (it happens for block device files 
at the end, it could happen for other things too). So the "returns zero 
means full" is the portable/safe thing to do.

		Linus

^ permalink raw reply

* Re: Slow fetches of tags
From: Junio C Hamano @ 2006-05-24 19:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Ralf Baechle
In-Reply-To: <Pine.LNX.4.64.0605240947580.5623@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Junio? Any ideas? I didn't want to do that tag-auto-following, and while I 
> admit it's damn convenient, it's really quite broken, methinks. 

I think the current setup is broken on two counts.  If you fetch
without remote tracking branch, I suspect that we end up asking
for the tip of the remote again -- because there is no ref that
says "this commit is known to be complete -- we just fetched
from them successfully".

But I think what Ralf is seeing is a bit different.  The example
given:

  git fetch git://git.kernel.org/pub/scm/linux/kernel/git/stable/\
       linux-2.6.16.y.git master:v2.6.16-stable

does use a tracking branch, and when the tag following kicks in,
v2.6.16-stable head should have been updated.  I suspect it is
just its head commit is older than tips of other branches, and
purely date based sorting done by fetch-pack.c::get_rev() ends
up walking them before it gets to the tip of the branch we just
fetched.

I wonder if we can do a dirty hack to give bias to commits
coming from refs that are newer (on the local filesystem -- that
is, mtime of .git/refs/heads/v2.6.16-stable must be a lot newer
than .git/refs/heads/master in this case because we just fetched
it)...

^ permalink raw reply

* Re: [PATCH 0/2] tagsize < 8kb restriction
From: Björn Engelmann @ 2006-05-24 19:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzmh81gfa.fsf@assigned-by-dhcp.cox.net>


>> 2.) Searching for a way to add objects to the database I spent quite a
>> while to find the right command. Don't you think it would be much more
>> intuitive having an
>>
>>     git-create-object [-t <type>] [-n] [-f] [-z] [--stdin] <file> [-r
>> <ref-name>]
>>
>> command for creating any type of object (-t blob as default).
>>     
>
> No, I do not think we would want to make it too easy and relaxed
> to create arbitrary object-looking thing.  Each type have
> defined format and semantics, and creation of an object of each
> type should be validated.  I do not want to encourage bypassing
> it by introducing such a backdoor.  The backdoor is easy to
> write, but I suspect it would actively harm us, instead of
> helping us, by encouraging "let's build a custom type of object,
> we do not care if other people would not understand it"
> mentality.
>   

Well, this is exactly what you have now in
    git-hash-object -w -t foo

That is why I said, all input should be validated by default. All I
proposed was
a) unify the tools in order to have less duplicate code (git-mktag,
git-mktree & git-hash-object do merely the same except for the
validating part)
b) remove the possibility to introduce unchecked objects of arbitrary
type (or only allow it with the -f = "force, use with caution"-option)

maybe I should have written "blob, tag, tree or commit" instead of
"arbitrary". I did not mean really arbitrary like it is implemented
right now in git-hash-object.

Bj

^ permalink raw reply

* Re: Slow fetches of tags
From: Linus Torvalds @ 2006-05-24 19:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ralf Baechle, Git Mailing List
In-Reply-To: <7v64jv8fdx.fsf@assigned-by-dhcp.cox.net>



On Wed, 24 May 2006, Junio C Hamano wrote:
> 
> A "have" object is not just has_sha1_file(), but it needs to be
> reachable from one of our tips we have already verified as
> complete

You're right.

And the strange part is that the commit we should give for the tag thing 
_should_ actually be pretty recent, and I wonder why we end up walking the 
whole damn tree history and saying "want" to basically them all. 

IOW, I think there's something more fundamentally wrong with the tag 
following. We _should_ have figured out much more quickly that we have it 
all.

I'm starting to suspect that it's actually a tag-specific problem: we do 
that reachability crud all by commit history, so the tags are a total 
special case, and if we don't send the proper HAVE/WANT for those or mark 
them properly with THEY_HAVE/COMMON etc, maybe the algorithm just gets 
confused.

I need to go pick up my youngest, so I'll be off-line on this for a while. 
Will try to think it through.

		Linus

^ permalink raw reply

* Re: [PATCH 0/2] tagsize < 8kb restriction
From: Junio C Hamano @ 2006-05-24 19:39 UTC (permalink / raw)
  To: Björn Engelmann; +Cc: git
In-Reply-To: <4474B10A.1020704@gmx.de>

Björn Engelmann <BjEngelmann@gmx.de> writes:

> That is why I said, all input should be validated by default. All I
> proposed was
> a) unify the tools in order to have less duplicate code
> (git-mktag, git-mktree & git-hash-object do merely the same
> except for the validating part)
> b) remove the possibility to introduce unchecked objects of arbitrary
> type (or only allow it with the -f = "force, use with caution"-option)
> maybe I should have written "blob, tag, tree or commit" instead of
> "arbitrary". I did not mean really arbitrary like it is implemented
> right now in git-hash-object.

Sorry, I forgot all about hash-objects X-<.  It was a convenient
way to try out new things such as 'gitlink'.  Thanks for the
clarification.

As to unification, I am not sure if there are a lot to unify.
Everybody starts with type, length and a LF, but after that each
type has its own format constraints.  A grand unified command
that knows about format constraints of every type under the sun
does not sound like a good approach.  While we have only handful
types (and I expect things will stay that way) it is not a big
deal either way, though.

And the common part is already shared (write_sha1_file_prepare()
and write_sha1_file() from sha1_file.c).

^ permalink raw reply

* Re: Clean up sha1 file writing
From: Junio C Hamano @ 2006-05-24 20:46 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605240820560.5623@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> ... On the other hand, from a longer-term 
> maintenance standpoint and from a "be much more careful when doing file 
> writes" standpoint, I think it's worth it.
>
> The re-write is "obviously correct" (famous last words) and is mostly 
> just moving code around and getting rid of a few temporaries that become 
> unnecessary as a result.
>
> The patch looks a bit messy: the changes aren't actually that big, but the 
> split-up and the resulting re-indentation makes the patch fairly 
> unreadable, so the cleanups are more obvious when you look at the 
> before-and-after side by side rather than when looking at the unified 
> diff..)

I usually work in text-only terminal, but with the above
warning, I did this:

	git cat-file -p HEAD^:sha1_file.c >/var/tmp/1
        xxdiff /var/tmp/1 sha1_file.c
        
with ignorespace and stuff enabled.  It was very pleasant to
read the changes that way, especially around write_sha1_to_fd()
vs repack_object().  xxdiff is my new friend.

^ permalink raw reply

* Re: A few stgit bugfixes
From: Catalin Marinas @ 2006-05-24 21:10 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <20060524060537.GA1173@diana.vm.bytemark.co.uk>

Karl Hasselström wrote:
> Fixes for a few bugs recently introduced in stgit by yours truly.

Applied. Thanks,

Catalin

^ permalink raw reply

* [PATCH] A Perforce importer for git.
From: Sean @ 2006-05-24 22:04 UTC (permalink / raw)
  To: git


Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca>
---
 Documentation/git-p4import.txt |  165 ++++++++++++++++++
 git-p4import.py                |  357 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 522 insertions(+), 0 deletions(-)


diff --git a/Documentation/git-p4import.txt b/Documentation/git-p4import.txt
new file mode 100644
index 0000000..b8ff1e9
--- /dev/null
+++ b/Documentation/git-p4import.txt
@@ -0,0 +1,165 @@
+git-p4import(1)
+===============
+
+NAME
+----
+git-p4import - Import a Perforce repository into git
+
+
+SYNOPSIS
+--------
+`git-p4import` [-q|-v] [--authors <file>] [-t <timezone>] <//p4repo/path> <branch>
+
+`git-p4import` --stitch <//p4repo/path>
+
+`git-p4import`
+
+
+DESCRIPTION
+-----------
+Import a Perforce repository into an existing git repository.  When
+a <//p4repo/path> and <branch> are specified a new branch with the
+given name will be created and the initial import will begin.
+
+Once the initial import is complete you can do an incremental import
+of new commits from the Perforce repository.  You do this by checking
+out the appropriate git branch and then running `git-p4import` without
+any options.
+
+The standard p4 client is used to communicate with the Perforce
+repository; it must be configured correctly in order for `git-p4import`
+to operate (see below).
+
+
+OPTIONS
+-------
+-q::
+	Do not display any progress information.
+
+-v::
+        Give extra progress information.
+
+\--authors::
+	Specify an authors file containing a mapping of Perforce user
+	ids to full names and email addresses (see Notes below).
+
+\--stitch::
+	Import the contents of the given perforce branch into the
+	currently checked out git branch.
+
+\--log::
+	Store debugging information in the specified file.
+
+-t::
+	Specify that the remote repository is in the specified timezone.
+	Timezone must be in the format "US/Pacific" or "Europe/London"
+	etc.  You only need to specify this once, it will be saved in
+	the git config file for the repository.
+
+<//p4repo/path>::
+	The Perforce path that will be imported into the specified branch.
+
+<branch>::
+	The new branch that will be created to hold the Perforce imports.
+
+
+P4 Client
+---------
+You must make the `p4` client command available in your $PATH and
+configure it to communicate with the target Perforce repository.
+Typically this means you must set the "$P4PORT" and "$P4CLIENT"
+environment variables.
+
+You must also configure a `p4` client "view" which maps the Perforce
+branch into the top level of your git repository, for example:
+
+------------
+Client: myhost
+
+Root:   /home/sean/import
+
+Options:   noallwrite clobber nocompress unlocked modtime rmdir
+
+View:
+        //public/jam/... //myhost/jam/...
+------------
+
+With the above `p4` client setup, you could import the "jam"
+perforce branch into a branch named "jammy", like so:
+
+------------
+$ mkdir -p /home/sean/import/jam
+$ cd /home/sean/import/jam
+$ git init-db
+$ git p4import //public/jam jammy
+------------
+
+
+Multiple Branches
+-----------------
+Note that by creating multiple "views" you can use `git-p4import`
+to import additional branches into the same git repository.
+However, the `p4` client has a limitation in that it silently
+ignores all but the last "view" that maps into the same local
+directory.  So the following will *not* work:
+
+------------
+View:
+        //public/jam/... //myhost/jam/...
+        //public/other/... //myhost/jam/...
+        //public/guest/... //myhost/jam/...
+------------
+
+If you want more than one Perforce branch to be imported into the
+same directory you must employ a workaround.  A simple option is
+to adjust your `p4` client before each import to only include a
+single view.
+
+Another option is to create multiple symlinks locally which all
+point to the same directory in your git repository and then use
+one per "view" instead of listing the actual directory.
+
+
+Tags
+----
+A git tag of the form p4/xx is created for every change imported from
+the Perforce repository where xx is the Perforce changeset number.
+Therefore after the import you can use git to access any commit by its
+Perforce number, eg. git show p4/327.
+
+The tag associated with the HEAD commit is also how `git-p4import`
+determines if their are new changes to incrementally import from the
+Perforce repository.
+
+If you import from a repository with many thousands of changes
+you will have an equal number of p4/xxxx git tags.  Git tags can
+be expensive in terms of disk space and repository operations.
+If you don't need to perform further incremental imports, you
+may delete the tags.
+
+
+Notes
+-----
+You can interrupt the import (eg. ctrl-c) at any time and restart it
+without worry.
+
+Author information is automatically determined by querying the
+Perforce "users" table using the id associated with each change.
+However, if you want to manually supply these mappings you can do
+so with the "--authors" option.  It accepts a file containing a list
+of mappings with each line containing one mapping in the format:
+
+------------
+    perforce_id = Full Name <email@address.com>
+------------
+
+
+Author
+------
+Written by Sean Estabrooks <seanlkml@sympatico.ca>
+
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/git-p4import.py b/git-p4import.py
new file mode 100644
index 0000000..74172ab
--- /dev/null
+++ b/git-p4import.py
@@ -0,0 +1,357 @@
+#!/usr/bin/python
+#
+# This tool is copyright (c) 2006, Sean Estabrooks.
+# It is released under the Gnu Public License, version 2.
+#
+# Import Perforce branches into Git repositories.
+# Checking out the files is done by calling the standard p4
+# client which you must have properly configured yourself
+#
+
+import marshal
+import os
+import sys
+import time
+import getopt
+
+from signal import signal, \
+   SIGPIPE, SIGINT, SIG_DFL, \
+   default_int_handler
+
+signal(SIGPIPE, SIG_DFL)
+s = signal(SIGINT, SIG_DFL)
+if s != default_int_handler:
+   signal(SIGINT, s)
+
+
+def die(msg, *args):
+    for a in args:
+        msg = "%s %s" % (msg, a)
+    print "git-p4import fatal error:", msg
+    sys.exit(1)
+
+def usage():
+    print "USAGE: git-p4import [-q|-v]  [--authors=<file>]  [-t <timezone>]  [//p4repo/path <branch>]"
+    sys.exit(1)
+
+verbosity = 1
+logfile = "/dev/null"
+ignore_warnings = False
+stitch = 0
+
+def report(level, msg, *args):
+    global verbosity
+    global logfile
+    for a in args:
+        msg = "%s %s" % (msg, a)
+    fd = open(logfile, "a")
+    fd.writelines(msg)
+    fd.close()
+    if level <= verbosity:
+        print msg
+
+class p4_command:
+    def __init__(self, _repopath):
+        try:
+            global logfile
+            self.userlist = {}
+            if _repopath[-1] == '/':
+                self.repopath = _repopath[:-1]
+            else:
+                self.repopath = _repopath
+            if self.repopath[-4:] != "/...":
+                self.repopath= "%s/..." % self.repopath
+            f=os.popen('p4 -V 2>>%s'%logfile, 'rb')
+            a = f.readlines()
+            if f.close():
+                raise
+        except:
+                die("Could not find the \"p4\" command")
+
+    def p4(self, cmd, *args):
+        global logfile
+        cmd = "%s %s" % (cmd, ' '.join(args))
+        report(2, "P4:", cmd)
+        f=os.popen('p4 -G %s 2>>%s' % (cmd,logfile), 'rb')
+        list = []
+        while 1:
+           try:
+                list.append(marshal.load(f))
+           except EOFError:
+                break
+        self.ret = f.close()
+        return list
+
+    def sync(self, id, force=False, trick=False, test=False):
+        if force:
+            ret = self.p4("sync -f %s@%s"%(self.repopath, id))[0]
+        elif trick:
+            ret = self.p4("sync -k %s@%s"%(self.repopath, id))[0]
+        elif test:
+            ret = self.p4("sync -n %s@%s"%(self.repopath, id))[0]
+        else:
+            ret = self.p4("sync    %s@%s"%(self.repopath, id))[0]
+        if ret['code'] == "error":
+             data = ret['data'].upper()
+             if data.find('VIEW') > 0:
+                 die("Perforce reports %s is not in client view"% self.repopath)
+             elif data.find('UP-TO-DATE') < 0:
+                 die("Could not sync files from perforce", self.repopath)
+
+    def changes(self, since=0):
+        try:
+            list = []
+            for rec in self.p4("changes %s@%s,#head" % (self.repopath, since+1)):
+                list.append(rec['change'])
+            list.reverse()
+            return list
+        except:
+            return []
+
+    def authors(self, filename):
+        f=open(filename)
+        for l in f.readlines():
+            self.userlist[l[:l.find('=')].rstrip()] = \
+                    (l[l.find('=')+1:l.find('<')].rstrip(),l[l.find('<')+1:l.find('>')])
+        f.close()
+        for f,e in self.userlist.items():
+                report(2, f, ":", e[0], "  <", e[1], ">")
+
+    def _get_user(self, id):
+        if not self.userlist.has_key(id):
+            try:
+                user = self.p4("users", id)[0]
+                self.userlist[id] = (user['FullName'], user['Email'])
+            except:
+                self.userlist[id] = (id, "")
+        return self.userlist[id]
+
+    def _format_date(self, ticks):
+        symbol='+'
+        name = time.tzname[0]
+        offset = time.timezone
+        if ticks[8]:
+            name = time.tzname[1]
+            offset = time.altzone
+        if offset < 0:
+            offset *= -1
+            symbol = '-'
+        localo = "%s%02d%02d %s" % (symbol, offset / 3600, offset % 3600, name)
+        tickso = time.strftime("%a %b %d %H:%M:%S %Y", ticks)
+        return "%s %s" % (tickso, localo)
+
+    def where(self):
+        try:
+            return self.p4("where %s" % self.repopath)[-1]['path']
+        except:
+            return ""
+
+    def describe(self, num):
+        desc = self.p4("describe -s", num)[0]
+        self.msg = desc['desc']
+        self.author, self.email = self._get_user(desc['user'])
+        self.date = self._format_date(time.localtime(long(desc['time'])))
+        return self
+
+class git_command:
+    def __init__(self):
+        try:
+            self.version = self.git("--version")[0][12:].rstrip()
+        except:
+            die("Could not find the \"git\" command")
+        try:
+            self.gitdir = self.get_single("rev-parse --git-dir")
+            report(2, "gdir:", self.gitdir)
+        except:
+            die("Not a git repository... did you forget to \"git init-db\" ?")
+        try:
+            self.cdup = self.get_single("rev-parse --show-cdup")
+            if self.cdup != "":
+                os.chdir(self.cdup)
+            self.topdir = os.getcwd()
+            report(2, "topdir:", self.topdir)
+        except:
+            die("Could not find top git directory")
+
+    def git(self, cmd):
+        global logfile
+        report(2, "GIT:", cmd)
+        f=os.popen('git %s 2>>%s' % (cmd,logfile), 'rb')
+        r=f.readlines()
+        self.ret = f.close()
+        return r
+
+    def get_single(self, cmd):
+        return self.git(cmd)[0].rstrip()
+
+    def current_branch(self):
+        try:
+            testit = self.git("rev-parse --verify HEAD")[0]
+            return self.git("symbolic-ref HEAD")[0][11:].rstrip()
+        except:
+            return None
+
+    def get_config(self, variable):
+        try:
+            return self.git("repo-config --get %s" % variable)[0].rstrip()
+        except:
+            return None
+
+    def set_config(self, variable, value):
+        try:
+            self.git("repo-config %s %s"%(variable, value) )
+        except:
+            die("Could not set %s to " % variable, value)
+
+    def make_tag(self, name, head):
+        self.git("tag -f %s %s"%(name,head))
+
+    def top_change(self, branch):
+        try:
+            a=self.get_single("name-rev --tags refs/heads/%s" % branch)
+            loc = a.find(' tags/') + 6
+            if a[loc:loc+3] != "p4/":
+                raise
+            return int(a[loc+3:][:-2])
+        except:
+            return 0
+
+    def update_index(self):
+        self.git("ls-files -m -d -o -z | git update-index --add --remove -z --stdin")
+
+    def checkout(self, branch):
+        self.git("checkout %s" % branch)
+
+    def repoint_head(self, branch):
+        self.git("symbolic-ref HEAD refs/heads/%s" % branch)
+
+    def remove_files(self):
+        self.git("ls-files | xargs rm")
+
+    def clean_directories(self):
+        self.git("clean -d")
+
+    def fresh_branch(self, branch):
+        report(1, "Creating new branch", branch)
+        self.git("ls-files | xargs rm")
+        os.remove(".git/index")
+        self.repoint_head(branch)
+        self.git("clean -d")
+
+    def basedir(self):
+        return self.topdir
+
+    def commit(self, author, email, date, msg, id):
+        self.update_index()
+        fd=open(".msg", "w")
+        fd.writelines(msg)
+        fd.close()
+        try:
+                current = self.get_single("rev-parse --verify HEAD")
+                head = "-p HEAD"
+        except:
+                current = ""
+                head = ""
+        tree = self.get_single("write-tree")
+        for r,l in [('DATE',date),('NAME',author),('EMAIL',email)]:
+            os.environ['GIT_AUTHOR_%s'%r] = l
+            os.environ['GIT_COMMITTER_%s'%r] = l
+        commit = self.get_single("commit-tree %s %s < .msg" % (tree,head))
+        os.remove(".msg")
+        self.make_tag("p4/%s"%id, commit)
+        self.git("update-ref HEAD %s %s" % (commit, current) )
+
+
+try:
+    opts, args = getopt.getopt(sys.argv[1:], "qhvt:",
+                    ["authors=","help","stitch=","timezone=","log=","ignore"])
+except getopt.GetoptError:
+    usage()
+
+for o, a in opts:
+    if o == "-q":
+        verbosity = 0
+    if o == "-v":
+        verbosity += 1
+    if o in ("--log"):
+        logfile = a
+    if o in ("-h", "--help"):
+        usage()
+    if o in ("--ignore"):
+        ignore_warnings = True
+
+git = git_command()
+branch=git.current_branch()
+
+for o, a in opts:
+    if o in ("-t", "--timezone"):
+        git.set_config("perforce.timezone", a)
+    if o in ("--stitch"):
+        git.set_config("perforce.%s.path" % branch, a)
+        stitch = 1
+
+if len(args) == 2:
+    branch = args[1]
+    git.checkout(branch)
+    if branch == git.current_branch():
+        die("Branch %s already exists!" % branch)
+    report(1, "Setting perforce to ", args[0])
+    git.set_config("perforce.%s.path" % branch, args[0])
+elif len(args) != 0:
+    die("You must specify the perforce //depot/path and git branch")
+
+p4path = git.get_config("perforce.%s.path" % branch)
+if p4path == None:
+    die("Do not know Perforce //depot/path for git branch", branch)
+
+p4 = p4_command(p4path)
+
+for o, a in opts:
+    if o in ("-a", "--authors"):
+        p4.authors(a)
+
+localdir = git.basedir()
+if p4.where()[:len(localdir)] != localdir:
+    report(1, "**WARNING** Appears p4 client is misconfigured")
+    report(1, "   for sync from %s to %s" % (p4.repopath, localdir))
+    if ignore_warnings != True:
+        die("Reconfigure or use \"--ignore\" on command line")
+
+if stitch == 0:
+    top = git.top_change(branch)
+else:
+    top = 0
+changes = p4.changes(top)
+count = len(changes)
+if count == 0:
+    report(1, "Already up to date...")
+    sys.exit(0)
+
+ptz = git.get_config("perforce.timezone")
+if ptz:
+    report(1, "Setting timezone to", ptz)
+    os.environ['TZ'] = ptz
+    time.tzset()
+
+if stitch == 1:
+    git.remove_files()
+    git.clean_directories()
+    p4.sync(changes[0], force=True)
+elif top == 0 and branch != git.current_branch():
+    p4.sync(changes[0], test=True)
+    report(1, "Creating new initial commit");
+    git.fresh_branch(branch)
+    p4.sync(changes[0], force=True)
+else:
+    p4.sync(changes[0], trick=True)
+
+report(1, "processing %s changes from p4 (%s) to git (%s)" % (count, p4.repopath, branch))
+for id in changes:
+    report(1, "Importing changeset", id)
+    change = p4.describe(id)
+    p4.sync(id)
+    git.commit(change.author, change.email, change.date, change.msg, id)
+    if stitch == 1:
+        git.clean_directories()
+        stitch = 0
+
-- 
1.3.3.g17cf3

^ permalink raw reply related

* What's in git.git
From: Junio C Hamano @ 2006-05-24 22:40 UTC (permalink / raw)
  To: git

* The 'master' branch has these since the last announcement.

 - lift length limits from mktag (Björn Engelmann)

 - misc fixes and cleanups (Alex Riesen, Björn Engelmann,
   Linus, Martin Waitz, Peter Eriksen, Sean Estabrooks)

 - git-svn updates, ignores svn:keywords (Eric Wong)

 - documentation updates (J. Bruce Fields)

 - cvsimport updates (Jeff King, Martin Langhoff)

 - built-in format-patch (Johannes Schindelin)

 - built-in many commands (Linus, Peter Eriksen, Timo Hirvonen, me)

 - remote tar-tree

 - "git status -u" (Matthias Lederhofer)

 - more cygwin portability bits (Yakov Lerner)

* The 'next' branch, in addition, has these.

 - handle patches at the beginning and the end of file properly
   (Catalin Marinas, Linus)

 - mailinfo updates (Eric W. Biederman).

 - cache-tree to speed up "apply & write-tree" cycle.

 - http-fetch possible segfault fix (Sean Estabrooks)

 - ref-log (Shawn Pearce)

^ permalink raw reply

* Re: [PATCH 5/5] Enable ref log creation in git checkout -b.
From: Junio C Hamano @ 2006-05-24 23:18 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060524035234.GA13329@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> I've swallowed all 10 and pushed them out in "pu", but could you
>> add tests to check the Porcelainish commands you touched with
>> this series to make sure they all log correctly?
>
> Sure.  I've been putting it off as I've been busy the past few days
> and have also been thinking about trying to rebuild reflog using a
> tag/annotation branch style, which might be more generally useful
> to others.

It appears that there is more serious breakage caused by the
lock_ref change.  http-fetch in "next" fails to clone, because
the call to lock-ref-sha1 in fetch.c::pull() forgets that the
program might be creating a new ref.

Another breakage I found (not related to ref-log) is that it
appears fetch.c, even in "master" branch [*1*], has current_ref
variable and does things depending on it, but nobody seems to
set that variable, so there are a lot of dead code that looks as
if they are doing something useful, enclosed in sections like:

	if (somethingelse && current_ref) {
        	dead code
	}

I'll probably revert the ref-log series from "next" in the next
round of updates, while killing the current_ref variable from
"master".


[Footnotes]

*1* It actually is worse than that.  Commit cd541a6 introduced
this variable but nobody touches it ever in the development
history of that variable.  I wonder what the original author and
the maintainer were smoking back then...

^ permalink raw reply

* Re: [PATCH 5/5] Enable ref log creation in git checkout -b.
From: Shawn Pearce @ 2006-05-24 23:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7wr3tc3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Junio C Hamano <junkio@cox.net> wrote:
> >> I've swallowed all 10 and pushed them out in "pu", but could you
> >> add tests to check the Porcelainish commands you touched with
> >> this series to make sure they all log correctly?
> >
> > Sure.  I've been putting it off as I've been busy the past few days
> > and have also been thinking about trying to rebuild reflog using a
> > tag/annotation branch style, which might be more generally useful
> > to others.
> 
> It appears that there is more serious breakage caused by the
> lock_ref change.  http-fetch in "next" fails to clone, because
> the call to lock-ref-sha1 in fetch.c::pull() forgets that the
> program might be creating a new ref.

Hmm.  I thought I was doing the same thing fetch used to do which
appeared to only work on refs which already exist, and not creating
new refs...
 
> Another breakage I found (not related to ref-log) is that it
> appears fetch.c, even in "master" branch [*1*], has current_ref
> variable and does things depending on it, but nobody seems to
> set that variable, so there are a lot of dead code that looks as
> if they are doing something useful, enclosed in sections like:
> 
> 	if (somethingelse && current_ref) {
>         	dead code
> 	}
 
I wondered about that code...

-- 
Shawn.

^ permalink raw reply

* Re: Clean up sha1 file writing
From: Linus Torvalds @ 2006-05-24 23:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vslmz5ewt.fsf@assigned-by-dhcp.cox.net>



On Wed, 24 May 2006, Junio C Hamano wrote:
>
> It was very pleasant to read the changes that way, especially around 
> write_sha1_to_fd() vs repack_object().  xxdiff is my new friend.

I think "kompare" (the KDE diff tool) is nicer.

I'm tolf xxdiff integrates with some of the other SCM tools well (svn and 
tla), so it people use xxdiff, maybe it could support the git way of doing 
things too..

		Linus

^ permalink raw reply

* Re: [PATCH 5/5] Enable ref log creation in git checkout -b.
From: Shawn Pearce @ 2006-05-24 23:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7wr3tc3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Junio C Hamano <junkio@cox.net> wrote:
> >> I've swallowed all 10 and pushed them out in "pu", but could you
> >> add tests to check the Porcelainish commands you touched with
> >> this series to make sure they all log correctly?
> >
> > Sure.  I've been putting it off as I've been busy the past few days
> > and have also been thinking about trying to rebuild reflog using a
> > tag/annotation branch style, which might be more generally useful
> > to others.
> 
> It appears that there is more serious breakage caused by the
> lock_ref change.  http-fetch in "next" fails to clone, because
> the call to lock-ref-sha1 in fetch.c::pull() forgets that the
> program might be creating a new ref.

The breakage is because of current_ref always being null.  The old
code would allow locking a non-existant ref in this case while the
new code was failing.  A simple change such as the following should
fix it:

-->8--
Fix fetch when using reflog.

Previously fetch was permitted to create refs if they did not exist;
this only worked as current_ref was always NULL and thus never
would get compared against the existing ref.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

---

2dad4178db978c01257fde949d808361589ee003
 fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

2dad4178db978c01257fde949d808361589ee003
diff --git a/fetch.c b/fetch.c
index fd57684..15110b8 100644
--- a/fetch.c
+++ b/fetch.c
@@ -213,7 +213,7 @@ int pull(char *target)
 	save_commit_buffer = 0;
 	track_object_refs = 0;
 	if (write_ref) {
-		lock = lock_ref_sha1(write_ref, current_ref, 1);
+		lock = lock_ref_sha1(write_ref, current_ref, 0);
 		if (!lock) {
 			error("Can't lock ref %s", write_ref);
 			return -1;
-- 
1.3.3.gfad60

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox