* Re: using-topic-branches.txt
From: J. Bruce Fields @ 2005-12-19 15:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff Garzik, git
In-Reply-To: <7v64plmz1e.fsf@assigned-by-dhcp.cox.net>
On Sun, Dec 18, 2005 at 09:33:17PM -0800, Junio C Hamano wrote:
> Jeff Garzik <jgarzik@pobox.com> writes:
>
> > master.kernel.org is non-public, so it shouldn't be mentioned in
> > documentation...
>
> I'm willing to be persuaded otherwise, but the reason I changed
> this part was twofold:
>
> - The documentation is meant to be an example for subsystem
> maintainers, who are "non-public" people anyway.
There are people without kernel.org accounts who do similar sorts of
work.
--b.
^ permalink raw reply
* Re: Branches and all commits
From: Andreas Ericsson @ 2005-12-19 16:15 UTC (permalink / raw)
To: Jon Nelson; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512190908140.6812@gheavc.wnzcbav.cig>
Jon Nelson wrote:
> Should *all* commits be reachable via at least one branch? I ran into a
> situation this weekend that has me a little confused. I had performed a
> number of commits and such and I noticed that the author and committer
> info had suboptimal values. A bit of searching led me to a comment made
> by Linus that basically said "go hack git-convert-objects", which I did.
> After performing git-convert-objects on every commit object in
> .git/refs/heads and the requisite pruning, etc... just about everything
> looked fine. However, I still had a long series of commits that
> contained the wrong information. Further inspection makes it appear as
> though these commits are not reachable via any branch, although they are
> /all/ reachable via a series of tags. I worked around the problem by
> further modifying git-convert-objects to also understand tags (at least
> the basic ones I've got) and that's all taken care of, but the question
> remains: should *all* commit objects be reachable by at least one
> branch?
>
AFAIU, yes.
For future reference, what you should have done is this;
$ git format-patch --mbox <first-unscrewed-commit-ish>
# edit commit-messages in generated patches
$ git reset --hard <first-unscrewed-commit-ish>
$ for i in 00*.txt; do git apply < $i; done
$ git prune;# to get rid of the unreachable objects AFTER you've checked
everything's all right
If things fail, do
$ git reset --hard ORIG_HEAD
and ask again.
I'm afraid I can't help you fix up your repository from the state it's
in now. AFAIK, there's no tool to do it automagically.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] Fix unconditional early exit in cg-fetch
From: Paolo 'Blaisorblade' Giarrusso @ 2005-12-19 16:17 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
When invoking cg-fetch, after fetching tags it exits here, probably for an
overlooked error handling (at least I corrected it this way).
This means, for instance, we exit without reporting the tag updates and without
removing the "fetch in progress" marker - leading to unconditional "Recovering
from interrupted fetch" at the very beginning with rsync transport. Indeed, this
is fixed by this patch.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---
cg-fetch | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/cg-fetch b/cg-fetch
index a2865ae..a489834 100755
--- a/cg-fetch
+++ b/cg-fetch
@@ -190,8 +190,8 @@ fetch_tags()
if [ "$get" = "get_rsync" ]; then
$get -i -s -u -d "$uri/refs/tags" "$_git/refs/tags" ||
- echo "unable to get tags list (non-fatal)" >&2
- exit $?
+ (echo "unable to get tags list (non-fatal)" >&2;
+ exit $?)
fi
git-ls-remote --tags "$uri" |
^ permalink raw reply related
* Re: [PATCH] Fix race and deadlock when sending pack
From: Paul Serice @ 2005-12-19 16:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzmmxlkbq.fsf@assigned-by-dhcp.cox.net>
> It appears cpfd() seems to mostly duplicate what is in copy.c;
> is there any particular reason?
No, I just wasn't aware of it. I've made another patch to account for
it. I wonder why copy_fd() (usually) closes ifd though. I also
wonder why, if it is going to close ifd, doesn't it do so when there
an error is detected after the write() call.
I would also like to mention that I'm on the fence about all of this.
I think a lot of it is a matter of policy, and I'm just not sure what
the policy is regarding the hooks. I wanted to submit the patch in it
full form to show that it is possible to redirect stdout of the hooks
to the terminal. I also feel that, even if the patch is ultimately
rejected (no hard feelings :-), it would be interesting to know what's
the underlying problem.
Paul Serice
========================================================================
Subject: [PATCH] Fix race and deadlock when sending pack.
The best way to reproduce the problem is to locally clone your
repository. When you perform a push, git-send-pack will directly set
up pipes connected to stdin and stdout of git-receive-pack. You
should then set up hook/post-update or hook/update to try to write
lots of text to stdout. (You want to use the local protocol because
ssh is robust enough to mask the worst behavior.)
The first problem is that git-send-pack closes git-receive-pack's
stdout (which is inherited by the hooks) immediately after sending the
pack. This almost always causes the hooks to receive SIGPIPE when
they try to write to stdout.
After fixing the SIGPIPE problem, you then run into a deadlock because
git-send-pack is blocked trying to reap git-receive-pack and
git-receive-pack (or one of its hooks) is blocked waiting for
git-send-pack to read its output.
I've also added an example a one-liner to both hooks demonstrating how
to redirect all subsequent output to stderr. Because
git-receive-pack's stderr is not redirected, it has always been safe
to write to stderr. Thus, all current status related output appears
on stderr. This can lead to confusing ordering of messages if only
the hooks are using stdout. The patch has the one-liner commented
out, but perhaps it should be enabled by default.
In addition, this commit reverts the work-around provided by
128aed684d0b3099092b7597c8644599b45b7503 which redirected stdin and
stdout to /dev/null.
Signed-off-by: Paul Serice <paul@serice.net>
---
receive-pack.c | 2 +-
run-command.c | 27 +++++++++++++--------------
run-command.h | 3 ---
send-pack.c | 9 ++++++++-
templates/hooks--post-update | 4 ++++
templates/hooks--update | 4 ++++
6 files changed, 30 insertions(+), 19 deletions(-)
87b7ee91cbe1b90bbd0937a85595de3933fc9459
diff --git a/receive-pack.c b/receive-pack.c
index cbe37e7..1873506 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -173,7 +173,7 @@ static void run_update_post_hook(struct
argc++;
}
argv[argc] = NULL;
- run_command_v_opt(argc, argv, RUN_COMMAND_NO_STDIO);
+ run_command_v(argc, argv);
}
/*
diff --git a/run-command.c b/run-command.c
index 8bf5922..38cd6cb 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,19 +2,23 @@
#include "run-command.h"
#include <sys/wait.h>
-int run_command_v_opt(int argc, char **argv, int flags)
+int run_command_v(int argc, char **argv)
{
- pid_t pid = fork();
+
+ pid_t pid = (pid_t)-1;
+
+ /* Because each process has independent buffering, if you
+ * don't flush before the fork, it can seem like the new
+ * output for the child occurs before the old output of the
+ * parent which can be confusing at times. */
+ fflush(stdout);
+ fflush(stderr);
+
+ pid = fork();
if (pid < 0)
return -ERR_RUN_COMMAND_FORK;
if (!pid) {
- if (flags & RUN_COMMAND_NO_STDIO) {
- int fd = open("/dev/null", O_RDWR);
- dup2(fd, 0);
- dup2(fd, 1);
- close(fd);
- }
execvp(argv[0], (char *const*) argv);
die("exec %s failed.", argv[0]);
}
@@ -42,11 +46,6 @@ int run_command_v_opt(int argc, char **a
}
}
-int run_command_v(int argc, char **argv)
-{
- return run_command_v_opt(argc, argv, 0);
-}
-
int run_command(const char *cmd, ...)
{
int argc;
@@ -65,5 +64,5 @@ int run_command(const char *cmd, ...)
va_end(param);
if (MAX_RUN_COMMAND_ARGS <= argc)
return error("too many args to run %s", cmd);
- return run_command_v_opt(argc, argv, 0);
+ return run_command_v(argc, argv);
}
diff --git a/run-command.h b/run-command.h
index 2469eea..5ee0972 100644
--- a/run-command.h
+++ b/run-command.h
@@ -11,9 +11,6 @@ enum {
ERR_RUN_COMMAND_WAITPID_NOEXIT,
};
-#define RUN_COMMAND_NO_STDIO 1
-
-int run_command_v_opt(int argc, char **argv, int opt);
int run_command_v(int argc, char **argv);
int run_command(const char *cmd, ...);
diff --git a/send-pack.c b/send-pack.c
index 6ce0d9f..5a99ba9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -319,8 +319,15 @@ int main(int argc, char **argv)
if (pid < 0)
return 1;
ret = send_pack(fd[0], fd[1], nr_heads, heads);
- close(fd[0]);
+
+ /* Close our side of the conversation. Wait for the child to
+ * close its side of the conversation (copying the remainder
+ * to our stdout). Note that copy_fd() has the side effect of
+ * closing fd[0]. */
close(fd[1]);
+ copy_fd(fd[0], fileno(stdout));
+
finish_connect(pid);
+
return ret;
}
diff --git a/templates/hooks--post-update b/templates/hooks--post-update
index bcba893..d470dcc 100644
--- a/templates/hooks--post-update
+++ b/templates/hooks--post-update
@@ -5,4 +5,8 @@
#
# To enable this hook, make this file executable by "chmod +x post-update".
+# If your stdout and stderr messages are interleaved, uncomment the
+# following line.
+#exec 1>&2
+
exec git-update-server-info
diff --git a/templates/hooks--update b/templates/hooks--update
index 6db555f..6199deb 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -8,6 +8,10 @@
# (2) make this file executable by "chmod +x update".
#
+# If your stdout and stderr messages are interleaved, uncomment the
+# following line.
+#exec 1>&2
+
recipient="commit-list@example.com"
if expr "$2" : '0*$' >/dev/null
--
0.99.9.GIT
^ permalink raw reply related
* Re: [PATCH] Fix race and deadlock when sending pack
From: Paul Serice @ 2005-12-19 17:29 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0512190130450.25300@iabervon.org>
> If, for some reason, a hook looked at stdin, it could get surprising
> results.
In the first patch, the hook would hang waiting for input. That was a
bug. In the second patch, the hook's standard input has been closed
by git-send-pack (which is the standard thing for the parent to do in
a pipeline), thus if a hook looks at stdin it will get EOF (the same
as if connected to /dev/null).
Paul
^ permalink raw reply
* Re: Branches and all commits
From: Jon Nelson @ 2005-12-19 17:39 UTC (permalink / raw)
Cc: git
In-Reply-To: <43A6DC90.3040403@op5.se>
On Mon, 19 Dec 2005, Andreas Ericsson wrote:
> Jon Nelson wrote:
> > Should *all* commits be reachable via at least one branch? I ran into a
> > situation this weekend that has me a little confused. I had performed a
> > number of commits and such and I noticed that the author and committer info
> > had suboptimal values. A bit of searching led me to a comment made by Linus
> > that basically said "go hack git-convert-objects", which I did. After
> > performing git-convert-objects on every commit object in .git/refs/heads and
> > the requisite pruning, etc... just about everything looked fine. However, I
> > still had a long series of commits that contained the wrong information.
> > Further inspection makes it appear as though these commits are not reachable
> > via any branch, although they are /all/ reachable via a series of tags. I
> > worked around the problem by further modifying git-convert-objects to also
> > understand tags (at least the basic ones I've got) and that's all taken care
> > of, but the question remains: should *all* commit objects be reachable by at
> > least one branch?
> >
>
> AFAIU, yes.
>
> For future reference, what you should have done is this;
>
> $ git format-patch --mbox <first-unscrewed-commit-ish>
> # edit commit-messages in generated patches
> $ git reset --hard <first-unscrewed-commit-ish>
> $ for i in 00*.txt; do git apply < $i; done
> $ git prune;# to get rid of the unreachable objects AFTER you've checked
> everything's all right
>
> If things fail, do
>
> $ git reset --hard ORIG_HEAD
>
> and ask again.
>
> I'm afraid I can't help you fix up your repository from the state it's
> in now. AFAIK, there's no tool to do it automagically.
The repository seems just fine with this single exception - no branch
contains a reference to the commit that forms the chain of commits that
would otherwise be described as a branch. As I understand it, then, the
only thing that is missing is an entry in .git/refs/heads.
Experimentally, I added that entry by determining the first commit in
that chain and echoing that sha1 into .git/refs/heads/some_name and that
works as expected.
I suspect that the root cause was a 'git branch -D' I issued a while
back. My question is this: if deleting a branch in that manner caused me
to enter this situation, is that a bug or no? The commits in question
*are* referenced by various tags, but not by any branch that exists any
more. The echo command effectively re-created the branch and all seems
well.
--
Jon Nelson <jnelson-git@jamponi.net>
^ permalink raw reply
* Re: Branches and all commits
From: Andreas Ericsson @ 2005-12-19 17:52 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.63.0512191104080.6812@gheavc.wnzcbav.cig>
Jon Nelson wrote:
> On Mon, 19 Dec 2005, Andreas Ericsson wrote:
>
>
>>Jon Nelson wrote:
>>
>>>Should *all* commits be reachable via at least one branch? I ran into a
>>>situation this weekend that has me a little confused. I had performed a
>>>number of commits and such and I noticed that the author and committer info
>>>had suboptimal values. A bit of searching led me to a comment made by Linus
>>>that basically said "go hack git-convert-objects", which I did. After
>>>performing git-convert-objects on every commit object in .git/refs/heads and
>>>the requisite pruning, etc... just about everything looked fine. However, I
>>>still had a long series of commits that contained the wrong information.
>>>Further inspection makes it appear as though these commits are not reachable
>>>via any branch, although they are /all/ reachable via a series of tags. I
>>>worked around the problem by further modifying git-convert-objects to also
>>>understand tags (at least the basic ones I've got) and that's all taken care
>>>of, but the question remains: should *all* commit objects be reachable by at
>>>least one branch?
>>>
>>
>>AFAIU, yes.
>>
>>For future reference, what you should have done is this;
>>
>>$ git format-patch --mbox <first-unscrewed-commit-ish>
>># edit commit-messages in generated patches
>>$ git reset --hard <first-unscrewed-commit-ish>
>>$ for i in 00*.txt; do git apply < $i; done
>>$ git prune;# to get rid of the unreachable objects AFTER you've checked
>>everything's all right
>>
>>If things fail, do
>>
>>$ git reset --hard ORIG_HEAD
>>
>>and ask again.
>>
>>I'm afraid I can't help you fix up your repository from the state it's
>>in now. AFAIK, there's no tool to do it automagically.
>
>
> The repository seems just fine with this single exception - no branch
> contains a reference to the commit that forms the chain of commits that
> would otherwise be described as a branch. As I understand it, then, the
> only thing that is missing is an entry in .git/refs/heads.
>
> Experimentally, I added that entry by determining the first commit in
> that chain and echoing that sha1 into .git/refs/heads/some_name and that
> works as expected.
>
Lucky thing. I expect you still had all the objects required for the
tree to do this. If you had run 'git prune' on the repo they would have
been lost for good though.
> I suspect that the root cause was a 'git branch -D' I issued a while
> back. My question is this: if deleting a branch in that manner caused me
> to enter this situation, is that a bug or no?
It's not a bug. You probably meant to do
$ git branch -d
-D forces removal even if there are objects reachable only through that
branch. The man-page says so, but in git'ish, which isn't always
intuitive until you've grown familiar with the glossary.txt doc.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Fix race and deadlock when sending pack
From: Daniel Barkalow @ 2005-12-19 18:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Serice, git
In-Reply-To: <7vzmmxlkbq.fsf@assigned-by-dhcp.cox.net>
On Sun, 18 Dec 2005, Junio C Hamano wrote:
> Paul Serice <paul@serice.net> writes:
>
> > The best way to reproduce the problem is to locally clone your
> > repository. When you perform a push, git-send-pack will directly set
> > up pipes connected to stdin and stdout of git-receive-pack. You
> > should then set up hook/post-update or hook/update to try to write
> > lots of text to stdout. (You want to use the local protocol because
> > ssh is robust enough to mask the worst behavior.)
>
> My immediate reaction was "do not do it then", but you are
> right. Hooks are run after all the protocol exchanges are done,
> so they should be free to throw any garbage at the other end.
If we extend it to transfer multiple things, wouldn't we want to run hooks
after each of them, rather than all at the end?
As for the policy:
We definitely want to let hooks write to stdout, because git programs that
you might want to run in hooks write to stdout. I can't figure out what
"cvs" does with trigger script output and "at" and "cron" email the output
to the owners. I'd sort of like to avoid making people expect that there
is necessarily a path for text going back to the user directly. We may,
for example, want to support these hooks with pushes over HTTP(/WebDAV). I
also think that messages are likely to be at least as useful to the owner
of the target repository as the person pushing, which is why I'd prefer a
log file. E.g., if you've got a group central repository that different
people push to, it may be other group members who want to know what
happened with the output from a post-update hook, not the group member who
pushed.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Branches and all commits
From: Jon Nelson @ 2005-12-19 18:59 UTC (permalink / raw)
Cc: git
In-Reply-To: <43A6F378.6010503@op5.se>
On Mon, 19 Dec 2005, Andreas Ericsson wrote:
> Jon Nelson wrote:
> > On Mon, 19 Dec 2005, Andreas Ericsson wrote:
> >
> >
> > >Jon Nelson wrote:
> > >
> > > >Should *all* commits be reachable via at least one branch? I ran into a
> > > >situation this weekend that has me a little confused. I had performed a
> > > >number of commits and such and I noticed that the author and committer
> > > >info
> > > >had suboptimal values. A bit of searching led me to a comment made by
> > > >Linus
> > > >that basically said "go hack git-convert-objects", which I did. After
> > > >performing git-convert-objects on every commit object in .git/refs/heads
> > > >and
> > > >the requisite pruning, etc... just about everything looked fine. However,
> > > >I
> > > >still had a long series of commits that contained the wrong information.
> > > >Further inspection makes it appear as though these commits are not
> > > >reachable
> > > >via any branch, although they are /all/ reachable via a series of tags. I
> > > >worked around the problem by further modifying git-convert-objects to
> > > >also
> > > >understand tags (at least the basic ones I've got) and that's all taken
> > > >care
> > > >of, but the question remains: should *all* commit objects be reachable by
> > > >at
> > > >least one branch?
> > > >
> > >
> > >AFAIU, yes.
> > >
> > >For future reference, what you should have done is this;
> > >
> > >$ git format-patch --mbox <first-unscrewed-commit-ish>
> > ># edit commit-messages in generated patches
> > >$ git reset --hard <first-unscrewed-commit-ish>
> > >$ for i in 00*.txt; do git apply < $i; done
> > >$ git prune;# to get rid of the unreachable objects AFTER you've checked
> > >everything's all right
> > >
> > >If things fail, do
> > >
> > >$ git reset --hard ORIG_HEAD
> > >
> > >and ask again.
> > >
> > >I'm afraid I can't help you fix up your repository from the state it's in
> > >now. AFAIK, there's no tool to do it automagically.
> >
> >
> > The repository seems just fine with this single exception - no branch
> > contains a reference to the commit that forms the chain of commits that
> > would otherwise be described as a branch. As I understand it, then, the only
> > thing that is missing is an entry in .git/refs/heads.
> >
> > Experimentally, I added that entry by determining the first commit in that
> > chain and echoing that sha1 into .git/refs/heads/some_name and that works as
> > expected.
> >
>
> Lucky thing. I expect you still had all the objects required for the tree to
> do this. If you had run 'git prune' on the repo they would have been lost for
> good though.
That's the thing. I *had* run 'git prune' numerous times.
> > I suspect that the root cause was a 'git branch -D' I issued a while back.
> > My question is this: if deleting a branch in that manner caused me to enter
> > this situation, is that a bug or no?
>
> It's not a bug. You probably meant to do
>
> $ git branch -d
>
> -D forces removal even if there are objects reachable only through that
> branch. The man-page says so, but in git'ish, which isn't always intuitive
> until you've grown familiar with the glossary.txt doc.
I tried 'git branch -d' initially and it refused to delete the branch.
So I tried 'git branch -D'.
Re-reading your last paragraph makes it clear what happened, then.
I'll note that I ran 'git branch -D' *days* ago and I've run git-prune
literally a couple dozen times since then. Is it possible the objects
weren't removed because they were still referenced by tags?
--
Jon Nelson <jnelson-git@jamponi.net>
^ permalink raw reply
* Re: Branches and all commits
From: Andreas Ericsson @ 2005-12-19 19:32 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.63.0512191257240.6812@gheavc.wnzcbav.cig>
Jon Nelson wrote:
> On Mon, 19 Dec 2005, Andreas Ericsson wrote:
>
>>>I suspect that the root cause was a 'git branch -D' I issued a while back.
>>>My question is this: if deleting a branch in that manner caused me to enter
>>>this situation, is that a bug or no?
>>
>>It's not a bug. You probably meant to do
>>
>> $ git branch -d
>>
>>-D forces removal even if there are objects reachable only through that
>>branch. The man-page says so, but in git'ish, which isn't always intuitive
>>until you've grown familiar with the glossary.txt doc.
>
>
> I tried 'git branch -d' initially and it refused to delete the branch.
> So I tried 'git branch -D'.
>
> Re-reading your last paragraph makes it clear what happened, then.
> I'll note that I ran 'git branch -D' *days* ago and I've run git-prune
> literally a couple dozen times since then. Is it possible the objects
> weren't removed because they were still referenced by tags?
>
I suppose it must have been, which sort of contradicts how I thought
tags worked. Lucky thing though, eh? :)
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: EAGAIN?
From: Linus Torvalds @ 2005-12-19 19:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhd95h02o.fsf@assigned-by-dhcp.cox.net>
On Mon, 19 Dec 2005, Junio C Hamano wrote:
>
> I was looking at "git grep -n EAGAIN" output and found that many
> places check "errno == EAGAIN" without checking "errno ==
> EINTR", both for read(2) and write(2).
I suspect it's mostly in my code. It's a stupid quirk of mine.
You might as well delete those thing, but yes, if you want to replace them
with testing both EAGAIN and EINTR, go right ahead.
Linus
^ permalink raw reply
* Re: [PATCH] Fix race and deadlock when sending pack
From: Junio C Hamano @ 2005-12-19 21:01 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Paul Serice, git
In-Reply-To: <Pine.LNX.4.64.0512191236290.25300@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
>> My immediate reaction was "do not do it then", but you are
>> right. Hooks are run after all the protocol exchanges are done,
>> so they should be free to throw any garbage at the other end.
>
> If we extend it to transfer multiple things, wouldn't we want to run hooks
> after each of them, rather than all at the end?
We do transfer multiple things already, and all protocol
exchange happens before everything is transferred. And hooks
are run for each refs being updated, one by one. What we do not
have is a reporting mechanism that says "we refused to update
this ref because of the hooks/update policy return value for
it". Even if we later add that reporting mechanism, as I
outlined in a separate message earlier, I think it is OK to keep
running the update hooks after the pack transfer part.
> As for the policy:
>
> We definitely want to let hooks write to stdout, because git programs that
> you might want to run in hooks write to stdout.
> ... I'd sort of like to avoid making people expect that there
> is necessarily a path for text going back to the user directly.
> ... I
> also think that messages are likely to be at least as useful to the owner
> of the target repository as the person pushing, which is why I'd prefer a
> log file.
This part I mostly agree with. Will have to think about the
details but probably I'd punt this for now and declare it post
1.0 ;-).
^ permalink raw reply
* Re: [PATCH] Fix race and deadlock when sending pack
From: Daniel Barkalow @ 2005-12-19 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Serice, git
In-Reply-To: <7vy82gg5t7.fsf@assigned-by-dhcp.cox.net>
On Mon, 19 Dec 2005, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> >> My immediate reaction was "do not do it then", but you are
> >> right. Hooks are run after all the protocol exchanges are done,
> >> so they should be free to throw any garbage at the other end.
> >
> > If we extend it to transfer multiple things, wouldn't we want to run hooks
> > after each of them, rather than all at the end?
>
> We do transfer multiple things already, and all protocol
> exchange happens before everything is transferred. And hooks
> are run for each refs being updated, one by one. What we do not
> have is a reporting mechanism that says "we refused to update
> this ref because of the hooks/update policy return value for
> it". Even if we later add that reporting mechanism, as I
> outlined in a separate message earlier, I think it is OK to keep
> running the update hooks after the pack transfer part.
If we have the reporting mechanism, that will effectively be part of the
protocol. It's obviously done transferring the pack at that point, but it
still wants fixed-format communication, so switching over to being the
stardard output of the hooks would cause problems with this.
> > As for the policy:
> >
> > We definitely want to let hooks write to stdout, because git programs that
> > you might want to run in hooks write to stdout.
> > ... I'd sort of like to avoid making people expect that there
> > is necessarily a path for text going back to the user directly.
> > ... I
> > also think that messages are likely to be at least as useful to the owner
> > of the target repository as the person pushing, which is why I'd prefer a
> > log file.
>
> This part I mostly agree with. Will have to think about the
> details but probably I'd punt this for now and declare it post
> 1.0 ;-).
It's probably worth making sure that all the hooks run with something
sane, and punt making it configurable andnice until post-1.0. I was only
really looking at post-update, so I don't know how the others run.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: EAGAIN?
From: H. Peter Anvin @ 2005-12-19 22:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0512191142410.4827@g5.osdl.org>
Linus Torvalds wrote:
>
> On Mon, 19 Dec 2005, Junio C Hamano wrote:
>
>>I was looking at "git grep -n EAGAIN" output and found that many
>>places check "errno == EAGAIN" without checking "errno ==
>>EINTR", both for read(2) and write(2).
>
>
> I suspect it's mostly in my code. It's a stupid quirk of mine.
>
> You might as well delete those thing, but yes, if you want to replace them
> with testing both EAGAIN and EINTR, go right ahead.
>
It might be that some of those should either be replaced by fwrite/fread
or there should be a generic wrapper (usually called xwrite/xread).
-hpa
^ permalink raw reply
* Re: using-topic-branches.txt
From: H. Peter Anvin @ 2005-12-19 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff Garzik, git
In-Reply-To: <7v64plmz1e.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Jeff Garzik <jgarzik@pobox.com> writes:
>
>>master.kernel.org is non-public, so it shouldn't be mentioned in
>>documentation...
>
> I'm willing to be persuaded otherwise
We want to publicize this machine as little as possible. Please don't
make it worse than necessary.
-hpa
^ permalink raw reply
* Re: using-topic-branches.txt
From: H. Peter Anvin @ 2005-12-19 22:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Brown, Len, git
In-Reply-To: <Pine.LNX.4.64.0512182225310.4827@g5.osdl.org>
Linus Torvalds wrote:
>
> Well, you can use "git://git.kernel.org/" for reading.
>
> Personally, I use master.kernel.org both for reading and writing, but I
> end up using an ssh key with an empty passphrase.
>
If you use ssh-agent then you don't have to use an empty passphrase.
ssh-agent should be automatically set up in all the current distros.
-hpa
^ permalink raw reply
* Re: [PATCH] Fix race and deadlock when sending pack
From: Junio C Hamano @ 2005-12-19 22:44 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512191645230.25300@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> If we have the reporting mechanism, that will effectively be part of the
> protocol. It's obviously done transferring the pack at that point, but it
> still wants fixed-format communication, so switching over to being the
> stardard output of the hooks would cause problems with this.
In order to add reporting mechanism later, I think we need to be
able to identify the protocol version in a backward compatible
way, something like the "server capabilities hidden behind the
NUL" trick we did for fetch-pack/upload-pack protocol. Once
that is in place, it does not cause harm even if the current
protocol program connects hooks' stdout to send-pack, at least
in theory. If we take Paul's patch now, however, it would add
more work for us later when we do that protocol change, because
we will need to wrap the output from the hook in the pkt-line
interface in the new protocol, in order to give that back to the
stdout of send-pack. Considering that, I think we may want to
drop Paul's patch and declare that hooks stdout does not come
back to send-pack.
Honestly speaking, I do not really care where stdout of hooks go
as long as that does not cause breakage/deadlocks, and I think
your earlier patch on December 7th is serving us well enough; we
needed to have told users to do an "exec 1>somewhere" in their
hooks before that fix, which was not nice at all (and we even
forgot to tell them that). If people want to send the output to
a log file, they can do so; if they want e-mails, they can do
so; if they want to show the output to the pusher, they can do
1>&2; all inside their hooks. I do "echo nitfol | at now" and
love the way that I do not have to worry about how "at" command
gives me back execution report via e-mail at all ;-).
> It's probably worth making sure that all the hooks run with something
> sane, and punt making it configurable andnice until post-1.0.
I think we agree that /dev/null is one of the sane choices as
you did in your earlier fix. Duping stderr would have been
another sane choice, but I honestly do not think we care much
either way.
^ permalink raw reply
* [PATCH] xread/xwrite: do not worry about EINTR at calling sites.
From: Junio C Hamano @ 2005-12-20 0:55 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Linus Torvalds, git
In-Reply-To: <43A732C9.2060509@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> Linus Torvalds wrote:
>> On Mon, 19 Dec 2005, Junio C Hamano wrote:
>>
>>>I was looking at "git grep -n EAGAIN" output and found that many
>>>places check "errno == EAGAIN" without checking "errno ==
>>>EINTR", both for read(2) and write(2).
>> I suspect it's mostly in my code. It's a stupid quirk of mine.
>> You might as well delete those thing, but yes, if you want to
>> replace them with testing both EAGAIN and EINTR, go right ahead.
>>
>
> It might be that some of those should either be replaced by fwrite/fread
> or there should be a generic wrapper (usually called xwrite/xread).
Good idea. Something like this I suppose....
-- >8 --
We had errno==EINTR check after read(2)/write(2) sprinkled all
over the places, always doing continue. Consolidate them into
xread()/xwrite() wrapper routines.
Credits for suggestion goes to HPA -- bugs are mine.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
apply.c | 23 ++++++-----------------
cat-file.c | 4 +---
copy.c | 19 +++++++------------
csum-file.c | 4 +---
git-compat-util.h | 22 ++++++++++++++++++++++
mktag.c | 9 ++-------
pkt-line.c | 11 +++--------
tar-tree.c | 4 +---
unpack-objects.c | 13 +++----------
9 files changed, 46 insertions(+), 63 deletions(-)
7c59f627268cd446fc7e1df9847dd1e2178072da
diff --git a/apply.c b/apply.c
index 1742ab2..d5e7bfd 100644
--- a/apply.c
+++ b/apply.c
@@ -84,14 +84,11 @@ static void *read_patch_file(int fd, uns
buffer = xrealloc(buffer, alloc);
nr = alloc - size;
}
- nr = read(fd, buffer + size, nr);
+ nr = xread(fd, buffer + size, nr);
if (!nr)
break;
- if (nr < 0) {
- if (errno == EAGAIN)
- continue;
+ if (nr < 0)
die("git-apply: read returned %s", strerror(errno));
- }
size += nr;
}
*sizep = size;
@@ -1006,13 +1003,8 @@ static int read_old_data(struct stat *st
return error("unable to open %s", path);
got = 0;
for (;;) {
- int ret = read(fd, buf + got, size - got);
- if (ret < 0) {
- if (errno == EAGAIN)
- continue;
- break;
- }
- if (!ret)
+ int ret = xread(fd, buf + got, size - got);
+ if (ret <= 0)
break;
got += ret;
}
@@ -1600,12 +1592,9 @@ static int try_create_file(const char *p
if (fd < 0)
return -1;
while (size) {
- int written = write(fd, buf, size);
- if (written < 0) {
- if (errno == EINTR || errno == EAGAIN)
- continue;
+ int written = xwrite(fd, buf, size);
+ if (written < 0)
die("writing file %s: %s", path, strerror(errno));
- }
if (!written)
die("out of space writing file %s", path);
buf += written;
diff --git a/cat-file.c b/cat-file.c
index 7594108..96d66b4 100644
--- a/cat-file.c
+++ b/cat-file.c
@@ -55,10 +55,8 @@ int main(int argc, char **argv)
die("git-cat-file %s: bad file", argv[2]);
while (size > 0) {
- long ret = write(1, buf, size);
+ long ret = xwrite(1, buf, size);
if (ret < 0) {
- if (errno == EAGAIN)
- continue;
/* Ignore epipe */
if (errno == EPIPE)
break;
diff --git a/copy.c b/copy.c
index e1cd5d0..7100eed 100644
--- a/copy.c
+++ b/copy.c
@@ -6,32 +6,27 @@ int copy_fd(int ifd, int ofd)
int len;
char buffer[8192];
char *buf = buffer;
- len = read(ifd, buffer, sizeof(buffer));
+ len = xread(ifd, buffer, sizeof(buffer));
if (!len)
break;
if (len < 0) {
int read_error;
- if (errno == EAGAIN)
- continue;
read_error = errno;
close(ifd);
return error("copy-fd: read returned %s",
strerror(read_error));
}
- while (1) {
- int written = write(ofd, buf, len);
+ while (len) {
+ int written = xwrite(ofd, buf, len);
if (written > 0) {
buf += written;
len -= written;
- if (!len)
- break;
}
- if (!written)
+ else if (!written)
return error("copy-fd: write returned 0");
- if (errno == EAGAIN || errno == EINTR)
- continue;
- return error("copy-fd: write returned %s",
- strerror(errno));
+ else
+ return error("copy-fd: write returned %s",
+ strerror(errno));
}
}
close(ifd);
diff --git a/csum-file.c b/csum-file.c
index c66b9eb..5f9249a 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -15,7 +15,7 @@ static int sha1flush(struct sha1file *f,
void *buf = f->buffer;
for (;;) {
- int ret = write(f->fd, buf, count);
+ int ret = xwrite(f->fd, buf, count);
if (ret > 0) {
buf += ret;
count -= ret;
@@ -25,8 +25,6 @@ static int sha1flush(struct sha1file *f,
}
if (!ret)
die("sha1 file '%s' write error. Out of diskspace", f->name);
- if (errno == EAGAIN || errno == EINTR)
- continue;
die("sha1 file '%s' write error (%s)", f->name, strerror(errno));
}
}
diff --git a/git-compat-util.h b/git-compat-util.h
index ead0ede..0c98c99 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -84,6 +84,28 @@ static inline void *xcalloc(size_t nmemb
return ret;
}
+static inline ssize_t xread(int fd, void *buf, size_t len)
+{
+ ssize_t nr;
+ while (1) {
+ nr = read(fd, buf, len);
+ if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
+static inline ssize_t xwrite(int fd, const void *buf, size_t len)
+{
+ ssize_t nr;
+ while (1) {
+ nr = write(fd, buf, len);
+ if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
/* Sane ctype - no locale, and works with signed chars */
#undef isspace
#undef isdigit
diff --git a/mktag.c b/mktag.c
index 97e270a..fc6a9bf 100644
--- a/mktag.c
+++ b/mktag.c
@@ -116,14 +116,9 @@ int main(int argc, char **argv)
// Read the signature
size = 0;
for (;;) {
- int ret = read(0, buffer + size, MAXSIZE - size);
- if (!ret)
+ int ret = xread(0, buffer + size, MAXSIZE - size);
+ if (ret <= 0)
break;
- if (ret < 0) {
- if (errno == EAGAIN)
- continue;
- break;
- }
size += ret;
}
diff --git a/pkt-line.c b/pkt-line.c
index 6947304..bb3bab0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -19,7 +19,7 @@
static void safe_write(int fd, const void *buf, unsigned n)
{
while (n) {
- int ret = write(fd, buf, n);
+ int ret = xwrite(fd, buf, n);
if (ret > 0) {
buf += ret;
n -= ret;
@@ -27,8 +27,6 @@ static void safe_write(int fd, const voi
}
if (!ret)
die("write error (disk full?)");
- if (errno == EAGAIN || errno == EINTR)
- continue;
die("write error (%s)", strerror(errno));
}
}
@@ -68,12 +66,9 @@ static void safe_read(int fd, void *buff
int n = 0;
while (n < size) {
- int ret = read(fd, buffer + n, size - n);
- if (ret < 0) {
- if (errno == EINTR || errno == EAGAIN)
- continue;
+ int ret = xread(fd, buffer + n, size - n);
+ if (ret < 0)
die("read error (%s)", strerror(errno));
- }
if (!ret)
die("unexpected EOF");
n += ret;
diff --git a/tar-tree.c b/tar-tree.c
index bacb23a..96bd143 100644
--- a/tar-tree.c
+++ b/tar-tree.c
@@ -34,10 +34,8 @@ struct path_prefix {
static void reliable_write(void *buf, unsigned long size)
{
while (size > 0) {
- long ret = write(1, buf, size);
+ long ret = xwrite(1, buf, size);
if (ret < 0) {
- if (errno == EAGAIN)
- continue;
if (errno == EPIPE)
exit(0);
die("git-tar-tree: %s", strerror(errno));
diff --git a/unpack-objects.c b/unpack-objects.c
index cfd61ae..5c5cb12 100644
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -31,12 +31,10 @@ static void * fill(int min)
offset = 0;
}
do {
- int ret = read(0, buffer + len, sizeof(buffer) - len);
+ int ret = xread(0, buffer + len, sizeof(buffer) - len);
if (ret <= 0) {
if (!ret)
die("early EOF");
- if (errno == EAGAIN || errno == EINTR)
- continue;
die("read error on input: %s", strerror(errno));
}
len += ret;
@@ -299,14 +297,9 @@ int main(int argc, char **argv)
/* Write the last part of the buffer to stdout */
while (len) {
- int ret = write(1, buffer + offset, len);
- if (!ret)
- break;
- if (ret < 0) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
+ int ret = xwrite(1, buffer + offset, len);
+ if (ret <= 0)
break;
- }
len -= ret;
offset += ret;
}
--
0.99.9.GIT
^ permalink raw reply related
* Re: [PATCH] Fix unconditional early exit in cg-fetch
From: Junio C Hamano @ 2005-12-20 1:15 UTC (permalink / raw)
To: Paolo 'Blaisorblade' Giarrusso; +Cc: Petr Baudis, git
In-Reply-To: <20051219161736.18245.98591.stgit@zion.home.lan>
Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> writes:
> if [ "$get" = "get_rsync" ]; then
> $get -i -s -u -d "$uri/refs/tags" "$_git/refs/tags" ||
> - echo "unable to get tags list (non-fatal)" >&2
> - exit $?
> + (echo "unable to get tags list (non-fatal)" >&2;
> + exit $?)
> fi
Why would you want a subshell that exits with a non-zero status
when nobody is checking that status anyway?
I suspect removing "exit $?" would suffice, if that condition is
non-fatal as the message says...
^ permalink raw reply
* Re: qgit reports errors in the git repository
From: Pavel Roskin @ 2005-12-20 2:11 UTC (permalink / raw)
To: Marco Costalba; +Cc: Junio C Hamano, git
In-Reply-To: <e5bfff550512170044v59f96262ica6511e981889ea9@mail.gmail.com>
Hello, Marco!
On Sat, 2005-12-17 at 09:44 +0100, Marco Costalba wrote:
> On 12/17/05, Junio C Hamano <junkio@cox.net> wrote:
> > Marco Costalba <mcostalba@gmail.com> writes:
> Pavel, please, test the following patch. If it works I will push the change.
I could not apply your patch (it got mangled), but the git server for
qgit is working now, so I updated from it, and it looks like your patch
is already there.
I don't see any error messages from any git repository I'm using. Thank
you for the fix!
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: [ANNOUNCE qgit-1.0rc1]
From: Pavel Roskin @ 2005-12-20 3:04 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550512180706y42bebd3frc142ade7f4d318ae@mail.gmail.com>
Hello, Marco!
On Sun, 2005-12-18 at 16:06 +0100, Marco Costalba wrote:
> A good amount of small fixes and just few non intrusive features
> added, biggest one are speed-up of ref reading at startup, a per
> repository charset encoding using i18n.commitencoding git config
> variable and an annotation progress bar.
>
> I plan to relase 1.0 for end of next week, so I would like to catch
> last minutes bugs with this rc1.
OK, here you go. qgit just crashed on me. I opened the patch pane, the
tree pane and the annotate window. While the annotate window was
focused, I tried to select multiple files in the tree pane. Then I
reproduced the crash in the debugger with qgit compiled without
optimization.
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 46912496291552 (LWP 19362)]
0x0000000000415943 in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7e2dc0) at src/annotate.cpp:102
102 if (!pa->isValid) {
(gdb) p pa
$1 = (FileAnnotation *) 0x0
(gdb) bt
#0 0x0000000000415943 in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7e2dc0) at src/annotate.cpp:102
#1 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7e4d10) at src/annotate.cpp:103
#2 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7e27d0) at src/annotate.cpp:103
#3 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x8db560) at src/annotate.cpp:103
#4 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x872bc0) at src/annotate.cpp:103
#5 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7b5c80) at src/annotate.cpp:103
#6 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7c2a50) at src/annotate.cpp:103
#7 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x790c20) at src/annotate.cpp:103
#8 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7b9a30) at src/annotate.cpp:103
#9 0x000000000041596d in Annotate::doAnnotate (this=0x7e6bd0, fileName=@0x7fffffc81f70,
sha=@0x7e3d58) at src/annotate.cpp:103
#10 0x0000000000415f5f in Annotate::run (this=0x7e6bd0, fileName=@0x7fffffc81f70,
histRevOrder=@0x7fffffc81f80, annotateOk=@0x7fffffc81f5f, canceled=@0x7fffffc81f5e)
at src/annotate.cpp:73
#11 0x0000000000435a54 in Git::runAnnotate (this=0x6cb5b0) at src/git.cpp:166
#12 0x00000000004832ab in Git::qt_invoke (this=0x6cb5b0, _id=9, _o=0x7fffffc82070)
at src/moc_git.cc:132
#13 0x000000308a452d3a in QObject::activate_signal () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#14 0x000000308a760923 in QSignal::signal () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#15 0x000000308a46c581 in QSignal::activate () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
---Type <return> to continue, or q <return> to quit---
#16 0x000000308a473d09 in QSingleShotTimer::event () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#17 0x000000308a3f7cfa in QApplication::internalNotify ()
from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#18 0x000000308a3f8674 in QApplication::notify () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#19 0x000000308a3edc4c in QEventLoop::activateTimers ()
from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#20 0x000000308a3a9d3b in QEventLoop::processEvents () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#21 0x000000308a40de37 in QEventLoop::enterLoop () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#22 0x000000308a40dd42 in QEventLoop::exec () from /usr/lib64/qt-3.3/lib/libqt-mt.so.3
#23 0x000000000046e976 in main (argc=1, argv=0x7fffffc82688) at src/qgit.cpp:29
(gdb)
That's Fedora Core 4 on x86_64.
Also, qgit doesn't look well with large fonts (not excessively large,
mind you, just with Sans 12):
http://red-bean.com/proski/qgit/
I see sizes are hardcoded in the *.ui files. Probably there should be a
way to make the widgets resize together with the font.
I don't want to be asked about range every time I start qgit. Can it be
an option? In half of the cases I just want to see commits in the last
2-4 days. In other cases, I want to see the whole history. Tags are
usually irrelevant.
The "Show All" checkbox in the range select dialog doesn't seem to have
any effect. I think it should disable "top" and "bottom" comboboxes.
Finally, if I close the range select window by using the window manager
button, I don't want qgit to continue. Maybe it's just because I'm
annoyed by the useless range dialog :-)
--
Regards,
Pavel Roskin
^ permalink raw reply
* [PATCH] cg-seek should not complain if run twice
From: Pavel Roskin @ 2005-12-20 4:55 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
cg-seek complains if run without arguments in a non-seeked repository:
rm: cannot remove `.git/refs/heads/cg-seek-point': No such file or directory
rm: cannot remove `.git/head-name': No such file or directory
In fact, it's OK for those files not to exist, so they should be removed
silently.
Signed-off-by: Pavel Roskin <proski@gnu.org>
diff --git a/cg-seek b/cg-seek
index 942267f..8356593 100755
--- a/cg-seek
+++ b/cg-seek
@@ -60,7 +60,7 @@ if [ "$seek_mode" = "away" ]; then
git-symbolic-ref HEAD "refs/heads/cg-seek-point"
else
git-symbolic-ref HEAD "refs/heads/$_git_head"
- rm "$_git/refs/heads/cg-seek-point" "$_git/head-name"
+ rm -f "$_git/refs/heads/cg-seek-point" "$_git/head-name"
rm -f "$_git/blocked"
fi
--
Regards,
Pavel Roskin
^ permalink raw reply related
* [PATCH] Racy GIT
From: Junio C Hamano @ 2005-12-20 8:38 UTC (permalink / raw)
To: git; +Cc: pasky, torvalds
This fixes the longstanding "Racy GIT" problem, which was pretty
much there from the beginning of time, but was first
demonstrated by Pasky in this message on October 24, 2005:
http://marc.theaimsgroup.com/?l=git&m=113014629716878
If you run the following sequence of commands:
echo frotz >infocom
git update-index --add infocom
echo xyzzy >infocom
so that the second update to file "infocom" does not change
st_mtime, what is recorded as the stat information for the cache
entry "infocom" exactly matches what is on the filesystem
(owner, group, inum, mtime, ctime, mode, length). After this
sequence, we incorrectly think "infocom" file still has string
"frotz" in it, and get really confused. E.g. git-diff-files
would say there is no change, git-update-index --refresh would
not even look at the filesystem to correct the situation.
Some ways of working around this issue were already suggested by
Linus in the same thread on the same day, including waiting
until the next second before returning from update-index if a
cache entry written out has the current timestamp, but that
means we can make at most one commit per second, and given that
the e-mail patch workflow used by Linus needs to process at
least 5 commits per second, it is not an acceptable solution.
Linus notes that git-apply is primarily used to update the index
while processing e-mailed patches, which is true, and
git-apply's up-to-date check is fooled by the same problem but
luckily in the other direction, so it is not really a big issue,
but still it is disturbing.
The function ce_match_stat() is called to bypass the comparison
against filesystem data when the stat data recorded in the cache
entry matches what stat() returns from the filesystem. This
patch tackles the problem by changing it to actually go to the
filesystem data for cache entries that have the same mtime as
the index file itself. This works as long as the index file and
working tree files are on the filesystems that share the same
monotonic clock. Files on network mounted filesystems sometimes
get skewed timestamps compared to "date" output, but as long as
working tree files' timestamps are skewed the same way as the
index file's, this approach still works. The only problematic
files are the ones that have the same timestamp as the index
file's, because two file updates that sandwitch the index file
update must happen within the same second to trigger the
problem.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
read-cache.c | 140 +++++++++++++++++++++++++++++++--------------------
t/t0010-racy-git.sh | 24 +++++++++
2 files changed, 110 insertions(+), 54 deletions(-)
create mode 100755 t/t0010-racy-git.sh
29e4d3635709778bcc808dbad0477efad82f8d7e
diff --git a/read-cache.c b/read-cache.c
index 6932736..780601f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -6,6 +6,7 @@
#include "cache.h"
struct cache_entry **active_cache = NULL;
+static time_t index_file_timestamp;
unsigned int active_nr = 0, active_alloc = 0, active_cache_changed = 0;
/*
@@ -28,6 +29,64 @@ void fill_stat_cache_info(struct cache_e
ce->ce_size = htonl(st->st_size);
}
+static int ce_compare_data(struct cache_entry *ce, struct stat *st)
+{
+ int match = -1;
+ int fd = open(ce->name, O_RDONLY);
+
+ if (fd >= 0) {
+ unsigned char sha1[20];
+ if (!index_fd(sha1, fd, st, 0, NULL))
+ match = memcmp(sha1, ce->sha1, 20);
+ close(fd);
+ }
+ return match;
+}
+
+static int ce_compare_link(struct cache_entry *ce, unsigned long expected_size)
+{
+ int match = -1;
+ char *target;
+ void *buffer;
+ unsigned long size;
+ char type[10];
+ int len;
+
+ target = xmalloc(expected_size);
+ len = readlink(ce->name, target, expected_size);
+ if (len != expected_size) {
+ free(target);
+ return -1;
+ }
+ buffer = read_sha1_file(ce->sha1, type, &size);
+ if (!buffer) {
+ free(target);
+ return -1;
+ }
+ if (size == expected_size)
+ match = memcmp(buffer, target, size);
+ free(buffer);
+ free(target);
+ return match;
+}
+
+static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
+{
+ switch (st->st_mode & S_IFMT) {
+ case S_IFREG:
+ if (ce_compare_data(ce, st))
+ return DATA_CHANGED;
+ break;
+ case S_IFLNK:
+ if (ce_compare_link(ce, st->st_size))
+ return DATA_CHANGED;
+ break;
+ default:
+ return TYPE_CHANGED;
+ }
+ return 0;
+}
+
int ce_match_stat(struct cache_entry *ce, struct stat *st)
{
unsigned int changed = 0;
@@ -83,57 +142,37 @@ int ce_match_stat(struct cache_entry *ce
if (ce->ce_size != htonl(st->st_size))
changed |= DATA_CHANGED;
- return changed;
-}
-
-static int ce_compare_data(struct cache_entry *ce, struct stat *st)
-{
- int match = -1;
- int fd = open(ce->name, O_RDONLY);
- if (fd >= 0) {
- unsigned char sha1[20];
- if (!index_fd(sha1, fd, st, 0, NULL))
- match = memcmp(sha1, ce->sha1, 20);
- close(fd);
- }
- return match;
-}
-
-static int ce_compare_link(struct cache_entry *ce, unsigned long expected_size)
-{
- int match = -1;
- char *target;
- void *buffer;
- unsigned long size;
- char type[10];
- int len;
+ /*
+ * Within 1 second of this sequence:
+ * echo xyzzy >file && git-update-index --add file
+ * running this command:
+ * echo frotz >file
+ * would give a falsely clean cache entry. The mtime and
+ * length match the cache, and other stat fields do not change.
+ *
+ * We could detect this at update-index time (the cache entry
+ * being registered/updated records the same time as "now")
+ * and delay the return from git-update-index, but that would
+ * effectively mean we can make at most one commit per second,
+ * which is not acceptable. Instead, we check cache entries
+ * whose mtime are the same as the index file timestamp more
+ * careful than others.
+ */
+ if (!changed &&
+ index_file_timestamp &&
+ index_file_timestamp <= ntohl(ce->ce_mtime.sec))
+ changed |= ce_modified_check_fs(ce, st);
- target = xmalloc(expected_size);
- len = readlink(ce->name, target, expected_size);
- if (len != expected_size) {
- free(target);
- return -1;
- }
- buffer = read_sha1_file(ce->sha1, type, &size);
- if (!buffer) {
- free(target);
- return -1;
- }
- if (size == expected_size)
- match = memcmp(buffer, target, size);
- free(buffer);
- free(target);
- return match;
+ return changed;
}
int ce_modified(struct cache_entry *ce, struct stat *st)
{
- int changed;
+ int changed, changed_fs;
changed = ce_match_stat(ce, st);
if (!changed)
return 0;
-
/*
* If the mode or type has changed, there's no point in trying
* to refresh the entry - it's not going to match
@@ -148,18 +187,9 @@ int ce_modified(struct cache_entry *ce,
if ((changed & DATA_CHANGED) && ce->ce_size != htonl(0))
return changed;
- switch (st->st_mode & S_IFMT) {
- case S_IFREG:
- if (ce_compare_data(ce, st))
- return changed | DATA_CHANGED;
- break;
- case S_IFLNK:
- if (ce_compare_link(ce, st->st_size))
- return changed | DATA_CHANGED;
- break;
- default:
- return changed | TYPE_CHANGED;
- }
+ changed_fs = ce_modified_check_fs(ce, st);
+ if (changed_fs)
+ return changed | changed_fs;
return 0;
}
@@ -471,6 +501,7 @@ int read_cache(void)
return active_nr;
errno = ENOENT;
+ index_file_timestamp = 0;
fd = open(get_index_file(), O_RDONLY);
if (fd < 0) {
if (errno == ENOENT)
@@ -504,6 +535,7 @@ int read_cache(void)
offset = offset + ce_size(ce);
active_cache[i] = ce;
}
+ index_file_timestamp = st.st_mtime;
return active_nr;
unmap:
diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
new file mode 100755
index 0000000..eb175b7
--- /dev/null
+++ b/t/t0010-racy-git.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='racy GIT'
+
+. ./test-lib.sh
+
+# This test can give false success if your machine is sufficiently
+# slow or your trial happened to happen on second boundary.
+
+for trial in 0 1 2 3 4 5 6 7 8 9
+do
+ rm -f .git/index
+ echo frotz >infocom
+ echo xyzzy >activision
+ git update-index --add infocom activision
+ echo xyzzy >infocom
+
+ files=`git diff-files -p`
+ test_expect_success \
+ "Racy GIT trial #$trial" \
+ 'test "" != "$files"'
+done
+
+test_done
--
0.99.9.GIT
^ permalink raw reply related
* Re: [PATCH] xread/xwrite: do not worry about EINTR at calling sites.
From: Johannes Schindelin @ 2005-12-20 10:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: H. Peter Anvin, Linus Torvalds, git
In-Reply-To: <7vvexkpoxf.fsf_-_@assigned-by-dhcp.cox.net>
Hi,
On Mon, 19 Dec 2005, Junio C Hamano wrote:
> +static inline ssize_t xwrite(int fd, const void *buf, size_t len)
> +{
> + ssize_t nr;
> + while (1) {
> + nr = write(fd, buf, len);
> + if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
> + continue;
> + return nr;
> + }
> +}
In another project I'm working on, a user insisted that on Solaris 2.7,
write(2) sometimes returns ENOENT when it really means "try again". I
cannot verify, since I don't have Solaris 2.7.
Ciao,
Dscho
^ permalink raw reply
* Re: [ANNOUNCE qgit-1.0rc1]
From: Marco Costalba @ 2005-12-20 12:55 UTC (permalink / raw)
To: Pavel Roskin; +Cc: git
In-Reply-To: <1135047865.2259.46.camel@dv>
> OK, here you go. qgit just crashed on me. I opened the patch pane, the
> tree pane and the annotate window. While the annotate window was
> focused, I tried to select multiple files in the tree pane.
Hi Pavel,
thanks for crash report, I will investigate it.
>
> Also, qgit doesn't look well with large fonts (not excessively large,
> mind you, just with Sans 12):
>
> http://red-bean.com/proski/qgit/
>
> I see sizes are hardcoded in the *.ui files. Probably there should be a
> way to make the widgets resize together with the font.
>
I will investigate what Qt designer allows abot font resizing.
> I don't want to be asked about range every time I start qgit. Can it be
> an option? In half of the cases I just want to see commits in the last
> 2-4 days. In other cases, I want to see the whole history. Tags are
> usually irrelevant.
>
I know what you mean ;-)
To avoid range select dialog simply call qgit with some command line option (arguments are feeded
to git-rev-parse, so the same options of gitk are allowed), qgit shows the dialog only if called
with no arguments.
If you don't want to type options just call qgit and press enter as soon as range select appears,
by default you will see commits from HEAD to latest tag.
If, instead, you want to see whole the history, press "Canc" to delete "to" field tag and then
press enter. That's the reason the focus is on "to" field by default ;-)
An option at startup to bybass range select could be an idea tough.
> The "Show All" checkbox in the range select dialog doesn't seem to have
> any effect. I think it should disable "top" and "bottom" comboboxes.
>
"Show All" is simply a shortcut for git-rev-parse --show-all option, to see all the branches
between selected commit range.
Tags selection is to define selected commit range
Peraphs it should be better call it "Show all branches" and add a new check "Show whole history"
Thanks for your feedback.
Marco
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox