Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Junio C Hamano @ 2006-03-31  8:28 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0603301652531.27203@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> This is an absolutely huge deal for anything like "git log -- <pathname>", 
> but also for some things that we don't do yet - like the "find where 
> things changed" logic I've described elsewhere, where we want to find the 
> previous revision that changed a file.
>...
> Btw, don't even bother testing this with the git archive. git itself is so 
> small that parsing the whole revision history for it takes about a second 
> even with path limiting.

By the way, I forgot to praise you ;-).  

Even on a fast machine, the old one was not very useful, but
this one is _instantaneous_.  Very good job.

$ PAGER=cat GIT_DIR=/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/ \
  /usr/bin/time git log -1 --pretty=short -- drivers/
commit ce362c009250340358a7221f3cdb7954cbf19c01
Merge: 064c94f... cd7a920...
Author: Linus Torvalds <torvalds@g5.osdl.org>

    Merge git://git.kernel.org/pub/scm/linux/kernel/git/kyle/parisc-2.6

15.44user 0.19system 0:25.11elapsed 62%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+18050minor)pagefaults 0swaps

$ PAGER=cat GIT_DIR=/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/ \
  /usr/bin/time ./git.pu log -1 --pretty=short -- drivers/
commit ce362c009250340358a7221f3cdb7954cbf19c01
Merge: 064c94f... cd7a920...
Author: Linus Torvalds <torvalds@g5.osdl.org>

    Merge git://git.kernel.org/pub/scm/linux/kernel/git/kyle/parisc-2.6

0.00user 0.00system 0:00.00elapsed 50%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+388minor)pagefaults 0swaps

^ permalink raw reply

* Re: Possible --boundary bug
From: Junio C Hamano @ 2006-03-31  7:58 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git, Paul Mackerras
In-Reply-To: <e5bfff550603301255j52c68963v4b8eebea697eeecf@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> Sorry, the good description is below, please ignore the wrong previous one.

I think this patch should fix it.

-- >8 --
rev-list --boundary: fix re-injecting boundary commits.

Marco reported that

	$ git rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d

misses these two boundary commits.

        c649657501bada28794a30102d9c13cc28ca0e5e
        eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

Indeed, we can see that gitk shows these two commits at the
bottom, because the --boundary code failed to output them.

The code did not check to avoid pushing the same uninteresting
commit twice to the result list.  I am not sure why this fixes
the reported problem, but this seems to fix it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/revision.c b/revision.c
index abc8745..c2a95aa 100644
--- a/revision.c
+++ b/revision.c
@@ -420,24 +420,33 @@ static void limit_list(struct rev_info *
 		p = &commit_list_insert(commit, p)->next;
 	}
 	if (revs->boundary) {
-		list = newlist;
-		while (list) {
+		/* mark the ones that are on the result list first */
+		for (list = newlist; list; list = list->next) {
 			struct commit *commit = list->item;
+			commit->object.flags |= TMP_MARK;
+		}
+		for (list = newlist; list; list = list->next) {
+			struct commit *commit = list->item;
 			struct object *obj = &commit->object;
-			struct commit_list *parent = commit->parents;
-			if (obj->flags & (UNINTERESTING|BOUNDARY)) {
-				list = list->next;
-				continue;
-			}
-			while (parent) {
+			struct commit_list *parent;
+			if (obj->flags & UNINTERESTING)
+				continue;
+			for (parent = commit->parents;
+			     parent;
+			     parent = parent->next) {
 				struct commit *pcommit = parent->item;
-				parent = parent->next;
 				if (!(pcommit->object.flags & UNINTERESTING))
 					continue;
 				pcommit->object.flags |= BOUNDARY;
+				if (pcommit->object.flags & TMP_MARK)
+					continue;
+				pcommit->object.flags |= TMP_MARK;
 				p = &commit_list_insert(pcommit, p)->next;
 			}
-			list = list->next;
+		}
+		for (list = newlist; list; list = list->next) {
+			struct commit *commit = list->item;
+			commit->object.flags &= ~TMP_MARK;
 		}
 	}
 	revs->commits = newlist;

^ permalink raw reply related

* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Junio C Hamano @ 2006-03-31  7:02 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0603302225160.27203@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> So the following is wrong:
>
>> diff --git a/revision.c b/revision.c
>> index 0e3f074..a55c0d1 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -404,10 +404,6 @@ static void limit_list(struct rev_info *
>>  		list = list->next;
>>  		free(entry);
>>  
>> -		if (revs->max_age != -1 && (commit->date < revs->max_age))
>> -			obj->flags |= UNINTERESTING;
>> -		if (revs->unpacked && has_sha1_pack(obj->sha1))
>> -			obj->flags |= UNINTERESTING;
>>  		add_parents_to_list(revs, commit, &list);
>>  		if (obj->flags & UNINTERESTING) {
>>  			mark_parents_uninteresting(commit);
>...
> ..but the later part of the patch looks fine (modulo testing, of course):
>
>> @@ -786,7 +773,9 @@ struct commit *get_revision(struct rev_i
>>  		if (revs->min_age != -1 && (commit->date > revs->min_age))
>>  			continue;
>>  		if (revs->max_age != -1 && (commit->date < revs->max_age))
>> -			return NULL;
>> +			continue;
>> +		if (revs->unpacked && has_sha1_pack(commit->object.sha1))
>> +			continue;

OK, so let's say I agree with you that --unpacked and --since
are "stop early" rules.  I fully agree with that from usability
and implementation point of view, but I now wonder if the
"output filter" in get_revision() and "stop early" in limit_list()
would do the same thing.  That is, if we are _otherwise_
limited, limit_list() would use them for "stop early" rule, but
if we end up not calling limit_list() we would simply filter
them and keep going (which is what my patch did for both
timestamp and packedness), or we could also stop there.

I am not sure which one is correct, but I suspect whichever we
do in get_revision() we would get inconsistent results between a
case where we call limit_list() and we do not.  That is, the set
of commits we are going to show when --(topo|date)-order is
given and not given can be different.  Is this acceptable?

I would say, from the implementation point of view, it should be
tolerated, but...

^ permalink raw reply

* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Junio C Hamano @ 2006-03-31  6:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603302153350.27203@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Sadly, it seems to react really badly to Junio's new --boundary logic for 
> some reason that I haven't quite figured out yet.

There already was a report that --boundary stuff is not quite
right, so what you are seeing might be that the new code exposes
its original breakage even more.  I haven't looked into the
breakage of the original version yet either, so I cannot really
say how your change breaks it.

> That reaction is independent of the actual pathname restriction, and seems 
> to be related to how the --boundary logic expected 
> pop_most_recent_commit() to work. In particular:
>
> 	...
>                         if (commit->object.flags & BOUNDARY) {
>                                 /* this is already uninteresting,
>                                  * so there is no point popping its
>                                  * parents into the list.
>                                  */
>
> that code is magic, and seems to depend on something subtle going on with 
> the list, and the incremental thing already popped the parent earlier and 
> screwed up whatever magic that the BOUNDARY code depends on.

This was not so magic, but the magic was actually in the code
added to limit_list().  Usually, "newlist" consists interesting
commits, and what are found interesting initially but marked as
uninteresting when a different ancestry chain coming from an
uninteresting head leading to it was later discovered.  The
magic code looks at still-interesting commits, and re-injects
its parents that are uninteresting to the list (and I just
spotted a bug there -- it does not check if what is being "re-"
injected are already on the list -- should I check for SEEN flag
there perhaps?), while marking them as boundary.  This was done
to make sure that all the open-circle (in gitk) commits are on
the resulting list.

The part of the code you quoted is just a short-cut for not
doing pop_most_recent_commit() -- we used to have
pop_most_recent_commit() there, which pushed the parents of the
commit being processed into the list.  Because we are processing
a boundary commit, we know it is uninteresting -- and by
definition, its parents are uninteresting and that is why it
just advances the list without calling pop_most_recent_commit(),
bypassing its side effect to push its parents into the list.

Since the new code has already advanced the list immediately
after the beginning of do {} block, I think you can remove the
entire "if (revs->max_count) {}" block.  As the code currently
stands, you are skipping what happens to be next to the boundary
commit on the result list.

> Junio? I think you did some funky special case with BOUNDARY commits, and 
> I broke it for you, can you look at the patch and see if you can see it? 
> I'd really like to have the incremental path-limiter, because it really 
> makes a huge difference in the usability of "git log pathname".

Oh, there is no question about making it streamable in more
cases is a good thing.

^ permalink raw reply

* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Linus Torvalds @ 2006-03-31  6:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr74jxhp3.fsf@assigned-by-dhcp.cox.net>



On Thu, 30 Mar 2006, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > The reason I put "RFC" in the subject line is that while I've validated it 
> > various ways, like doing
> >
> > 	git-rev-list HEAD -- drivers/char/ | md5sum 
> >
> > before-and-after on the kernel archive, it's "git-rev-list" after all. In 
> > other words, it's that really really subtle and complex central piece of 
> > software. So while I think this is important and should go in asap, I also 
> > think it should get lots of testing and eyeballs looking at the code.
> 
> Let me make sure I understand what is going on.
> 
> Currently, limit_list(), which is called when revs->limited is
> set, serves two different purposes:
> 
>  * traverse the ancestry chain and mark ancestors of
>    uninteresting commits also uninteresting;

Right.

We _also_ traverse the ancestry chain in the "walking" stage later in 
get_revision(), but if we have a "limit" case, we'll pre-traverse 
everything early in limit_list.

>  * "simplify" each commit that is still interesting, by
>    comparing the tree object of it and its parents.

Through "add_parent_to_list()", yes. 

> We used to call limit_list() when we are limiting the commits by
> paths, because that was what the latter does as a side effect of
> add_parents_to_list().  You made it streamable by moving the
> call to get_revision() and not calling limit_list() when you do
> not have other reasons to call it.

Yes, and by using "add_parent_to_list()" instead of the 
non-pathname-limit-aware "pop_most_recent_commit()".

So we don't call limit-list any more, because we do the same thing in 
get_revision() that it used to do at limit time.

> You currently do not do this streaming optimization when you
> have to show simplified parents, because the streaming version
> traverses ancestry chain one step at a time as it goes, and you
> cannot obviously see who the final "fake" parent is until you
> simplify the parents enouogh.

Yes. For exactly the same reason some other things ause us to do 
limit_list(): some ops simply need the fully connected end result in order 
to be able to work correctly.

>  * get_commit_reference() sets "limited" when the user gives ^exclude
>    commit.  no question about this --- we would need to see the
>    reachability in this case.

Right.

>  * when we are going to sort the result we are going to show,
>    we set "limited" -- we cannot sort without knowing the set we
>    are sorting.

Right.

>  * giving commit timestamp limits via --max-age, --min-age
>    etc. sets "limited".  I have doubts about this.

I agree. I looked at it when I did the "rev-list.c" vs "revision.c" split, 
but I wanted to do as few changes as possible, and for some reason we've 
always considered "time" to cause us to limit things.

However, I think your patch is wrong. Even if you don't limit things when 
we have a date specifier, you still want to stop doing traversal in 
limit_list when you hit that date. Why? Because it might be limited for 
some _other_ reason, eg due to --topo-order or or some other issue.

>  * As to handling of --unpacked, exactly the same comment as
>    "max-age" applies, but it might be even worse.  Marking an
>    unpacked one "uninteresting" means if a packed commit refers
>    to loose commit, the loose one will be also marked
>    uninteresting, I think.
> 
> "--objects --unpacked" is an idiom to repack objects that are
> still loose, but I suspect it would do interesting thing if the
> commit is in a pack but its trees and blobs are still loose.  Am
> I confused, or have I just spotted a potential (but hard to
> trigger) bug?

Regardless of where you do the "unpacked" test, it will always really end 
up stopping when it hits a packed commit. So you won't ever get away from 
that.

So same logic applies. Once you hit a packed commit, you should stop, both 
in limit_list _and_ in get_revision(). Otherwise you'll do too much work 
when trying to limit things, for no gain.

The fact is, "--unpacked" means that we traverse the part of the chain 
that hasn't been packed yet. Anything that is packed automatically cuts 
off parsing, whether there is anything else unpacked below it or not. It's 
not a bug, it's a feature, and if you want to pack everything, you should 
just do

	git repack -a -d

and not use --unpacked.

Same goes for dates, btw. We've always stopped when we reached a certain 
date, even though there _could_ be commits before it that are from a more 
recent date (due to clocks being set wrong). Neither --unpacked nor 
--since=xyzzy are meant to be any kind of "guarantees" that you get all 
commits that match some pattern. They are just "stop early" rules.

		Linus

So the following is wrong:

> diff --git a/revision.c b/revision.c
> index 0e3f074..a55c0d1 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -404,10 +404,6 @@ static void limit_list(struct rev_info *
>  		list = list->next;
>  		free(entry);
>  
> -		if (revs->max_age != -1 && (commit->date < revs->max_age))
> -			obj->flags |= UNINTERESTING;
> -		if (revs->unpacked && has_sha1_pack(obj->sha1))
> -			obj->flags |= UNINTERESTING;
>  		add_parents_to_list(revs, commit, &list);
>  		if (obj->flags & UNINTERESTING) {
>  			mark_parents_uninteresting(commit);
> @@ -415,8 +411,6 @@ static void limit_list(struct rev_info *
>  				break;
>  			continue;
>  		}
> -		if (revs->min_age != -1 && (commit->date > revs->min_age))
> -			continue;
>  		p = &commit_list_insert(commit, p)->next;
>  	}
>  	if (revs->boundary) {

..but the later part of the patch looks fine (modulo testing, of course):

> @@ -543,32 +537,26 @@ int setup_revisions(int argc, const char
>  			}
>  			if (!strncmp(arg, "--max-age=", 10)) {
>  				revs->max_age = atoi(arg + 10);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strncmp(arg, "--min-age=", 10)) {
>  				revs->min_age = atoi(arg + 10);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strncmp(arg, "--since=", 8)) {
>  				revs->max_age = approxidate(arg + 8);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strncmp(arg, "--after=", 8)) {
>  				revs->max_age = approxidate(arg + 8);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strncmp(arg, "--before=", 9)) {
>  				revs->min_age = approxidate(arg + 9);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strncmp(arg, "--until=", 8)) {
>  				revs->min_age = approxidate(arg + 8);
> -				revs->limited = 1;
>  				continue;
>  			}
>  			if (!strcmp(arg, "--all")) {
> @@ -635,7 +623,6 @@ int setup_revisions(int argc, const char
>  			}
>  			if (!strcmp(arg, "--unpacked")) {
>  				revs->unpacked = 1;
> -				revs->limited = 1;
>  				continue;
>  			}
>  			*unrecognized++ = arg;
> @@ -786,7 +773,9 @@ struct commit *get_revision(struct rev_i
>  		if (revs->min_age != -1 && (commit->date > revs->min_age))
>  			continue;
>  		if (revs->max_age != -1 && (commit->date < revs->max_age))
> -			return NULL;
> +			continue;
> +		if (revs->unpacked && has_sha1_pack(commit->object.sha1))
> +			continue;
>  		if (revs->no_merges &&
>  		    commit->parents && commit->parents->next)
>  			continue;
> 

^ permalink raw reply

* Re: Gitk strangeness..
From: Alex Riesen @ 2006-03-31  6:27 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <17452.28122.129442.49226@cargo.ozlabs.ibm.com>

On 3/31/06, Paul Mackerras <paulus@samba.org> wrote:
>
> > The old gitk produced a denser graph, which wasn't perfect too, but
> > had a higher count of visible commit titles (and this is two-monitor
> > setup, too).
>
> I just pushed a new version which does better on this.
>

This one looks reallly much better! Thanks!

^ permalink raw reply

* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Junio C Hamano @ 2006-03-31  6:16 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0603301652531.27203@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> The reason I put "RFC" in the subject line is that while I've validated it 
> various ways, like doing
>
> 	git-rev-list HEAD -- drivers/char/ | md5sum 
>
> before-and-after on the kernel archive, it's "git-rev-list" after all. In 
> other words, it's that really really subtle and complex central piece of 
> software. So while I think this is important and should go in asap, I also 
> think it should get lots of testing and eyeballs looking at the code.

Let me make sure I understand what is going on.

Currently, limit_list(), which is called when revs->limited is
set, serves two different purposes:

 * traverse the ancestry chain and mark ancestors of
   uninteresting commits also uninteresting;

 * "simplify" each commit that is still interesting, by
   comparing the tree object of it and its parents.

We used to call limit_list() when we are limiting the commits by
paths, because that was what the latter does as a side effect of
add_parents_to_list().  You made it streamable by moving the
call to get_revision() and not calling limit_list() when you do
not have other reasons to call it.

You currently do not do this streaming optimization when you
have to show simplified parents, because the streaming version
traverses ancestry chain one step at a time as it goes, and you
cannot obviously see who the final "fake" parent is until you
simplify the parents enouogh.

Now, I have some observations, not necessarily limited to this
patch but also to the code from the "master" version.

 * get_commit_reference() sets "limited" when the user gives ^exclude
   commit.  no question about this --- we would need to see the
   reachability in this case.

 * when we are going to sort the result we are going to show,
   we set "limited" -- we cannot sort without knowing the set we
   are sorting.

 * giving commit timestamp limits via --max-age, --min-age
   etc. sets "limited".  I have doubts about this.  We currently
   look at the commit timestamp in limit_list() and mark a
   commit old enough to be "uninteresting" -- which makes
   ancestor of such commit also uninteresting.  Similarly
   commits that are too young are not pushed into the result.

   There is a filter in get_revision() to omit ineligible
   commits by time range already, so I wonder if we can remove
   that code from limit_list() and not set "limited" when these
   timestamp ranges are given.  This would let us stream even
   more.

   Admittably, ancestors of an old commit had better be even
   older, so marking them uninteresting to stop the traversal
   early is a good way to optimize the full-traversal case, but
   not having to call limit_list() would help streaming usage.

   Also I have doubts about correctness issue about the max-age
   handling.  Is it correct to mark a commit that is old enough
   to be uninteresting, implying that its ancestors are all
   uninteresting?  With clock skew among people, it is possible
   to make a merge commit that has parents one of which is "in
   the future".

 * As to handling of --unpacked, exactly the same comment as
   "max-age" applies, but it might be even worse.  Marking an
   unpacked one "uninteresting" means if a packed commit refers
   to loose commit, the loose one will be also marked
   uninteresting, I think.

"--objects --unpacked" is an idiom to repack objects that are
still loose, but I suspect it would do interesting thing if the
commit is in a pack but its trees and blobs are still loose.  Am
I confused, or have I just spotted a potential (but hard to
trigger) bug?


--
diff --git a/revision.c b/revision.c
index 0e3f074..a55c0d1 100644
--- a/revision.c
+++ b/revision.c
@@ -404,10 +404,6 @@ static void limit_list(struct rev_info *
 		list = list->next;
 		free(entry);
 
-		if (revs->max_age != -1 && (commit->date < revs->max_age))
-			obj->flags |= UNINTERESTING;
-		if (revs->unpacked && has_sha1_pack(obj->sha1))
-			obj->flags |= UNINTERESTING;
 		add_parents_to_list(revs, commit, &list);
 		if (obj->flags & UNINTERESTING) {
 			mark_parents_uninteresting(commit);
@@ -415,8 +411,6 @@ static void limit_list(struct rev_info *
 				break;
 			continue;
 		}
-		if (revs->min_age != -1 && (commit->date > revs->min_age))
-			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
 	if (revs->boundary) {
@@ -543,32 +537,26 @@ int setup_revisions(int argc, const char
 			}
 			if (!strncmp(arg, "--max-age=", 10)) {
 				revs->max_age = atoi(arg + 10);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strncmp(arg, "--min-age=", 10)) {
 				revs->min_age = atoi(arg + 10);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strncmp(arg, "--since=", 8)) {
 				revs->max_age = approxidate(arg + 8);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strncmp(arg, "--after=", 8)) {
 				revs->max_age = approxidate(arg + 8);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strncmp(arg, "--before=", 9)) {
 				revs->min_age = approxidate(arg + 9);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strncmp(arg, "--until=", 8)) {
 				revs->min_age = approxidate(arg + 8);
-				revs->limited = 1;
 				continue;
 			}
 			if (!strcmp(arg, "--all")) {
@@ -635,7 +623,6 @@ int setup_revisions(int argc, const char
 			}
 			if (!strcmp(arg, "--unpacked")) {
 				revs->unpacked = 1;
-				revs->limited = 1;
 				continue;
 			}
 			*unrecognized++ = arg;
@@ -786,7 +773,9 @@ struct commit *get_revision(struct rev_i
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
 			continue;
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
-			return NULL;
+			continue;
+		if (revs->unpacked && has_sha1_pack(commit->object.sha1))
+			continue;
 		if (revs->no_merges &&
 		    commit->parents && commit->parents->next)
 			continue;

^ permalink raw reply related

* Re: [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Linus Torvalds @ 2006-03-31  6:05 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603301652531.27203@g5.osdl.org>



On Thu, 30 Mar 2006, Linus Torvalds wrote:
> 
> This makes git-rev-list able to do path-limiting without having to parse
> all of history before it starts showing the results.
> 
> This makes it things like "git log -- pathname" much more pleasant to use.

Sadly, it seems to react really badly to Junio's new --boundary logic for 
some reason that I haven't quite figured out yet.

That reaction is independent of the actual pathname restriction, and seems 
to be related to how the --boundary logic expected 
pop_most_recent_commit() to work. In particular:

	...
                        if (commit->object.flags & BOUNDARY) {
                                /* this is already uninteresting,
                                 * so there is no point popping its
                                 * parents into the list.
                                 */

that code is magic, and seems to depend on something subtle going on with 
the list, and the incremental thing already popped the parent earlier and 
screwed up whatever magic that the BOUNDARY code depends on.

Junio? I think you did some funky special case with BOUNDARY commits, and 
I broke it for you, can you look at the patch and see if you can see it? 
I'd really like to have the incremental path-limiter, because it really 
makes a huge difference in the usability of "git log pathname".

		Linus

^ permalink raw reply

* Re: Gitk strangeness..
From: Junio C Hamano @ 2006-03-31  1:50 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Alex Riesen, git, Linus Torvalds
In-Reply-To: <17452.28122.129442.49226@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> Alex Riesen writes:
>
>> The old gitk produced a denser graph, which wasn't perfect too, but
>> had a higher count of visible commit titles (and this is two-monitor
>> setup, too).
>
> I just pushed a new version which does better on this.

Thanks.  Pulled, merged and pushed out..

^ permalink raw reply

* Re: [PATCH] cvsimport: use git-update-ref when updating
From: Junio C Hamano @ 2006-03-31  1:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0603301405160.18852@wbgn013.biozentrum.uni-wuerzburg.de>

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

> This simplifies code, and also fixes a subtle bug: when importing in a
> shared repository, where another user last imported from CVS, cvsimport
> used to complain that it could not open <branch> for update.

The second hunk look sensible but I do not know about "use Fcntl"
since I do not see anything you are adding that starts to use it...

^ permalink raw reply

* [PATCH/RFC 2/2] Make path-limiting be incremental when possible.
From: Linus Torvalds @ 2006-03-31  1:05 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603301648530.27203@g5.osdl.org>


This makes git-rev-list able to do path-limiting without having to parse
all of history before it starts showing the results.

This makes it things like "git log -- pathname" much more pleasant to use.

This is actually a pretty small patch, and the biggest part of it is 
purely cleanups (turning the "goto next" statements into "continue"), but 
it's conceptually a lot bigger than it looks.

What it does is that if you do a path-limited revision list, and you do 
_not_ ask for pseudo-parenthood information, it won't do all the 
path-limiting up-front, but instead do it incrementally in 
"get_revision()".

This is an absolutely huge deal for anything like "git log -- <pathname>", 
but also for some things that we don't do yet - like the "find where 
things changed" logic I've described elsewhere, where we want to find the 
previous revision that changed a file.

The reason I put "RFC" in the subject line is that while I've validated it 
various ways, like doing

	git-rev-list HEAD -- drivers/char/ | md5sum 

before-and-after on the kernel archive, it's "git-rev-list" after all. In 
other words, it's that really really subtle and complex central piece of 
software. So while I think this is important and should go in asap, I also 
think it should get lots of testing and eyeballs looking at the code.

Btw, don't even bother testing this with the git archive. git itself is so 
small that parsing the whole revision history for it takes about a second 
even with path limiting. The thing that _really_ shows this off is doing

	git log drivers/

on the kernel archive, or even better, on the _historic_ kernel archive.

With this change, the response is instantaneous (although seeking to the 
end of the result will obviously take as long as it ever did). Before this 
change, the command would think about the result for tens of seconds - or 
even minutes, in the case of the bigger old kernel archive - before 
starting to output the results.

NOTE NOTE NOTE! Using path limiting with things like "gitk", which uses 
the "--parents" flag to actually generate a pseudo-history of the 
resulting commits won't actually see the improvement in interactivity, 
since that forces git-rev-list to do the whole-history thing after all. 

MAYBE we can fix that too at some point, but I won't promise anything.

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


diff --git a/revision.c b/revision.c
index 0330f9f..0e3f074 100644
--- a/revision.c
+++ b/revision.c
@@ -702,7 +702,13 @@ int setup_revisions(int argc, const char
 	if (revs->prune_data) {
 		diff_tree_setup_paths(revs->prune_data);
 		revs->prune_fn = try_to_simplify_commit;
-		revs->limited = 1;
+
+		/*
+		 * If we fix up parent data, we currently cannot
+		 * do that on-the-fly.
+		 */
+		if (revs->parents)
+			revs->limited = 1;
 	}
 
 	return left;
@@ -763,23 +769,32 @@ struct commit *get_revision(struct rev_i
 
 	do {
 		struct commit *commit = revs->commits->item;
+
+		revs->commits = revs->commits->next;
 
+		/*
+		 * If we haven't done the list limiting, we need to look at
+		 * the parents here
+		 */
+		if (!revs->limited)
+			add_parents_to_list(revs, commit, &revs->commits);
 		if (commit->object.flags & SHOWN)
-			goto next;
+			continue;
 		if (!(commit->object.flags & BOUNDARY) &&
 		    (commit->object.flags & UNINTERESTING))
-			goto next;
+			continue;
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
-			goto next;
+			continue;
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			return NULL;
 		if (revs->no_merges &&
 		    commit->parents && commit->parents->next)
-			goto next;
+			continue;
 		if (revs->prune_fn && revs->dense) {
 			if (!(commit->object.flags & TREECHANGE))
-				goto next;
-			rewrite_parents(commit);
+				continue;
+			if (revs->parents)
+				rewrite_parents(commit);
 		}
 		/* More to go? */
 		if (revs->max_count) {
@@ -792,13 +807,9 @@ struct commit *get_revision(struct rev_i
 				revs->commits = it->next;
 				free(it);
 			}
-			else
-				pop_most_recent_commit(&revs->commits, SEEN);
 		}
 		commit->object.flags |= SHOWN;
 		return commit;
-next:
-		pop_most_recent_commit(&revs->commits, SEEN);
 	} while (revs->commits);
 	return NULL;
 }

^ permalink raw reply related

* [PATCH/RFC 1/2] Move "--parent" parsing into generic revision.c library code
From: Linus Torvalds @ 2006-03-31  0:52 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Move "--parent" parsing into generic revision.c library code

Not only do we do it in both rev-list.c and git.c, the revision walking
code will soon want to know whether we should rewrite parenthood
information or not.

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

This is the trivial part. The next email will contain the real meat of the 
change, which starts doing path limiting incrementally, and makes doing 
things like

	git log drivers/

on the kernel a hell of a lot more pleasant, because it starts outputting 
the log immediately instead of pausing for about 20 seconds before it has 
parsed all of the history and then outputting it all in one go.

There's a HUGE difference to the "feel" of doing pathname limiting 
git-rev-list options. Not very well tested, but at least this initial 
preparatory patch is "obviously safe".

diff --git a/git.c b/git.c
index 0b40e30..72039c6 100644
--- a/git.c
+++ b/git.c
@@ -283,7 +283,6 @@ static int cmd_log(int argc, const char 
 	char *buf = xmalloc(LOGSIZE);
 	static enum cmit_fmt commit_format = CMIT_FMT_DEFAULT;
 	int abbrev = DEFAULT_ABBREV;
-	int show_parents = 0;
 	const char *commit_prefix = "commit ";
 
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
@@ -294,9 +293,6 @@ static int cmd_log(int argc, const char 
 			if (commit_format == CMIT_FMT_ONELINE)
 				commit_prefix = "";
 		}
-		else if (!strcmp(arg, "--parents")) {
-			show_parents = 1;
-		}
 		else if (!strcmp(arg, "--no-abbrev")) {
 			abbrev = 0;
 		}
@@ -317,7 +313,7 @@ static int cmd_log(int argc, const char 
 	while ((commit = get_revision(&rev)) != NULL) {
 		printf("%s%s", commit_prefix,
 		       sha1_to_hex(commit->object.sha1));
-		if (show_parents) {
+		if (rev.parents) {
 			struct commit_list *parents = commit->parents;
 			while (parents) {
 				struct object *o = &(parents->item->object);
diff --git a/rev-list.c b/rev-list.c
index f3a989c..19a547a 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -39,7 +39,6 @@ struct rev_info revs;
 static int bisect_list = 0;
 static int verbose_header = 0;
 static int abbrev = DEFAULT_ABBREV;
-static int show_parents = 0;
 static int show_timestamp = 0;
 static int hdr_termination = 0;
 static const char *commit_prefix = "";
@@ -54,7 +53,7 @@ static void show_commit(struct commit *c
 	if (commit->object.flags & BOUNDARY)
 		putchar('-');
 	fputs(sha1_to_hex(commit->object.sha1), stdout);
-	if (show_parents) {
+	if (revs.parents) {
 		struct commit_list *parents = commit->parents;
 		while (parents) {
 			struct object *o = &(parents->item->object);
@@ -336,10 +335,6 @@ int main(int argc, const char **argv)
 				commit_prefix = "";
 			else
 				commit_prefix = "commit ";
-			continue;
-		}
-		if (!strcmp(arg, "--parents")) {
-			show_parents = 1;
 			continue;
 		}
 		if (!strcmp(arg, "--timestamp")) {
diff --git a/revision.c b/revision.c
index abc8745..0330f9f 100644
--- a/revision.c
+++ b/revision.c
@@ -596,6 +596,10 @@ int setup_revisions(int argc, const char
 				revs->limited = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--parents")) {
+				revs->parents = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--dense")) {
 				revs->dense = 1;
 				continue;
diff --git a/revision.h b/revision.h
index 61e6bc9..0caeecf 100644
--- a/revision.h
+++ b/revision.h
@@ -34,7 +34,8 @@ struct rev_info {
 			edge_hint:1,
 			limited:1,
 			unpacked:1,
-			boundary:1;
+			boundary:1,
+			parents:1;
 
 	/* special limits */
 	int max_count;

^ permalink raw reply related

* [PATCH] cg-log: Remove unpleasant DEL characters.
From: Mark Wooding @ 2006-03-31  0:08 UTC (permalink / raw)
  To: git

From: Mark Wooding <mdw@distorted.org.uk>

There's a Bash bug (I'm running 3.1.5(1)-release from Debian testing) as
follows:

  $ foo=@; echo "<${foo:1}>" | cat -v
  <^?>

Without this fix, less gives me ugly standout `^?' markers for every
blank line in a commit message.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---

 cg-log |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cg-log b/cg-log
index 5d5407b..b3374e4 100755
--- a/cg-log
+++ b/cg-log
@@ -181,7 +181,7 @@ process_commit_line()
 {
 	if [ "$key" = "%" ] || [ "$key" = "%$colsignoff" ]; then
 		# The fast common case
-		[ "$state" = silent ] || msg="$msg    ${rest:1}
+		[ "$state" = silent ] || msg="$msg    ${rest#?}
 "
 		return
 	fi

^ permalink raw reply related

* Re: Gitk strangeness..
From: Paul Mackerras @ 2006-03-30 23:46 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <20060330205759.GA27131@steel.home>

Alex Riesen writes:

> The old gitk produced a denser graph, which wasn't perfect too, but
> had a higher count of visible commit titles (and this is two-monitor
> setup, too).

I just pushed a new version which does better on this.

Paul.

^ permalink raw reply

* Re: [PATCH] gitk: Use git wrapper to run git-ls-remote.
From: Johannes Schindelin @ 2006-03-30 23:20 UTC (permalink / raw)
  To: Mark Wooding; +Cc: git
In-Reply-To: <slrne2o8lr.l0.mdw@metalzone.distorted.org.uk>

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

Hi,

On Thu, 30 Mar 2006, Mark Wooding wrote:

> Junio C Hamano <junkio@cox.net> wrote:
> 
> > Does anybody know what is going on?
> 
> I'll try staring at the Tcl source code some time.  I'm rather too busy
> tonight, though.
> 
> There's also some very strange geometry management oddness going on in
> gitk.  I'll try to sort that out too.

That has been discussed. My feeling is that this is a bug of Tk with 
regard to rootless X servers. I never came around to do a proper patch, 
but I have explicit -height and -width arguments to all frames and 
panedwindows.

If you want to start working on it, I attached my current patch, which is 
good enough for me, but note that it changes the geometry subtly everytime 
gitk is called...

Hth,
Dscho


[-- Attachment #2: Type: TEXT/plain, Size: 1642 bytes --]

[PATCH] gitk: make geometry less weird on cygwin and macosx

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 gitk |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gitk b/gitk
index 03cd475..7339069 100755
--- a/gitk
+++ b/gitk
@@ -373,13 +373,13 @@ proc makewindow {rargs} {
 	set geometry(ctexth) [expr {($texth - 8) /
 				    [font metrics $textfont -linespace]}]
     }
-    frame .ctop.top
+    frame .ctop.top -height $geometry(canvh)
     frame .ctop.top.bar
     pack .ctop.top.bar -side bottom -fill x
     set cscroll .ctop.top.csb
     scrollbar $cscroll -command {allcanvs yview} -highlightthickness 0
     pack $cscroll -side right -fill y
-    panedwindow .ctop.top.clist -orient horizontal -sashpad 0 -handlesize 4
+    panedwindow .ctop.top.clist -orient horizontal -sashpad 0 -handlesize 4 -height $geometry(canvh)
     pack .ctop.top.clist -side top -fill both -expand 1
     .ctop add .ctop.top
     set canv .ctop.top.clist.canv
@@ -449,9 +449,10 @@ proc makewindow {rargs} {
     # for making sure type==Exact whenever loc==Pickaxe
     trace add variable findloc write findlocchange
 
-    panedwindow .ctop.cdet -orient horizontal
+    panedwindow .ctop.cdet -orient horizontal \
+	-height [expr $geometry(ctexth)*$linespc+4]
     .ctop add .ctop.cdet
-    frame .ctop.cdet.left
+    frame .ctop.cdet.left -width [expr $geometry(ctextw)*[font measure $textfont "0"]+8]
     set ctext .ctop.cdet.left.ctext
     text $ctext -bg white -state disabled -font $textfont \
 	-width $geometry(ctextw) -height $geometry(ctexth) \

^ permalink raw reply related

* Re: Gitk strangeness..
From: Paul Mackerras @ 2006-03-30 22:33 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <20060330205759.GA27131@steel.home>

Alex Riesen writes:

> Well... Could you take a look at these screenshots, please?
> http://home.arcor.de/fork0/bug/gitk1.jpg (this one is BIG, 400k, 2456x949)
> http://home.arcor.de/fork0/bug/gitk2.jpg
> http://home.arcor.de/fork0/bug/gitk3.jpg

Yes, I was just looking last night at the part of the git.git graph
that you have there in gitk1.jpg.  That's an artifact of some changes
I made to make sure there was a vertical line segment just before an
arrow.  The reason for doing that is that Tk 8.4 seems to just punt on
drawing an arrow on the end of a diagonal line segment.  The old gitk
just removed trailing diagonal segments of the line, but I thought I
could do better than that.

I'll try another approach.

Paul.

^ permalink raw reply

* Re: How to switch kernel customizations from 2.6.15.6 to 2.6.16?
From: Matt McCutchen @ 2006-03-30 21:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0603300919280.27203@g5.osdl.org>

On Thu, 2006-03-30 at 09:32 -0800, Linus Torvalds wrote:
> The beauty of git should be (and maybe that's not entirely true simply 
> because of practical concerns) that there really need not be any notion of 
> "more official".

I understand this, and it is one of several reasons why I prefer git to
other version control systems.  However, I thought there would be a
single official kernel repository even if git didn't require it.  Junio
explained to me that both yours and the stable one are official for
different purposes.  I think I will use the stable one because it is
current enough for my needs.

>  - the more fundamental one is that when you start mixing branches, you 
>    have to be very careful if you expect the upstream projects to pull the 
>    changes _back_. [...]

True.  It might help several branches coordinate development if a commit
could be marked as "equivalent" to another commit so that, if both were
involved in a merge, one could be thrown out.

-- 
Matt McCutchen
hashproduct@verizon.net
http://hashproduct.metaesthetics.net/

^ permalink raw reply

* Re: Gitk strangeness..
From: Alex Riesen @ 2006-03-30 20:57 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <17449.48630.370867.10251@cargo.ozlabs.ibm.com>

Paul Mackerras, Wed, Mar 29, 2006 00:51:34 +0200:
> Junio C Hamano writes:
> 
> > How about this alternative patch, then?  It turned out to be
> > quite convoluted as I feared.
> 
> That's brilliant.  Thank you!  With the patch to gitk below, the
> graph display on Linus' example looks much saner.
> 

Well... Could you take a look at these screenshots, please?
http://home.arcor.de/fork0/bug/gitk1.jpg (this one is BIG, 400k, 2456x949)
http://home.arcor.de/fork0/bug/gitk2.jpg
http://home.arcor.de/fork0/bug/gitk3.jpg

The compressed repository is being uploaded there:

http://home.arcor.de/fork0/bug/ggg.tar.bz2 (~6Mb)

The old gitk produced a denser graph, which wasn't perfect too, but
had a higher count of visible commit titles (and this is two-monitor
setup, too).

^ permalink raw reply

* Re: Possible --boundary bug
From: Marco Costalba @ 2006-03-30 20:55 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <e5bfff550603301034r58b38500ie5897ed06fce6e9a@mail.gmail.com>

Sorry, the good description is below, please ignore the wrong previous one.

On 3/30/06, Marco Costalba <mcostalba@gmail.com> wrote:
> Trying to convert qgit to use the new and cool --boundary option I
> found this one:
>
> From git tree
>
> $ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
> grep eb38cc689e8
> -e646de0d14bac20ef6e156c1742b9e62fb0b9020
> eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
> 4b953cdc04fec8783e2c654a671005492fda9b01
> 5ca5396c9ecba947c8faac7673195d309a6ba2ea
> eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
>
> and also
>
> $ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
> grep c64965750
> 8c0db2f5193153ea8a51bb45b0512c5a3889023b
> 21a02335f821c89a989cf0b533d2ae0adb6da16e
> c649657501bada28794a30102d9c13cc28ca0e5e
>

It seems the lines:
-c649657501bada28794a30102d9c13cc28ca0e5e  .......

and

-eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4 ......

are missing though the two revs are boundary revs.


Marco

 P.S: Sorry for lengthy output but  --abbrev option:

 git-rev-list --boundary --topo-order --abbrev=8 --parents  5aa44d5..ab57c8d

 does seems to work only for prettyprinted parent names, I guess this
from patches log messages because is not documented.

^ permalink raw reply

* Re: [PATCH] gitk: Use git wrapper to run git-ls-remote.
From: Christopher Faylor @ 2006-03-30 20:13 UTC (permalink / raw)
  To: Junio C Hamano, Mark Wooding, git
In-Reply-To: <7v8xqr3iwt.fsf@assigned-by-dhcp.cox.net>

On Thu, Mar 30, 2006 at 10:08:02AM -0800, Junio C Hamano wrote:
>Mark Wooding <mdw@distorted.org.uk> writes:
>
>> From: Mark Wooding <mdw@distorted.org.uk>
>>
>> For some reason, the Cygwin Tcl's `exec' command has trouble running
>> scripts...
>
>Yup, I've seen this and have a "personal edition" workaround
>exactly like yours.  I haven't bothered to put it in even "pu",
>because I am reluctant to add an workaround to a problem I do
>not understand (and I haven't bothered to try understanding the
>problem which happens only on Windows ;-).
>
>Does anybody know what is going on?

Currently, Cygwin's tcl is a pure windows version which uses
CreateProcess to run stuff.  It doesn't know about scripts and possibly
doesn't even know about cygwin paths.

cgf

^ permalink raw reply

* Possible --boundary bug
From: Marco Costalba @ 2006-03-30 18:34 UTC (permalink / raw)
  To: junkio; +Cc: git

Trying to convert qgit to use the new and cool --boundary option I
found this one:

>From git tree

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep eb38cc689e8
-e646de0d14bac20ef6e156c1742b9e62fb0b9020
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
4b953cdc04fec8783e2c654a671005492fda9b01
5ca5396c9ecba947c8faac7673195d309a6ba2ea
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

and also

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep c64965750
8c0db2f5193153ea8a51bb45b0512c5a3889023b
21a02335f821c89a989cf0b533d2ae0adb6da16e
c649657501bada28794a30102d9c13cc28ca0e5e

But perhaps correct output should be:

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep eb38cc689e8
-e646de0d14bac20ef6e156c1742b9e62fb0b9020
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
-4b953cdc04fec8783e2c654a671005492fda9b01
5ca5396c9ecba947c8faac7673195d309a6ba2ea
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

and

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep c64965750
-8c0db2f5193153ea8a51bb45b0512c5a3889023b
21a02335f821c89a989cf0b533d2ae0adb6da16e
c649657501bada28794a30102d9c13cc28ca0e5e

It seems the '-' flag is mistakenly missing because of boundary  revs:

c649657501bada28794a30102d9c13cc28ca0e5e and
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4


Marco

^ permalink raw reply

* Re: [PATCH] gitk: Use git wrapper to run git-ls-remote.
From: Mark Wooding @ 2006-03-30 18:26 UTC (permalink / raw)
  To: git
In-Reply-To: <7v8xqr3iwt.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:

> Does anybody know what is going on?

I'll try staring at the Tcl source code some time.  I'm rather too busy
tonight, though.

There's also some very strange geometry management oddness going on in
gitk.  I'll try to sort that out too.

-- [mdw]

^ permalink raw reply

* Re: [PATCH] gitk: Use git wrapper to run git-ls-remote.
From: Junio C Hamano @ 2006-03-30 18:08 UTC (permalink / raw)
  To: Mark Wooding; +Cc: git
In-Reply-To: <20060330123151.25779.73775.stgit@metalzone.distorted.org.uk>

Mark Wooding <mdw@distorted.org.uk> writes:

> From: Mark Wooding <mdw@distorted.org.uk>
>
> For some reason, the Cygwin Tcl's `exec' command has trouble running
> scripts...

Yup, I've seen this and have a "personal edition" workaround
exactly like yours.  I haven't bothered to put it in even "pu",
because I am reluctant to add an workaround to a problem I do
not understand (and I haven't bothered to try understanding the
problem which happens only on Windows ;-).

Does anybody know what is going on?

^ permalink raw reply

* Re: How to switch kernel customizations from 2.6.15.6 to 2.6.16?
From: Linus Torvalds @ 2006-03-30 17:32 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: git
In-Reply-To: <1143687710.2524.1.camel@mattlaptop.metaesthetics.net>



On Wed, 29 Mar 2006, Matt McCutchen wrote:
> 
> Perhaps this is just politics, but which kernel repository is more
> official, and why?  Linus's or the one I have been using,
> 	git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git
> ?

The beauty of git should be (and maybe that's not entirely true simply 
because of practical concerns) that there really need not be any notion of 
"more official".

You can fetch multiple different git trees from different sources into the 
same git tree, and then keep them _all_ around equally as different 
branches. You can move fixes between the different branches with "git 
cherry-pick", and you can merge different branches with "git merge"

Now, the reason I say "_should_ be" rather than "is" is two-fold:

 - right now, a lot of the infrastructure is simply set up more towards 
   the "one single source repository" model. When you do a "git clone", it 
   kind of makes the origin special. That's how all the documentation is 
   written, and that's also the only remote branch that git creates 
   _automatically_ for you.

   This really isn't a technical issue: the git code code doesn't care 
   about any special "original" repository. But the fact that you have to 
   create the ".git/remotes/linus" file by hand, and that all the examples 
   in the docs end up talking about a single "origin" branch means that 
   people _think_ of git as a "single origin" thing.

 - the more fundamental one is that when you start mixing branches, you 
   have to be very careful if you expect the upstream projects to pull the 
   changes _back_. In particular, that's where you have to think twice (or 
   three times) about doing a "git merge" (or a "git pull", which 
   implicitly merges for you if it's not a pure fast-forward).

   In particular, the fact that _you_ want to merge two trees that came 
   from different sources does _not_ imply that either of the two sources 
   might want to merge with each other. So if you merge the two together, 
   you may find it impossible to have either of them then pull from you: 
   they way want your changes, but they might not like the merge you did, 
   because they have different policies about that work than you did.

So while the first point is purely a "mental model" issue and about lack 
of helper scripts, the second point is fundamental.

For example, in your case it was almost certainly the right thing to do to 
cherry-pick your changes from the 2.6.15.6 branch onto the development 
branch, because I simply don't want to merge the 2.6.15.6 stuff into the 
standard tree: part of the _rules_ for the stable branch is that the 
things it fixes should have been fixed in the development tree already, so 
merging the stable tree should always be unnecessary (and often clash, 
although _hopefully_ in many cases the fixes in the stable tree are 1:1 
identical and will merge beautifully).

Anyway: from a technical standpoint, no tree should be "special" or "more 
official" for git usage. But when merging data back to any of those trees 
that aren't special, the source/history of the data is important to keep 
in mind. Branch "a" may not be any more special than branch "b", but when 
you push changes back to the source of branch "a", the history of those 
changes (relative to what the source was) is meaningful.

			Linus

^ permalink raw reply

* [PATCH] git-clone: exit early if repo isn't specified
From: Yasushi SHOJI @ 2006-03-30 17:01 UTC (permalink / raw)
  To: git

Subject: [PATCH] git-clone: exit early if repo isn't specified

git-clone without a repo isn't useful at all.  print message and get
out asap.

This patch also move the variable 'local' to where other variables are
initialized.

Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>


---

 git-clone.sh |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

a222289a855194280aade24baa005de9e55667d0
diff --git a/git-clone.sh b/git-clone.sh
index a731d15..7b05fd9 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -98,6 +98,7 @@ close FH;
 '
 
 quiet=
+local=no
 use_local=no
 local_shared=no
 no_checkout=
@@ -155,6 +156,13 @@ do
 	shift
 done
 
+repo="$1"
+if test -z "$repo"
+then
+    echo >&2 'you must specify a repository to clone.'
+    exit 1
+fi
+
 # --bare implies --no-checkout
 if test yes = "$bare"
 then
@@ -178,8 +186,6 @@ fi
 
 # Turn the source into an absolute path if
 # it is local
-repo="$1"
-local=no
 if base=$(get_repo_base "$repo"); then
 	repo="$base"
 	local=yes
-- 
1.3.0.rc1.gb8abc

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox