Git development
 help / color / mirror / Atom feed
* Re: [PATCH] No diff -b/-w output for all-whitespace changes
From: Greg Bacon @ 2009-11-19 22:25 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, gitster
In-Reply-To: <200911192314.41542.trast@student.ethz.ch>

Thomas Rast wrote

> Judging from the test, this parses as
> 
>   Change git-diff's whitespace-ignoring modes to generate
>   output only if a non-(empty patch, which git-apply
>   rejects) results.
> 
> which is a bit weird, isn't it? :-)

Yes, rotten wording. I meant git-apply rejects empty patches,
so let's not do that!

I chose this route rather than making git-apply more
forgiving because as currently implemented, git-diff is not
entirely ignoring whitespace when commanded to do so.

Greg

^ permalink raw reply

* Re: [PATCH 1/2] gitweb.js: fix null object exception in initials  calculation
From: Stephen Boyd @ 2009-11-19 22:48 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200911192240.27743.jnareb@gmail.com>

2009/11/19 Jakub Narebski <jnareb@gmail.com>:
>
> Hmmm... gitweb/gitweb.perl *was* one of files I have tested
> 'blame_incremental' view on, but I have not experienced any
> crashes.
>
> Perhaps it was the matter of different JavaScript engine (Mozilla 1.7.12
> with Gecko/20050923 engine, Konqueror 3.5.3-0.2.fc4), or different
> starting point for blame.
>
> I assume that crashing lead simply to not working blame, not to browser
> crash?

Yes. The blame stops at 20% or something but the browser is fine.

^ permalink raw reply

* Re: [PATCH 2/2] gitweb.js: use unicode encoding for nbsp instead of html entity
From: Jakub Narebski @ 2009-11-19 23:00 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git
In-Reply-To: <1258659887-5244-3-git-send-email-bebarino@gmail.com>

On Thu, 19 Nov 2009, Stephen Boyd wrote:

> It seems that in Firefox-3.5 inserting nbsp with javascript inserts the
> literal nbsp; instead of a space. Fix this by inserting the unicode
> representation for nbsp instead.

Errr... why are you avoiding writing &nbsp; or "&nbsp;" here?

> 
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---

[From "[PATCH 0/2] jn/gitweb-blame fixes"]
> This series is based on next's jn/gitweb-blame branch.

> The second patch fixes a visible bug I see in Firefox. Although I
> assume on other browsers it's not a problem? I haven't tested it on
> others so I can't be sure.

Well, since I moved from elem.innerHTML (which is non-standard, and
does not work for some browsers in strict XHTML mode) to setting
elem.firstChild.data (which assumes that firstChild exists and it
is a text node) I have had damned *intermittent* bugs where sometimes
'&nbsp;' would be shown literally, and sometimes this entity would
be correctly rendered.

I suspect this is either bug in Firefox, or unspecified part of DOM.

As we need this only for progress report, I am not against this change,
if it doesn't make it worse in other browsers.

>  gitweb/gitweb.js |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
> index 02454d8..30597dd 100644
> --- a/gitweb/gitweb.js
> +++ b/gitweb/gitweb.js
> @@ -31,12 +31,12 @@
>  
>  /**
>   * pad number N with nonbreakable spaces on the left, to WIDTH characters
> - * example: padLeftStr(12, 3, '&nbsp;') == '&nbsp;12'
> - *          ('&nbsp;' is nonbreakable space)
> + * example: padLeftStr(12, 3, '\u00A0') == '\u00A012'
> + *          ('\u00A0' is nonbreakable space)
>   *
>   * @param {Number|String} input: number to pad
>   * @param {Number} width: visible width of output
> - * @param {String} str: string to prefix to string, e.g. '&nbsp;'
> + * @param {String} str: string to prefix to string, e.g. '\u00A0'
>   * @returns {String} INPUT prefixed with (WIDTH - INPUT.length) x STR
>   */
>  function padLeftStr(input, width, str) {
> @@ -158,7 +158,7 @@ function updateProgressInfo() {
>  
>  	if (div_progress_info) {
>  		div_progress_info.firstChild.data  = blamedLines + ' / ' + totalLines +
> -			' (' + padLeftStr(percentage, 3, '&nbsp;') + '%)';
> +			' (' + padLeftStr(percentage, 3, '\u00A0') + '%)';
>  	}
>  
>  	if (div_progress_bar) {
> -- 
> 1.6.5.3.433.g11067
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 0/2] jn/gitweb-blame fixes
From: Jakub Narebski @ 2009-11-19 23:05 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git
In-Reply-To: <1258659887-5244-1-git-send-email-bebarino@gmail.com>

On Thu, 19 Nov 2009, Stephen Boyd wrote:

> This series is based on next's jn/gitweb-blame branch.
> 
> I was trying out the incremental blame and noticed it didn't work each 
> time. The first patch fixes the crashing I experience when blaming
> gitweb.perl (ironic ;-)
> 
> The second patch fixes a visible bug I see in Firefox. Although I assume
> on other browsers it's not a problem? I haven't tested it on others so I
> can't be sure.

Thanks for working on this.  Also it is nice to have incremental blame
tested for another browser, beside Mozilla 1.17.2 and Konqueror 3.5.3

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Name/documentation on git-log --full-diff is insufficient
From: Phil Miller @ 2009-11-19 23:26 UTC (permalink / raw)
  To: Git Mailing List

I've been trying for a few days to figure out how to see all the files
touched in all the commits that changed a particular path. I just
discovered that that 'git log --full-diff --stat path' gives me the
complete list of files changed in the displayed commits, and not just
the files matching path. Reading the documentation, I figured that it
either only applied when running 'git log -p' or that it implied the
'-p', neither of which was desirable.

Thus, I'd suggest a couple potential improvements:
1. Expand the manual text to clarify that --full-diff really means
'tell me about all of the files in the commits that I see' regardless
of what information (a list of files, a diffstat, or a full diff) is
being printed about those files
2. Add a synonym with a more general name. Something like --all-files
would be close to the mark, though there are probably other more
suitable names.

Phil

PS: I sent in a patch for git-cvsserver a week ago, and have seen no
response or action on it. Is there something I should have done
differently?
http://article.gmane.org/gmane.comp.version-control.git/132789

^ permalink raw reply

* Default history simplification
From: Tommy Wang @ 2009-11-19 23:30 UTC (permalink / raw)
  To: git

Hi all,

I'm working on a small perl script that needs to reproduce the results
of the default git history simplification used by log & rev-list when
a list of paths is given.  It is important that I reproduce its
results exactly.  I would love to simply use the rev-list built-in to
do my work for me; but I fear that I may have much too many path
limiters than the linux command-line can handle (which if I'm correct,
can only take so many arguments).

I can understand whether a commit is included or not rather easily: we
basically only include a commit iff the commit introduces a change to
the given path (ie, it is !TREESAME to any of its parents).

For merges, I get a little confused.  In the simple case of a 2-parent
merge, if we find that we are TREESAME with one parent -- it follows
that the interesting change must have come from that parent.  So we
follow it, and can safely assume that the other parent is
uninteresting.

If we find that we are TREESAME with both parents, it follows that
neither parent nor the merge commit is interesting.  Therefore, we
randomly pick the first parent and move up the graph looking for an
interesting commit.  Looking at this more closely, the parents we
ignored will fall under three scenarios:

1. It finds it way all the way up to a root commit without finding an
interesting commit -- which can happen if your repository has multiple
root commits.
2. It will move up the graph and eventually find a common ancestor
with the first parent.
3. It will find an interesting commit that has an identical/equivalent
commit somewhere in the first parent's ancestry.

In all 3 cases, it is safe to follow just the first TREESAME parent.
It is interesting to note that if are working with a repository with
multiple root commits (say, you imported a project without history,
and later merged in its upstream history), you could potentially lose
that history with this algorithm if you happened to merge it as the
second parent.  This is not necessarily a flaw (in fact, it may be a
feature!), since you have still fully explained the state of your HEAD
with respect to the first parent (which is probably your mainline).

In the case that neither parent was TREESAME, we can have 2 scenarios:

1. The merge commit itself made the interesting change.
2. The interesting changes came from both parents, in which we would
rightfully follow both parents.

My first question is this:

If the merge commit itself (D) made the interesting change, we could
potentially follow an uninteresting parent -- most likely all the way
up until we find a common ancestor (A).  This would produce a graph
that looks like:

A---B---D
 \-------/ (C pruned)

Or, if both parents were uninteresting:

A---D (both B & C pruned)
 \---/

I assume that the simplification takes care of this by removing
duplicate parents as well as searching for common ancestors?  (It is
not mentioned in the documentation).

My second question is then:

Given that we perform an extra simplification pass to remove common
ancestors and duplicate parents, what is the purpose of selectively
following parents?  Is it just for speed?  If we always followed all
parents and applied our duplicate simplification and common ancestor
simplification, we would not always arrive at the same result?  In
which case, if my application is not concerned with speed (ie, I don't
mind following all parents), would I always arrive at the same graph?

Thanks,

Tommy

^ permalink raw reply

* [PATCHv4] Re: gitk french translation
From: Nicolas Sebrecht @ 2009-11-19 23:57 UTC (permalink / raw)
  To: Emmanuel Trillaud
  Cc: Nicolas Sebrecht, Maximilien Noal, Matthieu Moy, Nicolas Pitre,
	Thomas Moulard, Guy Brand, Git Mailing List
In-Reply-To: <20091119222331.6ea1e691.etrillaud@gmail.com>

The 19/11/09, Emmanuel Trillaud wrote:

> Signed-off-by: Emmanuel Trillaud <etrillaud@gmail.com>
> Signed-off-by: Thomas Moulard <thomas.moulard@gmail.com>
> Signed-off-by: Guy Brand <gb@unistra.fr>
> 
> --

The two dashes are taken as the signature delimiter by my MUA, so I can't
comment without doing an annoying repairing. Please, keep the three
dashes undamaged.

> @@ -0,0 +1,1243 @@
> +
> +# French translation of the gitk package
> +# Copyright (C) 2005-2008 Paul Mackerras.  All rights reserved.
> +# This file is distributed under the same license as the gitk package.
> +# Translators:
> +# Emmanuel Trillaud <etrillaud@gmail.com>
> +#
> +msgid ""
> +msgstr ""
> +"Project-Id-Version: gitk\n"
> +"Report-Msgid-Bugs-To: \n"
> +"POT-Creation-Date: 2009-10-05 15:16+0200\n"
> +"PO-Revision-Date: 2009-11-19 22:11+0100\n"
> +"Last-Translator: YOUR NAME <E-MAIL@ADDRESS>\n"
> +"Language-Team: FRENCH\n"

Could you please fill in these fields?

> +#: gitk:113
> +msgid "Couldn't get list of unmerged files:"
> +msgstr "Impossible de r=E9cup=E9rer la liste des fichiers non fusionn=E9s =
> :"

The whole patch is broken because of wrapped lines like this one. I
can't believe any patch will be merged because the maintainer would have
to manually repair all these lines. Please, disable the line wrapping
option of Sylpheed to inline patches. 

> +#: gitk:2141
> +msgid "Diff"
> +msgstr "Diff=E9rences"

Didn't we agreed on the "Diff" translation?

> +#: gitk:2361 gitk:2378
> +msgid "Diff this -> selected"
> +msgstr "Diff=E9rences entre ceci et la s=E9lection"

Ditto.

> +#: gitk:2362 gitk:2379
> +msgid "Diff selected -> this"
> +msgstr "Diff=E9rence entre s=E9lection et ceci"

Ditto.

> +msgid "<Delete>, b\tScroll diff view up one page"
> +msgstr "<Supprimer>, b\tMonter d'une page dans la vue des diff=E9rences"
> +
> +#: gitk:2706
> +msgid "<Backspace>\tScroll diff view up one page"
> +msgstr "<Backspace>\tMonter d'une page dans la vue des diff=E9rences"
> +
> +#: gitk:2707
> +msgid "<Space>\t\tScroll diff view down one page"
> +msgstr "<Espace>\t\tDescendre d'une page dans la vue des diff=E9rences"
> +
> +#: gitk:2708
> +msgid "u\t\tScroll diff view up 18 lines"
> +msgstr "u\t\tMonter de 18 lignes dans la vue des diff=E9rences"
> +
> +#: gitk:2709
> +msgid "d\t\tScroll diff view down 18 lines"
> +msgstr "d\t\tDescendre de 18 lignes dans la vue des diff=E9rences"

Ditto for all the above.

> +#: gitk:2715
> +msgid "f\t\tScroll diff view to next file"
> +msgstr "f\t\tAller au prochain fichier dans la vue des diff=E9rences"

Ditto.

> +#: gitk:2716
> +#, tcl-format
> +msgid "<%s-S>\t\tSearch for next hit in diff view"
> +msgstr "<%s-S>\t\tAller au r=E9sultat suivant dans la vue des
> diff=E9rences" +
> +#: gitk:2717
> +#, tcl-format
> +msgid "<%s-R>\t\tSearch for previous hit in diff view"
> +msgstr "<%s-R>\t\tAller au r=E9sultat pr=E9c=E9dent dans la vue des
> diff=E9rences" +

Ditto.

Ok, I stop here the review. It really looks like you've sent a wrong
patch; maybe an earlier version which doesn't include the previous
comments.

-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: [PATCHv4] Re: gitk french translation
From: Emmanuel Trillaud @ 2009-11-20  0:28 UTC (permalink / raw)
  To: Nicolas Sebrecht
  Cc: Maximilien Noal, Matthieu Moy, Nicolas Pitre, Thomas Moulard,
	Guy Brand, Git Mailing List
In-Reply-To: <20091119235740.GA12954@vidovic>




On Fri, 20 Nov 2009 00:57:40 +0100
Nicolas Sebrecht <nicolas.s.dev@gmx.fr> wrote:

> The 19/11/09, Emmanuel Trillaud wrote:
> 
> > Signed-off-by: Emmanuel Trillaud <etrillaud@gmail.com>
> > Signed-off-by: Thomas Moulard <thomas.moulard@gmail.com>
> > Signed-off-by: Guy Brand <gb@unistra.fr>
> > 
> > --
> 
> The two dashes are taken as the signature delimiter by my MUA, so I can't
> comment without doing an annoying repairing. Please, keep the three
> dashes undamaged.


> > @@ -0,0 +1,1243 @@
> > +
> > +# French translation of the gitk package
> > +# Copyright (C) 2005-2008 Paul Mackerras.  All rights reserved.
> > +# This file is distributed under the same license as the gitk package.
> > +# Translators:
> > +# Emmanuel Trillaud <etrillaud@gmail.com>
> > +#
> > +msgid ""
> > +msgstr ""
> > +"Project-Id-Version: gitk\n"
> > +"Report-Msgid-Bugs-To: \n"
> > +"POT-Creation-Date: 2009-10-05 15:16+0200\n"
> > +"PO-Revision-Date: 2009-11-19 22:11+0100\n"
> > +"Last-Translator: YOUR NAME <E-MAIL@ADDRESS>\n"
> > +"Language-Team: FRENCH\n"
> 
> Could you please fill in these fields?
> 
> > +#: gitk:113
> > +msgid "Couldn't get list of unmerged files:"
> > +msgstr "Impossible de r=E9cup=E9rer la liste des fichiers non fusionn=E9s =
> > :"
> 
> The whole patch is broken because of wrapped lines like this one. I
> can't believe any patch will be merged because the maintainer would have
> to manually repair all these lines. Please, disable the line wrapping
> option of Sylpheed to inline patches. 
> 
> > +#: gitk:2141
> > +msgid "Diff"
> > +msgstr "Diff=E9rences"
> 
> Didn't we agreed on the "Diff" translation?
> 
> > +#: gitk:2361 gitk:2378
> > +msgid "Diff this -> selected"
> > +msgstr "Diff=E9rences entre ceci et la s=E9lection"
> 
> Ditto.
> 
> > +#: gitk:2362 gitk:2379
> > +msgid "Diff selected -> this"
> > +msgstr "Diff=E9rence entre s=E9lection et ceci"
> 
> Ditto.
> 
> > +msgid "<Delete>, b\tScroll diff view up one page"
> > +msgstr "<Supprimer>, b\tMonter d'une page dans la vue des diff=E9rences"
> > +
> > +#: gitk:2706
> > +msgid "<Backspace>\tScroll diff view up one page"
> > +msgstr "<Backspace>\tMonter d'une page dans la vue des diff=E9rences"
> > +
> > +#: gitk:2707
> > +msgid "<Space>\t\tScroll diff view down one page"
> > +msgstr "<Espace>\t\tDescendre d'une page dans la vue des diff=E9rences"
> > +
> > +#: gitk:2708
> > +msgid "u\t\tScroll diff view up 18 lines"
> > +msgstr "u\t\tMonter de 18 lignes dans la vue des diff=E9rences"
> > +
> > +#: gitk:2709
> > +msgid "d\t\tScroll diff view down 18 lines"
> > +msgstr "d\t\tDescendre de 18 lignes dans la vue des diff=E9rences"
> 
> Ditto for all the above.
> 
> > +#: gitk:2715
> > +msgid "f\t\tScroll diff view to next file"
> > +msgstr "f\t\tAller au prochain fichier dans la vue des diff=E9rences"
> 
> Ditto.
> 
> > +#: gitk:2716
> > +#, tcl-format
> > +msgid "<%s-S>\t\tSearch for next hit in diff view"
> > +msgstr "<%s-S>\t\tAller au r=E9sultat suivant dans la vue des
> > diff=E9rences" +
> > +#: gitk:2717
> > +#, tcl-format
> > +msgid "<%s-R>\t\tSearch for previous hit in diff view"
> > +msgstr "<%s-R>\t\tAller au r=E9sultat pr=E9c=E9dent dans la vue des
> > diff=E9rences" +
> 
> Ditto.
> 
> Ok, I stop here the review. It really looks like you've sent a wrong
> patch; maybe an earlier version which doesn't include the previous
> comments.
Sorry for sending the wrong patch. I don't exactly what happend, but
I'll send a correct patch soon and I'll be more carefull.
Thank you for the patience.

> -- 
> Nicolas Sebrecht


-- 
Emmanuel Trillaud <etrillaud@gmail.com>

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Jakub Narebski @ 2009-11-20  0:49 UTC (permalink / raw)
  To: George Dennie
  Cc: git, B.Steinbrink, 'Jason Sewall',
	'Jan Krüger', torvalds
In-Reply-To: <00d401ca6954$a29fa020$e7dee060$@com>

On Thu, 19 Nov 2009, George Dennie wrote:

> Thanks Jakub Narebski and Björn Steinbrink...Nice description Björn.
> 
> I think an important piece of conceptual information missing from the docs
> is a concise list of the conceptual properties defining the context of the
> working tree, index, and repository during normal use. This itemization
> would go far in explaining the synergies between the various commands.

If you didn't find sufficient description of underlying concepts behind
git in "Git User's Manual" (distributed with Git), "Git Community Book"
or "Pro Git", take a look at the following documents:

 * "Git for Computer Scientists"
 * "Git From Bottom's Up"
 * "The Git Parable"

> Functionally, all the commands merely manipulate these properties. If these
> properties were summarize in context one would expect that would represent a
> very complete functional model of Git. A user could review the description
> figure what they wanted to do and then find the command(s) to accomplish it.

I disagree.  While understanding underlying concepts of Git helps with
finding a way to get what one wants to achieve, I don't think that the way
presented here would work in practice.

> Presently this knowledge is accreted over time as oppose to merely being
> read and in the space of a few minutes "groked" (of course it could be that
> I am particularly limited :).

It is documented, see referenced mentioned above.

> For example, towards a functional model, is this close? (note: all
> properties can be blank/empty)...
> 
> REPOSITORIES
> 	Collection of Commits

Direct Acyclic Graph of Commits, where edges in graph point from commit
to zero or more its parents.

> 	Collection of Branches
> 		-- collection of commits without children

Errr... what?  Commit doesn't *have* [pointer to] children.  Also branch
can point to commit for which there exists other commit which has given
commit as parent (up-to-date or fast-forward situation, e.g.)


    a---b---c            <--- branch_a
             \
              \-d---e    <--- branch_b

Branches (or branch heads / branch tips) are named references into DAG
of commits, points where DAG of commits grow.

> 		-- as a result each commits either augments
> 		-- and existing branch or creates a new one

Commits do not create a new branch.  New commits must be crated on
existing branch (or on unnamed branch aka detached HEAD, but that is
advanced usage).

> 	Master Branch
> 		-- typically the publishable development history

TANSTAAMB. There ain't such thing as a master branch. ;-)))))

Well, at least not in a sense of there being a branch that is a trunk
branch distinguished by _technical_ means.

> 
> INDEX
> 	Collections of Parent/Merge Commits
> 		-- the commit will use all these as its parent

No.  The index is set of versions of files (blobs) that would go as
a contents (tree) of a next commit (if you use "git commit', not 
"git commit -a").

> 
> 	Staged Commit 
> 		-- these changes are shown relative to the working tree

Errr.... what?

> 
> 	Default Branch
> 		-- the history the staged commit is suppose to augment

Errr... what?

If by "default branch" you mean "current branch", it is currently checked
out branch, where new commit would go, pointed by HEAD symbolic reference.


> WORKING_TREE
> 	Collection of Files and Folders
> 	
> 
> As far as I can tell, the working tree is not suppose to be stateful, but it
> seems the commands treat it as such.

Stateful?

Working tree / working area is a working area.  It can be disconnected from
repository via core.worktree, --work-tree option and GIT_WORK_TREE 
environment, see also contrib/workdir/git-new-workdir


> Again, thanks for your patients.

patience.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 2/2] gitweb.js: use unicode encoding for nbsp instead of html entity
From: Stephen Boyd @ 2009-11-20  1:00 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200911200000.41658.jnareb@gmail.com>

Jakub Narebski wrote:
> On Thu, 19 Nov 2009, Stephen Boyd wrote
>> It seems that in Firefox-3.5 inserting nbsp with javascript inserts the
>> literal nbsp; instead of a space. Fix this by inserting the unicode
>> representation for nbsp instead.
>
> Errr... why are you avoiding writing &nbsp; or "&nbsp;" here?

Sorry, this part was rushed out. I can fix it in a resend if this is 
actually the right thing to do.

> Well, since I moved from elem.innerHTML (which is non-standard, and
> does not work for some browsers in strict XHTML mode) to setting
> elem.firstChild.data (which assumes that firstChild exists and it
> is a text node) I have had damned *intermittent* bugs where sometimes
> '&nbsp;' would be shown literally, and sometimes this entity would
> be correctly rendered.
>
> I suspect this is either bug in Firefox, or unspecified part of DOM.
>
> As we need this only for progress report, I am not against this change,
> if it doesn't make it worse in other browsers.

I've tested this in Opera-10.10 now and it looks good. I'll try to get 
my hands on a Windows machine to test with IE, but no promises.

^ permalink raw reply

* Re: [PATCH 0/2] jn/gitweb-blame fixes
From: Stephen Boyd @ 2009-11-20  1:00 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200911200005.08841.jnareb@gmail.com>

Jakub Narebski wrote:
>
> Thanks for working on this.  Also it is nice to have incremental blame
> tested for another browser, beside Mozilla 1.17.2 and Konqueror 3.5.3

For those following along, Opera-10.10 has been tested and works.

^ permalink raw reply

* Re: Headless tags don't have a follows or precedes?
From: Tim Mazid @ 2009-11-20  1:07 UTC (permalink / raw)
  To: git
In-Reply-To: <4AEF009E.5060005@drmicha.warpmail.net>


Hey list,

I was just wondering if there were any updates to this?

Also, I believe I forgot to mention the gitk version, it's 1.6.5.3-1.

Cheers,
Tim.


Michael J Gruber-2 wrote:
> 
> Tim Mazid venit, vidit, dixit 02.11.2009 14:07:
>> Michael J Gruber-2 wrote:
>>>
>>> Would would help:
>>>
>>> - saying you're talking about gitk/git view/whatever it is you're
>>> "clicking" on
>>>
>> My apologies, yes, in gitk.
>> 
> 
> Now we only need the version... but we'll see if current versions
> reproduce it.
> 
>> Michael J Gruber-2 wrote:
>>>
>>> - providing a minimal example others can reproduce. That would be one
>>> where a tag on a detached head (assuming that's what you mean) has no
>>> precedes/follow but a tag "on a branch" does have that info
>>>
>> 
>> Example (unless specified, commands as entered into bash)
> 
> Great example, thanks!
> 
>> 
>> mkdir temp
>> cd temp
>> git init
>> gitk --all &
>> git commit --allow-empty -m '1'
>> git tag v1
>> git commit --allow-empty -m '1.1'
>> git tag v1.1
>> git commit --allow-empty -m '1.2'
>> git tag v1.2
>> (in gitk, press ctrl+f5; all follows and precedes info is there)
>> git checkout v1.1
>> git commit --allow-empty -m '1.1.1'
>> git tag v1.1.1
>> (in gitk, press f5; follows and precedes info missing for v1.1 and
>> v1.1.1)
> 
> For me, v1.1.1 has no info and v1.1 is missing v1.1.1 in its precedes.
> 
>> (close gitk)
>> gitk --all &
>> (info still missing)
>> git commit --allow-empty -m '1.1.2'
>> git tag v1.1.2
>> (in gitk, press f5, info still missing)
> 
> v1.1.1 and v1.1.2 missing all follow/precede info.
> 
>> git checkout master
>> git commit --allow-empty -m '1.3'
>> git tag v1.3
>> (in gitk, press f5, info still missing)
> 
> Now, even v1.3 is missing its follows and v1.2 its precedes, even though
> they've got nothing to do with the "detached branch".
> 
>> git commit --allow-empty -m '1.4'
>> git tag v1.4
>> (in gitk, press f5, info still missing)
>> git checkout -b temp v1.2
>> git commit --allow-empty -m '1.2.1'
>> git tag v1.2.1
>> (in gitk, press f5, info still missing)
>> git checkout master
>> git branch -D temp
>> git commit --allow-empty -m '1.5'
>> git tag v1.5
>> (in gitk, press f5, info still missing)
>> 
>> 
>> In the end, the only follows/precedes info is:
>> v1: precedes v1.1
>> v1.1: follows v1, precedes v1.2
>> v1.2: follows v1.1
>> All the rest is missing.
> 
> So basically, all connectivity which has been created after detaching
> the head is missing, even that which has been created on a "proper
> branch", which means (to me) it has nothing to do with git's revision
> parsing (such as missing out on lightweight tags on detached heads).
> 
> I looked at the gitk code and got the expected result: no clue (tcl/tk
> doesn't tick my fancy). gitk's parsing of ancestry relations seems to be
> done completely in tcl (rather then relaying a lot to git-rev-parse,
> which may not be efficient here). So I'll take the liberty to cc the
> main gitk guy. A few more notes:
> 
> After generating v1.1.1 (which misses "follows"), .git/gitk.cache has
> this (\n added for clarity):
> 
> 1 1\n
> 6bfcf857ceef0507bb50ee17302c1d068b697540
> b67f4651e49a33ee8cc77157e4e51d1e635a7c0d
> {540abf2b75aec7ccbd8c0413863a018fc1c1eb37
> b67f4651e49a33ee8cc77157e4e51d1e635a7c0d}\n
> 1\n
> 
> If I move that out of the way and rerun gitk, everything's in apple pie
> order, and the cache file is:
> 
> 1 3\n
> 2fd83b12ccea07c88f5998aa6303003ef1e4858b
> 540abf2b75aec7ccbd8c0413863a018fc1c1eb37
> 540abf2b75aec7ccbd8c0413863a018fc1c1eb37\n
> 6bfcf857ceef0507bb50ee17302c1d068b697540
> 540abf2b75aec7ccbd8c0413863a018fc1c1eb37
> 540abf2b75aec7ccbd8c0413863a018fc1c1eb37\n
> 540abf2b75aec7ccbd8c0413863a018fc1c1eb37
> b67f4651e49a33ee8cc77157e4e51d1e635a7c0d
> b67f4651e49a33ee8cc77157e4e51d1e635a7c0d\n
> 1\n
> 
> Unsurprisingly, v1.1.2 (committed & tagged on a detached head) trips
> things up again, moving gitk.cache out of the way helps again.
> 
> Surprisingly, v1.3 (committed and tagged on a checked out branch) trips
> things up again, moving... helps again.
> 
> Paul, I hope you can make sense of this. Something in gitk.cache
> prevents gitk from rescanning for new children, an empty cache gets it
> right, but only until the next run.
> 
> Michael
> 

-- 
View this message in context: http://n2.nabble.com/Headless-tags-don-t-have-a-follows-or-precedes-tp3926483p4035472.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location
From: Tarmigan Casebolt @ 2009-11-20  1:22 UTC (permalink / raw)
  To: git; +Cc: peff, jaysoffian, drizzd, gitster, spearce, Tarmigan Casebolt

On my machine with CentOS, httpd is located at /usr/sbin/httpd, and
the modules are located at /usr/lib64/httpd/modules.  To enable easy
testing of httpd, we would like those locations to be detected
automatically.

uname might not be the best way to determine the default location for
httpd since different Linux distributions apparently put httpd in
different places, so we test a couple different locations for httpd,
and use the first one that we come across.  We do the same for the
modules directory.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---

Would any machines have httpd or the modules/ directory in several of
these locations?

Also I don't really know shell scripting, so while this Works For Me,
it may be completely wrong.

 t/lib-httpd.sh |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6765b08..6b86353 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -12,16 +12,23 @@ fi
 
 HTTPD_PARA=""
 
+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
+do
+	test -x "$DEFAULT_HTTPD_PATH" && break
+done
+
+for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
+                                 '/usr/lib/apache2/modules' \
+                                 '/usr/lib64/httpd/modules' \
+                                 '/usr/lib/httpd/modules'
+do
+	test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
+done
+
 case $(uname) in
 	Darwin)
-		DEFAULT_HTTPD_PATH='/usr/sbin/httpd'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/libexec/apache2'
 		HTTPD_PARA="$HTTPD_PARA -DDarwin"
 	;;
-	*)
-		DEFAULT_HTTPD_PATH='/usr/sbin/apache2'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/lib/apache2/modules'
-	;;
 esac
 
 LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
-- 
1.6.5.52.g35487

^ permalink raw reply related

* [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.
From: Tarmigan Casebolt @ 2009-11-20  1:22 UTC (permalink / raw)
  To: git; +Cc: peff, jaysoffian, drizzd, gitster, spearce, Tarmigan Casebolt
In-Reply-To: <1258680123-28684-1-git-send-email-tarmigan+git@gmail.com>

With smart http, git over http is likely to become much more common.
To increase testing of smart http, enable the http tests by default.

If we cannot detect httpd, we still skip these tests, so it should not
cause problems on platforms where we cannot run the tests.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/lib-httpd.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6b86353..db537b4 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -3,11 +3,12 @@
 # Copyright (c) 2008 Clemens Buchacher <drizzd@aon.at>
 #
 
-if test -z "$GIT_TEST_HTTPD"
+if test -n "$NO_GIT_TEST_HTTPD"
 then
-	say "skipping test, network testing disabled by default"
-	say "(define GIT_TEST_HTTPD to enable)"
+	say "Skipping http tests because NO_GIT_TEST_HTTPD is defined"
 	test_done
+else
+	say "Define NO_GIT_TEST_HTTPD to disable http testing"
 fi
 
 HTTPD_PARA=""
-- 
1.6.5.52.g35487

^ permalink raw reply related

* [PATCH v2] gitk: Honor TMPDIR when viewing diffs externally
From: David Aguilar @ 2009-11-20  1:27 UTC (permalink / raw)
  To: paulus; +Cc: peff, sam, git, David Aguilar

gitk's external diff fails when browsing read-only repositories.
This is due to gitk's assumption that the current directory is
always writable.  By honoring TMPDIR we avoid this problem and
allow users to define the location used for temporary files.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

This version of the patch is more careful to ensure that
the temporary files and directories created by gitk are not
easily predictable.

 gitk |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/gitk b/gitk
index 364c7a8..84ea8a9 100755
--- a/gitk
+++ b/gitk
@@ -3292,17 +3292,21 @@ proc flist_hl {only} {
     set gdttype [mc "touching paths:"]
 }
 
+proc randstr {} {
+    global initial_rand
+    return [format "%f" [expr rand() + $initial_rand]]
+}
+
 proc gitknewtmpdir {} {
-    global diffnum gitktmpdir gitdir
+    global diffnum gitktmpdir gitdir env
 
     if {![info exists gitktmpdir]} {
-	set gitktmpdir [file join [file dirname $gitdir] \
-			    [format ".gitk-tmp.%s" [pid]]]
-	if {[catch {file mkdir $gitktmpdir} err]} {
-	    error_popup "[mc "Error creating temporary directory %s:" $gitktmpdir] $err"
-	    unset gitktmpdir
-	    return {}
+	if {[info exists env(TMPDIR)]} {
+	    set tmpdir $env(TMPDIR)
+	} else {
+	    set tmpdir [file dirname $gitdir]
 	}
+	set gitktmpdir [file join $tmpdir ".gitk-tmp.[pid].[randstr]"]
 	set diffnum 0
     }
     incr diffnum
@@ -3339,10 +3343,12 @@ proc external_diff_get_one_file {diffid filename diffdir} {
 	return $nullfile
     }
     if {$diffid == $nullid2} {
-        set difffile [file join $diffdir "\[index\] [file tail $filename]"]
+        set difffile [file join $diffdir \
+	       "\[index-[randstr]\] [file tail $filename]"]
         return [save_file_from_commit :$filename $difffile index]
     }
-    set difffile [file join $diffdir "\[$diffid\] [file tail $filename]"]
+    set difffile [file join $diffdir \
+	       "\[$diffid-[randstr]\] [file tail $filename]"]
     return [save_file_from_commit $diffid:$filename $difffile \
 	       "revision $diffid"]
 }
@@ -8525,8 +8531,8 @@ proc diffcommits {a b} {
     global diffcontext diffids blobdifffd diffinhdr
 
     set tmpdir [gitknewtmpdir]
-    set fna [file join $tmpdir "commit-[string range $a 0 7]"]
-    set fnb [file join $tmpdir "commit-[string range $b 0 7]"]
+    set fna [file join $tmpdir "commit-[string range $a 0 7]-[randstr]"]
+    set fnb [file join $tmpdir "commit-[string range $b 0 7]-[randstr]"]
     if {[catch {
 	exec git diff-tree -p --pretty $a >$fna
 	exec git diff-tree -p --pretty $b >$fnb
@@ -11321,6 +11327,7 @@ if {[tk windowingsystem] eq "aqua"} {
     set textfont {Courier 9}
     set uifont {Helvetica 9 bold}
 }
+set initial_rand [expr srand([clock scan now])]
 set tabstop 8
 set findmergefiles 0
 set maxgraphpct 50
-- 
1.6.5.3.171.ge36e

^ permalink raw reply related

* [PATCH] submodule.c: Squelch a "use before assignment" warning
From: David Aguilar @ 2009-11-20  1:33 UTC (permalink / raw)
  To: gitster; +Cc: git, David Aguilar

i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler
(and probably others) mistakenly thinks variable 'right' is used
before assigned.  Work it around by giving it a fake initialization.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 submodule.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/submodule.c b/submodule.c
index 461faf0..0145a62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset)
 {
 	struct rev_info rev;
-	struct commit *commit, *left = left, *right;
+	struct commit *commit, *left = left, *right = NULL;
 	struct commit_list *merge_bases, *list;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
-- 
1.6.5.3.171.ge36e

^ permalink raw reply related

* Re: Hey - A Conceptual Simplication....
From: Dmitry Potapov @ 2009-11-20  1:35 UTC (permalink / raw)
  To: George Dennie; +Cc: 'Jan Krüger', git
In-Reply-To: <008401ca6880$33d7e550$9b87aff0$@com>

On Wed, Nov 18, 2009 at 01:51:56PM -0500, George Dennie wrote:
> 
> One of the concerns I have with the manual pick-n-commit is that you can
> forget a file or two.

It is more difficult to make this mistake with Git than many others
VCSes, because Git shows the list of files that are changed but not
committed as well as the list of untracked files when you try to commit
something. So, it has never been a real issue for me in practice...

> Consequently, unless you do a clean checkout and test
> of the commit, you don't know that your publishable version even compiles.

If you want to be sure that clean checkout will be compiled, the only
way to guarantee that is to do a clean checkout. Even if you commit all
files except those that are specified in .gitignore, it is not enough to
be sure that a clean checkout will be compiled... But in most cases, you
do not need to do that to be *reasonable* sure that a clean checkout
will be compiled later, and if you have any doubts, you can do a clean
checkout and testing _after_ committing your changes. There is no reason
to be afraid to commit something that may not work if you can amend that
later (until you publish your changes).

> It seems safer to commit the entirety of your work in its working state and
> then do a clean checkout from a dedicated publishable branch and manually
> merge the changes in that, test, and commit.

Maybe I did not understand your words, but I am not sure what is gained
in this way... Clearly there is no reason to publish a work that you
have not tested yet. And no one cares about crap that you keep in your
working tree either... So, a better approach is to commit your changes
as a series of patches that can be reviewed easily, then do all testing
and then publish them for integration with the main development branch.

> 
> It seems the intuitive model is to treat version control as applying to the
> whole document, not parts of it. In this respect the document is defined by
> the IDE, namely the entire solution, warts and all.

This is a very bogus idea. If you want to preserve all warts etc, you
just do backup of the whole disk and now you have a state that can be
compiled any time later (provided that your hardware do not change too
much). In my experience, in most cases when I was not able to compile
an old version were caused not by forgetting to commit something, but
changing in the environment (like new compiler, new libraries, etc).

But when your commits are fine-grained, you can always cherry-pick the
corresponding fix-up and compile this old version if it is necessary.

In my experience, the value of VCS history is the ability to look at it
(sometimes many years later) and understand who wrote this line and why.
Also, nearly all cases when I had to compile some old version were due
to bisecting some tricky bug. In both cases, having fine-grained commits
was crucial to success.

> When you start
> selectively saving parts of the document then you are doing two things,
> versioning and publishing; and at the same time.

No, you don't. Committing some changes and publishing them are two
separated operations in Git, and that it is pretty much fundamental.
Normally, you commit changes in a few separated patches, review them to
make sure that changes match commit messages, do all testing, and only
then you publish them.


Dmitry

^ permalink raw reply

* [RFC/PATCHv8 00/10] git notes
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce

Hi,

Here is the 8th iteration of the git-notes series. Changes in this
iteration are as follows:

Changes to existing patches:
- Rebased onto current 'next', dropping the early part of this series
  which has now been merged to 'next'.
- Patch 8 (was patch 22): Major rewrite of fast-import's notes handling
  code based on comments from Shawn.

New patches:
- Patch 9: Rename t9301 to t9350, to make room for more fast-import tests
- Patch 10: More fast-import tests

TODO:
- Builtin-ify git-notes shell script to take advantage of notes API
- Garbage collect notes whose referenced object is unreachable (gc_notes())
- Handle note objects that are not blobs, but trees


Have fun! :)

...Johan


Johan Herland (10):
  Notes API: get_commit_notes() -> format_note() + remove the commit restriction
  Notes API: init_notes(): Initialize the notes tree from the given notes ref
  Notes API: add_note(): Add note objects to the internal notes tree structure
  Notes API: get_note(): Return the note annotating the given object
  Notes API: for_each_note(): Traverse the entire notes tree with a callback
  Notes API: Allow multiple concurrent notes trees with new struct notes_tree
  Refactor notes concatenation into a flexible interface for combining notes
  fast-import: Proper notes tree manipulation using the notes API
  Rename t9301 to t9350, to make room for more fast-import tests
  Add more testcases to test fast-import of notes

 fast-import.c                                    |  297 +++++++++++-
 notes.c                                          |  336 +++++++++----
 notes.h                                          |  114 ++++-
 pretty.c                                         |    8 +-
 t/t9300-fast-import.sh                           |  156 ++++++-
 t/t9301-fast-import-notes.sh                     |  578 ++++++++++++++++++++++
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 7 files changed, 1370 insertions(+), 119 deletions(-)
 create mode 100755 t/t9301-fast-import-notes.sh
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

^ permalink raw reply

* [RFC/PATCHv8 02/10] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

Created by a simple refactoring of initialize_notes().

Also add a new 'flags' parameter, which is a bitwise combination of notes
initialization flags. For now, there is only one flag - NOTES_INIT_EMPTY -
which indicates that the notes tree should not auto-load the contents of
the given (or default) notes ref, but rather should leave the notes tree
initialized to an empty state. This will become useful in the future when
manipulating the notes tree through the notes API.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   27 ++++++++++++++++-----------
 notes.h |   20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/notes.c b/notes.c
index 0f7082f..f2bacbb 100644
--- a/notes.c
+++ b/notes.c
@@ -339,13 +339,25 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
-static void initialize_notes(const char *notes_ref_name)
+void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	assert(!initialized);
+	initialized = 1;
+
+	if (!notes_ref) {
+		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		if (env)
+			notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		else
+			notes_ref = GIT_NOTES_DEFAULT_REF;
+	}
+
+	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
+	    read_ref(notes_ref, object_sha1) ||
 	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
@@ -378,15 +390,8 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized) {
-		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		if (env)
-			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		else if (!notes_ref_name)
-			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		initialize_notes(notes_ref_name);
-		initialized = 1;
-	}
+	if (!initialized)
+		init_notes(NULL, 0);
 
 	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index d745ed1..6b52799 100644
--- a/notes.h
+++ b/notes.h
@@ -1,6 +1,26 @@
 #ifndef NOTES_H
 #define NOTES_H
 
+/*
+ * Flags controlling behaviour of notes tree initialization
+ *
+ * Default behaviour is to initialize the notes tree from the tree object
+ * specified by the given (or default) notes ref.
+ */
+#define NOTES_INIT_EMPTY 1
+
+/*
+ * Initialize internal notes tree structure with the notes tree at the given
+ * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
+ * variable is used, and if that is missing, the default notes ref is used
+ * ("refs/notes/commits").
+ *
+ * If you need to re-intialize the internal notes tree structure (e.g. loading
+ * from a different notes ref), please first de-initialize the current notes
+ * tree by calling free_notes().
+ */
+void init_notes(const char *notes_ref, int flags);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 04/10] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

Created by a simple cleanup and rename of lookup_notes().

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   15 ++++++++-------
 notes.h |    3 +++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/notes.c b/notes.c
index 49a3e86..2196a5f 100644
--- a/notes.c
+++ b/notes.c
@@ -377,12 +377,13 @@ void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
 	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
 }
 
-static unsigned char *lookup_notes(const unsigned char *object_sha1)
+const unsigned char *get_note(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
-	if (found)
-		return found->val_sha1;
-	return NULL;
+	struct leaf_node *found;
+
+	assert(initialized);
+	found = note_tree_find(&root_node, 0, object_sha1);
+	return found ? found->val_sha1 : NULL;
 }
 
 void free_notes(void)
@@ -396,7 +397,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
-	unsigned char *sha1;
+	const unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -404,7 +405,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	if (!initialized)
 		init_notes(NULL, 0);
 
-	sha1 = lookup_notes(object_sha1);
+	sha1 = get_note(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index 5f22852..21a8930 100644
--- a/notes.h
+++ b/notes.h
@@ -25,6 +25,9 @@ void init_notes(const char *notes_ref, int flags);
 void add_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
+/* Get the note object SHA1 containing the note data for the given object */
+const unsigned char *get_note(const unsigned char *object_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 01/10] Notes API: get_commit_notes() -> format_note() + remove the commit restriction
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

There is really no reason why only commit objects can be annotated. By
changing the struct commit parameter to get_commit_notes() into a sha1 we
gain the ability to annotate any object type. To reflect this in the function
naming as well, we rename get_commit_notes() to format_note().

This patch also fixes comments and variable names throughout notes.c as a
consequence of the removal of the unnecessary 'commit' restriction.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   33 ++++++++++++++++-----------------
 notes.h  |   11 ++++++++++-
 pretty.c |    8 ++++----
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/notes.c b/notes.c
index 50a4672..0f7082f 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "commit.h"
 #include "notes.h"
 #include "refs.h"
 #include "utf8.h"
@@ -25,10 +24,10 @@ struct int_node {
 /*
  * Leaf nodes come in two variants, note entries and subtree entries,
  * distinguished by the LSb of the leaf node pointer (see above).
- * As a note entry, the key is the SHA1 of the referenced commit, and the
+ * As a note entry, the key is the SHA1 of the referenced object, and the
  * value is the SHA1 of the note object.
  * As a subtree entry, the key is the prefix SHA1 (w/trailing NULs) of the
- * referenced commit, using the last byte of the key to store the length of
+ * referenced object, using the last byte of the key to store the length of
  * the prefix. The value is the SHA1 of the tree object containing the notes
  * subtree.
  */
@@ -211,7 +210,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (concatenate_notes(l->val_sha1,
 						entry->val_sha1))
 					die("failed to concatenate note %s "
-					    "into note %s for commit %s",
+					    "into note %s for object %s",
 					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->val_sha1),
 					    sha1_to_hex(l->key_sha1));
@@ -299,7 +298,7 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n)
 {
-	unsigned char commit_sha1[20];
+	unsigned char object_sha1[20];
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
@@ -312,23 +311,23 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 	prefix_len = subtree->key_sha1[19];
 	assert(prefix_len * 2 >= n);
-	memcpy(commit_sha1, subtree->key_sha1, prefix_len);
+	memcpy(object_sha1, subtree->key_sha1, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
-				commit_sha1 + prefix_len, 20 - prefix_len);
+				object_sha1 + prefix_len, 20 - prefix_len);
 		if (len < 0)
 			continue; /* entry.path is not a SHA1 sum. Skip */
 		len += prefix_len;
 
 		/*
-		 * If commit SHA1 is complete (len == 20), assume note object
-		 * If commit SHA1 is incomplete (len < 20), assume note subtree
+		 * If object SHA1 is complete (len == 20), assume note object
+		 * If object SHA1 is incomplete (len < 20), assume note subtree
 		 */
 		if (len <= 20) {
 			unsigned char type = PTR_TYPE_NOTE;
 			struct leaf_node *l = (struct leaf_node *)
 				xcalloc(sizeof(struct leaf_node), 1);
-			hashcpy(l->key_sha1, commit_sha1);
+			hashcpy(l->key_sha1, object_sha1);
 			hashcpy(l->val_sha1, entry.sha1);
 			if (len < 20) {
 				l->key_sha1[19] = (unsigned char) len;
@@ -342,12 +341,12 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 static void initialize_notes(const char *notes_ref_name)
 {
-	unsigned char sha1[20], commit_sha1[20];
+	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
-	    get_tree_entry(commit_sha1, "", sha1, &mode))
+	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
 	hashclr(root_tree.key_sha1);
@@ -355,9 +354,9 @@ static void initialize_notes(const char *notes_ref_name)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
-static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
+	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
 	if (found)
 		return found->val_sha1;
 	return NULL;
@@ -370,7 +369,7 @@ void free_notes(void)
 	initialized = 0;
 }
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
@@ -389,7 +388,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 		initialized = 1;
 	}
 
-	sha1 = lookup_notes(commit->object.sha1);
+	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index a1421e3..d745ed1 100644
--- a/notes.h
+++ b/notes.h
@@ -4,10 +4,19 @@
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
+/* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
 #define NOTES_INDENT 2
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+/*
+ * Fill the given strbuf with the notes associated with the given object.
+ *
+ * If the internal notes structure is not initialized, it will be auto-
+ * initialized to the default value (see documentation for init_notes() above).
+ *
+ * 'flags' is a bitwise combination of the above formatting flags.
+ */
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 5661cba..771d186 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,8 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		get_commit_notes(commit, sb, git_log_output_encoding ?
-			     git_log_output_encoding : git_commit_encoding, 0);
+		format_note(commit->object.sha1, sb, git_log_output_encoding ?
+			    git_log_output_encoding : git_commit_encoding, 0);
 		return 1;
 	}
 
@@ -1057,8 +1057,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		strbuf_addch(sb, '\n');
 
 	if (fmt != CMIT_FMT_ONELINE)
-		get_commit_notes(commit, sb, encoding,
-				 NOTES_SHOW_HEADER | NOTES_INDENT);
+		format_note(commit->object.sha1, sb, encoding,
+			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
 }
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 05/10] Notes API: for_each_note(): Traverse the entire notes tree with a callback
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

This includes a first attempt at creating an optimal fanout scheme (which
is calculated on-the-fly, while traversing).

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notes.h |   17 ++++++++++
 2 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index 2196a5f..9581b98 100644
--- a/notes.c
+++ b/notes.c
@@ -339,6 +339,101 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
+/*
+ * Determine optimal on-disk fanout for this part of the notes tree
+ *
+ * Given a (sub)tree and the level in the internal tree structure, determine
+ * whether or not the given existing fanout should be expanded for this
+ * (sub)tree.
+ *
+ * Values of the 'fanout' variable:
+ * - 0: No fanout (all notes are stored directly in the root notes tree)
+ * - 1: 2/38 fanout
+ * - 2: 2/2/36 fanout
+ * - 3: 2/2/2/34 fanout
+ * etc.
+ */
+static unsigned char determine_fanout(struct int_node *tree, unsigned char n,
+		unsigned char fanout)
+{
+	/*
+	 * The following is a simple heuristic that works well in practice:
+	 * For each even-numbered 16-tree level (remember that each on-disk
+	 * fanout level corresponds to two 16-tree levels), peek at all 16
+	 * entries at that tree level. If any of them are subtree entries, then
+	 * there are likely plenty of notes below this level, so we return an
+	 * incremented fanout immediately. Otherwise, we return an incremented
+	 * fanout only if all of the entries at this level are int_nodes.
+	 */
+	unsigned int i;
+	if ((n % 2) || (n > 2 * fanout))
+		return fanout;
+	for (i = 0; i < 16; i++) {
+		switch(GET_PTR_TYPE(tree->a[i])) {
+		case PTR_TYPE_SUBTREE:
+			return fanout + 1;
+		case PTR_TYPE_INTERNAL:
+			continue;
+		default:
+			return fanout;
+		}
+	}
+	return fanout + 1;
+}
+
+static void construct_path_with_fanout(const unsigned char *sha1,
+		unsigned char fanout, char *path)
+{
+	unsigned int i = 0, j = 0;
+	const char *hex_sha1 = sha1_to_hex(sha1);
+	assert(fanout < 20);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	strcpy(path + i, hex_sha1 + j);
+}
+
+static int for_each_note_helper(struct int_node *tree, unsigned char n,
+		unsigned char fanout, each_note_fn fn, void *cb_data)
+{
+	unsigned int i;
+	void *p;
+	int ret = 0;
+	struct leaf_node *l;
+	static char path[40 + 19 + 1];  /* hex SHA1 + 19 * '/' + NUL */
+
+	fanout = determine_fanout(tree, n, fanout);
+	for (i = 0; i < 16; i++) {
+redo:
+		p = tree->a[i];
+		switch(GET_PTR_TYPE(p)) {
+		case PTR_TYPE_INTERNAL:
+			/* recurse into int_node */
+			ret = for_each_note_helper(
+				CLR_PTR_TYPE(p), n + 1, fanout, fn, cb_data);
+			break;
+		case PTR_TYPE_SUBTREE:
+			/* unpack subtree and resume traversal */
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			tree->a[i] = NULL;
+			load_subtree(l, tree, n);
+			free(l);
+			goto redo;
+		case PTR_TYPE_NOTE:
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			construct_path_with_fanout(l->key_sha1, fanout, path);
+			ret = fn(l->key_sha1, l->val_sha1, path, cb_data);
+			break;
+		}
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
@@ -386,6 +481,12 @@ const unsigned char *get_note(const unsigned char *object_sha1)
 	return found ? found->val_sha1 : NULL;
 }
 
+int for_each_note(each_note_fn fn, void *cb_data)
+{
+	assert(initialized);
+	return for_each_note_helper(&root_node, 0, 0, fn, cb_data);
+}
+
 void free_notes(void)
 {
 	note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index 21a8930..f67bae8 100644
--- a/notes.h
+++ b/notes.h
@@ -28,6 +28,23 @@ void add_note(const unsigned char *object_sha1,
 /* Get the note object SHA1 containing the note data for the given object */
 const unsigned char *get_note(const unsigned char *object_sha1);
 
+/*
+ * Invoke the specified callback function for each note
+ *
+ * If the callback returns nonzero, the note walk is aborted, and the return
+ * value from the callback is returned from for_each_note().
+ *
+ * IMPORTANT: The callback function is NOT allowed to change the notes tree.
+ * In other words, the following functions can NOT be invoked (on the current
+ * notes tree) from within the callback:
+ * - add_note()
+ * - free_notes()
+ */
+typedef int each_note_fn(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, const char *note_tree_path,
+		void *cb_data);
+int for_each_note(each_note_fn fn, void *cb_data);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 03/10] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   11 +++++++++++
 notes.h |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index f2bacbb..49a3e86 100644
--- a/notes.c
+++ b/notes.c
@@ -366,6 +366,17 @@ void init_notes(const char *notes_ref, int flags)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
+void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+{
+	struct leaf_node *l;
+
+	assert(initialized);
+	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
+	hashcpy(l->key_sha1, object_sha1);
+	hashcpy(l->val_sha1, note_sha1);
+	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+}
+
 static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
 	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 6b52799..5f22852 100644
--- a/notes.h
+++ b/notes.h
@@ -21,6 +21,10 @@
  */
 void init_notes(const char *notes_ref, int flags);
 
+/* Add the given note object to the internal notes tree structure */
+void add_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 06/10] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

The new struct notes_tree encapsulates access to a specific notes tree.
It is provided to allow callers to interface with several different notes
trees simultaneously.

A struct notes_tree * parameter is added to every function in the notes API.
In all cases, NULL can be passed, in which case, a falback "default" notes
tree (declared in notes.c) is used.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   67 ++++++++++++++++++++++++++++++++++++++-----------------------
 notes.h  |   53 +++++++++++++++++++++++++++++++++++-------------
 pretty.c |    4 +-
 3 files changed, 82 insertions(+), 42 deletions(-)

diff --git a/notes.c b/notes.c
index 9581b98..a5d9736 100644
--- a/notes.c
+++ b/notes.c
@@ -50,9 +50,7 @@ struct leaf_node {
 #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
 	(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
 
-static struct int_node root_node;
-
-static int initialized;
+static struct notes_tree default_tree;
 
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n);
@@ -434,14 +432,15 @@ redo:
 	return 0;
 }
 
-void init_notes(const char *notes_ref, int flags)
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	assert(!initialized);
-	initialized = 1;
+	if (!t)
+		t = &default_tree;
+	assert(!t->initialized);
 
 	if (!notes_ref) {
 		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
@@ -451,6 +450,10 @@ void init_notes(const char *notes_ref, int flags)
 			notes_ref = GIT_NOTES_DEFAULT_REF;
 	}
 
+	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->initialized = 1;
+
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
 	    read_ref(notes_ref, object_sha1) ||
 	    get_tree_entry(object_sha1, "", sha1, &mode))
@@ -458,44 +461,56 @@ void init_notes(const char *notes_ref, int flags)
 
 	hashclr(root_tree.key_sha1);
 	hashcpy(root_tree.val_sha1, sha1);
-	load_subtree(&root_tree, &root_node, 0);
+	load_subtree(&root_tree, t->root, 0);
 }
 
-void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
+		const unsigned char *note_sha1)
 {
 	struct leaf_node *l;
 
-	assert(initialized);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
 }
 
-const unsigned char *get_note(const unsigned char *object_sha1)
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1)
 {
 	struct leaf_node *found;
 
-	assert(initialized);
-	found = note_tree_find(&root_node, 0, object_sha1);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
+	found = note_tree_find(t->root, 0, object_sha1);
 	return found ? found->val_sha1 : NULL;
 }
 
-int for_each_note(each_note_fn fn, void *cb_data)
+int for_each_note(struct notes_tree *t, each_note_fn fn, void *cb_data)
 {
-	assert(initialized);
-	return for_each_note_helper(&root_node, 0, 0, fn, cb_data);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
+	return for_each_note_helper(t->root, 0, 0, fn, cb_data);
 }
 
-void free_notes(void)
+void free_notes(struct notes_tree *t)
 {
-	note_tree_free(&root_node);
-	memset(&root_node, 0, sizeof(struct int_node));
-	initialized = 0;
+	if (!t)
+		t = &default_tree;
+	if (t->root)
+		note_tree_free(t->root);
+	free(t->root);
+	free(t->ref);
+	memset(t, 0, sizeof(struct notes_tree));
 }
 
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags)
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
 	const unsigned char *sha1;
@@ -503,10 +518,12 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized)
-		init_notes(NULL, 0);
+	if (!t)
+		t = &default_tree;
+	if (!t->initialized)
+		init_notes(t, NULL, 0);
 
-	sha1 = get_note(object_sha1);
+	sha1 = get_note(t, object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index f67bae8..ea1235f 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,21 @@
 #define NOTES_H
 
 /*
+ * Notes tree object
+ *
+ * Encapsulates the internal notes tree structure associated with a notes ref.
+ * Whenever a struct notes_tree pointer is required below, you may pass NULL in
+ * order to use the default/internal notes tree. E.g. you only need to pass a
+ * non-NULL value if you need to refer to several different notes trees
+ * simultaneously.
+ */
+struct notes_tree {
+	struct int_node *root;
+	char *ref;
+	int initialized;
+};
+
+/*
  * Flags controlling behaviour of notes tree initialization
  *
  * Default behaviour is to initialize the notes tree from the tree object
@@ -10,26 +25,32 @@
 #define NOTES_INIT_EMPTY 1
 
 /*
- * Initialize internal notes tree structure with the notes tree at the given
+ * Initialize the given notes_tree with the notes tree structure at the given
  * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
  * variable is used, and if that is missing, the default notes ref is used
  * ("refs/notes/commits").
  *
- * If you need to re-intialize the internal notes tree structure (e.g. loading
- * from a different notes ref), please first de-initialize the current notes
- * tree by calling free_notes().
+ * If you need to re-intialize a notes_tree structure (e.g. when switching from
+ * one notes ref to another), you must first de-initialize the notes_tree
+ * structure by calling free_notes(struct notes_tree *).
+ *
+ * If you pass t == NULL, the default internal notes_tree will be initialized.
+ *
+ * Precondition: The notes_tree structure is zeroed (this can be achieved with
+ * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
 
-/* Add the given note object to the internal notes tree structure */
-void add_note(const unsigned char *object_sha1,
+/* Add the given note object to the given notes_tree structure */
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
 /* Get the note object SHA1 containing the note data for the given object */
-const unsigned char *get_note(const unsigned char *object_sha1);
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1);
 
 /*
- * Invoke the specified callback function for each note
+ * Invoke the specified callback function for each note in the given notes_tree
  *
  * If the callback returns nonzero, the note walk is aborted, and the return
  * value from the callback is returned from for_each_note().
@@ -43,10 +64,10 @@ const unsigned char *get_note(const unsigned char *object_sha1);
 typedef int each_note_fn(const unsigned char *object_sha1,
 		const unsigned char *note_sha1, const char *note_tree_path,
 		void *cb_data);
-int for_each_note(each_note_fn fn, void *cb_data);
+int for_each_note(struct notes_tree *t, each_note_fn fn, void *cb_data);
 
-/* Free (and de-initialize) the internal notes tree structure */
-void free_notes(void);
+/* Free (and de-initialize) the give notes_tree structure */
+void free_notes(struct notes_tree *t);
 
 /* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
@@ -55,12 +76,14 @@ void free_notes(void);
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
- * If the internal notes structure is not initialized, it will be auto-
+ * If the given notes_tree structure is not initialized, it will be auto-
  * initialized to the default value (see documentation for init_notes() above).
+ * If the given notes_tree is NULL, the internal/default notes_tree will be
+ * used instead.
  *
  * 'flags' is a bitwise combination of the above formatting flags.
  */
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags);
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 771d186..eeb2ebd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,7 +775,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		format_note(commit->object.sha1, sb, git_log_output_encoding ?
+		format_note(NULL, commit->object.sha1, sb, git_log_output_encoding ?
 			    git_log_output_encoding : git_commit_encoding, 0);
 		return 1;
 	}
@@ -1057,7 +1057,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		strbuf_addch(sb, '\n');
 
 	if (fmt != CMIT_FMT_ONELINE)
-		format_note(commit->object.sha1, sb, encoding,
+		format_note(NULL, commit->object.sha1, sb, encoding,
 			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 07/10] Refactor notes concatenation into a flexible interface for combining notes
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

When adding a note to an object that already has an existing note, the
current solution is to concatenate the contents of the two notes. However,
the caller may instead wish to _overwrite_ the existing note with the new
note, or maybe even _ignore_ the new note, and keep the existing one. There
might also be other ways of combining notes that are only known to the
caller.

Therefore, instead of unconditionally concatenating notes, we let the caller
specify how to combine notes, by passing in a pointer to a function for
combining notes. The caller may choose to implement its own function for
notes combining, but normally one of the following three conveniently
supplied notes combination functions will be sufficient:

- combine_notes_concatenate() combines the two notes by appending the
  contents of the new note to the contents of the existing note.

- combine_notes_overwrite() replaces the existing note with the new note.

- combine_notes_ignore() keeps the existing note, and ignores the new note.

A combine_notes function can be passed to init_notes() to choose a default
combine_notes function for that notes tree. If NULL is given, the notes tree
falls back to combine_notes_concatenate() as the ultimate default.

A combine_notes function can also be passed directly to add_note(), to
control the notes combining behaviour for a note addition in particular.
If NULL is passed, the combine_notes function registered for the given
notes tree is used.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  132 +++++++++++++++++++++++++++++++++++---------------------------
 notes.h |   34 +++++++++++++++-
 2 files changed, 106 insertions(+), 60 deletions(-)

diff --git a/notes.c b/notes.c
index a5d9736..19ae492 100644
--- a/notes.c
+++ b/notes.c
@@ -127,55 +127,12 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
 	return NULL;
 }
 
-/* Create a new blob object by concatenating the two given blob objects */
-static int concatenate_notes(unsigned char *cur_sha1,
-		const unsigned char *new_sha1)
-{
-	char *cur_msg, *new_msg, *buf;
-	unsigned long cur_len, new_len, buf_len;
-	enum object_type cur_type, new_type;
-	int ret;
-
-	/* read in both note blob objects */
-	new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
-	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
-		free(new_msg);
-		return 0;
-	}
-	cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
-	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
-		free(cur_msg);
-		free(new_msg);
-		hashcpy(cur_sha1, new_sha1);
-		return 0;
-	}
-
-	/* we will separate the notes by a newline anyway */
-	if (cur_msg[cur_len - 1] == '\n')
-		cur_len--;
-
-	/* concatenate cur_msg and new_msg into buf */
-	buf_len = cur_len + 1 + new_len;
-	buf = (char *) xmalloc(buf_len);
-	memcpy(buf, cur_msg, cur_len);
-	buf[cur_len] = '\n';
-	memcpy(buf + cur_len + 1, new_msg, new_len);
-
-	free(cur_msg);
-	free(new_msg);
-
-	/* create a new blob object from buf */
-	ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
-	free(buf);
-	return ret;
-}
-
 /*
  * To insert a leaf_node:
  * Search to the tree location appropriate for the given leaf_node's key:
  * - If location is unused (NULL), store the tweaked pointer directly there
  * - If location holds a note entry that matches the note-to-be-inserted, then
- *   concatenate the two notes.
+ *   combine the two notes (by calling the given combine_notes function).
  * - If location holds a note entry that matches the subtree-to-be-inserted,
  *   then unpack the subtree-to-be-inserted into the location.
  * - If location holds a matching subtree entry, unpack the subtree at that
@@ -184,7 +141,8 @@ static int concatenate_notes(unsigned char *cur_sha1,
  *   node-to-be-inserted, and store the new int_node into the location.
  */
 static void note_tree_insert(struct int_node *tree, unsigned char n,
-		struct leaf_node *entry, unsigned char type)
+		struct leaf_node *entry, unsigned char type,
+		combine_notes_fn combine_notes)
 {
 	struct int_node *new_node;
 	struct leaf_node *l;
@@ -205,12 +163,11 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (!hashcmp(l->val_sha1, entry->val_sha1))
 					return;
 
-				if (concatenate_notes(l->val_sha1,
-						entry->val_sha1))
-					die("failed to concatenate note %s "
-					    "into note %s for object %s",
-					    sha1_to_hex(entry->val_sha1),
+				if (combine_notes(l->val_sha1, entry->val_sha1))
+					die("failed to combine notes %s and %s"
+					    " for object %s",
 					    sha1_to_hex(l->val_sha1),
+					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->key_sha1));
 				free(entry);
 				return;
@@ -233,7 +190,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 			*p = NULL;
 			load_subtree(l, tree, n);
 			free(l);
-			note_tree_insert(tree, n, entry, type);
+			note_tree_insert(tree, n, entry, type, combine_notes);
 			return;
 		}
 		break;
@@ -243,9 +200,9 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
 	new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
-	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p));
+	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p), combine_notes);
 	*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
-	note_tree_insert(new_node, n + 1, entry, type);
+	note_tree_insert(new_node, n + 1, entry, type, combine_notes);
 }
 
 /* Free the entire notes data contained in the given tree */
@@ -331,7 +288,8 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 				l->key_sha1[19] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
-			note_tree_insert(node, n, l, type);
+			note_tree_insert(node, n, l, type,
+					combine_notes_concatenate);
 		}
 	}
 	free(buf);
@@ -432,7 +390,59 @@ redo:
 	return 0;
 }
 
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
+int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	char *cur_msg, *new_msg, *buf;
+	unsigned long cur_len, new_len, buf_len;
+	enum object_type cur_type, new_type;
+	int ret;
+
+	/* read in both note blob objects */
+	new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
+	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
+		free(new_msg);
+		return 0;
+	}
+	cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
+	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
+		free(cur_msg);
+		free(new_msg);
+		hashcpy(cur_sha1, new_sha1);
+		return 0;
+	}
+
+	/* we will separate the notes by a newline anyway */
+	if (cur_msg[cur_len - 1] == '\n')
+		cur_len--;
+
+	/* concatenate cur_msg and new_msg into buf */
+	buf_len = cur_len + 1 + new_len;
+	buf = (char *) xmalloc(buf_len);
+	memcpy(buf, cur_msg, cur_len);
+	buf[cur_len] = '\n';
+	memcpy(buf + cur_len + 1, new_msg, new_len);
+	free(cur_msg);
+	free(new_msg);
+
+	/* create a new blob object from buf */
+	ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
+	free(buf);
+	return ret;
+}
+
+int combine_notes_overwrite(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	hashcpy(cur_sha1, new_sha1);
+	return 0;
+}
+
+int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	return 0;
+}
+
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
@@ -450,8 +460,12 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 			notes_ref = GIT_NOTES_DEFAULT_REF;
 	}
 
+	if (!combine_notes)
+		combine_notes = combine_notes_concatenate;
+
 	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
 	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->combine_notes = combine_notes;
 	t->initialized = 1;
 
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
@@ -465,17 +479,19 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 }
 
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1)
+		const unsigned char *note_sha1, combine_notes_fn combine_notes)
 {
 	struct leaf_node *l;
 
 	if (!t)
 		t = &default_tree;
 	assert(t->initialized);
+	if (!combine_notes)
+		combine_notes = t->combine_notes;
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
 const unsigned char *get_note(struct notes_tree *t,
@@ -521,7 +537,7 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!t)
 		t = &default_tree;
 	if (!t->initialized)
-		init_notes(t, NULL, 0);
+		init_notes(t, NULL, NULL, 0);
 
 	sha1 = get_note(t, object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index ea1235f..047a282 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,30 @@
 #define NOTES_H
 
 /*
+ * Function type for combining two notes annotating the same object.
+ *
+ * When adding a new note annotating the same object as an existing note, it is
+ * up to the caller to decide how to combine the two notes. The decision is
+ * made by passing in a function of the following form. The function accepts
+ * two SHA1s -- of the existing note and the new note, respectively. The
+ * function then combines the notes in whatever way it sees fit, and writes the
+ * resulting SHA1 into the first SHA1 argument (cur_sha1). A non-zero return
+ * value indicates failure.
+ *
+ * The two given SHA1s are guaranteed to be non-NULL and different.
+ *
+ * The default combine_notes function (you get this when passing NULL) is
+ * combine_notes_concatenate(), which appends the contents of the new note to
+ * the contents of the existing note.
+ */
+typedef int combine_notes_fn(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/* Common notes combinators */
+int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_overwrite(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/*
  * Notes tree object
  *
  * Encapsulates the internal notes tree structure associated with a notes ref.
@@ -13,6 +37,7 @@
 struct notes_tree {
 	struct int_node *root;
 	char *ref;
+	combine_notes_fn *combine_notes;
 	int initialized;
 };
 
@@ -36,14 +61,19 @@ struct notes_tree {
  *
  * If you pass t == NULL, the default internal notes_tree will be initialized.
  *
+ * The combine_notes function that is passed becomes the default combine_notes
+ * function for the given notes_tree. If NULL is passed, the default
+ * combine_notes function is combine_notes_concatenate().
+ *
  * Precondition: The notes_tree structure is zeroed (this can be achieved with
  * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags);
 
 /* Add the given note object to the given notes_tree structure */
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1);
+		const unsigned char *note_sha1, combine_notes_fn combine_notes);
 
 /* Get the note object SHA1 containing the note data for the given object */
 const unsigned char *get_note(struct notes_tree *t,
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox