Git development
 help / color / mirror / Atom feed
* Re: git log is a bit antisocial
From: Junio C Hamano @ 2006-04-14 21:28 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141719290.2215@localhost.localdomain>

Nicolas Pitre <nico@cam.org> writes:

> On Fri, 14 Apr 2006, Junio C Hamano wrote:
>
>> Nicolas Pitre <nico@cam.org> writes:
>> 
>> > $  git log -h
>> > fatal: unrecognized argument: -h
>> > $ git log --help
>> > fatal: unrecognized argument: --help
>> >
>> > Maybe the usage string could be printed in those cases?
>> 
>> Perhaps.  Alternatively, "git help log", perhaps.
>
> What about git-log then?

What about it?

Asking for help on log could be spelled as "git log --help" with
a patch like the attached, but I am not sure that is worth it...

-- >8 --
diff --git a/git.c b/git.c
index 78ed403..7fdacdd 100644
--- a/git.c
+++ b/git.c
@@ -497,6 +497,16 @@ int main(int argc, const char **argv, ch
 	}
 	argv[0] = cmd;
 
+	/* It could be git blah --help or git boo -h, but be
+	 * careful; most commands have their own '-h' and '--help'.
+	 */
+	if (argc == 2 &&
+	    (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))) {
+		argv[0] = "help";
+		argv[1] = cmd;
+		exit(cmd_help(1, argv, envp));
+	}
+
 	/*
 	 * We search for git commands in the following order:
 	 *  - git_exec_path()

^ permalink raw reply related

* Re: git log is a bit antisocial
From: Sébastien Pierre @ 2006-04-14 21:44 UTC (permalink / raw)
  To: git
In-Reply-To: <7vhd4vq23h.fsf@assigned-by-dhcp.cox.net>

Le vendredi 14 avril 2006 à 14:28 -0700, Junio C Hamano a écrit :

> What about it?
> 
> Asking for help on log could be spelled as "git log --help" with
> a patch like the attached, but I am not sure that is worth it...

I would say that it is very useful to newbies, or simply not to
frustrate users trying to get help. It is really worth it, at least for
me.

 -- Sébastien

^ permalink raw reply

* Re: git log is a bit antisocial
From: Junio C Hamano @ 2006-04-14 22:06 UTC (permalink / raw)
  To: Sébastien Pierre; +Cc: git
In-Reply-To: <1145051072.27704.1.camel@localhost.localdomain>

Sébastien Pierre <sebastien@xprima.com> writes:

> Le vendredi 14 avril 2006 à 14:28 -0700, Junio C Hamano a écrit :
>
>> What about it?
>> 
>> Asking for help on log could be spelled as "git log --help" with
>> a patch like the attached, but I am not sure that is worth it...
>
> I would say that it is very useful to newbies, or simply not to
> frustrate users trying to get help. It is really worth it, at least for
> me.

Have you read the patch, especially the comment in it?  With and
without the patch, this command would behave quite differently:

	$ git commit --help

^ permalink raw reply

* Re: git log is a bit antisocial
From: Sébastien Pierre @ 2006-04-14 22:15 UTC (permalink / raw)
  To: git
In-Reply-To: <7vu08volrp.fsf@assigned-by-dhcp.cox.net>

Le vendredi 14 avril 2006 à 15:06 -0700, Junio C Hamano a écrit :

> Have you read the patch, especially the comment in it?  With and
> without the patch, this command would behave quite differently:

I did not realize that at first (I thought this would be a fallback
method). 

Anyway, on git 1.2.3, here is something interesting:

>> git log -h
fatal: Not a git repository

>> git log --help
Usage: /home/sebastien/Local/bin/git-log [--max-count=<n>]
[<since>..<limit>] [--pretty=<format>] [git-rev-list options]

Which is confusing, so having a consistent behaviour for "git help cmd",
"git cmd help", "git cmd -h" and "git cmd --help" would be nice.

For instance, Darcs works just like that, which makes it easy for
newbies to find there ways through.

 -- Sébastien

^ permalink raw reply

* [PATCH] rev-list --bisect: limit list before bisecting.
From: Junio C Hamano @ 2006-04-14 22:40 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

I noticed bisect does not work well without both good and bad.
Running this script in git.git repository would give you quite
different results:

	#!/bin/sh
        initial=e83c5163316f89bfbde7d9ab23ca2e25604af290

        mid0=`git rev-list --bisect ^$initial --all`

        git rev-list $mid0 | wc -l
        git rev-list ^$mid0 --all | wc -l

        mid1=`git rev-list --bisect --all`

        git rev-list $mid1 | wc -l
        git rev-list ^$mid1 --all | wc -l

The $initial commit is the very first commit you made.  The
first midpoint bisects things evenly as designed, but the latter
does not.

The reason I got interested in this was because I was wondering
if something like the following would help people converting a
huge repository from foreign SCM, or preparing a repository to
be fetched over plain dumb HTTP only:

        #!/bin/sh

        N=4
        P=.git/objects/pack
        bottom=

        while test 0 \< $N
        do
                N=$((N-1))
                if test -z "$bottom"
                then
                        newbottom=`git rev-list --bisect --all`
                else
                        newbottom=`git rev-list --bisect ^$bottom --all`
                fi
                if test -z "$bottom"
                then
                        rev_list="$newbottom"
                elif test 0 = $N
                then
                        rev_list="^$bottom --all"
                else
                        rev_list="^$bottom $newbottom"
                fi
                p=$(git rev-list --unpacked --objects $rev_list |
                    git pack-objects $P/pack)
                git show-index <$P/pack-$p.idx | wc -l
                bottom=$newbottom
        done

The idea is to pack older half of the history to one pack, then
older half of the remaining history to another, to continue a
few times, using finer granularity as we get closer to the tip.

This may not matter, since for a truly huge history, running
bisect number of times could be quite time consuming, and we
might be better off running "git rev-list --all" once into a
temporary file, and manually pick cut-off points from the
resulting list of commits.  After all we are talking about
"approximately half" for such an usage, and older history does
not matter much.

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

---

 rev-list.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/rev-list.c b/rev-list.c
index 963707a..cb67b39 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -371,6 +371,8 @@ int main(int argc, const char **argv)
 
 	save_commit_buffer = verbose_header;
 	track_object_refs = 0;
+	if (bisect_list)
+		revs.limited = 1;
 
 	prepare_revision_walk(&revs);
 	if (revs.tree_objects)

^ permalink raw reply related

* Re: git log is a bit antisocial
From: Junio C Hamano @ 2006-04-14 22:45 UTC (permalink / raw)
  To: Sébastien Pierre; +Cc: git
In-Reply-To: <1145052905.27704.8.camel@localhost.localdomain>

Sébastien Pierre <sebastien@xprima.com> writes:

> Anyway, on git 1.2.3, here is something interesting:
>
>>> git log -h
> fatal: Not a git repository
>
>>> git log --help
> Usage: /home/sebastien/Local/bin/git-log [--max-count=<n>]
> [<since>..<limit>] [--pretty=<format>] [git-rev-list options]

You are talking about old codebase in the maitenance branch,
which is an independent issue, but thanks for noticing anyway.

The attached patch would help with that.

> Which is confusing, so having a consistent behaviour for "git help cmd",
> "git cmd help", "git cmd -h" and "git cmd --help" would be nice.
>
> For instance, Darcs works just like that, which makes it easy for
> newbies to find there ways through.

Patches welcome, but a new development should be based on the
"master" branch, not the maintenance 1.2.X series.

-- >8 --
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 025ef2d..d15747f 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -30,7 +30,7 @@ else
 fi
 
 case "$1" in
-	--h|--he|--hel|--help)
+	-h|--h|--he|--hel|--help)
 	echo "$LONG_USAGE"
 	exit
 esac

^ permalink raw reply related

* Re: Recent unresolved issues: shallow clone
From: Carl Worth @ 2006-04-14 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64lcqz9j.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]

On Fri, 14 Apr 2006 02:31:36 -0700, Junio C Hamano wrote:
>   Shallow clones (Carl Worth).
> 
>   The experiment last round did not work out very well, but as
>   existing repositories get bigger, and more projects being
>   migrated from foreign SCM systems, this would become a
>   must-have from would-be-nice-to-have.
> 
>   I am beginning to think using "graft" to cauterize history
>   for this, while it technically would work, would not be so
>   helpful to users, so the design needs to be worked out again.

As context, here is some of what you mentioned in IRC:

>>	Suppose you have this:
>>
>>	A---B---C
>>	 \       \ 
>>	  D---E---F---G
>>	 
>>	and you made a shallow clone of C (because that is where the
>>	upstream master was when you made that clone).  Then the
>>	upstream updated the master branch tip to G.
>>
>>	The next update from upstream to your shallow clone would break.
>>	The upstream says: I have G at master.
>>	You say: I want G then.  By the way, I have C.
>>
>>	What it means to tell the other end "I have X" is to promise
>>	that you have X and _everything_ behind it.  So the upstream
>>	would send objects necessary to complete D, E, F and G for
>>	"somebody who already have A and B".  As a consequence, you
>>	would not see A nor B.
>>
>>	Even if the only thing you are interested in is to be in sync
>>	with the tip of the upstream, you can end up with an
>>	incomplete tree for G, if some of the blobs or trees contained
>>	in G already exist in A or B.  They are not sent -- because
>>	you told the upstream that you have everything necessary to
>>	get to C.

So that's an argument against using a cauterizing graft for the
shallow clone of C. It definitely confuses the existing protocol to
say "I have C" if I have only a cauterized C, (its tree only, but none
of the commits that should be backing C).

I also read over some of your discussion of extending the protocol
with a new "shallow" extension.

I'm wondering if the shallow clone support couldn't be achieved
through a simpler tweak to the protocol semantics, (and no change to
protocol syntax), that would avoid the problem above. Specifically,
for shallow stuff, could we just do the same "want" and "have"
conversation with tree objects rather than commit objects?

So, in the scenario above, the original shallow clone of C would be:

	Want C->tree, have nothing.

and the later shallow update to G would be:

	Want G->tree, have C->tree

A final step of a shallow clone would then require creating a new
parent-less commit object so that there's something to point refs/head
at, (or maybe rather than being parentless, they could be chained
together with each update?).

I admit that this would result in a rather atypical kind of
repository, but it would contain plenty of valid trees and blobs, so
it should conceptually be fairly easy to promote such a thing to a
full repository.

But, even without any tool support for promotion, the ability to do
shallow clone and shallow updates would still provide a useful
capability [*].

-Carl

[*] For reference, what I'm looking for here is a way to justify
providing git support for jhbuild, which is a tool used by testers of
GNOME and other software to efficiently track the latest development
of an arbitrarily large number of packages. It's currently primarily a
CVS-based thing. Switching to git would be a huge win for the
incremental updates, but would currently cause quite a hit for the
first clone.

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-14 23:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v64lcqz9j.fsf@assigned-by-dhcp.cox.net>



On Fri, 14 Apr 2006, Junio C Hamano wrote:
> 
> * Message-ID: <Pine.LNX.4.64.0604121828370.14565@g5.osdl.org>
>   Common option parsing (Linus Torvalds)

Ok, here's a first cut at starting this.

This basically does a few things that are sadly somewhat interdependent, 
and nontrivial to split out

 - get rid of "struct log_tree_opt"

   The fields in "log_tree_opt" are moved into "struct rev_info", and all 
   users of log_tree_opt are changed to use the rev_info struct instead.

 - add the parsing for the log_tree_opt arguments to "setup_revision()"

 - make setup_revision set a flag (revs->diff) if the diff-related 
   arguments were used. This allows "git log" to decide whether it wants 
   to show diffs or not.

 - make setup_revision() also initialize the diffopt part of rev_info 
   (which we had from before, but we just didn't initialize it)

 - make setup_revision() do all the "finishing touches" on it all (it will 
   do the proper flag combination logic, and call "diff_setup_done()")

Now, that was the easy and straightforward part.

The slightly more involved part is that some of the programs that want to 
use the new-and-improved rev_info parsing don't actually want _commits_, 
they may want tree'ish arguments instead. That meant that I had to change 
setup_revision() to parse the arguments not into the "revs->commits" list, 
but into the "revs->pending_objects" list.

Then, when we do "prepare_revision_walk()", we walk that list, and create 
the sorted commit list from there. 

This actually cleaned some stuff up, but it's the less obvious part of the 
patch, and re-organized the "revision.c" logic somewhat. It actually paves 
the way for splitting argument parsing _entirely_ out of "revision.c", 
since now the argument parsing really is totally independent of the commit 
walking: that didn't use to be true, since there was lots of overlap with 
get_commit_reference() handling etc, now the _only_ overlap is the shared 
(and trivial) "add_pending_object()" thing.

However, I didn't do that file split, just because I wanted the diff 
itself to be smaller, and show the actual changes more clearly. If this 
gets accepted, I'll do further cleanups then - that includes the file 
split, but also using the new infrastructure to do a nicer "git diff" etc.

Even in this form, it actually ends up removing more lines than it adds.

It's nice to note how simple and straightforward this makes the built-in 
"git log" command, even though it continues to support all the diff flags 
too. It doesn't get much simpler that this.

I think this is worth merging soonish, because it does allow for future 
cleanup and even more sharing of code. However, it obviously touches 
"revision.c", which is subtle. I've tested that it passes all the tests we 
have, and it passes my "looks sane" detector, but somebody else should 
also give it a good look-over.

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

 diff-tree.c |   91 ++++++++++++++++-------------------
 git.c       |   68 ++------------------------
 log-tree.c  |   60 ++---------------------
 log-tree.h  |   22 ++------
 revision.c  |  155 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 revision.h  |   18 +++++++
 6 files changed, 202 insertions(+), 212 deletions(-)

diff --git a/diff-tree.c b/diff-tree.c
index 2b79dd0..54157e4 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -3,7 +3,7 @@ #include "diff.h"
 #include "commit.h"
 #include "log-tree.h"
 
-static struct log_tree_opt log_tree_opt;
+static struct rev_info log_tree_opt;
 
 static int diff_tree_commit_sha1(const unsigned char *sha1)
 {
@@ -62,66 +62,55 @@ int main(int argc, const char **argv)
 {
 	int nr_sha1;
 	char line[1000];
-	unsigned char sha1[2][20];
-	const char *prefix = setup_git_directory();
-	static struct log_tree_opt *opt = &log_tree_opt;
+	struct object *tree1, *tree2;
+	static struct rev_info *opt = &log_tree_opt;
+	struct object_list *list;
 	int read_stdin = 0;
 
 	git_config(git_diff_config);
 	nr_sha1 = 0;
-	init_log_tree_opt(opt);
+	argc = setup_revisions(argc, argv, opt, NULL);
 
-	for (;;) {
-		int opt_cnt;
-		const char *arg;
+	while (--argc > 0) {
+		const char *arg = *++argv;
 
-		argv++;
-		argc--;
-		arg = *argv;
-		if (!arg)
-			break;
-
-		if (*arg != '-') {
-			if (nr_sha1 < 2 && !get_sha1(arg, sha1[nr_sha1])) {
-				nr_sha1++;
-				continue;
-			}
-			break;
-		}
-
-		opt_cnt = log_tree_opt_parse(opt, argv, argc);
-		if (opt_cnt < 0)
-			usage(diff_tree_usage);
-		else if (opt_cnt) {
-			argv += opt_cnt - 1;
-			argc -= opt_cnt - 1;
-			continue;
-		}
-
-		if (!strcmp(arg, "--")) {
-			argv++;
-			argc--;
-			break;
-		}
 		if (!strcmp(arg, "--stdin")) {
 			read_stdin = 1;
 			continue;
 		}
 		usage(diff_tree_usage);
 	}
-
-	if (opt->combine_merges)
-		opt->ignore_merges = 0;
-
-	/* We can only do dense combined merges with diff output */
-	if (opt->dense_combined_merges)
-		opt->diffopt.output_format = DIFF_FORMAT_PATCH;
-
-	if (opt->diffopt.output_format == DIFF_FORMAT_PATCH)
-		opt->diffopt.recursive = 1;
 
-	diff_tree_setup_paths(get_pathspec(prefix, argv), opt);
-	diff_setup_done(&opt->diffopt);
+	/*
+	 * NOTE! "setup_revisions()" will have inserted the revisions
+	 * it parsed in reverse order. So if you do
+	 *
+	 *	git-diff-tree a b
+	 *
+	 * the commit list will be "b" -> "a" -> NULL, so we reverse
+	 * the order of the objects if the first one is not marked
+	 * UNINTERESTING.
+	 */
+	nr_sha1 = 0;
+	list = opt->pending_objects;
+	if (list) {
+		nr_sha1++;
+		tree1 = list->item;
+		list = list->next;
+		if (list) {
+			nr_sha1++;
+			tree2 = tree1;
+			tree1 = list->item;
+			if (list->next)
+				usage(diff_tree_usage);
+			/* Switch them around if the second one was uninteresting.. */
+			if (tree2->flags & UNINTERESTING) {
+				struct object *tmp = tree2;
+				tree2 = tree1;
+				tree1 = tmp;
+			}
+		}
+	}
 
 	switch (nr_sha1) {
 	case 0:
@@ -129,10 +118,12 @@ int main(int argc, const char **argv)
 			usage(diff_tree_usage);
 		break;
 	case 1:
-		diff_tree_commit_sha1(sha1[0]);
+		diff_tree_commit_sha1(tree1->sha1);
 		break;
 	case 2:
-		diff_tree_sha1(sha1[0], sha1[1], "", &opt->diffopt);
+		diff_tree_sha1(tree1->sha1,
+			       tree2->sha1,
+			       "", &opt->diffopt);
 		log_tree_diff_flush(opt);
 		break;
 	}
