Git development
 help / color / mirror / Atom feed
* Re: Fixes for option parsing
From: Junio C Hamano @ 2006-04-17  3:36 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161938070.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Also, I think the "---" printing should be removed, and moved into the 
> "diffstat" flushing code.

I am not sure if that belongs to diffstat flush.  I was instead
planning to  move it to diff-flush code, and show it only when we
actually show any diffs.  IOW, I think we would want it even
when we are doing "--pretty -p", not "--pretty --patch-with-stat".

^ permalink raw reply

* [PATCH] Makefile fixups.
From: A Large Angry SCM @ 2006-04-17  3:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


Signed-off-by: A Large Angry SCM <gitzilla@gmail.com>


---

 Makefile   |    2 +-
 t/Makefile |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

bb576e0c102e68cc55691fa4a5fc7e793e6e1219
diff --git a/Makefile b/Makefile
index 1130af4f3809610436f4c5d17c3970a947ddae5c..8371f7f522b6b02039af2a9d1684a8137197526e 100644
--- a/Makefile
+++ b/Makefile
@@ -653,7 +653,7 @@ ### Cleaning rules
 clean:
 	rm -f *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
-	rm -f $(ALL_PROGRAMS) git$X
+	rm -f $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	rm -f *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags
 	rm -rf $(GIT_TARNAME)
 	rm -f $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
diff --git a/t/Makefile b/t/Makefile
index fe65f53c5fbcf07bb69214b0f0cff8aef551e906..549598575b9fdcefe857ddef1b6d980a9773b033 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -25,5 +25,5 @@ clean:
 	rm -fr trash
 
 .PHONY: $(T) clean
-.NOPARALLEL:
+.NOTPARALLEL:
 
-- 
1.3.0.rc4.g5bc4

^ permalink raw reply related

* Re: Fixes for option parsing
From: Linus Torvalds @ 2006-04-17  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161938070.3701@g5.osdl.org>



On Sun, 16 Apr 2006, Linus Torvalds wrote:
> 
> Maybe. I'm not convinced, though. The reason? cmd_log_wc needs to generate 
> it regardless, for the "always" case.

Of course, you could just have the "diff" logic unconditionally do the 
call-back. That would be clean enough.

> Also, I think the "---" printing should be removed, and moved into the 
> "diffstat" flushing code. Right now it does the wrong thing entirely if 
> no diff exists, but we have always_show_header: it will print the "---" 
> for no good reason.

The alternative is to do something like this, but because the "diffstat" 
doesn't flush the header priperly, it doesn't add the "---" for merges 
(and it used to not show the log at all, even for "git log", before I 
fixed it).

Try it with

	git log --cc --patch-with-stat

to see what I mean.

I do agree that this would be much cleaner with a "print header" callback 
in the diffopt structure. This patch is the really hacky "continue to do 
things the ugly way" approach to fix some of the uglier output.

Only meant as a RFC and to illustrate what I think the output should look 
like (modulo the lack of "---" before a diffstat with no patch - for 
merges). Not meant to actually be applied, I think this can be done much 
more cleanly with the callback.

		Linus

---
diff --git a/git.c b/git.c
index fc4e429..dc577fa 100644
--- a/git.c
+++ b/git.c
@@ -284,7 +284,7 @@ static int cmd_log_wc(int argc, const ch
 	struct commit *commit;
 	char *buf = xmalloc(LOGSIZE);
 	const char *commit_prefix = "commit ";
-	int shown = 0;
+	int shown = 0, always_show_header;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
@@ -295,13 +295,19 @@ static int cmd_log_wc(int argc, const ch
 	if (rev->commit_format == CMIT_FMT_ONELINE)
 		commit_prefix = "";
 
+	/*
+	 * We handle always_show_header outselves, and leave the
+	 * "---" handling to log_tree_commit
+	 */
+	always_show_header = rev->always_show_header;
+	rev->always_show_header = 0;
+
 	prepare_revision_walk(rev);
 	setup_pager();
 	while ((commit = get_revision(rev)) != NULL) {
 		unsigned long ofs = 0;
 
-		if (shown && rev->diff &&
-		    rev->commit_format != CMIT_FMT_ONELINE)
+		if (shown && rev->commit_format != CMIT_FMT_ONELINE)
 			putchar('\n');
 
 		ofs = sprintf(buf, "%s", commit_prefix);
@@ -338,14 +344,16 @@ static int cmd_log_wc(int argc, const ch
 					   buf + ofs,
 					   LOGSIZE - ofs - 20,
 					   rev->abbrev);
+		if (always_show_header || !rev->diff) {
+			fputs(buf, stdout);
+			ofs = 0;
+		}
 
 		if (rev->diff) {
+			strcpy(buf + ofs, rev->diffopt.with_stat ? "---" : "\n");
 			rev->use_precomputed_header = buf;
-			strcpy(buf + ofs, "\n---\n");
 			log_tree_commit(rev, commit);
 		}
-		else
-			printf("%s\n", buf);
 		shown = 1;
 		free(commit->buffer);
 		commit->buffer = NULL;

^ permalink raw reply related

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-17  2:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604170138470.824@wbgn013.biozentrum.uni-wuerzburg.de>



On Mon, 17 Apr 2006, Johannes Schindelin wrote:
> 
> Since quite some time, I wanted to have a way to git-rev-list just the 
> revs between commit1 and commit2, i.e. all commits which are ancestors of 
> commit2, and which have commit1 as ancestor. With this, my task would have 
> been more than simple.

Yes. However, it's not trivial.

In fact, what you want is not what you claim you want. To be useful in 
general, you have to _also_ handle the case of "commit2" not beign a 
strict ancestor of "commit1". So what you actually want to do is

 - calculate the merge-head of cmit1 and cmit2 (and if there are multiple, 
   pick some "best" one).
 - pick the shortest path from the merge-head to the cmit1 (and, with a 
   flag, also pick the path from merge-head to cmit2 - sometimes you want 
   to see the whole path from one to the other, sometimes you might want 
   to see just the path from the last common point).

I suspect it ends up being not _that_ different from calculating the 
bisection point, but I haven't thought it through entirely.

		Linus

^ permalink raw reply

* Re: Fixes for option parsing
From: Linus Torvalds @ 2006-04-17  2:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqv1oxie.fsf@assigned-by-dhcp.cox.net>



On Sun, 16 Apr 2006, Junio C Hamano wrote:
> 
> In the mid-term, I am hoping we can drop the generate_header()
> callchain _and_ the custom code that formats commit log in-core,
> found in cmd_log_wc().

Maybe. I'm not convinced, though. The reason? cmd_log_wc needs to generate 
it regardless, for the "always" case.

Also, I think the "---" printing should be removed, and moved into the 
"diffstat" flushing code. Right now it does the wrong thing entirely if no 
diff exists, but we have always_show_header: it will print the "---" for 
no good reason. 

> I wish this "diff-tree SHA1 (from ANOTHERSHA1)" format can be
> dropped and replaced with "commit SHA1" format like "git log"
> does uniformly, but it might be already depended upon by
> Porcelains.

I think it might be worth just trying. A lot of the formats were pretty 
ad-hoc, and since everybody had their own routines, there was no reason to 
even try to keep them similar. Now, we'd be better off trying to make them 
look as close to each other as possible, and I suspect there's not a lot 
of porcelain that cares deeply about that format..

		Linus

^ permalink raw reply

* [PATCH] rev-list --boundary: show boundary commits even when limited otherwise.
From: Junio C Hamano @ 2006-04-17  1:53 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git, Linus Torvalds
In-Reply-To: <7vslodp4sk.fsf@assigned-by-dhcp.cox.net>

The boundary commits are shown for UI like gitk to draw them as
soon as topo-order sorting allows, and should not be omitted by
get_revision() filtering logic.  As long as their immediate
child commits are shown, we should not filter them out.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

  > The real issue is "git-rev-list --boundary ran..ge -- path" does
  > not show boundary.  I am not sure if it even worked in the past;
  > will take a look.

  I _think_ this fixes it, but this is rev-list which I feel a
  bit uneasy touching.  A lower-impact approach of not filtering
  BOUNDARY commits at all does not work well; it  shows many
  isolated open-circle commits in gitk that do not touch the
  specified paths.

  I got tired of updating the private object.flags users, so
  just reserved the lower 16 bits for use of revision.c while I
  was at it.

 http-push.c |   10 +++++-----
 rev-list.c  |    4 ++--
 revision.c  |   29 +++++++++++++++++++++++++++--
 revision.h  |    3 ++-
 4 files changed, 36 insertions(+), 10 deletions(-)

298196646f35b6f5bc2131074204826d39aff8cf
diff --git a/http-push.c b/http-push.c
index 19a0f77..e7f7f44 100644
--- a/http-push.c
+++ b/http-push.c
@@ -60,12 +60,12 @@ #define LOCK_REQUEST "<?xml version=\"1.
 #define LOCK_TIME 600
 #define LOCK_REFRESH 30
 
-/* bits #0-6 in revision.h */
+/* bits #0-15 in revision.h */
 
-#define LOCAL    (1u << 7)
-#define REMOTE   (1u << 8)
-#define FETCHING (1u << 9)
-#define PUSHING  (1u << 10)
+#define LOCAL    (1u<<16)
+#define REMOTE   (1u<<17)
+#define FETCHING (1u<<18)
+#define PUSHING  (1u<<19)
 
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
diff --git a/rev-list.c b/rev-list.c
index 963707a..f5511e7 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -8,9 +8,9 @@ #include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0-6 in revision.h */
+/* bits #0-15 in revision.h */
 
-#define COUNTED		(1u<<7)
+#define COUNTED		(1u<<16)
 
 static const char rev_list_usage[] =
 "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
diff --git a/revision.c b/revision.c
index 0505f3f..e1f9816 100644
--- a/revision.c
+++ b/revision.c
@@ -750,6 +750,17 @@ static void rewrite_parents(struct rev_i
 	}
 }
 
+static void mark_boundary_to_show(struct commit *commit)
+{
+	struct commit_list *p = commit->parents;
+	while (p) {
+		commit = p->item;
+		p = p->next;
+		if (commit->object.flags & BOUNDARY)
+			commit->object.flags |= BOUNDARY_SHOW;
+	}
+}
+
 struct commit *get_revision(struct rev_info *revs)
 {
 	struct commit_list *list = revs->commits;
@@ -787,8 +798,20 @@ struct commit *get_revision(struct rev_i
 		}
 		if (commit->object.flags & SHOWN)
 			continue;
-		if (!(commit->object.flags & BOUNDARY) &&
-		    (commit->object.flags & UNINTERESTING))
+
+		/* We want to show boundary commits only when their
+		 * children are shown.  When path-limiter is in effect,
+		 * rewrite_parents() drops some commits from getting shown,
+		 * and there is no point showing boundary parents that
+		 * are not shown.  After rewrite_parents() rewrites the
+		 * parents of a commit that is shown, we mark the boundary
+		 * parents with BOUNDARY_SHOW.
+		 */
+		if (commit->object.flags & BOUNDARY_SHOW) {
+			commit->object.flags |= SHOWN;
+			return commit;
+		}
+		if (commit->object.flags & UNINTERESTING)
 			continue;
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
 			continue;
@@ -801,6 +824,8 @@ struct commit *get_revision(struct rev_i
 			if (revs->parents)
 				rewrite_parents(revs, commit);
 		}
+		if (revs->boundary)
+			mark_boundary_to_show(commit);
 		commit->object.flags |= SHOWN;
 		return commit;
 	} while (revs->commits);
diff --git a/revision.h b/revision.h
index 8970b57..4b27043 100644
--- a/revision.h
+++ b/revision.h
@@ -7,7 +7,8 @@ #define TREECHANGE	(1u<<2)
 #define SHOWN		(1u<<3)
 #define TMP_MARK	(1u<<4) /* for isolated cases; clean after use */
 #define BOUNDARY	(1u<<5)
-#define ADDED		(1u<<6)	/* Parents already parsed and added? */
+#define BOUNDARY_SHOW	(1u<<6)
+#define ADDED		(1u<<7)	/* Parents already parsed and added? */
 
 struct rev_info;
 
-- 
1.3.0.rc4.g878b

^ permalink raw reply related

* Re: [BUG] gitk: breaks with both version and file limits
From: Paul Mackerras @ 2006-04-17  0:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Yann Dirson, git
In-Reply-To: <7vslodp4sk.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano writes:

> The real issue is "git-rev-list --boundary ran..ge -- path" does
> not show boundary.  I am not sure if it even worked in the past;
> will take a look.

Ah... and in addextraid I wasn't appending a 0 to commitlisted, so
commitlisted got out of sync with the displayorder list.  I just
committed this patch, which keeps them in sync.  I think that's
preferable to working around the missing elements.

Paul.

diff --git a/gitk b/gitk
index f88c06e..87e7162 100755
--- a/gitk
+++ b/gitk
@@ -1116,11 +1116,12 @@ proc layoutrows {row endrow last} {
 
 proc addextraid {id row} {
     global displayorder commitrow commitinfo
-    global commitidx
+    global commitidx commitlisted
     global parentlist childlist children
 
     incr commitidx
     lappend displayorder $id
+    lappend commitlisted 0
     lappend parentlist {}
     set commitrow($id) $row
     readcommit $id
@@ -1500,7 +1501,7 @@ proc drawcmittext {id row col rmx} {
 proc drawcmitrow {row} {
     global displayorder rowidlist
     global idrowranges idrangedrawn iddrawn
-    global commitinfo commitlisted parentlist numcommits
+    global commitinfo parentlist numcommits
 
     if {$row >= $numcommits} return
     foreach id [lindex $rowidlist $row] {

^ permalink raw reply related

* Re: Fixes for option parsing
From: Junio C Hamano @ 2006-04-17  0:29 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161433000.3701@g5.osdl.org>

Thanks for fixing this.  I've applied and merged it to the
"next" branch.

In the mid-term, I am hoping we can drop the generate_header()
callchain _and_ the custom code that formats commit log in-core,
found in cmd_log_wc().  rev->use_precomputed_header and perhaps
rev->header can be dropped and replaced with a call to a
function to print formatted log to stdout (or perhaps a FILE *,
because I want to reuse this to make output part of
"git-format-patch" internal) from log-tree.

For that, I'd need to define a new CMIT_FMT for the format-patch
output.  Another thing needed is to clean up the commit_prefix
local variable defined in cmd_log_wc() -- it should use
rev->header_prefix, but before that the part that rewrites
opt->header_prefix to have a blank line between commits, in
log-tree.c::do_diff_combined() and log_tree_commit() need to be
cleaned up.

For the header_prefix line, there also is a subtle difference
between the one-commit diff-tree and log.  The former lists
"this commit (from that commit)", which is perhaps useful for
cut and paste purposes.  The latter just says "this commit".

I wish this "diff-tree SHA1 (from ANOTHERSHA1)" format can be
dropped and replaced with "commit SHA1" format like "git log"
does uniformly, but it might be already depended upon by
Porcelains.

If the commit is not a merge, "commit SHA1" means the same thing
as "diff-tree SHA1 (from SHA1^)", and if it _is_ a merge, then
the merge parents are listed on the "Merge: " line anyway for
all formats other than --pretty=oneline, so unless this change
breaks an Porcelain, there is really no downside.

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 23:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161117360.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > Thanks for all your help, but in this case it was not irrelevant. Because 
> > I *had* the function in my working copy. And I had changed it. So I had to 
> > find out where to move the change.
> 
> Right, but it was irrelevant as far as "top-of-head" was concerned (which 
> is all that "git log" shows you - it doesn't care about your working 
> tree).
> 
> The fact that it _had_ been relevant in the state you used to be at is not 
> something "git log" and friends know or care about.
> 
> Now, I'm not disputing that we might want to make it easier to see what 
> _had_ been relevant at some earlier time. But you'd have to specify that 
> earlier time somehow.

Since quite some time, I wanted to have a way to git-rev-list just the 
revs between commit1 and commit2, i.e. all commits which are ancestors of 
commit2, and which have commit1 as ancestor. With this, my task would have 
been more than simple.

> I assume you had tried to do a "git rebase", and the problem with that 
> is that git rebase really doesn't help you at all when things go wrong, 
> exactly because "rebase" - by design - screws up the history and loses 
> that information for you.

Nope. Was a merge.

> If your problem state had been as a result of a "git merge", you'd 
> actually have had much better tools available to you, exactly because 
> merge doesn't screw up history, so you've got both sides of the merge you 
> can look at (HEAD and MERGE_HEAD, and "git diff --ours" and "--theirs").

That outputs too much. I really wanted to find just the commit which 
removed the function, in order to know where the code was moved to.

ORIG_HEAD would not help, because that commit is not an ancestor.

> [...] In particular, you can do
> 
> 	gitk ORIG_HEAD.. 

... and it will say

	Error: expected integer but got ""
	    while executing
	"clock format $d -format "%Y-%m-%d %H:%M:%S ""
	    (procedure "formatdate" line 2)
	    invoked from within
	"formatdate $date"
	    (procedure "drawcmittext" line 28)
	    invoked from within
	"drawcmittext $id $row $col $rmx"
	    (procedure "drawcmitrow" line 41)
	    invoked from within
	"drawcmitrow $row"
	    (procedure "showstuff" line 35)
	    invoked from within
	"showstuff $canshow"
	    (procedure "layoutmore" line 15)
	    invoked from within
	"layoutmore"
	    (procedure "getcommitlines" line 91)
	    invoked from within
	"getcommitlines file7"

Sorry, I am too tired now to investigate, have been working already to 
long. Tomorrow's another day.

Ciao,
Dscho

^ permalink raw reply

* Fixes for option parsing
From: Linus Torvalds @ 2006-04-16 22:17 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Make sure "git show" always show the header, regardless of whether there 
is a diff or not.

Also, make sure "always_show_header" actually works, since generate_header 
only tested it in one out of three return paths.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/git.c b/git.c
index c5de8d3..fc4e429 100644
--- a/git.c
+++ b/git.c
@@ -373,6 +373,7 @@ static int cmd_show(int argc, const char
 	rev.diffopt.recursive = 1;
 	rev.combine_merges = 1;
 	rev.dense_combined_merges = 1;
+	rev.always_show_header = 1;
 	rev.ignore_merges = 0;
 	rev.no_walk = 1;
 	return cmd_log_wc(argc, argv, envp, &rev);
diff --git a/log-tree.c b/log-tree.c
index 7d9f41e..81dff8f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -43,7 +43,7 @@ static int diff_root_tree(struct rev_inf
 	return retval;
 }
 
-static const char *generate_header(struct rev_info *opt,
+static const char *get_header(struct rev_info *opt,
 				   const unsigned char *commit_sha1,
 				   const unsigned char *parent_sha1,
 				   const struct commit *commit)
@@ -75,11 +75,21 @@ static const char *generate_header(struc
 	offset += pretty_print_commit(opt->commit_format, commit, len,
 				      this_header + offset,
 				      sizeof(this_header) - offset, abbrev);
+	return this_header;
+}
+
+static const char *generate_header(struct rev_info *opt,     
+					const unsigned char *commit_sha1,
+					const unsigned char *parent_sha1,
+					const struct commit *commit)
+{
+	const char *header = get_header(opt, commit_sha1, parent_sha1, commit);
+
 	if (opt->always_show_header) {
-		puts(this_header);
-		return NULL;
+		puts(header);
+		header = NULL;
 	}
-	return this_header;
+	return header;
 }
 
 static int do_diff_combined(struct rev_info *opt, struct commit *commit)

^ permalink raw reply related

* Re: [BUG] gitk: breaks with both version and file limits
From: Junio C Hamano @ 2006-04-16 21:51 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git, Paul Mackerras
In-Reply-To: <20060416115403.GS12638@nowhere.earth>

Yann Dirson <ydirson@altern.org> writes:

> As an example, on a current git tree, the following command triggers
> an 'Error: expected boolean value but got ""' dialog when scrolling to
> the bottom of the graph, and leaves the bottom of the graph not drawn.
> This happens with current master, but not with 1.2.4.

A workaround errorproofing would be to do something like this
(note that I do not really speak Tcl).

The real issue is "git-rev-list --boundary ran..ge -- path" does
not show boundary.  I am not sure if it even worked in the past;
will take a look.

-- >8 --
gitk: [lindex $list $no_such_idx] returns "" which is not a valid bool

When asking for commit that is not listed for some reason in
commitlisted, gitk barfed because return value from lindex was
"" which was not a bool acceptable in ($bool ? A : B) construct.

We are expecting "1" here for listed commits (this comes from
getcommitlines), so explicitly check for it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/gitk b/gitk
index f88c06e..5d2b0cc 100755
--- a/gitk
+++ b/gitk
@@ -1460,7 +1460,7 @@ proc drawcmittext {id row col rmx} {
     global linehtag linentag linedtag
     global mainfont namefont canvxmax
 
-    set ofill [expr {[lindex $commitlisted $row]? "blue": "white"}]
+    set ofill [expr {("1" == [lindex $commitlisted $row]) ? "blue" : "white"}]
     set x [xc $row $col]
     set y [yc $row]
     set orad [expr {$linespc / 3}]

^ permalink raw reply related

* [PATCH] reading $GIT_DIR/info/graft - skip comments correctly.
From: Junio C Hamano @ 2006-04-16 21:26 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git
In-Reply-To: <20060416123535.GT12638@nowhere.earth>

Noticed by Yann Dirson.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

  Yann Dirson <ydirson@altern.org> writes:

  > While looking at allowing empty lines in grafts files, I discovered
  > that comments had already been implemented.  However, current
  > git-read-tree segfaults when there is a comment line in info/grafts:
  >
  > dwitch@gandelf:/export/work/yann/git/git$ cat .git/info/grafts 
  > c118c026e44f02c3dbad00d924285eef2340f700
  > # foo
  > dwitch@gandelf:/export/work/yann/git/git$ git-read-tree master
  > Segmentation fault

  Thanks for noticing.

 commit.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

5bc4ce589646faf72c7a77a5d32d9496ccc8d456
diff --git a/commit.c b/commit.c
index ca25574..05c4c92 100644
--- a/commit.c
+++ b/commit.c
@@ -161,7 +161,7 @@ struct commit_graft *read_graft_line(cha
 	if (buf[len-1] == '\n')
 		buf[--len] = 0;
 	if (buf[0] == '#')
-		return 0;
+		return NULL;
 	if ((len + 1) % 41) {
 	bad_graft_data:
 		error("bad graft data: %s", buf);
@@ -192,6 +192,8 @@ int read_graft_file(const char *graft_fi
 		/* The format is just "Commit Parent1 Parent2 ...\n" */
 		int len = strlen(buf);
 		struct commit_graft *graft = read_graft_line(buf, len);
+		if (!graft)
+			continue;
 		if (register_commit_graft(graft, 1))
 			error("duplicate graft data: %s", buf);
 	}
-- 
1.3.0.rc4.g4024

^ permalink raw reply related

* [PATCH] Make cogito work with accented letters.
From: Yann Dirson @ 2006-04-16 20:46 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git


git-diff-index in default mode has an annoying behaviour wrt filenames
containing non-ascii chars.  As suggested by Pasky, we can use -z
mode, which gives us a much better way of handling all other special
chars.  With associated testcases ensuring it works with simple and
double quotes, backslashes, and spaces as well.

This is an improved version of the previous patch, which fixes other
commands like cg-switch which use tree_timewarp.  A couple of other
commands using git-diff-index still need to be fixed in a similar
manner.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 cg-Xlib                 |    7 ++----
 cg-commit               |   10 ++++----
 t/t9900-specialchars.sh |   59 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/cg-Xlib b/cg-Xlib
index 38172a0..e594986 100755
--- a/cg-Xlib
+++ b/cg-Xlib
@@ -555,10 +555,9 @@ tree_timewarp()
 		[ "$no_head_update" ] || git-update-ref HEAD "$branch" || :
 
 		# Kill gone files
-		git-diff-tree -r "$base" "$branch" |
-			# match ":100755 000000 14d43b1abf... 000000000... D"
-			sed -ne 's/^:[^\t]* D\t//p' |
-			xargs rm -f --
+		git-diff-tree -z --name-status -r "$base" "$branch" |
+			perl -n0e 'chomp; if (defined $meta) { print "$_\0" if $meta eq 'D'; $meta=undef } else { $meta = $_ }' |
+			xargs --null rm -f --
 		git-checkout-index -u -f -a
 
 		# FIXME: Can produce bogus "contains only garbage" messages.
diff --git a/cg-commit b/cg-commit
index 8dac57c..1d8de92 100755
--- a/cg-commit
+++ b/cg-commit
@@ -274,8 +274,8 @@ if [ "$ARGS" -o "$_git_relpath" ]; then
 		echo "${_git_relpath}$file" >>"$filter"
 	done
 
-	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index -r -m HEAD -- | \
-		sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index --name-status -z -r -m HEAD -- | \
+		perl -n0e 'chomp; if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = $_ }'))"
 	customfiles=1
 
 	[ "$review" ] && cat "$filter" | path_xargs git-diff-index -r -m -p HEAD -- > "$PATCH"
@@ -292,8 +292,8 @@ else
 	if [ ! "$ignorecache" ]; then
 		# \t instead of the tab character itself works only with new
 		# sed versions.
-		eval "commitfiles=($(git-diff-index -r -m HEAD | \
-			sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+		eval "commitfiles=($(git-diff-index --name-status -z -r -m HEAD | \
+			perl -n0e 'chomp; if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = $_ }'))"
 
 		if [ -s "$_git/commit-ignore" ]; then
 			newcommitfiles=()
@@ -439,7 +439,7 @@ __END__
 		exit 1
 	fi
 	if [ ! "$ignorecache" ] && [ ! "$merging" ] && [ ! "$review" ]; then
-		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed 's/^CG:F *\(.*\)$/"\1"/'))"
+		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed -e 's/\"/\\&/g' -e 's/^CG:F *\(.*\)$/"\1"/'))"
 		if [ ! "$force" ] && [ ! "${newcommitfiles[*]}" ]; then
 			rm "$LOGMSG" "$LOGMSG2"
 			[ "$quiet" ] && exit 0 || die 'Nothing to commit'
diff --git a/t/t9900-specialchars.sh b/t/t9900-specialchars.sh
new file mode 100755
index 0000000..a705052
--- /dev/null
+++ b/t/t9900-specialchars.sh
@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2006 Yann Dirson
+#
+test_description="Tests various commands with shell-special chars.
+
+Filenames with embedded spaces, quotes, non-ascii letter, you name it."
+
+. ./test-lib.sh
+
+rm -rf .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m . "a space"'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m . \"a'quote\""
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "d\"quote"'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "back\\slash"'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m . accént"
+
+## same without a file arg to cg-commit
+
+rm -rf * .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m .'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+test_done

^ permalink raw reply related

* [PATCH] Test that pulls a patch creating a file that got modified afterwards
From: Yann Dirson @ 2006-04-16 20:40 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416203448.10137.69093.stgit@gandelf.nowhere.earth>

From: Yann Dirson <ydirson@altern.org>

This demonstrates an issue wite has bitten me more than once: the stg
branch adds a file in one patch, and modifies it in a later patch; then all
patches get integrated in upstream tree, and at "stg pull" time, stgit
believes there is a conflict, even when the patches are exactly the same.

This is normal as it requires the --merged flag on push or pull.  So
we rollback with "push --undo" and "push --merge" to finish.
---

 t/t1200-push-modified.sh |   64 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
new file mode 100755
index 0000000..7847a38
--- /dev/null
+++ b/t/t1200-push-modified.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Exercise pushing patches applied upstream.
+
+Especially, consider the case of a patch that adds a file, while a
+subsequent one modifies it, so we have to use --merged for push to
+detect the merge.  Reproduce the common workflow where one does not
+specify --merged, then rollback and retry with the correct flag.'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Clone tree and setup changes' \
+    "stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'a\nc\n' > file && stg add file && stg refresh &&
+      stg new p2 -m p2
+      printf 'a\nb\nc\n' > file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull to sync with parent, preparing for the problem' \
+    "(cd bar && stg pop --all &&
+      stg pull
+     )
+"
+
+test_expect_failure \
+    'Attempt to push the first of those patches without --merged' \
+    "(cd bar && stg push
+     )
+"
+
+test_expect_success \
+    'Rollback the push' \
+    "(cd bar && stg push --undo
+     )
+"
+
+test_expect_success \
+    'Push those patches while checking they were merged upstream' \
+    "(cd bar && stg push --merged --all
+     )
+"
+
+test_done

^ permalink raw reply related

* [PATCH] Updated testcases for stgit pull with merge
From: Yann Dirson @ 2006-04-16 20:34 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

[Sorry for the previous buggy post]

In the previous series, patch 7 mostly demonstrate that I have missed
the appearance of the --merged flag in stgit 0.9 push and pull
commands, so we could just amend this testcase with that flag.
However, it appears that common workflow of forgetting that flag, then
trying to undo the failed push, and re-pushing with the flag still shows
a problem, as hopefully shown by this updated testcase.

As for the testcase in patch 8, it still demonstrates a problem IMHO:
the push without the --merged flag should have failed in much the same
way this one does.  However, I have seen GNU patch showing a similar
behaviour, so it is maybe not stgit's fault.

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* [PATCH 0/2] Updated testcases for stgit pull with merge
From: Yann Dirson @ 2006-04-16 20:31 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

In the previous series, patch 7 mostly demonstrate that I have
missed the appearance of the --merged flag in stgit 0.9 push and pull
commands.

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 18:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604162006050.19560@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> There's another reason it does not show it. ATM, you have to add "-p" to 
> the command line, or "-c" will not show *any* patch, let alone a combined 
> one.

Oh, it will show the "raw" patch, which is often very useful. I've grown 
quite fond of it, because it shows what happened on a bigger level, 
without showing the details within a file ("--name-status" makes it more 
readable, but I'm too lazy to type it, so I often just look at the raw 
git patch).

> Thanks for all your help, but in this case it was not irrelevant. Because 
> I *had* the function in my working copy. And I had changed it. So I had to 
> find out where to move the change.

Right, but it was irrelevant as far as "top-of-head" was concerned (which 
is all that "git log" shows you - it doesn't care about your working 
tree).

The fact that it _had_ been relevant in the state you used to be at is not 
something "git log" and friends know or care about.

Now, I'm not disputing that we might want to make it easier to see what 
_had_ been relevant at some earlier time. But you'd have to specify that 
earlier time somehow. I assume you had tried to do a "git rebase", and the 
problem with that is that git rebase really doesn't help you at all when 
things go wrong, exactly because "rebase" - by design - screws up the 
history and loses that information for you.

If your problem state had been as a result of a "git merge", you'd 
actually have had much better tools available to you, exactly because 
merge doesn't screw up history, so you've got both sides of the merge you 
can look at (HEAD and MERGE_HEAD, and "git diff --ours" and "--theirs").

That said, even "rebase" will help somewhat. You've got "ORIG_HEAD" to 
use, and that should help at least a _bit_. In particular, you can do

	gitk ORIG_HEAD.. 

to see all the changes you rebased across. But right now you'd have to 
fall back on the "-m -p" thing if you wanted to see it all..

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 18:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161052310.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > I finally found the commit which removed parse_whatchanged_opt() from 
> > log-tree.c by
> > 
> > 	git-rev-list 4a617..next | while read commit; do \
> > 		echo $commit; \
> > 		git diff $commit^..$commit log-tree.c | \
> > 			grep parse_whatchanged; \
> > 	done | less
> 
> Heh. You really should learn about "-m -p", which does the above, but 
> better (it compares against _all_ parents - you would have missed the 
> thing _again_ if the "lt/logopt" branch had been the main branch ;)

Somehow I forgot. Probably because I don't like the format either.

> [...] even
> 
> 	git show -c 43f934aa90
> 
> won't show that "log-tree.c" file AT ALL, because there was no content 
> conflict: the whole file was taken from one branch, unmodified. 

There's another reason it does not show it. ATM, you have to add "-p" to 
the command line, or "-c" will not show *any* patch, let alone a combined 
one.

> So "--cc" really does ignore everything that is irrelevant for the end 
> result, and in this case you are very much trying to find somethign that 
> is totally irrelevant for the end result, since the function you look for 
> had never even _existed_ in the file as far as the end result goes..

Thanks for all your help, but in this case it was not irrelevant. Because 
I *had* the function in my working copy. And I had changed it. So I had to 
find out where to move the change.

Again, thanks a bunch,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161931530.19020@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> I finally found the commit which removed parse_whatchanged_opt() from 
> log-tree.c by
> 
> 	git-rev-list 4a617..next | while read commit; do \
> 		echo $commit; \
> 		git diff $commit^..$commit log-tree.c | \
> 			grep parse_whatchanged; \
> 	done | less

Heh. You really should learn about "-m -p", which does the above, but 
better (it compares against _all_ parents - you would have missed the 
thing _again_ if the "lt/logopt" branch had been the main branch ;)

> However, the combined diff of that commit does not show it, while the diff 
> to the first parent does:
> 
> 	git-show --cc 43f934aa90 | grep parse_whatchanged

Combined merges really only show conflicts where the different parents do 
something different from the end result. Since the whole file was taken 
from the lt/logopt branch, even

	git show -c 43f934aa90

won't show that "log-tree.c" file AT ALL, because there was no content 
conflict: the whole file was taken from one branch, unmodified. 

If you want to see all changes against all parents, you really do need 
"-m -p" (or just "-m", which will show the raw diffs, and which will 
show how the file changes from one parent, but not the other).

Note that NORMALLY, you'd really never want to use "-m -p". It's a very 
very inconvenient format, since normally you want to see only the stuff 
that changed wrt the end result.

So "--cc" really does ignore everything that is irrelevant for the end 
result, and in this case you are very much trying to find somethign that 
is totally irrelevant for the end result, since the function you look for 
had never even _existed_ in the file as far as the end result goes..

		Linus

^ permalink raw reply

* [PATCH] Make cg-commit work with accented letters.
From: Yann Dirson @ 2006-04-16 17:57 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git


git-diff-index in default mode has an annoying behaviour wrt filenames
containing non-ascii chars.  As suggested by Pasky, we can use -z
mode, which gives us a much better way of handling all other special
chars.  With associated testcases ensuring it works with simple and
double quotes, backslashes, and spaces as well.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 cg-commit               |   10 ++++----
 t/t9900-specialchars.sh |   59 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/cg-commit b/cg-commit
index 8dac57c..9ec8289 100755
--- a/cg-commit
+++ b/cg-commit
@@ -274,8 +274,8 @@ if [ "$ARGS" -o "$_git_relpath" ]; then
 		echo "${_git_relpath}$file" >>"$filter"
 	done
 
-	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index -r -m HEAD -- | \
-		sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index -z -r -m HEAD -- | \
+		perl -n0e 'if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = (split(/\s/))[4] }'))"
 	customfiles=1
 
 	[ "$review" ] && cat "$filter" | path_xargs git-diff-index -r -m -p HEAD -- > "$PATCH"
@@ -292,8 +292,8 @@ else
 	if [ ! "$ignorecache" ]; then
 		# \t instead of the tab character itself works only with new
 		# sed versions.
-		eval "commitfiles=($(git-diff-index -r -m HEAD | \
-			sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+		eval "commitfiles=($(git-diff-index -z -r -m HEAD | \
+			perl -n0e 'if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = (split(/\s/))[4] }'))"
 
 		if [ -s "$_git/commit-ignore" ]; then
 			newcommitfiles=()
@@ -439,7 +439,7 @@ __END__
 		exit 1
 	fi
 	if [ ! "$ignorecache" ] && [ ! "$merging" ] && [ ! "$review" ]; then
-		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed 's/^CG:F *\(.*\)$/"\1"/'))"
+		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed -e 's/\"/\\&/g' -e 's/^CG:F *\(.*\)$/"\1"/'))"
 		if [ ! "$force" ] && [ ! "${newcommitfiles[*]}" ]; then
 			rm "$LOGMSG" "$LOGMSG2"
 			[ "$quiet" ] && exit 0 || die 'Nothing to commit'
diff --git a/t/t9900-specialchars.sh b/t/t9900-specialchars.sh
new file mode 100755
index 0000000..a705052
--- /dev/null
+++ b/t/t9900-specialchars.sh
@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2006 Yann Dirson
+#
+test_description="Tests various commands with shell-special chars.
+
+Filenames with embedded spaces, quotes, non-ascii letter, you name it."
+
+. ./test-lib.sh
+
+rm -rf .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m . "a space"'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m . \"a'quote\""
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "d\"quote"'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "back\\slash"'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m . accént"
+
+## same without a file arg to cg-commit
+
+rm -rf * .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m .'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+test_done

^ permalink raw reply related

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161905010.18184@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> just to make it clearer what I want:
> 
> 	git-whatchanged -p next | grep parse_whatchanged
> 
> as well as
> 
> 	git log -p next | grep parse_whatchanged
> 
> do not find that any line like
> 
> 	int parse_whatchanged_opt(int ac, [...]
> 
> was removed, but they find that this line was added. However, in the 
> working tree (which has a fresh checkout of next), there is no such line 
> in log-tree.c. So I really would like to know where it vanished!

It was removed by merge 43f934aa: "Merge branch 'lt/logopt' into next".

The "parse_whatchanged_opt()" function never existed in that lt/logopt 
branch, and merging it (manually, I assume) it doesn't exist in the result 
either.

General hint: if you don't find it in "--cc", then that means that it's 
almost certainly a merge. "--cc" will only find things that _clash_, ie 
the end result is different from either of the branches (and in this case, 
it wasn't different, since parse_whatchanged_opt() simply didn't exist in 
the branch that was merged).

Now, finding things in merges can be a bit painful, but the sure-fire safe 
way is the old one: use "-m -p" to show _all_ sides of a merge as a diff. 
That's a really inconvenient format for reading, but it literally shows 
all changes to all parents.

Again, "git log log-tree.c" actually does do the right thing: the current 
state of "log-tree.c" really has _all_ of its history coming from the 
lt/logopt branch, which is why when you do

	git log -m -p log-tree.c | grep int parse_whatchanged_opt

you won't get any result at all: the _current_ state of log-tree.c really 
has no history what-so-ever that involved parse_whatchanged_opt. That may 
sound strange, but it really is very true. Doing a "gitk log-tree.c" shows 
what the real history of the contents of that file is.

And this actually comes back to a very fundamental git issue: git tracks 
_contents_. It doesn't care one whit if there was another branch that had 
some other history for that file: if that other branch didn't affect the 
contents of the file, then that other branch simply doesn't exist as far 
as that particular file history is concerned. It only exists as a "bigger" 
issue.

But that "-m -p" thing can be useful when you do want to see the bigger 
issue. As might "--no-prune-merges".

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 17:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161000550.3701@g5.osdl.org>

Hi,

me again.

I finally found the commit which removed parse_whatchanged_opt() from 
log-tree.c by

	git-rev-list 4a617..next | while read commit; do \
		echo $commit; \
		git diff $commit^..$commit log-tree.c | \
			grep parse_whatchanged; \
	done | less

where obviously 4a617 is the commit I last merged with. The winner is: 

	43f934aa90... Merge branch 'lt/logopt' into next

However, the combined diff of that commit does not show it, while the diff 
to the first parent does:

	git-show --cc 43f934aa90 | grep parse_whatchanged

shows nothing (-c -p shows half of the truth), while

	git-diff 43f934aa90^..43f934aa90 | grep parse_whatchanged

shows something, and 

	git-diff 43f934aa90^2..43f934aa90 | grep parse_whatchanged

again does not.

Time to understand the combined diff logic, I guess...

Ciao,
Dscho

^ permalink raw reply

* Re: path limiting broken (NOT)
From: Johannes Schindelin @ 2006-04-16 17:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161000550.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > But you missed my point. The file log-tree.c had some opt handling. I 
> > changed it (as can be seen in my patch for combined diffstat), so the 
> > merge did not go well with next, which removed that code.
> > 
> > So I tried to be clever and find the (non-merge) commit where this 
> > happened. With what you describe, I should have hit *exactly one* commit 
> > removing the code.
> > 
> > But I hit *exactly none* with "git log --cc master log-tree.c".
> 
> Do you have the tree somewhere?

I have no quick way to publish it, if you mean that.

> In the current git tree, there really _is_ just one commit that touches 
> log-tree.c in "master". And "git log log-tree.c" picks that out without 
> trouble.

It is my master. Not Junio's.

The problem seems to be that the current next does not have the code. But 
when I last merged, it had it. So basically, I have a problem here that I 
thought "git log --cc" can solve, but it doesn't.

"git log -cc" is good to find out when code *that is present now* has 
crept in, but it is no good to find out when code that was in commit 
such-and-such, and is no longer in commit such-and-that, has been culled.

Pity.

Thanks for your help,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161835410.17985@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> But you missed my point. The file log-tree.c had some opt handling. I 
> changed it (as can be seen in my patch for combined diffstat), so the 
> merge did not go well with next, which removed that code.
> 
> So I tried to be clever and find the (non-merge) commit where this 
> happened. With what you describe, I should have hit *exactly one* commit 
> removing the code.
> 
> But I hit *exactly none* with "git log --cc master log-tree.c".

Do you have the tree somewhere?

In the current git tree, there really _is_ just one commit that touches 
log-tree.c in "master". And "git log log-tree.c" picks that out without 
trouble.

Did you mean to do

	git log --cc next log-tree.c

instead, perhaps? (Note the "next" branch specifier) That definitely shows 
three commits, including very much the one that moves the common option 
parsing to revision.c (do "git log --cc --full-diff next log-tree.c" if 
you want to see the whole diff whenever something touches log-tree.c).

Maybe I'm still missing something, but it seems to do the right thing 
for me..

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 17:05 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604160850230.3701@g5.osdl.org>

Hi,

just to make it clearer what I want:

	git-whatchanged -p next | grep parse_whatchanged

as well as

	git log -p next | grep parse_whatchanged

do not find that any line like

	int parse_whatchanged_opt(int ac, [...]

was removed, but they find that this line was added. However, in the 
working tree (which has a fresh checkout of next), there is no such line 
in log-tree.c. So I really would like to know where it vanished!

Ciao,
Dscho

^ 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