Recent unresolved issues

Here is a list of topics in the recent git traffic that I feel
inadequately addressed.  I've commented on some of them to give
people a feel for what my priorities are.  Somebody might want
to rehash the ones low on my priority list to conclusion with a
concrete proposal if they cared about them enough.  The list is
*not* ordered in any way.

Also please add whatever I missed (or dismissed).  I am hoping
this will be a good basis for 1.4 to-do list.

* Message-ID: [off-list ref]
  Common option parsing (Linus Torvalds)

* Message-ID: [off-list ref]
  Binary diff output? (Nicolas Pitre)

  I do not think this is needed for our primary audience (the
  kernel project), but I am sure it would be helpful for some
  other projects if we allowed them to exchange patches that
  describe binary file changes via e-mail, so I am not
  dismissing this.  Needs to wait "option parsing".

* Message-ID: [off-list ref]
  Colored diff? (Linus Torvalds)

  I am not opposed to it, but I'd like to do that internally if
  we go this route.  Needs to wait "option parsing".  Also
  Message-ID: <3536.10.10.10.24.1114117965.squirrel@linux1> is
  slightly related to this.

* Message-ID: [off-list ref]
  diff --with-raw, --with-stat? (me)

  I think "git diff" can be internalized next, after "option
  parsing" unification.  When that is done, --with-stat would
  help internalize format-patch's process_one(), and it would be
  trivial to do "git log --pretty=format-patch master..next".

* #irc 2006-04-10
  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.

* Message-ID: [off-list ref]
  git status does not distinguish contents changes and mode
  changes; it just says "modified" (Jon Loeliger).

  Unconditionally changing the status letter would break
  Porcelains so we would need an extra option to do this.
  An outline patch has been already prepared -- this perhaps has
  to wait until we sort out the "option parsing" one.

* Message-ID: [off-list ref]
  git could use diff3 instead of merge which is a wrapper around
  diff3. (Catalin Marinas)

  If having "diff3" is a lot more common than having "merge", I
  do not have problem with this; "merge" being a wrapper to
  "diff3", people who have been happy with the current code
  would certainly have "diff3" installed so changing to "diff3"
  would not break them.

* Message-ID: [off-list ref]
  Windows problems summary (Alex Riesen)

  A good list to keep in mind.

* Message-ID: [off-list ref]
  Huge packfiles (Linus Torvalds)

  Because I do not think asking users to break up packs to
  manageable and mmap()able size is too much to ask, I would not
  be advocating for updating the pack idx to 64-bit offset and
  mmap()ing parts of a packfile, at least too strongly.

  However, we currently lack tool support or recepe for users
  with such a repository to easily break up packs.

* Message-ID: <1143856098.3555.48.camel@dv>
  Per branch property, esp. where to merge from (Pavel Roskin)

  This involves user-level "world model" design, which is more
  Porcelainish than Plumbing, and as people know I do not do
  Porcelain well; interested parties need to come up with what
  they want and how they want to use it.

Dear diary, on Fri, Apr 14, 2006 at 11:31:36AM CEST, I got a letter
where Junio C Hamano [off-list ref] said that...

Here is a list of topics in the recent git traffic that I feel
inadequately addressed.  I've commented on some of them to give
people a feel for what my priorities are.  Somebody might want
to rehash the ones low on my priority list to conclusion with a
concrete proposal if they cared about them enough.  The list is
*not* ordered in any way.

Nice summary!

* Message-ID: [off-list ref]
  git could use diff3 instead of merge which is a wrapper around
  diff3. (Catalin Marinas)

  If having "diff3" is a lot more common than having "merge", I
  do not have problem with this; "merge" being a wrapper to
  "diff3", people who have been happy with the current code
  would certainly have "diff3" installed so changing to "diff3"
  would not break them.

I've decided to bite the bullet and made Cogito use diff3 instead of
merge as of now. Let's see if anybody complains...

* Message-ID: <1143856098.3555.48.camel@dv>
  Per branch property, esp. where to merge from (Pavel Roskin)

  This involves user-level "world model" design, which is more
  Porcelainish than Plumbing, and as people know I do not do
  Porcelain well; interested parties need to come up with what
  they want and how they want to use it.

-- 
				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.

On Fri, 14 Apr 2006 02:31:36 -0700
Junio C Hamano [off-list ref] wrote:

* Message-ID: [off-list ref]
  Colored diff? (Linus Torvalds)

  I am not opposed to it, but I'd like to do that internally if
  we go this route.  Needs to wait "option parsing".  Also
  Message-ID: <3536.10.10.10.24.1114117965.squirrel@linux1> is
  slightly related to this.

Moving it internal sounds like a good idea.  Would you be open to
including the GIT_DIFF_PAGER option now anyway?   It has utility
beyond just color diffs.

Sean

Dear diary, on Fri, Apr 14, 2006 at 11:31:36AM CEST, I got a letter
where Junio C Hamano [off-list ref] said that...

* Message-ID: [off-list ref]
  Colored diff? (Linus Torvalds)

  I am not opposed to it, but I'd like to do that internally if
  we go this route.  Needs to wait "option parsing".  Also
  Message-ID: <3536.10.10.10.24.1114117965.squirrel@linux1> is
  slightly related to this.

It might be worthwhile to make Git and Cogito compatible if you offer
colors customization. Cogito lets the user customize the colors through
the $CG_COLORS variable (see cg-diff(1)).

-- 
				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.

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:

quoted

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.

On Fri, 14 Apr 2006, Junio C Hamano wrote:

* Message-ID: [off-list ref]
  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;

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

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);

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);
 }

Here is a list of topics in the recent git traffic that I feel
inadequately addressed.  I've commented on some of them to give
people a feel for what my priorities are.  Somebody might want
to rehash the ones low on my priority list to conclusion with a
concrete proposal if they cared about them enough.

The list is *not* ordered in any way, except that the entries
kept from the previous issue of this message have been pushed
down to the bottom.  I will probably start dropping some entries
that did not get any reaction from the list in future issues of
this message, but for now I kept all of them from the first one.

* Message-ID: [off-list ref]
  Content-type charset for send-email (Bertrand Jacquin)

  The output from format-patch by default is unmarked, which
  means the commit message part is UTF-8 (by strong convention),
  and the contents of the diff is whatever the contents of the
  file is encoded in.

  David Woodhouse did a patch to allow specifying charset on the
  command line (and default to UTF-8) which is a move in the
  right direction, but Bertrand's system seems to have trouble
  with it.

  I think if we were to do this we probably need to teach
  format-patch to optionally do multi-part.  We may not
  necessarily want to mark the payload to be in the same
  encoding as the commit message (not that git-apply cares -- to
  it, the payload is just 8-bit unencoded text, but we would
  want to protect it from getting mangled by e-mail transport).

* Message-ID: [off-list ref]
  Perhaps "note" field in commit objects are useful?

* Message-ID: [off-list ref]

  An optional "git fetch --store newname URL refspecs..." to
  create an equivalent of remotes file so newname can then be
  used as a short-hand.  I still have somewhat negative reaction
  to it, but I am willing to apply it if there are enough people
  who want this.

-- carried over from the first issue of this list.

* Message-ID: [off-list ref]
  Binary diff output? (Nicolas Pitre)

  I do not think this is needed for our primary audience (the
  kernel project), but I am sure it would be helpful for some
  other projects if we allowed them to exchange patches that
  describe binary file changes via e-mail, so I am not
  dismissing this.

* #irc 2006-04-10
  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.

* Message-ID: [off-list ref]
  git status does not distinguish contents changes and mode
  changes; it just says "modified" (Jon Loeliger).

  Unconditionally changing the status letter would break
  Porcelains so we would need an extra option to do this.
  An outline patch has been already prepared -- this perhaps has
  to wait until we sort out the "option parsing" one.

* Message-ID: [off-list ref]
  git could use diff3 instead of merge which is a wrapper around
  diff3. (Catalin Marinas)

  If having "diff3" is a lot more common than having "merge", I
  do not have problem with this; "merge" being a wrapper to
  "diff3", people who have been happy with the current code
  would certainly have "diff3" installed so changing to "diff3"
  would not break them.

* Message-ID: [off-list ref]
  Windows problems summary (Alex Riesen)

  A good list to keep in mind.

* Message-ID: [off-list ref]
  Huge packfiles (Linus Torvalds)

  Because I do not think asking users to break up packs to
  manageable and mmap()able size is too much to ask, I would not
  be advocating for updating the pack idx to 64-bit offset and
  mmap()ing parts of a packfile, at least too strongly.

  However, we currently lack tool support or recepe for users
  with such a repository to easily break up packs.

* Message-ID: <1143856098.3555.48.camel@dv>
  Per branch property, esp. where to merge from (Pavel Roskin)

  This involves user-level "world model" design, which is more
  Porcelainish than Plumbing, and as people know I do not do
  Porcelain well; interested parties need to come up with what
  they want and how they want to use it.

Junio C Hamano wrote:

* #irc 2006-04-10
  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.

Perhaps use comment for marking graft as cauterizing history?

There was also talk about proposed git-splithist, which would move some of
the history to other (historical, archive) repository.

-- 
Jakub Narebski
Warsaw, Poland

Dear diary, on Thu, May 04, 2006 at 10:15:03AM CEST, I got a letter
where Junio C Hamano [off-list ref] said that...

* Message-ID: <1143856098.3555.48.camel@dv>
  Per branch property, esp. where to merge from (Pavel Roskin)

  This involves user-level "world model" design, which is more
  Porcelainish than Plumbing, and as people know I do not do
  Porcelain well; interested parties need to come up with what
  they want and how they want to use it.

Oh, my holey memory. In Cogito, I have just implemented a solution
suggested by Martin Mares, which is pretty simple, non-obtrusive
and will work equally fine with remotes as well as remote branches:

	if [ $branch != master ] && [ -s .git/branches/$branch-origin ]
		origin=.git/branches/$branch-origin
	else
		origin=.git/branches/origin
	fi

-- 
				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.

Hello, Petr!

On Thu, 2006-05-04 at 11:58 +0200, Petr Baudis wrote:

Oh, my holey memory. In Cogito, I have just implemented a solution
suggested by Martin Mares, which is pretty simple, non-obtrusive
and will work equally fine with remotes as well as remote branches:

	if [ $branch != master ] && [ -s .git/branches/$branch-origin ]
		origin=.git/branches/$branch-origin
	else
		origin=.git/branches/origin
	fi

Isn't ".git/branches" obsolete, at least in git?  I'm surprised it's
still referenced in git sources.

What is the future of ".git/branches"?  Is it becoming a Cogito specific
branch database?  Or is it now a database or branch dependencies?

I would prefer to have one single standard for branch origins that could
be used by git, StGIT and Cogito.  Using a location that is obsolete
outside Cogito is probably the worst possible approach.  I'd rather have
a separate directory, e.g. .git/origins or something.

Alternatively, we could reuse .git/refs by having files with "Pull:" but
without "URI:", e.g.

$ cat .git/refs/branch
Pull: branch-origin:branch

-- 
Regards,
Pavel Roskin

On Thu, 04 May 2006 01:15:03 -0700, Junio C Hamano wrote:

* #irc 2006-04-10
  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.

I've been meaning to follow up with some thoughts on this topic, so
thanks for the tickler.

For the one use case I had, (track latest tree), I had thrown out the
idea of using "faked", parent-less commit objects to point to the tree
of interest. Junio pointed out that there's no protocol to learn the
name of a remote commit's tree from the name of the commit. I worked
around that by simply making the parent-less commit object on the
server side, (branch name of "master-shallow", say).

That seemed to work just fine, and if someone really wanted to do
this, they could use a hook to maintain the master-shallow branch,
and no change to git itself would be needed. But there's a very
minimal amount of interesting functionality in this, and it's not
clear that it's much better than git-tar-tree. So I'm considering that
idea dead.

Meanwhile, a more general ability to use shallow clones would still be
very useful. I think what I'd like to be able to do is to pass
rev-list limiting options (--max-count, --max-age via --since,
etc.). That would limit the expansion of the WANT commits, and then
the existing logic to compute the necessary objects needed to satisfy
the list of desired commits should do the right thing.

Then, in order for this to actually be useful, when returning objects
from a limited fetch like this, the server should provide a list of
commits that should be noted as cauterized, (whether through the
existing grafts mechanism or otherwise).

Additionally, when doing a fetch into a tree that has any such
cauterized commits, the client must also provide its list of
cauterized commits. So the conversation changes from "I WANT
<fetch-heads> and I HAVE <heads>" to one of "I WANT <fetch-heads>, and
I HAVE <heads>, except that I'm MISSING <cauterized-commits>".

Finally, whenever a fetch receives an commit object that is in its
list of cauterized commits, it should remove that commit from the
list. This allows a shallow clone to be naturally migrated to
something unshallow. And the user can do this as incrementally as
desired based on the need to see more history:

get a bit:
	git fetch somewhere --since=2.weeks.ago

then a bit more:
	git fetch somewhere --since=1.year.ago

then get it all:
	git fetch somewhere

Maybe that's no different from Junio's original proposal. If not, what
do you see in the above that wouldn't work?

-Carl

On Thu, 4 May 2006, Junio C Hamano wrote:

* Message-ID: [off-list ref]

  An optional "git fetch --store newname URL refspecs..." to
  create an equivalent of remotes file so newname can then be
  used as a short-hand.  I still have somewhat negative reaction
  to it, but I am willing to apply it if there are enough people
  who want this.

I was just about to suggest something for this general use. It's currently 
kind of a pain to deal with the situation where you've got stuff on your 
workstation that you want to version control in a shared repository on a 
server.

I think it shouldn't be on fetch, though; I think a "git remote" command 
for describing, creating, and modifying remotes would be better, since you 
also sometimes want to add a "Push:" line.

Maybe:

 git remote <name>: Print info about <name>
 git remote add <name> <URL> [<direction> ...]: create a remote
 git remote <name> <direction> ...: modify a remote

 where <direction> is either:
  pull <remote> <local> or
  push <local> <remote>

	-Daniel
*This .sig left intentionally blank*

On Thu, 4 May 2006, Daniel Barkalow wrote:

I think it shouldn't be on fetch, though; I think a "git remote" command 
for describing, creating, and modifying remotes would be better, since you 
also sometimes want to add a "Push:" line.

I don't think this is wrong, but I think it's more important to try to 
decide on how we want to represent this information first, and stabilize 
that.

I realize that git has gotten a lot more porcelainish over time, but at 
the same time, now you're really starting to argue about syntax that 
really ends up being often a feature of the development environment. If 
you did development using an IDE that knows about git, I think the 
"remote" information ends up being not necessarily a git command at all, 
but really an interface in the IDE.

I'm actually growing pretty fond of the config file interfaces that Dscho 
is pushing. I really like the idea of "git pull" doing different things 
depending on which branch is active at the time, because different 
branches really can have different sources they come from.

Always pulling from the same default source seems wrong, and having to 
remember whose source some branch is associated with is just not all that 
user-friendly, but perhaps more importantly, it's also going to result in 
people making mistakes, pulling from the wrong branch (because they didn't 
think about where they were), and then having strange merges that they 
might not notice were wrong until it's too late and they pushed the result 
out.

So Johannes' patches seem to move into that direction, and having it all 
in the config file actually seems to be quite readable.

And that, in turn, may mean that a lot of porcelains really only care 
about that syntax, and then they may update the config file any way they 
please (whether by hand, or by using "git repo-config" or by using "git 
remote").

So I'd argue that (a) yes, we do want to have the "proto porcelain" that 
sets remote branch information without the user having to know the magic 
"git repo-config" incantation, or know which file in .git/remotes/ to 
edit, but that (b) it's even more important to try to decide on what the 
remote description format _is_.

I personally have just two preferences:

 - I'd like each branch I'm on to have a "default source" for pulling (and 
   _maybe_ for pushing too). I'd like to just say "git pull", and it would 
   automatically select the appropriate thing to pull from.

 - maybe the same per-branch thing for "push", but more importantly for 
   me, I like to push to multiple destinations, and I'd like the 
   description format to be sane. I think it may already be sane in the 
   form it is in now (supporting both config file _and_ .git/remotes/ 
   formats), I'd just like us to decide on exactly what the meaning is, 
   and hopefully get to the point where we can tell porcelain how to use 
   that meaning to their advantage (and not change it)

Others may disagree, or (equally importantly), may have additional 
preferences. We should try to find something that works for everybody, and 
that is easy to work with.

		Linus

Carl Worth [off-list ref] writes:

... So the conversation changes from "I WANT
<fetch-heads> and I HAVE <heads>" to one of "I WANT <fetch-heads>, and
I HAVE <heads>, except that I'm MISSING <cauterized-commits>".

Finally, whenever a fetch receives an commit object that is in its
list of cauterized commits, it should remove that commit from the
list. This allows a shallow clone to be naturally migrated to
something unshallow. And the user can do this as incrementally as
desired based on the need to see more history:

get a bit:
	git fetch somewhere --since=2.weeks.ago

then a bit more:
	git fetch somewhere --since=1.year.ago

then get it all:
	git fetch somewhere

Maybe that's no different from Junio's original proposal. If not, what
do you see in the above that wouldn't work?

Lack of actual code to do all that ;-)

Jokes aside, I think listing the updated conversation elements
like you did above is a good step forward.

The vocabulary we would want from the requestor side is probably
(at least):

	I WANT to have these
        I HAVE these
        I'm MISSING these
        Don't bother with these this time around (--since, ^v2.6.16, ...)

I am not sure how we would want to encode the last one and have
it used by rev-list on the upload-pack end safely and sanely.

And the responder side needs to be able to say, "Now you are
MISSING these, remember it and tell me you are missing them next
time you make a request".  That would be, in the simplest case,
a list of commit IDs to cauterize, but I am not sure what is the
right way to come up with that list.  Especially I do not know
if --boundary would/should work with --objects.

On 5/5/06, Junio C Hamano [off-list ref] wrote:

The vocabulary we would want from the requestor side is probably
(at least):

        I WANT to have these
        I HAVE these
        I'm MISSING these
        Don't bother with these this time around (--since, ^v2.6.16, ...)

Thinking... does the MISSING part matter at all? It seems that what
really matters are the "ignore rules". The pull may bring in a new
merge of a long-running branch, whose mergebase falls out of the
ignore rules.

In that case, the server should apply the ignore rules. Except that
later merges in the local repo would perhaps have to deal with missing
part of the history. I suspect it should refuse to merge something we
don't have all the merging parts for.

cheers,


martin

On Fri, 5 May 2006 17:17:10 +1200, "Martin Langhoff" wrote:

quoted

        I WANT to have these
        I HAVE these
        I'm MISSING these
        Don't bother with these this time around (--since, ^v2.6.16, ...)

Thinking... does the MISSING part matter at all?

Yes.

Imagine doing a shallow clone and then fetching a tree that includes a
blob that existed before MISSING. If we say HAVE without MISSING then
the server will not send that blob and we'll be left with a broken
tree.

In that case, the server should apply the ignore rules. Except that
later merges in the local repo would perhaps have to deal with missing
part of the history. I suspect it should refuse to merge something we
don't have all the merging parts for.

Yeah, shallow clones can shake up the conventions a bit. It's
definitely common for a repository to only have a single parent-less
commit, such that there is always an identifiable merge base for any
pair of revisions. Shallow clones would make (effectively) parent-less
commits much more common.

Should be fun to see what things fall over with this...

-Carl

Carl Worth wrote:

On Fri, 5 May 2006 17:17:10 +1200, "Martin Langhoff" wrote:

quoted

In that case, the server should apply the ignore rules. Except that
later merges in the local repo would perhaps have to deal with missing
part of the history. I suspect it should refuse to merge something we
don't have all the merging parts for.

Yeah, shallow clones can shake up the conventions a bit. It's
definitely common for a repository to only have a single parent-less
commit, such that there is always an identifiable merge base for any
pair of revisions. Shallow clones would make (effectively) parent-less
commits much more common.

I wonder if it would be possible for git to:
a) as for a fetch which would bring all the commits up to the merge base
   (and merge base has to be calculated on the server side I think),
   i.e. give command to use (for fetch or for force baseless merge)
b) fetch the commits
c) do merge
d) optionally re-cauterize history again

-- 
Jakub Narebski
Warsaw, Poland

On Thu, 4 May 2006, Junio C Hamano wrote:

Jokes aside, I think listing the updated conversation elements
like you did above is a good step forward.

The vocabulary we would want from the requestor side is probably
(at least):

	I WANT to have these
        I HAVE these
        I'm MISSING these
        Don't bother with these this time around (--since, ^v2.6.16, ...)

Actually, I think we can do something simpler that _most_ people might be 
happy with.

Namely just have a mode to "git-send-pack" that uses the "--no-walk" flag 
to generate the object list to send.

What that does is to never walk the object history: so it will just use 
the "I HAVE THESE" and "I WANT THESE" commit references to directly 
generate the list of commits, and then walks the trees to generate the 
list of trees/blobs that differ between the particular end-points.

We already have the "no_walk" flag internally, we just don't expose it.

So what you'd get is a _really_ cut down history that doesn't contain any 
commit history at all (just distinct "points in commit history time"), but 
that _does_ contain all the objects that the commits point to.

		Linus

Linus Torvalds wrote:

So what you'd get is a _really_ cut down history that doesn't contain any
commit history at all (just distinct "points in commit history time"), but
that _does_ contain all the objects that the commits point to.

So we would get 'skin-deep clone' rather than 'shallow' one?

-- 
Jakub Narebski
Warsaw, Poland

On Fri, 5 May 2006 08:10:50 -0700 (PDT), Linus Torvalds wrote:

Namely just have a mode to "git-send-pack" that uses the "--no-walk" flag 
to generate the object list to send.

What that does is to never walk the object history: so it will just use 
the "I HAVE THESE" and "I WANT THESE" commit references to directly 
generate the list of commits, and then walks the trees to generate the 
list of trees/blobs that differ between the particular end-points.

Oh, I think that's a great idea. I had proposed cutting the WANT list
down to a single commit, but I wasn't clever enough to think to also
cut down the HAVE walking to solve the problems that would have been
introduced by shallow clones.

And I think the resulting behavior is quite reasonable. I think one
can argue a sort of zero-one-infinity rule here. With history, either
you don't care about any of it, or else you really should care about
all of it.

-Carl

On Fri, 5 May 2006, Jakub Narebski wrote:

Linus Torvalds wrote:

quoted

So what you'd get is a _really_ cut down history that doesn't contain any
commit history at all (just distinct "points in commit history time"), but
that _does_ contain all the objects that the commits point to.

So we would get 'skin-deep clone' rather than 'shallow' one?

Well, it's really shallow, but perhaps more importantly, I think it should 
be really easy, and have totally unambiguous semantics. Never any question 
of how far back to go, and I think we already really do have all the 
support logic for doing it.

Now, we don't actually expose the internal "no_walk" flag with a 
"--no-walk" command line argument parsing, but that's a one-liner.

There's another approach that might be a bit friendlier, which is again to 
walk only the objects of the WANT/HAVE things, but then _do_ walk the 
history for just commit objects. Something close to what I think the http 
fetch thing does if you pass it "-c -t". That too shouldn't require too 
much extra complexity, and it would mean that "git log" at least works.

Of course, that would require another slight difference to "rev-list.c", 
where we'd only recurse into trees of selected commit objects (ie we'd 
have to mark the HAVE/WANT commits specially, but it's not exactly 
complex either).

Of course, the complexity of _both_ of these approaches is really in the 
fsck stage, and all the crud you need to then do other things with these 
pared-down repos. For example, do you allow cloning? And do you just 
automatically notice that you're cloning a shallow repo, and only do a 
shallow clone. Etc etc..

		Linus

Linus Torvalds [off-list ref] writes:

I'm actually growing pretty fond of the config file interfaces that Dscho 
is pushing. I really like the idea of "git pull" doing different things 
depending on which branch is active at the time, because different 
branches really can have different sources they come from.

Always pulling from the same default source seems wrong,...

So Johannes' patches seem to move into that direction, and having it all 
in the config file actually seems to be quite readable.

I share the same reasoning and that is why I am carrying the
series in "next".  I think per branch attributes are wonderful
things.

So I'd argue that (a) yes, we do want to have the "proto porcelain" that 
sets remote branch information without the user having to know the magic 
"git repo-config" incantation, or know which file in .git/remotes/ to 
edit, but that (b) it's even more important to try to decide on what the 
remote description format _is_.

Is it format you care about or the semantics?

I personally have just two preferences:

 - I'd like each branch I'm on to have a "default source" for pulling (and 
   _maybe_ for pushing too). I'd like to just say "git pull", and it would 
   automatically select the appropriate thing to pull from.

 - maybe the same per-branch thing for "push", but more importantly for 
   me, I like to push to multiple destinations, and I'd like the 
   description format to be sane. I think it may already be sane in the 
   form it is in now (supporting both config file _and_ .git/remotes/ 
   formats), I'd just like us to decide on exactly what the meaning is, 
   and hopefully get to the point where we can tell porcelain how to use 
   that meaning to their advantage (and not change it)

Others may disagree, or (equally importantly), may have additional 
preferences. We should try to find something that works for everybody, and 
that is easy to work with.

In my day job, I maintain a base code for a generic application
in "master", various topics, mostly branched from "master" but
sometimes from another topic branch, and one branch each per
customer installation, which pulls from the master, topics and
contains specific customizations.  While on master or any one of
generic topic branch, I need to remember not to pull from
installation branches.  For that matter, the installation
branches should not be pulled into anything else.  So not just
"this branch usually merges from there", but "this branch should
not be merged into others" (mark "installation branches" as
such), and "this branch should never merge from that one" (mark
"master" with "installation branches") would prevent mistakes.

One thing I noticed in "What's in libata.git" Jeff did by
mimicking my "What's in git.git" was that the description for
each topic branch included where it branched from (iow, what
other branch it builds on).  This is sometimes derivable, but
having it as a property for a branch is very handy.

On 5/6/06, Linus Torvalds [off-list ref] wrote:

Of course, that would require another slight difference to "rev-list.c",
where we'd only recurse into trees of selected commit objects (ie we'd
have to mark the HAVE/WANT commits specially, but it's not exactly
complex either).

Would it make sense to make all the shallow clone clone machinery walk
everything and trim only blob objects? In that case, all the machinery
that walks commits/trees would remain intact -- we only have to deal
with the case of not having blob objects, which affects less
codepaths.

It means that for a merge or checkout involving stuff we "don't have",
it's trivial to know we are missing, and so we can  attempt a fetch of
the missing objects or tell the user how to request them them before
retrying.

And in any case commits and trees are lightweight and compress well...

Of course, the complexity of _both_ of these approaches is really in the
fsck stage, and all the crud you need to then do other things with these
pared-down repos. For example, do you allow cloning? And do you just
automatically notice that you're cloning a shallow repo, and only do a
shallow clone. Etc etc..

Definitely.

cheers,


martin

On Fri, 5 May 2006, Junio C Hamano wrote:

quoted

So I'd argue that (a) yes, we do want to have the "proto porcelain" that 
sets remote branch information without the user having to know the magic 
"git repo-config" incantation, or know which file in .git/remotes/ to 
edit, but that (b) it's even more important to try to decide on what the 
remote description format _is_.

Is it format you care about or the semantics?

I _personally_ care about the semantics, but not very deeply - since I 
tend to actually have just one main branch, and a couple of throw-away 
ones if I ended up working on something.

But I think that for this thing to become useful, we want to care about 
the format - or at least the interface to the different users (with the 
acknowledgement that "users" should often be porcelain above us).

Right now we've basically had people hand-editing the remotes files, and I 
think cogito still uses the older branches format that came from cogito in 
the first place. I think we should just try to decide on a config file 
format, and make it easy for cogito etc to use it.

		Linus

On Sat, 6 May 2006 08:26:36 -0700 (PDT)
Linus Torvalds [off-list ref] wrote:

I _personally_ care about the semantics, but not very deeply - since I 
tend to actually have just one main branch, and a couple of throw-away 
ones if I ended up working on something.

But I think that for this thing to become useful, we want to care about 
the format - or at least the interface to the different users (with the 
acknowledgement that "users" should often be porcelain above us).

Right now we've basically had people hand-editing the remotes files, and I 
think cogito still uses the older branches format that came from cogito in 
the first place. I think we should just try to decide on a config file 
format, and make it easy for cogito etc to use it.

Linus,

Wondering why you feel so strongly that most "users" shouldn't be real people.
What is wrong with continuing to make git easier for developers to use without
needing any extra software?

Sean

On Sat, 6 May 2006, sean wrote:

Wondering why you feel so strongly that most "users" shouldn't be real people.
What is wrong with continuing to make git easier for developers to use without
needing any extra software?

Basically, it boils down to the end result.

If you design things for "people", then things tend to become hard to 
automate, and it's hard to make wrappers around it. Maybe you've even made 
the interfaces interactive, and thus any wrappers around it are simply 
screwed, or need to do insane things.

On the other hand, if you design things for automation, doing a "people 
wrapper" that uses the automation should be trivial if the design is even 
remotely any good at all.

In other words: you should always design things for automation, and 
consider the "people interface" to be be just _one_ wrapper layer among 
many.

This has worked really well in git. The whole system was designed from the 
start to be all about scripting and automation, and the "people wrappers" 
tend to be trivial scripts around it.

This was even more obvious when we had a number of basically one-liner 
scripts like "git log", which just did some trivial wrapping around

	git-rev-list | git-diff-tree --stdin | $PAGER

(Now we still have that trivial wrapper, but you just need to look into C 
code to see it, so it's not _as_ obviously trivial).

Contrast this with going the other way: if you talk about the interfaces 
that _people_ want first, you immediately start doing pretty-printing, 
nice parsing, maybe interactive stuff that asks questions. Nice GUIs. And 
the end result is CRAP. Exactly because it lost its ability to be generic.

To some degree, this is the fundamental difference between the Windows and 
the UNIX mindset. At least it used to be.

		Linus

On Sat, 6 May 2006 09:30:48 -0700 (PDT)
Linus Torvalds [off-list ref] wrote:

Basically, it boils down to the end result.

If you design things for "people", then things tend to become hard to 
automate, and it's hard to make wrappers around it. Maybe you've even made 
the interfaces interactive, and thus any wrappers around it are simply 
screwed, or need to do insane things.

Okay, I mistook the scope of you comments to apply to all of git rather than
as a reminder that we can't forget about the toolkit design.  So I take it
you're not at all against git including higher level user commands; just so
long as they're built on top of lower level toolkit commands that other
porcelain can use as well.

In this particular case I see "git repo-config" as the low level command that
any porcelain can use to access the remotes information and the proposed
"git remotes" as a simple convenience wrapper on top of this.  Of course,
everyone has to agree on the config file format; but that is true whether
the human-friendly wrapper exists or not.

Sean

On Sat, 6 May 2006, sean wrote:

Okay, I mistook the scope of you comments to apply to all of git rather than
as a reminder that we can't forget about the toolkit design.  So I take it
you're not at all against git including higher level user commands; just so
long as they're built on top of lower level toolkit commands that other
porcelain can use as well.

Correct. I think we've been able to handle that balance particularly well 
so far. Or maybe the porcelains don't complain enough.

In this particular case I see "git repo-config" as the low level command that
any porcelain can use to access the remotes information and the proposed
"git remotes" as a simple convenience wrapper on top of this.  Of course,
everyone has to agree on the config file format; but that is true whether
the human-friendly wrapper exists or not.

I agree, but my point is that in order for a porcelain to _use_ 
"repo-config", the config file format needs to be defined somewhere, and 
we need to tell people that it's not changing. Are we there yet?

That was my argument for why we should concentrate not on what the user 
wrapper should be named, but why we should look at what the low-level 
meaning of these things are.

Finally, I think "git repo-config" is buggy. Try with this .config file:

	[user]
		name = Bozo the Clown
		email = bozo@circus.com

	[core]
		filemode = true

	[merge]
		summary = true

and then do

	git repo-config core.gitproxy 'dummy example'

and look where it ends up. For me, it ends up at the end, in the "[merge]" 
section, which is obviously bogus.

So we'd really be screwing with porcelain if we made them use this ;)

		Linus

Junio C Hamano wrote:

The vocabulary we would want from the requestor side is probably
(at least):

        I WANT to have these
        I HAVE these
        I'm MISSING these
        Don't bother with these this time around (--since, ^v2.6.16, ...)

Wouldn't it be easier (sorry, no code yet) to have the following:

        I WANT to have these
        I HAVE these
        These are GRAFT PARENTLESS        

with the target side sending list of all parentless commits in the
info/grafts file. The source side will then do the grafting 'in memory' and
send the packs like normal, only with those cauterizing grafts in place.

Now I'm waiting for someone to say that it is too simple and cannot be done,
or that shallow clone/shallow fetch uses this method...

-- 
Jakub Narebski
Warsaw, Poland

On Thu, 2006-05-04 at 01:15 -0700, Junio C Hamano wrote:

* Message-ID:
[off-list ref]
  Content-type charset for send-email (Bertrand Jacquin)

  The output from format-patch by default is unmarked, which
  means the commit message part is UTF-8 (by strong convention),
  and the contents of the diff is whatever the contents of the
  file is encoded in.

Email without a Content-Type: header is supposed to be ASCII. If it
contains 8-bit characters, it's invalid. It'll be interpreted by
different systems in different ways -- not necessarily as UTF-8. Some
may even just reject it, on grounds of RFC non-compliance.

  David Woodhouse did a patch to allow specifying charset on the
  command line (and default to UTF-8) which is a move in the
  right direction, but Bertrand's system seems to have trouble
  with it.

I thought Bertrand then confirmed that he was having trouble _before_
applying my patch, too? His response when I asked it it appears without
my patch was "[it] appear without in 1.3.1 and I can't seed mail with
too. Also, 1.2.4 work fine here (without patch)."

  I think if we were to do this we probably need to teach
  format-patch to optionally do multi-part.  We may not
  necessarily want to mark the payload to be in the same
  encoding as the commit message (not that git-apply cares -- to
  it, the payload is just 8-bit unencoded text, but we would
  want to protect it from getting mangled by e-mail transport). 

I'm not sure about that. The payload is patches, isn't it? That's just
text, too -- we aren't going to deal with diffs of binary content very
well _anyway_, are we?

Obviously, there's nothing to stop people from storing binary blobs in
GIT, but unless you want to start sending actual _blobs_ as attachments
instead of sending patches, I think there's no need to play with MIME
multipart stuff.

I've no particular objection to it, but it's a separate issue to
Bertanrd's. That's a bug-fix, while multipart is an RFE without much
point, IMO.

-- 
dwmw2

On 5/9/06, David Woodhouse [off-list ref] wrote:

On Thu, 2006-05-04 at 01:15 -0700, Junio C Hamano wrote:

quoted

* Message-ID:
[off-list ref]

quoted

  David Woodhouse did a patch to allow specifying charset on the
  command line (and default to UTF-8) which is a move in the
  right direction, but Bertrand's system seems to have trouble
  with it.

I thought Bertrand then confirmed that he was having trouble _before_
applying my patch, too? His response when I asked it it appears without
my patch was "[it] appear without in 1.3.1 and I can't seed mail with
too. Also, 1.2.4 work fine here (without patch)."

Ok, to make short :
git-send-email 1.2.4 :
No EOF error on my smtp server.
git-send-email 1.3.1 :
EOF error on my smtp server.

I upgraded to 1.3.1 when I received patch from you, don't test 1.3.1
and then applied your patch, you test. And so test failed.

--
Beber
#e.fr@freenode

On Tue, 9 May 2006, David Woodhouse wrote:

I'm not sure about that. The payload is patches, isn't it? That's just
text, too -- we aren't going to deal with diffs of binary content very
well _anyway_, are we?

Yes we do.  GIT now has its own email friendly binary patch format.


Nicolas