diff --git a/git.c b/git.c
index 78ed403..e8d1fcc 100644
--- a/git.c
+++ b/git.c
@@ -287,74 +287,18 @@ static int cmd_log(int argc, const char 
 	int abbrev = DEFAULT_ABBREV;
 	int abbrev_commit = 0;
 	const char *commit_prefix = "commit ";
-	struct log_tree_opt opt;
 	int shown = 0;
-	int do_diff = 0;
-	int full_diff = 0;
 
-	init_log_tree_opt(&opt);
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strncmp(arg, "--pretty", 8)) {
-			commit_format = get_commit_format(arg + 8);
-			if (commit_format == CMIT_FMT_ONELINE)
-				commit_prefix = "";
-		}
-		else if (!strcmp(arg, "--no-abbrev")) {
-			abbrev = 0;
-		}
-		else if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-		}
-		else if (!strcmp(arg, "--abbrev-commit")) {
-			abbrev_commit = 1;
-		}
-		else if (!strncmp(arg, "--abbrev=", 9)) {
-			abbrev = strtoul(arg + 9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (40 < abbrev)
-				abbrev = 40;
-		}
-		else if (!strcmp(arg, "--full-diff")) {
-			do_diff = 1;
-			full_diff = 1;
-		}
-		else {
-			int cnt = log_tree_opt_parse(&opt, argv+1, argc-1);
-			if (0 < cnt) {
-				do_diff = 1;
-				argv += cnt;
-				argc -= cnt;
-				continue;
-			}
-			die("unrecognized argument: %s", arg);
-		}
+	if (argc > 1)
+		die("unrecognized argument: %s", argv[1]);
 
-		argc--; argv++;
-	}
-
-	if (do_diff) {
-		opt.diffopt.abbrev = abbrev;
-		opt.verbose_header = 0;
-		opt.always_show_header = 0;
-		opt.no_commit_id = 1;
-		if (opt.combine_merges)
-			opt.ignore_merges = 0;
-		if (opt.dense_combined_merges)
-			opt.diffopt.output_format = DIFF_FORMAT_PATCH;
-		if (opt.diffopt.output_format == DIFF_FORMAT_PATCH)
-			opt.diffopt.recursive = 1;
-		if (!full_diff && rev.prune_data)
-			diff_tree_setup_paths(rev.prune_data, &opt.diffopt);
-		diff_setup_done(&opt.diffopt);
-	}
+	rev.no_commit_id = 1;
 
 	prepare_revision_walk(&rev);
 	setup_pager();
 	while ((commit = get_revision(&rev)) != NULL) {
-		if (shown && do_diff && commit_format != CMIT_FMT_ONELINE)
+		if (shown && rev.diff && commit_format != CMIT_FMT_ONELINE)
 			putchar('\n');
 		fputs(commit_prefix, stdout);
 		if (abbrev_commit && abbrev)
@@ -388,8 +332,8 @@ static int cmd_log(int argc, const char 
 		pretty_print_commit(commit_format, commit, ~0, buf,
 				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
-		if (do_diff)
-			log_tree_commit(&opt, commit);
+		if (rev.diff)
+			log_tree_commit(&rev, commit);
 		shown = 1;
 		free(commit->buffer);
 		commit->buffer = NULL;
diff --git a/log-tree.c b/log-tree.c
index 3d40482..04a68e0 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -3,58 +3,8 @@ #include "diff.h"
 #include "commit.h"
 #include "log-tree.h"
 
-void init_log_tree_opt(struct log_tree_opt *opt)
+int log_tree_diff_flush(struct rev_info *opt)
 {
-	memset(opt, 0, sizeof *opt);
-	opt->ignore_merges = 1;
-	opt->header_prefix = "";
-	opt->commit_format = CMIT_FMT_RAW;
-	diff_setup(&opt->diffopt);
-}
-
-int log_tree_opt_parse(struct log_tree_opt *opt, const char **av, int ac)
-{
-	const char *arg;
-	int cnt = diff_opt_parse(&opt->diffopt, av, ac);
-	if (0 < cnt)
-		return cnt;
-	arg = *av;
-	if (!strcmp(arg, "-r"))
-		opt->diffopt.recursive = 1;
-	else if (!strcmp(arg, "-t")) {
-		opt->diffopt.recursive = 1;
-		opt->diffopt.tree_in_recursive = 1;
-	}
-	else if (!strcmp(arg, "-m"))
-		opt->ignore_merges = 0;
-	else if (!strcmp(arg, "-c"))
-		opt->combine_merges = 1;
-	else if (!strcmp(arg, "--cc")) {
-		opt->dense_combined_merges = 1;
-		opt->combine_merges = 1;
-	}
-	else if (!strcmp(arg, "-v")) {
-		opt->verbose_header = 1;
-		opt->header_prefix = "diff-tree ";
-	}
-	else if (!strncmp(arg, "--pretty", 8)) {
-		opt->verbose_header = 1;
-		opt->header_prefix = "diff-tree ";
-		opt->commit_format = get_commit_format(arg+8);
-	}
-	else if (!strcmp(arg, "--root"))
-		opt->show_root_diff = 1;
-	else if (!strcmp(arg, "--no-commit-id"))
-		opt->no_commit_id = 1;
-	else if (!strcmp(arg, "--always"))
-		opt->always_show_header = 1;
-	else
-		return 0;
-	return 1;
-}
-
-int log_tree_diff_flush(struct log_tree_opt *opt)
-{
 	diffcore_std(&opt->diffopt);
 	if (diff_queue_is_empty()) {
 		int saved_fmt = opt->diffopt.output_format;
@@ -73,7 +23,7 @@ int log_tree_diff_flush(struct log_tree_
 	return 1;
 }
 
-static int diff_root_tree(struct log_tree_opt *opt,
+static int diff_root_tree(struct rev_info *opt,
 			  const unsigned char *new, const char *base)
 {
 	int retval;
@@ -93,7 +43,7 @@ static int diff_root_tree(struct log_tre
 	return retval;
 }
 
-static const char *generate_header(struct log_tree_opt *opt,
+static const char *generate_header(struct rev_info *opt,
 				   const unsigned char *commit_sha1,
 				   const unsigned char *parent_sha1,
 				   const struct commit *commit)
@@ -129,7 +79,7 @@ static const char *generate_header(struc
 	return this_header;
 }
 
-static int do_diff_combined(struct log_tree_opt *opt, struct commit *commit)
+static int do_diff_combined(struct rev_info *opt, struct commit *commit)
 {
 	unsigned const char *sha1 = commit->object.sha1;
 
@@ -142,7 +92,7 @@ static int do_diff_combined(struct log_t
 	return 0;
 }
 
-int log_tree_commit(struct log_tree_opt *opt, struct commit *commit)
+int log_tree_commit(struct rev_info *opt, struct commit *commit)
 {
 	struct commit_list *parents;
 	unsigned const char *sha1 = commit->object.sha1;
diff --git a/log-tree.h b/log-tree.h
index da166c6..91a909b 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -1,23 +1,11 @@
 #ifndef LOG_TREE_H
 #define LOG_TREE_H
 
-struct log_tree_opt {
-	struct diff_options diffopt;
-	int show_root_diff;
-	int no_commit_id;
-	int verbose_header;
-	int ignore_merges;
-	int combine_merges;
-	int dense_combined_merges;
-	int always_show_header;
-	const char *header_prefix;
-	const char *header;
-	enum cmit_fmt commit_format;
-};
+#include "revision.h"
 
-void init_log_tree_opt(struct log_tree_opt *);
-int log_tree_diff_flush(struct log_tree_opt *);
-int log_tree_commit(struct log_tree_opt *, struct commit *);
-int log_tree_opt_parse(struct log_tree_opt *, const char **, int);
+void init_log_tree_opt(struct rev_info *);
+int log_tree_diff_flush(struct rev_info *);
+int log_tree_commit(struct rev_info *, struct commit *);
+int log_tree_opt_parse(struct rev_info *, const char **, int);
 
 #endif
diff --git a/revision.c b/revision.c
index 0505f3f..99077af 100644
--- a/revision.c
+++ b/revision.c
@@ -116,21 +116,27 @@ static void add_pending_object(struct re
 	add_object(obj, &revs->pending_objects, NULL, name);
 }
 
-static struct commit *get_commit_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)
+static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)
 {
 	struct object *object;
 
 	object = parse_object(sha1);
 	if (!object)
 		die("bad object %s", name);
+	object->flags |= flags;
+	return object;
+}
+
+static struct commit *handle_commit(struct rev_info *revs, struct object *object, const char *name)
+{
+	unsigned long flags = object->flags;
 
 	/*
 	 * Tag object? Look what it points to..
 	 */
 	while (object->type == tag_type) {
 		struct tag *tag = (struct tag *) object;
-		object->flags |= flags;
-		if (revs->tag_objects && !(object->flags & UNINTERESTING))
+		if (revs->tag_objects && !(flags & UNINTERESTING))
 			add_pending_object(revs, object, tag->tag);
 		object = parse_object(tag->tagged->sha1);
 		if (!object)
@@ -143,7 +149,6 @@ static struct commit *get_commit_referen
 	 */
 	if (object->type == commit_type) {
 		struct commit *commit = (struct commit *)object;
-		object->flags |= flags;
 		if (parse_commit(commit) < 0)
 			die("unable to parse commit %s", name);
 		if (flags & UNINTERESTING) {
@@ -449,14 +454,6 @@ static void limit_list(struct rev_info *
 		}
 	}
 	revs->commits = newlist;
-}
-
-static void add_one_commit(struct commit *commit, struct rev_info *revs)
-{
-	if (!commit || (commit->object.flags & SEEN))
-		return;
-	commit->object.flags |= SEEN;
-	commit_list_insert(commit, &revs->commits);
 }
 
 static int all_flags;
@@ -464,8 +461,8 @@ static struct rev_info *all_revs;
 
 static int handle_one_ref(const char *path, const unsigned char *sha1)
 {
-	struct commit *commit = get_commit_reference(all_revs, path, sha1, all_flags);
-	add_one_commit(commit, all_revs);
+	struct object *object = get_reference(all_revs, path, sha1, all_flags);
+	add_pending_object(all_revs, object, "");
 	return 0;
 }
 
@@ -494,6 +491,11 @@ void init_revisions(struct rev_info *rev
 
 	revs->topo_setter = topo_sort_default_setter;
 	revs->topo_getter = topo_sort_default_getter;
+
+	revs->header_prefix = "";
+	revs->commit_format = CMIT_FMT_RAW;
+
+	diff_setup(&revs->diffopt);
 }
 
 /*
@@ -526,13 +528,14 @@ int setup_revisions(int argc, const char
 
 	flags = 0;
 	for (i = 1; i < argc; i++) {
-		struct commit *commit;
+		struct object *object;
 		const char *arg = argv[i];
 		unsigned char sha1[20];
 		char *dotdot;
 		int local_flags;
 
 		if (*arg == '-') {
+			int opts;
 			if (!strncmp(arg, "--max-count=", 12)) {
 				revs->max_count = atoi(arg + 12);
 				continue;
@@ -638,6 +641,78 @@ int setup_revisions(int argc, const char
 			}
 			if (!strcmp(arg, "--unpacked")) {
 				revs->unpacked = 1;
+				continue;
+			}
+			if (!strcmp(arg, "-r")) {
+				revs->diff = 1;
+				revs->diffopt.recursive = 1;
+				continue;
+			}
+			if (!strcmp(arg, "-t")) {
+				revs->diff = 1;
+				revs->diffopt.recursive = 1;
+				revs->diffopt.tree_in_recursive = 1;
+				continue;
+			}
+			if (!strcmp(arg, "-m")) {
+				revs->ignore_merges = 0;
+				continue;
+			}
+			if (!strcmp(arg, "-c")) {
+				revs->diff = 1;
+				revs->combine_merges = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--cc")) {
+				revs->diff = 1;
+				revs->dense_combined_merges = 1;
+				revs->combine_merges = 1;
+				continue;
+			}
+			if (!strcmp(arg, "-v")) {
+				revs->verbose_header = 1;
+				revs->header_prefix = "diff-tree ";
+				continue;
+			}
+			if (!strncmp(arg, "--pretty", 8)) {
+				revs->verbose_header = 1;
+				revs->header_prefix = "diff-tree ";
+				revs->commit_format = get_commit_format(arg+8);
+				continue;
+			}
+			if (!strcmp(arg, "--root")) {
+				revs->show_root_diff = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--no-commit-id")) {
+				revs->no_commit_id = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--always")) {
+				revs->always_show_header = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--no-abbrev")) {
+				revs->abbrev = 0;
+				continue;
+			}
+			if (!strcmp(arg, "--abbrev")) {
+				revs->abbrev = DEFAULT_ABBREV;
+				continue;
+			}
+			if (!strcmp(arg, "--abbrev-commit")) {
+				revs->abbrev_commit = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--full-diff")) {
+				revs->diff = 1;
+				revs->full_diff = 1;
+				continue;
+			}
+			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
+			if (opts > 0) {
+				revs->diff = 1;
+				i += opts - 1;
 				continue;
 			}
 			*unrecognized++ = arg;
@@ -656,15 +731,15 @@ int setup_revisions(int argc, const char
 				this = "HEAD";
 			if (!get_sha1(this, from_sha1) &&
 			    !get_sha1(next, sha1)) {
-				struct commit *exclude;
-				struct commit *include;
+				struct object *exclude;
+				struct object *include;
 
-				exclude = get_commit_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
-				include = get_commit_reference(revs, next, sha1, flags);
+				exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
+				include = get_reference(revs, next, sha1, flags);
 				if (!exclude || !include)
 					die("Invalid revision range %s..%s", arg, next);
-				add_one_commit(exclude, revs);
-				add_one_commit(include, revs);
+				add_pending_object(revs, exclude, this);
+				add_pending_object(revs, include, next);
 				continue;
 			}
 			*dotdot = '.';
@@ -689,16 +764,16 @@ int setup_revisions(int argc, const char
 			revs->prune_data = get_pathspec(revs->prefix, argv + i);
 			break;
 		}
-		commit = get_commit_reference(revs, arg, sha1, flags ^ local_flags);
-		add_one_commit(commit, revs);
+		object = get_reference(revs, arg, sha1, flags ^ local_flags);
+		add_pending_object(revs, object, arg);
 	}
-	if (def && !revs->commits) {
+	if (def && !revs->pending_objects) {
 		unsigned char sha1[20];
-		struct commit *commit;
+		struct object *object;
 		if (get_sha1(def, sha1) < 0)
 			die("bad default revision '%s'", def);
-		commit = get_commit_reference(revs, def, sha1, 0);
-		add_one_commit(commit, revs);
+		object = get_reference(revs, def, sha1, 0);
+		add_pending_object(revs, object, def);
 	}
 
 	if (revs->topo_order || revs->unpacked)
@@ -708,13 +783,37 @@ int setup_revisions(int argc, const char
 		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
 		revs->prune_fn = try_to_simplify_commit;
 	}
+	if (revs->combine_merges) {
+		revs->ignore_merges = 0;
+		if (revs->dense_combined_merges)
+			revs->diffopt.output_format = DIFF_FORMAT_PATCH;
+	}
+	if (revs->diffopt.output_format == DIFF_FORMAT_PATCH)
+		revs->diffopt.recursive = 1;
+	if (!revs->full_diff && revs->prune_data)
+		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
+	diff_setup_done(&revs->diffopt);
 
 	return left;
 }
 
 void prepare_revision_walk(struct rev_info *revs)
 {
-	sort_by_date(&revs->commits);
+	struct object_list *list;
+
+	list = revs->pending_objects;
+	revs->pending_objects = NULL;
+	while (list) {
+		struct commit *commit = handle_commit(revs, list->item, list->name);
+		if (commit) {
+			if (!(commit->object.flags & SEEN)) {
+				commit->object.flags |= SEEN;
+				insert_by_date(commit, &revs->commits);
+			}
+		}
+		list = list->next;
+	}
+
 	if (revs->limited)
 		limit_list(revs);
 	if (revs->topo_order)
diff --git a/revision.h b/revision.h
index 8970b57..9a45986 100644
--- a/revision.h
+++ b/revision.h
@@ -38,6 +38,24 @@ struct rev_info {
 			boundary:1,
 			parents:1;
 
+	/* Diff flags */
+	unsigned int	diff:1,
+			full_diff:1,
+			show_root_diff:1,
+			no_commit_id:1,
+			verbose_header:1,
+			ignore_merges:1,
+			combine_merges:1,
+			dense_combined_merges:1,
+			always_show_header:1;
+
+	/* Format info */
+	unsigned int	abbrev_commit:1;
+	unsigned int	abbrev;
+	enum cmit_fmt	commit_format;
+	const char	*header_prefix;
+	const char	*header;
+
 	/* special limits */
 	int max_count;
 	unsigned long max_age;

^ permalink raw reply related

* Re: Recent unresolved issues: shallow clone
From: Johannes Schindelin @ 2006-04-15  0:17 UTC (permalink / raw)
  To: Carl Worth; +Cc: Junio C Hamano, git
In-Reply-To: <87irpb7oma.wl%cworth@cworth.org>

Hi,

On Fri, 14 Apr 2006, Carl Worth wrote:

> I also read over some of your discussion of extending the protocol
> with a new "shallow" extension.
> 
> I'm wondering if the shallow clone support couldn't be achieved
> through a simpler tweak to the protocol semantics, (and no change to
> protocol syntax), that would avoid the problem above. Specifically,
> for shallow stuff, could we just do the same "want" and "have"
> conversation with tree objects rather than commit objects?

It would not help your problem at all. "have commit" really means that you 
have the commit and all its ancestors and their combined tree objects and 
the combined tree objects' blob objects.

If you have a cauterized history, you know that you are lacking some of 
them. But you don't know which ones.

Now, issuing a pull could mean to get an object which was present in an 
old revision, which you unfortunately do not have (because you have a cut 
off history). Boom.

I know, this is probably unlikely, but not at all *impossible*, so you 
have to take care of that case. And you need a protocol extension for 
that.

Hth,
Dscho

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  0:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0604141637230.3701@g5.osdl.org>



On Fri, 14 Apr 2006, Linus Torvalds wrote:
> 
> It's nice to note how simple and straightforward this makes the built-in 
> "git log" command, even though it continues to support all the diff flags 
> too. It doesn't get much simpler that this.

Gaah. Missed this important part, which causes the thing to ignore the 
"--pretty=xyzzy" argument, since it would always use its own default 
format that is no longer ever changed.

I even tested that it works for git-diff-tree, just not for git log. Duh.

(Found the hard way - after I had already used the broken git version for 
doing several merges, and the "--pretty=oneline" format didn't work and 
screwed up the merge message ;)

		Linus

----
diff --git a/git.c b/git.c
index e8d1fcc..d5a4a24 100644
--- a/git.c
+++ b/git.c
@@ -283,9 +283,6 @@ static int cmd_log(int argc, const char 
 	struct rev_info rev;
 	struct commit *commit;
 	char *buf = xmalloc(LOGSIZE);
-	static enum cmit_fmt commit_format = CMIT_FMT_DEFAULT;
-	int abbrev = DEFAULT_ABBREV;
-	int abbrev_commit = 0;
 	const char *commit_prefix = "commit ";
 	int shown = 0;
 
@@ -298,11 +295,11 @@ static int cmd_log(int argc, const char 
 	prepare_revision_walk(&rev);
 	setup_pager();
 	while ((commit = get_revision(&rev)) != NULL) {
-		if (shown && rev.diff && commit_format != CMIT_FMT_ONELINE)
+		if (shown && rev.diff && rev.commit_format != CMIT_FMT_ONELINE)
 			putchar('\n');
 		fputs(commit_prefix, stdout);
-		if (abbrev_commit && abbrev)
-			fputs(find_unique_abbrev(commit->object.sha1, abbrev),
+		if (rev.abbrev_commit && rev.abbrev)
+			fputs(find_unique_abbrev(commit->object.sha1, rev.abbrev),
 			      stdout);
 		else
 			fputs(sha1_to_hex(commit->object.sha1), stdout);
@@ -325,12 +322,12 @@ static int cmd_log(int argc, const char 
 			     parents = parents->next)
 				parents->item->object.flags &= ~TMP_MARK;
 		}
-		if (commit_format == CMIT_FMT_ONELINE)
+		if (rev.commit_format == CMIT_FMT_ONELINE)
 			putchar(' ');
 		else
 			putchar('\n');
-		pretty_print_commit(commit_format, commit, ~0, buf,
-				    LOGSIZE, abbrev);
+		pretty_print_commit(rev.commit_format, commit, ~0, buf,
+				    LOGSIZE, rev.abbrev);
 		printf("%s\n", buf);
 		if (rev.diff)
 			log_tree_commit(&rev, commit);

^ permalink raw reply related

* Re: Recent unresolved issues: shallow clone
From: Junio C Hamano @ 2006-04-15  0:25 UTC (permalink / raw)
  To: Carl Worth; +Cc: git
In-Reply-To: <87irpb7oma.wl%cworth@cworth.org>

Carl Worth <cworth@cworth.org> writes:

> On Fri, 14 Apr 2006 02:31:36 -0700, Junio C Hamano wrote:
>>   I am beginning to think using "graft" to cauterize history
>>   for this, while it technically would work, would not be so
>>   helpful to users, so the design needs to be worked out again.
>
> As context, here is some of what you mentioned in IRC:
>
>>>	Suppose you have this:
>>>
>>>	A---B---C
>>>	 \       \ 
>>>	  D---E---F---G
>>>	 
>>>	and you made a shallow clone of C (because that is where the
>>>	upstream master was when you made that clone).  Then the
>>>	upstream updated the master branch tip to G.
>>>
>>>	The next update from upstream to your shallow clone would break.
>>>	The upstream says: I have G at master.
>>>	You say: I want G then.  By the way, I have C.
>>>
>>>	What it means to tell the other end "I have X" is to promise
>>>	that you have X and _everything_ behind it.  So the upstream
>>>	would send objects necessary to complete D, E, F and G for
>>>	"somebody who already have A and B".  As a consequence, you
>>>	would not see A nor B.
>>>
>>>	Even if the only thing you are interested in is to be in sync
>>>	with the tip of the upstream, you can end up with an
>>>	incomplete tree for G, if some of the blobs or trees contained
>>>	in G already exist in A or B.  They are not sent -- because
>>>	you told the upstream that you have everything necessary to
>>>	get to C.
>
> So that's an argument against using a cauterizing graft for the
> shallow clone of C. It definitely confuses the existing protocol to
> say "I have C" if I have only a cauterized C, (its tree only, but none
> of the commits that should be backing C).

That's what I meant by "graft technically works but is
inconvenient". 

Maybe after the update to G happens (which means you now have C,
F, G but not A B D E commits), the client side could enumerate
commits on "rev-list ^C G" and cauterize the ones with missing
parents (in this case, F does not have one of its parents).
While doing this would help keeping the resulting commit
ancestry sane, it does not solve the problem of missing blobs
and trees.  See below.

> So, in the scenario above, the original shallow clone of C would be:
>
> 	Want C->tree, have nothing.
>
> and the later shallow update to G would be:
>
> 	Want G->tree, have C->tree

When you ask for G, you do not know what G^{tree} is, so that is
fantasy without a protocol extention.  To solve the missing
blobs/trees problem we would probably need a protocol extention
that says it wants to receive enough data to complete trees and
blobs associated with the commits being sent _without_ assuming
the recipient has any trees or blobs other than what are
contained in "have" commits.  Then after such a successful
transfer, missing parents of commits listed in "rev-list ^C G"
are the ones from the side branch, so the client can cauterize
them (F in the above example) appropriately without bothering
the server.

However, I think this "do not assume I have any trees behind the
commits I explicitly say I have" must be an option, because it
makes the resulting transfer unnecessarily more expensive for
normal uses.  A fetch of the Linux kernel once a day would
update about a couple of hundered commits, each of which touches
only 3 paths on average (so that would be 600 files out of
18,000 file tree.  When side-branch merges are involved, usually
many things in G (and F) are unchanged since either A or C, but
the extention we are discussing forbids reusing what are found
in A (it still allows reusing what are found in C).

> A final step of a shallow clone would then require creating a new
> parent-less commit object so that there's something to point refs/head
> at, (or maybe rather than being parentless, they could be chained
> together with each update?).

Rewriting commit objects transferred to the cloner is something
you would _not_ want to do (e.g. rewriting F commits to say it
has only one parent C).  The history based on that would diverge
from parents and would become unmergeable.  It is cleaner to
just make a new graft entry to say "As far as this repository is
concerned, F has one parent C".  Shallowness of the repository
and its slightly different view of history is a local matter.

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  0:38 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141637230.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 14 Apr 2006, Junio C Hamano wrote:
>> 
>> * Message-ID: <Pine.LNX.4.64.0604121828370.14565@g5.osdl.org>
>>   Common option parsing (Linus Torvalds)
>
> Ok, here's a first cut at starting this.

Hmph.  You did it while I was still thinking about it ;-).

I was thinking long because I had an impression that anything
based on revision.c interface, if it wants to do a tree-diff on
the commit stream, would need two different diff options.  One
is used by revision.c internally so that it can use its own
add_remove/change for parent pruning, and another to control the
way diff is run by the user of revision.c.

I'll take a look at it tonight.  Thanks.

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  0:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0604141717280.3701@g5.osdl.org>



On Fri, 14 Apr 2006, Linus Torvalds wrote:
> 
> Gaah. Missed this important part, which causes the thing to ignore the 
> "--pretty=xyzzy" argument, since it would always use its own default 
> format that is no longer ever changed.

And here's one more fixup: get the default format right, and don't prefix 
the "oneline" format.

		Linus

----
diff --git a/git.c b/git.c
index d5a4a24..437e9b5 100644
--- a/git.c
+++ b/git.c
@@ -291,6 +291,8 @@ static int cmd_log(int argc, const char 
 		die("unrecognized argument: %s", argv[1]);
 
 	rev.no_commit_id = 1;
+	if (rev.commit_format == CMIT_FMT_ONELINE)
+		commit_prefix = "";
 
 	prepare_revision_walk(&rev);
 	setup_pager();
diff --git a/revision.c b/revision.c
index 99077af..0f98960 100644
--- a/revision.c
+++ b/revision.c
@@ -493,7 +493,7 @@ void init_revisions(struct rev_info *rev
 	revs->topo_getter = topo_sort_default_getter;
 
 	revs->header_prefix = "";
-	revs->commit_format = CMIT_FMT_RAW;
+	revs->commit_format = CMIT_FMT_DEFAULT;
 
 	diff_setup(&revs->diffopt);
 }

^ permalink raw reply related

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  0:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlku7n05x.fsf@assigned-by-dhcp.cox.net>



On Fri, 14 Apr 2006, Junio C Hamano wrote:
> 
> I was thinking long because I had an impression that anything
> based on revision.c interface, if it wants to do a tree-diff on
> the commit stream, would need two different diff options.  One
> is used by revision.c internally so that it can use its own
> add_remove/change for parent pruning, and another to control the
> way diff is run by the user of revision.c.

I think you're right, and I've probably broken "--full-diff" (causing the 
revparse to also use the empty set of paths). Gaah.

		Linus

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  0:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141748070.3701@g5.osdl.org>



On Fri, 14 Apr 2006, Linus Torvalds wrote:
> 
> I think you're right, and I've probably broken "--full-diff" (causing the 
> revparse to also use the empty set of paths). Gaah.

In fact, it's broken path-limited revisions entirely. Duh. We should have 
a test for that, so I would have noticed.

I think we need two diffopt structures there - one for the actual diff, 
and one for the pruning.

		Linus

^ permalink raw reply

* RFE:  Fix IMAP-send TCP connect
From: Jeff Garzik @ 2006-04-15  1:10 UTC (permalink / raw)
  To: Git Mailing List


Request:  Somebody please fix imap-send.c to use git_tcp_connect() so 
that it supports IPv6 and getaddrinfo().

Bonus points for also fixing:

On Linux, daemon should use the default Linux behavior, binding to both 
IPv4 and IPv6 sockets at the same time.  git is actually doing _more 
work_ to avoid this.

	Jeff

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141751270.3701@g5.osdl.org>



Ok, fourth time lucky?

		Linus

----
diff --git a/revision.c b/revision.c
index 0f98960..2061ca8 100644
--- a/revision.c
+++ b/revision.c
@@ -246,7 +246,7 @@ int rev_compare_tree(struct rev_info *re
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
-			   &revs->diffopt) < 0)
+			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
 	return tree_difference;
 }
@@ -269,7 +269,7 @@ int rev_same_tree_as_empty(struct rev_in
 	empty.size = 0;
 
 	tree_difference = 0;
-	retval = diff_tree(&empty, &real, "", &revs->diffopt);
+	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
 	return retval >= 0 && !tree_difference;
@@ -476,9 +476,9 @@ static void handle_all(struct rev_info *
 void init_revisions(struct rev_info *revs)
 {
 	memset(revs, 0, sizeof(*revs));
-	revs->diffopt.recursive = 1;
-	revs->diffopt.add_remove = file_add_remove;
-	revs->diffopt.change = file_change;
+	revs->pruning.recursive = 1;
+	revs->pruning.add_remove = file_add_remove;
+	revs->pruning.change = file_change;
 	revs->lifo = 1;
 	revs->dense = 1;
 	revs->prefix = setup_git_directory();
@@ -780,8 +780,10 @@ int setup_revisions(int argc, const char
 		revs->limited = 1;
 
 	if (revs->prune_data) {
-		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
+		diff_tree_setup_paths(revs->prune_data, &revs->pruning);
 		revs->prune_fn = try_to_simplify_commit;
+		if (!revs->full_diff)
+			diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
 	}
 	if (revs->combine_merges) {
 		revs->ignore_merges = 0;
@@ -790,8 +792,6 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->diffopt.output_format == DIFF_FORMAT_PATCH)
 		revs->diffopt.recursive = 1;
-	if (!revs->full_diff && revs->prune_data)
-		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
 	diff_setup_done(&revs->diffopt);
 
 	return left;
diff --git a/revision.h b/revision.h
index 9a45986..6eaa904 100644
--- a/revision.h
+++ b/revision.h
@@ -61,8 +61,9 @@ struct rev_info {
 	unsigned long max_age;
 	unsigned long min_age;
 
-	/* paths limiting */
+	/* diff info for patches and for paths limiting */
 	struct diff_options diffopt;
+	struct diff_options pruning;
 
 	topo_sort_set_fn_t topo_setter;
 	topo_sort_get_fn_t topo_getter;

^ permalink raw reply related

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  1:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141748070.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 14 Apr 2006, Junio C Hamano wrote:
>> 
>> I was thinking long because I had an impression that anything
>> based on revision.c interface, if it wants to do a tree-diff on
>> the commit stream, would need two different diff options.  One
>> is used by revision.c internally so that it can use its own
>> add_remove/change for parent pruning, and another to control the
>> way diff is run by the user of revision.c.
>
> I think you're right, and I've probably broken "--full-diff" (causing the 
> revparse to also use the empty set of paths). Gaah.

Another thing is that some revision.c users are not interested
in taking diff options at all.

I was going to suggest a new structure that captures struct
rev_info, struct log_tree_opt and miscellaneous bits cmd_log
uses such as do_diff, full_diff, etc., and move the option
parser out of cmd_log() to a separate function, and have that
shared across cmd_log(), cmd_show(), cmd_whatchanged(), and
cmd_diff() without affecting any of the existing revision.c
users.  That way, "rev-list --cc HEAD" will remain nonsense.

One nice property your approach has is that it makes
"git diff-tree a..b" magically starts working, unlike what
I suggested above.

^ permalink raw reply

* Re: Recent unresolved issues: shallow clone
From: Junio C Hamano @ 2006-04-15  2:11 UTC (permalink / raw)
  To: Carl Worth; +Cc: git
In-Reply-To: <7vr73zn0rb.fsf@assigned-by-dhcp.cox.net>

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

> Maybe after the update to G happens (which means you now have C,
> F, G but not A B D E commits), the client side could enumerate
> commits on "rev-list ^C G" and cauterize the ones with missing
> parents (in this case, F does not have one of its parents).
> While doing this would help keeping the resulting commit
> ancestry sane, it does not solve the problem of missing blobs
> and trees.  See below.

Actually, it is more involved than the above.

The sender would give you A B D E as well, so we would not be
able to cauterise at F; instead you would do so at A, making
your shallow clone a bit deeper.  When you look at the objects
the parent gave you by running "rev-list ^C G", you would notice
that you do not have any of real parents of A, and add a new
graft.  While you are at it, you would hopefully notice that the
real parent of commit C is something you now have -- so you
remove the graft entry for C.

However, depending on what you care about more, having the
sender not to send the side branch and cauterising the result at
F _might_ be a better thing to do.  This is probably quite
involved and I offhand would not know how to efficiently
compute.

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  2:22 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141808480.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Ok, fourth time lucky?
>
> 		Linus

With this, perhaps.

---
diff --git a/revision.c b/revision.c
index 2061ca8..1d26e0d 100644
--- a/revision.c
+++ b/revision.c
@@ -792,6 +792,7 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->diffopt.output_format == DIFF_FORMAT_PATCH)
 		revs->diffopt.recursive = 1;
+	revs->diffopt.abbrev = revs->abbrev;
 	diff_setup_done(&revs->diffopt);
 
 	return left;

^ permalink raw reply related

* [PATCH] Fix a warning.
From: Mike McCormack @ 2006-04-15  3:50 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

Signed-off-by: Mike McCormack <mike@codeweavers.com>


---

  diff-tree.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)


[-- Attachment #2: 2fa977d19bee245aef86d9beb307d742111890d7.diff --]
[-- Type: text/x-patch, Size: 476 bytes --]

2fa977d19bee245aef86d9beb307d742111890d7
diff --git a/diff-tree.c b/diff-tree.c
index 2b79dd0..7015b06 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -120,7 +120,7 @@ int main(int argc, const char **argv)
 	if (opt->diffopt.output_format == DIFF_FORMAT_PATCH)
 		opt->diffopt.recursive = 1;
 
-	diff_tree_setup_paths(get_pathspec(prefix, argv), opt);
+	diff_tree_setup_paths(get_pathspec(prefix, argv), &opt->diffopt);
 	diff_setup_done(&opt->diffopt);
 
 	switch (nr_sha1) {


^ permalink raw reply related

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  4:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vacanmxhe.fsf@assigned-by-dhcp.cox.net>



On Fri, 14 Apr 2006, Junio C Hamano wrote:
> 
> Another thing is that some revision.c users are not interested
> in taking diff options at all.

Well, it's easy enough to do something like

	if (rev->diff)
		usage(no_diff_cmd_usage);

for something like that.

> I was going to suggest a new structure that captures struct
> rev_info, struct log_tree_opt and miscellaneous bits cmd_log
> uses such as do_diff, full_diff, etc., and move the option
> parser out of cmd_log() to a separate function, and have that
> shared across cmd_log(), cmd_show(), cmd_whatchanged(), and
> cmd_diff() without affecting any of the existing revision.c
> users.  That way, "rev-list --cc HEAD" will remain nonsense.

Well, I actually was going to make git-rev-list just take the diff 
options, and it ends up doing the same thing as "git log" with them. 
There's no real downside.

> One nice property your approach has is that it makes
> "git diff-tree a..b" magically starts working, unlike what
> I suggested above.

Yeah. It just fell out automatically from using the rev-list parsing.

Although, the thing is, once we have a built-in "git diff", there's 
actually little enough reason to ever use the old "git-diff-tree" vs 
"git-diff-index" vs "git-diff-files" at all. 

It might actually be nice to prune some of the tons of git commands. At 
some point, the fact that

	echo bin/git-* | wc -w

returns 122 just makes you go "Hmm..".

		Linus

^ permalink raw reply

* Clean up trailing whitespace when pretty-printing commits
From: Linus Torvalds @ 2006-04-15  4:20 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Partly because we've messed up and now have some commits with trailing 
whitespace, but partly because this also just simplifies the code, let's 
remove trailing whitespace from the end when pretty-printing commits.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/commit.c b/commit.c
index c7bb8db..ca25574 100644
--- a/commit.c
+++ b/commit.c
@@ -557,16 +557,11 @@ unsigned long pretty_print_commit(enum c
 		if (fmt == CMIT_FMT_ONELINE)
 			break;
 	}
-	if (fmt == CMIT_FMT_ONELINE) {
-		/* We do not want the terminating newline */
-		if (buf[offset - 1] == '\n')
-			offset--;
-	}
-	else {
-		/* Make sure there is an EOLN */
-		if (buf[offset - 1] != '\n')
-			buf[offset++] = '\n';
-	}
+	while (offset && isspace(buf[offset-1]))
+		offset--;
+	/* Make sure there is an EOLN for the non-oneline case */
+	if (fmt != CMIT_FMT_ONELINE)
+		buf[offset++] = '\n';
 	buf[offset] = '\0';
 	return offset;
 }

^ permalink raw reply related

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  5:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604142104140.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Well, it's easy enough to do something like
>
> 	if (rev->diff)
> 		usage(no_diff_cmd_usage);
>
> for something like that.

You're right.  I've swallowed all four patches with a fixlet on
top; thanks.

> Although, the thing is, once we have a built-in "git diff", there's 
> actually little enough reason to ever use the old "git-diff-tree" vs 
> "git-diff-index" vs "git-diff-files" at all. 

True, unless you are writing a Porcelain, that is.

> It might actually be nice to prune some of the tons of git commands. At 
> some point, the fact that
>
> 	echo bin/git-* | wc -w
>
> returns 122 just makes you go "Hmm..".

Yes, but I thought the plan to deal with that was to set gitexecdir
somewhere other than $(prefix)/bin; removing git-diff-* siblings
would be unfriendly to Porcelains.

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  6:18 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141751270.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 14 Apr 2006, Linus Torvalds wrote:
>> 
>> I think you're right, and I've probably broken "--full-diff" (causing the 
>> revparse to also use the empty set of paths). Gaah.
>
> In fact, it's broken path-limited revisions entirely. Duh. We should have 
> a test for that, so I would have noticed.
>
> I think we need two diffopt structures there - one for the actual diff, 
> and one for the pruning.

Although I've already decided to merge it up, there are small
fallout from this.  I've fixed the ones I noticed, but there
probably remain some backward compatibility issues in commands
that I do not usually use.  We'll see.

Also I merged the commit prettyprinter change, but I was hoping
we could instead pass down the commit object and commit format
to places where log message needs to be output and write it out
to the standard output instead of formatting in core.

^ 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