Git development
 help / color / mirror / Atom feed
* Re: Git-windows and git-svn?
From: Johannes Schindelin @ 2007-11-02 22:07 UTC (permalink / raw)
  To: Abdelrazak Younes; +Cc: git
In-Reply-To: <fgg6cd$3ep$1@ger.gmane.org>

Hi,

On Fri, 2 Nov 2007, Abdelrazak Younes wrote:

> I would like to try git on Windows together with git-svn, I downloaded 
> Git-1.5.3-preview20071027.exe and tried:
> 
> yns@xxx /d/devel/lyx/git/trunk
> $ git-svn init svn://svn.lyx.org/lyx/lyx-devel/trunk
> Useless use of a constant in void context at /bin/git-svn line 848.
> Can't locate Digest/MD5.pm in @INC (@INC contains: /lib
> /usr/lib/perl5/5.6.1/msy
> s /usr/lib/perl5/5.6.1 /usr/lib/perl5/site_perl/5.6.1/msys
> /usr/lib/perl5/site_p
> erl/5.6.1 /usr/lib/perl5/site_perl .) at /bin/git-svn line 2420.
> BEGIN failed--compilation aborted at /bin/git-svn line 2420.
> 
> Is there something I am doing wrong?

The proper mailinglist for this is msysgit@googlegroups.com.  And right 
next to the "Downloads" tab you clicked on to download the installer, 
there is an "Issues" tab.  There you'll find that we're working on it.

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH] Mention that git-branch -M can cause problems for tracking branches
From: Jonas Fonseca @ 2007-11-02 22:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlk9g1k1q.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote Fri, Nov 02, 2007:
> Jonas Fonseca <fonseca@diku.dk> writes:
> 
> > Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
> > ---
> >  Documentation/git-branch.txt |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >  I made a patch to discard the overwritten branch's configuration
> >  section, which Spearce felt was too lossy a behaviour. However, since
> >  it confused me, I think it should at least be mentioned in the manpage.
> >  Maybe the warning message from git should also be added to improve its
> >  "googlability".
> >
> > diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> > index 5e81aa4..def4e85 100644
> > --- a/Documentation/git-branch.txt
> > +++ b/Documentation/git-branch.txt
> > @@ -165,6 +165,11 @@ If you are creating a branch that you want to immediately checkout, it's
> >  easier to use the git checkout command with its `-b` option to create
> >  a branch and check it out with a single command.
> >  
> > +When a branch is renamed so that it overwrites an existing branch unintended
> > +problems can arise. This is because git refuses to discard the configuration
> > +section of the overwritten branch. As a result git can become confused if, for
> > +example, the branches involved were used for tracking two different remote
> > +branches. The only way to fix this is to edit the configuration file manually.
> 
> I do not understand this bit about "refuse".
> 
>  - To "refuse to discard", somebody has to ask to discard ---
>    who asks so and when?

IMO, the user asks when using git-branch -M. And in case it is not clear
the problem arises for the command sequence:

	$ git branch --track o-next origin/next
	$ git branch --track m-next madcoder/next
	$ git branch -M o-next m-next
	$ git remote
	Warning: more than one branch.m-next.remote
	...

>  - Is there a reason to "refuse" when such a removal request is
>    made?  If so, what is it?  If not, why refusal?

Personally, I don't see why we need to refuse, since git-branch -M is
somewhat similar to saying -m (rename) plus adding a "force" flag
meaning: "yes, I know that this will potentially throw away settings for
an already existing branch".

The main reason to "refuse" the removal is that for the general case,
e.g. when using `git-config --rename-section`, this can potentially lead
to loss of valuable config settings. This was pointed out by Shawn in
his reply to my patch[0]. I agreed to this in my follow-up and asked if
it would be acceptable to add an additional flag to so that git-branch
can switch on this request for removal.

[0] http://thread.gmane.org/gmane.comp.version-control.git/61291

-- 
Jonas Fonseca

^ permalink raw reply

* Git-windows and git-svn?
From: Abdelrazak Younes @ 2007-11-02 21:55 UTC (permalink / raw)
  To: git

Hello,

I would like to try git on Windows together with git-svn, I downloaded 
  Git-1.5.3-preview20071027.exe and tried:

yns@xxx /d/devel/lyx/git/trunk
$ git-svn init svn://svn.lyx.org/lyx/lyx-devel/trunk
Useless use of a constant in void context at /bin/git-svn line 848.
Can't locate Digest/MD5.pm in @INC (@INC contains: /lib 
/usr/lib/perl5/5.6.1/msy
s /usr/lib/perl5/5.6.1 /usr/lib/perl5/site_perl/5.6.1/msys 
/usr/lib/perl5/site_p
erl/5.6.1 /usr/lib/perl5/site_perl .) at /bin/git-svn line 2420.
BEGIN failed--compilation aborted at /bin/git-svn line 2420.

Is there something I am doing wrong?

Thanks in advance for any help,
Abdel.

^ permalink raw reply

* Re: git rm --cached
From: Jing Xue @ 2007-11-02 21:41 UTC (permalink / raw)
  To: Remi Vanicat; +Cc: git
In-Reply-To: <87mytwiq1f.dlv@vanicat.homelinux.org>


Quoting Remi Vanicat <vanicat@debian.org>:

> Jing Xue <jingxue@digizenstudio.com> writes:
>
>> In the following scenario, why do I have to run 'git reset' following
>> 'git rm --cached 1.txt' to revert to exactly where I was before 'git add
>> 1.txt'?  Shouldn't 'git rm --cached' have done that already?
>
> Observed behavior are exactly what I expected: 'git rm --cached' mark
> the file in the index as been deleted without deleting it in the
> working directories, it did not but the index it was before the
> 'git add 1.txt'.

I was confused by two things I guess:

1. I looked at the "index" as a staging area for _changes_ not files  
themselves. So where 'man git-rm' says '--caches ... remove[s] the  
paths only from the index, leaving working tree files.'  I took it to  
mean that it removes the changes on those paths, rather than staging a  
new "path deletion" action for a later commit.

2. The FAQ entry "Why 'git rm' is not inverse of 'git add'" says "a  
natural inverse of 'add' is 'un-add', and that operation is called 'rm  
--cached',..."  Now I realize that only applies to adding a new file,  
but not changes on an existing file.

> You probably want to use git reset HEAD -- 1.txt to unstage
> modification on 1.txt

Sure.

Thanks.
-- 
Jing Xue

^ permalink raw reply

* Re: [PATCH] Make git-clone obey "--" (end argument parsing)
From: Junio C Hamano @ 2007-11-02 21:32 UTC (permalink / raw)
  To: Heikki Orsila; +Cc: git
In-Reply-To: <20071102142150.GD9632@jolt.modeemi.cs.tut.fi>

Heikki Orsila <shd@modeemi.fi> writes:

> Junio C Hamano wrote:
>> I do not think this breaks anything, but does it _help_
>> anything in practice?
>> 
>> What kind of breakage does this patch fix?
>
> It doesn't fix anything, but it is a good usability standard to obey --.
> I was creating a script that would _automatically_ clone repositories of 
> other users. As a little bit pedantic shell script writer I added "--" 
> and noticed that git clone doesn't handle that,...

The first real parameter will always be the remote repo
URL^Wlocator, and sane people won't have anything that begin
with a hyphen for that, will they?

So the change is only for consistency's sake.

I am not saying that is a bad thing.  I just wanted to
understand if there was a real breakage there.

Thanks. Will apply.

^ permalink raw reply

* Re: [PATCH 2/2] Support "history replay" for git log commands
From: Linus Torvalds @ 2007-11-02 21:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Costalba, Paul Mackerras, git
In-Reply-To: <7v4pg41hq0.fsf@gitster.siamese.dyndns.org>



On Fri, 2 Nov 2007, Junio C Hamano wrote:
> 
> Can one iteration of loop in log_tree_commit() smudge more
> than one commits with FORCE_REPLAY?  Maybe make the
> trigger_replay a counter and count down in this loop until we
> find that many commits that have the FORCE_REPLAY flag?

Good point, and yes, that sounds like the right thing to do.

		Linus

^ permalink raw reply

* Re: [PATCH 1/4] Add testcase for ammending and fixing author in git commit.
From: Kristian Høgsberg @ 2007-11-02 21:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7dg1kdg.fsf@gitster.siamese.dyndns.org>

On Fri, 2007-11-02 at 13:07 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> > ---
> >  t/t7501-commit.sh |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> > index b151b51..3f2112a 100644
> > --- a/t/t7501-commit.sh
> > +++ b/t/t7501-commit.sh
> > @@ -163,4 +163,14 @@ test_expect_success 'partial commit that involves removal (3)' '
> >  
> >  '
> >  
> > +author="The Real Author <someguy@his.email.org>"
> > +test_expect_success 'amend commit to fix author' '
> > +
> > +	git reset --hard
> > +	git cat-file -p HEAD | sed -e "s/author.*>/author $author/" > expected &&
> > +	git commit --amend --author="$author" &&
> > +	git cat-file -p HEAD > current &&
> > +	diff expected current
> > +	
> > +'
> >  test_done
> 
> This can't be right.  How are you ignoring the differences in
> committer dates?

t/test-lib.sh fixes GIT_COMMITTER_DATE so all commits have the date

committer C O Mitter <committer@example.com> 1112911993 -0700

unless you use test_tick.

> By the way, I _think_ git-commit.sh allows fixing author name/email
> without molesting the author timestamp (i.e. takes it from the
> amended commit).  That should probably be checked with the test
> as well.

You're right, I need to pick GIT_AUTHOR_DATE from the ammended commit.
Ok, I'll need to rewrite determine_author_info() a little bit.  I might
get an update patch out this weekend.

cheers,
Kristian

^ permalink raw reply

* Re: [PATCH 2/2] Support "history replay" for git log commands
From: Junio C Hamano @ 2007-11-02 21:05 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Marco Costalba, Paul Mackerras, git
In-Reply-To: <alpine.LFD.0.999.0711021333050.3342@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> This notices if we aren't in topological order, and replays the history. 
> Thus avoiding the need to sort history up front.
>     
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>
> See the code and the more complete explanations in [PATCH 0/2]. In 
> particular, see the last section there about the downsides of this: the 
> 50x expansion of output on the kernel is unacceptable, but if somebody can 
> make a graphical viewer that can react correctly to the "Replay" thing, 
> I'm sure I can make the replays themselves happen much more rarely.
>
>  builtin-blame.c |    2 +-
>  builtin-log.c   |   35 +++++++++++++++++++++++++++++++++++
>  log-tree.c      |   10 +++++++---
>  revision.c      |   29 ++++++++++++++++++++++-------
>  revision.h      |    6 +++++-
>  5 files changed, 70 insertions(+), 12 deletions(-)
>
> diff --git a/builtin-blame.c b/builtin-blame.c
> index 8432b82..7b6af8c 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -1502,7 +1502,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt)
>  		else {
>  			commit->object.flags |= UNINTERESTING;
>  			if (commit->object.parsed)
> -				mark_parents_uninteresting(commit);
> +				mark_parents_uninteresting(revs, commit);
>  		}
>  		/* treat root commit as boundary */
>  		if (!commit->parents && !show_root)
> diff --git a/builtin-log.c b/builtin-log.c
> index e8b982d..10e0821 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -77,6 +77,35 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
>  	}
>  }
>  
> +static void replay_history(struct rev_info *revs)
> +{
> +	struct commit_list *entry;
> +
> +	revs->trigger_replay = 0;
> +	while ((entry = revs->shown) != NULL) {
> +		struct commit *commit = entry->item;
> +		unsigned flags = commit->object.flags;
> +
> +		/* Undo the SHOWN and FORCE_REPLAY bits */
> +		commit->object.flags = flags & ~(SHOWN | FORCE_REPLAY);
> +		commit->indegree = 0;
> +
> +		/* Remove it from the shown list, put it on the commit list */
> +		revs->shown = entry->next;
> +		entry->next = revs->commits;
> +		revs->commits = entry;
> +
> +		printf("Replay %s\n", sha1_to_hex(commit->object.sha1));
> +
> +		/* Was this the one that caused us to replay? */
> +		if (flags & FORCE_REPLAY)
> +			break;

Can one iteration of loop in log_tree_commit() smudge more
than one commits with FORCE_REPLAY?  Maybe make the
trigger_replay a counter and count down in this loop until we
find that many commits that have the FORCE_REPLAY flag?

^ permalink raw reply

* Re: [PATCH] git-rev-list.txt: rev stands for revision, not reverse.
From: Junio C Hamano @ 2007-11-02 20:46 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <7vprys1k5m.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
>
>> Is the reverse chronological order the primary sorting key at all?
>
> It is mostly chrono but there is a topo element as well.  You
> would never see a parent none of whose child is shown.

To be precise...

Internally, it mainly works on topology with an additional logic
to tiebreak with commit timestamps.

 - We always have one or more "output candidates".

 - Initially, the set of output candidates are primed from the
   positive refs (e.g. "foo", foo in "bar..foo", foo and bar in
   "bar...foo") you give to rev-list and/or log family of
   commands, after reachability analysis with negative refs
   (e.g. "^foo", bar in "bar..foo", merge-base of foo and bar in
   "bar...foo") if present.

 - We output the latest one among output candidates.  The
   parents of the commit we output go through the reachability
   analysis and the ones that are reachable from any of the
   negative refs are removed.  The ones that survive this
   reachability analysis are added to the output candidates.
   And the process starts over.  The algorithm terminates when
   there is none.

In practice, because child commits tend to get later commit
timestamps than all of their parent commits, the output looks
mostly reverse chronological.

^ permalink raw reply

* [PATCH 2/2] Support "history replay" for git log commands
From: Linus Torvalds @ 2007-11-02 20:35 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Paul Mackerras, git
In-Reply-To: <alpine.LFD.0.999.0711021301200.3342@woody.linux-foundation.org>


This notices if we aren't in topological order, and replays the history. 
Thus avoiding the need to sort history up front.
    
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

See the code and the more complete explanations in [PATCH 0/2]. In 
particular, see the last section there about the downsides of this: the 
50x expansion of output on the kernel is unacceptable, but if somebody can 
make a graphical viewer that can react correctly to the "Replay" thing, 
I'm sure I can make the replays themselves happen much more rarely.

 builtin-blame.c |    2 +-
 builtin-log.c   |   35 +++++++++++++++++++++++++++++++++++
 log-tree.c      |   10 +++++++---
 revision.c      |   29 ++++++++++++++++++++++-------
 revision.h      |    6 +++++-
 5 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 8432b82..7b6af8c 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1502,7 +1502,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt)
 		else {
 			commit->object.flags |= UNINTERESTING;
 			if (commit->object.parsed)
-				mark_parents_uninteresting(commit);
+				mark_parents_uninteresting(revs, commit);
 		}
 		/* treat root commit as boundary */
 		if (!commit->parents && !show_root)
diff --git a/builtin-log.c b/builtin-log.c
index e8b982d..10e0821 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -77,6 +77,35 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	}
 }
 
