Git development
 help / color / mirror / Atom feed
* [PATCH] use xread where we are not checking for EAGAIN/EINTR
From: Andy Whitcroft @ 2007-01-05 10:54 UTC (permalink / raw)
  To: git


We have xread() to handle those OS's which will return EAGAIN or
EINTR when the read is interrupted.  We should use this where we
are not otherwise handling such errors.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---

    We have an xread() wrapper to help us with those nasty
    interrupt returns and yet we fail to use it consistently.
    This patch updates those plain read()'s which do not
    have any handling for errors, or which treat those errors
    as user visible fatal errors.

    This feels right to me, but perhaps there is some good
    reason that things are done this way ... if so could
    someone elighten me.

    If this is a sensible change, then I'll have a look at
    the write side.
---
diff --git a/dir.c b/dir.c
index 0338d6c..8fe0865 100644
--- a/dir.c
+++ b/dir.c
@@ -142,7 +142,7 @@ static int add_excludes_from_file_1(const char *fname,
 		return 0;
 	}
 	buf = xmalloc(size+1);
-	if (read(fd, buf, size) != size)
+	if (xread(fd, buf, size) != size)
 		goto err;
 	close(fd);
 
diff --git a/http-fetch.c b/http-fetch.c
index 396552d..50a3b00 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -175,7 +175,7 @@ static void start_object_request(struct object_request *obj_req)
 	prevlocal = open(prevfile, O_RDONLY);
 	if (prevlocal != -1) {
 		do {
-			prev_read = read(prevlocal, prev_buf, PREV_BUF_SIZE);
+			prev_read = xread(prevlocal, prev_buf, PREV_BUF_SIZE);
 			if (prev_read>0) {
 				if (fwrite_sha1_file(prev_buf,
 						     1,
diff --git a/http-push.c b/http-push.c
index ecefdfd..acb5c27 100644
--- a/http-push.c
+++ b/http-push.c
@@ -288,7 +288,7 @@ static void start_fetch_loose(struct transfer_request *request)
 	prevlocal = open(prevfile, O_RDONLY);
 	if (prevlocal != -1) {
 		do {
-			prev_read = read(prevlocal, prev_buf, PREV_BUF_SIZE);
+			prev_read = xread(prevlocal, prev_buf, PREV_BUF_SIZE);
 			if (prev_read>0) {
 				if (fwrite_sha1_file(prev_buf,
 						     1,
diff --git a/imap-send.c b/imap-send.c
index ad91858..3f1e542 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -224,7 +224,7 @@ socket_perror( const char *func, Socket_t *sock, int ret )
 static int
 socket_read( Socket_t *sock, char *buf, int len )
 {
-	int n = read( sock->fd, buf, len );
+	int n = xread( sock->fd, buf, len );
 	if (n <= 0) {
 		socket_perror( "read", sock, n );
 		close( sock->fd );
@@ -390,7 +390,7 @@ arc4_init( void )
 		fprintf( stderr, "Fatal: no random number source available.\n" );
 		exit( 3 );
 	}
-	if (read( fd, dat, 128 ) != 128) {
+	if (xread( fd, dat, 128 ) != 128) {
 		fprintf( stderr, "Fatal: cannot read random number source.\n" );
 		exit( 3 );
 	}
diff --git a/local-fetch.c b/local-fetch.c
index 7b6875c..21bcf75 100644
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -184,7 +184,7 @@ int fetch_ref(char *ref, unsigned char *sha1)
 		fprintf(stderr, "cannot open %s\n", filename);
 		return -1;
 	}
-	if (read(ifd, hex, 40) != 40 || get_sha1_hex(hex, sha1)) {
+	if (xread(ifd, hex, 40) != 40 || get_sha1_hex(hex, sha1)) {
 		close(ifd);
 		fprintf(stderr, "cannot read from %s\n", filename);
 		return -1;
diff --git a/path.c b/path.c
index 066f621..f6f9cfd 100644
--- a/path.c
+++ b/path.c
@@ -113,7 +113,7 @@ int validate_symref(const char *path)
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		return -1;
-	len = read(fd, buffer, sizeof(buffer)-1);
+	len = xread(fd, buffer, sizeof(buffer)-1);
 	close(fd);
 
 	/*
diff --git a/refs.c b/refs.c
index 121774c..f6afd61 100644
--- a/refs.c
+++ b/refs.c
@@ -284,7 +284,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		fd = open(path, O_RDONLY);
 		if (fd < 0)
 			return NULL;
-		len = read(fd, buffer, sizeof(buffer)-1);
+		len = xread(fd, buffer, sizeof(buffer)-1);
 		close(fd);
 
 		/*
diff --git a/sha1_file.c b/sha1_file.c
index d9622d9..0c9483c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1869,7 +1869,7 @@ int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
 			if (ret != Z_OK)
 				break;
 		}
-		size = read(fd, buffer + *bufposn, bufsize - *bufposn);
+		size = xread(fd, buffer + *bufposn, bufsize - *bufposn);
 		if (size <= 0) {
 			close(local);
 			unlink(tmpfile);
diff --git a/ssh-fetch.c b/ssh-fetch.c
index b006c5c..6ec9488 100644
--- a/ssh-fetch.c
+++ b/ssh-fetch.c
@@ -82,7 +82,7 @@ int fetch(unsigned char *sha1)
 		remote = conn_buf[0];
 		memmove(conn_buf, conn_buf + 1, --conn_buf_posn);
 	} else {
-		if (read(fd_in, &remote, 1) < 1)
+		if (xread(fd_in, &remote, 1) < 1)
 			return -1;
 	}
 	/* fprintf(stderr, "Got %d\n", remote); */
@@ -99,7 +99,7 @@ static int get_version(void)
 	char type = 'v';
 	write(fd_out, &type, 1);
 	write(fd_out, &local_version, 1);
-	if (read(fd_in, &remote_version, 1) < 1) {
+	if (xread(fd_in, &remote_version, 1) < 1) {
 		return error("Couldn't read version from remote end");
 	}
 	return 0;
@@ -111,10 +111,10 @@ int fetch_ref(char *ref, unsigned char *sha1)
 	char type = 'r';
 	write(fd_out, &type, 1);
 	write(fd_out, ref, strlen(ref) + 1);
-	read(fd_in, &remote, 1);
+	xread(fd_in, &remote, 1);
 	if (remote < 0)
 		return remote;
-	read(fd_in, sha1, 20);
+	xread(fd_in, sha1, 20);
 	return 0;
 }
 
diff --git a/ssh-upload.c b/ssh-upload.c
index 0b52ae1..3f2794c 100644
--- a/ssh-upload.c
+++ b/ssh-upload.c
@@ -23,7 +23,7 @@ static int serve_object(int fd_in, int fd_out) {
 	signed char remote;
 	int posn = 0;
 	do {
-		size = read(fd_in, sha1 + posn, 20 - posn);
+		size = xread(fd_in, sha1 + posn, 20 - posn);
 		if (size < 0) {
 			perror("git-ssh-upload: read ");
 			return -1;
@@ -54,7 +54,7 @@ static int serve_object(int fd_in, int fd_out) {
 
 static int serve_version(int fd_in, int fd_out)
 {
-	if (read(fd_in, &remote_version, 1) < 1)
+	if (xread(fd_in, &remote_version, 1) < 1)
 		return -1;
 	write(fd_out, &local_version, 1);
 	return 0;
@@ -67,7 +67,7 @@ static int serve_ref(int fd_in, int fd_out)
 	int posn = 0;
 	signed char remote = 0;
 	do {
-		if (read(fd_in, ref + posn, 1) < 1)
+		if (xread(fd_in, ref + posn, 1) < 1)
 			return -1;
 		posn++;
 	} while (ref[posn - 1]);
@@ -89,7 +89,7 @@ static void service(int fd_in, int fd_out) {
 	char type;
 	int retval;
 	do {
-		retval = read(fd_in, &type, 1);
+		retval = xread(fd_in, &type, 1);
 		if (retval < 1) {
 			if (retval < 0)
 				perror("git-ssh-upload: read ");
diff --git a/upload-pack.c b/upload-pack.c
index c568ef0..03a4156 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -242,7 +242,7 @@ static void create_pack_file(void)
 					*cp++ = buffered;
 					outsz++;
 				}
-				sz = read(pu_pipe[0], cp,
+				sz = xread(pu_pipe[0], cp,
 					  sizeof(data) - outsz);
 				if (0 < sz)
 						;
@@ -267,7 +267,7 @@ static void create_pack_file(void)
 				/* Status ready; we ship that in the side-band
 				 * or dump to the standard error.
 				 */
-				sz = read(pe_pipe[0], progress,
+				sz = xread(pe_pipe[0], progress,
 					  sizeof(progress));
 				if (0 < sz)
 					send_client_data(2, progress, sz);

^ permalink raw reply related

* a few remaining issues...
From: Junio C Hamano @ 2007-01-05 11:06 UTC (permalink / raw)
  To: git

This is not meant to be an exhaustive list, and I probably will
change my mind after I sleep on them, but before I go to bed,
here are a handful of glitches I think are worth fixing.

* Bare repository.

We have a heuristic to determine bareness and change our
behaviour (albeit slightly) based on it.  The heuristic is not
perfect, but the intent is to avoid things that are undesirable
for bare repository when we know (or guess) it is one, and allow
the repository owner to override if we guessed wrong.  Currently
the only such "undesirable thing" is the use of reflog even when
core.logallrefupdates is not set, but we have an RFC patch
floating around to forbid working-tree operations in a bare
repository to prevent accidents from happening.  I think at that
point it may be prudent to also give users a way to mark a bare
repository explicitly as such, say, with "core.bare = true".
Repository creation by init-db and clone with --bare option can
automatically set this, so adding this should not be too painful
for the users.

* Packed refs.

'git-pack-refs --all' leaves heads/ unpacked because they are
expected to change often, but it packs remotes/.  This does not
make any sense (it is another fallout from "separate remote"
layout that many people pushed, even though I was mildly against
it and mostly uninterested in it, and in the retrospect I think
they did not know about or knew but did not tell me about issues
like this, which makes me somewhat unhappy X-<).  I'd like to
change the command not to pack anything but tags/ hierarchy.
This keeps bases/ used by StGIT unpacked, which makes a lot of
sense -- the hierarchy is even more volatile than heads/.

'git-pack-refs' should default to --prune.  There is no point
not to, really.

'git-pack-refs' should probably learn how to unpack, although
there is no real need for it.

* Remote management.

I pushed out 'git-remote' in 'next' tonight, but as I said, I
think it does very limited things it should do in the current
shape.  What it involves is just scripting and requires no deep
core knowledge, so it might be a good project to enhance on for
new people.

Often people suggest "git checkout -b next origin/next" to add
branch.next.remote = origin and branch.next.merge = refs/heads/next.

I do not think it should be the default, but I do understand why
people would want this (what I mean is that I do not think -b
does not imply you would want to keep tracking and merging from
there for almost all the time -- rather I would suspect it would
be 50:50 thing), so I am not opposed to add an easy way to ask
for these two variables to be set up when the new branch is
made.  Perhaps "git checkout -B"?

* Handling paths that are unknown to the index.

I sent out patches tonight to teach "git reset <tree> -- <path>"
to restore the absense of path in the index from the tree
tonight.  There was another one recently brought up on the list:
"git commit -- <path>" for path that is no longer known to the
index.  While jumping the index is a practice I particularly do
not want to encourage by extending git to support it, we already
have support for most of the cases, so I think it makes sense to
do this for consistency.  I haven't thought about the necessary
changes yet, so people can beat me if they want to.  My vague
idea is to check HEAD to see if <path> exists and if so refrain
from complaining.

* Detached HEAD.

You've seen an experimental patch, discussion, and a few
follow-up patches, all in 'pu'.  I'm not actively looking at
this right now.

* Reflogs.

'git reflog show' needs to be done -- and preferrably in a way
that does not add too much code.

After rebasing a huge series, you need to know that N patches
were involved and have to say HEAD@{N}, instead of HEAD@{1}.
This is unfortunate --- we might want to find a way to make the
reflog's recording granularity match the user operation
granularity better.  But this is probably a fairly intrusive
change we would not want right now.

* Misconfigured "tracking branch" refspecs.

There is a special hack in git-pull that passes --update-head-ok
to git-fetch.  This is a workaround for a case where underlying
git-fetch is told to update the current branch that is also used
as a tracking branch.  This can happen either because the user
misconfigured "Pull: refs/heads/master:refs/heads/master", or
the user checked out a tracking branch to take a peek, and
forgot he was on such a branch and issued "git pull".  Both are
much less likely to happen in the separate remote layout, and I
think we should deprecate both --update-head-ok flag in
git-fetch and support for this situation in git-pull.

Instead, we should unconditionally allow fetching into the
current branch for bare repositories.

The reason we should not allow fetching into the current branch
for a repository with a working tree is that allowing so will
make the index and working tree useless.  Such a fetch updates
the value of HEAD, and after that happens the old value of HEAD
is lost and, there is no way to even run a 3-way merge between
the (old) HEAD, index and the working tree.  Even if the old
value of HEAD is kept somewhere before the fetch happens (which
is what --update-head-ok code allows git-pull to do), the
difference between old HEAD and HEAD needs to be propagated to
both index and the working tree separately, so it involves two
3-way merges, which is way complicated than it is really worth.

* Topic management.

In 'todo' branch there is a git-topic script I use to generate
the "What's cooking" messages.  Although the script in its
current form is probably too specific to my workflow (which has
one baseline with two development branches, and names topics in
certain ways -- the latter is customizable from the command
line), I think something like that may be useful for people who
need to manage multiple topic branches.

^ permalink raw reply

* Re: [PATCH 2/2] git-reset <tree> -- <path> restores absense of <path> in <tree>
From: Junio C Hamano @ 2007-01-05 11:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0701051128120.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Fri, 5 Jan 2007, Junio C Hamano wrote:
>
>>  * By the way, I noticed that "git log --diff-filter=A --
>>    t/t5401-update-hooks.sh" does not find the commit that
>>    introduced the path.  v1.4.1.1 seems to work but v1.4.2.4
>>    does not.  I haven't bisected it yet...
>
> It is this commit:
>
> 1798562: "log --raw: Don't descend into subdirectories by default"
>
> And sure enough,
>
> $ git log next -r --diff-filter=A -- t/t5401-update-hooks.sh
>
> works.

Soon after I accepted the patch, I had this exact reaction and
then after some digging I realized the workaround you showed
above.  And I forgot all of them X-<.  Silly me.

^ permalink raw reply

* Re: [PATCH] use xread where we are not checking for EAGAIN/EINTR
From: Junio C Hamano @ 2007-01-05 11:19 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <1cb8699724ff000fbf0c14ba3e15031e@pinky>

Andy Whitcroft <apw@shadowen.org> writes:

>     We have an xread() wrapper to help us with those nasty
>     interrupt returns and yet we fail to use it consistently.
>     This patch updates those plain read()'s which do not
>     have any handling for errors, or which treat those errors
>     as user visible fatal errors.
>
>     This feels right to me, but perhaps there is some good
>     reason that things are done this way ... if so could
>     someone elighten me.

Thanks.

I do not think any of the changes you did introduced new bugs,
but I think some of them are still wrong.  xread() protects us
from EINTR happening before any byte is read, but it can still
give a short read.  Many callers have a loop like this:

	do {
        	size = xread(...);
                yet_to_go -= size;
	} while (yet_to_go);

but some are not (e.g. add_excludes_from_file_1() in dir.c
expects xread() does not return before reading full buffer).

^ permalink raw reply

* Re: [PATCH 2/4] Improve cached content header of status output
From: Junio C Hamano @ 2007-01-05 11:22 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <459E2E57.6020503@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

>> +static void wt_status_print_cached_header(const char *reference)
>> +{
>> +	const char *c = color(WT_STATUS_HEADER);
>> +	color_printf_ln(c, "# Cached changes to be committed:");
>> ...
>
> We seem to be using 'Cached' and 'staged' here.  I thought we had
> updated all the docs to call files in the index "Staged for commit".

	# Changes staged to be committed:

Somebody did not like the verb "stage"; perhaps we can say:

	# You have added changes to these files to be committed:
	...
        # There are yet to be added changes to these files:
	...

?

^ permalink raw reply

* Re: [PATCH] Speedup recursive by flushing index only once for all entries
From: Alex Riesen @ 2007-01-05 11:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <7v8xgileza.fsf@assigned-by-dhcp.cox.net>

On 1/4/07, Junio C Hamano <junkio@cox.net> wrote:
> >> However, I was wondering if the index has to be written at all.
> >> I expect the written index (except the last one, of course) to have no
> >> user...
> >
> > Good question...
>
> That's most likely because you played safe, and started from the
> Python version whose only way to manipulate the index and write
> out a tree was to actually write the index out.

Yes, it was because of that. Just didn't thought about when to
update index at all (never really had a time).

> So let's step back a bit.

Excellent analysis, thanks! I suspect heavily it will work as is.
Now, if only someone could find time to code it up...

> By the way, Alex, you seem to heavily work on Cygwin, and I am
> interested in your experience with Shawn's sliding mmap() on
> real projects, as I suspect Cygwin would be more heavily
> impacted with any mmap() related changes.  You already said
> performance is "bearable".  Does that mean it was better and
> got worse but still bearable, or it was unusably slow but now it
> is bearably usable?

It is usably slow: ~30 sec for a commit (I stopped using normal
commit, using update-index and simplified index commit now),
around minute for the recursive merges (if they are simple),
~10 sec for a hot-cache hard reset. Avoiding gitk whenever
possible.

Compared to my only linux system here:

2-3 times slower in diff-tree for 44K files, around 9k differences
(0.2 sec against 0.6 sec, it quickly adds up if you do it often,
like when merging (it's slower for really big merges, constrained
by CPU and memory), commiting, gitk).

The windows machine is a corporate Lenovo 2.66MHz/1Gb,SATA laptop.
The linux machine is 1.2MHz/384Mb, IDE noname notebook.

I somehow adapted myself to it though (reading mails, drinking coffee).

^ permalink raw reply

* Re: a few remaining issues...
From: Alex Riesen @ 2007-01-05 11:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7iw1hgvt.fsf@assigned-by-dhcp.cox.net>

On 1/5/07, Junio C Hamano <junkio@cox.net> wrote:
> This is not meant to be an exhaustive list, and I probably will
> change my mind after I sleep on them, but before I go to bed,
> here are a handful of glitches I think are worth fixing.

Maybe we should at least mention another cygwin quirk:
cygwin (or is it its bash?) treats .exe files and +x-files without
extension somehow stupid: it prefers the file without extension
to the .exe. For example, after installation of git-merge-recursive
you have the old python script and git-merge-recursive.exe in
the same directory. Guess which one is used... Right, the old
python script. Same for count-objects and other recently
rewritten scripts.

^ permalink raw reply

* Re: [PATCH] use xread where we are not checking for EAGAIN/EINTR
From: Andy Whitcroft @ 2007-01-05 12:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvejlg1pg.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>     We have an xread() wrapper to help us with those nasty
>>     interrupt returns and yet we fail to use it consistently.
>>     This patch updates those plain read()'s which do not
>>     have any handling for errors, or which treat those errors
>>     as user visible fatal errors.
>>
>>     This feels right to me, but perhaps there is some good
>>     reason that things are done this way ... if so could
>>     someone elighten me.
> 
> Thanks.
> 
> I do not think any of the changes you did introduced new bugs,
> but I think some of them are still wrong.  xread() protects us
> from EINTR happening before any byte is read, but it can still
> give a short read.  Many callers have a loop like this:
> 
> 	do {
>         	size = xread(...);
>                 yet_to_go -= size;
> 	} while (yet_to_go);
> 
> but some are not (e.g. add_excludes_from_file_1() in dir.c
> expects xread() does not return before reading full buffer).

Yes, that is true.  I was going to fix that in the next step with the
writes.  But yes thats likely to involve them becoming 'read_in_full'
style thing and in fact churn us more.

Ignore this one and I'll look to do it 'right'.

-apw

^ permalink raw reply

* Re: [RFC] git-remote
From: Santi Béjar @ 2007-01-05 12:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfyarokk7.fsf@assigned-by-dhcp.cox.net>

+1. And much better than my RFC for "git clone --add".

Just some comments. I know it's in its early shape, tell me if you
want this kind of comments later.

* I think it is more coherent to list the tracked branches first and
second the "branch merges".

* In "git remote add <name> <remote>": git could use the remote url to
deduce a <name>, like what git-clone does.

* If a branch does not have a branch.<name>.remote git-remote does not
default it to "origin" and sends an:

Use of uninitialized value in string ne at
/home/santi/usr/stow/git/bin/git-remote line 222.

Maybe, in addition to this, git should require a branch.<name>.remote.

* With the config:

[remote "origin"]
        url = git://git2.kernel.org/pub/scm/git/git.git
        fetch = master:refs/remotes/origin/master
        fetch = next:refs/remotes/origin/next
        fetch = +refs/heads/pu:refs/remotes/origin/pu
        fetch = refs/heads/*:refs/remotes/origin/*

[branch "next"]
        merge=next

It outputs something as:
* remote origin
  URL: git://git2.kernel.org/pub/scm/git/git.git
  Tracked remote branches
    html maint man master next pu todo
  Tracked remote branches
    pu

* The first "Tracked ..." is for the wildcards and the second for the
explicit fetch. Maybe it should join the two or mark different as:

  Implicit tracked ...
  Tracked ...

* The next and master branches are missing because they are written
without the refs/heads/ prefix. And for this reason there is no:

Remote branch(es) merged with 'git pull' while on branch next
  next

(and the absence of branch.next.remote).

* In addition I would reformat this as:

Merges with 'git pull' while on branch:
  "next" merges "next"
  "topic" merges "master"
  ...

That's all for today :)

Santi

^ permalink raw reply

* Re: [PATCH 2/4] Improve cached content header of status output
From: Andy Whitcroft @ 2007-01-05 13:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6u9g1l4.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>> +static void wt_status_print_cached_header(const char *reference)
>>> +{
>>> +	const char *c = color(WT_STATUS_HEADER);
>>> +	color_printf_ln(c, "# Cached changes to be committed:");
>>> ...
>> We seem to be using 'Cached' and 'staged' here.  I thought we had
>> updated all the docs to call files in the index "Staged for commit".
> 
> 	# Changes staged to be committed:
> 
> Somebody did not like the verb "stage"; perhaps we can say:
> 
> 	# You have added changes to these files to be committed:
> 	...

# These files have changes and are marked for commit:

>         # There are yet to be added changes to these files:

# These files have changes but are not marked for commit:

> 	...

-apw

^ permalink raw reply

* Re: [RFC] git-remote
From: Johannes Schindelin @ 2007-01-05 13:53 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Junio C Hamano, git
In-Reply-To: <8aa486160701050459p4da586cdk2c2ddaabda5b4ca7@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 672 bytes --]

Hi,

On Fri, 5 Jan 2007, Santi Béjar wrote:

> * In "git remote add <name> <remote>": git could use the remote url to 
> deduce a <name>, like what git-clone does.

That does not make any sense. For example, I track 
"git://git.kernel.org/.../git.git" and "192.168.0.128:gits/git.git". 
Something very similar applies to the host name: if you track multiple 
Linux repos, chances are that most of them are on git.kernel.org.

I guess _if_ you have more than one upstream you are tracking (which is 
not the most common case, but hey, git-remote is for exactly that case) it 
is not uncommon to have similar urls.

IMHO Junio's proposal is as good as it gets.

Ciao,
Dscho

^ permalink raw reply

* Re: a few remaining issues...
From: Johannes Schindelin @ 2007-01-05 13:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7iw1hgvt.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 5 Jan 2007, Junio C Hamano wrote:

> * Reflogs.
> 
> 'git reflog show' needs to be done -- and preferrably in a way that does 
> not add too much code.

I do not have time to follow up on my earlier attempts to teach git-log 
about traversing reflogs instead of the commit parents. But Shawn had a 
working proposal, didn't he?

Ciao,
Dscho

^ permalink raw reply

* Re: [1/2 PATCH] git-svn: make multi-init less confusing
From: Seth Falcon @ 2007-01-05 15:38 UTC (permalink / raw)
  To: David Kågedal; +Cc: Eric Wong, git
In-Reply-To: <871wmaugh6.fsf@morpheus.local>

David Kågedal <davidk@lysator.liu.se> writes:
> Now if you could only clarify the documentation of dcommit to explain
> whether it creates one svn revision per commit in your branch, or if
> it creates a single svn revision with the full diff, and the
> documentation would be perfect :-)

dcommit creates one svn rev for each commit listed by:

   git log remotes/git-svn..HEAD

+ seth

^ permalink raw reply

* Re: [PATCH 2/2] git-reset <tree> -- <path> restores absense of <path> in <tree>
From: Juergen Ruehle @ 2007-01-05 16:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmz4xiz6t.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano writes:
 > When <path> exists in the index (either merged or unmerged), and
 > <tree> does not have it, git-reset should be usable to restore
 > the absense of it from the tree.  This implements it.

Great! It took some time, but I think I understand now why the first
part was even necessary:-)

 > diff --git a/git-reset.sh b/git-reset.sh
 > index a969370..76c8a81 100755
 > --- a/git-reset.sh
 > +++ b/git-reset.sh
 > @@ -44,8 +44,10 @@ if test $# != 0
 >  then
 >  	test "$reset_type" == "--mixed" ||
 >  		die "Cannot do partial $reset_type reset."
 > -	git ls-tree -r --full-name $rev -- "$@" |
 > -	git update-index --add --index-info || exit
 > +
 > +	git-diff-index --cached $rev -- "$@" |
 > +	sed -e 's/^:\([0-7][0-7]*\) [0-7][0-7]* \([0-9a-f][0-9a-f]*\) [0-9a-f][0-9a-f]* [A-Z]	\(.*\)$/\1 \2	\3/' |
 > +	git update-index --add --remove --index-info || exit

All other scripts calling update-index --index-info don't mention
add/remove (which are ignored anyway). Might be useful as a reminder
though.

 >  	git update-index --refresh
 >  	exit
 >  fi

^ permalink raw reply

* fatal: unable to create '.git/index': File exists
From: Len Brown @ 2007-01-05 16:50 UTC (permalink / raw)
  To: git

I kicked off a pull.
Realized I was on the wrong branch
and immediately did a ^C

Now I can't change branches:

# git checkout release
fatal: unable to create '.git/index': File exists
fatal: unable to create '.git/index': File exists

git reset --hard
does not help.

curiously, moving .git/index to /tmp and repeating
results in the same error.

clues?

thanks,
-Len

^ permalink raw reply

* Re: a few remaining issues...
From: Juergen Ruehle @ 2007-01-05 16:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7iw1hgvt.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano writes:
 > * Handling paths that are unknown to the index.
 > 
 > I sent out patches tonight to teach "git reset <tree> -- <path>"
 > to restore the absense of path in the index from the tree
 > tonight.  There was another one recently brought up on the list:
 > "git commit -- <path>" for path that is no longer known to the
 > index.  While jumping the index is a practice I particularly do
 > not want to encourage by extending git to support it, we already
 > have support for most of the cases, so I think it makes sense to
 > do this for consistency.  I haven't thought about the necessary
 > changes yet, so people can beat me if they want to.  My vague
 > idea is to check HEAD to see if <path> exists and if so refrain
 > from complaining.

This looks like a job for your para-walk topic. For this git-commit
case we just need a

  git-ls --no-workdir --error-unmatch HEAD -- <pattern>...

to replace git-ls-files. To my untrained eyes it looks like this is
mostly there (AFAICS it doesn't do pattern matching and the
--error-unmatch thingy).

^ permalink raw reply

* Re: [PATCH 2/4] Improve cached content header of status output
From: Juergen Ruehle @ 2007-01-05 17:14 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: Junio C Hamano, git
In-Reply-To: <459E4F2D.4000806@shadowen.org>

Andy Whitcroft writes:
 > Junio C Hamano wrote:
 > > Andy Whitcroft <apw@shadowen.org> writes:
 > > 
 > >>> +static void wt_status_print_cached_header(const char *reference)
 > >>> +{
 > >>> +	const char *c = color(WT_STATUS_HEADER);
 > >>> +	color_printf_ln(c, "# Cached changes to be committed:");
 > >>> ...
 > >> We seem to be using 'Cached' and 'staged' here.  I thought we had
 > >> updated all the docs to call files in the index "Staged for commit".
 > > 
 > > 	# Changes staged to be committed:
 > > 
 > > Somebody did not like the verb "stage"; perhaps we can say:
 > > 
 > > 	# You have added changes to these files to be committed:
 > > 	...
 > 
 > # These files have changes and are marked for commit:
 > 
 > >         # There are yet to be added changes to these files:
 > 
 > # These files have changes but are not marked for commit:

Does this better reflect that git tracks content and not files?

# Changes to these files will be committed:

# Changes to these files are not marked for commit:

BTW: how about also adding a hint how to review the changes in
question (i.e. diff --cached and diff; as an alternative to diff
--cached we could just advertise the --verbose switch to status and
commit).

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Chris Lee @ 2007-01-05 17:19 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <7v1wmalez6.fsf@assigned-by-dhcp.cox.net>

So, first up:

Using git-verify-pack from master does not fail. It actually does
verify the pack (after a pretty decent wait.) I should have tried
master first before sending out the first mail. :)

It takes about eleven minutes for git-verify-pack to complete, but it
does run to completion. So something that changed between 1.4.1 and
master made everything great again.

I haven't tried git-prune yet, but I'll report back with the results
from that next.

Junio: Did you still want me to try those steps with that patch
anyway, even though it works on master?

^ permalink raw reply

* Re: fatal: unable to create '.git/index': File exists
From: Manu @ 2007-01-05 17:30 UTC (permalink / raw)
  To: Len Brown; +Cc: git
In-Reply-To: <200701051150.09968.lenb@kernel.org>

Got this message once.
The reason was a git process still had a lock on the index file.
Make sure you have no git processes left running from the interruption
of git pull.

ps -ux | grep "git"

Manu

On 1/5/07, Len Brown <lenb@kernel.org> wrote:
> I kicked off a pull.
> Realized I was on the wrong branch
> and immediately did a ^C
>
> Now I can't change branches:
>
> # git checkout release
> fatal: unable to create '.git/index': File exists
> fatal: unable to create '.git/index': File exists
>
> git reset --hard
> does not help.
>
> curiously, moving .git/index to /tmp and repeating
> results in the same error.
>
> clues?
>
> thanks,
> -Len
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH 2/4] Improve cached content header of status output
From: Junio C Hamano @ 2007-01-05 18:42 UTC (permalink / raw)
  To: Juergen Ruehle; +Cc: Andy Whitcroft, git
In-Reply-To: <17822.34697.691000.253492@lapjr.intranet.kiel.bmiag.de>

Juergen Ruehle <j.ruehle@bmiag.de> writes:

> Andy Whitcroft writes:
>  > Junio C Hamano wrote:
>  > > 
>  > > Somebody did not like the verb "stage"; perhaps we can say:
>  > > 
>  > > 	# You have added changes to these files to be committed:
>  > > 	...
>  > 
>  > # These files have changes and are marked for commit:
>  > 
>  > >         # There are yet to be added changes to these files:
>  > 
>  > # These files have changes but are not marked for commit:
>
> Does this better reflect that git tracks content and not files?
>
> # Changes to these files will be committed:
>
> # Changes to these files are not marked for commit:

One of the goals is to find a pair of messages that make sense
when the same file appears on both lists.

> BTW: how about also adding a hint how to review the changes in
> question (i.e. diff --cached and diff; as an alternative to diff
> --cached we could just advertise the --verbose switch to status and
> commit).

Sounds sane.

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Junio C Hamano @ 2007-01-05 19:05 UTC (permalink / raw)
  To: Chris Lee; +Cc: git
In-Reply-To: <204011cb0701050919w2001105asefe2fd99165dfa95@mail.gmail.com>

"Chris Lee" <chris133@gmail.com> writes:

> Using git-verify-pack from master does not fail. It actually does
> verify the pack (after a pretty decent wait.) I should have tried
> master first before sending out the first mail. :)

Depends on which "master" -- I pushed out the "chuncked hashing"
fix on "master" as commit 8977c110 as part of the update last
night.

> Junio: Did you still want me to try those steps with that patch
> anyway, even though it works on master?

It would give us a confirmation that the above actually fixes
the problem, if your 1.4.1 fails to verify that same new pack
you just generated, on which you saw that the "master" (assuming
you mean the one with the above patch) works correctly.

If your "master" before 8977c110 already passes, then there is
something else going on, which would be worrysome.

^ permalink raw reply

* Re: a few remaining issues...
From: Shawn O. Pearce @ 2007-01-05 19:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0701051453520.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 5 Jan 2007, Junio C Hamano wrote:
> 
> > * Reflogs.
> > 
> > 'git reflog show' needs to be done -- and preferrably in a way that does 
> > not add too much code.
> 
> I do not have time to follow up on my earlier attempts to teach git-log 
> about traversing reflogs instead of the commit parents. But Shawn had a 
> working proposal, didn't he?

Yes, but my proposal added a lot of code.  Junio's comment is that
he would prefer one that doesn't, such as by reusing as much of the
revision listing machinary as possible.  Which you had looked into
doing.

I myself am also severly lacking in time right now.  *if* I get
any more Git time in the next week its going to be to finish out
some patches on the bash-completion, so I can try to sneak them
into the v1.5.0 release (if possible).  Its unlikely I'll be able
to look at a `reflog show` anytime soon.

-- 
Shawn.

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Chris Lee @ 2007-01-05 19:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vbqldfg56.fsf@assigned-by-dhcp.cox.net>

On 1/5/07, Junio C Hamano <junkio@cox.net> wrote:
> > Using git-verify-pack from master does not fail. It actually does
> > verify the pack (after a pretty decent wait.) I should have tried
> > master first before sending out the first mail. :)
>
> Depends on which "master" -- I pushed out the "chuncked hashing"
> fix on "master" as commit 8977c110 as part of the update last
> night.

Well, that would definitely explain it. :)

I did a fresh 'git pull' on master last night before I ran the
git-verify-pack, and that was around 11PM PST.

> > Junio: Did you still want me to try those steps with that patch
> > anyway, even though it works on master?
>
> It would give us a confirmation that the above actually fixes
> the problem, if your 1.4.1 fails to verify that same new pack
> you just generated, on which you saw that the "master" (assuming
> you mean the one with the above patch) works correctly.
>
> If your "master" before 8977c110 already passes, then there is
> something else going on, which would be worrysome.

The 'master' I had definitely included 8977c110. I can try it out with
the tip from before that commit, though, if you want.

Also, 'git-prune' took about 30 minutes to run to completion. Oddly,
git-prune didn't remove the older packs - does git-prune ignore packs?
'git-repack -a -d' did remove them.

^ permalink raw reply

* Re: Pushing into a repository with working directory?
From: Shawn O. Pearce @ 2007-01-05 19:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Whitcroft, git
In-Reply-To: <7vwt41j1le.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
> > Special casing the 'current' branch makes any sort of automated push
> > setup unreliable.  Indeed the special case preventing a fetch into the
> > current branch is pretty annoying for the same reason.  I would almost
> > prefer to relax that than add the same for push.
> 
> How would you relax the fetch case?  Fetching into the current
> branch, unless the repository is bare, is always a fishy
> operation.

And so is pushing into the current branch, so long as the current
branch has a working directory attached to it.

Most new users to Git expect to be able to push into the current
branch of a repository and `just have it work`.  Only they don't
really seem to have an idea of _how_ that operation should behave,
which means they really don't want it to work at all.  I certainly
don't want an operation to succeed if I can't reason about what
its success means!

Right now pushing into the current branch makes the index become
way out of sync from HEAD.  This causes git-runstatus to display a
large number of differences, basically undoing any of the changes
introduced by HEAD@{1}..HEAD.  The user is left with a dirty
working tree that they can commit - and committing it will just
revert the prior commits.  The user will later cuss at Git for
losing their changes.  Not pretty.

-- 
Shawn.

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Shawn O. Pearce @ 2007-01-05 19:39 UTC (permalink / raw)
  To: Chris Lee; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <204011cb0701051133r1ede14a6gd5093a3e7fa88cb5@mail.gmail.com>

Chris Lee <chris133@gmail.com> wrote:
> Also, 'git-prune' took about 30 minutes to run to completion. Oddly,
> git-prune didn't remove the older packs - does git-prune ignore packs?
> 'git-repack -a -d' did remove them.

git-prune is expensive.  Very expensive on very large projects,
as it must iterate every object to decide what is needed, before
it can start to remove objects that aren't needed.

Yes, it doesn't deal with removing pack files.  That's what the -d
to git-repack is for.

-- 
Shawn.

^ 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