* Re: [PATCH] Add more tests for git-clean
From: Johannes Schindelin @ 2007-11-04 23:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Bohrer, git
In-Reply-To: <7vve8hr3ch.fsf@gitster.siamese.dyndns.org>
Hi,
On Sun, 4 Nov 2007, Junio C Hamano wrote:
> Shawn Bohrer <shawn.bohrer@gmail.com> writes:
>
> > +test_expect_success 'git-clean with prefix' '
> > +
> > + mkdir -p build docs &&
> > + touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
> > + cd src/ &&
> > + git-clean &&
> > + cd - &&
>
> This is wrong for two reasons.
>
> - Is "cd -" portable?
>
> - What happens when git-clean fails? This test fails, and then
> it goes on to the next test without cd'ing back.
So it should be
(cd src/ && git clean) &&
right? (Note that I also removed the dash, since it will be a builtin
after the next commit.)
Ciao,
Dscho
^ permalink raw reply
* Re: Warning: cvsexportcommit considered dangerous
From: Johannes Schindelin @ 2007-11-04 23:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Steffen Prohaska, Git Mailing List, Alex Bennee
In-Reply-To: <200711050005.28561.robin.rosenberg.lists@dewire.com>
Hi,
On Mon, 5 Nov 2007, Robin Rosenberg wrote:
> BTW, wouldn't this err on the right side anyway, i.e. if an existing
> file was not up to date and was wrongly thought to not exist or a new
> file was thought to be up-to-date, I would get an error and would not be
> able to commit. I've never seen it though and I always have a clean CVS
> checkout so the potential bug seems unlikely to me.
The problem is that it can err. For example when I have new files, it
says that the files were already added by someone else. And then it
refuses to do anything.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add more tests for git-clean
From: Pierre Habouzit @ 2007-11-04 23:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Bohrer, git
In-Reply-To: <7vve8hr3ch.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 864 bytes --]
On Sun, Nov 04, 2007 at 11:35:42PM +0000, Junio C Hamano wrote:
> Shawn Bohrer <shawn.bohrer@gmail.com> writes:
>
> > +test_expect_success 'git-clean with prefix' '
> > +
> > + mkdir -p build docs &&
> > + touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
> > + cd src/ &&
> > + git-clean &&
> > + cd - &&
>
> This is wrong for two reasons.
>
> - Is "cd -" portable?
this is POSIX:
8910 − When a hyphen is used as the operand, this shall be equivalent to the command:
8911 cd "$OLDPWD" && pwd
8912 which changes to the previous working directory and then writes its name.
Meaning that cd $OLDPWD should work, and won't print $OLDPWD.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Add more tests for git-clean
From: Junio C Hamano @ 2007-11-04 23:35 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <11942029442710-git-send-email-shawn.bohrer@gmail.com>
Shawn Bohrer <shawn.bohrer@gmail.com> writes:
> +test_expect_success 'git-clean with prefix' '
> +
> + mkdir -p build docs &&
> + touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
> + cd src/ &&
> + git-clean &&
> + cd - &&
This is wrong for two reasons.
- Is "cd -" portable?
- What happens when git-clean fails? This test fails, and then
it goes on to the next test without cd'ing back.
^ permalink raw reply
* Re: gitk graph routing problem
From: Junio C Hamano @ 2007-11-04 23:14 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Alex Riesen, git
In-Reply-To: <18222.15870.945694.238217@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> Alex Riesen writes:
>
>> To reproduce, try running in git repo:
>>
>> gitk 02f630448e5d48e..06ea6ba9cf46ef5
>
> I can't reproduce it here, as my clone of the git repo doesn't have
> 02f630448e5d48e in it, even after updating...
Heh, me neither ;-).
^ permalink raw reply
* Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.
From: Pierre Habouzit @ 2007-11-04 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8x5dsjne.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
On Sun, Nov 04, 2007 at 10:58:13PM +0000, Junio C Hamano wrote:
> Two comments.
>
> * I have updated 1/10 with typo and indentation fixes.
>
> * I see you changed 2/10 to use OPTIONS_KEEPDASHDASH instead of
> PARSEOPT_OPTS, but the scripts that do not want the --keep
> behaviour do not set OPTIONS_KEEPDASHDASH to empty, so I do
> not see how this updatet would make _any_ difference. The
> user can still screw up by having OPTIONS_KEEPDASHDASH in
> their environments by mistake, curiosity or just plain
> stupidity.
Hmmm right, I was worried by the fact that the old PARSEOPT_OPTS was
being possibly diverted to inject malicious commands. I tend to find the
forced `OPTIONS_KEEPDASHDASH=` thing quite painful, but indeed it is
probably the sole way to guard ourselves against user stupidity (or more
likely unclean environments).
Do you mind adding: OPTIONS_KEEPDASHDASH= front to the 8 patches that
needs it, or should I send an updated series ? (actually it's more like
the 7 that needs it as git-clean has been rewritten as a builtin if I'm
correct).
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] errors: "strict subset" -> "ancestor"
From: Steffen Prohaska @ 2007-11-04 23:13 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20071104220810.GC22762@fieldses.org>
On Nov 4, 2007, at 11:08 PM, J. Bruce Fields wrote:
> On Sat, Nov 03, 2007 at 08:51:29AM +0100, Steffen Prohaska wrote:
>>
>> On Nov 3, 2007, at 3:39 AM, J. Bruce Fields wrote:
>>
>>> From: J. Bruce Fields <bfields@citi.umich.edu>
>>>
>>> The term "ancestor" is a bit more intuitive (and more consistent
>>> with
>>> the documentation) than the term "strict subset".
>>>
>>> Also, remove superfluous "ref", capitalize, and add some carriage
>>> returns, changing:
>>>
>>> error: remote 'refs/heads/master' is not a strict subset of
>>> local ref
>>> 'refs/heads/master'. maybe you are not up-to-date and need to
>>> pull first?
>>> error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/
>>> git.git'
>>>
>>> to:
>>>
>>> error: remote 'refs/heads/master' is not an ancestor of
>>> local 'refs/heads/master'.
>>> Maybe you are not up-to-date and need to pull first?
>>> error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/
>>> git.git'
>>
>>
>> Junio suggested in [1] (see also earlier messages in that
>> thread) to replace the recommendation to pull with a hint
>> where to look in the user manual.
>>
>> [1] http://marc.info/?l=git&m=119398999317677&w=2
>>
>>
>> The point is, there are various ways to resolve the problem.
>> pull is not necessarily the right solution. At least, you should
>> consider to rebase. Or maybe just something else went wrong.
>
> Yeah, actually in my case I usually want to force....
>
> So I think it's a good suggestion, but I'm putting it off for now
> as I'm
> not sure yet where to refer people to, and don't like making the
> error a
> lot longer.
I agree. And it's probably a waste of time anyway, because
sooner or later the mega-terse fetch output will be extended
to push.
> Hm. I wonder if extra "help" commandline flags would be a way to get
> people extra guidance on particular situations without cluttering
> up the
> default messages ("not sure what to try next? Try -h
> notanancestor..."
> Maybe not.)
The first step would be to describe the error messages in the
manual (or the man pages), and provide hints how to resolve
them. Currently we have no place we could link to, even if we
had a mechanism to do so.
Steffen
^ permalink raw reply
* Re: [PATCH] fix display overlap between remote and local progress
From: Junio C Hamano @ 2007-11-04 23:11 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Johannes Schindelin, git
In-Reply-To: <alpine.LFD.0.9999.0711041610520.21255@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Sun, 4 Nov 2007, Johannes Schindelin wrote:
>
>> On Sun, 4 Nov 2007, Nicolas Pitre wrote:
>>
>> > +#define SUFFIX "\e[K" /* change to " " if ANSI sequences don't work */
>>
>> I am almost certain (without even testing) that cmd.exe has problems with
>> that. It does not even understand colorisation.
>
> That's what I was expecting. This is why I suggested an alternative in
> the comment.
That's fine --- cmd.exe weenies can patch it away ;-).
The compiler at k.org complains of "\e" being non ISO-C, though.
^ permalink raw reply
* Re: Warning: cvsexportcommit considered dangerous
From: Robin Rosenberg @ 2007-11-04 23:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Steffen Prohaska, Git Mailing List, Alex Bennee
In-Reply-To: <Pine.LNX.4.64.0711042133330.4362@racer.site>
söndag 04 november 2007 skrev Johannes Schindelin:
> Hi,
>
> On Sun, 4 Nov 2007, Steffen Prohaska wrote:
>
> > On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
> >
> > > ever since the up-to-date check was changed to use just one call to
> > > "cvs status", a bug was present. Now cvsexportcommit expects "cvs
> > > status" to return the results in the same order as the file names were
> > > passed.
> > >
> > > This is not true, as I had to realise with one of my projects on
> > > sourceforge.
> > >
> > > Since time is so scarce on my side, I will not have time to fix this
> > > bug, but will instead return to my old "commit by hand" procedure.
> >
> > I introduced this 'optimization', which turned out to be a bug. So, I
> > feel responsible. Sorry for the trouble.
> >
> > In August this was already recognized and a patch submitted:
> >
> > http://marc.info/?t=118718458000004&r=1&w=2
> >
> > I do not know why it wasn't applied. I forgot re-checking after my
> > vacation.
>
> It slipped by me, because of holiday, too. (I was on my well needed
> holiday then.)
>
> But that patch really seems like a step back to me. The line "File: ...
> Status: ..." should be parsable enough to fix the bug properly, instead of
> undoing the optimisation.
Unfortunately it's not that easy to parse. It *can* be done by looking at the
repository path, and the CVS/Root etc, but it's not nice.
>
> AFAICS Robin replied with a "let's see if a proper fix materialises", and
> I kind of hope that it will materialise soon.
Still hoping :). BTW, wouldn't this err on the right side anyway, i.e. if an
existing file was not up to date and was wrongly thought to not exist or a new
file was thought to be up-to-date, I would get an error and would not be able
to commit. I've never seen it though and I always have a clean CVS checkout
so the potential bug seems unlikely to me.
The command I always use is.
git cvsexportcommit -u -c -w /my/cvs/checkout
Never bitten me yet (touch wood).
My real worry is on the other side, with bad conversion from CVS to git.
-- robin
^ permalink raw reply
* Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.
From: Junio C Hamano @ 2007-11-04 22:58 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <1194172262-1563-2-git-send-email-madcoder@debian.org>
Two comments.
* I have updated 1/10 with typo and indentation fixes.
* I see you changed 2/10 to use OPTIONS_KEEPDASHDASH instead of
PARSEOPT_OPTS, but the scripts that do not want the --keep
behaviour do not set OPTIONS_KEEPDASHDASH to empty, so I do
not see how this updatet would make _any_ difference. The
user can still screw up by having OPTIONS_KEEPDASHDASH in
their environments by mistake, curiosity or just plain
stupidity.
^ permalink raw reply
* [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-04 22:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Symonds
Signed-off-by: David Symonds <dsymonds@gmail.com>
---
Documentation/git-format-patch.txt | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 92c0ab6..d8a89a1 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -9,10 +9,11 @@ git-format-patch - Prepare patches for e-mail submission
SYNOPSIS
--------
[verse]
-'git-format-patch' [-n | -N | -k] [-o <dir> | --stdout] [--thread]
+'git-format-patch' [-k] [-o <dir> | --stdout] [--thread]
[--attach[=<boundary>] | --inline[=<boundary>]]
[-s | --signoff] [<common diff options>]
- [--start-number <n>] [--numbered-files]
+ [-n | --numbered-files | -N | --no-numbered]
+ [--start-number <n>]
[--in-reply-to=Message-Id] [--suffix=.<sfx>]
[--ignore-if-in-upstream]
[--subject-prefix=Subject-Prefix]
--
1.5.3.1
^ permalink raw reply related
* Re: [PATCH] errors: "strict subset" -> "ancestor"
From: J. Bruce Fields @ 2007-11-04 22:08 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <3F4A5458-AB2F-40C7-AA0E-9D26981BCE9D@zib.de>
On Sat, Nov 03, 2007 at 08:51:29AM +0100, Steffen Prohaska wrote:
>
> On Nov 3, 2007, at 3:39 AM, J. Bruce Fields wrote:
>
>> From: J. Bruce Fields <bfields@citi.umich.edu>
>>
>> The term "ancestor" is a bit more intuitive (and more consistent with
>> the documentation) than the term "strict subset".
>>
>> Also, remove superfluous "ref", capitalize, and add some carriage
>> returns, changing:
>>
>> error: remote 'refs/heads/master' is not a strict subset of local ref
>> 'refs/heads/master'. maybe you are not up-to-date and need to pull first?
>> error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
>>
>> to:
>>
>> error: remote 'refs/heads/master' is not an ancestor of
>> local 'refs/heads/master'.
>> Maybe you are not up-to-date and need to pull first?
>> error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
>
>
> Junio suggested in [1] (see also earlier messages in that
> thread) to replace the recommendation to pull with a hint
> where to look in the user manual.
>
> [1] http://marc.info/?l=git&m=119398999317677&w=2
>
>
> The point is, there are various ways to resolve the problem.
> pull is not necessarily the right solution. At least, you should
> consider to rebase. Or maybe just something else went wrong.
Yeah, actually in my case I usually want to force....
So I think it's a good suggestion, but I'm putting it off for now as I'm
not sure yet where to refer people to, and don't like making the error a
lot longer.
Hm. I wonder if extra "help" commandline flags would be a way to get
people extra guidance on particular situations without cluttering up the
default messages ("not sure what to try next? Try -h notanancestor..."
Maybe not.)
>
> Nonetheless I think it could be a good idea to keep the most
> likely cases. So, how about
>
> "Are you up-to-date? Did you forget to pull or rebase? See User's Manual
> for details."
>
> I put it as questions to avoid making a suggestion. The questions
> should give sufficient hints for searching in the User's Manual.
> I haven't found the single section that would explain exactly
> the situation we're dealing with.
Me neither. And I don't think a reference to the whole thing is
helpful.
--b.
^ permalink raw reply
* [PATCH] errors: "strict subset" -> "ancestor"
From: J. Bruce Fields @ 2007-11-04 21:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Symonds, git, Steffen Prohaska
In-Reply-To: <2B1935E5-C490-4EB4-9BE6-0DD7F33FFE36@zib.de>
From: J. Bruce Fields <bfields@citi.umich.edu>
The term "ancestor" is a bit more intuitive (and more consistent with
the documentation) than the term "strict subset".
Also, remove superfluous "ref", and capitalize, changing:
error: remote 'refs/heads/master' is not a strict subset of local ref 'refs/heads/master'. maybe you are not up-to-date and need to pull first?
error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
to:
error: remote 'refs/heads/master' is not an ancestor of
local 'refs/heads/master'.
Maybe you are not up-to-date and need to pull first?
error: failed to push to 'ssh://linux-nfs.org/~bfields/exports/git.git'
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
builtin-branch.c | 2 +-
http-push.c | 8 ++++----
send-pack.c | 6 +++---
3 files changed, 8 insertions(+), 8 deletions(-)
On Sat, Nov 03, 2007 at 08:24:29AM +0100, Steffen Prohaska wrote:
> They are not aligned. The second line is indented with one
> space. Look at examples in the commit message. The first line
> starts with "error:", which already destroys the alignment.
Yup, I think that's exactly what happened--I said "hey, maybe I should
try aligning this and see what it looks like?", then saw the problem,
then forgot to revert the extra space from everywhere.... Thanks for
noticing. Here's an updated patch.--b.
diff --git a/builtin-branch.c b/builtin-branch.c
index 3da8b55..e8de27e 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -142,7 +142,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
if (!force &&
!in_merge_bases(rev, &head_rev, 1)) {
- error("The branch '%s' is not a strict subset of "
+ error("The branch '%s' is not an ancestor of "
"your current HEAD.\n"
"If you are sure you want to delete it, "
"run 'git branch -D %s'.", argv[i], argv[i]);
diff --git a/http-push.c b/http-push.c
index c02a3af..5960d7c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2241,7 +2241,7 @@ static int delete_remote_branch(char *pattern, int force)
/* Remote branch must be an ancestor of remote HEAD */
if (!verify_merge_base(head_sha1, remote_ref->old_sha1)) {
- return error("The branch '%s' is not a strict subset of your current HEAD.\nIf you are sure you want to delete it, run:\n\t'git http-push -D %s %s'", remote_ref->name, remote->url, pattern);
+ return error("The branch '%s' is not an ancestor of your current HEAD.\nIf you are sure you want to delete it, run:\n\t'git http-push -D %s %s'", remote_ref->name, remote->url, pattern);
}
}
@@ -2424,9 +2424,9 @@ int main(int argc, char **argv)
* commits at the remote end and likely
* we were not up to date to begin with.
*/
- error("remote '%s' is not a strict "
- "subset of local ref '%s'. "
- "maybe you are not up-to-date and "
+ error("remote '%s' is not an ancestor of\n"
+ " local '%s'.\n"
+ " Maybe you are not up-to-date and "
"need to pull first?",
ref->name,
ref->peer_ref->name);
diff --git a/send-pack.c b/send-pack.c
index 5e127a1..fbf2462 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -297,9 +297,9 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
* commits at the remote end and likely
* we were not up to date to begin with.
*/
- error("remote '%s' is not a strict "
- "subset of local ref '%s'. "
- "maybe you are not up-to-date and "
+ error("remote '%s' is not an ancestor of\n"
+ " local '%s'.\n"
+ " Maybe you are not up-to-date and "
"need to pull first?",
ref->name,
ref->peer_ref->name);
--
1.5.3.5.475.g477d-dirty
^ permalink raw reply related
* Re: gitk graph routing problem
From: Paul Mackerras @ 2007-11-04 21:47 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071104104618.GA3078@steel.home>
Alex Riesen writes:
> To reproduce, try running in git repo:
>
> gitk 02f630448e5d48e..06ea6ba9cf46ef5
I can't reproduce it here, as my clone of the git repo doesn't have
02f630448e5d48e in it, even after updating...
Paul.
^ permalink raw reply
* Re: [PATCH] Fix an infinite loop in sq_quote_buf().
From: Pierre Habouzit @ 2007-11-04 21:37 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio Hamano, git
In-Reply-To: <200711042126.22512.johannes.sixt@telecom.at>
[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]
On Sun, Nov 04, 2007 at 08:26:22PM +0000, Johannes Sixt wrote:
> sq_quote_buf() treats single-quotes and exclamation marks specially, but
> it incorrectly parsed the input for single-quotes and backslashes.
>
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
> quote.c | 2 +-
> t/t5510-fetch.sh | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/quote.c b/quote.c
> index 482be05..919d092 100644
> --- a/quote.c
> +++ b/quote.c
> @@ -26,7 +26,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
>
> strbuf_addch(dst, '\'');
> while (*src) {
> - size_t len = strcspn(src, "'\\");
> + size_t len = strcspn(src, "'!");
> strbuf_add(dst, src, len);
> src += len;
> while (need_bs_quote(*src)) {
Ouch, good catch :/ sorry about that.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Warning: cvsexportcommit considered dangerous
From: Johannes Schindelin @ 2007-11-04 21:35 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Git Mailing List, Alex Bennee, Robin Rosenberg
In-Reply-To: <623F4AFA-FE43-4046-9D3F-435396BBE17D@zib.de>
Hi,
On Sun, 4 Nov 2007, Steffen Prohaska wrote:
> On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
>
> > ever since the up-to-date check was changed to use just one call to
> > "cvs status", a bug was present. Now cvsexportcommit expects "cvs
> > status" to return the results in the same order as the file names were
> > passed.
> >
> > This is not true, as I had to realise with one of my projects on
> > sourceforge.
> >
> > Since time is so scarce on my side, I will not have time to fix this
> > bug, but will instead return to my old "commit by hand" procedure.
>
> I introduced this 'optimization', which turned out to be a bug. So, I
> feel responsible. Sorry for the trouble.
>
> In August this was already recognized and a patch submitted:
>
> http://marc.info/?t=118718458000004&r=1&w=2
>
> I do not know why it wasn't applied. I forgot re-checking after my
> vacation.
It slipped by me, because of holiday, too. (I was on my well needed
holiday then.)
But that patch really seems like a step back to me. The line "File: ...
Status: ..." should be parsable enough to fix the bug properly, instead of
undoing the optimisation.
AFAICS Robin replied with a "let's see if a proper fix materialises", and
I kind of hope that it will materialise soon.
Ciao,
Dscho
^ permalink raw reply
* [RFC PATCH] Reduce the number of connects when fetching
From: Daniel Barkalow @ 2007-11-04 21:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This shares the connection between getting the remote ref list and
getting objects in the first batch. (A second connection is still used
to follow tags)
It passes all of the tests, and should be fine, but I don't entirely
understand the git protocol, so I'd like people who know it better to
check this over. In particular, I don't know if there's a way to have the
connection end up in a state where objects for more refs can be requested
after some refs have been requested and the resulting objects read.
The idea is to keep the open connection in the data for the transport in
between getting the list of refs and doing anything further. This
therefore moves the connection-handling aspects outside of fetch-pack()
and handles them primarily in transport.c.
This is on top of current next.
---
builtin-fetch-pack.c | 74 ++++++++++++++++++++++++++-----------------------
fetch-pack.h | 2 +
transport.c | 41 +++++++++++++++++++--------
3 files changed, 70 insertions(+), 47 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 862652b..7783c05 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -7,6 +7,7 @@
#include "pack.h"
#include "sideband.h"
#include "fetch-pack.h"
+#include "remote.h"
#include "run-command.h"
static int transfer_unpack_limit = -1;
@@ -558,14 +559,14 @@ static int get_pack(int xd[2], char **pack_lockfile)
}
static struct ref *do_fetch_pack(int fd[2],
+ const struct ref *orig_ref,
int nr_match,
char **match,
char **pack_lockfile)
{
- struct ref *ref;
+ struct ref *ref = copy_ref_list(orig_ref);
unsigned char sha1[20];
- get_remote_heads(fd[0], &ref, 0, NULL, 0);
if (is_repository_shallow() && !server_supports("shallow"))
die("Server does not support shallow clients");
if (server_supports("multi_ack")) {
@@ -583,10 +584,6 @@ static struct ref *do_fetch_pack(int fd[2],
fprintf(stderr, "Server supports side-band\n");
use_sideband = 1;
}
- if (!ref) {
- packet_flush(fd[1]);
- die("no matching remote head");
- }
if (everything_local(&ref, nr_match, match)) {
packet_flush(fd[1]);
goto all_done;
@@ -660,7 +657,7 @@ static void fetch_pack_setup(void)
int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
{
int i, ret, nr_heads;
- struct ref *ref;
+ struct ref *ref = NULL;
char *dest = NULL, **heads;
nr_heads = 0;
@@ -716,9 +713,34 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
if (!dest)
usage(fetch_pack_usage);
- ref = fetch_pack(&args, dest, nr_heads, heads, NULL);
+ int fd[2];
+ struct child_process *conn = git_connect(fd, (char *)dest, args.uploadpack,
+ args.verbose ? CONNECT_VERBOSE : 0);
+ if (conn) {
+ get_remote_heads(fd[0], &ref, 0, NULL, 0);
+
+ ref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL);
+ close(fd[0]);
+ close(fd[1]);
+ if (finish_connect(conn))
+ ref = NULL;
+ } else {
+ ref = NULL;
+ }
ret = !ref;
+ if (!ret && nr_heads) {
+ /* If the heads to pull were given, we should have
+ * consumed all of them by matching the remote.
+ * Otherwise, 'git-fetch remote no-such-ref' would
+ * silently succeed without issuing an error.
+ */
+ for (i = 0; i < nr_heads; i++)
+ if (heads[i] && heads[i][0]) {
+ error("no such remote ref %s", heads[i]);
+ ret = 1;
+ }
+ }
while (ref) {
printf("%s %s\n",
sha1_to_hex(ref->old_sha1), ref->name);
@@ -729,16 +751,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
}
struct ref *fetch_pack(struct fetch_pack_args *my_args,
+ int fd[], pid_t pid,
+ const struct ref *ref,
const char *dest,
int nr_heads,
char **heads,
char **pack_lockfile)
{
- int i, ret;
- int fd[2];
- struct child_process *conn;
- struct ref *ref;
struct stat st;
+ struct ref *ref_cpy;
fetch_pack_setup();
memcpy(&args, my_args, sizeof(args));
@@ -747,29 +768,15 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
st.st_mtime = 0;
}
- conn = git_connect(fd, (char *)dest, args.uploadpack,
- args.verbose ? CONNECT_VERBOSE : 0);
if (heads && nr_heads)
nr_heads = remove_duplicates(nr_heads, heads);
- ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile);
- close(fd[0]);
- close(fd[1]);
- ret = finish_connect(conn);
-
- if (!ret && nr_heads) {
- /* If the heads to pull were given, we should have
- * consumed all of them by matching the remote.
- * Otherwise, 'git-fetch remote no-such-ref' would
- * silently succeed without issuing an error.
- */
- for (i = 0; i < nr_heads; i++)
- if (heads[i] && heads[i][0]) {
- error("no such remote ref %s", heads[i]);
- ret = 1;
- }
+ if (!ref) {
+ packet_flush(fd[1]);
+ die("no matching remote head");
}
+ ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
- if (!ret && args.depth > 0) {
+ if (args.depth > 0) {
struct cache_time mtime;
char *shallow = git_path("shallow");
int fd;
@@ -798,8 +805,5 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
}
}
- if (ret)
- ref = NULL;
-
- return ref;
+ return ref_cpy;
}
diff --git a/fetch-pack.h b/fetch-pack.h
index a7888ea..8d35ef6 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -16,6 +16,8 @@ struct fetch_pack_args
};
struct ref *fetch_pack(struct fetch_pack_args *args,
+ int fd[], struct child_process *conn,
+ const struct ref *ref,
const char *dest,
int nr_heads,
char **heads,
diff --git a/transport.c b/transport.c
index f4577b7..175cf2c 100644
--- a/transport.c
+++ b/transport.c
@@ -567,6 +567,8 @@ struct git_transport_data {
unsigned thin : 1;
unsigned keep : 1;
int depth;
+ struct child_process *conn;
+ int fd[2];
const char *uploadpack;
const char *receivepack;
};
@@ -597,20 +599,20 @@ static int set_git_option(struct transport *connection,
return 1;
}
+static int connect_setup(struct transport *transport)
+{
+ struct git_transport_data *data = transport->data;
+ data->conn = git_connect(data->fd, transport->url, data->uploadpack, 0);
+ return 0;
+}
+
static struct ref *get_refs_via_connect(struct transport *transport)
{
struct git_transport_data *data = transport->data;
struct ref *refs;
- int fd[2];
- char *dest = xstrdup(transport->url);
- struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0);
- get_remote_heads(fd[0], &refs, 0, NULL, 0);
- packet_flush(fd[1]);
-
- finish_connect(conn);
-
- free(dest);
+ connect_setup(transport);
+ get_remote_heads(data->fd[0], &refs, 0, NULL, 0);
return refs;
}
@@ -621,7 +623,7 @@ static int fetch_refs_via_pack(struct transport *transport,
struct git_transport_data *data = transport->data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
char **origh = xmalloc(nr_heads * sizeof(*origh));
- struct ref *refs;
+ const struct ref *refs;
char *dest = xstrdup(transport->url);
struct fetch_pack_args args;
int i;
@@ -636,13 +638,27 @@ static int fetch_refs_via_pack(struct transport *transport,
for (i = 0; i < nr_heads; i++)
origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
- refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);
+
+ refs = transport_get_remote_refs(transport);
+ if (!data->conn) {
+ struct ref *refs_tmp;
+ connect_setup(transport);
+ get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0);
+ free_refs(refs_tmp);
+ }
+
+ refs = fetch_pack(&args, data->fd, data->conn, transport->remote_refs,
+ dest, nr_heads, heads, &transport->pack_lockfile);
+ close(data->fd[0]);
+ close(data->fd[1]);
+ if (finish_connect(data->conn))
+ refs = NULL;
+ data->conn = NULL;
for (i = 0; i < nr_heads; i++)
free(origh[i]);
free(origh);
free(heads);
- free_refs(refs);
free(dest);
return 0;
}
@@ -723,6 +739,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
ret->disconnect = disconnect_git;
data->thin = 1;
+ data->conn = NULL;
data->uploadpack = "git-upload-pack";
if (remote && remote->uploadpack)
data->uploadpack = remote->uploadpack;
--
1.5.3.4.1206.g5f96
^ permalink raw reply related
* Re: Warning: cvsexportcommit considered dangerous
From: Steffen Prohaska @ 2007-11-04 18:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Alex Bennee, Robin Rosenberg
In-Reply-To: <Pine.LNX.4.64.0711041638270.4362@racer.site>
On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
> ever since the up-to-date check was changed to use just one call to
> "cvs
> status", a bug was present. Now cvsexportcommit expects "cvs
> status" to
> return the results in the same order as the file names were passed.
>
> This is not true, as I had to realise with one of my projects on
> sourceforge.
>
> Since time is so scarce on my side, I will not have time to fix
> this bug,
> but will instead return to my old "commit by hand" procedure.
I introduced this 'optimization', which turned out to be a bug.
So, I feel responsible. Sorry for the trouble.
In August this was already recognized and a patch submitted:
http://marc.info/?t=118718458000004&r=1&w=2
I do not know why it wasn't applied. I forgot re-checking after my
vacation.
I put all on CC who replied to the patch in August.
Steffen
^ permalink raw reply
* Re: [PATCH] fix display overlap between remote and local progress
From: Nicolas Pitre @ 2007-11-04 21:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711041331520.4362@racer.site>
On Sun, 4 Nov 2007, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 4 Nov 2007, Nicolas Pitre wrote:
>
> > +#define SUFFIX "\e[K" /* change to " " if ANSI sequences don't work */
>
> I am almost certain (without even testing) that cmd.exe has problems with
> that. It does not even understand colorisation.
That's what I was expecting. This is why I suggested an alternative in
the comment.
Nicolas
^ permalink raw reply
* Re: [PATCH 3/2] Use parse-options in builtin-clean
From: Pierre Habouzit @ 2007-11-04 21:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <Pine.LNX.4.64.0711042023440.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 688 bytes --]
On Sun, Nov 04, 2007 at 08:24:31PM +0000, Johannes Schindelin wrote:
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> On Sun, 4 Nov 2007, Pierre Habouzit wrote:
>
> > On Sun, Nov 04, 2007 at 07:02:21PM +0000, Shawn Bohrer wrote:
> >
> > > + for (i = 1; i < argc; i++) {
> > > + const char *arg = argv[i];
> > > [...]
> >
> > Please, parse-options.c is now in next, please use it.
>
> Something like this?
something like this works for me :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] Build in ls-remote
From: Daniel Barkalow @ 2007-11-04 20:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This actually replaces peek-remote with ls-remote, since peek-remote
now handles everything. peek-remote remains an a second name for
ls-remote, although its help message now gives the "ls-remote" name.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Keeping scripts in .../examples makes the diffstat much less compelling.
This actually replaces a 142-line shell script with one line in git.c. I'm
not actually sure if this one is even worth keeping as an example, since
the git-specific portions just delegated to C code anyway.
Makefile | 3 +--
builtin-peek-remote.c => builtin-ls-remote.c | 10 +++++-----
builtin.h | 2 +-
.../examples/git-ls-remote.sh | 0
git.c | 3 ++-
5 files changed, 9 insertions(+), 9 deletions(-)
rename builtin-peek-remote.c => builtin-ls-remote.c (83%)
rename git-ls-remote.sh => contrib/examples/git-ls-remote.sh (100%)
diff --git a/Makefile b/Makefile
index 3ec1876..470e54a 100644
--- a/Makefile
+++ b/Makefile
@@ -210,7 +210,6 @@ BASIC_LDFLAGS =
SCRIPT_SH = \
git-bisect.sh git-checkout.sh \
git-clean.sh git-clone.sh git-commit.sh \
- git-ls-remote.sh \
git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
git-pull.sh git-rebase.sh git-rebase--interactive.sh \
git-repack.sh git-request-pull.sh \
@@ -345,6 +344,7 @@ BUILTIN_OBJS = \
builtin-log.o \
builtin-ls-files.o \
builtin-ls-tree.o \
+ builtin-ls-remote.o \
builtin-mailinfo.o \
builtin-mailsplit.o \
builtin-merge-base.o \
@@ -352,7 +352,6 @@ BUILTIN_OBJS = \
builtin-mv.o \
builtin-name-rev.o \
builtin-pack-objects.o \
- builtin-peek-remote.o \
builtin-prune.o \
builtin-prune-packed.o \
builtin-push.o \
diff --git a/builtin-peek-remote.c b/builtin-ls-remote.c
similarity index 83%
rename from builtin-peek-remote.c
rename to builtin-ls-remote.c
index b4106f5..003580c 100644
--- a/builtin-peek-remote.c
+++ b/builtin-ls-remote.c
@@ -3,10 +3,10 @@
#include "transport.h"
#include "remote.h"
-static const char peek_remote_usage[] =
-"git-peek-remote [--upload-pack=<git-upload-pack>] [<host>:]<directory>";
+static const char ls_remote_usage[] =
+"git-ls-remote [--upload-pack=<git-upload-pack>] [<host>:]<directory>";
-int cmd_peek_remote(int argc, const char **argv, const char *prefix)
+int cmd_ls_remote(int argc, const char **argv, const char *prefix)
{
int i;
const char *dest = NULL;
@@ -43,14 +43,14 @@ int cmd_peek_remote(int argc, const char **argv, const char *prefix)
flags |= REF_NORMAL;
continue;
}
- usage(peek_remote_usage);
+ usage(ls_remote_usage);
}
dest = arg;
break;
}
if (!dest || i != argc - 1)
- usage(peek_remote_usage);
+ usage(ls_remote_usage);
transport = transport_get(NULL, dest);
if (uploadpack != NULL)
diff --git a/builtin.h b/builtin.h
index 2335c01..525107f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -48,6 +48,7 @@ extern int cmd_log(int argc, const char **argv, const char *prefix);
extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
extern int cmd_ls_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_ls_remote(int argc, const char **argv, const char *prefix);
extern int cmd_mailinfo(int argc, const char **argv, const char *prefix);
extern int cmd_mailsplit(int argc, const char **argv, const char *prefix);
extern int cmd_merge_base(int argc, const char **argv, const char *prefix);
@@ -55,7 +56,6 @@ extern int cmd_merge_file(int argc, const char **argv, const char *prefix);
extern int cmd_mv(int argc, const char **argv, const char *prefix);
extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
-extern int cmd_peek_remote(int argc, const char **argv, const char *prefix);
extern int cmd_pickaxe(int argc, const char **argv, const char *prefix);
extern int cmd_prune(int argc, const char **argv, const char *prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
diff --git a/git-ls-remote.sh b/contrib/examples/git-ls-remote.sh
similarity index 100%
rename from git-ls-remote.sh
rename to contrib/examples/git-ls-remote.sh
diff --git a/git.c b/git.c
index 19a2172..b173f22 100644
--- a/git.c
+++ b/git.c
@@ -326,6 +326,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "log", cmd_log, RUN_SETUP | USE_PAGER },
{ "ls-files", cmd_ls_files, RUN_SETUP },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+ { "ls-remote", cmd_ls_remote },
{ "mailinfo", cmd_mailinfo },
{ "mailsplit", cmd_mailsplit },
{ "merge-base", cmd_merge_base, RUN_SETUP },
@@ -333,7 +334,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
{ "name-rev", cmd_name_rev, RUN_SETUP },
{ "pack-objects", cmd_pack_objects, RUN_SETUP },
- { "peek-remote", cmd_peek_remote },
+ { "peek-remote", cmd_ls_remote },
{ "pickaxe", cmd_blame, RUN_SETUP },
{ "prune", cmd_prune, RUN_SETUP },
{ "prune-packed", cmd_prune_packed, RUN_SETUP },
--
1.5.3.4.1206.g5f96
^ permalink raw reply related
* [PATCH qgit] Update to latest --early-output git log patch
From: Marco Costalba @ 2007-11-04 20:50 UTC (permalink / raw)
To: Git Mailing List; +Cc: Linus Torvalds
Fix broken implementation after Linus updated his
early output patch to version with improve output format.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
src/git.h | 2 +-
src/git_startup.cpp | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/git.h b/src/git.h
index 92879fb..789fea4 100644
--- a/src/git.h
+++ b/src/git.h
@@ -251,7 +251,7 @@ private:
bool startParseProc(SCList initCmd, FileHistory* fh, SCRef buf);
bool tryFollowRenames(FileHistory* fh);
bool populateRenamedPatches(SCRef sha, SCList nn, FileHistory* fh,
QStringList* on, bool bt);
- void doEarlyOutput(Rev* rev, int* start);
+ void doEarlyOutput(Rev* rev, const QByteArray& ba, int* start);
int addChunk(FileHistory* fh, const QByteArray& ba, int ofs);
void parseDiffFormat(RevFile& rf, SCRef buf);
void parseDiffFormatLine(RevFile& rf, SCRef line, int parNum);
diff --git a/src/git_startup.cpp b/src/git_startup.cpp
index df272fc..090d5f9 100644
--- a/src/git_startup.cpp
+++ b/src/git_startup.cpp
@@ -841,10 +841,10 @@ void Git::loadFileNames() {
indexTree();
}
-void Git::doEarlyOutput(Rev* rev, int* start) {
+void Git::doEarlyOutput(Rev* rev, const QByteArray& ba, int* start) {
delete rev;
- *start += QString("Final output:\n").length();
+ *start = ba.indexOf('\n', *start) + 1;
Rev* cl = NULL;
const Rev* r = revLookup(ZERO_SHA);
@@ -870,7 +870,7 @@ int Git::addChunk(FileHistory* fh, const
QByteArray& ba, int start) {
rev = new Rev(ba, start, fh->revOrder.count(), &nextStart,
!isMainHistory(fh));
if (nextStart == -2)
- doEarlyOutput(rev, &start);
+ doEarlyOutput(rev, ba, &start);
} while (nextStart == -2);
@@ -1377,8 +1377,8 @@ int Rev::indexData(bool quick, bool withDiff) const {
if (start > last) // offset 'start' points to the char after "commit "
return -1;
- if (uint(ba.at(start) == 'u'))
- return -2; // "Final output:", let caller handle this
+ if (uint(ba.at(start) == 'u')) // "Final output:", let caller handle this
+ return (ba.indexOf('\n', start) != -1 ? -2 : -1);
// take in account --boundary and --left-right options
startOfs = uint(ba.at(start) == '-' || ba.at(start) == '<' ||
ba.at(start) == '>');
--
1.5.3.5.565.g985b6
^ permalink raw reply related
* [PATCH] Fix an infinite loop in sq_quote_buf().
From: Johannes Sixt @ 2007-11-04 20:26 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
sq_quote_buf() treats single-quotes and exclamation marks specially, but
it incorrectly parsed the input for single-quotes and backslashes.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
quote.c | 2 +-
t/t5510-fetch.sh | 7 +++++++
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/quote.c b/quote.c
index 482be05..919d092 100644
--- a/quote.c
+++ b/quote.c
@@ -26,7 +26,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
strbuf_addch(dst, '\'');
while (*src) {
- size_t len = strcspn(src, "'\\");
+ size_t len = strcspn(src, "'!");
strbuf_add(dst, src, len);
src += len;
while (need_bs_quote(*src)) {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d217657..aad863d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -208,4 +208,11 @@ test_expect_success 'fetch with a non-applying branch.<name>.merge' '
git fetch blub
'
+# the strange name is: a\!'b
+test_expect_success 'quoting of a strangely named repo' '
+ ! git fetch "a\\!'\''b" > result 2>&1 &&
+ cat result &&
+ grep "fatal: '\''a\\\\!'\''b'\''" result
+'
+
test_done
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 3/2] Use parse-options in builtin-clean
From: Johannes Schindelin @ 2007-11-04 20:24 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <20071104194129.GA4207@artemis.corp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Sun, 4 Nov 2007, Pierre Habouzit wrote:
> On Sun, Nov 04, 2007 at 07:02:21PM +0000, Shawn Bohrer wrote:
>
> > + for (i = 1; i < argc; i++) {
> > + const char *arg = argv[i];
> > [...]
>
> Please, parse-options.c is now in next, please use it.
Something like this?
builtin-clean.c | 71 ++++++++++++++++++++----------------------------------
1 files changed, 26 insertions(+), 45 deletions(-)
diff --git a/builtin-clean.c b/builtin-clean.c
index 4141eb4..d6fc2ad 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -9,81 +9,62 @@
#include "builtin.h"
#include "cache.h"
#include "dir.h"
+#include "parse-options.h"
-static int disabled = 1;
+static int force = 0;
static int show_only = 0;
static int remove_directories = 0;
static int quiet = 0;
static int ignored = 0;
static int ignored_only = 0;
-static const char builtin_clean_usage[] =
-"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+static const char *const builtin_clean_usage[] = {
+ "git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...",
+ NULL
+};
static int git_clean_config(const char *var, const char *value)
{
if (!strcmp(var, "clean.requireforce")) {
- disabled = git_config_bool(var, value);
+ force = !git_config_bool(var, value);
}
return 0;
}
int cmd_clean(int argc, const char **argv, const char *prefix)
{
- int i, j;
+ int j;
struct strbuf directory;
struct dir_struct dir;
const char *path = ".";
const char *base = "";
int baselen = 0;
static const char **pathspec;
+ struct option options[] = {
+ OPT__QUIET(&quiet),
+ OPT__DRY_RUN(&show_only),
+ OPT_BOOLEAN('f', NULL, &force, "force"),
+ OPT_BOOLEAN('d', NULL, &remove_directories,
+ "remove whole directories"),
+ OPT_BOOLEAN('x', NULL, &ignored, "remove ignored files, too"),
+ OPT_BOOLEAN('X', NULL, &ignored_only,
+ "remove only ignored files"),
+ OPT_END()
+ };
- memset(&dir, 0, sizeof(dir));
git_config(git_clean_config);
+ argc = parse_options(argc, argv, options, builtin_clean_usage, 0);
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
-
- if (arg[0] != '-')
- break;
- if (!strcmp(arg, "--")) {
- i++;
- break;
- }
- if (!strcmp(arg, "-n")) {
- show_only = 1;
- disabled = 0;
- continue;
- }
- if (!strcmp(arg, "-f")) {
- disabled = 0;
- continue;
- }
- if (!strcmp(arg, "-d")) {
- remove_directories = 1;
- continue;
- }
- if (!strcmp(arg, "-q")) {
- quiet = 1;
- continue;
- }
- if (!strcmp(arg, "-x")) {
- ignored = 1;
- continue;
- }
- if (!strcmp(arg, "-X")) {
- ignored_only = 1;
- dir.show_ignored =1;
- dir.exclude_per_dir = ".gitignore";
- continue;
- }
- usage(builtin_clean_usage);
+ memset(&dir, 0, sizeof(dir));
+ if (ignored_only) {
+ dir.show_ignored =1;
+ dir.exclude_per_dir = ".gitignore";
}
if (ignored && ignored_only)
die("-x and -X cannot be used together");
- if (disabled)
+ if (!show_only && !force)
die("clean.requireForce set and -n or -f not given; refusing to clean");
dir.show_other_directories = 1;
@@ -96,7 +77,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
}
}
- pathspec = get_pathspec(prefix, argv + i);
+ pathspec = get_pathspec(prefix, argv);
read_cache();
read_directory(&dir, path, base, baselen, pathspec);
strbuf_init(&directory, 0);
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* [PATCH 3/2] Enhance --early-output format
From: Linus Torvalds @ 2007-11-04 20:12 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Marco Costalba, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0711041004220.15101@woody.linux-foundation.org>
This makes --early-output a bit more advanced, and actually makes it
generate multiple "Final output:" headers as it updates things
asynchronously. I realize that the "Final output:" line is now illogical,
since it's not really final until it also says "done", but
It now _always_ generates a "Final output:" header in front of any commit
list, and that output header gives you a *guess* at the maximum number of
commits available. However, it should be noted that the guess can be
completely off: I do a reasonable job estimating it, but it is not meant
to be exact.
So what happens is that you may get output like this:
- at 0.1 seconds:
Final output: 2 incomplete
.. 2 commits listed ..
- half a second later:
Final output: 33 incomplete
.. 33 commits listed ..
- another half a second after that:
Final output: 71 incomplete
.. 71 commits listed ..
- another half second later:
Final output: 136 incomplete
.. 100 commits listed: we hit the --early-output limit, and
.. will only output 100 commits, and after this you'll not
.. see an "incomplete" report any more since you got as much
.. early output as you asked for!
- .. and then finally:
Final output: 73106 done
.. all the commits ..
The above is a real-life scenario on my current kernel tree after having
flushed all the caches.
Tested with the experimental gitk patch that Paul sent out, and by looking
at the actual log output (and verifying that my commit count guesses
actually match real life fairly well).
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
On Sun, 4 Nov 2007, Linus Torvalds wrote:
>
> I'm looking at it now, I'll have to think about this a bit more. It might
> be trivial to fix, but this thing has real potential for being subtle.
It wasn't totally trivial, but it doesn't seem to be excessively subtle
either. About half the patch is moving around some code to look at whether
the commit is interesting or not and rewriting the parents, so that it can
be shared with the revision walker.
builtin-log.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++--------
revision.c | 63 +++++++++++++++++++++++-----------------
revision.h | 8 +++++
3 files changed, 119 insertions(+), 40 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 707add2..268a7af 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -77,17 +77,85 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
}
}
+/*
+ * This gives a rough estimate for how many commits we
+ * will print out in the list.
+ */
+static int estimate_commit_count(struct rev_info *rev, struct commit_list *list)
+{
+ int n = 0;
+
+ while (list) {
+ struct commit *commit = list->item;
+ unsigned int flags = commit->object.flags;
+
+ list = list->next;
+ if (flags & UNINTERESTING)
+ continue;
+ if (rev->prune_fn && rev->dense && !(flags & TREECHANGE)) {
+ if (commit->parents && !commit->parents->next)
+ continue;
+ }
+ n++;
+ }
+ return n;
+}
+
+static void show_early_header(struct rev_info *rev, const char *stage, int nr)
+{
+ if (rev->shown_one) {
+ rev->shown_one = 0;
+ if (rev->commit_format != CMIT_FMT_ONELINE)
+ putchar(rev->diffopt.line_termination);
+ }
+ printf("Final output: %d %s\n", nr, stage);
+}
+
+struct itimerval early_output_timer;
+
static void log_show_early(struct rev_info *revs, struct commit_list *list)
{
int i = revs->early_output;
+ int show_header = 1;
sort_in_topological_order(&list, revs->lifo);
while (list && i) {
struct commit *commit = list->item;
- log_tree_commit(revs, commit);
+ switch (simplify_commit(revs, commit)) {
+ case commit_show:
+ if (show_header) {
+ int n = estimate_commit_count(revs, list);
+ show_early_header(revs, "incomplete", n);
+ show_header = 0;
+ }
+ log_tree_commit(revs, commit);
+ i--;
+ break;
+ case commit_ignore:
+ break;
+ case commit_error:
+ return;
+ }
list = list->next;
- i--;
}
+
+ /* Did we already get enough commits for the early output? */
+ if (!i)
+ return;
+
+ /*
+ * ..if no, then repeat it twice a second until we
+ * do.
+ *
+ * NOTE! We don't use "it_interval", because if the
+ * reader isn't listening, we want our output to be
+ * throttled by the writing, and not have the timer
+ * trigger every second even if we're blocked on a
+ * reader!
+ */
+ early_output_timer.it_value.tv_sec = 0;
+ early_output_timer.it_value.tv_usec = 500000;
+ setitimer(ITIMER_REAL, &early_output_timer, NULL);
}
static void early_output(int signal)
@@ -98,7 +166,6 @@ static void early_output(int signal)
static void setup_early_output(struct rev_info *rev)
{
struct sigaction sa;
- struct itimerval v;
/*
* Set up the signal handler, minimally intrusively:
@@ -120,21 +187,16 @@ static void setup_early_output(struct rev_info *rev)
*
* This is a one-time-only trigger.
*/
- memset(&v, 0, sizeof(v));
- v.it_value.tv_sec = 0;
- v.it_value.tv_usec = 100000;
- setitimer(ITIMER_REAL, &v, NULL);
+ early_output_timer.it_value.tv_sec = 0;
+ early_output_timer.it_value.tv_usec = 100000;
+ setitimer(ITIMER_REAL, &early_output_timer, NULL);
}
static void finish_early_output(struct rev_info *rev)
{
+ int n = estimate_commit_count(rev, rev->commits);
signal(SIGALRM, SIG_IGN);
- if (rev->shown_one) {
- rev->shown_one = 0;
- if (rev->commit_format != CMIT_FMT_ONELINE)
- putchar(rev->diffopt.line_termination);
- }
- printf("Final output:\n");
+ show_early_header(rev, "done", n);
}
static int cmd_log_walk(struct rev_info *rev)
diff --git a/revision.c b/revision.c
index 26610bb..5d6f208 100644
--- a/revision.c
+++ b/revision.c
@@ -1398,6 +1398,36 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
commit->buffer, strlen(commit->buffer));
}
+enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
+{
+ if (commit->object.flags & SHOWN)
+ return commit_ignore;
+ if (revs->unpacked && has_sha1_pack(commit->object.sha1, revs->ignore_packed))
+ return commit_ignore;
+ if (commit->object.flags & UNINTERESTING)
+ return commit_ignore;
+ if (revs->min_age != -1 && (commit->date > revs->min_age))
+ return commit_ignore;
+ if (revs->no_merges && commit->parents && commit->parents->next)
+ return commit_ignore;
+ if (!commit_match(commit, revs))
+ return commit_ignore;
+ if (revs->prune_fn && revs->dense) {
+ /* Commit without changes? */
+ if (!(commit->object.flags & TREECHANGE)) {
+ /* drop merges unless we want parenthood */
+ if (!revs->parents)
+ return commit_ignore;
+ /* non-merge - always ignore it */
+ if (!commit->parents || !commit->parents->next)
+ return commit_ignore;
+ }
+ if (revs->parents && rewrite_parents(revs, commit) < 0)
+ return commit_error;
+ }
+ return commit_show;
+}
+
static struct commit *get_revision_1(struct rev_info *revs)
{
if (!revs->commits)
@@ -1425,36 +1455,15 @@ static struct commit *get_revision_1(struct rev_info *revs)
if (add_parents_to_list(revs, commit, &revs->commits) < 0)
return NULL;
}
- if (commit->object.flags & SHOWN)
- continue;
-
- if (revs->unpacked && has_sha1_pack(commit->object.sha1,
- revs->ignore_packed))
- continue;
- if (commit->object.flags & UNINTERESTING)
- continue;
- if (revs->min_age != -1 && (commit->date > revs->min_age))
- continue;
- if (revs->no_merges &&
- commit->parents && commit->parents->next)
- continue;
- if (!commit_match(commit, revs))
+ switch (simplify_commit(revs, commit)) {
+ case commit_ignore:
continue;
- if (revs->prune_fn && revs->dense) {
- /* Commit without changes? */
- if (!(commit->object.flags & TREECHANGE)) {
- /* drop merges unless we want parenthood */
- if (!revs->parents)
- continue;
- /* non-merge - always ignore it */
- if (!commit->parents || !commit->parents->next)
- continue;
- }
- if (revs->parents && rewrite_parents(revs, commit) < 0)
- return NULL;
+ case commit_error:
+ return -1;
+ default:
+ return commit;
}
- return commit;
} while (revs->commits);
return NULL;
}
diff --git a/revision.h b/revision.h
index d8a5a50..2232247 100644
--- a/revision.h
+++ b/revision.h
@@ -133,4 +133,12 @@ extern void add_object(struct object *obj,
extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);
+enum commit_action {
+ commit_ignore,
+ commit_show,
+ commit_error
+};
+
+extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit);
+
#endif
^ permalink raw reply related
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