+static void replay_history(struct rev_info *revs)
+{
+	struct commit_list *entry;
+
+	revs->trigger_replay = 0;
+	while ((entry = revs->shown) != NULL) {
+		struct commit *commit = entry->item;
+		unsigned flags = commit->object.flags;
+
+		/* Undo the SHOWN and FORCE_REPLAY bits */
+		commit->object.flags = flags & ~(SHOWN | FORCE_REPLAY);
+		commit->indegree = 0;
+
+		/* Remove it from the shown list, put it on the commit list */
+		revs->shown = entry->next;
+		entry->next = revs->commits;
+		revs->commits = entry;
+
+		printf("Replay %s\n", sha1_to_hex(commit->object.sha1));
+
+		/* Was this the one that caused us to replay? */
+		if (flags & FORCE_REPLAY)
+			break;
+	}
+
+	/* Ok, sort the list to be replayed properly now.. */
+	sort_in_topological_order(&revs->commits, revs->lifo);
+}
+
 static int cmd_log_walk(struct rev_info *rev)
 {
 	struct commit *commit;
@@ -84,6 +113,12 @@ static int cmd_log_walk(struct rev_info *rev)
 	prepare_revision_walk(rev);
 	while ((commit = get_revision(rev)) != NULL) {
 		log_tree_commit(rev, commit);
+
+		if (rev->replay_history) {
+			if (rev->trigger_replay)
+				replay_history(rev);
+			continue;
+		}
 		if (!rev->reflog_info) {
 			/* we allow cycles in reflog ancestry */
 			free(commit->buffer);
diff --git a/log-tree.c b/log-tree.c
index 3763ce9..ce9b887 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -6,12 +6,16 @@
 
 struct decoration name_decoration = { "object names" };
 
-static void show_parents(struct commit *commit, int abbrev)
+static void show_parents(struct rev_info *opt, struct commit *commit, int abbrev)
 {
 	struct commit_list *p;
 	for (p = commit->parents; p ; p = p->next) {
 		struct commit *parent = p->item;
 		printf(" %s", diff_unique_abbrev(parent->object.sha1, abbrev));
+		if (parent->object.flags & SHOWN) {
+			opt->trigger_replay = 1;
+			parent->object.flags |= FORCE_REPLAY;
+		}
 	}
 }
 
@@ -147,7 +151,7 @@ void show_log(struct rev_info *opt, const char *sep)
 		}
 		fputs(diff_unique_abbrev(commit->object.sha1, abbrev_commit), stdout);
 		if (opt->parents)
-			show_parents(commit, abbrev_commit);
+			show_parents(opt, commit, abbrev_commit);
 		show_decorations(commit);
 		putchar(opt->diffopt.line_termination);
 		return;
@@ -248,7 +252,7 @@ void show_log(struct rev_info *opt, const char *sep)
 		fputs(diff_unique_abbrev(commit->object.sha1, abbrev_commit),
 		      stdout);
 		if (opt->parents)
-			show_parents(commit, abbrev_commit);
+			show_parents(opt, commit, abbrev_commit);
 		if (parent)
 			printf(" (from %s)",
 			       diff_unique_abbrev(parent->object.sha1,
diff --git a/revision.c b/revision.c
index e85b4af..e40bc1c 100644
--- a/revision.c
+++ b/revision.c
@@ -79,7 +79,7 @@ void mark_tree_uninteresting(struct tree *tree)
 	tree->buffer = NULL;
 }
 
-void mark_parents_uninteresting(struct commit *commit)
+void mark_parents_uninteresting(struct rev_info *revs, struct commit *commit)
 {
 	struct commit_list *parents = commit->parents;
 
@@ -87,6 +87,10 @@ void mark_parents_uninteresting(struct commit *commit)
 		struct commit *commit = parents->item;
 		if (!(commit->object.flags & UNINTERESTING)) {
 			commit->object.flags |= UNINTERESTING;
+			if (commit->object.flags & SHOWN) {
+				revs->trigger_replay = 1;
+				commit->object.flags |= FORCE_REPLAY;
+			}
 
 			/*
 			 * Normally we haven't parsed the parent
@@ -97,7 +101,7 @@ void mark_parents_uninteresting(struct commit *commit)
 			 * to mark its parents recursively too..
 			 */
 			if (commit->parents)
-				mark_parents_uninteresting(commit);
+				mark_parents_uninteresting(revs, commit);
 		}
 
 		/*
@@ -167,8 +171,9 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
 			die("unable to parse commit %s", name);
 		if (flags & UNINTERESTING) {
 			commit->object.flags |= UNINTERESTING;
-			mark_parents_uninteresting(commit);
-			revs->limited = 1;
+			mark_parents_uninteresting(revs, commit);
+			if (!revs->replay_history)
+				revs->limited = 1;
 		}
 		return commit;
 	}
@@ -399,7 +404,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit, str
 				return -1;
 			p->object.flags |= UNINTERESTING;
 			if (p->parents)
-				mark_parents_uninteresting(p);
+				mark_parents_uninteresting(revs, p);
 			if (p->object.flags & SEEN)
 				continue;
 			p->object.flags |= SEEN;
@@ -542,7 +547,7 @@ static int limit_list(struct rev_info *revs)
 		if (add_parents_to_list(revs, commit, &list) < 0)
 			return -1;
 		if (obj->flags & UNINTERESTING) {
-			mark_parents_uninteresting(commit);
+			mark_parents_uninteresting(revs, commit);
 			if (everybody_uninteresting(list))
 				break;
 			continue;
@@ -995,6 +1000,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->parents = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--replay")) {
+				revs->replay_history = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--dense")) {
 				revs->dense = 1;
 				continue;
@@ -1386,7 +1395,13 @@ static struct commit *get_revision_1(struct rev_info *revs)
 		struct commit *commit = entry->item;
 
 		revs->commits = entry->next;
-		free(entry);
+
+		/* Are we going to potentially replay? */
+		if (revs->replay_history) {
+			entry->next = revs->shown;
+			revs->shown = entry;
+		} else
+			free(entry);
 
 		if (revs->reflog_info)
 			fake_reflog_parent(revs->reflog_info, commit);
diff --git a/revision.h b/revision.h
index 1f64576..75b320d 100644
--- a/revision.h
+++ b/revision.h
@@ -11,6 +11,7 @@
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
 #define SYMMETRIC_LEFT	(1u<<8)
 #define TOPOSORT	(1u<<9)	/* In the active toposort list.. */
+#define FORCE_REPLAY	(1u<<10)	/* This commit was wrong somehow */
 
 struct rev_info;
 struct log_info;
@@ -20,6 +21,7 @@ typedef void (prune_fn_t)(struct rev_info *revs, struct commit *commit);
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
+	struct commit_list *shown;
 	struct object_array pending;
 
 	/* Parents of shown commits */
@@ -36,6 +38,8 @@ struct rev_info {
 			no_walk:1,
 			remove_empty_trees:1,
 			simplify_history:1,
+			replay_history:1,
+			trigger_replay:1,
 			lifo:1,
 			topo_order:1,
 			tag_objects:1,
@@ -113,7 +117,7 @@ extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,
 extern int prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
 
-extern void mark_parents_uninteresting(struct commit *commit);
+extern void mark_parents_uninteresting(struct rev_info *revs, struct commit *commit);
 extern void mark_tree_uninteresting(struct tree *tree);
 
 struct name_path {

^ permalink raw reply related

* [PATCH 1/2] Simplify topo-sort logic
From: Linus Torvalds @ 2007-11-02 20:32 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Paul Mackerras, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0711021301200.3342@woody.linux-foundation.org>


.. by not using quite so much indirection.

This currently grows the "struct commit" a bit, which could be avoided by 
using a union for "util" and "indegree" (the topo-sort used to use "util" 
anyway, so you cannot use them together), but for now the goal of this was 
to simplify, not optimize.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 commit.c   |  150 +++++++++++++++++++++---------------------------------------
 commit.h   |   20 +--------
 revision.c |    7 +--
 revision.h |    4 +-
 4 files changed, 55 insertions(+), 126 deletions(-)

diff --git a/commit.c b/commit.c
index ac24266..c155a49 100644
--- a/commit.c
+++ b/commit.c
@@ -9,22 +9,6 @@
 
 int save_commit_buffer = 1;
 
-struct sort_node
-{
-	/*
-	 * the number of children of the associated commit
-	 * that also occur in the list being sorted.
-	 */
-	unsigned int indegree;
-
-	/*
-	 * reference to original list item that we will re-use
-	 * on output.
-	 */
-	struct commit_list * list_item;
-
-};
-
 const char *commit_type = "commit";
 
 static struct cmt_fmt_map {
@@ -1150,69 +1134,38 @@ struct commit *pop_commit(struct commit_list **stack)
 	return item;
 }
 
-void topo_sort_default_setter(struct commit *c, void *data)
-{
-	c->util = data;
-}
-
-void *topo_sort_default_getter(struct commit *c)
-{
-	return c->util;
-}
-
 /*
  * Performs an in-place topological sort on the list supplied.
  */
 void sort_in_topological_order(struct commit_list ** list, int lifo)
 {
-	sort_in_topological_order_fn(list, lifo, topo_sort_default_setter,
-				     topo_sort_default_getter);
-}
-
-void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
-				  topo_sort_set_fn_t setter,
-				  topo_sort_get_fn_t getter)
-{
-	struct commit_list * next = *list;
-	struct commit_list * work = NULL, **insert;
-	struct commit_list ** pptr = list;
-	struct sort_node * nodes;
-	struct sort_node * next_nodes;
-	int count = 0;
-
-	/* determine the size of the list */
-	while (next) {
-		next = next->next;
-		count++;
-	}
+	struct commit_list *next, *orig = *list;
+	struct commit_list *work, **insert;
+	struct commit_list **pptr;
 
-	if (!count)
+	if (!orig)
 		return;
-	/* allocate an array to help sort the list */
-	nodes = xcalloc(count, sizeof(*nodes));
-	/* link the list to the array */
-	next_nodes = nodes;
-	next=*list;
-	while (next) {
-		next_nodes->list_item = next;
-		setter(next->item, next_nodes);
-		next_nodes++;
-		next = next->next;
+	*list = NULL;
+
+	/* Mark them and clear the indegree */
+	for (next = orig; next; next = next->next) {
+		struct commit *commit = next->item;
+		commit->object.flags |= TOPOSORT;
+		commit->indegree = 0;
 	}
+
 	/* update the indegree */
-	next=*list;
-	while (next) {
+	for (next = orig; next; next = next->next) {
 		struct commit_list * parents = next->item->parents;
 		while (parents) {
-			struct commit * parent=parents->item;
-			struct sort_node * pn = (struct sort_node *) getter(parent);
+			struct commit *parent = parents->item;
 
-			if (pn)
-				pn->indegree++;
-			parents=parents->next;
+			if (parent->object.flags & TOPOSORT)
+				parent->indegree++;
+			parents = parents->next;
 		}
-		next=next->next;
 	}
+
 	/*
 	 * find the tips
 	 *
@@ -1220,55 +1173,56 @@ void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
 	 *
 	 * the tips serve as a starting set for the work queue.
 	 */
-	next=*list;
+	work = NULL;
 	insert = &work;
-	while (next) {
-		struct sort_node * node = (struct sort_node *) getter(next->item);
+	for (next = orig; next; next = next->next) {
+		struct commit *commit = next->item;
 
-		if (node->indegree == 0) {
-			insert = &commit_list_insert(next->item, insert)->next;
-		}
-		next=next->next;
+		if (!commit->indegree)
+			insert = &commit_list_insert(commit, insert)->next;
 	}
 
 	/* process the list in topological order */
 	if (!lifo)
 		sort_by_date(&work);
+
+	pptr = list;
+	*list = NULL;
 	while (work) {
-		struct commit * work_item = pop_commit(&work);
-		struct sort_node * work_node = (struct sort_node *) getter(work_item);
-		struct commit_list * parents = work_item->parents;
+		struct commit *commit;
+		struct commit_list *parents, *work_item;
 
-		while (parents) {
-			struct commit * parent=parents->item;
-			struct sort_node * pn = (struct sort_node *) getter(parent);
-
-			if (pn) {
-				/*
-				 * parents are only enqueued for emission
-				 * when all their children have been emitted thereby
-				 * guaranteeing topological order.
-				 */
-				pn->indegree--;
-				if (!pn->indegree) {
-					if (!lifo)
-						insert_by_date(parent, &work);
-					else
-						commit_list_insert(parent, &work);
-				}
+		work_item = work;
+		work = work_item->next;
+		work_item->next = NULL;
+
+		commit = work_item->item;
+		for (parents = commit->parents; parents ; parents = parents->next) {
+			struct commit *parent=parents->item;
+
+			if (!(parent->object.flags & TOPOSORT))
+				continue;
+
+			/*
+			 * parents are only enqueued for emission
+			 * when all their children have been emitted thereby
+			 * guaranteeing topological order.
+			 */
+			if (!--parent->indegree) {
+				if (!lifo)
+					insert_by_date(parent, &work);
+				else
+					commit_list_insert(parent, &work);
 			}
-			parents=parents->next;
 		}
 		/*
 		 * work_item is a commit all of whose children
 		 * have already been emitted. we can emit it now.
 		 */
-		*pptr = work_node->list_item;
-		pptr = &(*pptr)->next;
-		*pptr = NULL;
-		setter(work_item, NULL);
+		commit->object.flags &= ~TOPOSORT;
+		*pptr = work_item;
+		pptr = &work_item->next;
 	}
-	free(nodes);
 }
 
 /* merge-base stuff */
diff --git a/commit.h b/commit.h
index b661503..7c71471 100644
--- a/commit.h
+++ b/commit.h
@@ -14,6 +14,7 @@ struct commit_list {
 struct commit {
 	struct object object;
 	void *util;
+	unsigned int indegree;
 	unsigned long date;
 	struct commit_list *parents;
 	struct tree *tree;
@@ -82,31 +83,12 @@ void clear_commit_marks(struct commit *commit, unsigned int mark);
 /*
  * Performs an in-place topological sort of list supplied.
  *
- * Pre-conditions for sort_in_topological_order:
- *   all commits in input list and all parents of those
- *   commits must have object.util == NULL
- *
- * Pre-conditions for sort_in_topological_order_fn:
- *   all commits in input list and all parents of those
- *   commits must have getter(commit) == NULL
- *
- * Post-conditions:
  *   invariant of resulting list is:
  *      a reachable from b => ord(b) < ord(a)
  *   in addition, when lifo == 0, commits on parallel tracks are
  *   sorted in the dates order.
  */
-
-typedef void (*topo_sort_set_fn_t)(struct commit*, void *data);
-typedef void* (*topo_sort_get_fn_t)(struct commit*);
-
-void topo_sort_default_setter(struct commit *c, void *data);
-void *topo_sort_default_getter(struct commit *c);
-
 void sort_in_topological_order(struct commit_list ** list, int lifo);
-void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
-				  topo_sort_set_fn_t setter,
-				  topo_sort_get_fn_t getter);
 
 struct commit_graft {
 	unsigned char sha1[20];
diff --git a/revision.c b/revision.c
index e76da0d..e85b4af 100644
--- a/revision.c
+++ b/revision.c
@@ -677,9 +677,6 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->prune_fn = NULL;
 	revs->prune_data = NULL;
 
-	revs->topo_setter = topo_sort_default_setter;
-	revs->topo_getter = topo_sort_default_getter;
-
 	revs->commit_format = CMIT_FMT_DEFAULT;
 
 	diff_setup(&revs->diffopt);
@@ -1303,9 +1300,7 @@ int prepare_revision_walk(struct rev_info *revs)
 		if (limit_list(revs) < 0)
 			return -1;
 	if (revs->topo_order)
-		sort_in_topological_order_fn(&revs->commits, revs->lifo,
-					     revs->topo_setter,
-					     revs->topo_getter);
+		sort_in_topological_order(&revs->commits, revs->lifo);
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index 98a0a8f..1f64576 100644
--- a/revision.h
+++ b/revision.h
@@ -10,6 +10,7 @@
 #define CHILD_SHOWN	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
 #define SYMMETRIC_LEFT	(1u<<8)
+#define TOPOSORT	(1u<<9)	/* In the active toposort list.. */
 
 struct rev_info;
 struct log_info;
@@ -96,9 +97,6 @@ struct rev_info {
 	struct diff_options diffopt;
 	struct diff_options pruning;
 
-	topo_sort_set_fn_t topo_setter;
-	topo_sort_get_fn_t topo_getter;
-
 	struct reflog_walk_info *reflog_info;
 };
 

^ permalink raw reply related

* [PATCH 0/2] History replay support
From: Linus Torvalds @ 2007-11-02 20:31 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Paul Mackerras, git
In-Reply-To: <alpine.LFD.0.999.0711021114390.3342@woody.linux-foundation.org>


Ok, this is a rough first draft of avoiding the topological sort up-front, 
and instead incrementally sorting only when actually necessary.

The first patch is a pure cleanup. Quite frankly, I suspect the old 
topological sorting code would have worked perfectly fine as-is, but I 
couldn't really bear to watch it or think about debugging it the way it 
was before.

The indirection and other strangeness just blew my tiny little mind, and 
while I bet it had some historical reason, I ended up reworking that 
thing. I tried to keep it as similar as humanly possible to the old code 
(because it's easy to get wrong, and because I really didn't want to worry 
about the toposort itself), but the numbers speak for themselves:

	 4 files changed, 55 insertions(+), 126 deletions(-)

should tell you something. And it means that there are no subtle calling 
issues with preconditions for using the toposort etc - you can use it over 
and over again, and it should all be obvious. Knock wood.

The second diff is the one that actually adds the new feature, and it 
undoes most of the nice statistics of the first one:

	 5 files changed, 70 insertions(+), 12 deletions(-)

but we still end up with more deletions than insertions on the whole, 
*and* a new feature.

The new code is triggered by using the "--replay" flag, which will cause 
certain consistency checks to be done when a commit is shown by the log 
machinery. In particular:

 - if we print out a parent SHA1, and the parent has already been shown, 
   that's a topology violation, and causes a replay.
 - if we turn a commit unintersting, and the commit has already been 
   shown, that's a "uninteresting" violation, and causes a replay.

The second one I didn't test at all. It's probably hard to trigger, and 
I bet there are bugs there, but this is very much a WIP patch, with the 
hope that people other than me can work on it.

When a replay happens, the log code will print out

	Replay <sha1>

for each <sha1> commit that gets invalidated by the replay, and then start 
*that* part of history anew, with just the required part re-sorted.

Should it print just the number of commits? Perhaps. Play around with 
this.

Anyway, to give you some kind of idea of the effect of this, in the 
current git tree as it exists in my repo, I get 15 replay events, and if 
you compare the total log output, you see:

	[torvalds@woody git]$ wc -l t1 t2
	  170191 t1
	  178150 t2

where "t1" is without replays, and "t2" is with replays. The replays 
obviously do add lines (the replayed ones), but at least for git, it's on 
the order of 5% lines replayed.

The big difference is in the latency:

	time git log --parents --topo-order | head
	real    0m0.163s

vs

	time git log --parents --replay | head
	real    0m0.003s

ie you can see how the --replay thing starts outputtig commits 
immediately, because it knows it can just back up and fix any errors that 
happen.

That's the good news.

The bad news is that it doesn't work well in this simplistic form, because 
there is a O(n**2) behaviour when replays *do* happen, ie we end up having 
replays within replays, and rather than getting a 5% increase like for git 
itself, for the kernel archive this gets a roughly 50x increase for the 
replay. So the *latency* still improves dramatically (getting the first 
one hundred commits in 0.006 seconds vs 1.076s), but because of the bad 
behaviour wrt cascading replays, it's not really usable in this form (the 
full log goes from 8 seconds to 22s when writing to /dev/null - and is 
much worse if the receiver actually has to do something about it).

I think the right thing to do wrt this all would be to turn the replay 
logic into a latency-based one, where it would basically batch things up, 
but make sure to output something at least every half a second or so. But 
that's a pretty separate set of logic, so I thought I'd send this out 
as-is for comments..

			Linus

^ permalink raw reply

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Junio C Hamano @ 2007-11-02 20:29 UTC (permalink / raw)
  To: Ping Yin; +Cc: git
In-Reply-To: <1194004427-26934-1-git-send-email-pkufranky@gmail.com>

How does this work when you are a toplevel developer and do not
have the submodule cloned and checked out?

Our code should treat having the submodule directory and not
having it when there is a mode 160000 entry in the index equally
likely.  Cloning and checking-out is _not_ the norm (nor the
exception).

^ permalink raw reply

* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Junio C Hamano @ 2007-11-02 20:19 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Tom Prince, Steffen Prohaska, git
In-Reply-To: <7vfxzo3046.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> If you linearlize the history by rebasing the lower branch on
> top of upper, instead of merging, the bug becomes much easier to
> find and understand.  Your history would instead be:
>
>     ---Z---o---X'--...---o---A---o---o---Y'--...---o---B'--D'
>
> and there is a single commit Y' between A and B' that introduced
> the new calling site that still uses the new semantics of the
> function that was already in A.  "git show Y'" will be a much
> smaller patch than "git diff A C" and it is much easier to deal
> with.

Typo.  Y' uses "the old semantics of the function, even though
that was already modified at X'".

^ permalink raw reply

* Re: [PATCH] Mention that git-branch -M can cause problems for tracking branches
From: Junio C Hamano @ 2007-11-02 20:14 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20071102091734.GC10141@diku.dk>

Jonas Fonseca <fonseca@diku.dk> writes:

> Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
> ---
>  Documentation/git-branch.txt |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
>  I made a patch to discard the overwritten branch's configuration
>  section, which Spearce felt was too lossy a behaviour. However, since
>  it confused me, I think it should at least be mentioned in the manpage.
>  Maybe the warning message from git should also be added to improve its
>  "googlability".
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 5e81aa4..def4e85 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -165,6 +165,11 @@ If you are creating a branch that you want to immediately checkout, it's
>  easier to use the git checkout command with its `-b` option to create
>  a branch and check it out with a single command.
>  
> +When a branch is renamed so that it overwrites an existing branch unintended
> +problems can arise. This is because git refuses to discard the configuration
> +section of the overwritten branch. As a result git can become confused if, for
> +example, the branches involved were used for tracking two different remote
> +branches. The only way to fix this is to edit the configuration file manually.

I do not understand this bit about "refuse".

 - To "refuse to discard", somebody has to ask to discard ---
   who asks so and when?

 - Is there a reason to "refuse" when such a removal request is
   made?  If so, what is it?  If not, why refusal?

^ permalink raw reply

* Re: [PATCH] git-rev-list.txt: rev stands for revision, not reverse.
From: Junio C Hamano @ 2007-11-02 20:12 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20071102185509.GA5242@ins.uni-bonn.de>

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> Hello Junio,
>
> * Junio C Hamano wrote on Thu, Nov 01, 2007 at 08:51:11PM CET:
>> Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
>> 
>> > Yes, believe it or not, but I stumbled over the synopsis
>> >
>> > | git-rev-list - Lists commit objects in reverse chronological order
>> >
>> > asking myself whether rev could possibly mean "reverse".
>> > I hope this helps avoid this pitfall for others.
>> 
>> In addition to your patch,
>> 
>> 	git-rev-list - List commits from most recent to older
>> 
>> might be a good rewording?
>
> Is the reverse chronological order the primary sorting key at all?

It is mostly chrono but there is a topo element as well.  You
would never see a parent none of whose child is shown.

^ permalink raw reply

* Re: [PATCH 2/4] Remove unecessary hard-coding of EDITOR=':' VISUAL=':' in some test suites.
From: Junio C Hamano @ 2007-11-02 20:09 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1194017589-4669-2-git-send-email-krh@redhat.com>

Kristian Høgsberg <krh@redhat.com> writes:

> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> ---
>  t/t3501-revert-cherry-pick.sh |    4 ++--
>  t/t3901-i18n-patch.sh         |    8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)

This is a good patch, but a real commit message that says why
they are unnecessary is lacking.  Something like...

        Sourcing of ./test-lib.sh at the very beginning of test scripts
        already define and export them.

^ permalink raw reply

* Re: [PATCH 1/4] Add testcase for ammending and fixing author in git commit.
From: Junio C Hamano @ 2007-11-02 20:07 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1194017589-4669-1-git-send-email-krh@redhat.com>

Kristian Høgsberg <krh@redhat.com> writes:

> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> ---
>  t/t7501-commit.sh |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> index b151b51..3f2112a 100644
> --- a/t/t7501-commit.sh
> +++ b/t/t7501-commit.sh
> @@ -163,4 +163,14 @@ test_expect_success 'partial commit that involves removal (3)' '
>  
>  '
>  
> +author="The Real Author <someguy@his.email.org>"
> +test_expect_success 'amend commit to fix author' '
> +
> +	git reset --hard
> +	git cat-file -p HEAD | sed -e "s/author.*>/author $author/" > expected &&
> +	git commit --amend --author="$author" &&
> +	git cat-file -p HEAD > current &&
> +	diff expected current
> +	
> +'
>  test_done

This can't be right.  How are you ignoring the differences in
committer dates?

By the way, I _think_ git-commit.sh allows fixing author name/email
without molesting the author timestamp (i.e. takes it from the
amended commit).  That should probably be checked with the test
as well.

^ permalink raw reply

* [PATCH] New script: git-changelog.perl - revised
From: Ronald Landheer-Cieslak @ 2007-11-02 20:03 UTC (permalink / raw)
  To: git

I just noticed that I'd been sending personal replies to each and
every one of the replies I got - sorry about that.

Anyway, after Nicolas' suggestion, I moved the script into the contrib
directory and removed it from the Makefile, having the following patch
as net effect.
This is also available through git at
git://vlinder.landheer-cieslak.com/git/git.git#topic/git-log-changelog

As for difference wrt git2cl:
* less complicated
* less dependencies
* short hash for the commits are output in the change log
* git-changelog calls git-log with the proper parameters itself (no
need for the user to call it with a given set of parameters).

rlc

diff --git a/contrib/git-changelog/git-changelog.perl
b/contrib/git-changelog/git-changelog.perl
new file mode 100755
index 0000000..bffa1ab
--- /dev/null
+++ b/contrib/git-changelog/git-changelog.perl
@@ -0,0 +1,68 @@
+#!/usr/bin/perl -w
+
+use strict;
+
+sub fetch_log {
+       my $command='git log --pretty=format:"%ai|%an|%h|%s" ';
+       foreach (@_) {
+               $command .= ' ' . $_;
+       }
+       $command .= " |";
+       my $fh;
+       open $fh, $command
+               or die "Failed to fetch logs";
+       # logs are presented as lines in which fields are separated by pipes.
+       # the fields are the date in ISO format (so the date as we
want it extends to the
+       # first space), the name of the author, the abbreviated SHA1
hash and the subject line
+       my $date_time;
+       my $date;
+       my $cruft;
+       my $author;
+       my $hash;
+       my $subject;
+       my $prev_date="";
+       my @cache; # a cache of the changes for the current date (i.e.
while $prev_date eq $date)
+       my %entry; # a cache entry; $entry{'author'} is the entry and
@$entry{'changes'} are the changes for the author in question
+       while (<$fh>) {
+               ($date_time, $author, $hash, $subject) = split(/\|/);
+               ($date, $cruft) = split(/\s/, $date_time, 2);
+               chomp $author;
+
+               if ($date ne $prev_date)
+               {
+                       foreach (@cache)
+                       {
+                               # print out the line with the date and
the author
+                               my $changes = $_->{'changes'};
+                               print "\n" . $prev_date . "\t" .
$_->{'author'} . "\n" if ($#{$changes} != -1);
+                               foreach (@{$changes})
+                               {
+                                       print "\t" . $_->{'hash'} . ':
' . $_->{'subject'};
+                               }
+                       }
+                       $prev_date = $date;
+                       @cache = ();
+               }
+               # try to find an entry with the same author in the cache
+               my $found = -1;
+               my $i;
+               for ($i = 0; $i <= $#cache && $found == -1; $i++)
+               {
+                       if ($cache[$i]->{'author'} eq $author)
+                       {
+                               $found = $i;
+                       }
+               }
+               if ($found == -1)
+               {
+                       my $changes = ();
+                       push @cache, { 'author', $author, 'changes', $changes };
+                       $found = $#cache;
+               }
+               push @{$cache[$found]->{'changes'}}, { 'hash', $hash,
'subject', $subject };
+       }
+}
+
+fetch_log @ARGV;
+
+



-- 
Ronald Landheer-Cieslak
Software Architect
http://www.landheer-cieslak.com/
New White Paper: "Three Good Reasons to Plan Ahead"

^ permalink raw reply related

* Re: [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set
From: Blake Ramsdell @ 2007-11-02 20:03 UTC (permalink / raw)
  To: David Symonds, Junio C Hamano; +Cc: git
In-Reply-To: <ee77f5c20711020423t6ce58818gcc5220b6427ded1@mail.gmail.com>

On 11/2/07, David Symonds <dsymonds@gmail.com> wrote:
> It would probably be most appropriate for the autoconf script. Now
> that I look at configure.ac, there's already a test for iconv in
> there; is it not used?

The crux of this problem is that the prototype for iconv in
/usr/include/iconv.h is different between OS X 10.4 and OS X 10.5. So
the "right thing" is definitely to determine what is in the function
prototype, and then act accordingly.

>From OS X 10.4.10:

#define _LIBICONV_VERSION 0x0109    /* version number: (major<<8) + minor */
...
extern size_t iconv (iconv_t cd, const char* * inbuf, size_t
*inbytesleft, char* * outbuf, size_t *outbytesleft);

>From OS X 10.5:

#define _LIBICONV_VERSION 0x010B    /* version number: (major<<8) + minor */
...
size_t iconv (iconv_t /*cd*/,
        char ** __restrict /*inbuf*/,  size_t * __restrict /*inbytesleft*/,
        char ** __restrict /*outbuf*/, size_t * __restrict /*outbytesleft*/);

So what happened in git is that someone put in OLD_ICONV to
dynamically adjust the const-ness of parameter 2 to the iconv
function, and the way they chose to do that is to identify the OS
(more accurately, the kernel), and then I went and furthered that by
identifying the version.

Now that I look at it further, it seems that yanking OLD_ICONV
altogether is a better approach, and to just check _LIBICONV_VERSION
to make sure it's "new enough". Now, I'm not sure what that comparison
would be, but we know that "later than 0x0109" is a good start. Based
on the version difference between OS X 10.4 and 10.5 I note that there
is only 0x010A intervening.

This strategy presumes that this const parameter was const all the way
up to a particular point, and then stopped being const, which is
probably a reasonable assumption.

Blake
-- 
Blake Ramsdell | http://www.blakeramsdell.com

^ permalink raw reply

* Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options.
From: Junio C Hamano @ 2007-11-02 20:02 UTC (permalink / raw)
  To: David Symonds; +Cc: git, Andreas Ericsson
In-Reply-To: <11940160932021-git-send-email-dsymonds@gmail.com>

David Symonds <dsymonds@gmail.com> writes:

> Each of the new core.whitespace.* options (enumerated below) can be set to one
> of:
> 	* okay (default): Whitespace of this type is okay
> 	* warn: Whitespace of this type should be warned about
> 	* error: Whitespace of this type should raise an error
> 	* autofix: Whitespace of this type should be automatically fixed

Many problems at the conceptual level (I haven't look at the
patch yet).

We call these options (nowarn,warn,error,strip) in
apply.whitespace.  "strip" is a bit of misnomer, as we only
handled the trailing whitespace initially.  We should add "fix"
as a synonym to "strip".

The intention is to define what is an anomaly with
core.whitespace and then define what to do with it with
apply.whitespace.

And that is a good distinction.  You may usually use "fix", but
occasionally you would want to override it one-shot (when you do
want to have byte-to-byte identical application of the patch),
and the command line option "--whitespace=" lets you do so.
At least you need to extend --whitespace command line option
handling to allow these overridden.

Adding the "error" and "fix" to "diff" is a mistake --- there is
no error condition nor fixing there.  That shows how the
approach of your patch is inappropriate by trying to mix what
core.whitespace (give the definition of what is an error) and
apply.whitespace (specify what to do with an error) are designed
to do.

Defaulting to "nowarn" is wrong.  Trailing whitespace errors and
space before tab errors should be turned on by default as
before.

^ permalink raw reply

* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Junio C Hamano @ 2007-11-02 19:42 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Tom Prince, Steffen Prohaska, git
In-Reply-To: <472B2B8F.1060203@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> Tom Prince wrote:
>>
>> I haven't had occasion to use git-bisect much, but I was under the
>> impression that bisect could already handle merges, or any other shaped
>> history just fine.
>
> It appears the code supports your statement. I started writing on my
> hack-around about a year ago, and the merge-handling code got in with
> 1c4fea3a40e836dcee2f16091bf7bfba96c924d0 at Wed Mar 21 22:16:24 2007.
> Perhaps I shouldn't be so paranoid about useless merges anymore then.
> Hmm. I shall have to look into it. Perhaps Junio can clarify how it
> works? The man-page was terribly silent about how git-bisect handles
> merges.

Bisecting through merge is not a problem.  Not at all, from the
very beginning of the bisect command.

	Side note.  The commit you quote does not change (let
	alone fix) the semantics at all.  It is a pure
	optimization.  The theory behind how bisect works, see
	my OLS presentation (reachable from the gitwiki).

The real problem is what to do when the culprit turns out to be
a merge commit.  How to spot what really is wrong, and figure
out how to fix.  The problem is not for the tool but for the
human, and it is real.

Imagine this history.

      ---Z---o---X---...---o---A---C---D
          \                       /
           o---o---Y---...---o---B

Suppose that on the upper development line, the meaning of one
of the functions existed at Z was changed at commit X.  The
commits from Z leading to A change both the function's
implementation and all calling sites that existed at Z, as well
as new calling sites they add, to be consistent.  There is no
bug at A.

Suppose in the meantime the lower development line somebody
added a new calling site for that function at commit Y.  The
commits from Z leading to B all assume the old semantics of that
function and the callers and the callee are consistent with each
other.  There is no bug at B, either.

You merge to create C.  There is no textual conflict with this
three way merge, and the result merges cleanly.  You bisect
this, because you found D is bad and you know Z was good.  Your
bisect will find that C (merge) is broken.  Understandably so,
as at C, the new calling site of the function added by the lower
branch is not converted to the new semantics, while all the
other calling sites that already existed at Z would have been
converted by the merge.  The new calling site has semantic
adjustment needed, but you do not know that yet.  You need to
find out that is the cause of the breakage by looking at the
merge commit C and the history leading to it.

How would you do that?

Both "git diff A C" and "git diff B C" would be an enormous patch.
Each of them essentially shows the whole change on each branch
since they diverged.  The developers may have well behaved to
create good commits that follow the "commit small, commit often,
commit well contained units" mantra, and each individual commit
leading from Z to A and from Z to B may be easy to review and
understand, but looking at these small and easily reviewable
steps alone would not let you spot the breakage.  You need to
have a global picture of what the upper branch did (and
among many, one of them is to change the semantics of that
particular function) and look first at the huge "diff A C"
(which shows the change the lower branch introduces), and see if
that huge change is consistent with what have been done between
Z and A.

If you linearlize the history by rebasing the lower branch on
top of upper, instead of merging, the bug becomes much easier to
find and understand.  Your history would instead be:

    ---Z---o---X'--...---o---A---o---o---Y'--...---o---B'--D'

and there is a single commit Y' between A and B' that introduced
the new calling site that still uses the new semantics of the
function that was already in A.  "git show Y'" will be a much
smaller patch than "git diff A C" and it is much easier to deal
with.

^ permalink raw reply

* Re: Newbie: report of first experience with git-rebase.
From: Andreas Ericsson @ 2007-11-02 19:22 UTC (permalink / raw)
  To: Steven Grimm
  Cc: Junio C Hamano, J. Bruce Fields, Johannes Schindelin,
	Sergei Organov, git
In-Reply-To: <472B77AC.5080507@midwinter.com>

Steven Grimm wrote:
> Junio C Hamano wrote:
>> Andreas Ericsson <ae@op5.se> writes:
>>  
>>> Make "git rebase --skip" skip patches regardless of tree and index 
>>> state,
>>> but still refuse to *start* with dirty tree or index. That way, there's
>>> no risk of losing anything that can't be re-created unless the user asks
>>> for it.
>>>     
>>
>> Sounds like a plan.
>>   
> 
> I'm unclear how that helps the usability issue here at all, though it 
> doesn't sound like an unreasonable change on its own merit.
> 

Because 'git commit --skip' will not require the user to first edit the
tree so the index matches the upstream thing, and he doesn't have to
manually run 'git reset --hard' before --skip'ing.

It's a small improvement, but one that goes in the right direction.

Btw, the above sentence should've read "but still refuse to *start*
(as in, start a brand new rebase) with dirty tree or index"

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: git rm --cached
From: Remi Vanicat @ 2007-11-02 16:13 UTC (permalink / raw)
  To: git
In-Reply-To: <20071102021711.GA28703@fawkes.hq.digizenstudio.com>

Jing Xue <jingxue@digizenstudio.com> writes:

> In the following scenario, why do I have to run 'git reset' following
> 'git rm --cached 1.txt' to revert to exactly where I was before 'git add
> 1.txt'?  Shouldn't 'git rm --cached' have done that already?

Observed behavior are exactly what I expected: 'git rm --cached' mark
the file in the index as been deleted without deleting it in the
working directories, it did not but the index it was before the 
'git add 1.txt'.

You probably want to use git reset HEAD -- 1.txt to unstage
modification on 1.txt

-- 
Rémi Vanicat

^ permalink raw reply

* Re: [PATCH] New script: git-changelog.perl
From: Jakub Narebski @ 2007-11-02 19:18 UTC (permalink / raw)
  To: git

Subject: Re: [PATCH] New script: git-changelog.perl
From: "Ronald Landheer-Cieslak" <ronald@landheer-cieslak.com>
To: "Jakub Narebski" <jnareb@gmail.com>

On Nov 2, 2007 1:07 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> [Cc: Ronald Landheer-Cieslak <ronald@landheer-cieslak.com>,
>  git@vger.kernel.org]
>
> Ronald Landheer-Cieslak wrote:
>
>> I've written a little script that will format the changes as reported
>> by git-log in a ChangeLog-like format. Entries look like this:
>> <date>\t<author>\n\t<sha1>: <subject>
>
> How it compares to existing git2cl tool?:
>   http://josefsson.org/git2cl/
>   http://git.or.cz/gitwiki/InterfacesFrontendsAndTools

I must admit I didn't know about git2cl. The first difference I see is
that it's simpler and has less dependencies: my script doesn't use any
external modules (e.g. it doesn't parse the date) but just parses a
formatted output from git-log.

Other than that, there's not much difference: my output is a bit
sparser and my script invokes the proper git-log command by itself, so
you don't have to pipe through it (i.e. all you do is git-changelog >
ChangeLog to get your change log) but that's pretty much it.

If you want, you can have a look at it at
git://vlinder.landheer-cieslak.com/git/git.git#topic/git-log-changelog

rlc

-- 
Ronald Landheer-Cieslak
Software Architect
http://www.landheer-cieslak.com/
New White Paper: "Three Good Reasons to Plan Ahead"

^ 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