Git development
 help / color / mirror / Atom feed
* Re: [StGit PATCH v2 1/6] stg mail: Refactor __send_message and  friends
From: Karl Wiberg @ 2009-12-02  6:53 UTC (permalink / raw)
  To: Alex Chiang; +Cc: catalin.marinas, git
In-Reply-To: <20091202004605.7737.2077.stgit@bob.kio>

On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:

> +    if (smtppassword and not smtpuser):
> +        raise Exception('SMTP password supplied, username needed')
> +    if (smtpusetls and not smtpuser):
> +        raise Exception('SMTP over TLS requested, username needed')
> +    if (smtpuser and not smtppassword):
> +        smtppassword = getpass.getpass("Please enter SMTP password: ")

Sorry if I confused you with my earlier explanation; I only meant that
you should use the _form_ "raise Exception('message')", not that you
should change the exception type from CmdException to Exception. If
you try to trigger these errors, I think you'll find that in the case
of CmdException, StGit will print just the message and exit with an
error; whereas for straight Exception, it'll print the full backtrace
as well under the assumption that it's a program bug.

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle

^ permalink raw reply

* Re: [StGit PATCH v2 0/6] add support for git send-email
From: Karl Wiberg @ 2009-12-02  6:46 UTC (permalink / raw)
  To: Alex Chiang; +Cc: catalin.marinas, git
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:

> I also experimented with adding another test case for --git mode,
> basically duplicating t1900-mail.sh, and then adding the --git
> argument wherever it made sense.

Ah, good.

> However, that resulted in failure of the last 3 test cases, which is
> due to the fact that we no longer parse To/Cc/Bcc command line args
> in --git mode, and the resulting mbox file was missing the expected
> recipient addresses.
>
> I played around with that for a while, thinking that I could use git
> send-email --dry-run to do something equivalent, but then realized
> that git send-email's run-run mode is definitely not analogous to
> stg mail's --mbox mode.
>
> The upshot is that in stg mail, --git and --mbox don't interact
> well, and the resulting mbox file will lack the recipients. This
> might be fixed in the future if we teach git send-email how to
> generate mbox files, but then we introduce a versioning problem.

One wild idea: git send-email's --smtp-server flag will accept the
(full) path of a sendmail program; writing such a program, just
capable enough to receive the outgoing emails and dumping them to a
file, should be easy. Another option would be a program that speaks
just enough SMTP to accept the mails. (Incidentally, these two would
be useful in testing stg mail even without the --git option.)

I fully understand if you'd rather get on with scratching your actual
itch, though ...

> So let's just accept this wart for now, and say, if you want an mbox
> file generated, don't use --git. That seems reasonable to me.

Sure.

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle

^ permalink raw reply

* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Junio C Hamano @ 2009-12-02  6:35 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: Junio C Hamano, git
In-Reply-To: <20091202055632.GD31244@Knoppix>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> On Tue, Dec 01, 2009 at 12:42:29PM -0800, Junio C Hamano wrote:
>> 
>> It is somewhat unfortunate that a few changes I liked (e.g. the "debug"
>> bit), even though it was somewhat painful to read them due to coding style
>> differences, were not at the beginning of the series but instead buried
>> after changes that are much bigger and controversial (e.g. [6/8]).
>
> Funny, I considered some other stuff in series much more controversial than
> the 6/8 one.

I didn't mean the line count by "large".  I was referring to the size of
change at the conceptual level.  As Daniel already explained, it has been
one of the design assumption so far that there are built-in mappings from
some common <scheme>:// to backend "helpers".

I am _not_ saying that that particular design assumption must be cast in
stone (nothing is)---that is a totally different matter to be debated.
But the fact that it needs to be debated means it is not "a trivial 8-line
reduction", but rather a large conceptual change (perhaps improvement).

^ permalink raw reply

* Re: [PATCH v2 5/6] run test suite without dashed git-commands in PATH
From: Junio C Hamano @ 2009-12-02  6:25 UTC (permalink / raw)
  To: Matthew Ogilvie; +Cc: Jakub Narebski, git
In-Reply-To: <20091202054956.GA2089@comcast.net>

Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:

> Junio, do you want me to re-roll the 3 bin-wrappers patches to
> include both this and the "@@ vs __" patch, or do you want
> to just add or squash them in from the emails?

I expect that very soon I will be paying much less attention to topics
that will not be in 1.6.6 (in fact, I maybe am already as of tonight), to
save my mental bandwidth to concentrate more on bugfixes and regressions.

Please re-roll and feed me complete patches for anything that are not in
'next', telling me which ones to drop from 'pu' if I have older versions.

Just FYI, earlier you said to me something like "I see you already queued
this to 'pu', ...", but please be aware that being in 'pu' does not mean
that much---not much more than being in the mailing list archive.  They
are fair game for rewriting, replacing and dropping.  I merely keep them
on 'pu' so that it would be easier for me and others to look at them than
having to hunt for them in the mailing list archive.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 5/6] run test suite without dashed git-commands in PATH
From: Matthew Ogilvie @ 2009-12-02  5:49 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio Hamano
In-Reply-To: <m3fx7un7vb.fsf@localhost.localdomain>

On Tue, Dec 01, 2009 at 09:24:46AM -0800, Jakub Narebski wrote:
> Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:
> > +--with-dashes::
> > +	By default tests are run without dashed forms of
> > +	commands (like git-commit) in the PATH (it only uses
> > +	wrappers from TOP/git-bin).  Use this option to include TOP
> > +	in the PATH, which conains all the dashed forms of commands.
> > +	This option is currently implied by other options like --valgrind
> > +	and GIT_TEST_INSTALLED.
> > +
> 
> Shouldn't it be 'TOP/bin-wrappers' and not 'TOP/git-bin'?  
> Shouldn't TOP be explained somewhere, or is it obvious in the context?
> 
> s/conains/contains/.

I've appended a incremental patch for these.

Junio, do you want me to re-roll the 3 bin-wrappers patches to
include both this and the "@@ vs __" patch, or do you want
to just add or squash them in from the emails?

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

From: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
Date: Tue, 1 Dec 2009 22:16:19 -0700
Subject: [PATCH] t/README: fix spelling in --with-dashes documentation

Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
 t/README |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index 8c5d892..dcd3ebb 100644
--- a/t/README
+++ b/t/README
@@ -78,10 +78,11 @@ appropriately before running "make".
 --with-dashes::
 	By default tests are run without dashed forms of
 	commands (like git-commit) in the PATH (it only uses
-	wrappers from TOP/git-bin).  Use this option to include TOP
-	in the PATH, which conains all the dashed forms of commands.
-	This option is currently implied by other options like --valgrind
-	and GIT_TEST_INSTALLED.
+	wrappers from ../bin-wrappers).  Use this option to include
+	the build directory (..) in the PATH, which contains all
+	the dashed forms of commands.  This option is currently
+	implied by other options like --valgrind and
+	GIT_TEST_INSTALLED.
 
 You can also set the GIT_TEST_INSTALLED environment variable to
 the bindir of an existing git installation to test that installation.
-- 
1.6.4.GIT

^ permalink raw reply related

* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Ilari Liusvaara @ 2009-12-02  5:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vskbuwhmy.fsf@alter.siamese.dyndns.org>

On Tue, Dec 01, 2009 at 12:42:29PM -0800, Junio C Hamano wrote:
> 
> It is somewhat unfortunate that a few changes I liked (e.g. the "debug"
> bit), even though it was somewhat painful to read them due to coding style
> differences, were not at the beginning of the series but instead buried
> after changes that are much bigger and controversial (e.g. [6/8]).

Funny, I considered some other stuff in series much more controversial than
the 6/8 one.

And 6/8 large? Its smallest (source code files only) or second smallest (all
files) in number of line changes in the series.

If one looks at 6/8, what it basically does is:
- Alias remote-curl as remote-{http,ftp}{,s} since the special case dispatch
  rules are no more (.gitignore + makefile).
- Remove the special case dispatch rules (transport.c).

Taking diffstat of fixed version of 6/8 (I'll send that later as second round,
possibly with additional fixes):

 .gitignore  |    4 ++++
 Makefile    |   19 +++++++++++++++++++
 transport.c |    8 --------
 3 files changed, 23 insertions(+), 8 deletions(-)

And here's what it does to transport.c:

-       } else if (!prefixcmp(url, "http://")
-               || !prefixcmp(url, "https://")
-               || !prefixcmp(url, "ftp://")) {
-               /* These three are just plain special. */
-               transport_helper_init(ret, "curl");
-#ifdef NO_CURL
-               error("git was compiled without libcurl support.");
-#endif

That's 8 lines killed in transport.c, 4 new binary aliases (yeah, I'm not that
good with makefiles plus this seems to be somewhat nasty case).

-Ilari 

^ permalink raw reply

* Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports
From: Ilari Liusvaara @ 2009-12-02  5:55 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091201192233.GL21299@spearce.org>

On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:
> 
> This flies against every other convention we have.  git:// uses the
> string 'git-upload-pack' and 'git-receive-pack', and so does the
> smart-http code.  We should continue to use the git- prefix here,
> to be consistent, even though by context its clearly implied.

Changed for next round (put the git- -prefixes into names).
 
> Why 'OK'?  Currently remote-helpers return an empty blank line
> to any successful command, not 'OK'.

Changed to "" (i.e. blank line) for next round.
 
> FALLBACK almost makes sense, but ERROR we don't do in the
> the existing helper protocol.  Instead the helper simply
> prints its error message(s) to stderr and does exit(128).
> aka what die() does.

ERROR case changed to exit(128) of helper for next round.

> Why both connect-r and invoke-r?  Why isn't connect-r sufficient
> here?  Isn't it sufficient for any service that runs over git:// ?

Invoke supports those --upload-pack &co (a'la ssh://). connect
doesn't (a'la to git://).

-Ilari

^ permalink raw reply

* Re: [RFC PATCH 6/8] Remove special casing of http, https and ftp
From: Ilari Liusvaara @ 2009-12-02  5:52 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.2.00.0912011351220.14365@iabervon.org>

On Tue, Dec 01, 2009 at 02:15:17PM -0500, Daniel Barkalow wrote:
> On Tue, 1 Dec 2009, Ilari Liusvaara wrote:
> 
> > HTTP, HTTPS and FTP are no longer special to transport code. Also
> > add support for FTPS (curl supports it so it is easy).
> 
> We've been through this extensively, and settled on having a special case 
> for URLs that specify a pure location. That is, the distinction between 
> http and ftp is at the level of how you get to the content for that 
> location, not what you do to interact with it. (Even with webdav or the 
> git-specific smart server support, we use the same detection method on all 
> locations, and ftp simply never has the possibility of having these 
> features detected.)

Currently the only thing about http:// and co git main executable knows is
to pass them to curl remote helper (and print error if compiled with NO_CURL,
possibly causing problems with version desync). Git main executable does
not know any difference between say http:// and ftp:// (the remote helper must
obiviously know the difference, but remote helper is not part of git main
executable).

> It would be fine to add "ftps" to the list of URL schemes that indicate a 
> pure location, except that it's plausible that ftps supports writing, but 
> obviously not by webdav, which is what the push support via curl will 
> attempt, so it's more likely to be confusing than helpful.

remote-curl.c code doesn't seem to do anything stupid with ftps:// that it
wouldn't try with ftp://, and trying to push counts as "stupid" here (and
remember that many FTP servers do allow unencrypted uploads, especially with
authentication and CURL can AFAIK handle that).

-Ilari

^ permalink raw reply

* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Ilari Liusvaara @ 2009-12-02  5:50 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091201193009.GM21299@spearce.org>

On Tue, Dec 01, 2009 at 11:30:09AM -0800, Shawn O. Pearce wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> > 
> > For instance, to support new types of authentication for smart transports
> > without patching client git binaries (SSH has lots of failure modes that
> > are quite nasty to debug) or abusing GIT_PROXY (yuck). 
> 
> So the bulk of this series is about making a proxy for git://
> easier to tie into git?

This is making the "layer 5/6" parts of git:// easier to replace, for whatever
reason that replacement may be desired (and the lower layer is just assumed to
be some kind of full-duplex link).

The part about abusing GIT_PROXY is _very_ nasty hack to be able to layer 6
gateway git smart transports.

The git:// protocol stack is: 
- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)
- git:// (request signaling and data passing)
- TCP/IP (or comparable)

And ssh://:

- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)
- SSH (request signaling, data passing, encrypt & auth).
- TCP/IP (or comparable)

Smart-HTTP:

- RPC versions of git smart transport subprotocols
- HTTP
- TLS (optional)
- TCP/IP (or comparable)

This is about:

- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)
- Some prtocol layer(s) (request signaling, data passing, maybe encrypt & auth, etc...)
- TCP/IP (or comparable)

> Forgive me if I sound stupid, but for gits:// shouldn't that just
> be a matter of git_connect() forking a git-remote-gits process
> linked against openssl?  Or, maybe it just runs `openssl s_client`?

gits:// was just an example. There can be other interesting stuff (I don't
even pretend my imagination is the limit). And I would rather link the gits://
handler to GnuTLS than OpenSSL, but that's seperate matter...

As for "other interesting stuff": Smart transport using Kerberos auth (just
throwing ideas, probably not going to implement that)?

> Why go through all of this effort of making a really generic proxy
> protocol system when the long-term plan is to just ship native
> gits:// support as part of git-core?

gits:// is not actual goal of this series. Its just something to build on
top of it.

-Ilari

^ permalink raw reply

* Re: [Cygwin 1.7] git difftool does not work with Windows kdiff3
From: David Aguilar @ 2009-12-02  5:11 UTC (permalink / raw)
  To: David Antliff; +Cc: cygwin, git
In-Reply-To: <db95995b0911301445v3b757f75y7f4eaf4abaaa9f80@mail.gmail.com>

I added the git list to the CC: list.

On Tue, Dec 01, 2009 at 11:45:35AM +1300, David Antliff wrote:
> 
> git-mergetool works very well with a native Windows (i.e. not Cygwin)
> installation of kdiff3 because it creates its working files in the
> current working directory, usually called
> ./<original-file>.LOCAL.xxxx.<ext> and
> ./<original-file>.REMOTE.xxxx.<ext>. Because these paths are relative
> to the CWD, the non-Cygwin version of kdiff3 handles this fine. E.g:
> 
> kdiff3 --auto --L1 build.xml (Base) --L2 build.xml (Local) --L3
> build.xml (Remote) -o build.xml ./build.xml.BASE.5512.xml
> ./build.xml.LOCAL.5512.xml ./build.xml.REMOTE.5512.xml
> 
> But git-difftool does something slightly different - it creates the
> temporary versions of the file in /tmp with a random prefix, e.g.
> /tmp/Vc0BZy_<original-file>. This causes the Windows version of kdiff3
> to fail to open the file, because the path "/tmp/...." is invalid. In
> my case, the path that would work is "c:/cygwin-1.7/tmp/..." instead:
> 
> kdiff3 --auto --L1 "build.xml (A)" --L2 "build.xml (B)"
> /tmp/Vc0BZy_build.xml build.xml
> 
> It's the  /tmp/... bit that kdiff3 can't understand. On the other
> hand, this command does work:
> 
> kdiff3 --auto --L1 "build.xml (A)" --L2 "build.xml (B)"
> c:/cygwin-1.7/tmp/Vc0BZy_build.xml build.xml
> 
> 
> Perhaps git-difftool should create the temporary file in CWD just like
> git-mergetool, rather than the Cygwin-specific path /tmp?
> 
> I'm using the Windows version of kdiff3 to avoid dependency on the
> graphical X libraries that Cygwin's kdiff3 would require. I think it's
> a fairly common thing to do when working with git on Windows. I can
> see that the Cygwin version of kdiff3 would probably not exhibit this
> problem.
> 
> I imagine the same problem will occur with other Windows versions of
> merge/diff tools.
> 
> I also understand if there's no intention by the Cygwin git
> maintainer to support non-Cygwin gui merge tools, but I don't think
> I'm the only person using them extensively.
> 
> -- David.


git-difftool is built on top of git's GIT_EXTERNAL_DIFF
mechanism.  So this problem is not specific to difftool.

I don't have a general solution for you but I do have
a workaround until a better solution manifests itself.

Here's one workaround:

	$ TMPDIR=.
	$ export TMPDIR

git honors $TMPDIR and that'll make git write files in
the current directory.

Likewise, running `git difftool` should work since the
paths handed off to the difftool should be e.g.
./XXXXXX_build.xml.

That said, I don't know if git dereferences the "." thus
making the workaround invalid.  I haven't tested it.


If that doesn't work then there's another alternative.
You could write a simple kdiff3 wrapper script to
translate the cygwin paths into Windows-native paths.

Drop it in the front of $PATH and it'll magically
work.  Otherwise, set the difftool.kdiff3.path
configuration variable to the path of the wrapper
script and difftool will use your wrapper without
it being in $PATH.

I hope that helps for now.



An aside:

I've always tested git-difftool on Windows using msysgit.
Windows-native kdiff3 is one of the tools that I've tested
and didn't have any problems using msysgit and native kdiff3
together.

cygwin git and msysgit are very different, of course.
It's likely that msysgit does a lot more magic to
make this possible, but I just wanted to throw that
out there.

If anyone knows of a general solution to this problem
then I'm all ears.


Good luck and let us know how it goes,

-- 
		David

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2009, #01; Tue, 01)
From: Daniel Barkalow @ 2009-12-02  4:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaay2tkfh.fsf@alter.siamese.dyndns.org>

On Tue, 1 Dec 2009, Junio C Hamano wrote:

> * sr/vcs-helper (2009-11-18) 12 commits
>   (merged to 'next' on 2009-11-27 at 83268ab)
>  + Add Python support library for remote helpers
>  + Basic build infrastructure for Python scripts
>  + Allow helpers to report in "list" command that the ref is unchanged
>  + Fix various memory leaks in transport-helper.c
>  + Allow helper to map private ref names into normal names
>  + Add support for "import" helper command
>  + Allow specifying the remote helper in the url
>  + Add a config option for remotes to specify a foreign vcs
>  + Allow fetch to modify refs
>  + Use a function to determine whether a remote is valid
>  + Allow programs to not depend on remotes having urls
>  + Fix memory leak in helper method for disconnect

The bottom one here now needs another hunk that reverts a free of the same 
memory (including in cases that this series will need to keep it) that 
snuck in unannounced with ef08ef9ea0a271e5be5844408d2496a946d6e8d9.

diff --git a/transport-helper.c b/transport-helper.c
index 5078c71..d729146 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -72,7 +72,6 @@ static int disconnect_helper(struct transport 
*transport)
                free(data->helper);
                data->helper = NULL;
        }
-       free(data);
        return 0;
 }

^ permalink raw reply related

* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time
From: Tay Ray Chuan @ 2009-12-02  3:15 UTC (permalink / raw)
  To: Martin Storsjö
  Cc: Shawn O. Pearce, git, Nicholas Miell, gsky51, Clemens Buchacher,
	Mark Lodato, Johannes Schindelin
In-Reply-To: <alpine.DEB.2.00.0912011852030.5582@cone.home.martin.st>

Hi,

On Wed, Dec 2, 2009 at 12:59 AM, Martin Storsjö <martin@martin.st> wrote:
> On Tue, 1 Dec 2009, Shawn O. Pearce wrote:
>> The *correct* way to support an arbitrary rewind is to modify the
>> outgoing channel from remote-curl to its protocol engine (client.in
>> within the rpc_service method) to somehow request the protocol engine
>> (aka git-send-pack or git-fetch-pack) to stop and regenerate the
>> current request.
>
> That's a good idea!
>
>> Another approach would be to modify http-backend (and the protocol)
>> to support an "auth ping" request prior to spooling out the entire
>> payload if its more than an http.postBuffer size.  Basically we
>> do what the "Expect: 100-continue" protocol is supposed to do,
>> but in the application layer rather than the HTTP/1.1 layer, so
>> our CGI actually gets invoked.
>
> That's also quite a good idea, especially if it would be done in a way so
> that it's certain that the same curl session will be reused, instead of
> getting a potentially new curl session when using get_active_slot().

I think restarting the read by killing the protocol engine/client and
starting again would be the easier of the two.

Not just that, it would be neater than storing everything that the
protocol engine has spewed out, like Martin's patch does.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer
From: Tay Ray Chuan @ 2009-12-02  2:32 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Martin Storsjö, git, Nicholas Miell, gsky51,
	Clemens Buchacher, Mark Lodato, Johannes Schindelin
In-Reply-To: <7vzl62zisy.fsf@alter.siamese.dyndns.org>

Hi,

On Wed, Dec 2, 2009 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ... and if the current buffer isn't the first one, what do we do?
> [snip]
> What will this result in?  A failed request, then the user increases
> http.postBuffer, and re-runs the entire command?  I am not suggesting the
> code should do it differently (e.g.  retry with a larger buffer without
> having the user to help it).  At least not yet.  That is why my first
> question above was "what do we do?" and not "what should we do?".

I guess that by "we" you're referring to the "normal" users of git?

> I am primarily interested in _documenting_ the expected user experience in
> the failure case, so that people can notice the message, run "git grep" to
> find the above line and then run "git blame" to find the commit to read
> its log message to understand what is going on.

Yes, the code will just fail. As you might suspect, the code won't
attempt to mitigate the failure by doing anything, and would require
intervention on the part of the user.

What the user could do to make this work:

1. Turn off multi-pass authentication and just go with Basic.

2. Allow for persistent curl sessions. In theory, we get a 401 the
first time when we send a GET for info/refs; subsequently, curl knows
what authentication to use, so the POST request *should* take place
without the need for rewinding. In theory.

3. Increase http.postBuffer size in the config.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH v2] Add --track option to git clone
From: Sean Estabrooks @ 2009-12-02  2:08 UTC (permalink / raw)
  To: David Soria Parra; +Cc: git
In-Reply-To: <1259707865-6561-1-git-send-email-sn_@gmx.net>

On Tue,  1 Dec 2009 23:51:03 +0100
David Soria Parra <sn_@gmx.net> wrote:

> The following series adds a --track option to git clone. If the --track option
> is specified only the given remote branch will be received and checked out.

IMHO, the term "track" is already overloaded in Git and this doesn't help make
things clearer.

> It tries to make the following usecase possible:
> Imagine you are working on a project that has 1.x and a 2.x branch. The project
> itself requires a complex setup (webserver, configuration files, etc). Setting up
> 1.x and 2.x branch requires a lot of work, but a developer needs to maintain both.
> He'll use the --track option to clone the 2.x branch into a directory and does the same
> with the 1.x branch, where he setup the project. He can use locally separate repositories
> while still being able to push to just one remote repository.

This is already straightforward in Git without the limitation of tracking only
a single remote branch.   What is the necessity of doing this via the clone command?

  $ git init myrepo
  $ cd myrepo
  $ git remote add -t branch1.x -f origin <URL>
  $ git checkout -t origin/branch1.x

Sean

^ permalink raw reply

* [RFC/PATCHv9 11/11] Refactor notes concatenation into a flexible interface for combining notes
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 3f96ee6..14747bd 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,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;
@@ -450,8 +463,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 +482,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 +540,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/PATCHv9 04/11] Minor style fixes to notes.c
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/notes.c b/notes.c
index 50a4672..812aeb9 100644
--- a/notes.c
+++ b/notes.c
@@ -93,7 +93,7 @@ static void **note_tree_search(struct int_node **tree,
 
 	i = GET_NIBBLE(*n, key_sha1);
 	p = (*tree)->a[i];
-	switch(GET_PTR_TYPE(p)) {
+	switch (GET_PTR_TYPE(p)) {
 	case PTR_TYPE_INTERNAL:
 		*tree = CLR_PTR_TYPE(p);
 		(*n)++;
@@ -195,7 +195,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-	switch(GET_PTR_TYPE(*p)) {
+	switch (GET_PTR_TYPE(*p)) {
 	case PTR_TYPE_NULL:
 		assert(!*p);
 		*p = SET_PTR_TYPE(entry, type);
@@ -257,7 +257,7 @@ static void note_tree_free(struct int_node *tree)
 	unsigned int i;
 	for (i = 0; i < 16; i++) {
 		void *p = tree->a[i];
-		switch(GET_PTR_TYPE(p)) {
+		switch (GET_PTR_TYPE(p)) {
 		case PTR_TYPE_INTERNAL:
 			note_tree_free(CLR_PTR_TYPE(p));
 			/* fall through */
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 05/11] Notes API: get_commit_notes() -> format_note() + remove the commit restriction
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 812aeb9..dddca31 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 7f350bb..81791f5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -703,8 +703,8 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		format_decoration(sb, commit);
 		return 1;
 	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;
 	}
 
@@ -982,8 +982,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

* [RFC/PATCHv9 10/11] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 0d8ff2c..3f96ee6 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 81791f5..047dbb8 100644
--- a/pretty.c
+++ b/pretty.c
@@ -703,8 +703,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		format_decoration(sb, commit);
 		return 1;
 	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;
 	}
 
@@ -982,7 +983,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/PATCHv9 09/11] Notes API: for_each_note(): Traverse the entire notes tree with a callback
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 6a36ff9..0d8ff2c 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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 08/11] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 3c8a6e0..6a36ff9 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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 06/11] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 dddca31..23e62dd 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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 03/11] Add more testcases to test fast-import of notes
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

This patch adds testcases verifying correct behaviour in several scenarios
regarding fast-import of notes:
- using a mixture of 'N' and 'M' commands
- updating existing notes
- concatenation of notes
- 'deleteall' also removes notes
- fanout schemes is added/removed when needed
- git-fast-import's branch unload/reload preserves notes

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t9301-fast-import-notes.sh |  578 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 578 insertions(+), 0 deletions(-)
 create mode 100755 t/t9301-fast-import-notes.sh

diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
new file mode 100755
index 0000000..5a08a76
--- /dev/null
+++ b/t/t9301-fast-import-notes.sh
@@ -0,0 +1,578 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Johan Herland
+#
+
+test_description='test git fast-import of notes objects'
+. ./test-lib.sh
+
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in first commit
+EOF
+
+M 755 inline bar
+data <<EOF
+file bar in first commit
+EOF
+
+M 644 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in first commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in second commit
+EOF
+
+M 755 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in second commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in third commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth commit
+COMMIT
+
+M 755 inline bar
+data <<EOF
+file bar in fourth commit
+EOF
+
+INPUT_END
+
+test_expect_success 'set up master branch' '
+
+	git fast-import <input &&
+	git whatchanged master
+'
+
+commit4=$(git rev-parse refs/heads/master)
+commit3=$(git rev-parse "$commit4^")
+commit2=$(git rev-parse "$commit4~2")
+commit1=$(git rev-parse "$commit4~3")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first notes commit
+COMMIT
+
+M 644 inline $commit1
+data <<EOF
+first note for first commit
+EOF
+
+M 755 inline $commit2
+data <<EOF
+first note for second commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit3
+data <<EOF
+first note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+first note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    first note for fourth commit
+    third commit
+    first note for third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit1
+data <<EOF
+second note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+second note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+second note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+second note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    second note for fourth commit
+    third commit
+    second note for third commit
+    second commit
+    second note for second commit
+    first commit
+    second note for first commit
+EXPECT_END
+
+test_expect_success 'update existing notes with N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $(echo "$commit3" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for third commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for fourth commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^\(..\)\(..\)|\1/\2/|")
+data <<EOF
+pre-prefix of note for fourth commit
+EOF
+
+N inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+third note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+third note for fourth commit
+EOF
+
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    pre-prefix of note for fourth commit
+    prefix of note for fourth commit
+    third note for fourth commit
+    third commit
+    prefix of note for third commit
+    third note for third commit
+    second commit
+    third note for second commit
+    first commit
+    third note for first commit
+EXPECT_END
+
+test_expect_success 'add concatentation notes with M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fifth notes commit
+COMMIT
+
+from refs/notes/test^0
+deleteall
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first commit
+EXPECT_END
+
+test_expect_success 'verify that deleteall also removes notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+sixth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+M 644 inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit1
+data <<EOF
+fourth note for first commit
+EOF
+
+N inline $commit3
+data <<EOF
+fourth note for third commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    fourth note for third commit
+    second commit
+    first commit
+    fourth note for first commit
+EXPECT_END
+
+test_expect_success 'verify that N commands override M commands' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+# Write fast-import commands to create the given number of commits
+fast_import_commits () {
+	my_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		test_tick
+		cat >>"$my_append_to_file" <<INPUT_END
+commit $my_ref
+mark :$my_i
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+commit #$my_i
+COMMIT
+
+M 644 inline file
+data <<EOF
+file contents in commit #$my_i
+EOF
+
+INPUT_END
+	done
+}
+
+# Write fast-import commands to create the given number of notes annotating
+# the commits created by fast_import_commits()
+fast_import_notes () {
+	my_notes_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_note_append=$4
+	test_tick
+	cat >>"$my_append_to_file" <<INPUT_END
+commit $my_notes_ref
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+committing $my_num_commits notes
+COMMIT
+
+INPUT_END
+
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		cat >>"$my_append_to_file" <<INPUT_END
+N inline :$my_i
+data <<EOF
+note for commit #$my_i$my_note_append
+EOF
+
+INPUT_END
+	done
+}
+
+
+rm input expect
+num_commits=400
+# Create lots of commits
+fast_import_commits "refs/heads/many_commits" $num_commits input
+# Create one note per above commit
+fast_import_notes "refs/notes/many_notes" $num_commits input
+# Finally create the expected output from all these notes and commits
+i=$num_commits
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+    note for commit #$i
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success 'add lots of commits and notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that lots of notes trigger a fanout scheme' '
+
+	# None of the entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ge 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+commit1=$(git rev-parse "refs/heads/many_commits")
+commit2=$(git rev-parse "$commit1^")
+commit3=$(git rev-parse "$commit2^")
+commit4=$(git rev-parse "$commit3^")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/many_notes
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+replacing many notes with few notes
+COMMIT
+
+from refs/notes/many_notes^0
+deleteall
+N inline $commit1
+data <<EOF
+note for commit #$num_commits
+EOF
+
+N inline $commit2
+data <<EOF
+note for commit #$(($num_commits - 1))
+EOF
+
+N inline $commit3
+data <<EOF
+note for commit #$(($num_commits - 2))
+EOF
+
+N inline $commit4
+data <<EOF
+note for commit #$(($num_commits - 3))
+EOF
+
+INPUT_END
+
+i=$num_commits
+rm expect
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+EXPECT_END
+	if test $i -gt $(($num_commits - 4))
+	then
+		cat >>expect <<EXPECT_END
+    note for commit #$i
+EXPECT_END
+	fi
+	i=$(($i - 1))
+done
+
+test_expect_success 'remove lots of notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that removing notes trigger fanout consolidation' '
+
+	# All entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ne 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+rm input expect
+num_notes_refs=10
+num_commits=16
+some_commits=8
+# Create commits
+fast_import_commits "refs/heads/more_commits" $num_commits input
+# Create one note per above commit per notes ref
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $num_commits input
+done
+# Trigger branch reloading in git-fast-import by repeating the note creation
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $some_commits input " (2)"
+done
+# Finally create the expected output from the notes in refs/notes/more_notes_1
+i=$num_commits
+while test $i -gt 0
+do
+	note_data="note for commit #$i"
+	if test $i -le $some_commits
+	then
+		note_data="$note_data (2)"
+	fi
+	cat >>expect <<EXPECT_END
+    commit #$i
+    $note_data
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success "add notes to $num_commits commits in each of $num_notes_refs refs" '
+
+	git fast-import --active-branches=5 <input &&
+	GIT_NOTES_REF=refs/notes/more_notes_1 git log refs/heads/more_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_done
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 07/11] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-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 23e62dd..3c8a6e0 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.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

This patch teaches 'git fast-import' to automatically organize note objects
in a fast-import stream into an appropriate fanout structure. The notes API
in notes.h is NOT used to accomplish this, because trying to keep the
fast-import and notes data structures in sync would yield a significantly
larger patch with higher complexity.

Note objects are added to the fast-import tree structure with special mode
bits set, so that they can be recognized and restructured on-demand. The
special mode bits are ignored when generating the containing tree object,
hence the special mode bits are never visible externally.

The code keeps track of the number of note objects per branch through a
simple counter, and if/when the number of notes warrant a different fanout
level, the branch tree is traversed, renaming note objects into the
location dictated by the new fanout level.

Since note objects are stored in the same tree structure as other objects,
the unloading and reloading of a fast-import branches handle note objects
transparently.

Signed-off-by: Johan Herland <johan@herland.net>
---

On Thursday 26 November 2009, Shawn O. Pearce wrote:
> Yea, I agree, I'm not happy with the amount of complex code added
> to implement this.  But I can't say there's a better way to do it
> and still reuse the notes code.  Maybe its just worth breaking away
> from the notes code altogether?  fast-import also implements its
> own pack formatting functions because reusing them from pack-objects
> was just too ugly.

Ok, here is the promised rewrite that does it all within fast-import
instead of reusing the notes code. It's much smaller, both in size,
and in the impact on the existing code. I'm certainly a lot happier
with this patch, than with the previous iteration.


Have fun! :)

...Johan


 fast-import.c          |  141 +++++++++++++++++++++++++++++++++++++++++---
 t/t9300-fast-import.sh |  156 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 276 insertions(+), 21 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index b41d29f..b51ffbc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -161,6 +161,7 @@ Format of STDIN stream:
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
 #define DEPTH_BITS 13
 #define MAX_DEPTH ((1<<DEPTH_BITS)-1)
+#define NOTE_MODE 0170000

 struct object_entry
 {
@@ -245,6 +246,7 @@ struct branch
 	const char *name;
 	struct tree_entry branch_tree;
 	uintmax_t last_commit;
+	unsigned int num_notes;
 	unsigned active : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
@@ -693,6 +695,7 @@ static struct branch *new_branch(const char *name)
 	b->table_next_branch = branch_table[hc];
 	b->branch_tree.versions[0].mode = S_IFDIR;
 	b->branch_tree.versions[1].mode = S_IFDIR;
+	b->num_notes = 0;
 	b->active = 0;
 	b->pack_id = MAX_PACK_ID;
 	branch_table[hc] = b;
@@ -1306,10 +1309,12 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
 	strbuf_grow(b, maxlen);
 	for (i = 0; i < t->entry_count; i++) {
 		struct tree_entry *e = t->entries[i];
-		if (!e->versions[v].mode)
+		unsigned int mode = (unsigned int) e->versions[v].mode;
+		if (!mode)
 			continue;
-		strbuf_addf(b, "%o %s%c", (unsigned int)e->versions[v].mode,
-					e->name->str_dat, '\0');
+		else if ((mode & NOTE_MODE) == NOTE_MODE)
+			mode = (mode & ~NOTE_MODE) | S_IFREG;
+		strbuf_addf(b, "%o %s%c", mode, e->name->str_dat, '\0');
 		strbuf_add(b, e->versions[v].sha1, 20);
 	}
 }
@@ -1860,6 +1865,115 @@ static void load_branch(struct branch *b)
 	}
 }

+static unsigned char convert_num_notes_to_fanout(unsigned int 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;
+	assert(fanout < 20);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	memcpy(path + i, hex_sha1 + j, 40 - j);
+	path[i + 40 - j] = '\0';
+}
+
+static int adjust_num_notes(struct tree_entry *root, const char *p,
+		const unsigned char *sha1)
+{
+	/* Return -1/0/1 indicating how storing sha1 at p affects #notes */
+	struct tree_entry leaf;
+	int delete_note = is_null_sha1(sha1) ? 1 : 0;
+	int nonexisting_note = !(
+		tree_content_get(root, p, &leaf) &&
+		!is_null_sha1(leaf.versions[1].sha1) &&
+		(leaf.versions[1].mode & NOTE_MODE) == NOTE_MODE) ? 1 : 0;
+	return nonexisting_note - delete_note;
+}
+
+static unsigned int 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)
+{
+	struct tree_content *t = root->tree;
+	struct tree_entry *e, leaf;
+	unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len, num_notes = 0;
+	unsigned char sha1[20];
+	char realpath[60];
+	int is_note;
+
+	for (i = 0; i < t->entry_count; i++) {
+		e = t->entries[i];
+		is_note = (e->versions[1].mode & NOTE_MODE) == NOTE_MODE;
+		tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
+		tmp_fullpath_len = fullpath_len;
+
+		if (tmp_hex_sha1_len <= 40 && e->name->str_len >= 2) {
+			memcpy(hex_sha1 + hex_sha1_len, e->name->str_dat,
+				e->name->str_len);
+			if (tmp_fullpath_len)
+				fullpath[tmp_fullpath_len++] = '/';
+			memcpy(fullpath + tmp_fullpath_len, e->name->str_dat,
+				e->name->str_len);
+			tmp_fullpath_len += e->name->str_len;
+			assert(tmp_fullpath_len < 60);
+			fullpath[tmp_fullpath_len] = '\0';
+		} else {
+			assert(!is_note);
+			continue;
+		}
+
+		if (is_note) {
+			num_notes++;
+			assert(tmp_hex_sha1_len == 40);
+			if (get_sha1_hex(hex_sha1, sha1))
+				die("Invalid SHA1 sum %.40s", hex_sha1);
+			construct_path_with_fanout(hex_sha1, fanout, realpath);
+			if (!strcmp(fullpath, realpath))
+				continue; /* note is already in right place */
+
+			/* Rename fullpath to realpath */
+			if (!tree_content_remove(orig_root, fullpath, &leaf))
+				die("Failed to remove path %s", fullpath);
+			if (!leaf.versions[1].mode)
+				die("Path %s not in branch", fullpath);
+			tree_content_set(orig_root, realpath,
+				leaf.versions[1].sha1,
+				leaf.versions[1].mode,
+				leaf.tree);
+		} else if (tmp_hex_sha1_len < 40 && S_ISDIR(e->versions[1].mode)) {
+			/* Found a subdir that may contain a note */
+			num_notes += do_change_note_fanout(orig_root, e,
+				hex_sha1, tmp_hex_sha1_len,
+				fullpath, tmp_fullpath_len, fanout);
+		}
+
+		/* The above may have reallocated the current tree_content */
+		if (t != root->tree)
+			t = root->tree;
+	}
+	return num_notes;
+}
+
+static unsigned int change_note_fanout(struct tree_entry *root,
+		unsigned char fanout)
+{
+	char hex_sha1[40], path[60];
+	return do_change_note_fanout(root, root, hex_sha1, 0, path, 0, fanout);
+}
+
 static void file_change_m(struct branch *b)
 {
 	const char *p = command_buf.buf + 2;
@@ -2010,14 +2124,15 @@ static void file_change_cr(struct branch *b, int rename)
 		leaf.tree);
 }

-static void note_change_n(struct branch *b)
+static void note_change_n(struct branch *b, unsigned char fanout)
 {
 	const char *p = command_buf.buf + 2;
 	static struct strbuf uq = STRBUF_INIT;
 	struct object_entry *oe = oe;
 	struct branch *s;
 	unsigned char sha1[20], commit_sha1[20];
-	uint16_t inline_data = 0;
+	char path[60];
+	uint16_t inline_data = 0, mode;

 	/* <dataref> or 'inline' */
 	if (*p == ':') {
@@ -2080,8 +2195,10 @@ static void note_change_n(struct branch *b)
 			    typename(type), command_buf.buf);
 	}

-	tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
-		S_IFREG | 0644, NULL);
+	construct_path_with_fanout(sha1_to_hex(commit_sha1), fanout, path);
+	b->num_notes += adjust_num_notes(&b->branch_tree, path, sha1);
+	mode = (is_null_sha1(sha1) ? S_IFREG : NOTE_MODE) | 0644;
+	tree_content_set(&b->branch_tree, path, sha1, mode, NULL);
 }

 static void file_change_deleteall(struct branch *b)
@@ -2090,6 +2207,7 @@ static void file_change_deleteall(struct branch *b)
 	hashclr(b->branch_tree.versions[0].sha1);
 	hashclr(b->branch_tree.versions[1].sha1);
 	load_tree(&b->branch_tree);
+	b->num_notes = 0;
 }

 static void parse_from_commit(struct branch *b, char *buf, unsigned long size)
@@ -2213,6 +2331,7 @@ static void parse_new_commit(void)
 	char *committer = NULL;
 	struct hash_list *merge_list = NULL;
 	unsigned int merge_count;
+	unsigned char prev_fanout, new_fanout;

 	/* Obtain the branch name from the rest of our command */
 	sp = strchr(command_buf.buf, ' ') + 1;
@@ -2243,6 +2362,8 @@ static void parse_new_commit(void)
 		load_branch(b);
 	}

+	prev_fanout = convert_num_notes_to_fanout(b->num_notes);
+
 	/* file_change* */
 	while (command_buf.len > 0) {
 		if (!prefixcmp(command_buf.buf, "M "))
@@ -2254,7 +2375,7 @@ static void parse_new_commit(void)
 		else if (!prefixcmp(command_buf.buf, "C "))
 			file_change_cr(b, 0);
 		else if (!prefixcmp(command_buf.buf, "N "))
-			note_change_n(b);
+			note_change_n(b, prev_fanout);
 		else if (!strcmp("deleteall", command_buf.buf))
 			file_change_deleteall(b);
 		else {
@@ -2265,6 +2386,10 @@ static void parse_new_commit(void)
 			break;
 	}

+	new_fanout = convert_num_notes_to_fanout(b->num_notes);
+	if (new_fanout != prev_fanout)
+		b->num_notes = change_note_fanout(&b->branch_tree, new_fanout);
+
 	/* build the tree and the commit */
 	store_tree(&b->branch_tree);
 	hashcpy(b->branch_tree.versions[0].sha1,
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..bf8c509 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1092,9 +1092,12 @@ test_expect_success 'P: fail on blob mark in gitlink' '
 ### series Q (notes)
 ###

-note1_data="Note for the first commit"
-note2_data="Note for the second commit"
-note3_data="Note for the third commit"
+note1_data="The first note for the first commit"
+note2_data="The first note for the second commit"
+note3_data="The first note for the third commit"
+note1b_data="The second note for the first commit"
+note1c_data="The third note for the first commit"
+note2b_data="The second note for the second commit"

 test_tick
 cat >input <<INPUT_END
@@ -1169,7 +1172,45 @@ data <<EOF
 $note3_data
 EOF

+commit refs/notes/foobar
+mark :10
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:10)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1b_data
+EOF
+
+commit refs/notes/foobar2
+mark :11
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:11)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1c_data
+EOF
+
+commit refs/notes/foobar
+mark :12
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:12)
+COMMIT
+
+deleteall
+N inline :5
+data <<EOF
+$note2b_data
+EOF
+
 INPUT_END
+
 test_expect_success \
 	'Q: commit notes' \
 	'git fast-import <input &&
@@ -1224,8 +1265,8 @@ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 notes (:9)
 EOF
 test_expect_success \
-	'Q: verify notes commit' \
-	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	'Q: verify first notes commit' \
+	'git cat-file commit refs/notes/foobar~2 | sed 1d >actual &&
 	test_cmp expect actual'

 cat >expect.unsorted <<EOF
@@ -1235,23 +1276,112 @@ cat >expect.unsorted <<EOF
 EOF
 cat expect.unsorted | sort >expect
 test_expect_success \
-	'Q: verify notes tree' \
-	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	'Q: verify first notes tree' \
+	'git cat-file -p refs/notes/foobar~2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	 test_cmp expect actual'

 echo "$note1_data" >expect
 test_expect_success \
-	'Q: verify note for first commit' \
-	'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'
+	'Q: verify first note for first commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit1 >actual && test_cmp expect actual'

 echo "$note2_data" >expect
 test_expect_success \
-	'Q: verify note for second commit' \
-	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit2 >actual && test_cmp expect actual'
+
+echo "$note3_data" >expect
+test_expect_success \
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar~2`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:10)
+EOF
+test_expect_success \
+	'Q: verify second notes commit' \
+	'git cat-file commit refs/notes/foobar^ | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+100644 blob $commit2
+100644 blob $commit3
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify second notes tree' \
+	'git cat-file -p refs/notes/foobar^^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1b_data" >expect
+test_expect_success \
+	'Q: verify second note for first commit' \
+	'git cat-file blob refs/notes/foobar^:$commit1 >actual && test_cmp expect actual'
+
+echo "$note2_data" >expect
+test_expect_success \
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar^:$commit2 >actual && test_cmp expect actual'

 echo "$note3_data" >expect
 test_expect_success \
-	'Q: verify note for third commit' \
-	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar^:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:11)
+EOF
+test_expect_success \
+	'Q: verify third notes commit' \
+	'git cat-file commit refs/notes/foobar2 | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify third notes tree' \
+	'git cat-file -p refs/notes/foobar2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1c_data" >expect
+test_expect_success \
+	'Q: verify third note for first commit' \
+	'git cat-file blob refs/notes/foobar2:$commit1 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar^`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:12)
+EOF
+test_expect_success \
+	'Q: verify fourth notes commit' \
+	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit2
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify fourth notes tree' \
+	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note2b_data" >expect
+test_expect_success \
+	'Q: verify second note for second commit' \
+	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'

 test_done
--
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 02/11] Rename t9301 to t9350, to make room for more fast-import tests
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

diff --git a/t/t9301-fast-export.sh b/t/t9350-fast-export.sh
similarity index 100%
rename from t/t9301-fast-export.sh
rename to t/t9350-fast-export.sh
-- 
1.6.5.3.433.g11067

^ permalink raw reply


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