Confusing git log --- First time bug submission please advise on best practices

Confusing git log --- First time bug submission please advise on best practices

[Francis Stephens]

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

My co-worker has an inconsistent git log output. Please see that
attached files for output (I've made a best effort to remove
confidential info from them).

Comparing the two log commands we can see that master and
originssh/master have a shared common commit at

    <John Doe> (4 hours ago) d85832d
  More pom fixes

The top commit for originssh/master and the second to top for master.

I would expect that both logs would share an _identical_ history from
that commit onward. But the log for master contains the following

  <Jeremy Doe> (27 hours ago) 239ea21  (my-work)
  renamed class

  <Jeremy Doe> (28 hours ago) 27750b2
  Merge branch 'master' of
http://githost.companyname-dev.local/trading-development/sports-container-framework

and

  <Jeremy Doe> (2 days ago) a933acb
  white space changes

  <Jeremy Doe> (2 days ago) b5e51e7
  Merge branch 'master' of
http://githost.companyname-dev.local/trading-development/sports-container-framework

  <Jeremy Doe> (2 days ago) 3a0f787
  removed public methods

  <Jeremy Doe> (2 days ago) 4e91130
  added the xml deserialisation

None of which appear in the originssh/master log. Is there a scenario
in which this is expected. It was my understanding that any two
commits with the same hash have exactly the same history.

Thanks for your time.





Technical details:

He is using the Windows git client version 1.8.5.2.msysgit.0
Running on Windows 7
Every command was run through git bash

Background:

He previously had tortoise-git installed, but I had him uninstall it
to solve some ssh problems
While uninstalling tortoise-git we also reinstalled git - I don't know
what the previous version of git was.

Output of

    git branch -avv
    * master                   6833fd4 Completed merge
      my-work                  239ea21 renamed class
      remotes/origin/HEAD      -> origin/master
      remotes/origin/master    f269789 Cleaning up GIT step 1
      remotes/originssh/master d85832d More pom fixes

The background on the originssh remote branch is that his origin uses
an HTTP url which wasn't allowing him to push a large commit. I helped
him to create originssh to allow the push to be made.

