Git development
 help / color / mirror / Atom feed
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01  3:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606302046230.12404@g5.osdl.org>



On Fri, 30 Jun 2006, Linus Torvalds wrote:
>
> (it's not strictly a valid set operation, but it approaches being an 
> "xor" instead of a union or an intersection or a difference).

Oh, I guess it _is_ perfectly valid. It's called a "symmetric difference" 
in set theory.

So from a set standpoint:

	Git op:			Set theory:

	git-rev-list a..b	// difference: B - A
	git-rev-list b..a	// difference: A - B

	git-rev-list a b	// union of A B (order doesn't matter)

	git-rev-list a...b	// symmetric difference A B (order doesn't matter)

	git-rev-list $(git-merge-base --all a b)
				// intersection of A and B

I think.

		Linus

^ permalink raw reply

* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01  3:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7vedntn.fsf@assigned-by-dhcp.cox.net>



On Fri, 30 Jun 2006, Junio C Hamano wrote:
> 
> Heh, that's why I kept saying I want somebody to teach rev-list
> a new notation, A...B, to mean $(merge-base A B)..B ;-).

I actually don't think that expression makes any sense.

	$(merge-base A B)..B

as an expression only makes sense if there is a single point of forking, 
and no contact apart from that. In that case, what you suggest makes 
sense, because doing

	git diff A...B

is exactly what you want. 

HOWEVER. If there has been any other merges in between (but they aren't 
merge-bases because either branch _also_ did other things), your A...B
expression is meaningless, I think. To do a diff in that case, you really 
need to do my "merge+diff" thing, and no amount of "A...B" expressions on 
a commit relationship level can be meaningful.

Now, the expression

	A...B == B...A == A B --not $(git-merge-base --all A B)

is meaningful (and the one I want for merges), but it's largely useless 
for anything else. It just means "the set of all commits that aren't 
trivially in both" (it's not strictly a valid set operation, but it
approaches being an "xor" instead of a union or an intersection or a 
difference).

But the above isn't useful for "git diff" and friends any more, it's 
mainly just for merging.

			Linus

^ permalink raw reply

* Re: [PATCH] Enable tree (directory) history display
From: Linus Torvalds @ 2006-07-01  3:45 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606301954140.12404@g5.osdl.org>



On Fri, 30 Jun 2006, Linus Torvalds wrote:
> 
> BTW! Junio, I think this patch actually fixes a real bug.
> 
> Without this patch, the "--parents --full-history" combination (which 
> you'd get if you do something like
> 
> 	gitk --full-history Makefile
> 
> or similar) will actually _drop_ merges where all children are identical. 
> That's wrong in the --full-history case, because it measn that the graph 
> ends up missing lots of entries.

Here's some real numbers.

Before this patch

	git-rev-list --full-history HEAD -- Makefile | wc -l
	git-rev-list --parents --full-history HEAD -- Makefile | wc -l

both returned 971 commits on my current kernel tree, while

	git-rev-list --parents HEAD -- Makefile | wc -l

returned 145 commits, and

	git-rev-list --parents --no-merges HEAD -- Makefile | wc -l

returns 136.

That count of 145 is the number of commits that actually _change_ Makefile 
some way - and some of them really are merges, because they have a content 
merge, and the merge result is thus different from any of the children. So 
that's a real number. So is 136, in some sense - it just says that we 
don't care about commits, even if those commits _do_ end up changing the 
file.

But the important part to realize is that the "971" number is always 
wrong. It's never a really valid number. It contains a lot of extra 
merges, but it does _not_ contain enough of them to connect all the dots, 
and it's thus never correct. Either you should drop merges that don't 
change things (in which case you cannot have full connectivity, and 
"--parents" doesn't make sense), or you should keep them all (or at least 
enough to get full connectivity).

Now, AFTER this patch

	git-rev-list --full-history HEAD -- Makefile | wc -l

returns 145 commits (the same 145 commits that really change Makefile, but 
we've now properly decided that because we don't have "--parents" and 
don't need to keep connectivity we drop _all_ of the merges that have a 
child that is identical), while

	git-rev-list --parents --full-history HEAD -- Makefile | wc -l

returns 2323 commits, and now really has _all_ the merges (because it 
needs to include every single merge in the tree - otherwise the 
connectivity doesn't make sense).

Now, that 2323 is a bit unnecessary - we end up having merges to merges 
that don't actually have any changes at all in between, and it might be 
nice to simplify the merge history to create a minimal tree that still has 
all potential changes in it, but that's a much harder problem.

Anyway, the patch definitely makes a difference, and I think it's correct. 
The effects might be a bit easier to visualize on a smaller tree than the 
kernel ;)

