git notes: notes

git notes: notes

[Joey Hess]

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

Just a quick note that the new notes feature can break things that parse
git log. For example a parser that assumes it can split the log on blank
lines to separate the header and commit message, can easily become
confused by the new blank line before "Notes:".

Might be worth documenting in release notes, maybe too late now though.
But really, it's all good, notes are a great feature.

PS, Has anyone thought about using notes to warn bisect away from
commits that are known to be unbuildable or otherwise cause bisection
trouble?

-- 
see shy jo

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

Re: git notes: notes

[Thomas Rast]

Joey Hess wrote:
> Just a quick note that the new notes feature can break things that parse
> git log.

Umm.  git-log is porcelain and we're allowed to change it.  Worse,
even the user can change it in very significant ways, just try:

  git config format.pretty email
  git log

For a better alternative, I'm afraid you'll either have to look to
git-rev-list (which also takes --pretty) or 'git cat-file --batch'.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: git notes: notes

[Joey Hess]

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

Thomas Rast wrote:
> Umm.  git-log is porcelain and we're allowed to change it.  Worse,
> even the user can change it in very significant ways, just try:
> 
>   git config format.pretty email
>   git log

Is git log --pretty=raw --raw really intended to be porcelain?
Above does not affect it.

> For a better alternative, I'm afraid you'll either have to look to
> git-rev-list (which also takes --pretty) or 'git cat-file --batch'.

I don't see a way to get the per-commit diff-tree info using rev-list.

-- 
see shy jo

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

Re: git notes: notes

[Johan Herland]

On Wednesday 20 January 2010, Joey Hess wrote:
> Just a quick note that the new notes feature can break things that parse
> git log. For example a parser that assumes it can split the log on blank
> lines to separate the header and commit message, can easily become
> confused by the new blank line before "Notes:".

As Thomas already stated, git log is porcelain, and its output format is not 
set in stone. If you need a stable, script-friendly format, you should 
probably use the --format option, or use plumbing instead (such as e.g. git 
rev-list, which also has a --format option).

> Might be worth documenting in release notes, maybe too late now though.
> But really, it's all good, notes are a great feature.
> 
> PS, Has anyone thought about using notes to warn bisect away from
> commits that are known to be unbuildable or otherwise cause bisection
> trouble?

No, I haven't thought of that specific use case. Great idea! :)

BTW, since I started talking about git notes, people on this list have found 
more and more interesting use cases for them:

- Free-form text extension to the commit message

- Help in bug tracking with header-like lines such as:
    - Causes-Bug: #12345
    - Fixes-Bug: #54321

- Store after-the-fact "Acked-By", "Reviewed-By", etc. annotations

- In a repo converted from a merge-unfriendly VCS (such as CVS), use notes
  to identify merges without having to rewrite Git history (note that you
  can also use grafts, or "git replace" to accomplish this).

- Refer to related commits elsewhere in the repo (i.e. relationships that
  are not already apparent from the commit graph)

- When cherry-picking, add a reverse link from the source commit to the
  cherry-picked commit (since it may be of interest to people reviewing the
  source commit

- Rebasing public branches is forbidden, but if you wanted to change that,
  you could potentially help solve it by using notes to add reverse links
  from source commits to rebased commits, so that downstream people could
  more easily traverse your history when rebasing/merging their own
  branches.

- Initially, there were some discussion whether it could also be used to
  guide git blame to make better decisions, although I don't currently see
  how that would be done in practice.

In any case, it seems the notes idea may have the potential to become one of 
the more useful features in Git.


Have fun! :)

...Johan


[1]: ...almost 3 years ago (wow, time flies...):
     http://article.gmane.org/gmane.comp.version-control.git/46883

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git notes: notes

[Joey Hess]

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

Johan Herland wrote:
> As Thomas already stated, git log is porcelain, and its output format is not 
> set in stone. If you need a stable, script-friendly format, you should 
> probably use the --format option, or use plumbing instead (such as e.g. git 
> rev-list, which also has a --format option).

But git log --format=raw --raw output was changed by notes.

> > Might be worth documenting in release notes, maybe too late now though.
> > But really, it's all good, notes are a great feature.
> > 
> > PS, Has anyone thought about using notes to warn bisect away from
> > commits that are known to be unbuildable or otherwise cause bisection
> > trouble?
> 
> No, I haven't thought of that specific use case. Great idea! :)

Only problem I see with doing it is it might be too easy to overwrite
such a note with git notes edit -m

Did you consider having -m append a line to an existing note?

-- 
see shy jo

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

Re: git notes: notes

[Junio C Hamano]

Joey Hess <joey@kitenet.net> writes:

> Johan Herland wrote:
>> As Thomas already stated, git log is porcelain, and its output format is not 
>> set in stone. If you need a stable, script-friendly format, you should 
>> probably use the --format option, or use plumbing instead (such as e.g. git 
>> rev-list, which also has a --format option).
>
> But git log --format=raw --raw output was changed by notes.

Let me ask a stupid question.  Did the output change before and after the
notes code even when your history does not have notes?

>> > Might be worth documenting in release notes, maybe too late now though.

This depends on the answer to the above question.  If the answers is "No",
then I don't see the need to say much more than "New 'git notes' feature
allows comments applied to existing commits after the fact to be shown by
log and friends".  If it is "Yes", we should fix the code not to change
the output.

In any case, "log" is still a Porcelain, so it is understandable that by
triggering a new feature you would get output from the new feature.  It is
called progress ;-)

Re: git notes: notes

[Joey Hess]

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

Junio C Hamano wrote:
> Let me ask a stupid question.  Did the output change before and after the
> notes code even when your history does not have notes?

No. 

> >> > Might be worth documenting in release notes, maybe too late now though.
> 
> This depends on the answer to the above question.  If the answers is "No",
> then I don't see the need to say much more than "New 'git notes' feature
> allows comments applied to existing commits after the fact to be shown by
> log and friends".  If it is "Yes", we should fix the code not to change
> the output.
> 
> In any case, "log" is still a Porcelain, so it is understandable that by
> triggering a new feature you would get output from the new feature.  It is
> called progress ;-)

Do you think it makes sense for even git log --format=format:%s to be
porcelain and potentially change when new features are used? Seems to
me that parts of git log walk the line between porcelain and plumbing.
So it's not clear which parts are safe to use.

-- 
see shy jo

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

Re: git notes: notes

[Junio C Hamano]

Joey Hess <joey@kitenet.net> writes:

> Do you think it makes sense for even git log --format=format:%s to be
> porcelain and potentially change when new features are used?

If the series changed the meaning of "%s" format to mean "the subject of
the commit and notes information", with or without documenting it, then it
is just a bug we would like to fix.

But I cannot reproduce such a bug.  In my tree locally:

    $ git notes show a97a74
    Origin of commit notes feature was at
    a97a74).
    $ git show -s --pretty=short a97a74
    commit a97a74686d70a318cd802003498054cc1e8b0ae2
    Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>

        Introduce commit notes

    Notes:
        Origin of commit notes feature was at
        a97a74).
    $ git show -s --pretty=format:%s a97a74
    Introduce commit notes

Puzzled...

Re: git notes: notes

[Joey Hess]

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

Junio C Hamano wrote:
> Joey Hess <joey@kitenet.net> writes:
> 
> > Do you think it makes sense for even git log --format=format:%s to be
> > porcelain and potentially change when new features are used?
> 
> If the series changed the meaning of "%s" format to mean "the subject of
> the commit and notes information", with or without documenting it, then it
> is just a bug we would like to fix.
> 
> But I cannot reproduce such a bug.  In my tree locally:

I was asking hypothetically, trying to point out that parts of git log
seem to make sense to be used as plumbing, with the hope I can continue
to use it that way.

(Note that git instaweb parses output of git log --pretty=format:%H --raw
like it's plumbing.)

-- 
see shy jo

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

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 03:36:36PM -0500, Joey Hess wrote:

> I was asking hypothetically, trying to point out that parts of git log
> seem to make sense to be used as plumbing, with the hope I can continue
> to use it that way.
> 
> (Note that git instaweb parses output of git log --pretty=format:%H --raw
> like it's plumbing.)

I think this is a valid point. Note that "gitk" uses "git log
--pretty=raw". However, I believe it splits the entries on "^commit". So
I think there is some precedent for scripting "git log"; it has features
that are simply not available through other interfaces. And scripting
around "--pretty=raw" seems pretty reasonable to me, too. Why else would
you want the raw format?

Is splitting on blank lines an error? I don't think so. The original
format was never strictly defined, but given the --pretty=raw format, it
seems like a fairly obvious thing to do.

I am inclined to cut the notes output from --pretty=raw, and let callers
ask for them explicitly with --show-notes or something similar. We can
leave them on by default in the "normal" output. This will still break
scripts doing "git log | ./script", but I don't think we have ever
condoned that practice.

-Peff

Re: git notes: notes

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Is splitting on blank lines an error? I don't think so. The original
> format was never strictly defined, but given the --pretty=raw format, it
> seems like a fairly obvious thing to do.
>
> I am inclined to cut the notes output from --pretty=raw, and let callers
> ask for them explicitly with --show-notes or something similar. We can
> leave them on by default in the "normal" output. This will still break
> scripts doing "git log | ./script", but I don't think we have ever
> condoned that practice.

Sounds like a plan.

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 12:59:38PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Is splitting on blank lines an error? I don't think so. The original
> > format was never strictly defined, but given the --pretty=raw format, it
> > seems like a fairly obvious thing to do.
> >
> > I am inclined to cut the notes output from --pretty=raw, and let callers
> > ask for them explicitly with --show-notes or something similar. We can
> > leave them on by default in the "normal" output. This will still break
> > scripts doing "git log | ./script", but I don't think we have ever
> > condoned that practice.
> 
> Sounds like a plan.

We can start with this patch, which clears up Joey's problem.

-- >8 --
Subject: [PATCH] don't show notes for --pretty=raw

The --pretty=raw format of the log family is likely to be
used by scripts. Such scripts may parse the output into
records on blank lines, since doing so in the past has
always worked. However, with the recently added notes
output, such parsers will see an extra stanza for any
commits that have notes.

This patch turns off the notes output for the raw format to
avoid breaking such scripts.

Signed-off-by: Jeff King <peff@peff.net>
---
 pretty.c         |    2 +-
 t/t3301-notes.sh |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/pretty.c b/pretty.c
index 9001379..0674027 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,7 +1094,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 
-	if (fmt != CMIT_FMT_ONELINE)
+	if (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)
 		get_commit_notes(commit, sb, encoding,
 				 NOTES_SHOW_HEADER | NOTES_INDENT);
 
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 1e34f48..4c3de9d 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -147,4 +147,18 @@ test_expect_success 'show -m and -F notes' '
 	test_cmp expect-m-and-F output
 '
 
+cat >expect << EOF
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+tree e070e3af51011e47b183c33adf9736736a525709
+parent 1584215f1d29c65e99c6c6848626553fdd07fd75
+author A U Thor <author@example.com> 1112912173 -0700
+committer C O Mitter <committer@example.com> 1112912173 -0700
+
+    4th
+EOF
+test_expect_success 'git log --pretty=raw does not show notes' '
+	git log -1 --pretty=raw >output &&
+	test_cmp expect output
+'
+
 test_done
-- 
1.6.6.510.g159cf

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 04:31:38PM -0500, Jeff King wrote:

> We can start with this patch, which clears up Joey's problem.
> 
> -- >8 --
> Subject: [PATCH] don't show notes for --pretty=raw
> 
> The --pretty=raw format of the log family is likely to be
> used by scripts. Such scripts may parse the output into
> records on blank lines, since doing so in the past has
> always worked. However, with the recently added notes
> output, such parsers will see an extra stanza for any
> commits that have notes.
> 
> This patch turns off the notes output for the raw format to
> avoid breaking such scripts.

And the second half would be something like the patch below implementing
--show-notes, so that things like "gitk" which can handle the new
feature can turn it on.

I'm not that happy with this patch, though. If we have --show-notes, we
should probably have --no-show-notes, which this doesn't do, since a
lack of --show-notes simply means "do whatever the format dictates".

Also, passing everything through a pretty_print_context seems kind of
hack-ish, as we just end up copying items from the rev-list context into
the pp_context. I did so here only in the log_tree_commit case. I don't
think there are other places which would want to propagate it, but I
might have missed one. I wonder if we would do better to simply past the
rev-list options into pretty_print_commit and let it pick what it wants
out of the struct.

Anyway, I am out of time to work on git for now, so maybe somebody who
cares more about notes can pick this up. I think the first patch (the
one I am replying to) should definitely go in to un-break Joey's case
(or an alternative patch that turns it off in even more cases), and
people who care about it can fight about whether and by what mechanism
things like gitk should see the notes.

-- >8 --
Subject: [PATCH] teach log family to --show-notes

Most log formats will implicitly show commit notes, but the
raw format will not. Callers can now explicitly call
--show-notes to enable them.

Signed-off-by: Jeff King <peff@peff.net>
---
 commit.h         |    1 +
 log-tree.c       |    1 +
 pretty.c         |    3 ++-
 revision.c       |    2 ++
 revision.h       |    1 +
 t/t3301-notes.sh |   16 ++++++++++++++++
 6 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/commit.h b/commit.h
index 24128d7..4646751 100644
--- a/commit.h
+++ b/commit.h
@@ -71,6 +71,7 @@ struct pretty_print_context
 	enum date_mode date_mode;
 	int need_8bit_cte;
 	struct reflog_walk_info *reflog_info;
+	int show_notes;
 };
 
 extern int has_non_ascii(const char *text);
diff --git a/log-tree.c b/log-tree.c
index 0fdf159..9155a31 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -412,6 +412,7 @@ void show_log(struct rev_info *opt)
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
 	ctx.reflog_info = opt->reflog_info;
+	ctx.show_notes = opt->show_notes;
 	pretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);
 
 	if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 0674027..95fe39a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,7 +1094,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 
-	if (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)
+	if (context->show_notes ||
+	    (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))
 		get_commit_notes(commit, sb, encoding,
 				 NOTES_SHOW_HEADER | NOTES_INDENT);
 
diff --git a/revision.c b/revision.c
index 7328201..3815dd3 100644
--- a/revision.c
+++ b/revision.c
@@ -1175,6 +1175,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		get_commit_format("oneline", revs);
 		revs->abbrev_commit = 1;
+	} else if (!strcmp(arg, "--show-notes")) {
+		revs->show_notes = 1;
 	} else if (!strcmp(arg, "--graph")) {
 		revs->topo_order = 1;
 		revs->rewrite_parents = 1;
diff --git a/revision.h b/revision.h
index d368003..e51842f 100644
--- a/revision.h
+++ b/revision.h
@@ -83,6 +83,7 @@ struct rev_info {
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
+			show_notes:1,
 			date_mode_explicit:1;
 	unsigned int	disable_stdin:1;
 
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 4c3de9d..d51e8bd 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -161,4 +161,20 @@ test_expect_success 'git log --pretty=raw does not show notes' '
 	test_cmp expect output
 '
 
+cat >>expect <<EOF
+
+Notes:
+    spam
+$whitespace
+    xyzzy
+$whitespace
+    foo
+    bar
+    baz
+EOF
+test_expect_success 'git log --show-notes' '
+	git log -1 --pretty=raw --show-notes >output &&
+	test_cmp expect output
+'
+
 test_done
-- 
1.6.6.510.g159cf

Re: git notes: notes

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> diff --git a/pretty.c b/pretty.c
> index 0674027..95fe39a 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1094,7 +1094,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
>  	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
>  		strbuf_addch(sb, '\n');
>  
> -	if (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)
> +	if (context->show_notes ||
> +	    (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))
>  		get_commit_notes(commit, sb, encoding,
>  				 NOTES_SHOW_HEADER | NOTES_INDENT);

Heh, without this hunk I would have thought Peff and Gitster were the same
person ;-).

Once you introduce --no-notes, the above condition would not work well.

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 02:07:47PM -0800, Junio C Hamano wrote:

> > -	if (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW)
> > +	if (context->show_notes ||
> > +	    (fmt != CMIT_FMT_ONELINE && fmt != CMIT_FMT_RAW))
> >  		get_commit_notes(commit, sb, encoding,
> >  				 NOTES_SHOW_HEADER | NOTES_INDENT);
> 
> Heh, without this hunk I would have thought Peff and Gitster were the same
> person ;-).
> 
> Once you introduce --no-notes, the above condition would not work well.

Yeah, I know, or I would have just added the 2 lines for
--no-show-notes. :) I think your patch is better; I'll comment on it
separately.

-Peff

Re: git notes: notes

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:
> Joey Hess <joey@kitenet.net> writes:
> 
>> Do you think it makes sense for even git log --format=format:%s to be
>> porcelain and potentially change when new features are used?
> 
> If the series changed the meaning of "%s" format to mean "the subject of
> the commit and notes information", with or without documenting it, then it
> is just a bug we would like to fix.

No, but outputting the note as part of the log is the standard. So for
example, when you do a format-patch | apply cycle, format-patch will
insert the note as part of the commit message, and apply will *store*
the note text (including Note:\n) as part of the commit message of the
new commit.

So, I would say the notes feature is not that well integrated right now,
and either log has to learn --no-notes (and format-patch has to use it,
or rather the corresponding internal flag), or apply has to learn to
parse "Note:" headers. Or, depending on how you use notes, it may be
better if format-patch puts the note after the "--"; that way you can
store the usual "after-the-message" patch comments in a note.

Similarly, I don't think rebasing and cherry-picking adjust the notes
tree for commits with notes whose sha1 changes - which may or may not be
the appropriate behaviour.

In both cases, the "right" way depends on how you use notes, and there
should be an easy way to specify your choice.

I'm not complaining, I actually have this on a maybe-to-do list, but the
way the series went kept me from investing time.

Cheers,
Michael

Re: git notes: notes

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:
>> Joey Hess <joey@kitenet.net> writes:
>> 
>>> Do you think it makes sense for even git log --format=format:%s to be
>>> porcelain and potentially change when new features are used?
>> 
>> If the series changed the meaning of "%s" format to mean "the subject of
>> the commit and notes information", with or without documenting it, then it
>> is just a bug we would like to fix.
>
> No, but outputting the note as part of the log is the standard. So for
> example, when you do a format-patch | apply cycle, format-patch will
> insert the note as part of the commit message, and apply will *store*
> the note text (including Note:\n) as part of the commit message of the
> new commit.

Thanks; that was the kind of breakage report I was looking for (and wished
to have heard a lot earlier).  Personally I find it is unexcusable that
format-patch defaults to giving notes.

> So, I would say the notes feature is not that well integrated right now,

No question about it.

> I'm not complaining, I actually have this on a maybe-to-do list, but the
> way the series went kept me from investing time.

Hmm, that hints there is a failure in the review and merge process.  Care
to explain how we could have done better please?

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 01:08:07PM -0800, Junio C Hamano wrote:

> > No, but outputting the note as part of the log is the standard. So for
> > example, when you do a format-patch | apply cycle, format-patch will
> > insert the note as part of the commit message, and apply will *store*
> > the note text (including Note:\n) as part of the commit message of the
> > new commit.
> 
> Thanks; that was the kind of breakage report I was looking for (and wished
> to have heard a lot earlier).  Personally I find it is unexcusable that
> format-patch defaults to giving notes.

I agree. I noticed this while doing the "don't show in raw" feature
elsewhere in the thread and wanted to ask: which formats _should_ have
notes by default?

To be honest, I am not sure _any_ format should have it by default. If I
am running "git log" and my notes are filled with random automatically
generated bisection cruft, I don't want to see that cluttering my
output. Yes, all of our test notes are human-written annotations, but I
think we really don't know yet what sorts of things people will be
putting in them.

Long ago I proposed a set of notes namespaces to deal with this (so
automatic bisection cruft would go into its own notes namespace, and
human-readable ones would be in some default namespace), but I don't
know how much of that idea (if any) survived into the current
implementation.

> > I'm not complaining, I actually have this on a maybe-to-do list, but the
> > way the series went kept me from investing time.
> 
> Hmm, that hints there is a failure in the review and merge process.  Care
> to explain how we could have done better please?

Personally, I stopped paying attention simply because it was gigantic
and I am not all that interested in using the feature personally.

-Peff

Re: git notes: notes

[Junio C Hamano]

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

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> No, but outputting the note as part of the log is the standard. So for
>> example, when you do a format-patch | apply cycle, format-patch will
>> insert the note as part of the commit message, and apply will *store*
>> the note text (including Note:\n) as part of the commit message of the
>> new commit.
>
> Thanks; that was the kind of breakage report I was looking for (and wished
> to have heard a lot earlier).  Personally I find it is unexcusable that
> format-patch defaults to giving notes.
>
>> So, I would say the notes feature is not that well integrated right now,
>
> No question about it.

How about solving it this way?

It _could_ break some tests, if the set of tests were carefully written to
cover not only the positive ("I am showing off my shiny new toy") cases
but also the negative ("These commands share the same codepath touched by
the series, but I don't intend to change their behaviour, and here is to
make sure the new toy does not affect them") cases and the latter set
assumed it is ok to sprinkle notes in commit log messages without being
asked, but I haven't tried running the test suite yet.

---
Subject: Fix "log" family not to be too agressive about showing notes

Giving "Notes" information in the default output format of "log" and
"show" is a sensible progress (the user has asked for it by having the
notes), but for some commands (e.g. "format-patch") spewing notes into the
formatted commit log message without being asked is too aggressive.

Enable notes output only for "log", "show", "whatchanged" by default;
other users can ask for it by setting show_notes field to true.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-log.c |    2 ++
 commit.h      |    1 +
 log-tree.c    |    1 +
 pretty.c      |    2 +-
 revision.c    |    4 ++++
 revision.h    |    1 +
 6 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 41b6df4..da0ba1d 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -41,6 +41,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
+	else
+		rev->show_notes = 1;
 	rev->verbose_header = 1;
 	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
diff --git a/commit.h b/commit.h
index e5332ef..2c0742b 100644
--- a/commit.h
+++ b/commit.h
@@ -70,6 +70,7 @@ struct pretty_print_context
 	const char *after_subject;
 	enum date_mode date_mode;
 	int need_8bit_cte;
+	int show_notes;
 	struct reflog_walk_info *reflog_info;
 };
 
diff --git a/log-tree.c b/log-tree.c
index 0fdf159..27afcf6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -284,6 +284,7 @@ void show_log(struct rev_info *opt)
 	struct pretty_print_context ctx = {0};
 
 	opt->loginfo = NULL;
+	ctx.show_notes = opt->show_notes;
 	if (!opt->verbose_header) {
 		graph_show_commit(opt->graph);
 
diff --git a/pretty.c b/pretty.c
index 8f5bd1a..b2ee7fe 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,7 +1094,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 
-	if (fmt != CMIT_FMT_ONELINE)
+	if (context->show_notes)
 		get_commit_notes(commit, sb, encoding,
 				 NOTES_SHOW_HEADER | NOTES_INDENT);
 
diff --git a/revision.c b/revision.c
index 25fa14d..03c280f 100644
--- a/revision.c
+++ b/revision.c
@@ -1165,6 +1165,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!prefixcmp(arg, "--pretty=") || !prefixcmp(arg, "--format=")) {
 		revs->verbose_header = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--show-notes")) {
+		revs->show_notes = 1;
+	} else if (!strcmp(arg, "--no-notes")) {
+		revs->show_notes = 0;
 	} else if (!strcmp(arg, "--oneline")) {
 		revs->verbose_header = 1;
 		get_commit_format("oneline", revs);
diff --git a/revision.h b/revision.h
index d368003..4167c1e 100644
--- a/revision.h
+++ b/revision.h
@@ -80,6 +80,7 @@ struct rev_info {
 	/* Format info */
 	unsigned int	shown_one:1,
 			show_merge:1,
+			show_notes:1,
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 01:59:36PM -0800, Junio C Hamano wrote:

> Subject: Fix "log" family not to be too agressive about showing notes
> 
> Giving "Notes" information in the default output format of "log" and
> "show" is a sensible progress (the user has asked for it by having the
> notes), but for some commands (e.g. "format-patch") spewing notes into the
> formatted commit log message without being asked is too aggressive.
> 
> Enable notes output only for "log", "show", "whatchanged" by default;
> other users can ask for it by setting show_notes field to true.

What I didn't get out of reading this but did from reading the code (I
think) is what you meant by "by default" here. That is, doing:

  git log

will show notes, but neither

  git log --pretty=raw

nor even

  git log --pretty=medium

will do so, even though the latter otherwise produces identical output
to the default.

That seems like a reasonable rule to me, but I just wanted to make sure
that was both what was happening and what was intended.

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin-log.c |    2 ++
>  commit.h      |    1 +
>  log-tree.c    |    1 +
>  pretty.c      |    2 +-
>  revision.c    |    4 ++++
>  revision.h    |    1 +
>  6 files changed, 10 insertions(+), 1 deletions(-)

No tests or docs, of course. :) You can squash the --pretty=raw test
from my patch, but you will need to exercise --show-notes and
--no-show-notes, too, as well as checking other formats and things like
format-patch. So probably writing your own tests will make it easier to
more thoroughly check each case.

-Peff

Re: git notes: notes

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> No tests or docs, of course. :) You can squash the --pretty=raw test
> from my patch, but you will need to exercise --show-notes and
> --no-show-notes, too, as well as checking other formats and things like
> format-patch. So probably writing your own tests will make it easier to
> more thoroughly check each case.

Thanks, but Ugh.

Didn't I say elsewhere that I am too busy to become a janitor for
everybody's itch, especially for topics that are merely "Meh" to me?

If there is no volunteer, I might be forced to do something about it, but
no promises, and I am reasonably certain that not much will happen at my
end for coming 48 hours, as I am cutting 1.6.6.1 (and perhaps 1.6.5.8) and
looking at other topics in 'next' that deserve to go to 1.7.0-rc0.

Re: git notes: notes

[Johannes Schindelin]

Hi,

On Wed, 20 Jan 2010, Junio C Hamano wrote:

> Subject: Fix "log" family not to be too agressive about showing notes
> 
> Giving "Notes" information in the default output format of "log" and
> "show" is a sensible progress (the user has asked for it by having the
> notes), but for some commands (e.g. "format-patch") spewing notes into the
> formatted commit log message without being asked is too aggressive.
> 
> Enable notes output only for "log", "show", "whatchanged" by default;
> other users can ask for it by setting show_notes field to true.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---

Makes sense, and the patch actually removes what could be seen as an ugly 
side effect (why it only ONELINE not getting notes?).

I would agree with Peff about the mention of --pretty disabling notes 
(unless asked for by a user format) in the commit notes as well as in the 
pretty options, but I fully disagree on the need for tests.  We should not 
have a thorough test suite that runs for days on end, but we should 
concentrate on things that are more likely to get broken.  And the added 
code is just too obvious for that.

(Anybody remember the initial suggestion for testing git-commit before 
making it builtin?  It had something like 70 tests.)

Ciao,
Dscho

Re: git notes: notes

[Jeff King]

On Wed, Jan 20, 2010 at 11:58:07PM +0100, Johannes Schindelin wrote:

> I would agree with Peff about the mention of --pretty disabling notes 
> (unless asked for by a user format) in the commit notes as well as in the 
> pretty options, but I fully disagree on the need for tests.  We should not 
> have a thorough test suite that runs for days on end, but we should 
> concentrate on things that are more likely to get broken.  And the added 
> code is just too obvious for that.

Sure, we don't have to go all out. But I think there is some confusion
right now about just what behavior we _should_ have, so I think
documenting it in the form of tests is reasonable.

-Peff

Re: git notes: notes

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Makes sense, and the patch actually removes what could be seen as an ugly 
> side effect (why it only ONELINE not getting notes?).

Thanks.

The motivation was that the user should be able to get notes even under
ONELINE mode if desired.  But then the call to get_commit_notes() may want
to inspect the commit format being used and tweak the flag parameter; right
now it always sends NOTES_SHOW_HEADER and NOTES_INDENT.

> I would agree with Peff about the mention of --pretty disabling notes 
> (unless asked for by a user format) in the commit notes as well as in the 
> pretty options,...

Actually I am of two minds regarding --pretty={short,medium} and the
like.  The "how about this" patch may be the safest for people who are
used to read "log --pretty=xxx" output with scripts, but it does look
inconsistent and hard to explain to new people who do not even know that
there were versions of git that does not know about notes.

> but I fully disagree on the need for tests.  We should not 
> have a thorough test suite that runs for days on end, but we should 
> concentrate on things that are more likely to get broken.  And the added 
> code is just too obvious for that.

I agree with that principle, but it doesn't explain nor justify the lack
of tests for format-patch, which would have caught the breakage a lot
earlier.

Or perhaps we all (not just you but I am just as guilty) misjudged "things
that are more likely to get broken", even though we are very well aware
that touching log-tree infrastructure will have fallout all over the "log"
family.

Re: git notes: notes

[Johan Herland]

On Thursday 21 January 2010, Junio C Hamano wrote:
> [...]

I just want to note that I've read the whole thread (up to here), and I 
agree with pretty much everything that's been said so far:

- We should be more conservative about showing notes, especially in contexts 
that may be used by scripts. Disabling notes by default when --pretty/--
format is in use, sounds like a good idea. So does adding a --show-notes 
option for overriding the default.

- The format-patch bug is grave and unexcusable and must be fixed. Michael: 
Thanks for discovering.

- I'd still like to keep notes as part of the default output from git log 
and friends (when NOT using --pretty/--format). Only notes from a single 
notes ref (typically the default "refs/notes/commits") should be shown.

- Re. Peff's worry that "git log" will fill up with random bisection cruft: 
Any notes that are related to bisection (or any other special use case for 
notes) should live on its own notes ref (typically "refs/notes/bisect" for 
bisection cruft) that is not used by "git log" (unless you explicitly say so 
through $GIT_NOTES_REF or core.notesRef).

- Re. Junio's worry that he will become the janitor for these patches. 
Please don't. As long as the patch series is in 'pu', it is MY 
responsibility to address issues and organize any additional patches on top 
of the series. Feel free to ignore all additional patches, and wait for an 
updated series from my end.

- Yes, there should be more tests verifying that there is no negative impact 
on git log and friends. Docs must be updated as well, where needed.


Unfortunately I don't have the time to work on this right now, but I'll do 
my best to get around to it as soon as possible (at least by the end of the 
coming weekend).


Again, thanks for your involvement. It is really appreciated.

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git notes: notes

[Junio C Hamano]

Johan Herland <johan@herland.net> writes:

> - Re. Junio's worry that he will become the janitor for these patches. 
>
> Please don't. As long as the patch series is in 'pu', it is MY 
> responsibility ...

I am not worried so much about what is not in 'next' yet.  My worry right
now is primarily about what to do with upcoming 1.7.0 and also the 1.6.6.X
series, which already shipped with "format-patch" that injects notes in
its output.

Re: git notes: notes

[Junio C Hamano]

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

> Actually I am of two minds regarding --pretty={short,medium} and the
> like.  The "how about this" patch may be the safest for people who are
> used to read "log --pretty=xxx" output with scripts, but it does look
> inconsistent and hard to explain to new people who do not even know that
> there were versions of git that does not know about notes.

Heh, it is not surprising that we have this bug, given that all of us just
missed how bogus my "how about this" patch was ;-)

The check for "if (fmt_pretty)" only kicks in when that thing was read
from the configuration; handling of command line --pretty and --pretty=
options happen long after that, when we call setup_revisions().

So if we really wanted to say "If the user explicitly tells us to run
under a particular --pretty mode, we don't show notes by default.", we
would need a patch like this on top of it.

Another thing to note is that "work differently between no --pretty on the
command line and an explicit --pretty=medium" is more work than "we always
default to show notes if showing in the default verbosity".

This version considers a user supplied "format.pretty" configuration as
"the user told us to use this specific format, and we won't show notes
unless explicitly told".  I personally don't care either way exactly
because I don't use that configuration (nor teach others to use it), but
in a sense the configuration is setting a personal "default", so I think
it could be argued that we should instead show the notes by default in
that case (i.e. remove "rev->pretty_given = 1" in the first hunk).

 builtin-log.c |    9 ++++++---
 revision.c    |    4 ++++
 revision.h    |    2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 3bc3919..1e05b0f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -39,10 +39,10 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
-	if (fmt_pretty)
+	if (fmt_pretty) {
 		get_commit_format(fmt_pretty, rev);
-	else
-		rev->show_notes = 1;
+		rev->pretty_given = 1;
+	}
 	rev->verbose_header = 1;
 	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
@@ -60,6 +60,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		usage(builtin_log_usage);
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 
+	if (!rev->show_notes_given && !rev->pretty_given)
+		rev->show_notes = 1;
+
 	if (rev->diffopt.pickaxe || rev->diffopt.filter)
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
diff --git a/revision.c b/revision.c
index 7e00a6c..0de78fb 100644
--- a/revision.c
+++ b/revision.c
@@ -1161,14 +1161,18 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 	} else if (!strcmp(arg, "--pretty")) {
 		revs->verbose_header = 1;
+		revs->pretty_given = 1;
 		get_commit_format(arg+8, revs);
 	} else if (!prefixcmp(arg, "--pretty=") || !prefixcmp(arg, "--format=")) {
 		revs->verbose_header = 1;
+		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--show-notes")) {
 		revs->show_notes = 1;
+		revs->show_notes_given = 1;
 	} else if (!strcmp(arg, "--no-notes")) {
 		revs->show_notes = 0;
+		revs->show_notes_given = 1;
 	} else if (!strcmp(arg, "--oneline")) {
 		revs->verbose_header = 1;
 		get_commit_format("oneline", revs);
diff --git a/revision.h b/revision.h
index 4167c1e..a14deef 100644
--- a/revision.h
+++ b/revision.h
@@ -81,6 +81,8 @@ struct rev_info {
 	unsigned int	shown_one:1,
 			show_merge:1,
 			show_notes:1,
+			show_notes_given:1,
+			pretty_given:1,
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,

Re: git notes: notes

[Michael J Gruber]

[Adding cc from elsewhere in this thread, for fairness.]
Junio C Hamano venit, vidit, dixit 20.01.2010 22:08:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Junio C Hamano venit, vidit, dixit 20.01.2010 21:10:
>>> Joey Hess <joey@kitenet.net> writes:
>>>
>>>> Do you think it makes sense for even git log --format=format:%s to be
>>>> porcelain and potentially change when new features are used?
>>>
>>> If the series changed the meaning of "%s" format to mean "the subject of
>>> the commit and notes information", with or without documenting it, then it
>>> is just a bug we would like to fix.
>>
>> No, but outputting the note as part of the log is the standard. So for
>> example, when you do a format-patch | apply cycle, format-patch will
>> insert the note as part of the commit message, and apply will *store*
>> the note text (including Note:\n) as part of the commit message of the
>> new commit.
> 
> Thanks; that was the kind of breakage report I was looking for (and wished
> to have heard a lot earlier).  Personally I find it is unexcusable that
> format-patch defaults to giving notes.
> 
>> So, I would say the notes feature is not that well integrated right now,
> 
> No question about it.
> 
>> I'm not complaining, I actually have this on a maybe-to-do list, but the
>> way the series went kept me from investing time.
> 
> Hmm, that hints there is a failure in the review and merge process.  Care
> to explain how we could have done better please?

Well, I can only recall why it kept *me* from investing more time. I
actually have very little free time available, I contribute to Git
because it's fun, or I want a certain feature, or, admittedly, having a
commit in a project like Git gives me a certain satisfaction.

The notes feature looked very promising to me, and when Dscho came up
with it (or followed up on someone else's proposal, I don't remember) I
invested time in testing and contributing (minor) fixes. Then the series
took on a life on it's own, first "disappearing" (when I was wondering
if anything is going forward), then reappearing with (not only my)
commits squashed in (which took away the satisfaction part), then going
through a lengthy technical discussion on fan-out schemes (which was
necessary, but took away the fun part). [The last two parts may have
been the other way round, which doesn't matter.]

When the first part of the series landed I began to look at it again and
use it for the comments which go after the "--" part of a patch, only to
find out that format-patch issue. I noticed the bad consequence on
format-patch|apply only later. So I looked at the code, the log flags,
and was about to code when the notes API changed again (on pu). So I
decided to wait until the API is baked (that decision may have been
influenced by my expectation that my patches would get squashed in
again) and to fix that issue before 1.7.0.

Note that I have to match the project's necessities with time
availability on my side - otherwise I would have written that patch when
more of that series had landed. Now I reported it because it came up in
some disguise (and didn't want anyone spend time needlessly fixing a
side issue), and I'm not the one fixing it, but that's fine.

Besides the sociological aspect, I think you mentioned the main
technical aspects:

* If you introduce a new features, write extensive tests covering
non-uses and mixed uses of the feature.

* Write redundancy tests, such as checking that format-patch|apply and
apply|format-patch both amount to the "identity" in the appropriate sense.

Right now we do very atomic testing, which does have its merits (for
determining the cause of a breakage). But since many features and
commands are not orthogonal, atomic testing does not test for side
effects, and test repos are minimal. Trying to test for specific
combinations makes you miss some combinations, especially combinations
with future features. But testing for those identity operations
(quasi-noops, or cycles) should ensure some consistency.

Maybe we should have a test repo which has all kinds of features turned
on and used, and on which a set of those identities are tested. With
every new feature, the repo as well as the set of supposed identities
would need to be amended (maybe by cloning the repo, adding commits, and
testing on an increasing set of repos). That would have caught at least
the current issue immediately.

Cheers,
Michael

Re: git notes: notes

[Matthieu Moy]

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

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> No, but outputting the note as part of the log is the standard. So for
>> example, when you do a format-patch | apply cycle, format-patch will
>> insert the note as part of the commit message, and apply will *store*
>> the note text (including Note:\n) as part of the commit message of the
>> new commit.
>
> Thanks; that was the kind of breakage report I was looking for (and wished
> to have heard a lot earlier).  Personally I find it is unexcusable that
> format-patch defaults to giving notes.

OTOH, format-patch could give the notes, below the ---, where it will
be ignored by apply. That would make notes handy to prepare a patch
serie with additional messages: prepare everything within Git, and use
git send-email to send it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git notes: notes

[Sverre Rabbelier]

Heya,

On Sun, Jan 24, 2010 at 15:20, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:

> OTOH, format-patch could give the notes, below the ---, where it will
> be ignored by apply. That would make notes handy to prepare a patch
> serie with additional messages: prepare everything within Git, and use
> git send-email to send it.

I like that idea, but as an orthogonal feature, prepare notes in a
special namespace, say refs/notes/format-patch/, and then teach
format-patch a flag to honor that namespace.

-- 
Cheers,

Sverre Rabbelier

Re: git notes: notes

[Johan Herland]

On Wednesday 20 January 2010, Joey Hess wrote:
> Johan Herland wrote:
> > > PS, Has anyone thought about using notes to warn bisect away from
> > > commits that are known to be unbuildable or otherwise cause bisection
> > > trouble?
> >
> > No, I haven't thought of that specific use case. Great idea! :)
> 
> Only problem I see with doing it is it might be too easy to overwrite
> such a note with git notes edit -m

Well, you would have to run "git notes edit -m" with core.notesRef or 
$GIT_NOTES_REF set to the notes ref where bisect information is stored (e.g. 
"refs/notes/bisect").

In any case, I would not use "git notes" to maintain the bisect hints. 
Rather, I'd add subcommands to "git bisect" that would take care of 
maintaining the notes tree @ "refs/notes/bisect". Much more user-friendly 
than telling the user to write their own bisect-notes by hand.

> Did you consider having -m append a line to an existing note?

Hmm. Not really. The "git notes" porcelain was originally written by Dscho, 
and my builtin-ification of it (currently in 'pu') preserves the original 
semantics of "git notes edit -m". It might make sense to change the 
defaults; what do you think, Dscho?


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git notes: notes

[Johannes Schindelin]

Hi,

On Thu, 21 Jan 2010, Johan Herland wrote:

> On Wednesday 20 January 2010, Joey Hess wrote:
>
> > Did you consider having -m append a line to an existing note?
> 
> Hmm. Not really. The "git notes" porcelain was originally written by 
> Dscho, and my builtin-ification of it (currently in 'pu') preserves the 
> original semantics of "git notes edit -m". It might make sense to change 
> the defaults; what do you think, Dscho?

I do not really care as long as there is a nice way to edit the complete 
note interactively.

Of course, I _do_ expect people to get confused just like they do with the 
current inconsistencies: "git commit -m" does not really append, but set 
the commit message, even if you amend a commit.

So maybe you want to use a different command line option for that.

Ciao,
Dscho

Re: git notes: notes

[Joey Hess]

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

Johannes Schindelin wrote:
> I do not really care as long as there is a nice way to edit the complete 
> note interactively.
> 
> Of course, I _do_ expect people to get confused just like they do with the 
> current inconsistencies: "git commit -m" does not really append, but set 
> the commit message, even if you amend a commit.
> 
> So maybe you want to use a different command line option for that.

Maybe: git notes add [-m|-F]

-- 
see shy jo

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

Re: git notes: notes

[Johan Herland]

On Thursday 21 January 2010, Joey Hess wrote:
> Johannes Schindelin wrote:
> > I do not really care as long as there is a nice way to edit the
> > complete note interactively.
> >
> > Of course, I _do_ expect people to get confused just like they do with
> > the current inconsistencies: "git commit -m" does not really append,
> > but set the commit message, even if you amend a commit.
> >
> > So maybe you want to use a different command line option for that.
> 
> Maybe: git notes add [-m|-F]

Thanks for the suggestion. I've added this to the new iteration of the 
jh/notes series.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git notes: notes

[John Koleszar]

On Wed, 2010-01-20 at 21:05 -0500, Johan Herland wrote:
> On Wednesday 20 January 2010, Joey Hess wrote:
> > Johan Herland wrote:
> > > > PS, Has anyone thought about using notes to warn bisect away from
> > > > commits that are known to be unbuildable or otherwise cause bisection
> > > > trouble?
> > >
> > > No, I haven't thought of that specific use case. Great idea! :)
> > 
[...]
> 
> In any case, I would not use "git notes" to maintain the bisect hints. 
> Rather, I'd add subcommands to "git bisect" that would take care of 
> maintaining the notes tree @ "refs/notes/bisect". Much more user-friendly 
> than telling the user to write their own bisect-notes by hand.
> 

I haven't read up on notes more than enough to know its in the pipe, but
I had a similar idea for using them to store bisect hints. I've been
doing a lot of bisecting lately into a range that had a couple dormant
bugs where I'm trying to bisect bug B but bug A prevents me from making
a determination. Rather than skip what I know is an interesting commit,
I cherry-pick the bugfix commit(s) A' and test that, then reset and
continue bisecting.

Teaching bisect to consistently skip a commit, or to automatically
squash in A' if we have A and not A', would be a desirable feature. I
will have to read up some more on notes.

Re: git notes: notes

[Christian Couder]

On lundi 25 janvier 2010, John Koleszar wrote:
> On Wed, 2010-01-20 at 21:05 -0500, Johan Herland wrote:
> > On Wednesday 20 January 2010, Joey Hess wrote:
>
> > In any case, I would not use "git notes" to maintain the bisect hints.
> > Rather, I'd add subcommands to "git bisect" that would take care of
> > maintaining the notes tree @ "refs/notes/bisect". Much more
> > user-friendly than telling the user to write their own bisect-notes by
> > hand.
>
> I haven't read up on notes more than enough to know its in the pipe, but
> I had a similar idea for using them to store bisect hints. I've been
> doing a lot of bisecting lately into a range that had a couple dormant
> bugs where I'm trying to bisect bug B but bug A prevents me from making
> a determination. Rather than skip what I know is an interesting commit,
> I cherry-pick the bugfix commit(s) A' and test that, then reset and
> continue bisecting.
>
> Teaching bisect to consistently skip a commit, or to automatically
> squash in A' if we have A and not A', would be a desirable feature. I
> will have to read up some more on notes.

Perhaps you can read about "git replace" in my article:

http://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html

and/or my related presentation:

http://www.linux-kongress.org/2009/slides/fighting_regressions_with_git_bisect_christian_couder.pdf

I think in the long run it's much better to use git replace rather than 
notes, especially as replace refs for bisecting could be in their own 
refs/replace/bisect namespace. I may take the time to implement that soon 
if you or other people are interested.

Regards,
Christian.