* Re: Testing if a certain commit is in the current branch
From: Johannes Schindelin @ 2010-01-27 8:56 UTC (permalink / raw)
To: Frans Pop; +Cc: git
In-Reply-To: <201001270819.39819.elendil@planet.nl>
Hi,
On Wed, 27 Jan 2010, Frans Pop wrote:
> I'm currently using the following command for this:
> git log --pretty=format:%H | grep -q '^<commit id>"
>
> Problem is that with large repos that can be quite slow.
>
> Is there a faster way to do the test?
You are more likely looking for "git branch --contains". But this gives
you _more_ information.
Ciao,
Dscho
^ permalink raw reply
* [PATCH v2 11/12] Makefile: list standalone program object files in PROGRAM_OBJS
From: Jonathan Nieder @ 2010-01-27 9:07 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126155423.GL4895@progeny.tock>
Because of new commands like git-remote-http, the OBJECTS list
contains fictitious objects such as remote-http.o. Thus any
out-of-tree rules that require all $(OBJECTS) to be buildable
are broken. Add a list of real program objects to avoid this
problem.
To avoid duplication of effort, calculate the command list in
the PROGRAMS variable using the expansion of PROGRAM_OBJS.
This calculation occurs at the time $(PROGRAMS) is expanded,
so later additions to PROGRAM_OBJS will be reflected in it,
provided they occur before the build rules begin on line 1489.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The previous version of this patch had a bug: I assumed that
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
would add to PROGRAMS a list of command names derived from the value
of PROGRAM_OBJS at the time that line is first read; but in fact, what
matters is the value of PROGRAM_OBJS at expansion time.
More importantly, my process had a bug: I didn’t try 'make install'
before sending. Sorry about that. Here’s a fixed patch, at least.
I have pushed out this fix to
git://repo.or.cz/git/jrn.git autodep-rebased
git://repo.or.cz/git/jrn.git autodep
The former has been rerolled to include the fixed patch 11; the
latter includes a fixup commit at the end.
Thanks to Junio for the report.
Makefile | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/Makefile b/Makefile
index df36173..630687f 100644
--- a/Makefile
+++ b/Makefile
@@ -341,6 +341,7 @@ COMPAT_CFLAGS =
COMPAT_OBJS =
LIB_H =
LIB_OBJS =
+PROGRAM_OBJS =
PROGRAMS =
SCRIPT_PERL =
SCRIPT_PYTHON =
@@ -390,12 +391,15 @@ EXTRA_PROGRAMS =
# ... and all the rest that could be moved out of bindir to gitexecdir
PROGRAMS += $(EXTRA_PROGRAMS)
-PROGRAMS += git-fast-import$X
-PROGRAMS += git-imap-send$X
-PROGRAMS += git-shell$X
-PROGRAMS += git-show-index$X
-PROGRAMS += git-upload-pack$X
-PROGRAMS += git-http-backend$X
+
+PROGRAM_OBJS += fast-import.o
+PROGRAM_OBJS += imap-send.o
+PROGRAM_OBJS += shell.o
+PROGRAM_OBJS += show-index.o
+PROGRAM_OBJS += upload-pack.o
+PROGRAM_OBJS += http-backend.o
+
+PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
TEST_PROGRAMS_NEED_X += test-ctype
@@ -1139,11 +1143,12 @@ else
REMOTE_CURL_PRIMARY = git-remote-http$X
REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X
REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
- PROGRAMS += $(REMOTE_CURL_NAMES) git-http-fetch$X
+ PROGRAM_OBJS += http-fetch.o
+ PROGRAMS += $(REMOTE_CURL_NAMES)
curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
ifeq "$(curl_check)" "070908"
ifndef NO_EXPAT
- PROGRAMS += git-http-push$X
+ PROGRAM_OBJS += http-push.o
endif
endif
ifndef NO_EXPAT
@@ -1163,7 +1168,7 @@ endif
EXTLIBS += -lz
ifndef NO_POSIX_ONLY_PROGRAMS
- PROGRAMS += git-daemon$X
+ PROGRAM_OBJS += daemon.o
endif
ifndef NO_OPENSSL
OPENSSL_LIBSSL = -lssl
@@ -1670,9 +1675,8 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
-GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
- git.o http.o http-walker.o remote-curl.o \
- $(patsubst git-%$X,%.o,$(PROGRAMS))
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
+ git.o http.o http-walker.o remote-curl.o
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
--
1.6.6
^ permalink raw reply related
* Re: [PATCH 0/4] Fix various integer overflows
From: Stephen R. van den Berg @ 2010-01-27 8:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7vk4v4zlhg.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
>Looks trivially correct; thanks.
I'm just curious, but is this based on an actual bug which someone
experienced, or is this just based on mere theoretical code analysis?
--
Sincerely,
Stephen R. van den Berg.
"Am I paying for this abuse or is it extra?"
^ permalink raw reply
* Re: 3 question about how to use Git .
From: Crane Cai @ 2010-01-27 9:32 UTC (permalink / raw)
To: honglei junan; +Cc: git
In-Reply-To: <e78dc7d71001270035p4e235f18o59029b93bea25d4e@mail.gmail.com>
On Wed, Jan 27, 2010 at 04:35:53PM +0800, honglei junan wrote:
> hi,i have three questions seem not not appear in GitFaq,yet confuse me much :
> *) since I've already commit many patches before telling git my name
> and email,should i roll back and recommit all patches to make my name
> in repository?
One way I do:
#checkout your commits
git format-patch since-your-commit
#reset head to begin
git reset --hard since-your-commit
#change your commits in patches, then am
git am your-patches
> *) when cloning a git repository,the network is broken down.How could
> i went on the clone process when the network being OK ?
> since the download speed is very low, i really don't want to remove
> all things and download them again.
you need not to remove, just redo clone.
--
Best Regards,
- Crane
^ permalink raw reply
* Re: 3 question about how to use Git .
From: Jeff King @ 2010-01-27 9:52 UTC (permalink / raw)
To: honglei junan; +Cc: git
In-Reply-To: <e78dc7d71001270035p4e235f18o59029b93bea25d4e@mail.gmail.com>
On Wed, Jan 27, 2010 at 04:35:53PM +0800, honglei junan wrote:
> hi,i have three questions seem not not appear in GitFaq,yet confuse me much :
> *) since I've already commit many patches before telling git my name
> and email,should i roll back and recommit all patches to make my name
> in repository?
You can use filter-branch to do it automatically. Something like:
export BAD_EMAIL='The Wrong Email in Your Commits'
export GOOD_NAME='Your Correct Name'
export GOOD_EMAIL='Your Correct Email'
git filter-branch --env-filter '
if test "$GIT_COMMITTER_EMAIL" = "$BAD_EMAIL"; then
GIT_COMMITTER_NAME=$GOOD_NAME
GIT_COMMITTER_EMAIL=$GOOD_EMAIL
fi
if test "$GIT_AUTHOR_EMAIL" = "$BAD_EMAIL"; then
GIT_AUTHOR_NAME=$GOOD_NAME
GIT_AUTHOR_EMAIL=$GOOD_EMAIL
fi
'
which will rewrite just the commits with the problem. Note that this
will rewrite the history of those commits. If you've shared the old ones
with others, they will see them as entirely new commits instead of
replacements.
> *) i create a new branch ms_port,and then commit several patches like,
> commit 1->commit 2->commit 3->commit 4
> now i just want to change commit2,without change any others,how could
> i do it?since i only change the local repository,i think this should
> be allowed.
Try "git rebase -i commit1".
> *) when cloning a git repository,the network is broken down.How could
> i went on the clone process when the network being OK ?
> since the download speed is very low, i really don't want to remove
> all things and download them again.
Git doesn't currently support restartable clone. It's something that
would be nice to have, but there are some technical issues that make it
more difficult than simply starting where we left off (namely that the
pack that is being downloaded via the git protocol is generated on the
fly). You can find some previous discussions by searching the list for
"restartable clone".
-Peff
^ permalink raw reply
* Re: [PATCH 0/4] Fix various integer overflows
From: Ilari Liusvaara @ 2010-01-27 9:57 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: Junio C Hamano, git
In-Reply-To: <20100127085952.GA21535@cuci.nl>
On Wed, Jan 27, 2010 at 09:59:52AM +0100, Stephen R. van den Berg wrote:
> Junio C Hamano wrote:
> >Looks trivially correct; thanks.
>
> I'm just curious, but is this based on an actual bug which someone
> experienced, or is this just based on mere theoretical code analysis?
Theoretical at first, but I did construct packfile that hits one of
those overflows (the one in patch_delta(), 32 bits only).
In real world, hitting this bug would require hitting exactly 2^32-1
byte file, and that is quite rare size for file.
And what can happen with them in real world git usage is different
than what can happen with them if packs are suitably manipulated
("transport streams" and bundles both contain packs in them).
-Ilari
^ permalink raw reply
* Re: Testing if a certain commit is in the current branch
From: Jonathan Nieder @ 2010-01-27 10:19 UTC (permalink / raw)
To: Frans Pop; +Cc: Junio C Hamano, git
In-Reply-To: <201001270936.14935.elendil@planet.nl>
Frans Pop wrote:
> The use case is a wrapper script for kernel builds in which some variation
> are needed (due to changes in the build system) when building older
> kernels, e.g. during bisections or when building older stable updates.
> So I test for the presence of commits that introduced relevant changes.
Neat idea. How about this (untested)?
git rev-list ^HEAD --no-walk commit1 commit2 ...
^ permalink raw reply
* Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
From: Jeff King @ 2010-01-27 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vockgzliv.fsf@alter.siamese.dyndns.org>
On Tue, Jan 26, 2010 at 11:58:00AM -0800, Junio C Hamano wrote:
> > The first one is that @{usptream} silently becomes @{0}. I think
> > we need to double-check whether approxidate found absolutely nothing,
> > and complain if that is the case.
>
> This is not a new problem introduced by Dscho's @{u} series; it was there
> even before 861f00e (fix reflog approxidate parsing bug, 2008-04-30).
True, though now that there is something besides an approxidate to
misspell, it is slightly worse. :)
> Introduce approxidate_careful() that takes an optional pointer to an
> integer, that gets assigned 1 when the input does not make sense as a
> timestamp.
A minor nit, but wouldn't:
int approxidate_careful(const char *str, unsigned long *out);
returning an error code be the more usual pattern for a function with
error-plus-output (your approxidate wrapper would have to be a function then,
not a macro)?
> As I am too lazy to fix all the callers that use approxidate(), most of
> the callers do not take advantage of the error checking, but convert the
> code to parse reflog to use it as a demonstration.
I think that is fine, and was the approach I was also going to take.
Approxidate was meant to be "try to make sense of anything". It is
mainly a problem here because we are combining it with other input, and
it is hard to tell a misspelling of that other input from a nonsensical
approxidate. Now that the _careful infrastructure is there, we can fix
other callsites if people care.
> @@ -413,8 +413,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> } else if (0 <= nth)
> at_time = 0;
> else {
> + int errors = 0;
> char *tmp = xstrndup(str + at + 2, reflog_len);
> - at_time = approxidate(tmp);
> + at_time = approxidate_careful(tmp, &errors);
> + if (errors)
> + die("Bogus timestamp '%s'", tmp);
> free(tmp);
I was just going to "return -1" here, which yields:
$ git show @{bogosity}
fatal: ambiguous argument '@{bogosity}': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
instead of
$ git show @{bogosity}
fatal: Bogus timestamp 'bogosity'
I can't imagine anybody being upset that they had a path '@{usptream}'
in their repository which was prematurely interpreted as a bogus ref
(especially since it is _always_ a ref in the current behavior), but it
just seemed more consistent with the rest of get_sha1_basic, which
generally does not die.
On the other hand, I think the error message the user sees in your case
is probably more helpful.
> +test_expect_success '@{30.years.ago} shows old' '
> + check_at @{30.years.ago} one
Side note: I chose this because we needed to go back from the current
time beyond where test_tick would place the commit. Which means this
test has a 2035 bug. :)
I had originally had 100.years.ago, but we seem to have some data-type
issues with our date handling. We represent seconds-since-epoch as
unsigned long, which means:
- even though approxidate handles it internally, we can't represent
dates earlier than the epoch. If you simply store approxidate's
output as a signed time_t, as test-date does, it does work back to
1901. But the reflog code treats it as unsigned, so 100.years.ago,
though representable, is considered to be far in the future.
- on many platforms ulong is 32 bits, meaning we have 2038 problems
(though because it's unsigned, I am not sure if we actually have
2106 problems; I suspect it may be a mix because of the way
different parts of the system use time_t versus ulong). Presumably
time_t will move to 64 bits in the next decade or two, and hopefully
ulong along with it.
Now obviously neither is a pressing issue. Probably nobody is importing
any pre-epoch commits, and we have a few decades to worry about future
issues. But I did legitimately see the bug when trying to guess a "long
time ago" value. It might be nice to use a signed time_t, and to deal
with the overflow (even if it is just to barf and say "bad time" instead
of silently producing wrong, future results).
-Peff
^ permalink raw reply
* show/diff --check clarification needed
From: Yann Dirson @ 2010-01-27 10:41 UTC (permalink / raw)
To: git
I am experiencing an unexpected behaviour, related to whitespace
(non-)checking.
In a my local repository I have a pre-commit hook checking for "git
diff-index --check --cached" (borrowed from the sample template), and
the server I push to does a "git diff-tree --check" on each pushed
commit to make sure someone who forgot the pre-commit hook does not
push a whitespace-unclean commit by error.
Now I had to import some 3rd-party stuff (an eclipse deltapack if you
wonder) which includes CRLF line-endings, but must not get modified
because of checksums. So I added the following .gitattributes to the
tree where I import this stuff:
$ cat deltapack/.gitattributes
* binary
.gitattributes -binary
That is just fine for the pre-commit hook, and "git show --check" on
it in my local repository reports no problems.
Now when I attempt to push, the update hook refuses my commit; and if
I run the same "git show --check" on the server (in the bare repo
where the ref has not been updated by the push, but where the pushed
commit has not been pruned yet), I get the complaints which I have
intended the .gitattributes to silent.
What happens is apparently that --check does not know which repository
revision to take .gitattributes from, and takes them from working
tree. Indeed, removing deltapack/.gitattributes from my worktree and
rerunning "show --check" on that unmodified commit does trigger the errors.
While having "diff-tree --check" defaulting to looking at working tree
by default may make sense, it would seem more natural for "show" to
use by default those of the commit we're looking at - and one could
argue that "diff-tree" could use the positive treeish for that matter
as well. But defaulting is one matter, it would be really useful to
be able to specify from where the attributes should be taken for any
given operation. Did not look at the code - anyone knows how easy
that would be ?
Unless I missed something, I suggest the following plan:
- document in maint that --check only takes worktree into account when
looking for .gitattributes, and more globally add a note to the
gitattributes manpage to explicitely say that too.
- add a global flag to allow something like
"git --attributes-tree=<treeish> <command>"
- adjust defaults to agreed-upon values
- add any config entries that would be meaningful
It may just be me having badly written the update hook - here is the
relevant part for reference. Under the "refs/heads/*,commit)" case of
the sample update hook, I have something like:
# checks to be done in all commits
# exclude all commits previously-reachable by any ref
exclude_oldrevs=$( git for-each-ref --format='^%(refname)' )
for rev in $(git rev-list $exclude_oldrevs $newrev); do
[...]
# special case for root commits
if git rev-parse HEAD^ >/dev/null 2>&1; then
ref=""
else
# use an empty tree to compare against
ref=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi
# checks for whitespace
if ! git diff-tree --check $ref $rev >/dev/null; then
echo "*** Whitespace problems in $rev" >&2
fails_whitespace=1
fi
[...]
done
^ permalink raw reply
* Re: [PATCH not-for-mainline] Implement git-vcs-p4
From: Tor Arvid Lund @ 2010-01-27 11:18 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.2.00.1001251628431.14365@iabervon.org>
On Mon, Jan 25, 2010 at 10:35 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> This is probably not particularly appropriate for mainline
> application, and is somewhat buggy, not extensively tested, and
> incomplete. The push support is also currently based on a transport helper
> export design that isn't upstream and I don't like any more; a better
> design is probably to have the core send an "export" command and then a
> gfi stream, but I haven't worked on this.
>
> It has two implementations of the interaction with the Perforce
> server: one that uses the command-line client (and therefore makes a
> ton of separate connections to the server) and one that uses the
> (closed source, vaguely licensed) C++ API. The former does not support
> everything used in push/submit correctly at this point.
>
> It also adds support to the Makefile for building C++ object files and
> linking with a C++ linker. It should be easy to omit entirely for
> builds that don't use p4, and it's at least somewhat out of the way.
>
> The biggest flaw currently is that it doesn't save its analysis of the
> structure of the history, and doesn't have a way to push it out of memory,
> so a long or complex history will run you out of memory or will take a
> long time to do an incremental fetch.
>
> Fetch features:
>
> - following integrations (with some guessing)
> - finding other branches of a codeline
>
> Push features (only with the C++ API):
>
> - works if you don't do anything at all complicated
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
<snip>
Hi, and thank you for posting this.
I tried applying it to current master, and got it to compile using the
p4 c++ api.
However, I'm having trouble getting it to run. This is most certainly
my own fault, and I'm guessing it has to do with my .git/config file
setup.
I tried doing 'git init', and making a .git/config file like so:
------------
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
[vcs-p4]
port = perforce.mycompany.com:1666
client = toral
[remote "origin"]
vcs = p4
codeline = //depot/path/to/my/existing/test/project
------------
Then, I did 'git fetch', and got a seg fault. I got around it by
commenting out a line:
diff --git a/transport.c b/transport.c
index 7714fdb..5b404f7 100644
--- a/transport.c
+++ b/transport.c
@@ -924,7 +924,7 @@ struct transport *transport_get(struct remote
*remote, const char *url)
ret->url = url;
/* In case previous URL had helper forced, reset it. */
- remote->foreign_vcs = NULL;
+/* remote->foreign_vcs = NULL;*/
/* maybe it is a foreign URL? */
if (url) {
-------------
So - now I get this:
$ GIT_TRANSPORT_HELPER_DEBUG=1 git fetch
Debug: Remote helper: -> capabilities
Debug: Remote helper: Waiting...
Debug: Remote helper: <- import
Debug: Got cap import
Debug: Remote helper: Waiting...
Debug: Remote helper: <- export
Debug: Got cap export
Debug: Remote helper: Waiting...
Debug: Remote helper: <-
Debug: Capabilities complete.
Debug: Remote helper: Waiting...
Debug: Remote helper: <- ? refs/p4/depot/path/to/my/existing/test/project
Debug: Remote helper: Waiting...
Debug: Remote helper: <- ? refs/p4/depot/path/to/my/existing/test/project
Debug: Remote helper: Waiting...
Debug: Remote helper: <-
Debug: Read ref listing.
fatal: Couldn't find remote ref HEAD
-------------
I also tried setting vcs-p4.findbranches to 'true'. The only
difference in the output, is that the "<- ? refs/p4/..." line is just
output once.
So if anyone has a clue for me, I shall, well, cease to be clueless.
-Tor Arvid-
^ permalink raw reply related
* Re: git notes issues (was: cvs revision number -> git commit name?)
From: Johan Herland @ 2010-01-27 11:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Aaron Crane, Hallvard B Furuseth
In-Reply-To: <7vpr4wq6w3.fsf@alter.siamese.dyndns.org>
On Wednesday 27 January 2010, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > The notes feature is still very new, and there are still outstanding
> > patches to be merged, but the basics are there in v1.6.6.
>
> By the way, we should seriously rethink how notes should propagate
> through rebases and amends. I've been using this in my post-applypatch
> hook lately:
>
> [...]
>
> A few observations I made myself so far:
>
> - I used to fix minor issues (styles, decl-after-stmt, etc.) using
> rebase-i long after running "am" in bulk, but these days I find myself
> going back to my "inbox" and fix them in MUA; this is only because I
> know these notes do not propagate across rebases and
> amends---adjusting the workflow to the tool's limitation is not very
> good.
Agreed. I simply haven't had time to look much into this yet. We should
probably add options (both command-line options and config variables) to
'git rebase', 'git commit --amend', 'git cherry-pick', etc. for bringing
notes across a commit rewrite. We should probably also add "git notes
move/copy <old_object> <new_object>" subcommands to make the same operations
available to scripts (and users).
> - The interface to tell tools to use which notes ref to use should be
> able to say "these refs", not just "this ref" i.e. GIT_NOTES_REF=a:b
> just like PATH=a:b:c...); I am fairly certain that we would want to
> store different kind of information in separate notes trees and
> aggregate them, as we gain experience with notes.
Agreed.
> - There should be an interface to tell tools to use which notes refs via
> command line options; "!alias" does not TAB-complete, and "git lgm"
> above doesn't, either. "git log --notes=notes/amlog
> --notes=notes/other" would probably be the way to go.
Agreed.
> - While reviewing the "inbox", I sometimes wonder if I applied a message
> to somewhere already, but there is no obvious way to grep in the notes
> tree and get the object name that a note is attached to. Of course I
> know I can "git grep -c johan@herland.net notes/amlog" and it will
> give me something like:
>
> notes/amlog:65807ee697a28cb30b8ad38ebb8b84cebd3f255d:1
> notes/amlog:c789176020d6a008821e01af8b65f28abc138d4b:1
>
> but this won't scale and needs scripting to mechanize, once we start
> rebalancing the notes tree with different fan-outs. The end user (me
> in this case) is interested in "set of objects that match this grep
> criteria", not "the pathnames the notes tree's implementation happens
> to use to store notes for them in the hierarchy".
Agreed. Should add a "git notes grep" subcommand.
I hope that once the updated notes API is in (I'll send an update to what's
in 'pu' shortly), people will have the tools they need to start integrating
notes with their pet Git features. As you illustrate perfectly above, there
are many corners to be smoothed out before this is well integrated with the
other Git tools.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
From: Jeff King @ 2010-01-27 11:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vpr4wy2lk.fsf@alter.siamese.dyndns.org>
On Tue, Jan 26, 2010 at 01:32:07PM -0800, Junio C Hamano wrote:
> I don't know if it deals with things like "@{-1}@{u}@{now}"; the users
> should have every right to expect it to, but I didn't consciously try to
> make that work with this patch.
Nice. This also fixes "git log -g @{-1}". Static uses like "git show
@{u}@{1.week.ago}" and "git show @{-1}@{1.week.ago}" were already fine,
so I think the bug was really confined to the reflog walker (and your
fix is therefore correct).
Using "git show @{-1}@{u}" is still broken, though.
I tried tracing the parsing through get_sha1_basic and
interpret_branch_name, but it's pretty confusing. Especially as we seem
to deal with @{upstream}, @{now}, and @{-1} at different places.
I think the patch below does what we want, but the whole thing feels
overly complicated to me, especially with the split of parsing @{...}
between get_sha1_basic and interpret_branch_name. I guess we have spots
that don't take reflogs but do take branch names, but I think the code
would be much simpler if the syntax were parsed in one place, and then
we threw out or complained about bogus semantics (like "checkout
@{now}").
---
diff --git a/sha1_name.c b/sha1_name.c
index ed4c028..ef8f3fa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -881,8 +881,28 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
if (!len)
return len; /* syntax Ok, not enough switches */
- if (0 < len)
- return len; /* consumed from the front */
+ if (0 < len && len == namelen)
+ return len; /* consumed all */
+ else if (0 < len) {
+ /* we have extra data, which might need further processing */
+ struct strbuf tmp = STRBUF_INIT;
+ int used = buf->len;
+ int ret;
+
+ strbuf_add(buf, name + len, namelen - len);
+ ret = interpret_branch_name(buf->buf, &tmp);
+ /* that data was not interpreted, remove our cruft */
+ if (ret < 0) {
+ strbuf_setlen(buf, used);
+ return len;
+ }
+ strbuf_reset(buf);
+ strbuf_addbuf(buf, &tmp);
+ strbuf_release(&tmp);
+ /* tweak for size of {-N} versus expanded ref name */
+ return ret - used + len;
+ }
+
cp = strchr(name, '@');
if (!cp)
return -1;
^ permalink raw reply related
* Re: git commit --cleanup and templates
From: Jeff King @ 2010-01-27 11:48 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git
In-Reply-To: <hj7bh9$adp$1@ger.gmane.org>
On Wed, Jan 20, 2010 at 05:37:47PM +0100, Sebastian Schuberth wrote:
> when I recently for the first time used commit templates, I was
> surprised to see that git by default strips leading empty lines that
> I deliberately added to the template file when opening the template
> in the editor. I understand that I could use a different cleanup
> mode, but in fact I *want* git to clean everything, but only *after*
> I finished editing the commit message, not already when opening the
> editor.
>
> Before submitting a patch, I'd like to hear what other people think
> about this, or whether I'm overseeing something.
Nobody has responded, so I doubt anybody has strong objections (or maybe
they just didn't read your message. :) ).
Personally, the behavior you are proposing makes more sense to me, and I
don't think you will really be hurting anybody by changing it. You will
not affect the final commit message, but only what the user sees in
their editor. And if people are really attached to not having extra
whitespace in the editor, then they can simply clean up their templates.
So I would suggest working on a patch if it is something you want.
-Peff
^ permalink raw reply
* [PATCHv12 03/23] Add tests for checking correct handling of $GIT_NOTES_REF and core.notesRef
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t3301-notes.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 5d9604b..18aad53 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -206,4 +206,52 @@ do
'
done
+test_expect_success 'create other note on a different notes ref (setup)' '
+ : > a5 &&
+ git add a5 &&
+ test_tick &&
+ git commit -m 5th &&
+ GIT_NOTES_REF="refs/notes/other" git notes edit -m "other note"
+'
+
+cat > expect-other << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date: Thu Apr 7 15:17:13 2005 -0700
+
+ 5th
+
+Notes:
+ other note
+EOF
+
+cat > expect-not-other << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date: Thu Apr 7 15:17:13 2005 -0700
+
+ 5th
+EOF
+
+test_expect_success 'Do not show note on other ref by default' '
+ git log -1 > output &&
+ test_cmp expect-not-other output
+'
+
+test_expect_success 'Do show note when ref is given in GIT_NOTES_REF' '
+ GIT_NOTES_REF="refs/notes/other" git log -1 > output &&
+ test_cmp expect-other output
+'
+
+test_expect_success 'Do show note when ref is given in core.notesRef config' '
+ git config core.notesRef "refs/notes/other" &&
+ git log -1 > output &&
+ test_cmp expect-other output
+'
+
+test_expect_success 'Do not show note when core.notesRef is overridden' '
+ GIT_NOTES_REF="refs/notes/wrong" git log -1 > output &&
+ test_cmp expect-not-other output
+'
+
test_done
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 01/23] Minor non-functional fixes to notes.c
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/notes.c b/notes.c
index 023adce..47e38a1 100644
--- a/notes.c
+++ b/notes.c
@@ -1,7 +1,6 @@
#include "cache.h"
#include "commit.h"
#include "notes.h"
-#include "refs.h"
#include "utf8.h"
#include "strbuf.h"
#include "tree-walk.h"
@@ -93,7 +92,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 +194,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 +256,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 */
@@ -274,7 +273,7 @@ static void note_tree_free(struct int_node *tree)
* - hex_len - Length of above segment. Must be multiple of 2 between 0 and 40
* - sha1 - Partial SHA1 value is written here
* - sha1_len - Max #bytes to store in sha1, Must be >= hex_len / 2, and < 20
- * Returns -1 on error (invalid arguments or invalid SHA1 (not in hex format).
+ * Returns -1 on error (invalid arguments or invalid SHA1 (not in hex format)).
* Otherwise, returns number of bytes written to sha1 (i.e. hex_len / 2).
* Pads sha1 with NULs up to sha1_len (not included in returned length).
*/
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 05/23] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-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 3f4ae35..2c0d14e 100644
--- a/notes.c
+++ b/notes.c
@@ -368,6 +368,17 @@ void init_notes(const char *notes_ref, int flags)
load_subtree(&root_tree, &root_node, 0);
}
+void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+{
+ struct leaf_node *l;
+
+ assert(initialized);
+ l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
+ hashcpy(l->key_sha1, object_sha1);
+ hashcpy(l->val_sha1, note_sha1);
+ note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+}
+
static unsigned char *lookup_notes(const unsigned char *object_sha1)
{
struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 6b52799..5f22852 100644
--- a/notes.h
+++ b/notes.h
@@ -21,6 +21,10 @@
*/
void init_notes(const char *notes_ref, int flags);
+/* Add the given note object to the internal notes tree structure */
+void add_note(const unsigned char *object_sha1,
+ const unsigned char *note_sha1);
+
/* Free (and de-initialize) the internal notes tree structure */
void free_notes(void);
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 06/23] Notes API: remove_note(): Remove note objects from the notes tree structure
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
This includes adding internal functions for maintaining a healthy notes tree
structure after removing individual notes.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
notes.h | 3 ++
2 files changed, 87 insertions(+), 1 deletions(-)
diff --git a/notes.c b/notes.c
index 2c0d14e..2e82d71 100644
--- a/notes.c
+++ b/notes.c
@@ -44,7 +44,7 @@ struct leaf_node {
#define CLR_PTR_TYPE(ptr) ((void *) ((uintptr_t) (ptr) & ~3))
#define SET_PTR_TYPE(ptr, type) ((void *) ((uintptr_t) (ptr) | (type)))
-#define GET_NIBBLE(n, sha1) (((sha1[n >> 1]) >> ((~n & 0x01) << 2)) & 0x0f)
+#define GET_NIBBLE(n, sha1) (((sha1[(n) >> 1]) >> ((~(n) & 0x01) << 2)) & 0x0f)
#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
@@ -249,6 +249,79 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
note_tree_insert(new_node, n + 1, entry, type);
}
+/*
+ * How to consolidate an int_node:
+ * If there are > 1 non-NULL entries, give up and return non-zero.
+ * Otherwise replace the int_node at the given index in the given parent node
+ * with the only entry (or a NULL entry if no entries) from the given tree,
+ * and return 0.
+ */
+static int note_tree_consolidate(struct int_node *tree,
+ struct int_node *parent, unsigned char index)
+{
+ unsigned int i;
+ void *p = NULL;
+
+ assert(tree && parent);
+ assert(CLR_PTR_TYPE(parent->a[index]) == tree);
+
+ for (i = 0; i < 16; i++) {
+ if (GET_PTR_TYPE(tree->a[i]) != PTR_TYPE_NULL) {
+ if (p) /* more than one entry */
+ return -2;
+ p = tree->a[i];
+ }
+ }
+
+ /* replace tree with p in parent[index] */
+ parent->a[index] = p;
+ free(tree);
+ return 0;
+}
+
+/*
+ * To remove a leaf_node:
+ * Search to the tree location appropriate for the given leaf_node's key:
+ * - If location does not hold a matching entry, abort and do nothing.
+ * - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
+ * - Consolidate int_nodes repeatedly, while walking up the tree towards root.
+ */
+static void note_tree_remove(struct int_node *tree, unsigned char n,
+ struct leaf_node *entry)
+{
+ struct leaf_node *l;
+ struct int_node *parent_stack[20];
+ unsigned char i, j;
+ void **p = note_tree_search(&tree, &n, entry->key_sha1);
+
+ assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
+ if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
+ return; /* type mismatch, nothing to remove */
+ l = (struct leaf_node *) CLR_PTR_TYPE(*p);
+ if (hashcmp(l->key_sha1, entry->key_sha1))
+ return; /* key mismatch, nothing to remove */
+
+ /* we have found a matching entry */
+ free(l);
+ *p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
+
+ /* consolidate this tree level, and parent levels, if possible */
+ if (!n)
+ return; /* cannot consolidate top level */
+ /* first, build stack of ancestors between root and current node */
+ parent_stack[0] = &root_node;
+ for (i = 0; i < n; i++) {
+ j = GET_NIBBLE(i, entry->key_sha1);
+ parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
+ }
+ assert(i == n && parent_stack[i] == tree);
+ /* next, unwind stack until note_tree_consolidate() is done */
+ while (i > 0 &&
+ !note_tree_consolidate(parent_stack[i], parent_stack[i - 1],
+ GET_NIBBLE(i - 1, entry->key_sha1)))
+ i--;
+}
+
/* Free the entire notes data contained in the given tree */
static void note_tree_free(struct int_node *tree)
{
@@ -379,6 +452,16 @@ void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
}
+void remove_note(const unsigned char *object_sha1)
+{
+ struct leaf_node l;
+
+ assert(initialized);
+ hashcpy(l.key_sha1, object_sha1);
+ hashclr(l.val_sha1);
+ return note_tree_remove(&root_node, 0, &l);
+}
+
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 5f22852..9e66855 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);
+/* Remove the given note object from the internal notes tree structure */
+void remove_note(const unsigned char *object_sha1);
+
/* Free (and de-initialize) the internal notes tree structure */
void free_notes(void);
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 07/23] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-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 2e82d71..a0a85b4 100644
--- a/notes.c
+++ b/notes.c
@@ -462,12 +462,13 @@ void remove_note(const unsigned char *object_sha1)
return note_tree_remove(&root_node, 0, &l);
}
-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)
@@ -481,7 +482,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;
@@ -489,7 +490,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 9e66855..c0714f4 100644
--- a/notes.h
+++ b/notes.h
@@ -28,6 +28,9 @@ void add_note(const unsigned char *object_sha1,
/* Remove the given note object from the internal notes tree structure */
void remove_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);
+
/* Free (and de-initialize) the internal notes tree structure */
void free_notes(void);
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 09/23] Notes API: write_notes_tree(): Store the notes tree in the database
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
Uses for_each_note() to traverse the notes tree, and produces tree
objects on the fly representing the "on-disk" version of the notes
tree with appropriate fanout.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 38 +++++++++++++++-
2 files changed, 180 insertions(+), 3 deletions(-)
diff --git a/notes.c b/notes.c
index eabd6f3..b576f7e 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "notes.h"
+#include "tree.h"
#include "utf8.h"
#include "strbuf.h"
#include "tree-walk.h"
@@ -540,6 +541,126 @@ redo:
return 0;
}
+struct tree_write_stack {
+ struct tree_write_stack *next;
+ struct strbuf buf;
+ char path[2]; /* path to subtree in next, if any */
+};
+
+static inline int matches_tree_write_stack(struct tree_write_stack *tws,
+ const char *full_path)
+{
+ return full_path[0] == tws->path[0] &&
+ full_path[1] == tws->path[1] &&
+ full_path[2] == '/';
+}
+
+static void write_tree_entry(struct strbuf *buf, unsigned int mode,
+ const char *path, unsigned int path_len, const
+ unsigned char *sha1)
+{
+ strbuf_addf(buf, "%06o %.*s%c", mode, path_len, path, '\0');
+ strbuf_add(buf, sha1, 20);
+}
+
+static void tree_write_stack_init_subtree(struct tree_write_stack *tws,
+ const char *path)
+{
+ struct tree_write_stack *n;
+ assert(!tws->next);
+ assert(tws->path[0] == '\0' && tws->path[1] == '\0');
+ n = (struct tree_write_stack *)
+ xmalloc(sizeof(struct tree_write_stack));
+ n->next = NULL;
+ strbuf_init(&n->buf, 256 * (32 + 40)); /* assume 256 entries per tree */
+ n->path[0] = n->path[1] = '\0';
+ tws->next = n;
+ tws->path[0] = path[0];
+ tws->path[1] = path[1];
+}
+
+static int tree_write_stack_finish_subtree(struct tree_write_stack *tws)
+{
+ int ret;
+ struct tree_write_stack *n = tws->next;
+ unsigned char s[20];
+ if (n) {
+ ret = tree_write_stack_finish_subtree(n);
+ if (ret)
+ return ret;
+ ret = write_sha1_file(n->buf.buf, n->buf.len, tree_type, s);
+ if (ret)
+ return ret;
+ strbuf_release(&n->buf);
+ free(n);
+ tws->next = NULL;
+ write_tree_entry(&tws->buf, 040000, tws->path, 2, s);
+ tws->path[0] = tws->path[1] = '\0';
+ }
+ return 0;
+}
+
+static int write_each_note_helper(struct tree_write_stack *tws,
+ const char *path, unsigned int mode,
+ const unsigned char *sha1)
+{
+ size_t path_len = strlen(path);
+ unsigned int n = 0;
+ int ret;
+
+ /* Determine common part of tree write stack */
+ while (tws && 3 * n < path_len &&
+ matches_tree_write_stack(tws, path + 3 * n)) {
+ n++;
+ tws = tws->next;
+ }
+
+ /* tws point to last matching tree_write_stack entry */
+ ret = tree_write_stack_finish_subtree(tws);
+ if (ret)
+ return ret;
+
+ /* Start subtrees needed to satisfy path */
+ while (3 * n + 2 < path_len && path[3 * n + 2] == '/') {
+ tree_write_stack_init_subtree(tws, path + 3 * n);
+ n++;
+ tws = tws->next;
+ }
+
+ /* There should be no more directory components in the given path */
+ assert(memchr(path + 3 * n, '/', path_len - (3 * n)) == NULL);
+
+ /* Finally add given entry to the current tree object */
+ write_tree_entry(&tws->buf, mode, path + 3 * n, path_len - (3 * n),
+ sha1);
+
+ return 0;
+}
+
+struct write_each_note_data {
+ struct tree_write_stack *root;
+};
+
+static int write_each_note(const unsigned char *object_sha1,
+ const unsigned char *note_sha1, char *note_path,
+ void *cb_data)
+{
+ struct write_each_note_data *d =
+ (struct write_each_note_data *) cb_data;
+ size_t note_path_len = strlen(note_path);
+ unsigned int mode = 0100644;
+
+ if (note_path[note_path_len - 1] == '/') {
+ /* subtree entry */
+ note_path_len--;
+ note_path[note_path_len] = '\0';
+ mode = 040000;
+ }
+ assert(note_path_len <= 40 + 19);
+
+ return write_each_note_helper(d->root, note_path, mode, note_sha1);
+}
+
void init_notes(const char *notes_ref, int flags)
{
unsigned char sha1[20], object_sha1[20];
@@ -604,6 +725,30 @@ int for_each_note(int flags, each_note_fn fn, void *cb_data)
return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
}
+int write_notes_tree(unsigned char *result)
+{
+ struct tree_write_stack root;
+ struct write_each_note_data cb_data;
+ int ret;
+
+ assert(initialized);
+
+ /* Prepare for traversal of current notes tree */
+ root.next = NULL; /* last forward entry in list is grounded */
+ strbuf_init(&root.buf, 256 * (32 + 40)); /* assume 256 entries */
+ root.path[0] = root.path[1] = '\0';
+ cb_data.root = &root;
+
+ /* Write tree objects representing current notes tree */
+ ret = for_each_note(FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+ FOR_EACH_NOTE_YIELD_SUBTREES,
+ write_each_note, &cb_data) ||
+ tree_write_stack_finish_subtree(&root) ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+ strbuf_release(&root.buf);
+ return ret;
+}
+
void free_notes(void)
{
note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index c319fd8..ebc301c 100644
--- a/notes.h
+++ b/notes.h
@@ -21,11 +21,23 @@
*/
void init_notes(const char *notes_ref, int flags);
-/* Add the given note object to the internal notes tree structure */
+/*
+ * Add the given note object to the internal notes tree structure
+ *
+ * IMPORTANT: The changes made by add_note() to the internal notes tree structure
+ * are not persistent until a subsequent call to write_notes_tree() returns
+ * zero.
+ */
void add_note(const unsigned char *object_sha1,
const unsigned char *note_sha1);
-/* Remove the given note object from the internal notes tree structure */
+/*
+ * Remove the given note object from the internal notes tree structure
+ *
+ * IMPORTANT: The changes made by remove_note() to the internal notes tree
+ * structure are not persistent until a subsequent call to write_notes_tree()
+ * returns zero.
+ */
void remove_note(const unsigned char *object_sha1);
/* Get the note object SHA1 containing the note data for the given object */
@@ -78,7 +90,27 @@ typedef int each_note_fn(const unsigned char *object_sha1,
void *cb_data);
int for_each_note(int flags, each_note_fn fn, void *cb_data);
-/* Free (and de-initialize) the internal notes tree structure */
+/*
+ * Write the internal notes tree structure to the object database
+ *
+ * Creates a new tree object encapsulating the current state of the
+ * internal notes tree, and stores its SHA1 into the 'result' argument.
+ *
+ * Returns zero on success, non-zero on failure.
+ *
+ * IMPORTANT: Changes made to the internal notes tree structure are not
+ * persistent until this function has returned zero. Please also remember
+ * to create a corresponding commit object, and update the appropriate
+ * notes ref.
+ */
+int write_notes_tree(unsigned char *result);
+
+/*
+ * Free (and de-initialize) the internal notes tree structure
+ *
+ * IMPORTANT: Changes made to the notes tree since the last, successful
+ * call to write_notes_tree() will be lost.
+ */
void free_notes(void);
/* Flags controlling how notes are formatted */
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 10/23] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-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 make use of 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 the fallback "default" notes
tree (default_notes_tree) is used.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 90 ++++++++++++++++++++++++++++++++++++++-----------------------
notes.h | 81 +++++++++++++++++++++++++++++++++++--------------------
pretty.c | 7 +++--
3 files changed, 112 insertions(+), 66 deletions(-)
diff --git a/notes.c b/notes.c
index b576f7e..08a369a 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;
+struct notes_tree default_notes_tree;
static void load_subtree(struct leaf_node *subtree, struct int_node *node,
unsigned int n);
@@ -287,8 +285,8 @@ static int note_tree_consolidate(struct int_node *tree,
* - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
* - Consolidate int_nodes repeatedly, while walking up the tree towards root.
*/
-static void note_tree_remove(struct int_node *tree, unsigned char n,
- struct leaf_node *entry)
+static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
+ unsigned char n, struct leaf_node *entry)
{
struct leaf_node *l;
struct int_node *parent_stack[20];
@@ -310,7 +308,7 @@ static void note_tree_remove(struct int_node *tree, unsigned char n,
if (!n)
return; /* cannot consolidate top level */
/* first, build stack of ancestors between root and current node */
- parent_stack[0] = &root_node;
+ parent_stack[0] = t->root;
for (i = 0; i < n; i++) {
j = GET_NIBBLE(i, entry->key_sha1);
parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
@@ -661,14 +659,15 @@ static int write_each_note(const unsigned char *object_sha1,
return write_each_note_helper(d->root, note_path, mode, note_sha1);
}
-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_notes_tree;
+ assert(!t->initialized);
if (!notes_ref)
notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
@@ -677,6 +676,10 @@ void init_notes(const char *notes_ref, int flags)
if (!notes_ref)
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))
return;
@@ -686,52 +689,65 @@ 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_notes_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);
}
-void remove_note(const unsigned char *object_sha1)
+void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
{
struct leaf_node l;
- assert(initialized);
+ if (!t)
+ t = &default_notes_tree;
+ assert(t->initialized);
hashcpy(l.key_sha1, object_sha1);
hashclr(l.val_sha1);
- return note_tree_remove(&root_node, 0, &l);
+ return note_tree_remove(t, t->root, 0, &l);
}
-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_notes_tree;
+ assert(t->initialized);
+ found = note_tree_find(t->root, 0, object_sha1);
return found ? found->val_sha1 : NULL;
}
-int for_each_note(int flags, each_note_fn fn, void *cb_data)
+int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
+ void *cb_data)
{
- assert(initialized);
- return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
+ if (!t)
+ t = &default_notes_tree;
+ assert(t->initialized);
+ return for_each_note_helper(t->root, 0, 0, flags, fn, cb_data);
}
-int write_notes_tree(unsigned char *result)
+int write_notes_tree(struct notes_tree *t, unsigned char *result)
{
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
- assert(initialized);
+ if (!t)
+ t = &default_notes_tree;
+ assert(t->initialized);
/* Prepare for traversal of current notes tree */
root.next = NULL; /* last forward entry in list is grounded */
@@ -740,7 +756,7 @@ int write_notes_tree(unsigned char *result)
cb_data.root = &root;
/* Write tree objects representing current notes tree */
- ret = for_each_note(FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+ ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
FOR_EACH_NOTE_YIELD_SUBTREES,
write_each_note, &cb_data) ||
tree_write_stack_finish_subtree(&root) ||
@@ -749,15 +765,19 @@ int write_notes_tree(unsigned char *result)
return ret;
}
-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_notes_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;
@@ -765,10 +785,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_notes_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 ebc301c..a599a0d 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.
+ */
+extern struct notes_tree {
+ struct int_node *root;
+ char *ref;
+ int initialized;
+} default_notes_tree;
+
+/*
* Flags controlling behaviour of notes tree initialization
*
* Default behaviour is to initialize the notes tree from the tree object
@@ -10,44 +25,50 @@
#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
+ * Add the given note object to the given notes_tree structure
*
- * IMPORTANT: The changes made by add_note() to the internal notes tree structure
+ * IMPORTANT: The changes made by add_note() to the given notes_tree structure
* are not persistent until a subsequent call to write_notes_tree() returns
* zero.
*/
-void add_note(const unsigned char *object_sha1,
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
const unsigned char *note_sha1);
/*
- * Remove the given note object from the internal notes tree structure
+ * Remove the given note object from the given notes_tree structure
*
- * IMPORTANT: The changes made by remove_note() to the internal notes tree
+ * IMPORTANT: The changes made by remove_note() to the given notes_tree
* structure are not persistent until a subsequent call to write_notes_tree()
* returns zero.
*/
-void remove_note(const unsigned char *object_sha1);
+void remove_note(struct notes_tree *t, 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);
+const unsigned char *get_note(struct notes_tree *t,
+ const unsigned char *object_sha1);
/*
* Flags controlling behaviour of for_each_note()
*
* Default behaviour of for_each_note() is to traverse every single note object
- * in the notes tree, unpacking subtree entries along the way.
+ * in the given notes tree, unpacking subtree entries along the way.
* The following flags can be used to alter the default behaviour:
*
* - DONT_UNPACK_SUBTREES causes for_each_note() NOT to unpack and recurse into
@@ -71,7 +92,7 @@ const unsigned char *get_note(const unsigned char *object_sha1);
#define FOR_EACH_NOTE_YIELD_SUBTREES 2
/*
- * 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(). Hence, a zero
@@ -88,30 +109,30 @@ 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, char *note_path,
void *cb_data);
-int for_each_note(int flags, each_note_fn fn, void *cb_data);
+int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
+ void *cb_data);
/*
- * Write the internal notes tree structure to the object database
+ * Write the given notes_tree structure to the object database
*
- * Creates a new tree object encapsulating the current state of the
- * internal notes tree, and stores its SHA1 into the 'result' argument.
+ * Creates a new tree object encapsulating the current state of the given
+ * notes_tree, and stores its SHA1 into the 'result' argument.
*
* Returns zero on success, non-zero on failure.
*
- * IMPORTANT: Changes made to the internal notes tree structure are not
- * persistent until this function has returned zero. Please also remember
- * to create a corresponding commit object, and update the appropriate
- * notes ref.
+ * IMPORTANT: Changes made to the given notes_tree are not persistent until
+ * this function has returned zero. Please also remember to create a
+ * corresponding commit object, and update the appropriate notes ref.
*/
-int write_notes_tree(unsigned char *result);
+int write_notes_tree(struct notes_tree *t, unsigned char *result);
/*
- * Free (and de-initialize) the internal notes tree structure
+ * Free (and de-initialize) the given notes_tree structure
*
- * IMPORTANT: Changes made to the notes tree since the last, successful
+ * IMPORTANT: Changes made to the given notes_tree since the last, successful
* call to write_notes_tree() will be lost.
*/
-void free_notes(void);
+void free_notes(struct notes_tree *t);
/* Flags controlling how notes are formatted */
#define NOTES_SHOW_HEADER 1
@@ -120,12 +141,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 076b918..f999485 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,9 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
}
return 0; /* unknown %g placeholder */
case 'N':
- format_note(commit->object.sha1, sb, git_log_output_encoding ?
- git_log_output_encoding : git_commit_encoding, 0);
+ format_note(NULL, commit->object.sha1, sb,
+ git_log_output_encoding ? git_log_output_encoding
+ : git_commit_encoding, 0);
return 1;
}
@@ -1095,7 +1096,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
strbuf_addch(sb, '\n');
if (context->show_notes)
- format_note(commit->object.sha1, sb, encoding,
+ format_note(NULL, commit->object.sha1, sb, encoding,
NOTES_SHOW_HEADER | NOTES_INDENT);
free(reencoded);
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 11/23] Refactor notes concatenation into a flexible interface for combining notes
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-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 | 138 ++++++++++++++++++++++++++++++++++++--------------------------
notes.h | 34 +++++++++++++++-
2 files changed, 112 insertions(+), 60 deletions(-)
diff --git a/notes.c b/notes.c
index 08a369a..dc4e4f6 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "notes.h"
+#include "blob.h"
#include "tree.h"
#include "utf8.h"
#include "strbuf.h"
@@ -127,55 +128,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 +142,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 +164,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 +191,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 +201,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);
}
/*
@@ -406,7 +364,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);
@@ -659,7 +618,64 @@ static int write_each_note(const unsigned char *object_sha1,
return write_each_note_helper(d->root, note_path, mode, note_sha1);
}
-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 = NULL, *new_msg = NULL, *buf;
+ unsigned long cur_len, new_len, buf_len;
+ enum object_type cur_type, new_type;
+ int ret;
+
+ /* read in both note blob objects */
+ if (!is_null_sha1(new_sha1))
+ 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;
+ }
+ if (!is_null_sha1(cur_sha1))
+ 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_type, 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;
@@ -676,8 +692,12 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
if (!notes_ref)
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 ||
@@ -693,17 +713,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_notes_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);
}
void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -788,7 +810,7 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1,
if (!t)
t = &default_notes_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 a599a0d..5c49465 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 must both be non-NULL and different from each other.
+ *
+ * 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 @@
extern struct notes_tree {
struct int_node *root;
char *ref;
+ combine_notes_fn *combine_notes;
int initialized;
} default_notes_tree;
@@ -36,10 +61,15 @@ extern 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
@@ -49,7 +79,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
* zero.
*/
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);
/*
* Remove the given note object from the given notes_tree structure
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 13/23] t3301: Verify successful annotation of non-commits
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
Adds a testcase verifying that git-notes works successfully on
tree, blob, and tag objects.
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t3301-notes.sh | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 10f62f4..fd5e593 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -282,4 +282,21 @@ test_expect_success 'Do not show note when core.notesRef is overridden' '
test_cmp expect-not-other output
'
+test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
+ echo "Note on a tree" > expect
+ git notes edit -m "Note on a tree" HEAD: &&
+ git notes show HEAD: > actual &&
+ test_cmp expect actual &&
+ echo "Note on a blob" > expect
+ filename=$(git ls-tree --name-only HEAD | head -n1) &&
+ git notes edit -m "Note on a blob" HEAD:$filename &&
+ git notes show HEAD:$filename > actual &&
+ test_cmp expect actual &&
+ echo "Note on a tag" > expect
+ git tag -a -m "This is an annotated tag" foobar HEAD^ &&
+ git notes edit -m "Note on a tag" foobar &&
+ git notes show foobar > actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 08/23] Notes API: for_each_note(): Traverse the entire notes tree with a callback
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 47 ++++++++++++++++++++++
2 files changed, 180 insertions(+), 0 deletions(-)
diff --git a/notes.c b/notes.c
index a0a85b4..eabd6f3 100644
--- a/notes.c
+++ b/notes.c
@@ -413,6 +413,133 @@ 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 all of them are either int_nodes or
+ * subtree entries, then there are likely plenty of notes below this
+ * level, so we return an incremented fanout.
+ */
+ 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:
+ 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, int flags, 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, flags, fn, cb_data);
+ break;
+ case PTR_TYPE_SUBTREE:
+ l = (struct leaf_node *) CLR_PTR_TYPE(p);
+ /*
+ * Subtree entries in the note tree represent parts of
+ * the note tree that have not yet been explored. There
+ * is a direct relationship between subtree entries at
+ * level 'n' in the tree, and the 'fanout' variable:
+ * Subtree entries at level 'n <= 2 * fanout' should be
+ * preserved, since they correspond exactly to a fanout
+ * directory in the on-disk structure. However, subtree
+ * entries at level 'n > 2 * fanout' should NOT be
+ * preserved, but rather consolidated into the above
+ * notes tree level. We achieve this by unconditionally
+ * unpacking subtree entries that exist below the
+ * threshold level at 'n = 2 * fanout'.
+ */
+ if (n <= 2 * fanout &&
+ flags & FOR_EACH_NOTE_YIELD_SUBTREES) {
+ /* invoke callback with subtree */
+ unsigned int path_len =
+ l->key_sha1[19] * 2 + fanout;
+ assert(path_len < 40 + 19);
+ construct_path_with_fanout(l->key_sha1, fanout,
+ path);
+ /* Create trailing slash, if needed */
+ if (path[path_len - 1] != '/')
+ path[path_len++] = '/';
+ path[path_len] = '\0';
+ ret = fn(l->key_sha1, l->val_sha1, path,
+ cb_data);
+ }
+ if (n > fanout * 2 ||
+ !(flags & FOR_EACH_NOTE_DONT_UNPACK_SUBTREES)) {
+ /* unpack subtree and resume traversal */
+ tree->a[i] = NULL;
+ load_subtree(l, tree, n);
+ free(l);
+ goto redo;
+ }
+ break;
+ 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];
@@ -471,6 +598,12 @@ const unsigned char *get_note(const unsigned char *object_sha1)
return found ? found->val_sha1 : NULL;
}
+int for_each_note(int flags, each_note_fn fn, void *cb_data)
+{
+ assert(initialized);
+ return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
+}
+
void free_notes(void)
{
note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index c0714f4..c319fd8 100644
--- a/notes.h
+++ b/notes.h
@@ -31,6 +31,53 @@ void remove_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);
+/*
+ * Flags controlling behaviour of for_each_note()
+ *
+ * Default behaviour of for_each_note() is to traverse every single note object
+ * in the notes tree, unpacking subtree entries along the way.
+ * The following flags can be used to alter the default behaviour:
+ *
+ * - DONT_UNPACK_SUBTREES causes for_each_note() NOT to unpack and recurse into
+ * subtree entries while traversing the notes tree. This causes notes within
+ * those subtrees NOT to be passed to the callback. Use this flag if you
+ * don't want to traverse _all_ notes, but only want to traverse the parts
+ * of the notes tree that have already been unpacked (this includes at least
+ * all notes that have been added/changed).
+ *
+ * - YIELD_SUBTREES causes any subtree entries that are encountered to be
+ * passed to the callback, before recursing into them. Subtree entries are
+ * not note objects, but represent intermediate directories in the notes
+ * tree. When passed to the callback, subtree entries will have a trailing
+ * slash in their path, which the callback may use to differentiate between
+ * note entries and subtree entries. Note that already-unpacked subtree
+ * entries are not part of the notes tree, and will therefore not be yielded.
+ * If this flag is used together with DONT_UNPACK_SUBTREES, for_each_note()
+ * will yield the subtree entry, but not recurse into it.
+ */
+#define FOR_EACH_NOTE_DONT_UNPACK_SUBTREES 1
+#define FOR_EACH_NOTE_YIELD_SUBTREES 2
+
+/*
+ * 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(). Hence, a zero
+ * return value from for_each_note() indicates that all notes were walked
+ * successfully.
+ *
+ * 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()
+ * - remove_note()
+ * - free_notes()
+ */
+typedef int each_note_fn(const unsigned char *object_sha1,
+ const unsigned char *note_sha1, char *note_path,
+ void *cb_data);
+int for_each_note(int flags, each_note_fn fn, void *cb_data);
+
/* Free (and de-initialize) the internal notes tree structure */
void free_notes(void);
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 14/23] t3305: Verify that adding many notes with git-notes triggers increased fanout
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
Add a test verifying that the notes code automatically restructures the
notes tree into a deeper fanout level, when many notes are added with
"git notes".
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t3305-notes-fanout.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 50 insertions(+), 0 deletions(-)
create mode 100755 t/t3305-notes-fanout.sh
diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
new file mode 100755
index 0000000..823b0ff
--- /dev/null
+++ b/t/t3305-notes-fanout.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test that adding many notes triggers automatic fanout restructuring'
+
+. ./test-lib.sh
+
+test_expect_success 'creating many notes with git-notes' '
+ num_notes=300 &&
+ i=0 &&
+ while test $i -lt $num_notes
+ do
+ i=$(($i + 1)) &&
+ test_tick &&
+ echo "file for commit #$i" > file &&
+ git add file &&
+ git commit -q -m "commit #$i" &&
+ git notes edit -m "note #$i" || return 1
+ done
+'
+
+test_expect_success 'many notes created correctly with git-notes' '
+ git log | grep "^ " > output &&
+ i=300 &&
+ while test $i -gt 0
+ do
+ echo " commit #$i" &&
+ echo " note #$i" &&
+ i=$(($i - 1));
+ done > expect &&
+ test_cmp expect output
+'
+
+test_expect_success 'many notes created with git-notes triggers fanout' '
+ # Expect entire notes tree to have a fanout == 1
+ git ls-tree -r --name-only refs/notes/commits |
+ while read path
+ do
+ case "$path" in
+ ??/??????????????????????????????????????)
+ : true
+ ;;
+ *)
+ echo "Invalid path \"$path\"" &&
+ return 1
+ ;;
+ esac
+ done
+'
+
+test_done
--
1.6.6.405.g80ed6
^ permalink raw reply related
* [PATCHv12 15/23] Teach notes code to properly preserve non-notes in the notes tree
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>
The note tree structure allows for non-note entries to coexist with note
entries in a notes tree. Although we certainly expect there to be very
few non-notes in a notes tree, we should still support them to a certain
degree.
This patch teaches the notes code to preserve non-notes when updating the
notes tree with write_notes_tree(). Non-notes are not affected by fanout
restructuring.
For non-notes to be handled correctly, we can no longer allow subtree
entries that do not match the fanout structure produced by the notes code
itself. This means that fanouts like 4/36, 6/34, 8/32, 4/4/32, etc. are
no longer recognized as note subtrees; only 2-based fanouts are allowed
(2/38, 2/2/36, 2/2/2/34, etc.). Since the notes code has never at any point
_produced_ non-2-based fanouts, it is highly unlikely that this change will
cause problems for anyone.
The patch also adds some tests verifying the correct handling of non-notes
in a notes tree.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 219 +++++++++++++++++++++++++++++++++++++--------
notes.h | 1 +
t/t3303-notes-subtrees.sh | 28 ++++---
t/t3304-notes-mixed.sh | 36 +++++++-
4 files changed, 233 insertions(+), 51 deletions(-)
diff --git a/notes.c b/notes.c
index dc4e4f6..d432517 100644
--- a/notes.c
+++ b/notes.c
@@ -37,6 +37,21 @@ struct leaf_node {
unsigned char val_sha1[20];
};
+/*
+ * A notes tree may contain entries that are not notes, and that do not follow
+ * the naming conventions of notes. There are typically none/few of these, but
+ * we still need to keep track of them. Keep a simple linked list sorted alpha-
+ * betically on the non-note path. The list is populated when parsing tree
+ * objects in load_subtree(), and the non-notes are correctly written back into
+ * the tree objects produced by write_notes_tree().
+ */
+struct non_note {
+ struct non_note *next; /* grounded (last->next == NULL) */
+ char *path;
+ unsigned int mode;
+ unsigned char sha1[20];
+};
+
#define PTR_TYPE_NULL 0
#define PTR_TYPE_INTERNAL 1
#define PTR_TYPE_NOTE 2
@@ -53,8 +68,8 @@ struct leaf_node {
struct notes_tree default_notes_tree;
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
- unsigned int n);
+static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
+ struct int_node *node, unsigned int n);
/*
* Search the tree until the appropriate location for the given key is found:
@@ -71,7 +86,7 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
* - an unused leaf node (NULL)
* In any case, set *tree and *n, and return pointer to the tree location.
*/
-static void **note_tree_search(struct int_node **tree,
+static void **note_tree_search(struct notes_tree *t, struct int_node **tree,
unsigned char *n, const unsigned char *key_sha1)
{
struct leaf_node *l;
@@ -83,9 +98,9 @@ static void **note_tree_search(struct int_node **tree,
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
(*tree)->a[0] = NULL;
- load_subtree(l, *tree, *n);
+ load_subtree(t, l, *tree, *n);
free(l);
- return note_tree_search(tree, n, key_sha1);
+ return note_tree_search(t, tree, n, key_sha1);
}
}
@@ -95,15 +110,15 @@ static void **note_tree_search(struct int_node **tree,
case PTR_TYPE_INTERNAL:
*tree = CLR_PTR_TYPE(p);
(*n)++;
- return note_tree_search(tree, n, key_sha1);
+ return note_tree_search(t, tree, n, key_sha1);
case PTR_TYPE_SUBTREE:
l = (struct leaf_node *) CLR_PTR_TYPE(p);
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
(*tree)->a[i] = NULL;
- load_subtree(l, *tree, *n);
+ load_subtree(t, l, *tree, *n);
free(l);
- return note_tree_search(tree, n, key_sha1);
+ return note_tree_search(t, tree, n, key_sha1);
}
/* fall through */
default:
@@ -116,10 +131,11 @@ static void **note_tree_search(struct int_node **tree,
* Search to the tree location appropriate for the given key:
* If a note entry with matching key, return the note entry, else return NULL.
*/
-static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
+static struct leaf_node *note_tree_find(struct notes_tree *t,
+ struct int_node *tree, unsigned char n,
const unsigned char *key_sha1)
{
- void **p = note_tree_search(&tree, &n, key_sha1);
+ void **p = note_tree_search(t, &tree, &n, key_sha1);
if (GET_PTR_TYPE(*p) == PTR_TYPE_NOTE) {
struct leaf_node *l = (struct leaf_node *) CLR_PTR_TYPE(*p);
if (!hashcmp(key_sha1, l->key_sha1))
@@ -141,13 +157,13 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
* - Else, create a new int_node, holding both the node-at-location and the
* 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,
+static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
+ unsigned char n, struct leaf_node *entry, unsigned char type,
combine_notes_fn combine_notes)
{
struct int_node *new_node;
struct leaf_node *l;
- void **p = note_tree_search(&tree, &n, entry->key_sha1);
+ void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
l = (struct leaf_node *) CLR_PTR_TYPE(*p);
@@ -178,7 +194,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
if (!SUBTREE_SHA1_PREFIXCMP(l->key_sha1,
entry->key_sha1)) {
/* unpack 'entry' */
- load_subtree(entry, tree, n);
+ load_subtree(t, entry, tree, n);
free(entry);
return;
}
@@ -189,9 +205,10 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
if (!SUBTREE_SHA1_PREFIXCMP(entry->key_sha1, l->key_sha1)) {
/* unpack 'l' and restart insert */
*p = NULL;
- load_subtree(l, tree, n);
+ load_subtree(t, l, tree, n);
free(l);
- note_tree_insert(tree, n, entry, type, combine_notes);
+ note_tree_insert(t, tree, n, entry, type,
+ combine_notes);
return;
}
break;
@@ -201,9 +218,10 @@ 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), combine_notes);
+ note_tree_insert(t, 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, combine_notes);
+ note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
}
/*
@@ -249,7 +267,7 @@ static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
struct leaf_node *l;
struct int_node *parent_stack[20];
unsigned char i, j;
- void **p = note_tree_search(&tree, &n, entry->key_sha1);
+ void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
@@ -324,14 +342,67 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
return len;
}
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
- unsigned int n)
+static int non_note_cmp(const struct non_note *a, const struct non_note *b)
+{
+ return strcmp(a->path, b->path);
+}
+
+static void add_non_note(struct notes_tree *t, const char *path,
+ unsigned int mode, const unsigned char *sha1)
+{
+ struct non_note *p = t->prev_non_note, *n;
+ n = (struct non_note *) xmalloc(sizeof(struct non_note));
+ n->next = NULL;
+ n->path = xstrdup(path);
+ n->mode = mode;
+ hashcpy(n->sha1, sha1);
+ t->prev_non_note = n;
+
+ if (!t->first_non_note) {
+ t->first_non_note = n;
+ return;
+ }
+
+ if (non_note_cmp(p, n) < 0)
+ ; /* do nothing */
+ else if (non_note_cmp(t->first_non_note, n) <= 0)
+ p = t->first_non_note;
+ else {
+ /* n sorts before t->first_non_note */
+ n->next = t->first_non_note;
+ t->first_non_note = n;
+ return;
+ }
+
+ /* n sorts equal or after p */
+ while (p->next && non_note_cmp(p->next, n) <= 0)
+ p = p->next;
+
+ if (non_note_cmp(p, n) == 0) { /* n ~= p; overwrite p with n */
+ assert(strcmp(p->path, n->path) == 0);
+ p->mode = n->mode;
+ hashcpy(p->sha1, n->sha1);
+ free(n);
+ t->prev_non_note = p;
+ return;
+ }
+
+ /* n sorts between p and p->next */
+ n->next = p->next;
+ p->next = n;
+}
+
+static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
+ struct int_node *node, unsigned int n)
{
unsigned char object_sha1[20];
unsigned int prefix_len;
void *buf;
struct tree_desc desc;
struct name_entry entry;
+ int len, path_len;
+ unsigned char type;
+ struct leaf_node *l;
buf = fill_tree_descriptor(&desc, subtree->val_sha1);
if (!buf)
@@ -342,31 +413,68 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
assert(prefix_len * 2 >= n);
memcpy(object_sha1, subtree->key_sha1, prefix_len);
while (tree_entry(&desc, &entry)) {
- int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
+ path_len = strlen(entry.path);
+ len = get_sha1_hex_segment(entry.path, path_len,
object_sha1 + prefix_len, 20 - prefix_len);
if (len < 0)
- continue; /* entry.path is not a SHA1 sum. Skip */
+ goto handle_non_note; /* entry.path is not a SHA1 */
len += prefix_len;
/*
* If object SHA1 is complete (len == 20), assume note object
- * If object SHA1 is incomplete (len < 20), assume note subtree
+ * If object SHA1 is incomplete (len < 20), and current
+ * component consists of 2 hex chars, assume note subtree
*/
if (len <= 20) {
- unsigned char type = PTR_TYPE_NOTE;
- struct leaf_node *l = (struct leaf_node *)
+ type = PTR_TYPE_NOTE;
+ l = (struct leaf_node *)
xcalloc(sizeof(struct leaf_node), 1);
hashcpy(l->key_sha1, object_sha1);
hashcpy(l->val_sha1, entry.sha1);
if (len < 20) {
- if (!S_ISDIR(entry.mode))
- continue; /* entry cannot be subtree */
+ if (!S_ISDIR(entry.mode) || path_len != 2)
+ goto handle_non_note; /* not subtree */
l->key_sha1[19] = (unsigned char) len;
type = PTR_TYPE_SUBTREE;
}
- note_tree_insert(node, n, l, type,
+ note_tree_insert(t, node, n, l, type,
combine_notes_concatenate);
}
+ continue;
+
+handle_non_note:
+ /*
+ * Determine full path for this non-note entry:
+ * The filename is already found in entry.path, but the
+ * directory part of the path must be deduced from the subtree
+ * containing this entry. We assume here that the overall notes
+ * tree follows a strict byte-based progressive fanout
+ * structure (i.e. using 2/38, 2/2/36, etc. fanouts, and not
+ * e.g. 4/36 fanout). This means that if a non-note is found at
+ * path "dead/beef", the following code will register it as
+ * being found on "de/ad/beef".
+ * On the other hand, if you use such non-obvious non-note
+ * paths in the middle of a notes tree, you deserve what's
+ * coming to you ;). Note that for non-notes that are not
+ * SHA1-like at the top level, there will be no problems.
+ *
+ * To conclude, it is strongly advised to make sure non-notes
+ * have at least one non-hex character in the top-level path
+ * component.
+ */
+ {
+ char non_note_path[PATH_MAX];
+ char *p = non_note_path;
+ const char *q = sha1_to_hex(subtree->key_sha1);
+ int i;
+ for (i = 0; i < prefix_len; i++) {
+ *p++ = *q++;
+ *p++ = *q++;
+ *p++ = '/';
+ }
+ strcpy(p, entry.path);
+ add_non_note(t, non_note_path, entry.mode, entry.sha1);
+ }
}
free(buf);
}
@@ -426,9 +534,9 @@ static void construct_path_with_fanout(const unsigned char *sha1,
strcpy(path + i, hex_sha1 + j);
}
-static int for_each_note_helper(struct int_node *tree, unsigned char n,
- unsigned char fanout, int flags, each_note_fn fn,
- void *cb_data)
+static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
+ unsigned char n, unsigned char fanout, int flags,
+ each_note_fn fn, void *cb_data)
{
unsigned int i;
void *p;
@@ -443,7 +551,7 @@ redo:
switch (GET_PTR_TYPE(p)) {
case PTR_TYPE_INTERNAL:
/* recurse into int_node */
- ret = for_each_note_helper(CLR_PTR_TYPE(p), n + 1,
+ ret = for_each_note_helper(t, CLR_PTR_TYPE(p), n + 1,
fanout, flags, fn, cb_data);
break;
case PTR_TYPE_SUBTREE:
@@ -481,7 +589,7 @@ redo:
!(flags & FOR_EACH_NOTE_DONT_UNPACK_SUBTREES)) {
/* unpack subtree and resume traversal */
tree->a[i] = NULL;
- load_subtree(l, tree, n);
+ load_subtree(t, l, tree, n);
free(l);
goto redo;
}
@@ -596,8 +704,29 @@ static int write_each_note_helper(struct tree_write_stack *tws,
struct write_each_note_data {
struct tree_write_stack *root;
+ struct non_note *next_non_note;
};
+static int write_each_non_note_until(const char *note_path,
+ struct write_each_note_data *d)
+{
+ struct non_note *n = d->next_non_note;
+ int cmp, ret;
+ while (n && (!note_path || (cmp = strcmp(n->path, note_path)) <= 0)) {
+ if (note_path && cmp == 0)
+ ; /* do nothing, prefer note to non-note */
+ else {
+ ret = write_each_note_helper(d->root, n->path, n->mode,
+ n->sha1);
+ if (ret)
+ return ret;
+ }
+ n = n->next;
+ }
+ d->next_non_note = n;
+ return 0;
+}
+
static int write_each_note(const unsigned char *object_sha1,
const unsigned char *note_sha1, char *note_path,
void *cb_data)
@@ -615,7 +744,9 @@ static int write_each_note(const unsigned char *object_sha1,
}
assert(note_path_len <= 40 + 19);
- return write_each_note_helper(d->root, note_path, mode, note_sha1);
+ /* Weave non-note entries into note entries */
+ return write_each_non_note_until(note_path, d) ||
+ write_each_note_helper(d->root, note_path, mode, note_sha1);
}
int combine_notes_concatenate(unsigned char *cur_sha1,
@@ -696,6 +827,8 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
combine_notes = combine_notes_concatenate;
t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+ t->first_non_note = NULL;
+ t->prev_non_note = NULL;
t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
t->combine_notes = combine_notes;
t->initialized = 1;
@@ -709,7 +842,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
hashclr(root_tree.key_sha1);
hashcpy(root_tree.val_sha1, sha1);
- load_subtree(&root_tree, t->root, 0);
+ load_subtree(t, &root_tree, t->root, 0);
}
void add_note(struct notes_tree *t, const unsigned char *object_sha1,
@@ -725,7 +858,7 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
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, combine_notes);
+ note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
}
void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -748,7 +881,7 @@ const unsigned char *get_note(struct notes_tree *t,
if (!t)
t = &default_notes_tree;
assert(t->initialized);
- found = note_tree_find(t->root, 0, object_sha1);
+ found = note_tree_find(t, t->root, 0, object_sha1);
return found ? found->val_sha1 : NULL;
}
@@ -758,7 +891,7 @@ int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
if (!t)
t = &default_notes_tree;
assert(t->initialized);
- return for_each_note_helper(t->root, 0, 0, flags, fn, cb_data);
+ return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
}
int write_notes_tree(struct notes_tree *t, unsigned char *result)
@@ -776,11 +909,13 @@ int write_notes_tree(struct notes_tree *t, unsigned char *result)
strbuf_init(&root.buf, 256 * (32 + 40)); /* assume 256 entries */
root.path[0] = root.path[1] = '\0';
cb_data.root = &root;
+ cb_data.next_non_note = t->first_non_note;
/* Write tree objects representing current notes tree */
ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
FOR_EACH_NOTE_YIELD_SUBTREES,
write_each_note, &cb_data) ||
+ write_each_non_note_until(NULL, &cb_data) ||
tree_write_stack_finish_subtree(&root) ||
write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
strbuf_release(&root.buf);
@@ -794,6 +929,12 @@ void free_notes(struct notes_tree *t)
if (t->root)
note_tree_free(t->root);
free(t->root);
+ while (t->first_non_note) {
+ t->prev_non_note = t->first_non_note->next;
+ free(t->first_non_note->path);
+ free(t->first_non_note);
+ t->first_non_note = t->prev_non_note;
+ }
free(t->ref);
memset(t, 0, sizeof(struct notes_tree));
}
diff --git a/notes.h b/notes.h
index 5c49465..f25643e 100644
--- a/notes.h
+++ b/notes.h
@@ -36,6 +36,7 @@ int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1)
*/
extern struct notes_tree {
struct int_node *root;
+ struct non_note *first_non_note, *prev_non_note;
char *ref;
combine_notes_fn *combine_notes;
int initialized;
diff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh
index edc4bc8..75ec187 100755
--- a/t/t3303-notes-subtrees.sh
+++ b/t/t3303-notes-subtrees.sh
@@ -95,12 +95,12 @@ INPUT_END
test_expect_success 'test notes in 2/38-fanout' 'test_sha1_based "s|^..|&/|"'
test_expect_success 'verify notes in 2/38-fanout' 'verify_notes'
-test_expect_success 'test notes in 4/36-fanout' 'test_sha1_based "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout' 'verify_notes'
-
test_expect_success 'test notes in 2/2/36-fanout' 'test_sha1_based "s|^\(..\)\(..\)|\1/\2/|"'
test_expect_success 'verify notes in 2/2/36-fanout' 'verify_notes'
+test_expect_success 'test notes in 2/2/2/34-fanout' 'test_sha1_based "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|"'
+test_expect_success 'verify notes in 2/2/2/34-fanout' 'verify_notes'
+
test_same_notes () {
(
start_note_commit &&
@@ -128,14 +128,17 @@ INPUT_END
git fast-import --quiet
}
-test_expect_success 'test same notes in 4/36-fanout and 2/38-fanout' 'test_same_notes "s|^..|&/|" "s|^....|&/|"'
-test_expect_success 'verify same notes in 4/36-fanout and 2/38-fanout' 'verify_notes'
+test_expect_success 'test same notes in no fanout and 2/38-fanout' 'test_same_notes "s|^..|&/|" ""'
+test_expect_success 'verify same notes in no fanout and 2/38-fanout' 'verify_notes'
+
+test_expect_success 'test same notes in no fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" ""'
+test_expect_success 'verify same notes in no fanout and 2/2/36-fanout' 'verify_notes'
test_expect_success 'test same notes in 2/38-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'
test_expect_success 'verify same notes in 2/38-fanout and 2/2/36-fanout' 'verify_notes'
-test_expect_success 'test same notes in 4/36-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'
-test_expect_success 'verify same notes in 4/36-fanout and 2/2/36-fanout' 'verify_notes'
+test_expect_success 'test same notes in 2/2/2/34-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|"'
+test_expect_success 'verify same notes in 2/2/2/34-fanout and 2/2/36-fanout' 'verify_notes'
test_concatenated_notes () {
(
@@ -176,13 +179,16 @@ verify_concatenated_notes () {
test_cmp expect output
}
-test_expect_success 'test notes in 4/36-fanout concatenated with 2/38-fanout' 'test_concatenated_notes "s|^..|&/|" "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout concatenated with 2/38-fanout' 'verify_concatenated_notes'
+test_expect_success 'test notes in no fanout concatenated with 2/38-fanout' 'test_concatenated_notes "s|^..|&/|" ""'
+test_expect_success 'verify notes in no fanout concatenated with 2/38-fanout' 'verify_concatenated_notes'
+
+test_expect_success 'test notes in no fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" ""'
+test_expect_success 'verify notes in no fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
test_expect_success 'test notes in 2/38-fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'
test_expect_success 'verify notes in 2/38-fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
-test_expect_success 'test notes in 4/36-fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
+test_expect_success 'test notes in 2/2/36-fanout concatenated with 2/2/2/34-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|" "s|^\(..\)\(..\)|\1/\2/|"'
+test_expect_success 'verify notes in 2/2/36-fanout concatenated with 2/2/2/34-fanout' 'verify_concatenated_notes'
test_done
diff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh
index 256687f..c975a6d 100755
--- a/t/t3304-notes-mixed.sh
+++ b/t/t3304-notes-mixed.sh
@@ -131,6 +131,17 @@ data <<EOF
another non-note with SHA1-like name
EOF
+M 644 inline de/adbeefdeadbeefdeadbeefdeadbeefdeadbeef
+data <<EOF
+This is actually a valid note, albeit to a non-existing object.
+It is needed in order to trigger the "mishandling" of the dead/beef non-note.
+EOF
+
+M 644 inline dead/beef
+data <<EOF
+yet another non-note with SHA1-like name
+EOF
+
INPUT_END
git fast-import --quiet <input &&
git config core.notesRef refs/notes/commits
@@ -158,6 +169,9 @@ EXPECT_END
cat >expect_nn3 <<EXPECT_END
another non-note with SHA1-like name
EXPECT_END
+cat >expect_nn4 <<EXPECT_END
+yet another non-note with SHA1-like name
+EXPECT_END
test_expect_success "verify contents of non-notes" '
@@ -166,7 +180,27 @@ test_expect_success "verify contents of non-notes" '
git cat-file -p refs/notes/commits:deadbeef > actual_nn2 &&
test_cmp expect_nn2 actual_nn2 &&
git cat-file -p refs/notes/commits:de/adbeef > actual_nn3 &&
- test_cmp expect_nn3 actual_nn3
+ test_cmp expect_nn3 actual_nn3 &&
+ git cat-file -p refs/notes/commits:dead/beef > actual_nn4 &&
+ test_cmp expect_nn4 actual_nn4
+'
+
+test_expect_success "git-notes preserves non-notes" '
+
+ test_tick &&
+ git notes edit -m "foo bar"
+'
+
+test_expect_success "verify contents of non-notes after git-notes" '
+
+ git cat-file -p refs/notes/commits:foobar/non-note.txt > actual_nn1 &&
+ test_cmp expect_nn1 actual_nn1 &&
+ git cat-file -p refs/notes/commits:deadbeef > actual_nn2 &&
+ test_cmp expect_nn2 actual_nn2 &&
+ git cat-file -p refs/notes/commits:de/adbeef > actual_nn3 &&
+ test_cmp expect_nn3 actual_nn3 &&
+ git cat-file -p refs/notes/commits:dead/beef > actual_nn4 &&
+ test_cmp expect_nn4 actual_nn4
'
test_done
--
1.6.6.405.g80ed6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox