Git development
 help / color / mirror / Atom feed
* Re: [RFC PATCH v3 5/8] Support taking over transports
From: Shawn O. Pearce @ 2009-12-07 17:49 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1260116931-16549-6-git-send-email-ilari.liusvaara@elisanet.fi>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> @@ -109,9 +120,21 @@ static struct child_process *get_helper(struct transport *transport)
>  		die("Unable to run helper: git %s", helper->argv[0]);
>  	data->helper = helper;
>  
> +	/* Open the output as FILE* so strbuf_getline() can be used.
> +	   Do this with duped fd because fclose() will close the fd,
> +	   and stuff like disowning will require the fd to remain.
> +
> +	   Set the stream to unbuffered because some reads are critical
> +	   in sense that any overreading will cause deadlocks.
> +	*/
> +        duped = dup(helper->out);

Formatting error here, the line is indented wrong.

> +	if (duped < 0)
> +		die_errno("Can't dup helper output fd");
> +	data->out = xfdopen(duped, "r");
> +	setvbuf(data->out, NULL, _IONBF, 0);

I wonder if this is really a good idea.  Most helpers actually
use a lot of text based IO to communicate.  Disabling buffering
for those helpers to avoid overreading the advertisement from a
connect is a problem.

Maybe we could leave buffering on, but use a handshake protocol
with the helper during connect:

  (1) > "connect git-upload-pack\n"
  (2) < "\n"
  (3) > "begin\n"

During 2 we are still buffered, but the only content on the pipe
should be the single blank line, so we pull that in and the FILE*
buffer should be empty.

After writing message 2 the remote helper blocks reading for the
"begin\n" message 3.  This gives the transport-helper.c code time
to switch off the FILE* and start using raw IO off the pipe before
any data starts coming down the line.

It does mean that the helper may need to run unbuffered IO, but
if the helper is only a smart helper supporting connect then this
isn't really a problem.  It can run buffered IO until connect is
received, switch to unbuffered, and use unbuffered for the single
"begin\n" message it has to consume before it starts writing output
or reading input.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-07 16:43 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, gitster
In-Reply-To: <1260185254-1523-2-git-send-email-johan@herland.net>

Johan Herland <johan@herland.net> wrote:
> +static unsigned char convert_num_notes_to_fanout(uintmax_t num_notes)
> +{
> +	unsigned char fanout = 0;
> +	while ((num_notes >>= 8))
> +		fanout++;
> +	return fanout;
> +}
> +
> +static void construct_path_with_fanout(const char *hex_sha1,
> +		unsigned char fanout, char *path)
> +{
> +	unsigned int i = 0, j = 0;
> +	if (fanout >= 20)
> +		die("Too large fanout (%u)", fanout);

Shouldn't convert_num_notes_to_fanout have a guard to prevent this
case from happening?

-- 
Shawn.

^ permalink raw reply

* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-07 16:41 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, gitster
In-Reply-To: <1260185254-1523-2-git-send-email-johan@herland.net>

Johan Herland <johan@herland.net> wrote:
> As stated in the cover letter, I simply cannot store note information in
> the tree_entry mode bits. So, I chose this somewhat more simple and crude
> approach, which I still think solves the problems quite nicely.

Oh, duh.  Of course you can't.  You lose the note mode when the
tree is flushed to disk, purged from memory, and reloaded later.
Whoops, sorry I missed that during the last round of review.
 
> +static uintmax_t do_change_note_fanout(
> +		struct tree_entry *orig_root, struct tree_entry *root,
> +		char *hex_sha1, unsigned int hex_sha1_len,
> +		char *fullpath, unsigned int fullpath_len,
> +		unsigned char fanout)

I think this function winds up processing all notes twice.  Yuck.

tree_content_set() adds a new tree entry to the end of the current
tree.  So when converting "1a9029b006484e8b9aca06ff261beb2324bb9916"
into "1a" (to go from fanout 0 to fanout 1) we'll place 1a at the
end of orig_root, and this function will walk 1a/ recursively,
examining 1a9029b006484e8b9aca06ff261beb2324bb9916 all over again.

If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?

As we empty out a tree the object will be recycled into a pool of
trees which can be reused at a later point.  It might actually make
sense to build the new tree under a different root.  We won't scan
entries we've moved, and the memory difference should be fairly
small as tree_content_remove() will make a subtree available for
reuse as soon as its empty.  So we're only dealing with a handful
of additional tree objects as we do the conversion.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Ilari Liusvaara @ 2009-12-07 16:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hsz9qxj.fsf@alter.siamese.dyndns.org>

On Sun, Dec 06, 2009 at 11:36:08PM -0800, Junio C Hamano wrote:
> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
> while doing so.  Please sanity check the result.

The conflict resolution seems sane. 

Sorry about leaving lots of codingstyle stuff unfixed.

-Ilari

^ permalink raw reply

* Re: [Admins] Re: git-svn breakage on repository rename
From: Jérôme Petazzoni @ 2009-12-07 16:21 UTC (permalink / raw)
  To: Guido Stevens; +Cc: Eric Wong, wichert, George Kuk, admins, clark.alex, git
In-Reply-To: <4B1BDAC4.9070305@cosent.net>

Guido Stevens wrote:
> Thanks everybody for helping to solve this problem! I can now integrate 
> the analysis of Products.CMFPlone into the overall Plone ecosystem 
> analysis, and it would have been very awkward not to be able to do that 
> because of this renaming issue. Again: thanks.
>   

You're very welcome :-)

^ permalink raw reply

* Re: help: bisect single file from repos
From: walter harms @ 2009-12-07 16:05 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4B1D1A5A.9060004@drmicha.warpmail.net>



Michael J Gruber schrieb:
> walter harms venit, vidit, dixit 07.12.2009 13:59:
>> Hi list,
>> i am new to git (using: git version 1.6.0.2).
> 
> though your git is not that new ;)
> 
>> I would like to bisect a single file but i have only commit id, no tags.
>>
>> Background:
>> I have a copy of the busybox git repos, and i know there is (perhaps) a bug
>> in ash.c.
>>
>> how can i do that ?
> 
> You don't need any tags for bisecting. The man page of git-bisect has
> several examples on how to use it. Do you have a test script which
> exposes the bug?
> 

unfortunately no, the error shows up very nicely when booting my embdedded system
but not else (this is the reason i would to bisect that file only and not busybox
completely). And from the man pages i got the impression that it is only possible the
start with a tag.

i already had the hint that i need to do:
git bisect start bad_commit_id good_commit_id -- ash.c

Ntl, there is one more question, how can i make sure that
i use the right version ? first i toughed  that cherry-pick is the right idea
but it seems that that will apply onyl certain patches ?

re,
 wh

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Nicolas Pitre @ 2009-12-07 15:44 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Ilari Liusvaara, git
In-Reply-To: <20091207210608.6117@nanako3.lavabit.com>

On Mon, 7 Dec 2009, Nanako Shiraishi wrote:

> Quoting Junio C Hamano <gitster@pobox.com>
> 
> > I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
> > while doing so.  Please sanity check the result.
> 
> I see that you changed many "char* variable" to "char *variable", but
> what is the reason for these changes?

The * is an attribute of the variable and not of the type.  This makes 
for clearer code.


Nicolas

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2009, #02; Sat, 05)
From: Shawn O. Pearce @ 2009-12-07 15:37 UTC (permalink / raw)
  To: Martin Storsj?, Junio C Hamano; +Cc: git, rctay89
In-Reply-To: <alpine.DEB.2.00.0912061738580.5582@cone.home.martin.st>

Martin Storsj? <martin@martin.st> wrote:
> On Sun, 6 Dec 2009, Junio C Hamano wrote:
> > 
> > * tr/http-updates (2009-12-01) 3 commits
> >  - Allow curl to rewind the RPC read buffer
> >  - Add an option for using any HTTP authentication scheme, not only basic
> >  - http: maintain curl sessions
> > 
> > There was a discussion on a better structure not to require rewinding in
> > the first place?  I didn't follow it closely...
> 
> I think the conclusion is: Rewinding support isn't strictly necessary, 
> there's a number of mechanisms in both git and curl that should make sure 
> that those cases shouldn't surface. A few of them in curl have an 
> unfortunate conincidence of bugs up until the latest version, though, 
> leaving much fewer mechanisms in place to avoid this.
> 
> Since that patch is quite non-intrusive I think it's a good safeguard, 
> though. What do you think, Tay, keep it or leave it?

I think the conclusion of the thread was that what you have queued
in tr/http-updates is OK as-is.  The patch to grow the postbuffer
to store the entire request wasn't a good idea and got dropped.

-- 
Shawn.

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.6.5.4
From: Todd Zullinger @ 2009-12-07 15:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Schwab, Michael J Gruber, git
In-Reply-To: <7vzl5vbp7y.fsf@alter.siamese.dyndns.org>

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

Junio C Hamano wrote:
>> This does set MAN_BASE_URL unconditionally, pointing to kernel.org.
>
> I'd change this to point at "file://$(htmldir)/", though.

Thanks.  I can delete the mail with an updated patch that I started
over the weekend and didn't finish, which did that as well. ;)

Many thanks for all your hard work Junio.

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Aim Low, Reach Your Goals, Avoid Disappointment.


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

^ permalink raw reply

* Re: help: bisect single file from repos
From: Michael J Gruber @ 2009-12-07 15:08 UTC (permalink / raw)
  To: wharms; +Cc: git
In-Reply-To: <4B1CFC4C.6090406@bfs.de>

walter harms venit, vidit, dixit 07.12.2009 13:59:
> Hi list,
> i am new to git (using: git version 1.6.0.2).

though your git is not that new ;)

> I would like to bisect a single file but i have only commit id, no tags.
> 
> Background:
> I have a copy of the busybox git repos, and i know there is (perhaps) a bug
> in ash.c.
> 
> how can i do that ?

You don't need any tags for bisecting. The man page of git-bisect has
several examples on how to use it. Do you have a test script which
exposes the bug?

Michael

^ permalink raw reply

* Re: [RFC/PATCH] Detailed diagnostic when parsing an object name fails.
From: Matthieu Moy @ 2009-12-07 13:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <vpqiqcrd93l.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>> Perhaps the second step would be to teach the machinery to understand a
>> syntax like "<tree-ish>:./<path>" and have it prefix the path to the
>> current subdirectory from the root of the work tree, and with such an
>> enhancement, the suggestion given by this patch would probably change to
>> "Did you mean 'HEAD:./test-lib.sh'?", but that would be a separate
>> topic.
>
> Exactly. I think this HEAD:./relative-path syntax has been discussed
> here already, but I don't remember the outcome of the discussion. If
> it's ever implemented, my patch, modified as you suggest will help
> users to discover the feature ;-).

I gave it a try, and it seems it's not as easy to implement as it
seems.

The main task is to teach get_sha1_with_mode_1(..., prefix, ...) to
notice a ./ in "HEAD:./filename", and to replace it with prefix, which
is easy. The problem is to get "prefix" reliably. Since
get_sha1_with_mode_1 is called by get_sha1, which doesn't know about
prefix, and which is called from at least 92 places in the code, this
would require changing all these calls to add the "prefix" argument
(and to find out where to get this prefix from).

So, I guess I'll give up :-(. (unless someone either show a great
motivation for the feature, or a magic formula to make the patch
short)

FYI, here's the toy patch I started (works for "git show HEAD:./file",
but not for "git rev-parse HEAD:./file" for example):

diff --git a/cache.h b/cache.h
index c122bfa..a44f06f 100644
--- a/cache.h
+++ b/cache.h
@@ -708,10 +708,10 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
-static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix, int gently);
+static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix)
 {
-	return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
+	return get_sha1_with_mode_1(str, sha1, mode, prefix, 1);
 }
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
diff --git a/revision.c b/revision.c
index 25fa14d..19ddb21 100644
--- a/revision.c
+++ b/revision.c
@@ -944,7 +944,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		local_flags = UNINTERESTING;
 		arg++;
 	}
-	if (get_sha1_with_mode(arg, sha1, &mode))
+	if (get_sha1_with_mode(arg, sha1, &mode, revs->prefix))
 		return -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
@@ -1419,7 +1419,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		unsigned char sha1[20];
 		struct object *object;
 		unsigned mode;
-		if (get_sha1_with_mode(revs->def, sha1, &mode))
+		if (get_sha1_with_mode(revs->def, sha1, &mode, revs->prefix))
 			die("bad default revision '%s'", revs->def);
 		object = get_reference(revs, revs->def, sha1, 0);
 		add_pending_object_with_mode(revs, object, revs->def, mode);
diff --git a/setup.c b/setup.c
index 5792eb7..60c1e08 100644
--- a/setup.c
+++ b/setup.c
@@ -79,7 +79,7 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
 	unsigned char sha1[20];
 	unsigned mode;
 	/* try a detailed diagnostic ... */
-	get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
+	get_sha1_with_mode_1(arg, sha1, &mode, prefix, 0);
 	/* ... or fall back the most general message. */
 	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
 	    "Use '--' to separate paths from revisions", arg);
diff --git a/sha1_name.c b/sha1_name.c
index ca8f9db..330d3fe 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -801,7 +801,7 @@ release_return:
 int get_sha1(const char *name, unsigned char *sha1)
 {
 	unsigned unused;
-	return get_sha1_with_mode(name, sha1, &unused);
+	return get_sha1_with_mode(name, sha1, &unused, NULL);
 }
 
 /* Must be called only when object_name:filename doesn't exist. */
@@ -893,7 +893,7 @@ static void diagnose_invalid_index_path(int stage,
 }
 
 
-int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, const char *prefix, int gently)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -961,11 +961,26 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
-			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
-			if (!gently) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1, object_name);
-				free(object_name);
+			if (filename[0] == '.' && filename[1] == '/') {
+				if (!prefix)
+					prefix = "";
+				char *absfilename = xmalloc((strlen(filename) - 2)
+							    + strlen(prefix) + 1);
+				strcpy(absfilename, prefix);
+				strcat(absfilename, filename+2);
+				ret = get_tree_entry(tree_sha1, absfilename, sha1, mode);
+				if (!gently) {
+					diagnose_invalid_sha1_path(prefix, absfilename,
+								   tree_sha1, object_name);
+					free(object_name);
+				}
+			} else {
+				ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+				if (!gently) {
+					diagnose_invalid_sha1_path(prefix, filename,
+								   tree_sha1, object_name);
+					free(object_name);
+				}
 			}
 			return ret;
 		} else {


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

^ permalink raw reply related

* help: bisect single file from repos
From: walter harms @ 2009-12-07 12:59 UTC (permalink / raw)
  To: git

Hi list,
i am new to git (using: git version 1.6.0.2).

I would like to bisect a single file but i have only commit id, no tags.

Background:
I have a copy of the busybox git repos, and i know there is (perhaps) a bug
in ash.c.

how can i do that ?

re,
 wh

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Erik Faye-Lund @ 2009-12-07 12:57 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Ilari Liusvaara, git
In-Reply-To: <20091207210608.6117@nanako3.lavabit.com>

On Mon, Dec 7, 2009 at 1:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Junio C Hamano <gitster@pobox.com>
>
>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
>> while doing so.  Please sanity check the result.
>
> I see that you changed many "char* variable" to "char *variable", but
> what is the reason for these changes?
>

Documentation/CodingGuidelines:

 - When declaring pointers, the star sides with the variable
   name, i.e. "char *string", not "char* string" or
   "char * string".  This makes it easier to understand code
   like "char *string, c;".

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: Re: [RFC PATCH v2 0/2] git-gui: (un)stage a range of changes at once
From: Heiko Voigt @ 2009-12-07 12:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jeff Epler, git
In-Reply-To: <20091205213613.GG5610@spearce.org>

On Sat, Dec 05, 2009 at 01:36:13PM -0800, Shawn O. Pearce wrote:
> Jeff Epler <jepler@unpythonic.net> wrote:
> > I've found another problem, which I'll work on as soon as I find a
> > chance.
> > 
> > When staging multiple "+" lines preceded by a "-" line that must be
> > turned into context, the converted "-" line must come after *all* the
> > "+" lines, not just the first one.
> 
> So the reason this series got stuck was this message, this bug is
> enough to suggest we shouldn't apply it to my tree yet, so I've
> been waiting for an update on the topic.

In an attempt to help this series forward I tried to reproduce this bug
but were unsuccessfull. It seems that a change like this:

@@ -13,7 +13,9 @@ set appvers {@@GITGUI_VERSION@@}
 set copyright [encoding convertfrom utf-8 {
 Copyright © 2006, 2007 Shawn Pearce, et. al.
 
-This program is free software; you can redistribute it and/or modify
+Blabla
+blubblub
+lalala
 it under the terms of the GNU General Public License as published by
 the Free Software Foundation; either version 2 of the License, or
 (at your option) any later version.

and then trying to stage part of the '+' lines is not enough. Jeff could you
clarify or provide an example?

cheers Heiko

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Nanako Shiraishi @ 2009-12-07 12:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7v7hsz9qxj.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>

> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
> while doing so.  Please sanity check the result.

I see that you changed many "char* variable" to "char *variable", but
what is the reason for these changes?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Sverre Rabbelier @ 2009-12-07 12:04 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Michael Haggerty, git, gitster, git, Johannes.Schindelin,
	bgustavsson
In-Reply-To: <vpqtyw3808q.fsf@bauges.imag.fr>

Heya,

On Mon, Dec 7, 2009 at 12:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
> 08:13:23 2009).

Oh, wow, I've been missing out then :D. Thanks both!

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Matthieu Moy @ 2009-12-07 11:57 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Michael Haggerty, git, gitster, git, Johannes.Schindelin,
	bgustavsson
In-Reply-To: <fabb9a1e0912070326s6cda5c8r442c4816538d0e2a@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
>
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
08:13:23 2009).

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

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Michael Haggerty @ 2009-12-07 11:41 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, gitster, git, Johannes.Schindelin, bgustavsson
In-Reply-To: <fabb9a1e0912070326s6cda5c8r442c4816538d0e2a@mail.gmail.com>

Sverre Rabbelier wrote:
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
> 
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

That command is called "reword".

Michael

^ permalink raw reply

* [RFC/PATCHv10 11/11] Refactor notes concatenation into a flexible interface for combining notes
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 |  135 ++++++++++++++++++++++++++++++++++++---------------------------
 notes.h |   34 +++++++++++++++-
 2 files changed, 109 insertions(+), 60 deletions(-)

diff --git a/notes.c b/notes.c
index efd0007..04f0dae 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 */
@@ -333,7 +290,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);
@@ -434,7 +392,62 @@ 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;
@@ -452,8 +465,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 ||
@@ -467,17 +484,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,
@@ -523,7 +542,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 10/11] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 fallback "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 |    7 +++--
 3 files changed, 84 insertions(+), 43 deletions(-)

diff --git a/notes.c b/notes.c
index de5a847..efd0007 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);
@@ -436,14 +434,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);
@@ -453,6 +452,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))
@@ -460,44 +463,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;
@@ -505,10 +520,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 fe77090..b4882cc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,9 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		format_note(commit->object.sha1, sb, git_log_output_encoding ?
-			    git_log_output_encoding : git_commit_encoding, 0);
+		format_note(NULL, commit->object.sha1, sb,
+			    git_log_output_encoding ? git_log_output_encoding
+						    : git_commit_encoding, 0);
 		return 1;
 	}
 
@@ -1095,7 +1096,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 08/11] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 79bfa24..110404a 100644
--- a/notes.c
+++ b/notes.c
@@ -379,12 +379,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)
@@ -398,7 +399,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;
@@ -406,7 +407,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 09/11] Notes API: for_each_note(): Traverse the entire notes tree with a callback
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 110404a..de5a847 100644
--- a/notes.c
+++ b/notes.c
@@ -341,6 +341,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];
@@ -388,6 +483,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 07/11] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 e0dfa24..79bfa24 100644
--- a/notes.c
+++ b/notes.c
@@ -368,6 +368,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 06/11] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 5590414..e0dfa24 100644
--- a/notes.c
+++ b/notes.c
@@ -341,13 +341,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;
 
@@ -380,15 +392,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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 05/11] Notes API: get_commit_notes() -> format_note() + remove the commit restriction
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-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 57d5cdc..5590414 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) {
 				if (!S_ISDIR(entry.mode))
@@ -344,12 +343,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);
@@ -357,9 +356,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;
@@ -372,7 +371,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";
@@ -391,7 +390,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 8f5bd1a..fe77090 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,8 @@ static size_t format_commit_one(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;
 	}
 
@@ -1095,8 +1095,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.5.3.433.g11067

^ 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