* Re: the war on trailing whitespace
From: Andreas Ericsson @ 2006-02-27 14:10 UTC (permalink / raw)
To: Uwe Zeisberger; +Cc: git
In-Reply-To: <20060227133124.GA8794@informatik.uni-freiburg.de>
Uwe Zeisberger wrote:
> Hello,
>
> Andreas Ericsson wrote:
>
>>I think the question is whether completely empty lines are also ignored
>>by Python, or if they start a new block of code. Whatever the case, it
>>must hold true for both 2.3 and 2.4.
>
> see
> http://www.python.org/doc/2.2.3/ref/blank-lines.html
> http://www.python.org/doc/2.3.5/ref/blank-lines.html
> http://www.python.org/doc/2.4.2/ref/blank-lines.html
>
So in essence, a multi-line statement is closed when a completely empty
line is found, which means that making git internals recognize and strip
such lines will result in Python code never being manageable by git.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: the war on trailing whitespace
From: Uwe Zeisberger @ 2006-02-27 13:31 UTC (permalink / raw)
To: git
In-Reply-To: <4402E56D.4010606@op5.se>
Hello,
Andreas Ericsson wrote:
> I think the question is whether completely empty lines are also ignored
> by Python, or if they start a new block of code. Whatever the case, it
> must hold true for both 2.3 and 2.4.
see
http://www.python.org/doc/2.2.3/ref/blank-lines.html
http://www.python.org/doc/2.3.5/ref/blank-lines.html
http://www.python.org/doc/2.4.2/ref/blank-lines.html
Best regards
Uwe
--
Uwe Zeisberger
http://www.google.com/search?q=gravity+on+earth%3D
^ permalink raw reply
* [PATCH] git-format-patch: Always add a blank line between headers and body.
From: Alexandre Julliard @ 2006-02-27 13:09 UTC (permalink / raw)
To: git
If the second line of the commit message isn't empty, git-format-patch
needs to add an empty line in order to generate a properly formatted
mail. Otherwise git-rebase drops the rest of the commit message.
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
git-format-patch.sh | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
dcd0ed6ced98990c92a32416c0acc3ec298f5b91
diff --git a/git-format-patch.sh b/git-format-patch.sh
index eb75de4..2bd2639 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -174,7 +174,7 @@ titleScript='
process_one () {
perl -w -e '
my ($keep_subject, $num, $signoff, $commsg) = @ARGV;
-my ($signoff_pattern, $done_header, $done_subject, $signoff_seen,
+my ($signoff_pattern, $done_header, $done_subject, $done_separator, $signoff_seen,
$last_was_signoff);
if ($signoff) {
@@ -228,6 +228,11 @@ while (<FH>) {
$done_subject = 1;
next;
}
+ unless ($done_separator) {
+ print "\n";
+ $done_separator = 1;
+ next if (/^$/);
+ }
$last_was_signoff = 0;
if (/Signed-off-by:/i) {
--
1.2.3.gb348-dirty
--
Alexandre Julliard
julliard@winehq.org
^ permalink raw reply related
* Re: [PATCH] Handle branch names with slashes
From: Karl Hasselström @ 2006-02-27 13:09 UTC (permalink / raw)
To: catalin.marinas; +Cc: Sam Vilain, git
In-Reply-To: <1141043391.3438.66.camel@pc1117>
On 2006-02-27 12:29:51 +0000, Catalin Marinas wrote:
> On Mon, 2006-02-27 at 13:11 +0100, Karl Hasselström wrote:
>
> > There was a bug here after all: I just tried "stg pick
> > multi@kha/patches" (to pick a patch named "multi" from the branch
> > "kha/patches"), and StGIT tried to pick the patch from branch
> > "kha".
>
> I haven't applied your patch yet (too busy to properly review it).
And as I just demonstrated, it certainly needed reviewing! (Actually,
I believe I said that back when I posted the patch, too.)
> > Looking closer, I realized that the complete patch specification
> > syntax is "patchname@branchname/bottom", not
> > "patchname/bottom@branchname" as I had assumed. This is obviously
> > hard to reconcile with branch names containing /.
>
> I don't have any strong opinion on either. Maybe we should use the
> latter if it makes things easier for supporting branch names with
> /'s.
The problem is that the current from is better (bottom is a modifier
to patch@branch, not just patch). And using the other form will break
when someone decides that patches with slashes in their names are a
good idea (not a joke).
Perhaps change /bottom to #bottom (making the complete form
patchname@branchname#bottom), and for backward compatibility accept
patchname@branchname/bottom as well when no branch called
"branchname/bottom" exists.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: Teach the "git" command to handle some commands internally
From: Michal Ostrowski @ 2006-02-27 12:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Git Mailing List, Andreas Ericsson, Alex Riesen
In-Reply-To: <7vy7zx65v0.fsf@assigned-by-dhcp.cox.net>
On Sun, 2006-02-26 at 15:46 -0800, Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
>
> > On Sun, 26 Feb 2006, Junio C Hamano wrote:
> >>
> >> > There's one other change: the search order for external programs is
> >> > modified slightly, so that the first entry remains GIT_EXEC_DIR, but the
> >> > second entry is the same directory as the git wrapper itself was executed
> >> > out of - if we can figure it out from argv[0], of course.
> >>
> >> I am not sure about this part, though.
> >
> > Well, what it means is that _if_ you install all your "git" binaries in
> > some directory that is not in your patch and is not GIT_EXEC_DIR, they
> > will still magically work, assuming you don't do something strange.
>
> I understood that part. I was wondering if this change defeats
> what Michal (you sensibly CC'ed your message to) wanted to do
> earlier, going great length trying to avoid mucking with PATH
> and "where-ever git itself is found" in the last round. After
> reviewing the change in 77cb17 commit, I realize my worry was
> unfounded.
The changes seem reasonable for now. We can't avoid mucking with PATH
as long as we are going to be running shell scripts that depend on PATH
to invoke "git-xxx" or even "git xxx". I don't seen any easy solution
to this that would not involve changing every script (albeit
mechanically) and would not be prone to lapses in discipline.
Any solution to this original problem (i.e. a special "PATH" for "git*")
would seem to be applicable to the behavior this patch introduces.
--
Michal Ostrowski <mostrows@watson.ibm.com>
^ permalink raw reply
* [PATCH 2/2] combine-diff: Honour -z option correctly.
From: Mark Wooding @ 2006-02-27 12:52 UTC (permalink / raw)
To: git
In-Reply-To: <20060227124815.25144.83101.stgit@metalzone.distorted.org.uk>
From: Mark Wooding <mdw@distorted.org.uk>
Combined diffs don't null terminate things in the same way as standard
diffs. This is presumably wrong.
Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---
combine-diff.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 984103e..a23894d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -726,7 +726,7 @@ static int show_patch_diff(struct combin
if (header) {
shown_header++;
- puts(header);
+ printf("%s%c", header, opt->line_termination);
}
printf("diff --%s ", dense ? "cc" : "combined");
if (quote_c_style(elem->path, NULL, NULL, 0))
@@ -799,7 +799,7 @@ static void show_raw_diff(struct combine
inter_name_termination = 0;
if (header)
- puts(header);
+ printf("%s%c", header, line_termination);
for (i = 0; i < num_parent; i++) {
if (p->parent[i].mode)
^ permalink raw reply related
* [PATCH 1/2] combine-diff: Honour --full-index.
From: Mark Wooding @ 2006-02-27 12:52 UTC (permalink / raw)
To: git
In-Reply-To: <20060227124815.25144.83101.stgit@metalzone.distorted.org.uk>
From: Mark Wooding <mdw@distorted.org.uk>
For some reason, combined diffs don't honour the --full-index flag when
emitting patches. Fix this.
Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---
combine-diff.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index d812600..984103e 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -621,7 +621,8 @@ static void reuse_combine_diff(struct sl
}
static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
- int dense, const char *header)
+ int dense, const char *header,
+ struct diff_options *opt)
{
unsigned long size, cnt, lno;
char *result, *cp, *ep;
@@ -631,6 +632,7 @@ static int show_patch_diff(struct combin
char ourtmp_buf[TMPPATHLEN];
char *ourtmp = ourtmp_buf;
int working_tree_file = !memcmp(elem->sha1, null_sha1, 20);
+ int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
/* Read the result of merge first */
if (!working_tree_file) {
@@ -735,10 +737,10 @@ static int show_patch_diff(struct combin
printf("index ");
for (i = 0; i < num_parent; i++) {
abb = find_unique_abbrev(elem->parent[i].sha1,
- DEFAULT_ABBREV);
+ abbrev);
printf("%s%s", i ? "," : "", abb);
}
- abb = find_unique_abbrev(elem->sha1, DEFAULT_ABBREV);
+ abb = find_unique_abbrev(elem->sha1, abbrev);
printf("..%s\n", abb);
if (mode_differs) {
@@ -862,7 +864,7 @@ int show_combined_diff(struct combine_di
default:
case DIFF_FORMAT_PATCH:
- return show_patch_diff(p, num_parent, dense, header);
+ return show_patch_diff(p, num_parent, dense, header, opt);
}
}
^ permalink raw reply related
* [PATCH 0/2] combine-diff consistency fixes
From: Mark Wooding @ 2006-02-27 12:48 UTC (permalink / raw)
To: git
The output of git diff-tree --cc on a merge is not consistent with its
output for a normal commit. In particular:
* the index lines on a combined diff are abbreviated even if
--full-index is given, and
* the headers on a combined diff are not null terminated, even if -z
is given.
For example, run
git-diff-tree --cc -z --full-index f0b0af1b04f558b684cae2a3b805ca4bab84d45f
-- [mdw]
^ permalink raw reply
* Re: [PATCH] Handle branch names with slashes
From: Catalin Marinas @ 2006-02-27 12:29 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Sam Vilain, git
In-Reply-To: <20060227121108.GA22398@diana.vm.bytemark.co.uk>
On Mon, 2006-02-27 at 13:11 +0100, Karl Hasselström wrote:
> There was a bug here after all: I just tried "stg pick
> multi@kha/patches" (to pick a patch named "multi" from the branch
> "kha/patches"), and StGIT tried to pick the patch from branch "kha".
I haven't applied your patch yet (too busy to properly review it).
> Looking closer, I realized that the complete patch specification
> syntax is "patchname@branchname/bottom", not
> "patchname/bottom@branchname" as I had assumed. This is obviously hard
> to reconcile with branch names containing /.
I don't have any strong opinion on either. Maybe we should use the
latter if it makes things easier for supporting branch names with /'s.
--
Catalin
^ permalink raw reply
* Re: [PATCH] Handle branch names with slashes
From: Karl Hasselström @ 2006-02-27 12:11 UTC (permalink / raw)
To: Sam Vilain; +Cc: Catalin Marinas, git
In-Reply-To: <20060217042108.GB28114@diana.vm.bytemark.co.uk>
On 2006-02-17 05:21:08 +0100, Karl Hasselström wrote:
> On 2006-02-17 16:01:10 +1300, Sam Vilain wrote:
>
> > Karl Hasselström wrote:
> >
> > > Let StGIT grok branch names with slashes in them. It used to
> > > fall flat on its face when confronted with them.
> > >
> > > I think I've covered all, or at least most cases, but there are
> > > probably some bugs left if you look hard enough.
> >
> > Does `stgit -r patchname/bottom` still work?
>
> Yes (if you mean 'stg diff -r ... ' :-). It's just branches that can
> have slashes in their names, not patches.
There was a bug here after all: I just tried "stg pick
multi@kha/patches" (to pick a patch named "multi" from the branch
"kha/patches"), and StGIT tried to pick the patch from branch "kha".
Looking closer, I realized that the complete patch specification
syntax is "patchname@branchname/bottom", not
"patchname/bottom@branchname" as I had assumed. This is obviously hard
to reconcile with branch names containing /.
Thoughts?
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: the war on trailing whitespace
From: Johannes Schindelin @ 2006-02-27 11:55 UTC (permalink / raw)
To: Adrien Beau; +Cc: git
In-Reply-To: <94fc236b0602270326s3079d737l102d5728d59f0c98@mail.gmail.com>
On Mon, 27 Feb 2006, Adrien Beau wrote:
> A logical line that contains only spaces and tabs is ignored by
> Python. (All the "dirty" lines in git-merge-recursive.py are such
> lines.)
>
> Hope this helps,
It does.
Thanks,
Dscho
^ permalink raw reply
* Re: the war on trailing whitespace
From: Andreas Ericsson @ 2006-02-27 11:41 UTC (permalink / raw)
To: Adrien Beau
Cc: Johannes Schindelin, MIke Galbraith, Dave Jones, Linus Torvalds,
Andrew Morton, junkio, git
In-Reply-To: <94fc236b0602270326s3079d737l102d5728d59f0c98@mail.gmail.com>
Adrien Beau wrote:
> On 2/27/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
>>there is a good reason not to enable the no-whitespace-at-eol checking in
>>pre-commit by default (at least for *all* files) for git development:
>>
>> Python.
>>
>>Just do a "/ $" in git-merge-recursive.py. These whitespaces are not an
>>error, but a syntactic *requirement*.
>
>
> No, they aren't.
>
> A logical line that contains only spaces and tabs is ignored by
> Python. (All the "dirty" lines in git-merge-recursive.py are such
> lines.)
>
I think the question is whether completely empty lines are also ignored
by Python, or if they start a new block of code. Whatever the case, it
must hold true for both 2.3 and 2.4.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: the war on trailing whitespace
From: Adrien Beau @ 2006-02-27 11:26 UTC (permalink / raw)
To: Johannes Schindelin
Cc: MIke Galbraith, Dave Jones, Linus Torvalds, Andrew Morton, junkio,
git
In-Reply-To: <Pine.LNX.4.63.0602271004130.5937@wbgn013.biozentrum.uni-wuerzburg.de>
On 2/27/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> there is a good reason not to enable the no-whitespace-at-eol checking in
> pre-commit by default (at least for *all* files) for git development:
>
> Python.
>
> Just do a "/ $" in git-merge-recursive.py. These whitespaces are not an
> error, but a syntactic *requirement*.
No, they aren't.
A logical line that contains only spaces and tabs is ignored by
Python. (All the "dirty" lines in git-merge-recursive.py are such
lines.)
Besides, spaces, tabs and newlines can be used interchangeably to
separate tokens, so trailing whitespace is never *required*.
Hope this helps,
Adrien
^ permalink raw reply
* Re: the war on trailing whitespace
From: Andrew Morton @ 2006-02-27 9:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: efault, davej, torvalds, junkio, git
In-Reply-To: <Pine.LNX.4.63.0602271004130.5937@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Hi,
>
> there is a good reason not to enable the no-whitespace-at-eol checking in
> pre-commit by default (at least for *all* files) for git development:
>
> Python.
That's not a good reason. People will discover that git has started
shouting at them and they'll work out how to make it stop.
The probem is getting C users to turn the check on, not in getting python
users to turn it off.
^ permalink raw reply
* Re: NT directory traversal speed on 25K files on Cygwin
From: Andreas Ericsson @ 2006-02-27 9:19 UTC (permalink / raw)
To: git; +Cc: Christopher Faylor, git
In-Reply-To: <20060226231701.GA11961@nospam.com>
Rutger Nijlunsing wrote:
> On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
>
>>On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
>>
>>>filesystem is slow and locked down, and exec-attribute is NOT really
>>>useful even on NTFS (it is somehow related to execute permission and
>>>open files. I still cannot figure out how exactly are they related).
>>
>>Again, it's not clear if you're talking about Windows or Cygwin but
>>under Cygwin, in the default configuration, the exec attribute means the
>>same thing to cygwin as it does to linux.
>
>
> I don't know about native Windows speed, but comparing NutCracker with
> Cygwin on a simple 'find . | wc -l' already gives a clue that looking
> at Cygwin to benchmark NT file inspection IO will give a skewed
> picture:
>
Well, naturally. Cygwin is a userland implementation of a sane
filesystem on top of a less sane one. File IO is bound to be slower when
one FS is emulated on top of another. I think cygwin users are aware of
this and simply accept the speed-for-sanity tradeoff. I know I would.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: the war on trailing whitespace
From: Johannes Schindelin @ 2006-02-27 9:07 UTC (permalink / raw)
To: MIke Galbraith; +Cc: Dave Jones, Linus Torvalds, Andrew Morton, junkio, git
In-Reply-To: <1141008633.7593.13.camel@homer>
Hi,
there is a good reason not to enable the no-whitespace-at-eol checking in
pre-commit by default (at least for *all* files) for git development:
Python.
Just do a "/ $" in git-merge-recursive.py. These whitespaces are not an
error, but a syntactic *requirement*.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH] First cut at libifying revlist generation
From: Johannes Schindelin @ 2006-02-27 9:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vpsl93395.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 26 Feb 2006, Junio C Hamano wrote:
> Johannes gets a test-pilot star for this. This also means we need a bit
> better set of tests.
Well, I don't deserve this. I cheated.
In my personal version of git, there are a few subtle things different
than in the official version. Most of them, I sent out already, and they
were rejected, such as
http://article.gmane.org/gmane.comp.version-control.git/10718, which
helped me tremendously in identyfing the bug.
But there is also a test case in my version, which was a failure,
originally. I wrote it to demonstrate that the stupid version of
git-fetch was stupid. It did not demonstrate that, but rather quite a bit
of (my) normal usage. That is the reason it takes a really long time for a
test case, and that in turn is the reason I did not dare to submit it.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a Documentation/git-tools.txt
From: Alexandre Julliard @ 2006-02-27 8:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marco Costalba, git
In-Reply-To: <7vslq57mzn.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Also is the emacs one really pcl-cvs? I thought it was modeled
> after pcl-cvs, but this is a different implementation to deal
> with git. If Alexandre does not have a name for it, I'd say
> we'll list it as "git.el".
It doesn't really have a specific name, so yes, "git.el" is fine.
--
Alexandre Julliard
julliard@winehq.org
^ permalink raw reply
* Re: [PATCH] Add a Documentation/git-tools.txt
From: Marco Costalba @ 2006-02-27 6:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alexandre Julliard
In-Reply-To: <7vslq57mzn.fsf@assigned-by-dhcp.cox.net>
On 2/26/06, Junio C Hamano <junkio@cox.net> wrote:
> >
> > Please consider for apply.
>
> Thanks. I've considered it, but it is seriously linewrapped.
>
Sorry for this. It is ok 80 columns max?
> > + - *pcl-cvs* (contrib/)
> > +
> > + This is an Emacs interface for git. The user interface is
> > modeled on
> > + pcl-cvs.
>
> Also is the emacs one really pcl-cvs? I thought it was modeled
> after pcl-cvs, but this is a different implementation to deal
> with git. If Alexandre does not have a name for it, I'd say
> we'll list it as "git.el".
>
Ok, waiting for Alexandre's answer.
This week I foreseen to be quite busy, so perhaps the patch will be
delayed a little bit.
Marco
^ permalink raw reply
* Re: [PATCH 4/4] Read author names and emails from a file
From: Junio C Hamano @ 2006-02-27 5:51 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: git, Karl Hasselström
In-Reply-To: <440195D4.7080905@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Karl Hasselström wrote:
>> Read a file with lines on the form
>> username User's Full Name <email@addres.org>
>> and use "User's Full Name <email@addres.org>" as the GIT author and
>> committer for Subversion commits made by "username". If encountering a
>> commit made by a user not in the list, abort.
>
> This is a good thing, but wouldn't it be better to use the same format
> as that of cvsimport's -A flag?
If both CVS and SVN have their own native format to express
things like this, and if the format they use are different, then
that is a valid reason for git-{cvs,svn}import to use different
file format.
But if that is not the case, I tend to agree that it might be
easier for users if we had just one format. I do not think,
however, any single project is likely to have to deal with both
CVS and SVN upstream, importing into the same git repository, so
reusing the mapping file would not be an issue, but having to
learn how to write the mapping just once is a good thing.
I do not offhand recall if SVN has its own native format; if it
has, it may be better to use that, instead of matching what
git-cvsimport does, since I do not think of a reason why the
version with an equal sign is preferrable over the version with
a space. If the version with '=' were the CVS native format
then that might be a reason to prefer it, but if I recall
correctly that is not the case. so...
^ permalink raw reply
* Re: [PATCH] First cut at libifying revlist generation
From: Linus Torvalds @ 2006-02-27 5:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vy7zx1j6u.fsf@assigned-by-dhcp.cox.net>
On Sun, 26 Feb 2006, Junio C Hamano wrote:
>
> Not really, because the second invocation of add_one_commit()
> says "I've seen that *commit*", which is correct. And the story
> is obviously the same if you used longhand "^v1.0.0^0 v1.0.0".
Ok.
I suspect that the simplest fix is to just move "limited" into the "revs"
structure, the way I did pretty much everything else. That way nothing
really changes: we'll have the exact same logic, the flag just moved
around.
Linus
^ permalink raw reply
* Re: [PATCH] First cut at libifying revlist generation
From: Junio C Hamano @ 2006-02-27 5:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0602261914270.22647@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sun, 26 Feb 2006, Junio C Hamano wrote:
>>
>> I am clueless about the "limited = (list && list->next)" part,
>> but there is only one commit involved hence the test is false
>> with my testcase "git-rev-list --objects v1.0.0^0..v1.0.0"; I
>> think the old code said dotdot is a limited case.
>
> dotdot should insert _two_ commits onto the list - the positive and
> the negative one. Doesn't it?
Not really, because the second invocation of add_one_commit()
says "I've seen that *commit*", which is correct. And the story
is obviously the same if you used longhand "^v1.0.0^0 v1.0.0".
As a symbolic notation v1.0.0^0..v1.0.0 may not make much sense,
but the point is "the other end says he has that commit object,
but now he wants the tag we later attached to that commit
object; let's list the objects we need to send him". This is
what upload-pack does.
A bad consequence of not limiting is that:
git-rev-list ^v1.0.0^0 v1.0.0 | tail -n 1
gives this commit ;-):
e83c5163316f89bfbde7d9ab23ca2e25604af290
Argh.
^ permalink raw reply
* Re: [PATCH] First cut at libifying revlist generation
From: Linus Torvalds @ 2006-02-27 3:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vpsl93395.fsf@assigned-by-dhcp.cox.net>
On Sun, 26 Feb 2006, Junio C Hamano wrote:
>
> I am clueless about the "limited = (list && list->next)" part,
> but there is only one commit involved hence the test is false
> with my testcase "git-rev-list --objects v1.0.0^0..v1.0.0"; I
> think the old code said dotdot is a limited case.
dotdot should insert _two_ commits onto the list - the positive and
the negative one. Doesn't it?
So the
if (list && list->next)
check should be correct. If we have just one entry, there's no reason to
do everything up-front, we can just run with it (and get the nice
streaming behaviour).
> -static struct object_list *pending_objects = NULL;
> -
> - for (pending = pending_objects; pending; pending = pending->next) {
> + for (pending = revs.pending_objects; pending; pending = pending->next) {
But this part is obviously correct. I already sent out the same patch a
minute ago ;)
> - if (revs.max_age || revs.min_age)
> + if (revs.max_age != -1 || revs.min_age != -1)
As is this. I for a while had zero meaning "no age", and I actually think
it probably should be that way, but then we'd have to switch the
date-related functions around, which is why I decided not to do it after
all (but missed this one that I had already written for that case).
Linus
^ permalink raw reply
* Re: [PATCH] First cut at libifying revlist generation
From: Junio C Hamano @ 2006-02-27 3:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0602251608160.22647@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> Comments? I think this is safe to apply, because I've done a diff of the
> old non-split rev-list.c against both the new rev-list.c and revision.c,
> and all the changes _look_ like just moving things around and taking the
> new "struct rev_info" into account.
I think you would need something the attached patch, at least.
I have a suspicion that this was your easter egg to see how
careful I am reading what I apply -- and I failed. Johannes
gets a test-pilot star for this. This also means we need a bit
better set of tests.
The diff between old rev-list.c and new split files would not
have caught the static variable gotcha.
I am clueless about the "limited = (list && list->next)" part,
but there is only one commit involved hence the test is false
with my testcase "git-rev-list --objects v1.0.0^0..v1.0.0"; I
think the old code said dotdot is a limited case.
---
diff --git a/rev-list.c b/rev-list.c
index d1c52a6..630626e 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -214,8 +214,6 @@ static struct object_list **process_tree
return p;
}
-static struct object_list *pending_objects = NULL;
-
static void show_commit_list(struct commit_list *list)
{
struct object_list *objects = NULL, **p = &objects, *pending;
@@ -226,7 +224,7 @@ static void show_commit_list(struct comm
if (process_commit(commit) == STOP)
break;
}
- for (pending = pending_objects; pending; pending = pending->next) {
+ for (pending = revs.pending_objects; pending; pending = pending->next) {
struct object *obj = pending->item;
const char *name = pending->name;
if (obj->flags & (UNINTERESTING | SEEN))
@@ -675,7 +673,7 @@ int main(int argc, const char **argv)
}
list = revs.commits;
- if (list && list->next)
+ if (list)
limited = 1;
if (revs.topo_order)
@@ -689,7 +687,7 @@ int main(int argc, const char **argv)
limited = 1;
diff_tree_setup_paths(revs.paths);
}
- if (revs.max_age || revs.min_age)
+ if (revs.max_age != -1 || revs.min_age != -1)
limited = 1;
save_commit_buffer = verbose_header;
^ permalink raw reply related
* Re: [PATCH] First cut at libifying revlist generation
From: Linus Torvalds @ 2006-02-27 3:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0602270257110.4147@wbgn013.biozentrum.uni-wuerzburg.de>
On Mon, 27 Feb 2006, Johannes Schindelin wrote:
>
> beware of that patch. It breaks at least one thing: cloning a repository
> with a tag pointing to a tag object (the tag is cloned, but not the tag
> object).
>
> Sorry to not fix it right away, but I am just too tired.
Ahh. I know what it is.
I'm pretty sure this trivial and stupid patch on top of the patch I sent
out should fix it.
Duh. I moved "pending_objects" into the "revs" structure, but didn't
remove the stale one, along with its use.
Linus
---
diff --git a/rev-list.c b/rev-list.c
index d1c52a6..e9e371c 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -214,8 +214,6 @@ static struct object_list **process_tree
return p;
}
-static struct object_list *pending_objects = NULL;
-
static void show_commit_list(struct commit_list *list)
{
struct object_list *objects = NULL, **p = &objects, *pending;
@@ -226,7 +224,7 @@ static void show_commit_list(struct comm
if (process_commit(commit) == STOP)
break;
}
- for (pending = pending_objects; pending; pending = pending->next) {
+ for (pending = revs.pending_objects; pending; pending = pending->next) {
struct object *obj = pending->item;
const char *name = pending->name;
if (obj->flags & (UNINTERESTING | SEEN))
^ 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