We could still potentially improve on the "--parents --full-history" case, 
but --full-history currently means always walking all possible chains, and 
that will be shown in the output (ie we will have all possible paths in 
the result if "--parents" is used, even if those paths end up being 
totally uninteresting)

  "Hey - you asked for full history, you got it. Don't blame me if you got 
   a lot of totally uninteresting crud"

				Linus

^ permalink raw reply

* Re: [PATCH] Enable tree (directory) history display
From: Linus Torvalds @ 2006-07-01  3:21 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Junio C Hamano, git
In-Reply-To: <20060701024309.63001.qmail@web31805.mail.mud.yahoo.com>



On Fri, 30 Jun 2006, Luben Tuikov wrote:
> 
> I took a comparative look with and without "--full-history",
> and FWIW, enabling full history just clobbers the output with a lot
> of unnecessary information.  I.e. it shows merges which do not have
> direct consequence or change to the files in the path spec specified
> after the "--".

Oh, you're right - "--full-history" right now is literally geared solely 
towards the graphical kind, which needs all the merges (regardless of 
whether they change a file or not) to make sense of the history.

> I.e. no new, relevant information is being shown when "--full-history"
> is enabled.  In fact the default git-rev-list case, simplify_history=1,
> still shows a merge here and there which doesn't have any direct
> changes into what is being sought, but the difference is
> about 48% less clobber.

Well, with history simplification, we still show merges that are required 
to make the history _complete_, ie say that you had

	  a
	  |
	  b
	 / \
	c   d
	|   |

and neither "a" nor "b" actually changed the file, but both "c" and "d" 
did: in this case we have to leave "b" around just because otherwise there 
would be no way to show the _relationship_, even if "b" itself doesn't 
actually change the tree in any way what-so-ever.

> Can you consider the default case to be simplify_history=1,
> which is currently the default behaviour of git-rev-list.

Actually, for your case, you don't want _any_ merges, unless those merges 
literally changed the tree from all of the parents.

I think it would make sense to make that further simplification if the 
"--parents" flag wasn't present. 

Hmm. Maybe something like this..

BTW! Junio, I think this patch actually fixes a real bug.

Without this patch, the "--parents --full-history" combination (which 
you'd get if you do something like

	gitk --full-history Makefile

or similar) will actually _drop_ merges where all children are identical. 
That's wrong in the --full-history case, because it measn that the graph 
ends up missing lots of entries.

In the process, this also should make

	git-rev-list --full-history Makefile

give just the _true_ list of all commits that changed Makefile (and 
properly ignore merges that were identical in one parent), because now 
we're not asking for "--parent", so we don't need the unnecessary merge 
commits to keep the history together.

Luben, does this fix the problem for you?

		Linus

----
diff --git a/revision.c b/revision.c
index b963f2a..1cf6276 100644
--- a/revision.c
+++ b/revision.c
@@ -280,7 +280,7 @@ int rev_same_tree_as_empty(struct rev_in
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 {
 	struct commit_list **pp, *parent;
-	int tree_changed = 0;
+	int tree_changed = 0, tree_same = 0;
 
 	if (!commit->tree)
 		return;
@@ -298,6 +298,7 @@ static void try_to_simplify_commit(struc
 		parse_commit(p);
 		switch (rev_compare_tree(revs, p->tree, commit->tree)) {
 		case REV_TREE_SAME:
+			tree_same = 1;
 			if (!revs->simplify_history || (p->object.flags & UNINTERESTING)) {
 				/* Even if a merge with an uninteresting
 				 * side branch brought the entire change
@@ -334,7 +335,7 @@ static void try_to_simplify_commit(struc
 		}
 		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
 	}
-	if (tree_changed)
+	if (tree_changed && !tree_same)
 		commit->object.flags |= TREECHANGE;
 }
 
@@ -896,6 +897,8 @@ static int rewrite_one(struct rev_info *
 		struct commit *p = *pp;
 		if (!revs->limited)
 			add_parents_to_list(revs, p, &revs->commits);
+		if (p->parents && p->parents->next)
+			return 0;
 		if (p->object.flags & (TREECHANGE | UNINTERESTING))
 			return 0;
 		if (!p->parents)
@@ -988,8 +991,15 @@ struct commit *get_revision(struct rev_i
 		    commit->parents && commit->parents->next)
 			continue;
 		if (revs->prune_fn && revs->dense) {
-			if (!(commit->object.flags & TREECHANGE))
-				continue;
+			/* 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);
 		}

^ permalink raw reply related

* Re: [PATCH] Enable tree (directory) history display
From: Luben Tuikov @ 2006-07-01  3:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7v3bdmf2p6.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > I took a comparative look with and without "--full-history",
> > and FWIW, enabling full history just clobbers the output with a lot
> > of unnecessary information.  I.e. it shows merges which do not have
> > direct consequence or change to the files in the path spec specified
> > after the "--".
> >...
> > FWIW, I think that the original intention (had there been a choice)
> > would've been to show only most relevant history, i.e. changes
> > directly related to paths and files after the "--".
> 
> Sounds sane.  Maybe we should clearly explain this behaviour
> change in the commit log and claim it as an improvement.  We
> might even want to do --no-merges if we go that route.

I agree.  Indeed, this would definitely be an improvement.

   Luben

^ permalink raw reply

* Re: A note on merging conflicts..
From: Junio C Hamano @ 2006-07-01  3:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606301927260.12404@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Now, the downside is that the above is both a pain to type, and we don't 
> actually even save the MERGE_BASE as a head, so you actually have to 
> compute it yourself. It's easy enough to do:
>
> 	git-merge-base HEAD MERGE_HEAD > .git/MERGE_BASE
>
> will do it, but the fact is, we should make this even easier.

Heh, that's why I kept saying I want somebody to teach rev-list
a new notation, A...B, to mean $(merge-base A B)..B ;-).

> In fact, after writing the above a few times, I really think there's a 
> case for making a helper function that does exactly the above for us. 
> Including all the "conflicting-filename" thing. It would be nice if
>
> 	git log -p --merge [[--] filenames...]
>
> would basically expand to
>
> 	git log -p HEAD MERGE_HEAD
> 		^$(git-merge-base HEAD MERGE_HEAD)
> 		-- $(git-ls-files -u [filenames...])
>
> so that I wouldn't have to type that by hand ever again, and doing a
>
> 	git log -p --merge drivers/
>
> would automatically give me exactly that for all the unmerged files in 
> drivers/.

> Anybody want to try to make me happy, and learn some git internals at the 
> same time?

I fall in the former category but as the current maintainer I
feel I should leave chance to do the latter to others first.  I
wouldn't call it "trivial" but it is not that hard -- I think I
can write it in my head (as Linus can).

^ permalink raw reply

* Re: [PATCH] Enable tree (directory) history display
From: Junio C Hamano @ 2006-07-01  3:01 UTC (permalink / raw)
  To: ltuikov; +Cc: git, Linus Torvalds
In-Reply-To: <20060701024309.63001.qmail@web31805.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> I took a comparative look with and without "--full-history",
> and FWIW, enabling full history just clobbers the output with a lot
> of unnecessary information.  I.e. it shows merges which do not have
> direct consequence or change to the files in the path spec specified
> after the "--".
>...
> FWIW, I think that the original intention (had there been a choice)
> would've been to show only most relevant history, i.e. changes
> directly related to paths and files after the "--".

Sounds sane.  Maybe we should clearly explain this behaviour
change in the commit log and claim it as an improvement.  We
might even want to do --no-merges if we go that route.

^ permalink raw reply

* [PATCH] don't load objects needlessly when repacking
From: Nicolas Pitre @ 2006-07-01  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If no delta is attempted on some objects then it is useless to load them 
in memory, neither create any delta index for them.  The best thing to 
do is therefore to load and index them only when really needed.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

With this patch, a git-repack -a on the Linux kernel repo takes 19 
seconds instead of 25 seconds on my machine.  At this point the cost of 
creating a pack is largely dominated by git-rev-list alone while the 
actual pack creation is basically free.

diff --git a/pack-objects.c b/pack-objects.c
index 47da33b..b486ea5 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -970,11 +970,12 @@ struct unpacked {
  * one.
  */
 static int try_delta(struct unpacked *trg, struct unpacked *src,
-		     struct delta_index *src_index, unsigned max_depth)
+		     unsigned max_depth)
 {
 	struct object_entry *trg_entry = trg->entry;
 	struct object_entry *src_entry = src->entry;
-	unsigned long size, src_size, delta_size, sizediff, max_size;
+	unsigned long trg_size, src_size, delta_size, sizediff, max_size, sz;
+	char type[10];
 	void *delta_buf;
 
 	/* Don't bother doing diffs between different types */
@@ -1009,19 +1010,38 @@ static int try_delta(struct unpacked *tr
 		return 0;
 
 	/* Now some size filtering heuristics. */
-	size = trg_entry->size;
-	max_size = size/2 - 20;
+	trg_size = trg_entry->size;
+	max_size = trg_size/2 - 20;
 	max_size = max_size * (max_depth - src_entry->depth) / max_depth;
 	if (max_size == 0)
 		return 0;
 	if (trg_entry->delta && trg_entry->delta_size <= max_size)
 		max_size = trg_entry->delta_size-1;
 	src_size = src_entry->size;
-	sizediff = src_size < size ? size - src_size : 0;
+	sizediff = src_size < trg_size ? trg_size - src_size : 0;
 	if (sizediff >= max_size)
 		return 0;
 
-	delta_buf = create_delta(src_index, trg->data, size, &delta_size, max_size);
+	/* Load data if not already done */
+	if (!trg->data) {
+		trg->data = read_sha1_file(trg_entry->sha1, type, &sz);
+		if (sz != trg_size)
+			die("object %s inconsistent object length (%lu vs %lu)",
+			    sha1_to_hex(trg_entry->sha1), sz, trg_size);
+	}
+	if (!src->data) {
+		src->data = read_sha1_file(src_entry->sha1, type, &sz);
+		if (sz != src_size)
+			die("object %s inconsistent object length (%lu vs %lu)",
+			    sha1_to_hex(src_entry->sha1), sz, src_size);
+	}
+	if (!src->index) {
+		src->index = create_delta_index(src->data, src_size);
+		if (!src->index)
+			die("out of memory");
+	}
+
+	delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, max_size);
 	if (!delta_buf)
 		return 0;
 
@@ -1054,8 +1074,6 @@ static void find_deltas(struct object_en
 	while (--i >= 0) {
 		struct object_entry *entry = list[i];
 		struct unpacked *n = array + idx;
-		unsigned long size;
-		char type[10];
 		int j;
 
 		if (!entry->preferred_base)
@@ -1082,11 +1100,8 @@ static void find_deltas(struct object_en
 		free_delta_index(n->index);
 		n->index = NULL;
 		free(n->data);
+		n->data = NULL;
 		n->entry = entry;
-		n->data = read_sha1_file(entry->sha1, type, &size);
-		if (size != entry->size)
-			die("object %s inconsistent object length (%lu vs %lu)",
-			    sha1_to_hex(entry->sha1), size, entry->size);
 
 		j = window;
 		while (--j > 0) {
@@ -1097,7 +1112,7 @@ static void find_deltas(struct object_en
 			m = array + other_idx;
 			if (!m->entry)
 				break;
-			if (try_delta(n, m, m->index, depth) < 0)
+			if (try_delta(n, m, depth) < 0)
 				break;
 		}
 		/* if we made n a delta, and if n is already at max
@@ -1107,10 +1122,6 @@ static void find_deltas(struct object_en
 		if (entry->delta && depth <= entry->depth)
 			continue;
 
-		n->index = create_delta_index(n->data, size);
-		if (!n->index)
-			die("out of memory");
-
 		idx++;
 		if (idx >= window)
 			idx = 0;

^ permalink raw reply related

* A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01  2:44 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano


Ok, over the last week or so, I've been having a lot more content 
conflicts than usual, mostly because of 

 (a) just the fact that the way the merge window happens for the kernel 
     these days, rather than have incremental small merges, we often end 
     up having lots of big ones.
and

 (b) I ended up merging a few trees that had lots of small changes all 
     over, notably to header files having their <config.h> include 
     removed, causing trivial conflicts.

Now, the good news is that I have to say that our conflict resolution 
rocks. It's all been _very_ easy to do. In fact, it's been even more 
pleasant than BK was, because of one big issue: you could resolve the 
conflict in the tree, then _test_ it (perhaps just compile-test it), and 
commit the resolved result separately. With BK, you had to resolve and 
commit atomically, and you never had access to a "preliminary resolve" to 
test.

However, I also notived one particular thing that I did that we make less 
than perfectly easy.

One thing that is _very_ useful to do is to do when you have a conflict is 
this:

	git log -p HEAD MERGE_BASE..MERGE_HEAD -- conflicting-filename

because this shows all the changes (with their explanations) for that 
filename since the MERGE_BASE in _both_ branches you're trying to merge. 
This simple command really makes conflict resolution a hell of a lot 
easier, because you can see what caused the conflict, and you get a real 
feel for what both branches were doing, making it a _lot_ more likely that 
you actually do the right thing.

Now, the downside is that the above is both a pain to type, and we don't 
actually even save the MERGE_BASE as a head, so you actually have to 
compute it yourself. It's easy enough to do:

	git-merge-base HEAD MERGE_HEAD > .git/MERGE_BASE

will do it, but the fact is, we should make this even easier.

In fact, after writing the above a few times, I really think there's a 
case for making a helper function that does exactly the above for us. 
Including all the "conflicting-filename" thing. It would be nice if

	git log -p --merge [[--] filenames...]

would basically expand to

	git log -p HEAD MERGE_HEAD
		^$(git-merge-base HEAD MERGE_HEAD)
		-- $(git-ls-files -u [filenames...])

so that I wouldn't have to type that by hand ever again, and doing a

	git log -p --merge drivers/

would automatically give me exactly that for all the unmerged files in 
drivers/.

Anybody want to try to make me happy, and learn some git internals at the 
same time?

			Linus

^ permalink raw reply

* Re: [PATCH] Enable tree (directory) history display
From: Luben Tuikov @ 2006-07-01  2:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7vy7vef52m.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > This patch allows history display of whole trees/directories a la
> > "git-rev-list HEAD -- <dir or file>".  I find this useful especially
> > when a project lives in its own subdirectory, as opposed to being all
> > of the GIT repository (i.e. when a sub-project is merged into a
> > super-project).
> 
> Both patches from you were seriously damaged.  Check your MUA
> before sending further patches, please.

Yes, I'll take a look.

> 
> They were trivial for me to apply by hand, so no need to resend
> them.  I like the effect of this one I am replying to very much.

Junio, Linus,

I took a comparative look with and without "--full-history",
and FWIW, enabling full history just clobbers the output with a lot
of unnecessary information.  I.e. it shows merges which do not have
direct consequence or change to the files in the path spec specified
after the "--".

I.e. no new, relevant information is being shown when "--full-history"
is enabled.  In fact the default git-rev-list case, simplify_history=1,
still shows a merge here and there which doesn't have any direct
changes into what is being sought, but the difference is
about 48% less clobber.

Can you consider the default case to be simplify_history=1,
which is currently the default behaviour of git-rev-list.

If not, is it possible to make this a configurable flag
in gitweb, so that people can decide how much non-related
history to show?  I.e. I'd opt for the default case,
no full-history.

FWIW, I think that the original intention (had there been a choice)
would've been to show only most relevant history, i.e. changes
directly related to paths and files after the "--".

Thanks,
  Luben

^ permalink raw reply

* Re: [PATCH] Enable tree (directory) history display
From: Junio C Hamano @ 2006-07-01  2:10 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20060701010012.1559.qmail@web31809.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> This patch allows history display of whole trees/directories a la
> "git-rev-list HEAD -- <dir or file>".  I find this useful especially
> when a project lives in its own subdirectory, as opposed to being all
> of the GIT repository (i.e. when a sub-project is merged into a
> super-project).

Both patches from you were seriously damaged.  Check your MUA
before sending further patches, please.

They were trivial for me to apply by hand, so no need to resend
them.  I like the effect of this one I am replying to very much.

^ permalink raw reply

* Re: [PATCH] Speed up history generation
From: Luben Tuikov @ 2006-07-01  1:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3bdmglxo.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > --- Junio C Hamano <junkio@cox.net> wrote:
> >...
> >> > @@ -2295,16 +2295,12 @@ sub git_history {
> >> >  	      "</div>\n";
> >> >  	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
> >> >  
> >> > -	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
> >> > \'$file_name\'";
> >> > -	my $commit;
> >> > +	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
> >> 
> >> This would speed things up but at the same time it changes the
> >> semantics because it involves merge simplification, no?
> >> 
> >> At least that should be noted in the commit log.
> >
> > Ok, I guess this should be in the log.  Can you add it please when
> > commiting to the master git branch?
> 
> Well, by "at least", what I meant was that it might make sense
> to pass "--full-history" option to be more compatible with the
> original output.  For graphical output like gitk, --full-history
> makes a mess on the screen, but a list-oriented output like
> gitweb it might be less confusing to show all the alternate
> paths that touched the path than leaving some histories out.

Ok, I can add the --full-history option, test it out and resubmit.

    Luben

^ permalink raw reply

* Re: [PATCH] Speed up history generation
From: Junio C Hamano @ 2006-07-01  1:20 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20060701011112.892.qmail@web31813.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> --- Junio C Hamano <junkio@cox.net> wrote:
>...
>> > @@ -2295,16 +2295,12 @@ sub git_history {
>> >  	      "</div>\n";
>> >  	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
>> >  
>> > -	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
>> > \'$file_name\'";
>> > -	my $commit;
>> > +	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
>> 
>> This would speed things up but at the same time it changes the
>> semantics because it involves merge simplification, no?
>> 
>> At least that should be noted in the commit log.
>
> Ok, I guess this should be in the log.  Can you add it please when
> commiting to the master git branch?

Well, by "at least", what I meant was that it might make sense
to pass "--full-history" option to be more compatible with the
original output.  For graphical output like gitk, --full-history
makes a mess on the screen, but a list-oriented output like
gitweb it might be less confusing to show all the alternate
paths that touched the path than leaving some histories out.

^ permalink raw reply

* Re: [PATCH] Speed up history generation
From: Linus Torvalds @ 2006-07-01  1:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: ltuikov, git
In-Reply-To: <7v7j2ygmou.fsf@assigned-by-dhcp.cox.net>



On Fri, 30 Jun 2006, Junio C Hamano wrote:

> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > Speed up history generation as suggested by Linus.
> > @@ -2295,16 +2295,12 @@ sub git_history {
> >  	      "</div>\n";
> >  	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
> >  
> > -	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
> > \'$file_name\'";
> > -	my $commit;
> > +	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
> 
> This would speed things up but at the same time it changes the
> semantics because it involves merge simplification, no?

Or just add a flag or config option that enables "--full-history" on that 
git-rev-list. Perhaps it should be the default fir gitweb.

With --full-history, it should still be better to do this inside 
git-rev-list than piping things into git-diff-tree just to limit by 
pathname..

		Linus

^ permalink raw reply

* Re: [PATCH] Speed up history generation
From: Luben Tuikov @ 2006-07-01  1:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j2ygmou.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > Speed up history generation as suggested by Linus.
> > @@ -2295,16 +2295,12 @@ sub git_history {
> >  	      "</div>\n";
> >  	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
> >  
> > -	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
> > \'$file_name\'";
> > -	my $commit;
> > +	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
> 
> This would speed things up but at the same time it changes the
> semantics because it involves merge simplification, no?
> 
> At least that should be noted in the commit log.

Ok, I guess this should be in the log.  Can you add it please when
commiting to the master git branch?

   Luben

^ permalink raw reply

* Re: [PATCH] Speed up history generation
From: Junio C Hamano @ 2006-07-01  1:04 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20060701005924.7726.qmail@web31812.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> Speed up history generation as suggested by Linus.
> @@ -2295,16 +2295,12 @@ sub git_history {
>  	      "</div>\n";
>  	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
>  
> -	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
> \'$file_name\'";
> -	my $commit;
> +	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";

This would speed things up but at the same time it changes the
semantics because it involves merge simplification, no?

At least that should be noted in the commit log.

^ permalink raw reply

* [PATCH] Enable tree (directory) history display
From: Luben Tuikov @ 2006-07-01  1:00 UTC (permalink / raw)
  To: git

This patch allows history display of whole trees/directories a la
"git-rev-list HEAD -- <dir or file>".  I find this useful especially
when a project lives in its own subdirectory, as opposed to being all
of the GIT repository (i.e. when a sub-project is merged into a
super-project).

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.cgi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index 2705a93..d808900 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -1676,6 +1676,7 @@ #			      " | " . $cgi->a({-href => "$my
 			      "</td>\n" .
 			      "<td class=\"link\">" .
 			      $cgi->a({-href => "$my_uri?" .
esc_param("p=$project;a=tree;h=$t_hash$base_key;f=$base$t_name")}, "tree") .
+			      " | " . $cgi->a({-href => "$my_uri?" .
esc_param("p=$project;a=history;h=$hash_base;f=$base$t_name")}, "history") .
 			      "</td>\n";
 		}
 		print "</tr>\n";
-- 
1.4.1.rc2.g4ce4

^ permalink raw reply related

* [PATCH] Speed up history generation
From: Luben Tuikov @ 2006-07-01  0:59 UTC (permalink / raw)
  To: git

Speed up history generation as suggested by Linus.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.cgi |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index 035e76d..2705a93 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -2295,16 +2295,12 @@ sub git_history {
 	      "</div>\n";
 	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
 
-	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin --
\'$file_name\'";
-	my $commit;
+	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
 	print "<table cellspacing=\"0\">\n";
 	my $alternate = 0;
 	while (my $line = <$fd>) {
 		if ($line =~ m/^([0-9a-fA-F]{40})/){
-			$commit = $1;
-			next;
-		}
-		if ($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\t(.*)$/ &&
(defined $commit)) {
+			my $commit = $1;
 			my %co = git_read_commit($commit);
 			if (!%co) {
 				next;
@@ -2336,7 +2332,6 @@ sub git_history {
 			}
 			print "</td>\n" .
 			      "</tr>\n";
-			undef $commit;
 		}
 	}
 	print "</table>\n";
-- 
1.4.1.rc2.g4ce4

^ permalink raw reply related

* Re: [RFC/PATCH 14] autoconf: Added --with/--without for openssl, curl, expat to ./configure
From: Jakub Narebski @ 2006-06-30 22:34 UTC (permalink / raw)
  To: git
In-Reply-To: <200606302345.17045.jnareb@gmail.com>

Jakub Narebski wrote:

> +AC_ARG_WITH(expat,
> +AS_HELP_STRING([--with-expat],[support git-push using http:// and https:// transports via WebDAV (default is YES)])
> +AS_HELP_STRING([],            [ARG can be also prefix for expat library and headers]),\
> +MY_PARSE_WITH(EXPAT))

Of course to do anything for --with-expat=PATH case this patch needs
the one that introduces EXPATDIR (there was such patch on the list,
I'm not sure if it got accepted).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [RFC/PATCH 14] autoconf: Added --with/--without for openssl, curl, expat to ./configure
From: Jakub Narebski @ 2006-06-30 22:32 UTC (permalink / raw)
  To: git
In-Reply-To: <1151704626.12008.5.camel@dv>

<posted & mailed>

Pavel Roskin wrote:

> On Fri, 2006-06-30 at 23:45 +0200, Jakub Narebski wrote:
>> I'm not autoconf/m4 expert: could anyone tell me how to uppercase
>> PACKAGE name, so one could write MY_PARSE_WITH(openssl)?
> 
> I don't quite understand what you want, but you could check m4_toupper.

Thanks. It works great. GIT_PARSE_WITH is now:

AC_DEFUN([GIT_PARSE_WITH],
[[PACKAGE=m4_toupper($1); \
if test "$withval" = "no"; then \
    GIT_APPEND_LINE(NO_${PACKAGE}=YesPlease); \
  elif test "$withval" != "yes"; then \
    GIT_APPEND_LINE(${PACKAGE}DIR=$withval); \
  fi; \
]])# GIT_PARSE_WITH

>> How to add [=PATH] to --with-PACKAGE option description in a way
>> which does not screw up AS_HELP_WITH calculations. I could use
>> @<:@=PATH@:>@ which transforms to [=PATH], but AS_HELP_WITH counts
>> number of characters in source I think.
> 
> Try another pair of square brackets as quotes for literal contents.  In
> this case, use [[=PATH]]

I suspect that AS_HELP_WITH does some strange quoting, or stripping. Both
[=PATH] and [[=PATH]] produces =PATH in ./configure --help output.
When using @<:@=PATH@:>@ I get [=PATH], but the description of option begins
line below.

>> +# MY_PARSE_WITH(PACKAGE)
> 
> By the way, it's better to use a prefix other than MY.  I thing
> GIT_PARSE_WITH would be great.

Any ideas for name of MY_APPEND_LINE(LINE)/GIT_APPEND_LINE(LINE) macro,
which as a result adds line to output (e.g. LINE = "NO_CURL=YesPlease")?

Thanks for all suggestions
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [RFC/PATCH 14] autoconf: Added --with/--without for openssl, curl, expat to ./configure
From: Pavel Roskin @ 2006-06-30 21:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200606302345.17045.jnareb@gmail.com>

Hi Jakub,

On Fri, 2006-06-30 at 23:45 +0200, Jakub Narebski wrote:
> I'm not autoconf/m4 expert: could anyone tell me how to uppercase
> PACKAGE name, so one could write MY_PARSE_WITH(openssl)?

I don't quite understand what you want, but you could check m4_toupper.

> How to add [=PATH] to --with-PACKAGE option description in a way
> which does not screw up AS_HELP_WITH calculations. I could use
> @<:@=PATH@:>@ which transforms to [=PATH], but AS_HELP_WITH counts
> number of characters in source I think.

Try another pair of square brackets as quotes for literal contents.  In
this case, use [[=PATH]]

> +# MY_PARSE_WITH(PACKAGE)

By the way, it's better to use a prefix other than MY.  I thing
GIT_PARSE_WITH would be great.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* [RFC/PATCH 14] autoconf: Added --with/--without for openssl, curl, expat to ./configure
From: Jakub Narebski @ 2006-06-30 21:45 UTC (permalink / raw)
  To: git
In-Reply-To: <200606301711.39635.jnareb@gmail.com>


Added --with-PACKAGE[=PATH] (where PATH is prefix for PACKAGE
libraries and includes) and --without-PACKAGE (--with-PACKAGE=no)
support for curl, openssl, expat to configure.ac

Signed-off-by: Jakub Narebski <jnareb@gmail.com>

---

I'm not autoconf/m4 expert: could anyone tell me how to uppercase
PACKAGE name, so one could write MY_PARSE_WITH(openssl)?

How to add [=PATH] to --with-PACKAGE option description in a way
which does not screw up AS_HELP_WITH calculations. I could use
@<:@=PATH@:>@ which transforms to [=PATH], but AS_HELP_WITH counts
number of characters in source I think.

 configure.ac |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index fcfc9ce..26d6f4d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -19,6 +19,22 @@ # Append LINE to file ${config_append}
 AC_DEFUN([MY_APPEND_LINE],
 [[echo "$1" >> "${config_append}"]])# MY_APPEND_LINE
 
+# MY_PARSE_WITH(PACKAGE)
+# ----------------------
+# For use in AC_ARG_WITH action-if-found, for packages default ON. 
+# * Set NO_PACKAGE=YesPlease  for --without-PACKAGE
+# * Set PACKAGEDIR=ARG for --with-PACKAGE=ARG
+# * Do nothing for --with-PACKAGE without ARG
+# PACKAGE option must be all uppercase
+AC_DEFUN([MY_PARSE_WITH],
+[[if test "$withval" = "no"; then \
+    MY_APPEND_LINE(NO_$1=YesPlease); \
+  elif test "$withval" != "yes"; then \
+    MY_APPEND_LINE($1DIR=$withval); \
+  fi; \
+]])# MY_PARSE_WITH
+
+
 # Checks for libraries.
 AC_MSG_NOTICE([CHECKS for libraries])
 AC_CHECK_LIB([crypto], [SHA1_Init],,MY_APPEND_LINE(NO_OPENSSL=YesPlease))
@@ -48,6 +64,24 @@ AC_CHECK_FUNC(strlcpy,,MY_APPEND_LINE(NO
 AC_CHECK_FUNC(setenv,,MY_APPEND_LINE(NO_SETENV=YesPlease))
 
 
+# Site configuration
+AC_MSG_NOTICE([CHECKS for site configuration])
+AC_ARG_WITH(curl, 
+AS_HELP_STRING([--with-curl],[support http(s):// transports (default is YES)])
+AS_HELP_STRING([],           [ARG can be also prefix for curl library and headers]),\
+MY_PARSE_WITH(CURL))
+
+AC_ARG_WITH(openssl,
+AS_HELP_STRING([--with-openssl],[use OpenSSL library (default is YES)])
+AS_HELP_STRING([],              [ARG can be prefix for openssl library and headers]),\
+MY_PARSE_WITH(OPENSSL))
+
+AC_ARG_WITH(expat,
+AS_HELP_STRING([--with-expat],[support git-push using http:// and https:// transports via WebDAV (default is YES)])
+AS_HELP_STRING([],            [ARG can be also prefix for expat library and headers]),\
+MY_PARSE_WITH(EXPAT))
+
+
 # Output files
 AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
 AC_OUTPUT
-- 
1.4.0

^ permalink raw reply related

* Re: Prepare "git-merge-tree" for future work
From: Junio C Hamano @ 2006-06-30 20:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606291925230.12404@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> In contrast, the ones to diff_filespec I've never really used, and I did 
> not want to compare blob objects, I very much wanted to compare in-memory 
> buffers (_and_ potentially blobs).
>
> So if you can show an easy example of how to populate a set of filespec 
> pairs (not with blobs - with in-memory generated data) and insert them 
> onto the lists, that would be good.

Ah, I see.  Your origin() function always returns a in-core
buffer from an existing blob (or NULL if it is a new file) but
result() returns either an existing blob resulting from the
tree-level merge, or a 3-way content merge result that does not
have an existing blob, and it is not obvious how to express the
latter as a filespec (all other cases you can stuff the blob
object name in the sha1[] member and if you choose to do the
read_sha1_file() yourself store the result in data member or you
can let diff_populate_filespec() read the data when diff
machinery needs it).

I think a filespec that has 0{40} sha1, with data already
populated and should_free/should_munmap members both set to
false might work for the in-memory data but I haven't tried.

I'd take the hint, and I will (eventually) take a look at it
if nobody beats me to it, but most likely not now, sorry.

^ permalink raw reply

* Re: [PATCH 13] autoconf: Append '-include config.mak.autogen' to config.mak if it is not present
From: Jakub Narebski @ 2006-06-30 20:29 UTC (permalink / raw)
  To: git
In-Reply-To: <200606301711.39635.jnareb@gmail.com>

Jakub Narebski wrote:

> +grep -q -s -F "-include ${config_file}" config.mak || \
> +        echo  "-include ${config_file}" >> config.mak

Gah, should be of course

+grep -q -s -F -e "-include ${config_file}" config.mak || \
+            echo "-include ${config_file}" >> config.mak

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] git-grep: --and to combine patterns with and instead of or
From: Junio C Hamano @ 2006-06-30 20:26 UTC (permalink / raw)
  To: jnareb; +Cc: git
In-Reply-To: <e83t0m$4s0$2@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Matthias version is truly more expressive, especially with context limiting
> extension. 

That's orthogonal.  I do not think there is any reason you
cannot make the version whose --near is similar to --and to
understand different ranges for each "neighbor search"
expression using --near=M:N syntax.

Now stop talking and code it up, please ;-).

^ permalink raw reply


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