[-- Attachment #2: master --]
[-- Type: application/octet-stream, Size: 1364 bytes --]

git lg master

<Jeremy Doe> (2 hours ago) 6833fd4  (HEAD, master)

Completed merge



<John Doe> (4 hours ago) d85832d  (originssh/master)

More pom fixes



<John Doe> (20 hours ago) 39f3e87 

few pom fixes so project can build in nexus



<John Doe> (20 hours ago) f3cd7ad 

fixed a few bugs that was stopping it from building



<John Doe> (21 hours ago) 782fb1c 

updated kangarooboxing-rnd module



<John Doe> (21 hours ago) 93b56d3 

.gitignore file to ignore .iml and .idea files



<John Doe> (21 hours ago) 9dec2df 

Updated code for creation incl. unit tests



<Jane Doe> (22 hours ago) 8f4ba15 

Added market model



<Jeremy Doe> (27 hours ago) 239ea21  (my-work)

renamed class



<Jeremy Doe> (28 hours ago) 27750b2 

Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework



<Jane Doe> (2 days ago) f269789  (origin/master, origin/HEAD)

Cleaning up GIT step 1



<Jeremy Doe> (2 days ago) a933acb 

white space changes



<Jeremy Doe> (2 days ago) b5e51e7 

Merge branch 'master' of http://githost.companyname-dev.local/trading-development/sports-container-framework



<Jeremy Doe> (2 days ago) 3a0f787 

removed public methods



<Jeremy Doe> (2 days ago) 4e91130 

added the xml deserialisation



<John Doe> (2 days ago) e10e877 

Merged creation to Openbet code with Jane's newest code


[-- Attachment #3: originssh_master --]
[-- Type: application/octet-stream, Size: 749 bytes --]

git lg originssh/master

<John Doe> (4 hours ago) d85832d  (originssh/master)

More pom fixes



<John Doe> (20 hours ago) 39f3e87 

few pom fixes so project can build in nexus



<John Doe> (20 hours ago) f3cd7ad 

fixed a few bugs that was stopping it from building



<John Doe> (21 hours ago) 782fb1c 

updated kangarooboxing-rnd module



<John Doe> (21 hours ago) 93b56d3 

.gitignore file to ignore .iml and .idea files



<John Doe> (21 hours ago) 9dec2df 

Updated code for creation incl. unit tests



<Jane Doe> (22 hours ago) 8f4ba15 

Added market model



<Jane Doe> (2 days ago) f269789  (origin/master, origin/HEAD)

Cleaning up GIT step 1



<John Doe> (2 days ago) e10e877 

Merged creation to Openbet code with Jane's newest code

Re: Confusing git log --- First time bug submission please advise on best practices

[Vincent van Ravesteijn]

On Thu, Feb 6, 2014 at 3:02 PM, Francis Stephens
<francisstephens@gmail.com> wrote:
>
> My co-worker has an inconsistent git log output. Please see that
> attached files for output (I've made a best effort to remove
> confidential info from them).
>
> Comparing the two log commands we can see that master and
> originssh/master have a shared common commit at
>
> <John Doe> (4 hours ago) d85832d
> More pom fixes
>
> The top commit for originssh/master and the second to top for master.
>
> I would expect that both logs would share an _identical_ history from
> that commit onward. But the log for master contains the following
>
> <Jeremy Doe> (27 hours ago) 239ea21 (my-work)
> renamed class
>
> <Jeremy Doe> (28 hours ago) 27750b2
> Merge branch 'master' of
> http://githost.companyname-dev.local/trading-development/sports-container-framework
>
> and
>
> <Jeremy Doe> (2 days ago) a933acb
> white space changes
>
> <Jeremy Doe> (2 days ago) b5e51e7
> Merge branch 'master' of
> http://githost.companyname-dev.local/trading-development/sports-container-framework
>
> <Jeremy Doe> (2 days ago) 3a0f787
> removed public methods
>
> <Jeremy Doe> (2 days ago) 4e91130
> added the xml deserialisation
>
> None of which appear in the originssh/master log. Is there a scenario
> in which this is expected. It was my understanding that any two
> commits with the same hash have exactly the same history.
>
> Thanks for your time.

The commits that are in the log for master and which are not in the
log for originssh/master are merged in at "6833fd4 (HEAD, master);
Completed merge".

As "git log" can only present the commits in a linear way, it shows
the commits from the ancentry of both parents of HEAD in a reverse
chronological order. This means that the commits from the two
ancestries are mixed and commits that are shown after each other don't
have to be parent and child. See the documentation of "git log" and
the section "Commit Ordering": "By default, the commits are shown in
reverse chronological order."

Vincent

Re: Confusing git log --- First time bug submission please advise on best practices

[David Kastrup]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> The commits that are in the log for master and which are not in the
> log for originssh/master are merged in at "6833fd4 (HEAD, master);
> Completed merge".
>
> As "git log" can only present the commits in a linear way, it shows
> the commits from the ancentry of both parents of HEAD in a reverse
> chronological order. This means that the commits from the two
> ancestries are mixed and commits that are shown after each other don't
> have to be parent and child. See the documentation of "git log" and
> the section "Commit Ordering": "By default, the commits are shown in
> reverse chronological order."

git log --graph can help with getting a better picture.

-- 
David Kastrup

Re: Confusing git log --- First time bug submission please advise on best practices

[Francis Stephens]

Thanks for your clear response. I can see where I went wrong now.

On Thu, Feb 6, 2014 at 4:10 PM, David Kastrup <dak@gnu.org> wrote:
> Vincent van Ravesteijn <vfr@lyx.org> writes:
>
>> The commits that are in the log for master and which are not in the
>> log for originssh/master are merged in at "6833fd4 (HEAD, master);
>> Completed merge".
>>
>> As "git log" can only present the commits in a linear way, it shows
>> the commits from the ancentry of both parents of HEAD in a reverse
>> chronological order. This means that the commits from the two
>> ancestries are mixed and commits that are shown after each other don't
>> have to be parent and child. See the documentation of "git log" and
>> the section "Commit Ordering": "By default, the commits are shown in
>> reverse chronological order."
>
> git log --graph can help with getting a better picture.
>
> --
> David Kastrup

Re: Confusing git log --- First time bug submission please advise on best practices

[Duy Nguyen]

On Fri, Feb 07, 2014 at 09:43:46AM +0000, Francis Stephens wrote:
> Thanks for your clear response. I can see where I went wrong now.

Maybe something like this would help avoid confusion a bit in the
future? This toy patch puts a horizontal line as a break between two
commits if they are not related, so we can clearly see linear commit
segments.

--graph definitely helps, but it's too many threads for topic-based
development model like git.git that I avoid it most of the time.

-- 8< --
diff --git a/log-tree.c b/log-tree.c
index 08970bf..7841bf2 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -795,6 +795,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 
 int log_tree_commit(struct rev_info *opt, struct commit *commit)
 {
+	static struct commit_list *last_parents;
 	struct log_info log;
 	int shown;
 
@@ -805,6 +806,17 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
+	if (last_parents) {
+		struct commit_list *p = last_parents;
+		int got_parent = 0;
+		for (; p && !got_parent; p = p->next)
+			got_parent = !hashcmp(p->item->object.sha1,
+					      commit->object.sha1);
+		if (!got_parent)
+			printf("______________________________________________________________________\n");
+		free_commit_list(last_parents);
+		last_parents = NULL;
+	}
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
@@ -813,5 +825,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	}
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
+	last_parents = copy_commit_list(commit->parents);
 	return shown;
 }
-- 8< --

> 
> On Thu, Feb 6, 2014 at 4:10 PM, David Kastrup <dak@gnu.org> wrote:
> > Vincent van Ravesteijn <vfr@lyx.org> writes:
> >
> >> The commits that are in the log for master and which are not in the
> >> log for originssh/master are merged in at "6833fd4 (HEAD, master);
> >> Completed merge".
> >>
> >> As "git log" can only present the commits in a linear way, it shows
> >> the commits from the ancentry of both parents of HEAD in a reverse
> >> chronological order. This means that the commits from the two
> >> ancestries are mixed and commits that are shown after each other don't
> >> have to be parent and child. See the documentation of "git log" and
> >> the section "Commit Ordering": "By default, the commits are shown in
> >> reverse chronological order."
> >
> > git log --graph can help with getting a better picture.

Re: Confusing git log --- First time bug submission please advise on best practices

[demerphq]

On 7 February 2014 18:26, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Feb 07, 2014 at 09:43:46AM +0000, Francis Stephens wrote:
>> Thanks for your clear response. I can see where I went wrong now.
>
> Maybe something like this would help avoid confusion a bit in the
> future? This toy patch puts a horizontal line as a break between two
> commits if they are not related, so we can clearly see linear commit
> segments.

FWIW, this would have saved a lot of head scratching at work over the years.

I'd love to see this in place.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

[PATCH] log: add --show-linear-break to help see non-linear history

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This is a more serious attempt to make non-linear history more
 visible without --graph. It looks like this

    commit e4ddb05720710213108cd13ddd5a115e12a6211d
    Author: Andy Spencer <andy753421@gmail.com>
    
        tree_entry_interesting: match against all pathspecs
    
                        ..........
    
    commit a0332337be53f266682279c72a5e553986638c87
    Author: Jeff King <peff@peff.net>
    
        t7700: do not use "touch" unnecessarily
    
    commit 088304bf73b9b4149e04d2246fe08a06eef6e795
    Author: Jeff King <peff@peff.net>
    
        t7501: fix "empty commit" test with NO_PERL
    
                        ..........
    
    commit 74b4f7f27736f3e196a4eb3db41c68e37a6e2160
    Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
    
        tree-walk.c: ignore trailing slash on submodule in tree_entry_interesting()

 You can use fancier break bars from [1] instead of plain dots. I'm
 not sure it works with all combinations of rev-list. But at least
 log (with/out pathspec) and log --reverse work.
 
 [1] http://www.ascii-art.de/ascii/ab/border.txt

 Documentation/rev-list-options.txt |  7 +++++
 log-tree.c                         |  4 +++
 revision.c                         | 52 +++++++++++++++++++++++++++++++++++---
 revision.h                         |  6 +++++
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 03533af..a0780be 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -750,6 +750,13 @@ This enables parent rewriting, see 'History Simplification' below.
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
 
+--show-linear-break[=<break bar>]::
+	When --graph is not used, all history branches are flatten and
+	could be hard to see that the two consecutive commits do not
+	belong to a linear branch. This option puts a break bar in
+	between them in that case. If `<break bar>` is specified, it
+	is the string that will be shown instead of the default one.
+
 ifdef::git-rev-list[]
 --count::
 	Print a number stating how many commits would have been
diff --git a/log-tree.c b/log-tree.c
index 08970bf..17862f6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -805,12 +805,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
+	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
 		show_log(opt);
 		shown = 1;
 	}
+	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
 	return shown;
diff --git a/revision.c b/revision.c
index a0df72f..ff3107f 100644
--- a/revision.c
+++ b/revision.c
@@ -1836,6 +1836,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->notes_opt.use_default_notes = 1;
 	} else if (!strcmp(arg, "--show-signature")) {
 		revs->show_signature = 1;
+	} else if (!strcmp(arg, "--show-linear-break")) {
+		revs->track_linear = 1;
+		revs->break_bar = "                    ..........";
+	} else if (starts_with(arg, "--show-linear-break=")) {
+		revs->track_linear = 1;
+		revs->break_bar = xstrdup(arg + 20);
 	} else if (starts_with(arg, "--show-notes=") ||
 		   starts_with(arg, "--notes=")) {
 		struct strbuf buf = STRBUF_INIT;
@@ -2901,6 +2907,32 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
 	return action;
 }
 
+define_commit_slab(saved_linear, int);
+
+static void track_linear(struct rev_info *revs, struct commit *commit)
+{
+	struct commit_list *p = revs->previous_parents;
+
+	if (p) {
+		int got_parent = 0;
+		for (; p && !got_parent; p = p->next)
+			got_parent = !hashcmp(p->item->object.sha1,
+					      commit->object.sha1);
+		revs->linear = got_parent;
+		free_commit_list(revs->previous_parents);
+	} else
+		revs->linear = 1;
+	if (revs->reverse) {
+		if (!revs->saved_linear_slab) {
+			revs->saved_linear_slab = xmalloc(sizeof(struct saved_linear));
+			init_saved_linear(revs->saved_linear_slab);
+		}
+
+		*saved_linear_at(revs->saved_linear_slab, commit) = revs->linear;
+	}
+	revs->previous_parents = copy_commit_list(commit->parents);
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
 	if (!revs->commits)
@@ -2933,6 +2965,9 @@ static struct commit *get_revision_1(struct rev_info *revs)
 				    sha1_to_hex(commit->object.sha1));
 		}
 
+		if (revs->track_linear)
+			track_linear(revs, commit);
+
 		switch (simplify_commit(revs, commit)) {
 		case commit_ignore:
 			continue;
@@ -3106,14 +3141,25 @@ struct commit *get_revision(struct rev_info *revs)
 		revs->reverse_output_stage = 1;
 	}
 
-	if (revs->reverse_output_stage)
-		return pop_commit(&revs->commits);
+	if (revs->reverse_output_stage) {
+		c = pop_commit(&revs->commits);
+		if (revs->track_linear)
+			revs->linear = *saved_linear_at(revs->saved_linear_slab, c);
+		return c;
+	}
 
 	c = get_revision_internal(revs);
 	if (c && revs->graph)
 		graph_update(revs->graph, c);
-	if (!c)
+	if (!c) {
 		free_saved_parents(revs);
+		if (revs->saved_linear_slab)
+			clear_saved_linear(revs->saved_linear_slab);
+		if (revs->previous_parents) {
+			free_commit_list(revs->previous_parents);
+			revs->previous_parents = NULL;
+		}
+	}
 	return c;
 }
 
diff --git a/revision.h b/revision.h
index 88967d6..bffdc15 100644
--- a/revision.h
+++ b/revision.h
@@ -134,6 +134,7 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1,
+			track_linear:1,
 			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 	unsigned int	leak_pending:1;
@@ -195,6 +196,11 @@ struct rev_info {
 
 	/* copies of the parent lists, for --full-diff display */
 	struct saved_parents *saved_parents_slab;
+
+	struct commit_list *previous_parents;
+	struct saved_linear *saved_linear_slab;
+	char *break_bar;
+	unsigned int linear;
 };
 
 extern int ref_excluded(struct string_list *, const char *path);
-- 
1.8.5.2.240.g8478abd

[PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 renames the option name to --nonlinear-barrier and fixes using it
 with --dense. Best used with --no-merges to see patch series.

 Documentation/rev-list-options.txt |  7 ++++++
 log-tree.c                         |  4 +++
 revision.c                         | 51 +++++++++++++++++++++++++++++++++++---
 revision.h                         |  6 +++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 03533af..435aa2d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -750,6 +750,13 @@ This enables parent rewriting, see 'History Simplification' below.
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
 
+--nonlinear-barrier[=<barrier>]::
+	When --graph is not used, all history branches are flatten and
+	could be hard to see that the two consecutive commits do not
+	belong to a linear branch. This option puts a barrier in
+	between them in that case. If `<barrier>` is specified, it
+	is the string that will be shown instead of the default one.
+
 ifdef::git-rev-list[]
 --count::
 	Print a number stating how many commits would have been
diff --git a/log-tree.c b/log-tree.c
index 08970bf..17862f6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -805,12 +805,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
+	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
 		show_log(opt);
 		shown = 1;
 	}
+	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
 	return shown;
diff --git a/revision.c b/revision.c
index a68fde6..0a4849e 100644
--- a/revision.c
+++ b/revision.c
@@ -1832,6 +1832,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->notes_opt.use_default_notes = 1;
 	} else if (!strcmp(arg, "--show-signature")) {
 		revs->show_signature = 1;
+	} else if (!strcmp(arg, "--nonlinear-barrier")) {
+		revs->track_linear = 1;
+		revs->break_bar = "                    ..........";
+	} else if (starts_with(arg, "--nonlinear-barrier=")) {
+		revs->track_linear = 1;
+		revs->break_bar = xstrdup(arg + 20);
 	} else if (starts_with(arg, "--show-notes=") ||
 		   starts_with(arg, "--notes=")) {
 		struct strbuf buf = STRBUF_INIT;
@@ -2897,6 +2903,32 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
 	return action;
 }
 
+define_commit_slab(saved_linear, int);
+
+static void track_linear(struct rev_info *revs, struct commit *commit)
+{
+	struct commit_list *p = revs->previous_parents;
+
+	if (p) {
+		int got_parent = 0;
+		for (; p && !got_parent; p = p->next)
+			got_parent = !hashcmp(p->item->object.sha1,
+					      commit->object.sha1);
+		revs->linear = got_parent;
+		free_commit_list(revs->previous_parents);
+	} else
+		revs->linear = 1;
+	if (revs->reverse) {
+		if (!revs->saved_linear_slab) {
+			revs->saved_linear_slab = xmalloc(sizeof(struct saved_linear));
+			init_saved_linear(revs->saved_linear_slab);
+		}
+
+		*saved_linear_at(revs->saved_linear_slab, commit) = revs->linear;
+	}
+	revs->previous_parents = copy_commit_list(commit->parents);
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
 	if (!revs->commits)
@@ -2936,6 +2968,8 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			die("Failed to simplify parents of commit %s",
 			    sha1_to_hex(commit->object.sha1));
 		default:
+			if (revs->track_linear)
+				track_linear(revs, commit);
 			return commit;
 		}
 	} while (revs->commits);
@@ -3102,14 +3136,25 @@ struct commit *get_revision(struct rev_info *revs)
 		revs->reverse_output_stage = 1;
 	}
 
-	if (revs->reverse_output_stage)
-		return pop_commit(&revs->commits);
+	if (revs->reverse_output_stage) {
+		c = pop_commit(&revs->commits);
+		if (revs->track_linear)
+			revs->linear = *saved_linear_at(revs->saved_linear_slab, c);
+		return c;
+	}
 
 	c = get_revision_internal(revs);
 	if (c && revs->graph)
 		graph_update(revs->graph, c);
-	if (!c)
+	if (!c) {
 		free_saved_parents(revs);
+		if (revs->saved_linear_slab)
+			clear_saved_linear(revs->saved_linear_slab);
+		if (revs->previous_parents) {
+			free_commit_list(revs->previous_parents);
+			revs->previous_parents = NULL;
+		}
+	}
 	return c;
 }
 
diff --git a/revision.h b/revision.h
index 88967d6..bffdc15 100644
--- a/revision.h
+++ b/revision.h
@@ -134,6 +134,7 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1,
+			track_linear:1,
 			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 	unsigned int	leak_pending:1;
@@ -195,6 +196,11 @@ struct rev_info {
 
 	/* copies of the parent lists, for --full-diff display */
 	struct saved_parents *saved_parents_slab;
+
+	struct commit_list *previous_parents;
+	struct saved_linear *saved_linear_slab;
+	char *break_bar;
+	unsigned int linear;
 };
 
 extern int ref_excluded(struct string_list *, const char *path);
-- 
1.9.0.40.gaa8c3ea

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Eric Sunshine]

On Mon, Mar 17, 2014 at 8:51 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  v2 renames the option name to --nonlinear-barrier and fixes using it
>  with --dense. Best used with --no-merges to see patch series.
>
>  Documentation/rev-list-options.txt |  7 ++++++
>  log-tree.c                         |  4 +++
>  revision.c                         | 51 +++++++++++++++++++++++++++++++++++---
>  revision.h                         |  6 +++++
>  4 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 03533af..435aa2d 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -750,6 +750,13 @@ This enables parent rewriting, see 'History Simplification' below.
>  This implies the `--topo-order` option by default, but the
>  `--date-order` option may also be specified.
>
> +--nonlinear-barrier[=<barrier>]::
> +       When --graph is not used, all history branches are flatten and

s/flatten/flattened/

> +       could be hard to see that the two consecutive commits do not

s/and could be/which can make it/

> +       belong to a linear branch. This option puts a barrier in
> +       between them in that case. If `<barrier>` is specified, it
> +       is the string that will be shown instead of the default one.
> +
>  ifdef::git-rev-list[]
>  --count::
>         Print a number stating how many commits would have been

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  v2 renames the option name to --nonlinear-barrier and fixes using it
>  with --dense. Best used with --no-merges to see patch series.

I think that the earlier name "show linear-break" is more easily
understood than the new name, but maybe that is just me.  It's not
like you are blocking something from going forward with a barrier,
and internally it is called a "break-bar".

>  Documentation/rev-list-options.txt |  7 ++++++
>  log-tree.c                         |  4 +++
>  revision.c                         | 51 +++++++++++++++++++++++++++++++++++---
>  revision.h                         |  6 +++++
>  4 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 03533af..435aa2d 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -750,6 +750,13 @@ This enables parent rewriting, see 'History Simplification' below.
>  This implies the `--topo-order` option by default, but the
>  `--date-order` option may also be specified.
>  
> +--nonlinear-barrier[=<barrier>]::
> +	When --graph is not used, all history branches are flatten and

s/flatten and/flattened and it/, perhaps?

> +	could be hard to see that the two consecutive commits do not
> +	belong to a linear branch. This option puts a barrier in
> +	between them in that case. If `<barrier>` is specified, it
> +	is the string that will be shown instead of the default one.
> +
>  ifdef::git-rev-list[]
>  --count::
>  	Print a number stating how many commits would have been
> diff --git a/log-tree.c b/log-tree.c
> index 08970bf..17862f6 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -805,12 +805,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
>  	if (opt->line_level_traverse)
>  		return line_log_print(opt, commit);
>  
> +	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
> +		printf("\n%s\n", opt->break_bar);
>  	shown = log_tree_diff(opt, commit, &log);
>  	if (!shown && opt->loginfo && opt->always_show_header) {
>  		log.parent = NULL;
>  		show_log(opt);
>  		shown = 1;
>  	}
> +	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
> +		printf("\n%s\n", opt->break_bar);

Each time get_revision() returns a commit, it is inspected and
opt->linear is updated, this function is called and we show the
break in the single-strand-of-pearls if this is not a parent of the
commit we showed immediately before.  If we are showing the commit
in reverse, we have to go the other way around, showing the commit
and then the break.

OK.  It makes sense to me.

>  	opt->loginfo = NULL;
>  	maybe_flush_or_die(stdout, "stdout");
>  	return shown;

Does this new feature interact with -z format output in any way?
Should it, and if so how?

> diff --git a/revision.c b/revision.c
> index a68fde6..0a4849e 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1832,6 +1832,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		revs->notes_opt.use_default_notes = 1;
>  	} else if (!strcmp(arg, "--show-signature")) {
>  		revs->show_signature = 1;
> +	} else if (!strcmp(arg, "--nonlinear-barrier")) {
> +		revs->track_linear = 1;
> +		revs->break_bar = "                    ..........";
> +	} else if (starts_with(arg, "--nonlinear-barrier=")) {
> +		revs->track_linear = 1;
> +		revs->break_bar = xstrdup(arg + 20);
>  	} else if (starts_with(arg, "--show-notes=") ||
>  		   starts_with(arg, "--notes=")) {
>  		struct strbuf buf = STRBUF_INIT;
> @@ -2897,6 +2903,32 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
>  	return action;
>  }
>  
> +define_commit_slab(saved_linear, int);
> +
> +static void track_linear(struct rev_info *revs, struct commit *commit)
> +{
> +	struct commit_list *p = revs->previous_parents;
> +
> +	if (p) {
> +		int got_parent = 0;
> +		for (; p && !got_parent; p = p->next)
> +			got_parent = !hashcmp(p->item->object.sha1,
> +					      commit->object.sha1);
> +		revs->linear = got_parent;
> +		free_commit_list(revs->previous_parents);
> +	} else
> +		revs->linear = 1;
> +	if (revs->reverse) {
> +		if (!revs->saved_linear_slab) {
> +			revs->saved_linear_slab = xmalloc(sizeof(struct saved_linear));
> +			init_saved_linear(revs->saved_linear_slab);
> +		}
> +
> +		*saved_linear_at(revs->saved_linear_slab, commit) = revs->linear;
> +	}
> +	revs->previous_parents = copy_commit_list(commit->parents);

We are showing commit, and the parents (after history
simplification) of the commit we showed before this commit is kept
in previous-parents.  If we are one of them, we are showing
linearly, which makes sense.  While we are accumulating the output
in the forward direction in preparation for later emitting them in
reverse, we need to save away the linear-ness bit somewhere, and a
slab is a logical place to save that, which also makes sense.  But
why do you need a full int?  Doesn't an unsigned char wide enough?

I also wondered if the saved-parents slab we already have can be
easily reused for this, but it probably would not help.

I do not quite understand the "if we do not have previous parents"
bit, though.  Is it meant to trigger only at the very beginning?

Thanks.

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Duy Nguyen]

On Tue, Mar 18, 2014 at 3:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  v2 renames the option name to --nonlinear-barrier and fixes using it
>>  with --dense. Best used with --no-merges to see patch series.
>
> I think that the earlier name "show linear-break" is more easily
> understood than the new name, but maybe that is just me.  It's not
> like you are blocking something from going forward with a barrier,
> and internally it is called a "break-bar".

I'll change it back.

>>       opt->loginfo = NULL;
>>       maybe_flush_or_die(stdout, "stdout");
>>       return shown;
>
> Does this new feature interact with -z format output in any way?

Hmm.. never thought of it. Right now it's part of the previous commit.

> Should it, and if so how?

No idea.

>> +define_commit_slab(saved_linear, int);
>> +
>> +static void track_linear(struct rev_info *revs, struct commit *commit)
>> +{
>> +     struct commit_list *p = revs->previous_parents;
>> +
>> +     if (p) {
>> +             int got_parent = 0;
>> +             for (; p && !got_parent; p = p->next)
>> +                     got_parent = !hashcmp(p->item->object.sha1,
>> +                                           commit->object.sha1);
>> +             revs->linear = got_parent;
>> +             free_commit_list(revs->previous_parents);
>> +     } else
>> +             revs->linear = 1;
>> +     if (revs->reverse) {
>> +             if (!revs->saved_linear_slab) {
>> +                     revs->saved_linear_slab = xmalloc(sizeof(struct saved_linear));
>> +                     init_saved_linear(revs->saved_linear_slab);
>> +             }
>> +
>> +             *saved_linear_at(revs->saved_linear_slab, commit) = revs->linear;
>> +     }
>> +     revs->previous_parents = copy_commit_list(commit->parents);
>
> We are showing commit, and the parents (after history
> simplification) of the commit we showed before this commit is kept
> in previous-parents.  If we are one of them, we are showing
> linearly, which makes sense.  While we are accumulating the output
> in the forward direction in preparation for later emitting them in
> reverse, we need to save away the linear-ness bit somewhere, and a
> slab is a logical place to save that, which also makes sense.  But
> why do you need a full int?  Doesn't an unsigned char wide enough?

Yes it is. Will change.

> I also wondered if the saved-parents slab we already have can be
> easily reused for this, but it probably would not help.

That could end up a maintenance nightmare. revision.c is complex
enough as it is.

> I do not quite understand the "if we do not have previous parents"
> bit, though.  Is it meant to trigger only at the very beginning?

Only at the beginning.
-- 
Duy

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> I do not quite understand the "if we do not have previous parents"
>> bit, though.  Is it meant to trigger only at the very beginning?
>
> Only at the beginning.

Then I am not sure !revs->previous_parents match that condition.

What happens in a history with more than one root commits and the
first one the revision traversal finds is one of the roots?  When
showing the second commit, after processing the root and storing its
parent list, which is NULL, to revs->previous_parents, wouldn't it
mistakenly think that it is the first round and fail to show the
break line?

[PATCH v3 1/2] object.h: centralize object flag allocation

[Nguyễn Thái Ngọc Duy]

While the field "flags" is mainly used by the revision walker, it is
also used in many other places. Centralize the whole flag allocation to
one place for a better overview (and easier to move flags if we have
too).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 The next step could be define (instead of document) all of them in
 a single place. But that may result in naming conflicts and stuff
 for little gain.

 bisect.c        |  3 +--
 builtin/blame.c |  2 +-
 bundle.c        |  1 +
 commit.c        |  2 +-
 fetch-pack.c    |  1 +
 http-push.c     |  3 +--
 object.h        | 13 +++++++++++++
 revision.h      |  1 +
 sha1_name.c     |  2 ++
 upload-pack.c   |  2 +-
 walker.c        |  1 +
 11 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/bisect.c b/bisect.c
index 8448d27..d6e851d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,8 +21,7 @@ static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
 static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
 
-/* bits #0-15 in revision.h */
-
+/* Remember to update object flag allocation in object.h */
 #define COUNTED		(1u<<16)
 
 /*
diff --git a/builtin/blame.c b/builtin/blame.c
index e5b5d71..88cb799 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -74,7 +74,7 @@ static unsigned blame_copy_score;
 #define BLAME_DEFAULT_MOVE_SCORE	20
 #define BLAME_DEFAULT_COPY_SCORE	40
 
-/* bits #0..7 in revision.h, #8..11 used for merge_bases() in commit.c */
+/* Remember to update object flag allocation in object.h */
 #define METAINFO_SHOWN		(1u<<12)
 #define MORE_THAN_ONE_PATH	(1u<<13)
 
diff --git a/bundle.c b/bundle.c
index a85e0e4..1222952 100644
--- a/bundle.c
+++ b/bundle.c
@@ -120,6 +120,7 @@ static int list_refs(struct ref_list *r, int argc, const char **argv)
 	return 0;
 }
 
+/* Remember to update object flag allocation in object.h */
 #define PREREQ_MARK (1u<<16)
 
 int verify_bundle(struct bundle_header *header, int verbose)
diff --git a/commit.c b/commit.c
index 0f28902..f479331 100644
--- a/commit.c
+++ b/commit.c
@@ -721,7 +721,7 @@ void sort_in_topological_order(struct commit_list **list, enum rev_sort_order so
 
 /* merge-base stuff */
 
-/* bits #0..15 in revision.h */
+/* Remember to update object flag allocation in object.h */
 #define PARENT1		(1u<<16)
 #define PARENT2		(1u<<17)
 #define STALE		(1u<<18)
diff --git a/fetch-pack.c b/fetch-pack.c
index f061f1f..fd470e7 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -26,6 +26,7 @@ static int agent_supported;
 static struct lock_file shallow_lock;
 static const char *alternate_shallow_file;
 
+/* Remember to update object flag allocation in object.h */
 #define COMPLETE	(1U << 0)
 #define COMMON		(1U << 1)
 #define COMMON_REF	(1U << 2)
diff --git a/http-push.c b/http-push.c
index d4b40c9..f2c56c8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -64,8 +64,7 @@ enum XML_Status {
 #define LOCK_TIME 600
 #define LOCK_REFRESH 30
 
-/* bits #0-15 in revision.h */
-
+/* Remember to update object flag allocation in object.h */
 #define LOCAL    (1u<<16)
 #define REMOTE   (1u<<17)
 #define FETCHING (1u<<18)
diff --git a/object.h b/object.h
index 732bf4d..768490b 100644
--- a/object.h
+++ b/object.h
@@ -26,6 +26,19 @@ struct object_array {
 #define OBJECT_ARRAY_INIT { 0, 0, NULL }
 
 #define TYPE_BITS   3
+/*
+ * object flag allocation:
+ * revision.h:      0------10
+ * fetch-pack.c:    0---4
+ * walker.c:        0-2
+ * upload-pack.c:             11---------19
+ * builtin/blame.c:              12-13
+ * bisect.c:                          16
+ * bundle.c:                          16
+ * http-push.c:                       16-19
+ * commit.c:                          16-19
+ * sha1_name.c:                            20
+ */
 #define FLAG_BITS  27
 
 /*
diff --git a/revision.h b/revision.h
index 1eb94c1..0262bbd 100644
--- a/revision.h
+++ b/revision.h
@@ -7,6 +7,7 @@
 #include "commit.h"
 #include "diff.h"
 
+/* Remember to update object flag allocation in object.h */
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
 #define TREESAME	(1u<<2)
diff --git a/sha1_name.c b/sha1_name.c
index 6fca869..2b6322f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -819,6 +819,8 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
  * For future extension, ':/!' is reserved. If you want to match a message
  * beginning with a '!', you have to repeat the exclamation mark.
  */
+
+/* Remember to update object flag allocation in object.h */
 #define ONELINE_SEEN (1u<<20)
 
 static int handle_one_ref(const char *path,
diff --git a/upload-pack.c b/upload-pack.c
index 9314c25..27df12f 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -17,7 +17,7 @@
 
 static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";
 
-/* bits #0..7 in revision.h, #8..10 in commit.c */
+/* Remember to update object flag allocation in object.h */
 #define THEY_HAVE	(1u << 11)
 #define OUR_REF		(1u << 12)
 #define WANTED		(1u << 13)
diff --git a/walker.c b/walker.c
index 633596e..1dd86b8 100644
--- a/walker.c
+++ b/walker.c
@@ -60,6 +60,7 @@ static int process_tree(struct walker *walker, struct tree *tree)
 	return 0;
 }
 
+/* Remember to update object flag allocation in object.h */
 #define COMPLETE	(1U << 0)
 #define SEEN		(1U << 1)
 #define TO_SCAN		(1U << 2)
-- 
1.9.0.201.g13d9d8b

[PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

[Nguyễn Thái Ngọc Duy]

Option explanation is in rev-list-options.txt. The interaction with -z
is left undecided.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 * Revert back to the old option name --show-linear-break
 * Get rid of saved_linear, use another flag in struct object instead
 * Fix not showing the break bar after a root commit if the dag graph
   has multiple roots
 * Make it work with --graph (although I don't really see the point of
   using both at the same time)
 * Let the next contributor deal with -z

 Documentation/rev-list-options.txt |  7 ++++++
 log-tree.c                         |  9 ++++++++
 object.h                           |  2 +-
 revision.c                         | 45 +++++++++++++++++++++++++++++++++++---
 revision.h                         |  9 +++++++-
 5 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 9a3da36..b813961 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -758,6 +758,13 @@ This enables parent rewriting, see 'History Simplification' below.
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
 
+--show-linear-break[=<barrier>]::
+	When --graph is not used, all history branches are flattened
+	which can make it hard to see that the two consecutive commits
+	do not belong to a linear branch. This option puts a barrier
+	in between them in that case. If `<barrier>` is specified, it
+	is the string that will be shown instead of the default one.
+
 ifdef::git-rev-list[]
 --count::
 	Print a number stating how many commits would have been
diff --git a/log-tree.c b/log-tree.c
index 5ce217d..c51a878 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -805,12 +805,21 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
+	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage) {
+		if (opt->graph)
+			graph_show_padding(opt->graph);
+		else
+			puts("\n");
+		printf("%s\n", opt->break_bar);
+	}
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
 		show_log(opt);
 		shown = 1;
 	}
+	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
 	return shown;
diff --git a/object.h b/object.h
index 768490b..9ee1959 100644
--- a/object.h
+++ b/object.h
@@ -28,7 +28,7 @@ struct object_array {
 #define TYPE_BITS   3
 /*
  * object flag allocation:
- * revision.h:      0------10
+ * revision.h:      0------10                 26
  * fetch-pack.c:    0---4
  * walker.c:        0-2
  * upload-pack.c:             11---------19
diff --git a/revision.c b/revision.c
index 78b5c3a..b9afdce 100644
--- a/revision.c
+++ b/revision.c
@@ -1831,6 +1831,18 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->notes_opt.use_default_notes = 1;
 	} else if (!strcmp(arg, "--show-signature")) {
 		revs->show_signature = 1;
+	} else if (!strcmp(arg, "--show-linear-break") ||
+		   starts_with(arg, "--show-linear-break=")) {
+		if (starts_with(arg, "--show-linear-break="))
+			revs->break_bar = xstrdup(arg + 20);
+		else
+			revs->break_bar = "                    ..........";
+		revs->track_linear = 1;
+		/*
+		 * make track_linear() not a break bar before the
+		 * first shown commit.
+		 */
+		commit_list_insert(NULL, &revs->previous_parents);
 	} else if (starts_with(arg, "--show-notes=") ||
 		   starts_with(arg, "--notes=")) {
 		struct strbuf buf = STRBUF_INIT;
@@ -2896,6 +2908,22 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
 	return action;
 }
 
+static void track_linear(struct rev_info *revs, struct commit *commit)
+{
+	struct commit_list *p;
+	for (p = revs->previous_parents; p; p = p->next)
+		if (p->item == NULL || /* first commit */
+		    !hashcmp(p->item->object.sha1, commit->object.sha1))
+			break;
+	revs->linear = p != NULL;
+	if (revs->reverse) {
+		if (revs->linear)
+			commit->object.flags |= TRACK_LINEAR;
+	}
+	free_commit_list(revs->previous_parents);
+	revs->previous_parents = copy_commit_list(commit->parents);
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
 	if (!revs->commits)
@@ -2935,6 +2963,8 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			die("Failed to simplify parents of commit %s",
 			    sha1_to_hex(commit->object.sha1));
 		default:
+			if (revs->track_linear)
+				track_linear(revs, commit);
 			return commit;
 		}
 	} while (revs->commits);
@@ -3101,14 +3131,23 @@ struct commit *get_revision(struct rev_info *revs)
 		revs->reverse_output_stage = 1;
 	}
 
-	if (revs->reverse_output_stage)
-		return pop_commit(&revs->commits);
+	if (revs->reverse_output_stage) {
+		c = pop_commit(&revs->commits);
+		if (revs->track_linear)
+			revs->linear = !!(c && c->object.flags & TRACK_LINEAR);
+		return c;
+	}
 
 	c = get_revision_internal(revs);
 	if (c && revs->graph)
 		graph_update(revs->graph, c);
-	if (!c)
+	if (!c) {
 		free_saved_parents(revs);
+		if (revs->previous_parents) {
+			free_commit_list(revs->previous_parents);
+			revs->previous_parents = NULL;
+		}
+	}
 	return c;
 }
 
diff --git a/revision.h b/revision.h
index 0262bbd..64503e9 100644
--- a/revision.h
+++ b/revision.h
@@ -19,7 +19,8 @@
 #define SYMMETRIC_LEFT	(1u<<8)
 #define PATCHSAME	(1u<<9)
 #define BOTTOM		(1u<<10)
-#define ALL_REV_FLAGS	((1u<<11)-1)
+#define TRACK_LINEAR	(1u<<26)
+#define ALL_REV_FLAGS	(((1u<<11)-1) | TRACK_LINEAR)
 
 #define DECORATE_SHORT_REFS	1
 #define DECORATE_FULL_REFS	2
@@ -138,6 +139,9 @@ struct rev_info {
 			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 	unsigned int	leak_pending:1;
+	/* --show-linear-break */
+	unsigned int	track_linear:1,
+			linear:1;
 
 	enum date_mode date_mode;
 
@@ -198,6 +202,9 @@ struct rev_info {
 
 	/* copies of the parent lists, for --full-diff display */
 	struct saved_parents *saved_parents_slab;
+
+	struct commit_list *previous_parents;
+	const char *break_bar;
 };
 
 extern int ref_excluded(struct string_list *, const char *path);
-- 
1.9.0.201.g13d9d8b

Re: [PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Option explanation is in rev-list-options.txt. The interaction with -z
> is left undecided.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Thanks.

>  * Revert back to the old option name --show-linear-break
>  * Get rid of saved_linear, use another flag in struct object instead

I cannot offhand say if I like this change or not.  A flag bit is a
scarce and limited resource; commit slabs felt more suited for
implementation of corner case eye-candies.

>  * Fix not showing the break bar after a root commit if the dag graph
>    has multiple roots

I definitely do not like the way a commit-list data structure is
abused to hold a phoney element that points at a NULL with its item
pointer.  Allocate a single bit in revs that says "I haven't done
anything yet" if you want to catch the "first-ness" without breaking
what commit_list_insert() and friends are expecting to see---they
never expect to see a NULL asked to be on the list, AFAIK.

>  * Make it work with --graph (although I don't really see the point of
>    using both at the same time)

I do not see the point, either.  I vaguely recall that the previous
iteration refused the combination at the option parser level, which
I think would be the right thing to do.

>  * Let the next contributor deal with -z

That is fine.

Re: [PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

[Duy Nguyen]

On Fri, Mar 21, 2014 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>  * Get rid of saved_linear, use another flag in struct object instead
>
> I cannot offhand say if I like this change or not.  A flag bit is a
> scarce and limited resource; commit slabs felt more suited for
> implementation of corner case eye-candies.

My thinking was like this:

OK an int for a flag is wasteful and Junio suggested that unsigned
char is used. But that still wastes 7 bits. So what if I rename it to
commit_flags and make it usable as a flag storage for other parts as
well? Wait don't we have some flags in struct object#flags. It turns
out we have _7_ flags left that nobody touches. Let's take one. If we
run out of flags in future, we can bring back commit_flags slab,
rearrange the flags and move rarely used ones to commit_flags.
-- 
Duy

[PATCH v4 1/2] object.h: centralize object flag allocation

[Nguyễn Thái Ngọc Duy]

While the field "flags" is mainly used by the revision walker, it is
also used in many other places. Centralize the whole flag allocation to
one place for a better overview (and easier to move flags if we have
too).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 bisect.c        |  3 +--
 builtin/blame.c |  2 +-
 bundle.c        |  1 +
 commit.c        |  2 +-
 fetch-pack.c    |  1 +
 http-push.c     |  3 +--
 object.h        | 13 +++++++++++++
 revision.h      |  1 +
 sha1_name.c     |  2 ++
 upload-pack.c   |  2 +-
 walker.c        |  1 +
 11 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/bisect.c b/bisect.c
index 8448d27..d6e851d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,8 +21,7 @@ static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
 static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
 
-/* bits #0-15 in revision.h */
-
+/* Remember to update object flag allocation in object.h */
 #define COUNTED		(1u<<16)
 
 /*
diff --git a/builtin/blame.c b/builtin/blame.c
index e5b5d71..88cb799 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -74,7 +74,7 @@ static unsigned blame_copy_score;
 #define BLAME_DEFAULT_MOVE_SCORE	20
 #define BLAME_DEFAULT_COPY_SCORE	40
 
-/* bits #0..7 in revision.h, #8..11 used for merge_bases() in commit.c */
+/* Remember to update object flag allocation in object.h */
 #define METAINFO_SHOWN		(1u<<12)
 #define MORE_THAN_ONE_PATH	(1u<<13)
 
diff --git a/bundle.c b/bundle.c
index a85e0e4..1222952 100644
--- a/bundle.c
+++ b/bundle.c
@@ -120,6 +120,7 @@ static int list_refs(struct ref_list *r, int argc, const char **argv)
 	return 0;
 }
 
+/* Remember to update object flag allocation in object.h */
 #define PREREQ_MARK (1u<<16)
 
 int verify_bundle(struct bundle_header *header, int verbose)
diff --git a/commit.c b/commit.c
index 0f28902..f479331 100644
--- a/commit.c
+++ b/commit.c
@@ -721,7 +721,7 @@ void sort_in_topological_order(struct commit_list **list, enum rev_sort_order so
 
 /* merge-base stuff */
 
-/* bits #0..15 in revision.h */
+/* Remember to update object flag allocation in object.h */
 #define PARENT1		(1u<<16)
 #define PARENT2		(1u<<17)
 #define STALE		(1u<<18)
diff --git a/fetch-pack.c b/fetch-pack.c
index 90d47da..eeee2bb 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -26,6 +26,7 @@ static int agent_supported;
 static struct lock_file shallow_lock;
 static const char *alternate_shallow_file;
 
+/* Remember to update object flag allocation in object.h */
 #define COMPLETE	(1U << 0)
 #define COMMON		(1U << 1)
 #define COMMON_REF	(1U << 2)
diff --git a/http-push.c b/http-push.c
index d4b40c9..f2c56c8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -64,8 +64,7 @@ enum XML_Status {
 #define LOCK_TIME 600
 #define LOCK_REFRESH 30
 
-/* bits #0-15 in revision.h */
-
+/* Remember to update object flag allocation in object.h */
 #define LOCAL    (1u<<16)
 #define REMOTE   (1u<<17)
 #define FETCHING (1u<<18)
diff --git a/object.h b/object.h
index 732bf4d..9918777 100644
--- a/object.h
+++ b/object.h
@@ -26,6 +26,19 @@ struct object_array {
 #define OBJECT_ARRAY_INIT { 0, 0, NULL }
 
 #define TYPE_BITS   3
+/*
+ * object flag allocation:
+ * revision.h:      0---------10
+ * fetch-pack.c:    0---4
+ * walker.c:        0-2
+ * upload-pack.c:               11----------------19
+ * builtin/blame.c:               12-13
+ * bisect.c:                               16
+ * bundle.c:                               16
+ * http-push.c:                            16-----19
+ * commit.c:                               16-----19
+ * sha1_name.c:                                     20
+ */
 #define FLAG_BITS  27
 
 /*
diff --git a/revision.h b/revision.h
index 1eb94c1..0262bbd 100644
--- a/revision.h
+++ b/revision.h
@@ -7,6 +7,7 @@
 #include "commit.h"
 #include "diff.h"
 
+/* Remember to update object flag allocation in object.h */
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
 #define TREESAME	(1u<<2)
diff --git a/sha1_name.c b/sha1_name.c
index 6fca869..2b6322f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -819,6 +819,8 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
  * For future extension, ':/!' is reserved. If you want to match a message
  * beginning with a '!', you have to repeat the exclamation mark.
  */
+
+/* Remember to update object flag allocation in object.h */
 #define ONELINE_SEEN (1u<<20)
 
 static int handle_one_ref(const char *path,
diff --git a/upload-pack.c b/upload-pack.c
index 286a9ed..01de944 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -17,7 +17,7 @@
 
 static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";
 
-/* bits #0..7 in revision.h, #8..10 in commit.c */
+/* Remember to update object flag allocation in object.h */
 #define THEY_HAVE	(1u << 11)
 #define OUR_REF		(1u << 12)
 #define WANTED		(1u << 13)
diff --git a/walker.c b/walker.c
index 633596e..1dd86b8 100644
--- a/walker.c
+++ b/walker.c
@@ -60,6 +60,7 @@ static int process_tree(struct walker *walker, struct tree *tree)
 	return 0;
 }
 
+/* Remember to update object flag allocation in object.h */
 #define COMPLETE	(1U << 0)
 #define SEEN		(1U << 1)
 #define TO_SCAN		(1U << 2)
-- 
1.9.1.345.ga1a145c

[PATCH v4 2/2] log: add --show-linear-break to help see non-linear history

[Nguyễn Thái Ngọc Duy]

Option explanation is in rev-list-options.txt. The interaction with -z
is left undecided.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Fri, Mar 21, 2014 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
 >>  * Get rid of saved_linear, use another flag in struct object instead
 >
 > I cannot offhand say if I like this change or not.  A flag bit is a
 > scarce and limited resource; commit slabs felt more suited for
 > implementation of corner case eye-candies.

 I leave it in bit 26. We can move it out when we run low on flag bits.

 >>  * Fix not showing the break bar after a root commit if the dag graph
 >>    has multiple roots
 >
 > I definitely do not like the way a commit-list data structure is
 > abused to hold a phoney element that points at a NULL with its item
 > pointer.  Allocate a single bit in revs that says "I haven't done
 > anything yet" if you want to catch the "first-ness" without breaking
 > what commit_list_insert() and friends are expecting to see---they
 > never expect to see a NULL asked to be on the list, AFAIK.

 Fixed.

 >>  * Make it work with --graph (although I don't really see the point of
 >>    using both at the same time)
 >
 > I do not see the point, either.  I vaguely recall that the previous
 > iteration refused the combination at the option parser level, which
 > I think would be the right thing to do.

 Fixed.

 Documentation/rev-list-options.txt |  7 ++++++
 log-tree.c                         |  4 ++++
 object.h                           |  2 +-
 revision.c                         | 48 +++++++++++++++++++++++++++++++++++---
 revision.h                         | 10 +++++++-
 5 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 9a3da36..b813961 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -758,6 +758,13 @@ This enables parent rewriting, see 'History Simplification' below.
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
 
+--show-linear-break[=<barrier>]::
+	When --graph is not used, all history branches are flattened
+	which can make it hard to see that the two consecutive commits
+	do not belong to a linear branch. This option puts a barrier
+	in between them in that case. If `<barrier>` is specified, it
+	is the string that will be shown instead of the default one.
+
 ifdef::git-rev-list[]
 --count::
 	Print a number stating how many commits would have been
diff --git a/log-tree.c b/log-tree.c
index 5ce217d..cf2f86c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -805,12 +805,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
+	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
 		show_log(opt);
 		shown = 1;
 	}
+	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
+		printf("\n%s\n", opt->break_bar);
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
 	return shown;
diff --git a/object.h b/object.h
index 9918777..6e12f2c 100644
--- a/object.h
+++ b/object.h
@@ -28,7 +28,7 @@ struct object_array {
 #define TYPE_BITS   3
 /*
  * object flag allocation:
- * revision.h:      0---------10
+ * revision.h:      0---------10                                26
  * fetch-pack.c:    0---4
  * walker.c:        0-2
  * upload-pack.c:               11----------------19
diff --git a/revision.c b/revision.c
index 78b5c3a..f834aa9 100644
--- a/revision.c
+++ b/revision.c
@@ -1831,6 +1831,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->notes_opt.use_default_notes = 1;
 	} else if (!strcmp(arg, "--show-signature")) {
 		revs->show_signature = 1;
+	} else if (!strcmp(arg, "--show-linear-break") ||
+		   starts_with(arg, "--show-linear-break=")) {
+		if (starts_with(arg, "--show-linear-break="))
+			revs->break_bar = xstrdup(arg + 20);
+		else
+			revs->break_bar = "                    ..........";
+		revs->track_linear = 1;
+		revs->track_first_time = 1;
 	} else if (starts_with(arg, "--show-notes=") ||
 		   starts_with(arg, "--notes=")) {
 		struct strbuf buf = STRBUF_INIT;
@@ -1954,6 +1962,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 			unkv[(*unkc)++] = arg;
 		return opts;
 	}
+	if (revs->graph && revs->track_linear)
+		die("--show-linear-break and --graph are incompatible");
 
 	return 1;
 }
@@ -2896,6 +2906,27 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
 	return action;
 }
 
+static void track_linear(struct rev_info *revs, struct commit *commit)
+{
+	if (revs->track_first_time) {
+		revs->linear = 1;
+		revs->track_first_time = 0;
+	} else {
+		struct commit_list *p;
+		for (p = revs->previous_parents; p; p = p->next)
+			if (p->item == NULL || /* first commit */
+			    !hashcmp(p->item->object.sha1, commit->object.sha1))
+				break;
+		revs->linear = p != NULL;
+	}
+	if (revs->reverse) {
+		if (revs->linear)
+			commit->object.flags |= TRACK_LINEAR;
+	}
+	free_commit_list(revs->previous_parents);
+	revs->previous_parents = copy_commit_list(commit->parents);
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
 	if (!revs->commits)
@@ -2935,6 +2966,8 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			die("Failed to simplify parents of commit %s",
 			    sha1_to_hex(commit->object.sha1));
 		default:
+			if (revs->track_linear)
+				track_linear(revs, commit);
 			return commit;
 		}
 	} while (revs->commits);
@@ -3101,14 +3134,23 @@ struct commit *get_revision(struct rev_info *revs)
 		revs->reverse_output_stage = 1;
 	}
 
-	if (revs->reverse_output_stage)
-		return pop_commit(&revs->commits);
+	if (revs->reverse_output_stage) {
+		c = pop_commit(&revs->commits);
+		if (revs->track_linear)
+			revs->linear = !!(c && c->object.flags & TRACK_LINEAR);
+		return c;
+	}
 
 	c = get_revision_internal(revs);
 	if (c && revs->graph)
 		graph_update(revs->graph, c);
-	if (!c)
+	if (!c) {
 		free_saved_parents(revs);
+		if (revs->previous_parents) {
+			free_commit_list(revs->previous_parents);
+			revs->previous_parents = NULL;
+		}
+	}
 	return c;
 }
 
diff --git a/revision.h b/revision.h
index 0262bbd..d9907dd 100644
--- a/revision.h
+++ b/revision.h
@@ -19,7 +19,8 @@
 #define SYMMETRIC_LEFT	(1u<<8)
 #define PATCHSAME	(1u<<9)
 #define BOTTOM		(1u<<10)
-#define ALL_REV_FLAGS	((1u<<11)-1)
+#define TRACK_LINEAR	(1u<<26)
+#define ALL_REV_FLAGS	(((1u<<11)-1) | TRACK_LINEAR)
 
 #define DECORATE_SHORT_REFS	1
 #define DECORATE_FULL_REFS	2
@@ -138,6 +139,10 @@ struct rev_info {
 			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 	unsigned int	leak_pending:1;
+	/* --show-linear-break */
+	unsigned int	track_linear:1,
+			track_first_time:1,
+			linear:1;
 
 	enum date_mode date_mode;
 
@@ -198,6 +203,9 @@ struct rev_info {
 
 	/* copies of the parent lists, for --full-diff display */
 	struct saved_parents *saved_parents_slab;
+
+	struct commit_list *previous_parents;
+	const char *break_bar;
 };
 
 extern int ref_excluded(struct string_list *, const char *path);
-- 
1.9.1.345.ga1a145c

Re: [PATCH v4 2/2] log: add --show-linear-break to help see non-linear history

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Option explanation is in rev-list-options.txt. The interaction with -z
> is left undecided.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  On Fri, Mar 21, 2014 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>  >>  * Get rid of saved_linear, use another flag in struct object instead
>  >
>  > I cannot offhand say if I like this change or not.  A flag bit is a
>  > scarce and limited resource; commit slabs felt more suited for
>  > implementation of corner case eye-candies.
>
>  I leave it in bit 26. We can move it out when we run low on flag bits.
>
>  >>  * Fix not showing the break bar after a root commit if the dag graph
>  >>    has multiple roots
>  >
>  > I definitely do not like the way a commit-list data structure is
>  > abused to hold a phoney element that points at a NULL with its item
>  > pointer.  Allocate a single bit in revs that says "I haven't done
>  > anything yet" if you want to catch the "first-ness" without breaking
>  > what commit_list_insert() and friends are expecting to see---they
>  > never expect to see a NULL asked to be on the list, AFAIK.
>
>  Fixed.
>
>  >>  * Make it work with --graph (although I don't really see the point of
>  >>    using both at the same time)
>  >
>  > I do not see the point, either.  I vaguely recall that the previous
>  > iteration refused the combination at the option parser level, which
>  > I think would be the right thing to do.
>
>  Fixed.

All changes look good to me.

Especially on the last one because the new "printf()" calls do not
even attempt to call into the graph API to tell it that it created
a gutter above or give it a chance to draw all vertical lines to
connect the graph part.

Will replace and advance them to "Will merge to 'next'" state.

Thanks.