git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re-instate index file write optimization
From: Linus Torvalds @ 2005-10-01 20:39 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List; +Cc: Wolfgang Denk


This makes "git-update-index" avoid the new index file write if it didn't 
make any changes to the index.

It still doesn't make things like "git status" be read-only operations in 
general, but if the index file doesn't need refreshing, it now will at 
least avoid making unnecessary changes.

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

This only makes sense after applying my previous patch, since otherwise a 
"--refresh" will always end up marking the index dirty anyway.

Also, while I tried to verify that we always mark the index dirty when we 
change it, I can't guarantee it. In particular, since we now map the index 
file with PROT_READ | PROT_WRITE, somebody can modify the index entries in 
place without getting a SIGSEGV.

Originally you couldn't do the modify-in-place, and used to be that you 
could depend on the active_cache[] being unmodified only by actually 
assigning to the pointer array itself.

I don't think git-update-index writes to the entries directly, though 
(git-read-tree does, but it doesn't do the optimization).

diff --git a/update-index.c b/update-index.c
--- a/update-index.c
+++ b/update-index.c
@@ -391,9 +391,11 @@ int main(int argc, const char **argv)
 			update_one(buf.buf, prefix, prefix_length);
 		}
 	}
-	if (write_cache(newfd, active_cache, active_nr) ||
-	    commit_index_file(&cache_file))
-		die("Unable to write new cachefile");
+	if (active_cache_changed) {
+		if (write_cache(newfd, active_cache, active_nr) ||
+		    commit_index_file(&cache_file))
+			die("Unable to write new cachefile");
+	}
 
 	return has_errors ? 1 : 0;
 }

^ permalink raw reply

* Re: Destructive side-effect of "cg-status"
From: Linus Torvalds @ 2005-10-01 20:24 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20051001194216.EE3E5353D8E@atlas.denx.de>



On Sat, 1 Oct 2005, Wolfgang Denk wrote:
> 
> The "error: open failed" should at leats include the  file  name  and
> the  errno/strerror  message.  Same  for  the "read_cache: Permission
> denied" - of course, if you knot the git internals you will know what
> this means, but the average user has no idea that he should check the
> permissions of .git/index.

Agreed.

Something like this would seem sane.

		Linus

---
Subject: Better error reporting for "git status"

Instead of "git status" ignoring (and hiding) potential errors from the 
"git-update-index" call, make it exit if it fails, and show the error.

In order to do this, use the "-q" flag (to ignore not-up-to-date files) 
and add a new "--unmerged" flag that allows unmerged entries in the index
without any errors.

This also avoids marking the index "changed" if an entry isn't actually 
modified, and makes sure that we exit with an understandable error message 
if the index is corrupt or unreadable. "read_cache()" no longer returns an 
error for the caller to check.

Finally, make die() and usage() exit with recognizable error codes, if we
ever want to check the failure reason in scripts.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/git-status.sh b/git-status.sh
--- a/git-status.sh
+++ b/git-status.sh
@@ -37,7 +37,7 @@ refs/heads/master) ;;
 *)	echo "# On branch $branch" ;;
 esac
 
-git-update-index --refresh >/dev/null 2>&1
+git-update-index -q --unmerged --refresh || exit
 
 if test -f "$GIT_DIR/HEAD"
 then
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -464,11 +464,15 @@ int read_cache(void)
 
 	errno = EBUSY;
 	if (active_cache)
-		return error("more than one cachefile");
+		return active_nr;
+
 	errno = ENOENT;
 	fd = open(get_index_file(), O_RDONLY);
-	if (fd < 0)
-		return (errno == ENOENT) ? 0 : error("open failed");
+	if (fd < 0) {
+		if (errno == ENOENT)
+			return 0;
+		die("index file open failed (%s)", strerror(errno));
+	}
 
 	size = 0; // avoid gcc warning
 	map = MAP_FAILED;
@@ -480,7 +484,7 @@ int read_cache(void)
 	}
 	close(fd);
 	if (map == MAP_FAILED)
-		return error("mmap failed");
+		die("index file mmap failed (%s)", strerror(errno));
 
 	hdr = map;
 	if (verify_hdr(hdr, size) < 0)
@@ -501,7 +505,7 @@ int read_cache(void)
 unmap:
 	munmap(map, size);
 	errno = EINVAL;
-	return error("verify header failed");
+	die("index file corrupt");
 }
 
 #define WRITE_BUFFER_SIZE 8192
diff --git a/update-index.c b/update-index.c
--- a/update-index.c
+++ b/update-index.c
@@ -13,7 +13,7 @@
  * like "git-update-index *" and suddenly having all the object
  * files be revision controlled.
  */
-static int allow_add = 0, allow_remove = 0, allow_replace = 0, not_new = 0, quiet = 0, info_only = 0;
+static int allow_add = 0, allow_remove = 0, allow_replace = 0, allow_unmerged = 0, not_new = 0, quiet = 0, info_only = 0;
 static int force_remove;
 
 /* Three functions to allow overloaded pointer return; see linux/err.h */
@@ -135,7 +135,7 @@ static struct cache_entry *refresh_entry
 
 	changed = ce_match_stat(ce, &st);
 	if (!changed)
-		return ce;
+		return NULL;
 
 	if (ce_modified(ce, &st))
 		return ERR_PTR(-EINVAL);
@@ -156,16 +156,20 @@ static int refresh_cache(void)
 		struct cache_entry *ce, *new;
 		ce = active_cache[i];
 		if (ce_stage(ce)) {
-			printf("%s: needs merge\n", ce->name);
-			has_errors = 1;
 			while ((i < active_nr) &&
 			       ! strcmp(active_cache[i]->name, ce->name))
 				i++;
 			i--;
+			if (allow_unmerged)
+				continue;
+			printf("%s: needs merge\n", ce->name);
+			has_errors = 1;
 			continue;
 		}
 
 		new = refresh_entry(ce);
+		if (!new)
+			continue;
 		if (IS_ERR(new)) {
 			if (not_new && PTR_ERR(new) == -ENOENT)
 				continue;
@@ -335,6 +339,10 @@ int main(int argc, const char **argv)
 				allow_remove = 1;
 				continue;
 			}
+			if (!strcmp(path, "--unmerged")) {
+				allow_unmerged = 1;
+				continue;
+			}
 			if (!strcmp(path, "--refresh")) {
 				has_errors |= refresh_cache();
 				continue;
diff --git a/usage.c b/usage.c
--- a/usage.c
+++ b/usage.c
@@ -15,7 +15,7 @@ static void report(const char *prefix, c
 void usage(const char *err)
 {
 	fprintf(stderr, "usage: %s\n", err);
-	exit(1);
+	exit(129);
 }
 
 void die(const char *err, ...)
@@ -25,7 +25,7 @@ void die(const char *err, ...)
 	va_start(params, err);
 	report("fatal: ", err, params);
 	va_end(params);
-	exit(1);
+	exit(128);
 }
 
 int error(const char *err, ...)

^ permalink raw reply

* Re: Destructive side-effect of "cg-status"
From: Wolfgang Denk @ 2005-10-01 19:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0510010934290.3378@g5.osdl.org>

In message <Pine.LNX.4.64.0510010934290.3378@g5.osdl.org>
Linus Torvalds wrote:
> 
> Also, arguably we should try to avoid writing the index file when not 
> necessary, although the fact is, that cg-status (and "git status") _do_ 
> need to actually keep it up-to-date in order to do the right thing. Also 
> true of some other programs that might otherwise appear to be read-only 
> (ie I've considered doing the same thing for "git diff").

But shouldn't it be possible to run such  commands  as  "status"  and
"diff" in a repository for which I have only read permissions? Or how
can  I find out about the status of another user's repository without
actually modifying it?

Also, error reporting is IMHO  not  sufficient  and  misleading.  For
example:

	$ git status 2>&1 | less
	error: open failed
	#
	# Updated but not checked in:
	#   (will commit)
	#
	#       deleted:  CHANGELOG
	#       deleted:  COPYING
	#       deleted:  CREDITS
	#       deleted:  MAINTAINERS
	#       deleted:  MAKEALL
	#       deleted:  Makefile
	#       deleted:  README
	...
	[all files in the repository flagged as "deleted" !]
	#
	error: open failed
	read_cache: Permission denied

The "error: open failed" should at leats include the  file  name  and
the  errno/strerror  message.  Same  for  the "read_cache: Permission
denied" - of course, if you knot the git internals you will know what
this means, but the average user has no idea that he should check the
permissions of .git/index.

Finally, a thick fat warning should be  added  to  the  documentation
that  these  commands  actually (may) modify the repository. This was
totally unexpected for me.

Thanks.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
In an infinite universe all things are possible, including the possi-
bility that the universe does not exist.
                        - Terry Pratchett, _The Dark Side of the Sun_

^ permalink raw reply

* Honor extractor's umask in git-tar-tree.
From: Junio C Hamano @ 2005-10-01 19:07 UTC (permalink / raw)
  To: git; +Cc: Rene Scharfe, Linus Torvalds
In-Reply-To: <7vr7b53y0n.fsf@assigned-by-dhcp.cox.net>

The archive generated with git-tar-tree had 0755 and 0644 mode bits.
This inconvenienced the extractor with umask 002 by robbing g+w bit
unconditionally.  Just write it out with loose permissions bits and
let the umask of the extractor do its job.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
Junio C Hamano <junkio@cox.net> writes:

> Unrelated to the topic at hand, but related to the mode bits --
> tar-tree generates archives with 0644/0755 permission bits.  It
> might not be a bad idea to just let the tar command honor umask
> of the extracter, by storing 0666 and 0777 in the archive.
>
> I always work in an environment where umask 002 is the norm, and
> get irritated when upstream tarballs of other peoples' projects
> create directories with mode 0755, making me do chmod 2775 on
> them.

diff --git a/tar-tree.c b/tar-tree.c
--- a/tar-tree.c
+++ b/tar-tree.c
@@ -353,6 +353,7 @@ static void traverse_tree(void *buffer, 
 
 		if (size < namelen + 20 || sscanf(buffer, "%o", &mode) != 1)
 			die("corrupt 'tree' file");
+		mode |= (mode & 0100) ? 0777 : 0666;
 		buffer = sha1 + 20;
 		size -= namelen + 20;
 

^ permalink raw reply

* [PATCH] Fix git+ssh's indefinite halts during long fetches
From: Dan Aloni @ 2005-10-01 18:39 UTC (permalink / raw)
  To: git, Junio C Hamano

The problem with the old implementation is that the socket input buffers
get full and then both ends halt waiting for each other. This take cares
of it, by buffering at the fetching side while still trying to send. It's
quite hackish but does the work (I managed to locally fetch the kernel with
its ~85000 objects that sum up to 250MB).

Signed-off-by: Dan Aloni <da-x@monatomic.org>

---
commit 958b0c00525fb63276430783dccb18316cec73c9
tree 4003cf1069e9ac4bbb8aade512047dd41a466ae2
parent 60fb5b2c4d9e26204f480f8a18ae1ff0051a6440
author Dan Aloni <da-x@monatomic.org> Sat, 01 Oct 2005 21:38:25 +0300
committer Dan Aloni <da-x@monatomic.org> Sat, 01 Oct 2005 21:38:25 +0300

 cache.h     |   18 +++++++-
 sha1_file.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 ssh-fetch.c |   76 ++++++++++++++++++++++++-----------
 3 files changed, 174 insertions(+), 49 deletions(-)

diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
@@ -214,8 +214,22 @@ extern int check_sha1_signature(const un
 /* Read a tree into the cache */
 extern int read_tree(void *buffer, unsigned long size, int stage, const char **paths);
 
-extern int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
-			      size_t bufsize, size_t *bufposn);
+struct input_segment {
+	unsigned char *buffer;
+	size_t size;
+
+	struct input_segment *next;
+	struct input_segment *prev;
+};
+
+struct input_buffer {
+	struct input_segment *first;
+	struct input_segment *last;
+};
+
+extern int read_input_buffers(int block, int fd_in, struct input_buffer *inputbuffer);
+extern int write_sha1_from_fd(const unsigned char *sha1, int fd, struct input_buffer *inputbuffer);
+
 extern int write_sha1_to_fd(int fd, const unsigned char *sha1);
 
 extern int has_sha1_pack(const unsigned char *sha1);
diff --git a/sha1_file.c b/sha1_file.c
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1413,11 +1413,88 @@ int write_sha1_to_fd(int fd, const unsig
 	return 0;
 }
 
-int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
-		       size_t bufsize, size_t *bufposn)
+int cut_last_input_segment(struct input_buffer *inputbuffer, struct input_segment *segment, size_t size_left)
 {
-	char *filename = sha1_file_name(sha1);
+	if (size_left) {
+		memmove(segment->buffer, segment->buffer + segment->size - size_left, size_left);
+		segment->size = size_left;
+		return 1;
+	}
+	
+	if (segment == inputbuffer->first) 
+		inputbuffer->first = NULL;
+
+	free(segment->buffer);
+	segment = segment->prev;
+	free(inputbuffer->last);
+	inputbuffer->last = segment;
+	return 0;
+}
+
+int read_input_buffers(int block, int fd_in, struct input_buffer *inputbuffer)
+{
+	unsigned long old_flags = 0;
+	size_t bufsize;
+	int readsize;
+	struct input_segment *segment;
+	int ret = 0;
+	unsigned char *buf;
+
+	if (!block) {
+		old_flags = fcntl(fd_in, F_GETFL);
+		fcntl(fd_in, F_SETFL, old_flags | O_NONBLOCK);
+	}
+
+	do {
+		bufsize = 0x10000;
+		buf = malloc(bufsize);
+		if (!buf) {
+			ret = -1;
+			break;
+		}
+		
+		readsize = read(fd_in, buf, bufsize);
+		if (readsize <= 0) {
+			free(buf);
+			if (readsize == 0) {
+				ret = -1;
+			}
+			break;
+		}
+			
+		buf = realloc(buf, readsize);
+		segment = (typeof(segment))(malloc(sizeof(*segment)));
+		if (!segment) {
+			free(buf);
+			ret = -1;
+			break;
+		}
+
+		segment->size = readsize;
+		segment->buffer = buf;
+		segment->next = inputbuffer->first;
+		segment->prev = NULL;
+
+		if (inputbuffer->first == NULL) {
+			inputbuffer->last = segment;
+		} else {
+			inputbuffer->first->prev = segment;
+		}
+		inputbuffer->first = segment;
+	} while (!block);
+
+	if (!block) {
+		fcntl(fd_in, F_SETFL, old_flags);
+	}
+
+	return ret;
+}
 
+extern int write_sha1_from_fd(const unsigned char *sha1, int fd, struct input_buffer *inputbuffer)
+{
+	char *filename = sha1_file_name(sha1);
+	int read_result_byte = 0;
+	signed char result_byte;
 	int local;
 	z_stream stream;
 	unsigned char real_sha1[20];
@@ -1437,10 +1514,20 @@ int write_sha1_from_fd(const unsigned ch
 	SHA1_Init(&c);
 
 	do {
-		ssize_t size;
-		if (*bufposn) {
-			stream.avail_in = *bufposn;
-			stream.next_in = (unsigned char *) buffer;
+		struct input_segment *segment = inputbuffer->last;
+		if (segment) {
+			if (!read_result_byte) {
+				int more_to_read;
+				result_byte = segment->buffer[0];
+				read_result_byte = 1;
+
+				more_to_read = cut_last_input_segment(inputbuffer, segment, segment->size - 1);
+				if (!more_to_read)
+					continue;
+			}
+
+			stream.avail_in = segment->size;
+			stream.next_in = segment->buffer;
 			do {
 				stream.next_out = discard;
 				stream.avail_out = sizeof(discard);
@@ -1448,23 +1535,19 @@ 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);
-			memmove(buffer, buffer + *bufposn - stream.avail_in,
-				stream.avail_in);
-			*bufposn = stream.avail_in;
-			if (ret != Z_OK)
-				break;
-		}
-		size = read(fd, buffer + *bufposn, bufsize - *bufposn);
-		if (size <= 0) {
-			close(local);
-			unlink(filename);
-			if (!size)
+
+			write(local, segment->buffer, segment->size - stream.avail_in);
+			cut_last_input_segment(inputbuffer, segment, stream.avail_in);
+ 			if (ret != Z_OK)
+ 				break;
+		} else {
+			ret = read_input_buffers(1, fd, inputbuffer);
+			if (ret) {
+				close(local);
+				unlink(filename);
 				return error("Connection closed?");
-			perror("Reading from connection");
-			return -1;
-		}
-		*bufposn += size;
+			}
+ 		}
 	} while (1);
 	inflateEnd(&stream);
 
diff --git a/ssh-fetch.c b/ssh-fetch.c
--- a/ssh-fetch.c
+++ b/ssh-fetch.c
@@ -14,57 +14,85 @@
 #include "fetch.h"
 #include "refs.h"
 
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <unistd.h>
+
 static int fd_in;
 static int fd_out;
 
 static unsigned char remote_version = 0;
 static unsigned char local_version = 1;
+static struct input_buffer inputbuffer = {NULL, };
 
-static ssize_t force_write(int fd, void *buffer, size_t length)
+static ssize_t force_write(void *buffer, size_t length)
 {
+	fd_set rfds, wfds;
 	ssize_t ret = 0;
+	int retval;
+	struct timeval tv;
+
+	FD_ZERO(&rfds);
+	FD_ZERO(&wfds);
+	FD_SET(fd_in, &rfds);
+	FD_SET(fd_out, &wfds);
+
 	while (ret < length) {
-		ssize_t size = write(fd, buffer + ret, length - ret);
-		if (size < 0) {
-			return size;
+		tv.tv_sec = 1;
+		tv.tv_usec = 0;
+		
+		retval = select(FD_SETSIZE, &rfds, &wfds, NULL, &tv);
+		if (!retval)
+			continue;
+
+		if (FD_ISSET(fd_in, &rfds)) {
+			read_input_buffers(0, fd_in, &inputbuffer);
+			continue;
 		}
-		if (size == 0) {
-			return ret;
+
+		if (FD_ISSET(fd_out, &wfds)) {
+			ssize_t size;
+			unsigned long old_flags = 0;
+
+			old_flags = fcntl(fd_out, F_GETFL);
+			fcntl(fd_out, F_SETFL, old_flags | O_NONBLOCK);
+			size = write(fd_out, buffer + ret, length - ret);
+			fcntl(fd_out, F_SETFL, old_flags);
+			
+			if (size < 0) {
+				return size;
+			}
+			if (size == 0) {
+				return ret;
+			}		
+	
+			ret += size;
 		}
-		ret += size;
 	}
+
 	return ret;
 }
 
 void prefetch(unsigned char *sha1)
 {
 	char type = 'o';
-	force_write(fd_out, &type, 1);
-	force_write(fd_out, sha1, 20);
+	read_input_buffers(0, fd_in, &inputbuffer);
+	force_write(&type, 1);
+	force_write(sha1, 20);
 	//memcpy(requested + 20 * prefetches++, sha1, 20);
 }
 
-static char conn_buf[4096];
-static size_t conn_buf_posn = 0;
-
 int fetch(unsigned char *sha1)
 {
 	int ret;
-	signed char remote;
 
-	if (conn_buf_posn) {
-		remote = conn_buf[0];
-		memmove(conn_buf, conn_buf + 1, --conn_buf_posn);
-	} else {
-		if (read(fd_in, &remote, 1) < 1)
-			return -1;
-	}
-	//fprintf(stderr, "Got %d\n", remote);
-	if (remote < 0)
-		return remote;
-	ret = write_sha1_from_fd(sha1, fd_in, conn_buf, 4096, &conn_buf_posn);
+	read_input_buffers(0, fd_in, &inputbuffer);
+	ret = write_sha1_from_fd(sha1, fd_in, &inputbuffer);
 	if (!ret)
 		pull_say("got %s\n", sha1_to_hex(sha1));
+
 	return ret;
 }
 

-- 
Dan Aloni
da-x@monatomic.org, da-x@colinux.org, da-x@gmx.net

^ permalink raw reply

* Re: Destructive side-effect of "cg-status"
From: Junio C Hamano @ 2005-10-01 18:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Wolfgang Denk, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0510010934290.3378@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> -	return open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0600);
> +	return open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0666);

Good spotting - thanks.  We tried to use 0666/0777 everywhere
and let umask do its job, but this was one of the two places we
still had 0[67]00.  I'd do the same for the other 0600 in
mailsplit.c, not that I think it matters, but just for
consistency.

Unrelated to the topic at hand, but related to the mode bits --
tar-tree generates archives with 0644/0755 permission bits.  It
might not be a bad idea to just let the tar command honor umask
of the extracter, by storing 0666 and 0777 in the archive.

I always work in an environment where umask 002 is the norm, and
get irritated when upstream tarballs of other peoples' projects
create directories with mode 0755, making me do chmod 2775 on
them.

^ permalink raw reply

* Re: Destructive side-effect of "cg-status"
From: Linus Torvalds @ 2005-10-01 16:41 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20050930160353.F025C352B7B@atlas.denx.de>



On Fri, 30 Sep 2005, Wolfgang Denk wrote:
>
> So far I  thought  "cg-status"  is  a  harmless  command  which  just
> displays  some  status information. It ain't so. One of our engineers
> reported a  corrupted  repository  after  I  ran  "cg-status"  in  his
> directory:

Well, it's not corrupted, but yes, the index file ends up unreadable.

> That means, that "cg-status" actually *rewrote* .git/index,  with  me
> (wd)  as  new  owner, and - ignoring my umask - with permissions that
> prevent the original owner (sr) to access the file!

The umask thing looks like a bug. Fixed thus.

Also, arguably we should try to avoid writing the index file when not 
necessary, although the fact is, that cg-status (and "git status") _do_ 
need to actually keep it up-to-date in order to do the right thing. Also 
true of some other programs that might otherwise appear to be read-only 
(ie I've considered doing the same thing for "git diff").

We used to have that optimization, but it was broken. I fixed it but 
disabled it for fear of other bugs.

But honoring umask would seem to be a no-brainer.

		Linus

----
diff --git a/index.c b/index.c
--- a/index.c
+++ b/index.c
@@ -29,7 +29,7 @@ int hold_index_file_for_update(struct ca
 		signal(SIGINT, remove_lock_file_on_signal);
 		atexit(remove_lock_file);
 	}
-	return open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0600);
+	return open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0666);
 }
 
 int commit_index_file(struct cache_file *cf)

^ permalink raw reply

* Re: Packing on kernel.org
From: H. Peter Anvin @ 2005-10-01 14:21 UTC (permalink / raw)
  To: Martin Coxall; +Cc: Git Mailing List, users
In-Reply-To: <068ea79fc648433faa44a6d4cc287614@cream.org>

Martin Coxall wrote:
> Was there an cron process or kernel.org that should be repacking the 
> public repositories periodically?

No, too many people complained.

> The git/cogito/sparse/linux-2.6 repositories all now have several 
> thousand unpacked objects a piece, and it takes so long to do an http 
> clone it's not even funny.

HARP: Please pack your repositories periodically.  PLEASE.  It matters 
especially now when kernel.org is down one server.

If your username is high on this list, it's imperative that you pack 
your trees:

brodo                 197469
wim                   184343
marcelo                68442
jgarzik                59860
lm                     39680
mpm                    38995
pavel                  37624
lenb                   36406
hch                    34037
davem                  27671
jejb                   23553
willy                  21626
pasky                  17019
sfrench                15912
smurf                  15236
acme                   12504
torvalds                8834
aegl                    7369
ericvh                  6750
roland                  6296
airlied                 6053
chrisw                  5619
axboe                   5221
dwmw2                   4101
gregkh                  3659
dtor                    3537
hpa                     3350
paulus                  2074
perex                   1999
bart                    1955
cvaroqui                1537
kay                     1250
junio                   1119
sam                     1073
kkeil                   1050

	-hpa

^ permalink raw reply

* Re: Destructive side-effect of "cg-status"
From: Martin Langhoff @ 2005-10-01 10:24 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: git
In-Reply-To: <20050930160353.F025C352B7B@atlas.denx.de>

On 10/1/05, Wolfgang Denk <wd@denx.de> wrote:
> So far I  thought  "cg-status"  is  a  harmless  command  which  just
> displays  some  status information. It ain't so. One of our engineers
> reported a  corrupted  repository  after  I  ran  "cg-status"  in  his
> directory:

Interesting... cg-status, and sometimes cg-diff, have to update the
index. The index is the trick behind git's performance (and some other
smarts). It's never really touched by Cogito, but by the git-*-index
commands.

Perhaps git-*-index commands should check ownership vs current uid and
print a warning?

cheers,


martin

^ permalink raw reply

* Re: git on OpenBSD
From: Martin Langhoff @ 2005-10-01  9:57 UTC (permalink / raw)
  To: Han Boetes; +Cc: git
In-Reply-To: <20051001062348.GA7903@boetes.org>

Hi,

There is an old version in the ports, but too old to be useful. A guy
at work got it going on NetBSD without a problem. I think the only
thing we changed was the shebang line in gitk because /bin/wish is
just a stupid script that complains and tells you to call
wish<version>.

cheers,


martin

^ permalink raw reply

* Packing on kernel.org
From: Martin Coxall @ 2005-10-01  9:02 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0509302320560.3378@g5.osdl.org>

Was there an cron process or kernel.org that should be repacking the 
public repositories periodically?

The git/cogito/sparse/linux-2.6 repositories all now have several 
thousand unpacked objects a piece, and it takes so long to do an http 
clone it's not even funny.

Martin

^ permalink raw reply

* Re: [howto] Kernel hacker's guide to git, updated
From: Junio C Hamano @ 2005-10-01  7:36 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jeff Garzik, Linux Kernel, git
In-Reply-To: <Pine.LNX.4.64.0509301112100.3378@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Hey, even more impressive is "git pull --all", which will happily try to 
> create an octopus of every single ref available at the other end.

True.

However, I think --all is a mistake even if you use it without
merging in 'git fetch', so I am not planning to do refs/heads/
side, at least not yet.  Even if you prevent an Octopus, what
would you do then?  If you choose to merge one of them, which
one?  Not merging any that is not explicitly specified on the
command line, seems to me the most sensible and safe option.

The rule for 'pull' to decide which refs to merge is:

 (1) if command line has explicit refspecs (--tags and --heads
     do not count), they are all merged.

 (2) if command line has no explicit refspecs (--tags and
     --heads do not count), the default one found from either
     remotes or branches file is merged.

Notice that I am forbidding remotes file to say "by default I
always merge these three heads from there to make an Octopus" by
the above rule (branches file cannot even name more than one
head so this is not an issue).  Since everybody seems to agree
that Octopus is not something that is done mechanically and
routinely anyway [*1*], I think this is a sensible way to guard
against accidental Octopus.

We could consider fetching all heads, by minimally renaming
remote master to origin and getting everything else under the
same name, but I'd really want to keep the local namespace for
branches isolated from each other.  Many kernel.org public
repositories seem to have 'test' and 'release' branches and if
you are a maintainer of such a tree, and if you are interested
in another maintainer's tree, and if that other maintainer has
the 'test' and 'release' branches, --heads (or --tags)
overwriting your 'test' with his 'test' is obviously not what
you want.

Possibly, something like this could be arranged later:

	* git fetch --heads=$ns $remote "$@"

	In addition to the usual refspecs (the rest of the
	command line arguments), fetch all remote heads and
	store remote refs/heads/$a under local refs/heads/$ns/$a
	for all $a.  If $ns is empty, remote "master" is renamed
	"origin".

	* git fetch --heads $remote "$@"

        shorthand for empty $ns

[Footnote]

*1* I do make many Octopus merges, but they happen across my
local topic branches.  Topics merged change day-by-day, and even
the set of topics alive at the time changes everyday.  IOW, it
is not something I would want to do with the same sets of heads
every time by describing them in the remotes file.

^ permalink raw reply

* Re: time to update with new packs
From: Junio C Hamano @ 2005-10-01  7:18 UTC (permalink / raw)
  To: Tony Luck; +Cc: git
In-Reply-To: <12c511ca0509302140o4263020bsc337594609175173@mail.gmail.com>

Tony Luck <tony.luck@gmail.com> writes:

> 1) Link the new pack files from Linus' objects/pack directory
> to my objects/pack directory.
>
> 2) $ GIT_DIR=. git prune-packed # in my directory
>
> 3) $ GIT_DIR=. git update-server-info # ditto

When everybody uses more recent clients, you could say
"/pub/scm/.../torvalds/linux-2.6.git/objects" in your
objects/info/alternates file and not worry about step (1).  git
native transport has known about the alternate mechanism since
the inception.  The alternate mechanism support in "git fetch"
and "git clone" for rsync and http transports is a relatively
recent addition (appeared just before 0.99.7).

So (1) is not necessary for git native transport once you have
"alternate" file, but for other transports (1) is probably still
helpful at this point.

Also, enabling hooks/post-update in your public repository would
save you from worrying about (3), provided if the only way you
update it is by pushing into it, which I think is what usually
people are doing.

^ permalink raw reply

* Re: [PATCH] HTTP partial transfer support for object, pack, and index transfers
From: Junio C Hamano @ 2005-10-01  7:17 UTC (permalink / raw)
  To: Nick Hengeveld; +Cc: git
In-Reply-To: <20050930232747.GB15593@reactrix.com>

Thanks.

^ permalink raw reply

* Re: [COGITO] cg-status in an empty repo spits out git-diff-index usage info
From: Junio C Hamano @ 2005-10-01  7:17 UTC (permalink / raw)
  To: Elfyn McBratney; +Cc: git mailing list
In-Reply-To: <20050930181809.GB13582@emcb.local>

Elfyn McBratney <beu@gentoo.org> writes:

> which I'm guessing isn't expected behaviour ;)  `git status` does the
> same, FWIW.

Yes, I noticed 'git status' did it that some time ago, and I
thought "gee, that was ugly".  But then I imagined what any
other realist maintainer would have said if I reported it as a
bug.

    Lbh xabj lbh'ir whfg perngrq lbhe ercb naq vg vf rzcgl.
    Jung qvq lbh rkcrpg sebz 'fgnghf' bhgchg?  Jub pnerf!  Naq
    ubj bsgra jbhyq lbh rkcrpg gb eha 'tvg fgnghf' va n arjyl
    perngrq ercbfvgbel sebz abj ba, abj lbh xabj jung lbh jbhyq
    frr?  Trg hfrq gb vg.

But I fixed it anyway, exactly a month ago.

^ permalink raw reply

* Re: Flag empty patches as errors
From: Junio C Hamano @ 2005-10-01  7:15 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0509302320560.3378@g5.osdl.org>

Thanks.  Applied.

^ permalink raw reply

* Re: git on OpenBSD
From: Junio C Hamano @ 2005-10-01  7:15 UTC (permalink / raw)
  To: Han Boetes; +Cc: git
In-Reply-To: <20051001062348.GA7903@boetes.org>

Han Boetes <han@mijncomputer.nl> writes:

> iconv is installed in /usr/local that's why I had to add it to the
> searchpath.

Is it the standard practice to have iconv in /usr/local on
OpenBSD, or is it just your particular setup?  If the former
that's fine, but if the latter, I am afraid that this change
does not belong to the PLATFOR_DEFINES of the Makefile.

I had to work this exact issue around on a borrowed Solaris box
today (eh, yesterday), and near the proposed update branch head
there is a commit to let you specify where to find iconv stuff
from the command line of the make (or gmake).  The box had some
home compiled stuff in unusual places, so the site specific
things are made configurable via make command line, without
hardcoding particular value in the Makefile.

In either case, thanks for the patch, and please let me know
about the /usr/local vs iconv issue.  If the answer is "yes
iconv is in /usr/local everywhere on a vanilla OpenBSD box",
then I'll queue this patch on top of my today's Solaris
portability patch.

^ permalink raw reply

* Flag empty patches as errors
From: Linus Torvalds @ 2005-10-01  6:25 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


A patch that contains no actual diff, and that doesn't change any 
meta-data is bad. It shouldn't be a patch at all, and git-apply shouldn't 
just accept it.

This caused a corrupted patch to be silently applied as an empty change in 
the kernel, because the corruption ended up making the patch look empty.

