threads / discuss / 3740

What's in git.git

Subject: What's in git.git

## tl;dr

33 messages between Mar 28, 2006 and Mar 31, 2006.

replies: 32people: 8as markdown or json

Junio C Hamano· Mar 28, 2006, 00:28 UTC · lore
GIT 1.3.0-rc1 is pushed out and will be mirrored out soon.

All of the things that were not in the "master" branch were either cooked long enough in "next" without causing problems (e.g. insanely fast rename detector or true built-in diff) or isolated in a specific subsystem (e.g. tar-tree and svnimport).

So I am clearing the deck to prepare for a 1.3.0. Remaining wrinkles, if any, will be ironed out in the "master" branch.

------------ Changes since the last announcement:

 - updates around git-clone:
   . --use-separate-remote
   . --reference <repo>
   . fetch,parse-remote,fmt-merge-msg: refs/remotes/* support (Eric Wong)
   . sha1_name() understands refs/remotes/$foo/HEAD
 - sha1_name safety and core.warnambiguousrefs
 - git-merge knows some strategies want to skip trivial merges
 - insanely fast rename detection (Linus and me)
 - tar-tree updates (Rene Scharfe)
 - send-email updates (Eric Wong)
 - truly built-in diff (Linus with Davide)
 - ls-{files,tree} --abbrev (Eric Wong)
 - git-svnimport: if a limit is specified, respect it (Anand Kumria)
 - documentation (J. Bruce Fields)
 - build fix (Johannes Schindelin)
 - git-ls-files --others --directory --no-empty-directory (Petr Baudis)
 - gitk updates (Martin Mares, Paul Mackerras)
 - GIT 1.3.0 rc1 (me)
Currently "next" and "pu" are empty.
Jason Riedy· Mar 28, 2006, 01:15 UTC · re: Junio C Hamano · lore

[PATCH] Add ALL_LDFLAGS to the git target.

For some reason, I need ALL_LDFLAGS in the git target only on AIX. Once it builds, only one test "fails" on AIX 5.1 with 1.3.0.rc1, t5500-fetch-pack.sh, but it looks like it's some odd tool problem in the tester + my setup and not a real bug.

Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
diff --git a/Makefile b/Makefile
index 4edb383..d945546 100644
--- a/Makefile
+++ b/Makefile
@@ -455,7 +455,8 @@ strip: $(PROGRAMS) git$X
 
 git$X: git.c common-cmds.h $(LIB_FILE)
 	$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
-		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) $(LIBS)
+		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) \
+		$(ALL_LDFLAGS) $(LIBS)
 
 common-cmds.h: Documentation/git-*.txt
 	./generate-cmdlist.sh > $@
Junio C Hamano· Mar 28, 2006, 01:25 UTC · re: Jason Riedy · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
> For some reason, I need ALL_LDFLAGS in the git target only on
> AIX.

I wonder what the dependency is, since ALL_LDFLAGS is not modified on AIX, but you are right. That is the only binary that does not link with ALL_LDFLAGS which can include whatever user passes via LDFLAGS.

> Once it builds, only one test "fails" on AIX 5.1 with 
> 1.3.0.rc1, t5500-fetch-pack.sh, but it looks like it's some
> odd tool problem in the tester + my setup and not a real bug.
Curious and would appreciate more details.
Jason Riedy· Mar 28, 2006, 03:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

And Junio C Hamano writes:
 - I wonder what the dependency is, since ALL_LDFLAGS is not
 - modified on AIX, [...]

Specifically, -lcrypto. Mine is in a funny place, so I need LDFLAGS passed in.

 - > Once it builds, only one test "fails" on AIX 5.1 with 
 - > 1.3.0.rc1, t5500-fetch-pack.sh, but it looks like it's some
 - > odd tool problem in the tester + my setup and not a real bug.
 - 
 - Curious and would appreciate more details.

I just found it. The progress meter stuff in pack-objects splats all over the output. So trash/client/log.txt is completely mangled. Everything functions correctly, but the textual output is garbage. If I set progress to 0 in pack-objects.c, everthing's happy.

There's no way to pass -q through fetch-pack to upload-pack... Gee, look, a comment that says "Yeah, yeah, fixme." I have no real desire to add an args argument and propagate that change through all the connect routines. An alternative is to add a "quiet" command to the protocol. Another would be to dup all three file descriptors. yech. Preference?

(I haven't updated git in a while on this platform. Recompiling and testing takes a while on a 375 MHz Power3.)

Jason
Junio C Hamano· Mar 28, 2006, 06:21 UTC · re: Jason Riedy · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
Show 6 quoted lines
> And Junio C Hamano writes:
>  - I wonder what the dependency is, since ALL_LDFLAGS is not
>  - modified on AIX, [...]
>
> Specifically, -lcrypto.  Mine is in a funny place, so I need
> LDFLAGS passed in.
Thanks.  That is the right fix, then.
Show 11 quoted lines
>  - > Once it builds, only one test "fails" on AIX 5.1 with 
>  - > 1.3.0.rc1, t5500-fetch-pack.sh, but it looks like it's some
>  - > odd tool problem in the tester + my setup and not a real bug.
>  - 
>  - Curious and would appreciate more details.
>
> I just found it.  The progress meter stuff in pack-objects
> splats all over the output.  So trash/client/log.txt is
> completely mangled.  Everything functions correctly, but
> the textual output is garbage.  If I set progress to 0 in 
> pack-objects.c, everthing's happy.

Hmph. We do fprintf(stderr, "blah\r") to draw them. The standard says that "standard error stream is not fully buffered", but I guess it does not necessarily mean it is unbuffered, so we probably need to fflush(3) there. Would something like this help?

-- >8 --
diff --git a/fetch-clone.c b/fetch-clone.c
index da1b3ff..252e5ec 100644
--- a/fetch-clone.c
+++ b/fetch-clone.c
@@ -230,6 +230,7 @@
 					total >> 20,
 					1000*((total >> 10) & 1023)>>10,
 					avg_bytes / avg_time );
+				fflush(stderr);
 			}
 		}
 	}
diff --git a/imap-send.c b/imap-send.c
index e33c78b..dcfa8d8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1345,6 +1345,7 @@
 	while (1) {
 		unsigned percent = n * 100 / total;
 		fprintf( stderr, "%4u%% (%d/%d) done\r", percent, n, total );
+		fflush(stderr);
 		if (!split_msg( &all_msgs, &msg, &ofs ))
 			break;
 		r = imap_store_msg( ctx, &msg, &uid );
diff --git a/pack-objects.c b/pack-objects.c
index 49357c6..7c85348 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -360,6 +360,7 @@
 			if (progress_update || percent != last_percent) {
 				fprintf(stderr, "%4u%% (%u/%u) done\r",
 					percent, written, nr_result);
+				fflush(stderr);
 				progress_update = 0;
 				last_percent = percent;
 			}
@@ -570,6 +571,7 @@
  already_added:
 	if (progress_update) {
 		fprintf(stderr, "Counting objects...%d\r", nr_objects);
+		fflush(stderr);
 		progress_update = 0;
 	}
 	if (exclude)
@@ -912,6 +914,7 @@
 			if (percent != last_percent || progress_update) {
 				fprintf(stderr, "%4u%% (%u/%u) done\r",
 					percent, processed, nr_result);
+				fflush(stderr);
 				progress_update = 0;
 				last_percent = percent;
 			}
diff --git a/read-tree.c b/read-tree.c
index eaff444..6a2aa16 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -325,6 +325,7 @@
 				    progress_update) {
 					fprintf(stderr, "%4u%% (%u/%u) done\r",
 						percent, cnt, total);
+					fflush(stderr);
 					last_percent = percent;
 				}
 			}
diff --git a/unpack-objects.c b/unpack-objects.c
index 815a1b3..8596f9b 100644
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -220,6 +220,7 @@
 			last_sec = now.tv_sec;
 			last_percent = percentage;
 			fprintf(stderr, "%4u%% (%u/%u) done\r", percentage, nr, total);
+			fflush(stderr);
 		}
 	}
 	switch (type) {
Jason Riedy· Mar 28, 2006, 19:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

And Junio C Hamano writes:
 - Hmph.  We do fprintf(stderr, "blah\r") to draw them.  The
 - standard says that "standard error stream is not fully
 - buffered", but I guess it does not necessarily mean it is
 - unbuffered, so we probably need to fflush(3) there.  Would
 - something like this help?

I suppose I should have mentioned that I tried flushing stderr. Your more comprehensive flushing also does not fix it, giving outputs like:

> Unpacking Total 3333 objects
> , written 33 (delta 1), reused 0 (delta 0)

The problem is that stderr from a child is not tied to any stream of its parent. Generally, as far as I know, you cannot make any assumptions about how pipes from separate processes are interleaved in the output. Some standard may say something, but I have no idea what or if anyone listens. And this particular system is a busy SMP node, making the problem worse.

Line-buffered streams like stdout tend to work, but not unbuffered streams like stderr. We can't make stderr line- buffered without breaking the status indicator...

If I add a third fd to all the pipes and dup it to stderr, the tests work. I never read from that fd, so I never get the status output... Progress needs to be part of the protocol so front ends can handle it cleanly rather than using stderr tricks.

So some possibilities:
  1) Add the ability to pass options through the whole
     connect system.  Then pass -q in the tester.
  2) Add a specific "quiet" command to the protocol for
     just passing -q from git-fetch-pack.  Pass -q in the 
     tester.
  3) Add an option to pack-objects that dumps progress
     output to stdout in a special packet format.  Then
     update everyone who talks through upload-pack to
     expect another phase of informational messages after
     negotiating object differences and before the pack
     data.

The first two are cosmetic fixes only, and #2 is a cheap, ugly, but easy hack.

This problem is (to me) low priority. It unfortunately breaks a test case on AIX, but I can live with it for now. If others here start to listen to the gospel of git, well, I'll need to fix it. (But I once recommended Arch, and people stopped listening after they tried it.)

Folks using moderately-loaded SMPs may experience similar problems. But if they're fetching large packs, the problem likely won't appear at all.

Jason

P.S. For the whole finding-a-function-name business, some of us are using git on fixed-format Fortran. Every non-comment line begins with whitespace... ;) And in free format, many people don't add that first indentation within subroutines.

Mark Wooding· Mar 28, 2006, 22:48 UTC · re: Jason Riedy · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Jason Riedy <ejr@EECS.Berkeley.EDU> wrote:
> P.S. For the whole finding-a-function-name business, some of 
> us are using git on fixed-format Fortran.  Every non-comment
> line begins with whitespace...  ;)  And in free format, many
> people don't add that first indentation within subroutines.

Urgh. So, which regex library do people want to use? ;-) (My vote's for pcre.)

-- [mdw]
Linus Torvalds· Mar 28, 2006, 23:03 UTC · re: Mark Wooding · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

On Tue, 28 Mar 2006, Mark Wooding wrote:
Show 9 quoted lines
> Jason Riedy <ejr@EECS.Berkeley.EDU> wrote:
> 
> > P.S. For the whole finding-a-function-name business, some of 
> > us are using git on fixed-format Fortran.  Every non-comment
> > line begins with whitespace...  ;)  And in free format, many
> > people don't add that first indentation within subroutines.
> 
> Urgh.  So, which regex library do people want to use? ;-)  (My vote's
> for pcre.)

I'd really just prefer to make the "-p" switch configurable, the way it was before. No regexps, just the same rules as for GNU diff, perhaps with the difference being that it would be on by default.

Another possible approach is to say
 - if the first line of the real diff matches the rules, do NOT add 
   another line that matches the rule at the @@-line.

since the simple @@-line rule really doesn't make sense for any file that is "dense" (ie where most lines start with non-whitespace).

		Linus
Junio C Hamano· Mar 28, 2006, 23:20 UTC · re: Linus Torvalds · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Linus Torvalds <torvalds@osdl.org> writes:
Show 15 quoted lines
> On Tue, 28 Mar 2006, Mark Wooding wrote:
>
>> Jason Riedy <ejr@EECS.Berkeley.EDU> wrote:
>> 
>> > P.S. For the whole finding-a-function-name business, some of 
>> > us are using git on fixed-format Fortran.  Every non-comment
>> > line begins with whitespace...  ;)  And in free format, many
>> > people don't add that first indentation within subroutines.
>> 
>> Urgh.  So, which regex library do people want to use? ;-)  (My vote's
>> for pcre.)
>
> I'd really just prefer to make the "-p" switch configurable, the way it 
> was before. No regexps, just the same rules as for GNU diff, perhaps with 
> the difference being that it would be on by default.

Strictly speaking, "No regexps" and "same rules as for GNU diff" are mutually incompatible, since GNU diff -p defaults to "^[[:alpha:]$_]" but the regexp is configurable.

My preference is to ignore FORTRAN, keep Mark's current rules, perhaps with a way to turn it off if people really find it annoying (I do not mind having it always on).

Show 6 quoted lines
> Another possible approach is to say
>  - if the first line of the real diff matches the rules, do NOT add 
>    another line that matches the rule at the @@-line.
>
> since the simple @@-line rule really doesn't make sense for any file that 
> is "dense" (ie where most lines start with non-whitespace).

I think this is a good rule. If "the first non-empty line" may be even better; we do not want to see the name of previous function for a huke like this:

	@@ -a,b +c,d @@
        int frotz(void)
        {
            ...
Jason Riedy· Mar 28, 2006, 23:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

And Junio C Hamano writes:
 - My preference is to ignore FORTRAN, keep Mark's current rules,
 - perhaps with a way to turn it off if people really find it
 - annoying (I do not mind having it always on).

Sorry; I had meant my comment as an aside and not a request. I had never noticed the function definition in patches, and now I typically use Emacs's tools.

And as of Fortran 90, it's now officially Fortran and not FORTRAN.

Jason
Junio C Hamano· Mar 29, 2006, 00:01 UTC · re: Jason Riedy · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
> And Junio C Hamano writes:
>  - My preference is to ignore FORTRAN, keep Mark's current rules,
>  - perhaps with a way to turn it off if people really find it
>  - annoying (I do not mind having it always on).
Sorry I forgot to add smiley to the above ;-).
Mark Wooding· Mar 28, 2006, 23:21 UTC · re: Linus Torvalds · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Linus Torvalds <torvalds@osdl.org> wrote:
> I'd really just prefer to make the "-p" switch configurable, the way
> it was before. No regexps, just the same rules as for GNU diff,

The rules for GNU diff aren't actually good enough if you can't configure them. We used to be able to put runes in GIT_DIFF_OPTS.

> perhaps with the difference being that it would be on by default.
I thought it /was/ on by default:
: static const char *diff_opts = "-pu";
(killed in cebff98db).
Show 6 quoted lines
> Another possible approach is to say
>  - if the first line of the real diff matches the rules, do NOT add 
>    another line that matches the rule at the @@-line.
>
> since the simple @@-line rule really doesn't make sense for any file that 
> is "dense" (ie where most lines start with non-whitespace).
It's true, and that's an easy fix.  But it doesn't do any actual harm.
-- [mdw]
Petr Baudis· Mar 29, 2006, 00:16 UTC · re: Linus Torvalds · lore

[PATCH] Support for pickaxe matching regular expressions

Dear diary, on Wed, Mar 29, 2006 at 01:03:05AM CEST, I got a letter where Linus Torvalds <torvalds@osdl.org> said that...

Show 5 quoted lines
> On Tue, 28 Mar 2006, Mark Wooding wrote:
> > Urgh.  So, which regex library do people want to use? ;-)  (My vote's
> > for pcre.)
> 
> ... No regexps, ...

To toss a random feature idea around, in the recent days I've found myself thinking about regexp pickaxe several times.

And while already tossing stuff, what about a naive proof-of-concept patch? A silly example:

	git-whatchanged --pickaxe-regex -p -S' +$' | less -p '^[-+ ].* +$'

Then keep hitting 'n'. Good that most of the matches are deletions. :) (Or commit messages.)

---

git-diff-* --pickaxe-regex will change the -S pickaxe to match POSIX extended regular expressions instead of fixed strings.

The regex.h library is a rather stupid interface and I like pcre too, but with any luck it will be everywhere we will want to run Git on, it being POSIX.2 and all. I'm not sure if we can expect platforms like AIX to conform to POSIX.2 or if win32 has regex.h. We might add a flag to Makefile if there is a portability trouble potential.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---
 Documentation/diff-options.txt |    4 ++
 diff.c                         |    2 +
 diff.h                         |    1 +
 diffcore-pickaxe.c             |   68 ++++++++++++++++++++++++++++++----------
 4 files changed, 58 insertions(+), 17 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 2a0275e..ec6811c 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -69,6 +69,10 @@
 	changeset, not just the files that contain the change
 	in <string>.
 
+--pickaxe-regex::
+	Make the <string> not a plain string but an extended POSIX
+	regex to match.
+
 -O<orderfile>::
 	Output the patch in the order specified in the
 	<orderfile>, which has one shell glob pattern per line.
diff --git a/diff.c b/diff.c
index 8b37477..e006adb 100644
--- a/diff.c
+++ b/diff.c
@@ -883,6 +883,8 @@ int diff_opt_parse(struct diff_options *
 		options->filter = arg + 14;
 	else if (!strcmp(arg, "--pickaxe-all"))
 		options->pickaxe_opts = DIFF_PICKAXE_ALL;
+	else if (!strcmp(arg, "--pickaxe-regex"))
+		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
 	else if (!strncmp(arg, "-B", 2)) {
 		if ((options->break_opt =
 		     diff_scoreopt_parse(arg)) == -1)
diff --git a/diff.h b/diff.h
index 8fac465..564c94f 100644
--- a/diff.h
+++ b/diff.h
@@ -112,6 +112,7 @@ #define DIFF_DETECT_RENAME	1
 #define DIFF_DETECT_COPY	2
 
 #define DIFF_PICKAXE_ALL	1
+#define DIFF_PICKAXE_REGEX	2
 
 extern void diffcore_std(struct diff_options *);
 
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 50e46ab..d89f314 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -1,12 +1,15 @@
 /*
  * Copyright (C) 2005 Junio C Hamano
  */
+#include <regex.h>
+
 #include "cache.h"
 #include "diff.h"
 #include "diffcore.h"
 
 static unsigned int contains(struct diff_filespec *one,
-			     const char *needle, unsigned long len)
+			     const char *needle, unsigned long len,
+			     regex_t *regexp)
 {
 	unsigned int cnt;
 	unsigned long offset, sz;
@@ -17,15 +20,28 @@ static unsigned int contains(struct diff
 	sz = one->size;
 	data = one->data;
 	cnt = 0;
-
-	/* Yes, I've heard of strstr(), but the thing is *data may
-	 * not be NUL terminated.  Sue me.
-	 */
-	for (offset = 0; offset + len <= sz; offset++) {
-		/* we count non-overlapping occurrences of needle */
-		if (!memcmp(needle, data + offset, len)) {
-			offset += len - 1;
+
+	if (regexp) {
+		regmatch_t regmatch;
+		int flags = 0;
+
+		while (*data && !regexec(regexp, data, 1, &regmatch, flags)) {
+			flags |= REG_NOTBOL;
+			data += regmatch.rm_so;
+			if (*data) data++;
 			cnt++;
+		}
+
+	} else { /* Classic exact string match */
+		/* Yes, I've heard of strstr(), but the thing is *data may
+		 * not be NUL terminated.  Sue me.
+		 */
+		for (offset = 0; offset + len <= sz; offset++) {
+			/* we count non-overlapping occurrences of needle */
+			if (!memcmp(needle, data + offset, len)) {
+				offset += len - 1;
+				cnt++;
+			}
 		}
 	}
 	return cnt;
@@ -36,10 +52,24 @@ void diffcore_pickaxe(const char *needle
 	struct diff_queue_struct *q = &diff_queued_diff;
 	unsigned long len = strlen(needle);
 	int i, has_changes;
+	regex_t regex, *regexp = NULL;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
 
+	if (opts & DIFF_PICKAXE_REGEX) {
+		int err;
+		err = regcomp(&regex, needle, REG_EXTENDED | REG_NEWLINE);
+		if (err) {
+			/* The POSIX.2 people are surely sick */
+			char errbuf[1024];
+			regerror(err, &regex, errbuf, 1024);
+			regfree(&regex);
+			die("invalid pickaxe regex: %s", errbuf);
+		}
+		regexp = &regex;
+	}
+
 	if (opts & DIFF_PICKAXE_ALL) {
 		/* Showing the whole changeset if needle exists */
 		for (i = has_changes = 0; !has_changes && i < q->nr; i++) {
@@ -48,16 +78,16 @@ void diffcore_pickaxe(const char *needle
 				if (!DIFF_FILE_VALID(p->two))
 					continue; /* ignore unmerged */
 				/* created */
-				if (contains(p->two, needle, len))
+				if (contains(p->two, needle, len, regexp))
 					has_changes++;
 			}
 			else if (!DIFF_FILE_VALID(p->two)) {
-				if (contains(p->one, needle, len))
+				if (contains(p->one, needle, len, regexp))
 					has_changes++;
 			}
 			else if (!diff_unmodified_pair(p) &&
-				 contains(p->one, needle, len) !=
-				 contains(p->two, needle, len))
+				 contains(p->one, needle, len, regexp) !=
+				 contains(p->two, needle, len, regexp))
 				has_changes++;
 		}
 		if (has_changes)
@@ -80,16 +110,16 @@ void diffcore_pickaxe(const char *needle
 				if (!DIFF_FILE_VALID(p->two))
 					; /* ignore unmerged */
 				/* created */
-				else if (contains(p->two, needle, len))
+				else if (contains(p->two, needle, len, regexp))
 					has_changes = 1;
 			}
 			else if (!DIFF_FILE_VALID(p->two)) {
-				if (contains(p->one, needle, len))
+				if (contains(p->one, needle, len, regexp))
 					has_changes = 1;
 			}
 			else if (!diff_unmodified_pair(p) &&
-				 contains(p->one, needle, len) !=
-				 contains(p->two, needle, len))
+				 contains(p->one, needle, len, regexp) !=
+				 contains(p->two, needle, len, regexp))
 				has_changes = 1;
 
 			if (has_changes)
@@ -97,6 +127,10 @@ void diffcore_pickaxe(const char *needle
 			else
 				diff_free_filepair(p);
 		}
+
+	if (opts & DIFF_PICKAXE_REGEX) {
+		regfree(&regex);
+	}
 
 	free(q->queue);
 	*q = outq;
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.
Johannes Schindelin· Mar 29, 2006, 11:42 UTC · re: Mark Wooding · lore

Re: [PATCH] Add ALL_LDFLAGS to the git target.

Hi,
On Tue, 28 Mar 2006, Mark Wooding wrote:
Show 9 quoted lines
> Jason Riedy <ejr@EECS.Berkeley.EDU> wrote:
> 
> > P.S. For the whole finding-a-function-name business, some of 
> > us are using git on fixed-format Fortran.  Every non-comment
> > line begins with whitespace...  ;)  And in free format, many
> > people don't add that first indentation within subroutines.
> 
> Urgh.  So, which regex library do people want to use? ;-)  (My vote's
> for pcre.)

My vote is against adding such a dependency for so little gain. We already use regex.h (probably my fault).

Ciao, Dscho

Petr Baudis· Mar 29, 2006, 13:09 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Support for pickaxe matching regular expressions

Dear diary, on Wed, Mar 29, 2006 at 02:16:33AM CEST, I got a letter where Petr Baudis <pasky@suse.cz> said that...

Show 5 quoted lines
> The regex.h library is a rather stupid interface and I like pcre too, but
> with any luck it will be everywhere we will want to run Git on, it being
> POSIX.2 and all. I'm not sure if we can expect platforms like AIX to
> conform to POSIX.2 or if win32 has regex.h. We might add a flag to
> Makefile if there is a portability trouble potential.

Dear diary, on Wed, Mar 29, 2006 at 01:42:00PM CEST, I got a letter where Johannes Schindelin <Johannes.Schindelin@gmx.de> said that...

> We already use regex.h (probably my fault).

Indeed, and since noone complained yet, the portability consideration is apparently a non-issue.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.
Linus Torvalds· Mar 28, 2006, 02:05 UTC · re: Junio C Hamano · lore

Gitk strangeness..

On Mon, 27 Mar 2006, Junio C Hamano wrote:
>
> GIT 1.3.0-rc1 is pushed out and will be mirrored out soon.
I did 
	gitk ORIG_HEAD..

with this, and the end result looks horrible. I think it's the new gitk that does it.

Paul, do this on the current git tree:
	gitk b0a3de42..dff86e28
and tell me it doesn't look horrid.

Maybe it's not a new thing, and it's just that the recent pattern of merges in the git tree makes any version of gitk do horrible things.

		Linus
Junio C Hamano· Mar 28, 2006, 02:12 UTC · re: Linus Torvalds · lore

Re: Gitk strangeness..

Linus Torvalds <torvalds@osdl.org> writes:
> Maybe it's not a new thing, and it's just that the recent pattern of 
> merges in the git tree makes any version of gitk do horrible things.
It is both, but new gitk plays a major part of it.

There are too wide horizontal lines when many merges are involved. My "next" branch from yesterday (which is essentially what my "master" branch today) was somewhat more pleasant to read with older gitk, but only somewhat.

Paul Mackerras· Mar 28, 2006, 02:31 UTC · re: Linus Torvalds · lore

Re: Gitk strangeness..

Linus Torvalds writes:
Show 5 quoted lines
> Paul, do this on the current git tree:
> 
> 	gitk b0a3de42..dff86e28
> 
> and tell me it doesn't look horrid.
Wow!  That's spectacular! :)
> Maybe it's not a new thing, and it's just that the recent pattern of 
> merges in the git tree makes any version of gitk do horrible things.

A large part of it is that I took out the stuff where gitk used to reorder the commits it got from git-rev-list. One of the side-effects of doing the reordering was that for commits which aren't listed in the git-rev-list output (i.e. which are drawn with open circles), gitk was able to draw them immediately after their last child. Now gitk doesn't discover that they aren't listed until it has drawn all the commits that are listed, which means we can get a whole pile of open-circle commits at the bottom of the graph.

I think the best thing to do is to change git-rev-list. One possibility would be to add an option to make git-rev-list omit parents that are not in the requested set, which would mean that gitk would not draw the open-circle commits any more.

The other option would be to make git-rev-list list the open-circle commits explicitly, with an indication that they are not in the requested set but are parents of commits in the requested set.

Or I can put the logic back into gitk. I'd rather do it in git-rev-list though since it will be faster that way.

Do you think that having the open-circle commits in the graph is useful?

Paul.
Linus Torvalds· Mar 28, 2006, 02:52 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

On Tue, 28 Mar 2006, Paul Mackerras wrote:
Show 5 quoted lines
> 
> I think the best thing to do is to change git-rev-list.  One
> possibility would be to add an option to make git-rev-list omit
> parents that are not in the requested set, which would mean that gitk
> would not draw the open-circle commits any more.

I love the open circles. I often want to know what the previous commit was. For example, I use gitk mainly for "gitk ORIG_HEAD..", and then I see the thing that the newly merged stuff was based on (ie was it a major release, or some random point).

> The other option would be to make git-rev-list list the open-circle
> commits explicitly, with an indication that they are not in the
> requested set but are parents of commits in the requested set.
Hmm. That shouldn't be hard to do, but what would be syntax be?
		Linus
Junio C Hamano· Mar 28, 2006, 02:54 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
> The other option would be to make git-rev-list list the open-circle
> commits explicitly, with an indication that they are not in the
> requested set but are parents of commits in the requested set.

I might be off the mark, but are you thinking about something like the attached patch?

> Do you think that having the open-circle commits in the graph is
> useful?
Of course.

-- >8 -- rev-list: --parents-with-boundary

The new flag acts like --parents, but uninteresting parents are marked by prefied '-' sign.

        $ git-rev-list --parents-with-boundary HEAD^^..HEAD
        acb7257... 9c48666...
        9c48666... -dff86e2..
---
diff --git a/rev-list.c b/rev-list.c
index 441c437..58fc449 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -39,6 +39,8 @@
 static int bisect_list = 0;
 static int verbose_header = 0;
 static int abbrev = DEFAULT_ABBREV;
+#define SHOW_PARENTS 1
+#define SHOW_PARENTS_BOUNDARY 2
 static int show_parents = 0;
 static int show_timestamp = 0;
 static int hdr_termination = 0;
@@ -59,7 +61,11 @@
 			parents = parents->next;
 			if (o->flags & TMP_MARK)
 				continue;
-			printf(" %s", sha1_to_hex(o->sha1));
+			if (show_parents == SHOW_PARENTS_BOUNDARY &&
+			    o->flags & UNINTERESTING)
+				printf(" -%s", sha1_to_hex(o->sha1));
+			else
+				printf(" %s", sha1_to_hex(o->sha1));
 			o->flags |= TMP_MARK;
 		}
 		/* TMP_MARK is a general purpose flag that can
@@ -337,7 +343,11 @@
 			continue;
 		}
 		if (!strcmp(arg, "--parents")) {
-			show_parents = 1;
+			show_parents = SHOW_PARENTS;
+			continue;
+		}
+		if (!strcmp(arg, "--parents-with-boundary")) {
+			show_parents = SHOW_PARENTS_BOUNDARY;
 			continue;
 		}
 		if (!strcmp(arg, "--timestamp")) {
Paul Mackerras· Mar 28, 2006, 04:31 UTC · re: Junio C Hamano · lore

Re: Gitk strangeness..

Junio C Hamano writes:
> I might be off the mark, but are you thinking about something
> like the attached patch?

The thing is that I need to know when I have seen the last child of the boundary parent, because I only want to draw the open-circle commit after I have drawn all its children.

Would it be possible to put the '-' in only for the last child that has that parent?

Paul.
Junio C Hamano· Mar 28, 2006, 05:38 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
> Would it be possible to put the '-' in only for the last child that
> has that parent?

Not trivially. We do not keep track of who are children of a commit.

Paul Mackerras· Mar 28, 2006, 06:18 UTC · re: Junio C Hamano · lore

Re: Gitk strangeness..

Show 5 quoted lines
> > Would it be possible to put the '-' in only for the last child that
> > has that parent?
> 
> Not trivially.  We do not keep track of who are children of a
> commit.

Hmmm... how does the --topo-order logic ensure that parents are shown after all of their children? Essentially I want that logic applied to the boundary parent commits as well as the requested commits.

The other thing is that if git-rev-list can actually list those boundary parents, complete with the whole commit message if --header is given, then that will save gitk from having to do a git-cat-file to get that information.

Paul.
Junio C Hamano· Mar 28, 2006, 07:52 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
Show 6 quoted lines
>> Not trivially.  We do not keep track of who are children of a
>> commit.
>
> Hmmm... how does the --topo-order logic ensure that parents are shown
> after all of their children?  Essentially I want that logic applied to
> the boundary parent commits as well as the requested commits.

The sort happens after we sift which commits are interesting and which are not, and uninteresting ones are not subject to sorting, so that is too late.

> The other thing is that if git-rev-list can actually list those
> boundary parents, complete with the whole commit message if --header
> is given, then that will save gitk from having to do a git-cat-file to
> get that information.

How about this alternative patch, then? It turned out to be quite convoluted as I feared.

For example, with this graph:
              C side
             /
	A---B---D master
This command
	$ git rev-list --boundary --header --parents side..master
would give:
	D B
        tree D^{tree}
        parent B
        ...
        \0-B A
        tree B^{tree}
        parent A
        ...
        \0

That is, it includes the UNINTERESING commits at the boundary, which are usually not shown, in its output, but their object names are prefixed with a '-'.

---
diff --git a/rev-list.c b/rev-list.c
index 441c437..a1f129b 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -7,9 +7,9 @@
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0-4 in revision.h */
+/* bits #0-5 in revision.h */
 
-#define COUNTED		(1u<<5)
+#define COUNTED		(1u<<6)
 
 static const char rev_list_usage[] =
 "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -51,6 +51,8 @@
 		printf("%lu ", commit->date);
 	if (commit_prefix[0])
 		fputs(commit_prefix, stdout);
+	if (commit->object.flags & BOUNDARY)
+		putchar('-');
 	fputs(sha1_to_hex(commit->object.sha1), stdout);
 	if (show_parents) {
 		struct commit_list *parents = commit->parents;
diff --git a/revision.c b/revision.c
index d67718c..a9b8f9d 100644
--- a/revision.c
+++ b/revision.c
@@ -390,6 +390,21 @@
 	}
 }
 
+void debug_list(struct commit_list *l)
+{
+	int i = 0;
+	while (l) {
+		struct commit *commit = l->item;
+		printf("%d: %x %s\n",
+		       i,
+		       commit->object.flags,
+		       sha1_to_hex(commit->object.sha1));
+		printf("  %s\n", commit->buffer);
+		l = l->next;
+		i++;
+	}
+}
+
 static void limit_list(struct rev_info *revs)
 {
 	struct commit_list *list = revs->commits;
@@ -418,6 +433,27 @@
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
 			continue;
 		p = &commit_list_insert(commit, p)->next;
+	}
+	if (revs->boundary) {
+		list = newlist;
+		while (list) {
+			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 *pcommit = parent->item;
+				parent = parent->next;
+				if (!(pcommit->object.flags & UNINTERESTING))
+					continue;
+				pcommit->object.flags |= BOUNDARY;
+				p = &commit_list_insert(pcommit, p)->next;
+			}
+			list = list->next;
+		}
 	}
 	revs->commits = newlist;
 }
@@ -587,10 +623,14 @@
 				revs->remove_empty_trees = 1;
 				continue;
 			}
-			if (!strncmp(arg, "--no-merges", 11)) {
+			if (!strcmp(arg, "--no-merges")) {
 				revs->no_merges = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--boundary")) {
+				revs->boundary = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--objects")) {
 				revs->tag_objects = 1;
 				revs->tree_objects = 1;
@@ -731,13 +771,17 @@
 	do {
 		struct commit *commit = revs->commits->item;
 
-		if (commit->object.flags & (UNINTERESTING|SHOWN))
+		if (commit->object.flags & SHOWN)
+			goto next;
+		if (!(commit->object.flags & BOUNDARY) &&
+		    (commit->object.flags & UNINTERESTING))
 			goto next;
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
 			goto next;
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			return NULL;
-		if (revs->no_merges && commit->parents && commit->parents->next)
+		if (revs->no_merges &&
+		    commit->parents && commit->parents->next)
 			goto next;
 		if (revs->prune_fn && revs->dense) {
 			if (!(commit->object.flags & TREECHANGE))
@@ -745,8 +789,12 @@
 			rewrite_parents(commit);
 		}
 		/* More to go? */
-		if (revs->max_count)
-			pop_most_recent_commit(&revs->commits, SEEN);
+		if (revs->max_count) {
+			unsigned flag = SEEN;
+			if (commit->object.flags & BOUNDARY)
+				flag |= UNINTERESTING;
+			pop_most_recent_commit(&revs->commits, flag);
+		}
 		commit->object.flags |= SHOWN;
 		return commit;
 next:
diff --git a/revision.h b/revision.h
index 6c2beca..61e6bc9 100644
--- a/revision.h
+++ b/revision.h
@@ -6,6 +6,7 @@
 #define TREECHANGE	(1u<<2)
 #define SHOWN		(1u<<3)
 #define TMP_MARK	(1u<<4) /* for isolated cases; clean after use */
+#define BOUNDARY	(1u<<5)
 
 struct rev_info;
 
@@ -32,7 +33,8 @@
 			blob_objects:1,
 			edge_hint:1,
 			limited:1,
-			unpacked:1;
+			unpacked:1,
+			boundary:1;
 
 	/* special limits */
 	int max_count;
Paul Mackerras· Mar 28, 2006, 22:51 UTC · re: Junio C Hamano · lore

Re: Gitk strangeness..

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.

Could you check in your patch to the git.git repository, please?

Thanks, Paul.

diff --git a/gitk b/gitk
index 03cd475..1989aa5 100755
--- a/gitk
+++ b/gitk
@@ -46,7 +46,7 @@ proc start_rev_list {rlargs} {
     }
     if {[catch {
 	set commfd [open [concat | git-rev-list --header $order \
-			      --parents $rlargs] r]
+			      --parents --boundary $rlargs] r]
     } err]} {
 	puts stderr "Error executing git-rev-list: $err"
 	exit 1
@@ -114,8 +114,13 @@ proc getcommitlines {commfd}  {
 	set start [expr {$i + 1}]
 	set j [string first "\n" $cmit]
 	set ok 0
+	set listed 1
 	if {$j >= 0} {
 	    set ids [string range $cmit 0 [expr {$j - 1}]]
+	    if {[string range $ids 0 0] == "-"} {
+		set listed 0
+		set ids [string range $ids 1 end]
+	    }
 	    set ok 1
 	    foreach id $ids {
 		if {[string length $id] != 40} {
@@ -133,8 +138,12 @@ proc getcommitlines {commfd}  {
 	    exit 1
 	}
 	set id [lindex $ids 0]
-	set olds [lrange $ids 1 end]
-	set commitlisted($id) 1
+	if {$listed} {
+	    set olds [lrange $ids 1 end]
+	    set commitlisted($id) 1
+	} else {
+	    set olds {}
+	}
 	updatechildren $id $olds
 	set commitdata($id) [string range $cmit [expr {$j + 1}] end]
 	set commitrow($id) $commitidx
Junio C Hamano· Mar 29, 2006, 00:50 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
Show 9 quoted lines
> 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.
>
> Could you check in your patch to the git.git repository, please?

The patch I sent was a total mess, and the one in "pu" right now was somewhat cleaned up but was still far suboptimal. **Blush**

Most notably, the code from yesterday was re-injecting the parents of the boundary commits into the list marked as UNINTERESTING, which was unnecessary and stupid. This one just pops boundary commits off the list after consuming it.

Here is a cleaned-up one for eyeballing.

Although I am reasonably sure that this does not affect the way it works when --boundary is not given, I'd pretty much appreciate an independent sanity check on this one. rev-list is so fundamental to git.

-- >8 --
diff --git a/rev-list.c b/rev-list.c
index 441c437..f3a989c 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -7,9 +7,9 @@ #include "blob.h"
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0-4 in revision.h */
+/* bits #0-5 in revision.h */
 
-#define COUNTED		(1u<<5)
+#define COUNTED		(1u<<6)
 
 static const char rev_list_usage[] =
 "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -51,6 +51,8 @@ static void show_commit(struct commit *c
 		printf("%lu ", commit->date);
 	if (commit_prefix[0])
 		fputs(commit_prefix, stdout);
+	if (commit->object.flags & BOUNDARY)
+		putchar('-');
 	fputs(sha1_to_hex(commit->object.sha1), stdout);
 	if (show_parents) {
 		struct commit_list *parents = commit->parents;
diff --git a/revision.c b/revision.c
index d7678cf..745b0d2 100644
--- a/revision.c
+++ b/revision.c
@@ -419,6 +419,27 @@ static void limit_list(struct rev_info *
 			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
+	if (revs->boundary) {
+		list = newlist;
+		while (list) {
+			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 *pcommit = parent->item;
+				parent = parent->next;
+				if (!(pcommit->object.flags & UNINTERESTING))
+					continue;
+				pcommit->object.flags |= BOUNDARY;
+				p = &commit_list_insert(pcommit, p)->next;
+			}
+			list = list->next;
+		}
+	}
 	revs->commits = newlist;
 }
 
@@ -591,6 +612,10 @@ int setup_revisions(int argc, const char
 				revs->no_merges = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--boundary")) {
+				revs->boundary = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--objects")) {
 				revs->tag_objects = 1;
 				revs->tree_objects = 1;
@@ -731,13 +756,17 @@ struct commit *get_revision(struct rev_i
 	do {
 		struct commit *commit = revs->commits->item;
 
-		if (commit->object.flags & (UNINTERESTING|SHOWN))
+		if (commit->object.flags & SHOWN)
+			goto next;
+		if (!(commit->object.flags & BOUNDARY) &&
+		    (commit->object.flags & UNINTERESTING))
 			goto next;
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
 			goto next;
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			return NULL;
-		if (revs->no_merges && commit->parents && commit->parents->next)
+		if (revs->no_merges &&
+		    commit->parents && commit->parents->next)
 			goto next;
 		if (revs->prune_fn && revs->dense) {
 			if (!(commit->object.flags & TREECHANGE))
@@ -745,8 +774,19 @@ struct commit *get_revision(struct rev_i
 			rewrite_parents(commit);
 		}
 		/* More to go? */
-		if (revs->max_count)
-			pop_most_recent_commit(&revs->commits, SEEN);
+		if (revs->max_count) {
+			if (commit->object.flags & BOUNDARY) {
+				/* this is already uninteresting,
+				 * so there is no point popping its
+				 * parents into the list.
+				 */
+				struct commit_list *it = revs->commits;
+				revs->commits = it->next;
+				free(it);
+			}
+			else
+				pop_most_recent_commit(&revs->commits, SEEN);
+		}
 		commit->object.flags |= SHOWN;
 		return commit;
 next:
diff --git a/revision.h b/revision.h
index 6c2beca..61e6bc9 100644
--- a/revision.h
+++ b/revision.h
@@ -6,6 +6,7 @@ #define UNINTERESTING   (1u<<1)
 #define TREECHANGE	(1u<<2)
 #define SHOWN		(1u<<3)
 #define TMP_MARK	(1u<<4) /* for isolated cases; clean after use */
+#define BOUNDARY	(1u<<5)
 
 struct rev_info;
 
@@ -32,7 +33,8 @@ struct rev_info {
 			blob_objects:1,
 			edge_hint:1,
 			limited:1,
-			unpacked:1;
+			unpacked:1,
+			boundary:1;
 
 	/* special limits */
 	int max_count;
Junio C Hamano· Mar 29, 2006, 06:41 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
> That's brilliant.  Thank you!  With the patch to gitk below, the
> graph display on Linus' example looks much saner.
Indeed this looks much saner.  Thanks.
Alex Riesen· Mar 30, 2006, 20:57 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras, Wed, Mar 29, 2006 00:51:34 +0200:
Show 8 quoted lines
> 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).

Paul Mackerras· Mar 30, 2006, 22:33 UTC · re: Alex Riesen · lore

Re: Gitk strangeness..

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.
Paul Mackerras· Mar 30, 2006, 23:46 UTC · re: Alex Riesen · lore

Re: Gitk strangeness..

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.
Junio C Hamano· Mar 31, 2006, 01:50 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

Paul Mackerras <paulus@samba.org> writes:
Show 7 quoted lines
> 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..
Alex Riesen· Mar 31, 2006, 06:27 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

On 3/31/06, Paul Mackerras <paulus@samba.org> wrote:
Show 7 quoted lines
>
> > 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!
Linus Torvalds· Mar 28, 2006, 02:57 UTC · re: Paul Mackerras · lore

Re: Gitk strangeness..

On Tue, 28 Mar 2006, Paul Mackerras wrote:
>
> The other option would be to make git-rev-list list the open-circle
> commits explicitly, with an indication that they are not in the
> requested set but are parents of commits in the requested set.
Just as an indication of _how_ simple that is, here's a stupid patch.
It just puts a "-" after a parent that isn't going to be shown.

Play with it (and it probably needs a new flag to enable it, since doing it unconditionally like this will break old versions of gitk and probably anything else that uses the "--parent" flag).

		Linus
----
diff --git a/rev-list.c b/rev-list.c
index 441c437..822a740 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -60,6 +60,8 @@
 			if (o->flags & TMP_MARK)
 				continue;
 			printf(" %s", sha1_to_hex(o->sha1));
+			if (o->flags & UNINTERESTING)
+				putchar('-');
 			o->flags |= TMP_MARK;
 		}
 		/* TMP_MARK is a general purpose flag that can

← back to recent threads