An example of such a patch is one that contains the patch header, but 
where the initial fragment header (the "@@ -nr,.." line) is missing, 
causing us to not parse any fragments.

The real "patch" program will also flag such patches as bad, with the 
message

	patch: **** Only garbage was found in the patch input.

and we should do likewise.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/apply.c b/apply.c
--- a/apply.c
+++ b/apply.c
@@ -723,6 +723,16 @@ static int parse_single_patch(char *line
 	return offset;
 }
 
+static inline int metadata_changes(struct patch *patch)
+{
+	return	patch->is_rename > 0 ||
+		patch->is_copy > 0 ||
+		patch->is_new > 0 ||
+		patch->is_delete ||
+		(patch->old_mode && patch->new_mode &&
+		 patch->old_mode != patch->new_mode);
+}
+
 static int parse_chunk(char *buffer, unsigned long size, struct patch *patch)
 {
 	int hdrsize, patchsize;
@@ -733,6 +743,9 @@ static int parse_chunk(char *buffer, uns
 
 	patchsize = parse_single_patch(buffer + offset + hdrsize, size - offset - hdrsize, patch);
 
+	if (!patchsize && !metadata_changes(patch))
+		die("patch with only garbage at line %d", linenr);
+
 	return offset + hdrsize + patchsize;
 }
 

^ permalink raw reply

* git on OpenBSD
From: Han Boetes @ 2005-10-01  6:23 UTC (permalink / raw)
  To: git

Hi,

I just managed to get git compile on OpenBSD,

I modified the Makefile a bit to make it work. I suppose these
modifications will make it work on Free- and NetBSD as well, but
I haven't tested it so I'm not sure.

iconv is installed in /usr/local that's why I had to add it to the
searchpath.


diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -162,6 +162,10 @@ ifeq ($(shell uname -s),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 endif
+ifeq ($(shell uname -s),OpenBSD)
+	NEEDS_LIBICONV = YesPlease
+	PLATFORM_DEFINES += -I/usr/local/include -L/usr/local/lib
+endif
 ifeq ($(shell uname -s),SunOS)
 	NEEDS_SOCKET = YesPlease
 	NEEDS_NSL = YesPlease




# Han
-- 
Lbh unir whfg ivbyngrq gur Qvtvgny Zvyyraavhz Pbclevtug Npg  ol  oernxvat  gur
cebgrpgvba bs pbclevtugrq zngrevny. Vs lbh ner abg n pvgvmra  be  erfvqrag  bs
gur HFN, lbh evfx orvat vzcevfbarq naq uryq jvgubhg onvy sbe hc gb  gjb  jrrxf
hcba ragel gb gur HFN (c) Copyright 2001 by Hartmann Schaffer (signature only)

^ permalink raw reply

* Re: local clone much slower than remote clone
From: Junio C Hamano @ 2005-10-01  5:57 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: git
In-Reply-To: <433DF862.4020500@pobox.com>

Jeff Garzik <jgarzik@pobox.com> writes:

> time git clone /spare/repo/linux-2.6 scsi-sas-2.6

    $ git clone -l -s -n

would be the fastest.

Yes, -n is an irrelevant cheating ;-).

^ permalink raw reply

* time to update with new packs
From: Tony Luck @ 2005-10-01  4:40 UTC (permalink / raw)
  To: git

I see that Linus made a new incremental pack when he
released 2.6.14-rc3 ... so to be nice to the kernel.org
mirroring daemons I should make use of them.

After the excitement I had last time with packs and trying to
make use of alternate directories, I thought I'd check the
process before I touched anything.

1) Link the new pack files from Linus' objects/pack directory
to my objects/pack directory.

2) $ GIT_DIR=. git prune-packed # in my directory

3) $ GIT_DIR=. git update-server-info # ditto

Ok?

-Tony

^ permalink raw reply

* Re: [PATCH] Support SPARSE in Makefile, better SPARSE_FLAGS
From: Pavel Roskin @ 2005-10-01  4:13 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Junio C Hamano, git
In-Reply-To: <433DB950.3010909@zytor.com>

Quoting "H. Peter Anvin" <hpa@zytor.com>:

> Pavel Roskin wrote:
> >
> > I know.  That's what I'm using in the wrapper (plus -m64 and some
> > warnings).  But it should be the default.  Until then, hassle-free
> > sparse support in the Makefile is only possible for the projects that
> > already know the architecture (e.g. the Linux kernel).
> >
>
> I think that's debatable.  It introduces main-compiler dependencies into
> sparse which is undesirable.

I see sparse is already moving in this direction.  Right now, it's somewhere in
the middle, which is quite inconvenient.  It hardcodes gcc version numbers but
not the architecture.

> A much simpler option would be to write a "sparsegcc" script which would
> be invoked just like gcc, extract the appropriate macro information
> based on options, and then invoke sparse.

I guess that's what cgcc is trying to be.  Since it also runs the compiler, no
special support for cgcc should be needed in Makefile other than using $(CC).

--
Regards,
Pavel Roskin

^ permalink raw reply

* Re: local clone much slower than remote clone
From: Yasushi SHOJI @ 2005-10-01  4:10 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Git Mailing List
In-Reply-To: <433DF862.4020500@pobox.com>

At Fri, 30 Sep 2005 22:45:54 -0400,
Jeff Garzik wrote:
> 
> Downloading an entire kernel tree over cable modem is almost a minute 
> faster than a local clone!  IMHO the local clone should just hardlink 
> the packs and objects, and be done with it.

    git clone -l

perhaps?  -s would be nice too.
--
        yashi

^ permalink raw reply

* local clone much slower than remote clone
From: Jeff Garzik @ 2005-10-01  2:45 UTC (permalink / raw)
  To: Git Mailing List


Case 1: remote clone

time git clone 
rsync://rsync.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git 
scsi-sas-2.6
...
real    3m36.432s
user    0m0.584s
sys     0m0.656s


Case 2: local clone
time git clone /spare/repo/linux-2.6 scsi-sas-2.6
...
Packing 99084 objects
Unpacking 99084 objects
...
real    4m31.876s
user    4m43.618s
sys     0m40.047s


Downloading an entire kernel tree over cable modem is almost a minute 
faster than a local clone!  IMHO the local clone should just hardlink 
the packs and objects, and be done with it.

	Jeff

^ permalink raw reply

* Re: [howto] Kernel hacker's guide to git, updated
From: Jeff Garzik @ 2005-10-01  0:17 UTC (permalink / raw)
  To: Horst von Brand; +Cc: Linux Kernel, Junio C Hamano, git
In-Reply-To: <200509301813.j8UIDXr5015488@laptop11.inf.utfsm.cl>

Horst von Brand wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Thanks for all the comments.  I just updated the KHGtG with the
>>feedback I received.  Go to
>>
>>	http://linux.yyz.us/git-howto.html
>>
>>and click reload.  Continued criticism^H^H^Hcomments welcome!
> 
> 
> - To know the current branch, "git branch" is enough (the one '*'-ed)

Click reload, this is already mentioned.


> - rsync(1) a repository is dangerous, it might catch it in the middle of
>   a update and give you an incomplete/messed up copy. Repeat rsync until no
>   change, perhaps?

Usually that's just unlucky.  I have caught kernel.org in the middle of 
a sync once, maybe twice.


> - I understand "git checkout -f" blows away any local changes, no questions
>   asked. Not very nice to suggest that to a newbie...

I constantly run into problems if I -do not- use the "-f" flag.  I 
habitually use it at all times, now.

Thanks,

	Jeff

^ 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;
as well as URLs for NNTP newsgroup(s).