* Re: Improving merge failure message
From: Jeff King @ 2009-09-08 7:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Nanako Shiraishi, git
In-Reply-To: <7vk509ev20.fsf@alter.siamese.dyndns.org>
On Tue, Sep 08, 2009 at 12:48:23AM -0700, Junio C Hamano wrote:
> Thanks, very true indeed. "It would be clobbered if we were to continue
> hence we abort." is how we want to explain our behaviour, so "would" is
> definitely better here.
While we're picking apart your wording, is "clobbered" the word we want
to use? Everywhere else that is user-facing we tend to use the term
"overwritten".
-Peff
^ permalink raw reply
* Re: Improving merge failure message
From: Junio C Hamano @ 2009-09-08 7:48 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Nanako Shiraishi, git
In-Reply-To: <fabb9a1e0909080020i3daa92aar952fc5102df62bdb@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> On Tue, Sep 8, 2009 at 09:15, Junio C Hamano<gitster@pobox.com> wrote:
>> + /* would_overwrite */
>> + "Your local changes to '%s' will be clobbered by merge. Aborting.",
>
> Still scary, shouldn't that be s/will be/would be/ ?
Thanks, very true indeed. "It would be clobbered if we were to continue
hence we abort." is how we want to explain our behaviour, so "would" is
definitely better here.
>> + /* not_uptodate_dir */
>> + "Updating '%s' would lose untracked files in it. Aborting.",
This is "merge would resolve to have a file X, but you have a directory X
in your work tree and it is not empty" case.
I'll leave the exact wording up to other people. My primary focus was to
end all of these messages with "Aborting."
This turns out to be a continuation of an older discussion thread back in
May 2008, and I do not know if anybody took up the challenge back then. I
wouldn't be surprised if "checkout", which was the topic of the old
thread, has some other scary plumbing message still seeping through to the
UI layer. Perhaps there are some other commands that needs similar kind
of love.
^ permalink raw reply
* [PATCH] Add test for ignoring pushInsteadOf when remote has explicit pushurl
From: Josh Triplett @ 2009-09-08 7:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090908071513.GA3236@feather>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
On Tue, Sep 08, 2009 at 12:15:13AM -0700, Josh Triplett wrote:
> On Tue, Sep 08, 2009 at 12:02:42AM -0700, Junio C Hamano wrote:
> > Josh Triplett <josh@joshtriplett.org> writes:
> >
> > > This configuration option allows systematically rewriting fetch-only
> > > URLs to push-capable URLs when used with push. For instance:
> > >
> > > [url "ssh://example.org/"]
> > > pushInsteadOf = "git://example.org/"
> > >
> > > This will allow clones of "git://example.org/path/to/repo" to
> > > subsequently push to "ssh://example.org/path/to/repo", without manually
> > > configuring pushurl for that remote.
> > >
> > > Includes documentation for the new option, bash completion updates, and
> > > test cases (both that pushInsteadOf applies to push and that it does
> > > *not* apply to fetch).
> >
> > Thanks.
> >
> > I will queue these patches, but I presume you would also want to add a
> > test that pushInsteadOf is ignored for remotes with an explicit pushURL?
>
> Will do.
Done. Please add this to the queue, optionally squashing it into patch
2/2 if you prefer.
t/t5516-fetch-push.sh | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8f455c7..6889a53 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -193,6 +193,22 @@ test_expect_success 'push with pushInsteadOf' '
)
'
+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' '
+ mk_empty &&
+ TRASH="$(pwd)/" &&
+ git config "url.trash2/.pushInsteadOf" trash/ &&
+ git config remote.r.url trash/wrong &&
+ git config remote.r.pushurl "$TRASH/testrepo" &&
+ git push r refs/heads/master:refs/remotes/origin/master &&
+ (
+ cd testrepo &&
+ r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+ test "z$r" = "z$the_commit" &&
+
+ test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+ )
+'
+
test_expect_success 'push with matching heads' '
mk_test heads/master &&
--
1.6.3.3
^ permalink raw reply related
* Re: Improving merge failure message
From: Sverre Rabbelier @ 2009-09-08 7:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7veiqhgb4y.fsf@alter.siamese.dyndns.org>
Heya,
On Tue, Sep 8, 2009 at 09:15, Junio C Hamano<gitster@pobox.com> wrote:
> + /* would_overwrite */
> + "Your local changes to '%s' will be clobbered by merge. Aborting.",
Still scary, shouldn't that be s/will be/would be/ ?
> + /* not_uptodate_file */
> + "Your local changes to '%s' will be clobbered by merge. Aborting.",
Ditto.
> + /* not_uptodate_dir */
> + "Updating '%s' would lose untracked files in it. Aborting.",
Not sure, but maybe s/in it//
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: Improving merge failure message
From: Junio C Hamano @ 2009-09-08 7:15 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git
In-Reply-To: <7vbplmhr0i.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> [2]% git merge feature
>> error: Entry 'cool' not uptodate. Cannot merge.
>> fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
>>
>> ... the messages look unnecessarily scary, with two
>> "error" and "fatal" comments, and long sha1 commit names.
>
> Just a technical nit. I think these are tree object names.
>
>> Those of us who used git for some time can tell what it wants to say.
>> The merge checked the files in the working tree before doing anything,
>> found that the user has uncommitted change to a file that is involved in
>> the merge, and it stopped. And it didn't change anything. It may be "fatal"
>> but the user has much less reason to be scared about this failure than
>> the conflicting case.
>>
>> It would be nice if the message in the latter case can be toned down.
>
> Yeah, it would be nice. This actually was something that bothered me as
> well while trying to explain the recovery procedure for these two cases.
> Give me half an hour or so to cook up something...
It turns out to be a lot simpler than I thought, because 8ccba00
(unpack-trees: allow Porcelain to give different error messages,
2008-05-17) already laid enough groundwork for doing this kind of thing
easily.
Notable points are:
- End the messages with "Aborting."; they are given when the three-way
merge stops without harming the work tree;
- Do not give the extra message after unpack_trees() already errored out.
This "merging of trees failed" message was primarily for debugging
merge-recursive itself, and the end user cannot do much with the object
names given in the message anyway.
But do give it under higher verbosity level, or when it happens during
the inner merge (the "recursive" one), as unpack_trees() should not
fail for the inner merge under normal conditions.
We could later add instructions on how to recover (i.e. "stash changes
away or commit on a side branch and retry") instead of the silent
exit(128) I have down there, and then use Peff's advice.* mechanism to
squelch it (e.g. "advice.mergeindirtytree"), but they are separate topics.
merge-recursive.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 10d7913..a237240 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -170,6 +170,18 @@ static int git_merge_trees(int index_only,
int rc;
struct tree_desc t[3];
struct unpack_trees_options opts;
+ static struct unpack_trees_error_msgs msgs = {
+ /* would_overwrite */
+ "Your local changes to '%s' will be clobbered by merge. Aborting.",
+ /* not_uptodate_file */
+ "Your local changes to '%s' will be clobbered by merge. Aborting.",
+ /* not_uptodate_dir */
+ "Updating '%s' would lose untracked files in it. Aborting.",
+ /* would_lose_untracked */
+ "Untracked working tree file '%s' would be %s by merge. Aborting",
+ /* bind_overlap -- will not happen here */
+ NULL,
+ };
memset(&opts, 0, sizeof(opts));
if (index_only)
@@ -181,6 +193,7 @@ static int git_merge_trees(int index_only,
opts.fn = threeway_merge;
opts.src_index = &the_index;
opts.dst_index = &the_index;
+ opts.msgs = msgs;
init_tree_desc_from_tree(t+0, common);
init_tree_desc_from_tree(t+1, head);
@@ -1188,10 +1201,14 @@ int merge_trees(struct merge_options *o,
code = git_merge_trees(o->call_depth, common, head, merge);
- if (code != 0)
- die("merging of trees %s and %s failed",
- sha1_to_hex(head->object.sha1),
- sha1_to_hex(merge->object.sha1));
+ if (code != 0) {
+ if (show(o, 4) || o->call_depth)
+ die("merging of trees %s and %s failed",
+ sha1_to_hex(head->object.sha1),
+ sha1_to_hex(merge->object.sha1));
+ else
+ exit(128);
+ }
if (unmerged_cache()) {
struct string_list *entries, *re_head, *re_merge;
^ permalink raw reply related
* Re: [PATCHv2 2/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Josh Triplett @ 2009-09-08 7:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vws49gbql.fsf@alter.siamese.dyndns.org>
On Tue, Sep 08, 2009 at 12:02:42AM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > This configuration option allows systematically rewriting fetch-only
> > URLs to push-capable URLs when used with push. For instance:
> >
> > [url "ssh://example.org/"]
> > pushInsteadOf = "git://example.org/"
> >
> > This will allow clones of "git://example.org/path/to/repo" to
> > subsequently push to "ssh://example.org/path/to/repo", without manually
> > configuring pushurl for that remote.
> >
> > Includes documentation for the new option, bash completion updates, and
> > test cases (both that pushInsteadOf applies to push and that it does
> > *not* apply to fetch).
>
> Thanks.
>
> I will queue these patches, but I presume you would also want to add a
> test that pushInsteadOf is ignored for remotes with an explicit pushURL?
Will do.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH resend] git-pull: fix fetch-options.txt to not document --quiet and --verbose twice in git-pull.txt
From: Emmanuel Trillaud @ 2009-09-08 7:07 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Git Mailing List
In-Reply-To: <vpqd462kl4p.fsf@bauges.imag.fr>
Thank you for your patience.
Because I want to be kind to our maintainer :-), I've just finish a
complete reading of SubmitingPatches. Next time I promise : no more
whistespace mess and a commit message were it is meant to be.
Best regards
Emmanuel Trillaud
^ permalink raw reply
* Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"
From: Junio C Hamano @ 2009-09-08 7:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlo Marcelo Arenas Belon, git
In-Reply-To: <7v7hwar1fp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
>
>> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail
>> to generate a numeric count and result in the following error :
>>
>> /usr/libexec/git-core/git-rebase--interactive: line 110: (standard
>> input):1+(standard input):0: missing `)' (error token is
>> "input):1+(standard input):0")
>>
>> instead of grep counting use `wc -l` to return the line count.
>
> Thanks.
>
> How does your patch help when the user has GREP_OPTIONS=-C3 in the
> environment?
>
> I think a saner workaround for this user environment bug (or GNU grep
> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
> even in git-sh-setup.
Or even this.
git.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/git.c b/git.c
index 0b22595..3548154 100644
--- a/git.c
+++ b/git.c
@@ -450,11 +450,24 @@ static int run_argv(int *argcp, const char ***argv)
return done_alias;
}
+static void sanitize_env(void) {
+ static const char *vars[] = {
+ "GREP_OPTIONS",
+ "GREP_COLOR",
+ "GREP_COLORS",
+ NULL,
+ };
+ const char **p;
+
+ for (p = vars; *p; p++)
+ unsetenv(*p);
+}
int main(int argc, const char **argv)
{
const char *cmd;
+ sanitize_env();
cmd = git_extract_argv0_path(argv[0]);
if (!cmd)
cmd = "git-help";
^ permalink raw reply related
* Re: [PATCHv2 2/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Junio C Hamano @ 2009-09-08 7:02 UTC (permalink / raw)
To: Josh Triplett; +Cc: git
In-Reply-To: <5e58748923d9b4a182499a6ba8fa4636bce4810e.1252313313.git.josh@joshtriplett.org>
Josh Triplett <josh@joshtriplett.org> writes:
> This configuration option allows systematically rewriting fetch-only
> URLs to push-capable URLs when used with push. For instance:
>
> [url "ssh://example.org/"]
> pushInsteadOf = "git://example.org/"
>
> This will allow clones of "git://example.org/path/to/repo" to
> subsequently push to "ssh://example.org/path/to/repo", without manually
> configuring pushurl for that remote.
>
> Includes documentation for the new option, bash completion updates, and
> test cases (both that pushInsteadOf applies to push and that it does
> *not* apply to fetch).
Thanks.
I will queue these patches, but I presume you would also want to add a
test that pushInsteadOf is ignored for remotes with an explicit pushURL?
^ permalink raw reply
* [PATCH] completion: complete --scissors option of git-am
From: Stephen Boyd @ 2009-09-08 6:57 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf688e1..226e3ea 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -676,7 +676,7 @@ _git_am ()
--3way --committer-date-is-author-date --ignore-date
--ignore-whitespace --ignore-space-change
--interactive --keep --no-utf8 --signoff --utf8
- --whitespace=
+ --whitespace= --scissors
"
return
esac
--
1.6.5.rc0
^ permalink raw reply related
* Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"
From: Junio C Hamano @ 2009-09-08 6:54 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belon; +Cc: git
In-Reply-To: <20090908064756.GA14155@sajinet.com.pe>
Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
> On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:
>>
>> How does your patch help when the user has GREP_OPTIONS=-C3 in the
>> environment?
>
> It wouldn't help but at least wouldn't break aborting with an script
> error since you will always get a number.
That's actually worse, don't you think?
It is trying to count how many actions are done and how many are
remaining, and if you miscount it in that shell function, you will get
incorrect result. The function happens to be merely for reporting, but
the point is that it is better to fail loudly than doing wrong thing.
>> I think a saner workaround for this user environment bug (or GNU grep
>> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
>> even in git-sh-setup.
>
> agree, and since grep is used almost everywhere filtering in git-sh-setup
> like CDPATH is makes sense, with the only user of grep that wouldn't
> benefit from that being git-mergetool--lib.sh AFAIK.
Not at all. "git grep" itself will be broken. See my other patch for a
possible alternative approach.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2009, #02; Mon, 07)
From: Junio C Hamano @ 2009-09-08 6:48 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git
In-Reply-To: <20090908153109.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Junio C Hamano <gitster@pobox.com>
>
>> ... I'm also hoping to tag the real 1.6.5 before I vanish for
>> about a week and half later in the month....
>
> Are you coming to Japan to promote your book, perhaps giving autographs at bookstores?
It will come out around 19th if things go as planned, but I'll be arriving
the week after that so I'll be too late for that kind of thing.
Besides, who would want an autographed copy of a technical book?
I should start asking the usual suspect to take things over while I'm
offline...
^ permalink raw reply
* Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"
From: Carlo Marcelo Arenas Belon @ 2009-09-08 6:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hwar1fp.fsf@alter.siamese.dyndns.org>
On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:
>
> How does your patch help when the user has GREP_OPTIONS=-C3 in the
> environment?
It wouldn't help but at least wouldn't break aborting with an script
error since you will always get a number.
> I think a saner workaround for this user environment bug (or GNU grep
> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
> even in git-sh-setup.
agree, and since grep is used almost everywhere filtering in git-sh-setup
like CDPATH is makes sense, with the only user of grep that wouldn't
benefit from that being git-mergetool--lib.sh AFAIK.
will test and submit a fix for that later, but still think the original
patch at least improves the status quo (will protect also when using
custom grep wrappers as reported earlier) and doesn't do any harm as wc
is already a dependency as well and was part of the original code as well.
Carlo
^ permalink raw reply
* Re: Improving merge failure message
From: Junio C Hamano @ 2009-09-08 6:47 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git
In-Reply-To: <20090908153101.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> [2]% git merge feature
> error: Entry 'cool' not uptodate. Cannot merge.
> fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
>
> ... the messages look unnecessarily scary, with two
> "error" and "fatal" comments, and long sha1 commit names.
Just a technical nit. I think these are tree object names.
> Those of us who used git for some time can tell what it wants to say.
> The merge checked the files in the working tree before doing anything,
> found that the user has uncommitted change to a file that is involved in
> the merge, and it stopped. And it didn't change anything. It may be "fatal"
> but the user has much less reason to be scared about this failure than
> the conflicting case.
>
> It would be nice if the message in the latter case can be toned down.
Yeah, it would be nice. This actually was something that bothered me as
well while trying to explain the recovery procedure for these two cases.
Give me half an hour or so to cook up something...
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2009, #02; Mon, 07)
From: Nanako Shiraishi @ 2009-09-08 6:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtyzexnhm.fsf@alter.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.com>
> I tagged and pushed out 1.6.5-rc0. There are still some topics cooking in
> 'next' I would rather not leave out, but I've been hoping to keep this
> cycle short, and I'm also hoping to tag the real 1.6.5 before I vanish for
> about a week and half later in the month. so in the worst case there may
> have to be a 1.6.6 to include the leftover topics.
Are you coming to Japan to promote your book, perhaps giving autographs at bookstores?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Improving merge failure message
From: Nanako Shiraishi @ 2009-09-08 6:31 UTC (permalink / raw)
To: git
I often see my students confused after a failed merge and they can't
figure out what to do next. Two typical error messages they get are
[1]% git merge master
Auto-merging cool
CONFLICT (content): Merge conflict in cool
Automatic merge failed; fix conflicts and then commit the result.
[2]% git merge feature
error: Entry 'cool' not uptodate. Cannot merge.
fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
In the former case, the merge command gives a helpful message that
automatic merge failed because it found a conflict and tells enough
instruction to the user.
But in the latter case, the messages look unnecessarily scary, with two
"error" and "fatal" comments, and long sha1 commit names.
Those of us who used git for some time can tell what it wants to say.
The merge checked the files in the working tree before doing anything,
found that the user has uncommitted change to a file that is involved in
the merge, and it stopped. And it didn't change anything. It may be "fatal"
but the user has much less reason to be scared about this failure than
the conflicting case.
It would be nice if the message in the latter case can be toned down.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH resend] git-pull: fix fetch-options.txt to not document --quiet and --verbose twice in git-pull.txt
From: Matthieu Moy @ 2009-09-08 6:26 UTC (permalink / raw)
To: Emmanuel Trillaud; +Cc: Git Mailing List
In-Reply-To: <9f50533b0909070534q2375a793mf5d676b519eae69@mail.gmail.com>
Emmanuel Trillaud <etrillaud@gmail.com> writes:
> Hello all,
> In git-pull(1) we can read :
[...]
> Best regard
I guess this part of your message is not meant to appear in the commit
message. If you leave it here, Junio will have to edit it manually,
whereas if you put it where Documentation/SubmitingPatches suggests,
i.e.
> ---
here (between --- and diffstat), it's done automatically. Be nice to
our maintainer, and our mainainer will be nice to you ;-).
> Documentation/fetch-options.txt | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] Add more instructions about how to install git.
From: Matthieu Moy @ 2009-09-08 6:20 UTC (permalink / raw)
To: Alex Riesen; +Cc: Thiago Farina, git
In-Reply-To: <81b0412b0909072244k21a4ddf6vdc38d9f713a4a084@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> On Tue, Sep 8, 2009 at 07:11, Thiago Farina<tfransosi@gmail.com> wrote:
>> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>> ---
>> INSTALL | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/INSTALL b/INSTALL
>> index ae7f750..67abfc7 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -1,6 +1,15 @@
>>
>> Git installation
>>
>> +To install git follow these 4 steps inside your git directory:
>> +$ autoconf # Generates the configure file from configure.ac
>> +$ ./configure # Configures everything needed to build git
>> +$ make all # Compiles git based in the Makefile
>> +$ make install # Installs git in your own ~/bin directory
Your commit message doesn't say what's wrong with
| Alternatively you can use autoconf generated ./configure script to
| set up install paths (via config.mak.autogen), so you can write
| instead
|
| $ make configure ;# as yourself
| $ ./configure --prefix=/usr ;# as yourself
| $ make all doc ;# as yourself
| # make install install-doc install-html;# as root
which is already in INSTALL a few lines after what your patch adds.
>> +If you don't have autoconf installed, you can do this by:
>> +$ sudo apt-get install autoconf
>
> ... if you're on a Debian-derived system.
and if you have sudo configured.
BTW, apt-get is anyway somehow deprecaded, you should
s/apt-get/aptitude/.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] Add more instructions about how to install git.
From: Alex Riesen @ 2009-09-08 5:44 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
In-Reply-To: <1252386686-15689-1-git-send-email-tfransosi@gmail.com>
On Tue, Sep 8, 2009 at 07:11, Thiago Farina<tfransosi@gmail.com> wrote:
> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
> ---
> INSTALL | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index ae7f750..67abfc7 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -1,6 +1,15 @@
>
> Git installation
>
> +To install git follow these 4 steps inside your git directory:
> +$ autoconf # Generates the configure file from configure.ac
> +$ ./configure # Configures everything needed to build git
> +$ make all # Compiles git based in the Makefile
> +$ make install # Installs git in your own ~/bin directory
> +
> +If you don't have autoconf installed, you can do this by:
> +$ sudo apt-get install autoconf
... if you're on a Debian-derived system. The other half of the
world, which is RedHat-derived, uses RPM. And there are
other package management tools. Maybe that was a reason
why the instructions weren't included before.
^ permalink raw reply
* [PATCH] Add more instructions about how to install git.
From: Thiago Farina @ 2009-09-08 5:11 UTC (permalink / raw)
To: git; +Cc: Thiago Farina
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
INSTALL | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/INSTALL b/INSTALL
index ae7f750..67abfc7 100644
--- a/INSTALL
+++ b/INSTALL
@@ -1,6 +1,15 @@
Git installation
+To install git follow these 4 steps inside your git directory:
+$ autoconf # Generates the configure file from configure.ac
+$ ./configure # Configures everything needed to build git
+$ make all # Compiles git based in the Makefile
+$ make install # Installs git in your own ~/bin directory
+
+If you don't have autoconf installed, you can do this by:
+$ sudo apt-get install autoconf
+
Normally you can just do "make" followed by "make install", and that
will install the git programs in your own ~/bin/ directory. If you want
to do a global install, you can do
--
1.6.0.4
^ permalink raw reply related
* Re: [PATCHv5 00/14] git notes
From: Junio C Hamano @ 2009-09-08 4:16 UTC (permalink / raw)
To: Johan Herland
Cc: git, gitster, Johannes.Schindelin, trast, tavestbo, git,
chriscool, spearce
In-Reply-To: <200909080512.34634.johan@herland.net>
Johan Herland <johan@herland.net> writes:
> Furthermore, the following notes tree structures were tested:
>
> - "no-notes": Testing without any notes at all. This is only present as a
> baseline, and to verify that the notes code does not negatively affect
> performance when not in use.
Minor nit.
For this to be a baseline, you would need to have another algorithm before
"before", i.e., without any of these notes implementation.
Comparison with "before" alone is not meaningful. That is like starting
with a state with unknown performance regression compared to the stock
version, and then boast improvements made by various variations.
You would need to compare overhead of various "algorithms" with the stock
git in "no-notes" case as well. It would give us the true performance
cost of supporting notes.
^ permalink raw reply
* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-08 3:12 UTC (permalink / raw)
To: git; +Cc: gitster, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <1252376822-6138-1-git-send-email-johan@herland.net>
[-- Attachment #1: Type: Text/Plain, Size: 4066 bytes --]
On Tuesday 08 September 2009, Johan Herland wrote:
> I have some performance numbers that I will send in a separate email.
Ok, here we go:
Test scenario:
Linux kernel repo with 157118 commits, 1 note per commit, with notes
organized into various fanout schemes.
Hardware is Intel Core 2 Quad with 4GB RAM.
The tests were done on the following algorithms:
- "before": This is the state of the notes code after applying patches 1-9.
It uses the original notes-in-hash-map implementation, and does not grok
any fanout scheme.
- "16tree": This is the state of the notes code after applying patch 10.
It uses the 16-tree data structure that parses the SHA-1 based fanout
schemes.
- "flexible": This is the state of the notes code after applying the entire
patch series. This code parses a variety of date- and SHA1-based fanout
schemes.
Furthermore, the following notes tree structures were tested:
- "no-notes": Testing without any notes at all. This is only present as a
baseline, and to verify that the notes code does not negatively affect
performance when not in use.
- "no-fanout": All notes stored directly inside the root notes tree object.
- "2_38": All notes stored in a SHA1-based 2/38 fanout scheme.
- "2_2_36": All notes stored in a SHA1-based 2/2/36 fanout scheme.
- "ym": Notes are organized within "yYYYYmMM"-named subtrees, where "YYYY"
and "MM" are the year and month (respectively) from the annotated commit's
commit date.
- "ym_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYYmMM"-named subtrees.
- "ymd": Notes are organized within "yYYYYmMMdDD"-named subtrees.
- "ymd_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYYmMMdDD"-named subtrees.
- "y_m": Notes are organized within two-level "yYYYY/mMM" subtrees.
- "y_m_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYY/mMM"-named subtrees.
- "y_m_d": Notes are organized within three-level "yYYYY/mMM/dDD" subtrees.
- "y_m_d_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme
within the "yYYYY/mMM/dDD"-named subtrees.
Here are the runtime numbers, the first column shows the runtime for 100
repetitions of "git log -n10" (which we assume to be a common use case),
and the second column shows the runtime from a single run of
"git log --all" (which is somewhat closer to a worst case).
Algorithm / Notes tree git log -n10 (x100) git log --all
------------------------------------------------------------
before / no-notes 4.78s 63.90s
before / no-fanout 56.85s 65.69s
16tree / no-notes 4.77s 64.18s
16tree / no-fanout 30.35s 65.39s
16tree / 2_38 5.57s 65.42s
16tree / 2_2_36 5.19s 65.76s
flexible / no-notes 4.78s 63.91s
flexible / no-fanout 30.34s 65.57s
flexible / 2_38 5.57s 65.46s
flexible / 2_2_36 5.18s 65.72s
flexible / ym 5.13s 65.66s
flexible / ym_2_38 5.08s 65.63s
flexible / ymd 5.30s 65.45s
flexible / ymd_2_38 5.29s 65.90s
flexible / y_m 5.11s 65.72s
flexible / y_m_2_38 5.08s 65.67s
flexible / y_m_d 5.06s 65.50s
flexible / y_m_d_2_38 5.07s 65.79s
Finally, I have also looked at the memory consumption of the various
algorithms and fanout schemes:
The memory usage was measured by calculating the #bytes dynamically
allocated for the notes data structure, and printing the current
usage every time get_commit_notes() was called during a complete run
of "git log --all".
The results are attached as two gnuplot graphs, one with regular
axes, and one with logarithmic axes.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
[-- Attachment #2: memusage_gnuplot.png --]
[-- Type: image/png, Size: 18323 bytes --]
[-- Attachment #3: memusage_gnuplot_log.png --]
[-- Type: image/png, Size: 28646 bytes --]
^ permalink raw reply
* [PATCHv5 10/14] Teach notes code to free its internal data structures on request.
From: Johan Herland @ 2009-09-08 2:26 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <1252376822-6138-1-git-send-email-johan@herland.net>
There's no need to be rude to memory-concious callers...
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 7 +++++++
notes.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/notes.c b/notes.c
index 84c30c1..008c3d4 100644
--- a/notes.c
+++ b/notes.c
@@ -160,3 +160,10 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
free(msg);
}
+
+void free_commit_notes()
+{
+ free(hash_map.entries);
+ memset(&hash_map, 0, sizeof(struct hash_map));
+ initialized = 0;
+}
diff --git a/notes.h b/notes.h
index 7f3eed4..41802e5 100644
--- a/notes.h
+++ b/notes.h
@@ -7,4 +7,6 @@
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
const char *output_encoding, int flags);
+void free_commit_notes();
+
#endif
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv5 09/14] Add '%N'-format for pretty-printing commit notes
From: Johan Herland @ 2009-09-08 2:26 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <1252376822-6138-1-git-send-email-johan@herland.net>
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
---
Documentation/pretty-formats.txt | 1 +
pretty.c | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 2a845b1..5fb10b3 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -123,6 +123,7 @@ The placeholders are:
- '%s': subject
- '%f': sanitized subject line, suitable for a filename
- '%b': body
+- '%N': commit notes
- '%Cred': switch color to red
- '%Cgreen': switch color to green
- '%Cblue': switch color to blue
diff --git a/pretty.c b/pretty.c
index 01eadd0..7f350bb 100644
--- a/pretty.c
+++ b/pretty.c
@@ -702,6 +702,10 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
case 'd':
format_decoration(sb, commit);
return 1;
+ case 'N':
+ get_commit_notes(commit, sb, git_log_output_encoding ?
+ git_log_output_encoding : git_commit_encoding, 0);
+ return 1;
}
/* For the rest we have to parse the commit header. */
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv5 13/14] Allow flexible organization of notes trees, using both commit date and SHA1
From: Johan Herland @ 2009-09-08 2:27 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <1252376822-6138-1-git-send-email-johan@herland.net>
This is a major expansion of the notes lookup code to allow for variations
in the notes tree organization. The variations allowed include mixing fanout
schemes based on the commit dates of the annotated commits (aka. date-based
fanout) with fanout schemes based on the SHA1 of the annotated commits (aka.
SHA1-based fanout).
Using date-based fanout in the notes tree structure enables considerable
speedup in the notes lookup process, since notes are almost always looked up
sequentially in the (reverse) chronological order of their associated commits.
Furthermore, organizing notes in a way that allow (near) sequential lookup,
enables us to decrease memory consumption both by lazily loading parts of the
notes tree structure on-demand, and freeing parts of the notes structure that
are unlikely to be used again soon.
The new flexible organization of the notes tree changes the rules for valid
note tree entries. The new rules are as follows:
1. Note objects are named by the SHA1 of the commit they annotate, possibly
split across several SHA1-based fanout levels (this is the same as is
implemented earlier in this series).
2. Note entries are located within zero or more date-based fanout levels.
3. Date-based fanout schemes may use the year, month and day values of the
associated commit's timestamp. The values must be prefixed by 'y', 'm'
and 'd' (respectively) in the notes tree.
4. The date-based components can be combined in one fanout level, or split
across multiple fanout levels. Individual components may not be split
across multiple fanout levels.
5. The year/month/date values must be specified in that order, and month or
date values may not occur without the preceding year or month value.
6. All entries of a tree object in the notes tree structure must follow the
same scheme used at that level.
Thus, the following example note entries are all valid locations for a note
annotating commit 123456789abcdef0123456789abcdef0123456789 at 2009-09-01:
- 123456789abcdef0123456789abcdef0123456789
- 12/3456789abcdef0123456789abcdef0123456789
- 1234/56789abcdef0123456789abcdef0123456789
- 12/34/56789abcdef0123456789abcdef0123456789
- 1234/5678/9abcdef0123456789abcdef0123456789
- 1234/56/78/9abcdef0123456789abcdef0123456789
- y2009/123456789abcdef0123456789abcdef0123456789
- y2009/m09/12/3456789abcdef0123456789abcdef0123456789
- y2009/m09/d01/123456789abcdef0123456789abcdef0123456789
- y2009m09/12/34/56789abcdef0123456789abcdef0123456789
- y2009m09/d01/1234/567/89abcdef0123456789abcdef0123456789
- y2009/m09d01/12/34/56/78/9abcdef0123456789abcdef0123456789
- y2009m09d01/123456789abcdef0123456789abcdef0123456789
Conversely, the following example note entries are all invalid:
- 1/23456789abcdef0123456789abcdef0123456789 (violates #1)
- 123/456789abcdef0123456789abcdef0123456789 (violates #1)
- 12/345/6789abcdef0123456789abcdef0123456789 (violates #1)
- y2009123456789abcdef0123456789abcdef0123456789 (violates #2)
- 2009/09/01/123456789abcdef0123456789abcdef0123456789 (violates #3)
- y20/09/m09/12/3456789abcdef0123456789abcdef0123456789 (violates #4)
- y20/09m09/d01/123456789abcdef0123456789abcdef0123456789 (violates #4)
- y2009m/09/12/34/56789abcdef0123456789abcdef0123456789 (violates #4)
- y2009/d01/1234/5678/9abcdef0123456789abcdef0123456789 (violates #5)
- m09/y2009/d01/12/34/56/78/9abcdef0123456789abcdef0123456789 (violates #5)
>From rule #6, we see that the following example notes tree is valid:
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/123456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/d31/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/4567/89abcdef0123456789abcdef0123456789abc
- y2008m01/d16/5678/9abcdef0123456789abcdef0123456789abcd
Conversely the following structure is invalid (violates rule #6):
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/12/3456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/45/6789abcdef0123456789abcdef0123456789abc
- y2008/m01d16/5678/9abcdef0123456789abcdef0123456789abcd
The flexibility added by this patch adds considerable complexity to the notes
tree parser, but the runtime and memory usage is not significantly affected
(except for the effects introduced by the chosen notes tree structure).
Internally, the 16-tree data structure introduced in earlier patches is still
used to hold the SHA1-based fanout levels and the note entries themselves.
However, this patch adds a hierarchical date-based linked-list structure
around the 16-tree structure that mirrors the fanout scheme used in the
actual notes tree.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 403 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 364 insertions(+), 39 deletions(-)
diff --git a/notes.c b/notes.c
index 6926aa6..a3e3f83 100644
--- a/notes.c
+++ b/notes.c
@@ -7,6 +7,70 @@
#include "tree-walk.h"
/*
+ * Format of entries in the notes tree structure:
+ *
+ * note-entry ::= (period sep)? sha1-spec
+ * period ::= year sep?
+ * (month sep?
+ * (date sep?)?
+ * )?;
+ * year ::= 'y' yearnum;
+ * month ::= 'm' monthnum;
+ * date ::= 'd' datenum;
+ * yearnum ::= # 4-digit decimal year, from annotated commit's timestamp;
+ * monthnum ::= # 2-digit decimal month, from annotated commit's timestamp;
+ * datenum ::= # 2-digit decimal date, from annotated commit's timestamp;
+ * sha1-spec ::= (hex-fragment sep?){20}
+ * sep ::= '/';
+ * hex-fragment ::= # Fragment of hexsha1 (2 bytes);
+ * hexsha1 ::= # SHA1 of annotated commit in hex format (40 bytes);
+ *
+ * Thus, the following example note entries are all valid:
+ * - 0123456789abcdef0123456789abcdef012345678
+ * - 01/23456789abcdef0123456789abcdef012345678
+ * - 0123/456789abcdef0123456789abcdef012345678
+ * - 01/23/456789abcdef0123456789abcdef012345678
+ * - 0123/4567/89abcdef0123456789abcdef012345678
+ * - 0123/45/67/89abcdef0123456789abcdef012345678
+ * - y2009/0123456789abcdef0123456789abcdef012345678
+ * - y2009/m09/01/23456789abcdef0123456789abcdef012345678
+ * - y2009/m09/d01/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/01/23/456789abcdef0123456789abcdef012345678
+ * - y2009m09/d01/0123/4567/89abcdef0123456789abcdef012345678
+ * - y2009/m09d01/01/23/45/67/89abcdef0123456789abcdef012345678
+ * - y2009m09d01/0123456789abcdef0123456789abcdef012345678
+ *
+ * and the following example note entries are all invalid:
+ * - 0/123456789abcdef0123456789abcdef012345678
+ * - 012/3456789abcdef0123456789abcdef012345678
+ * - 01/234/56789abcdef0123456789abcdef012345678
+ * - y20090123456789abcdef0123456789abcdef012345678
+ * - y20/09/m09/01/23456789abcdef0123456789abcdef012345678
+ * - y20/09m09/d01/0123456789abcdef0123456789abcdef012345678
+ * - y2009m/09/01/23/456789abcdef0123456789abcdef012345678
+ * - y2009/d01/0123/4567/89abcdef0123456789abcdef012345678
+ * - m09/y2009/d01/01/23/45/67/89abcdef0123456789abcdef012345678
+ *
+ * In addition to the above per-entry rules, we require that _all_ entries at
+ * a given level in the notes tree (levels are separated by '/') follow the
+ * exact same format at that level. Thus the following structure is valid:
+ * - y2009m09/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/123456789abcdef0123456789abcdef0123456789
+ * - y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
+ * - y2008m01/d31/34/56789abcdef0123456789abcdef0123456789ab
+ * - y2008m01/d16/4567/89abcdef0123456789abcdef0123456789abc
+ * - y2008m01/d16/5678/9abcdef0123456789abcdef0123456789abcd
+ *
+ * but the following structure is invalid:
+ * - y2009m09/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/12/3456789abcdef0123456789abcdef0123456789
+ * - y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
+ * - y2008m01/34/56789abcdef0123456789abcdef0123456789ab
+ * - y2008m01/d16/45/6789abcdef0123456789abcdef0123456789abc
+ * - y2008/m01d16/5678/9abcdef0123456789abcdef0123456789abcd
+ */
+
+/*
* Use a non-balancing simple 16-tree structure with struct int_node as
* internal nodes, and struct leaf_node as leaf nodes. Each int_node has a
* 16-array of pointers to its children.
@@ -17,9 +81,45 @@
* - ptr & 3 == 3 - pointer to subtree entry - cast to struct leaf_node *
*
* The root node is a statically allocated struct int_node.
+ *
+ * In order to allow date-based fanout schemes in addition to the original
+ * SHA1-based fanout schemes, we need to overload this structure, as follows:
+ * If the first pointer in the 16-array is ~0 (i.e. 0xffffffff on 32-bit
+ * systems and 0xffffffffffffffff on 64-bit systems), then the int_node is NOT
+ * to be interpreted as a 16-array of child node pointers. Rather, the int_node
+ * now represents a period-based node with the following properties:
+ * - The node has a pointer to a "child" node of type struct int_node, which is
+ * EITHER a "regular" int_node object representing the root node of a 16-tree
+ * structure holding notes associated with commits with timestamps within
+ * that time period, OR another period-based int_node representing some
+ * subdivision of the time period.
+ * - The node also has a pointer to a "previous" period-based int_node, which
+ * represents the previous time period for which there exist note objects.
+ * - The node has a pointer to a "parent" node, which is the period-based
+ * int_node that has this int_node as one of its children. This is needed
+ * when traversing the date-based int_nodes looking for a period matching the
+ * given commit. For top-level objects, this is set to NULL.
+ * - The node stores the SHA1 sum of the tree object that represents its child
+ * (within the notes tree structure). Thus, we keep a reference to the child
+ * structure that without necessarily allocating the child node (and
+ * underlying structure).
+ * - Finally, the node has a period string, which indicates the time period of
+ * the notes contained within, typically of the form "YYYY", "YYYY-MM" or
+ * "YYYY-MM-DD", depending on the granularity of the corresponding
+ * period-based entries in the notes tree structure.
*/
struct int_node {
- void *a[16];
+ union {
+ void *a[16];
+ struct {
+ void *magic; /* ~0 "enables" this part of the union */
+ struct int_node *child;
+ struct int_node *prev;
+ struct int_node *parent;
+ unsigned char tree_sha1[20];
+ char period[11]; /* Enough to hold "YYYY-MM-DD" */
+ };
+ };
};
/*
@@ -51,12 +151,18 @@ struct leaf_node {
#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
+#define SUBTREE_DATE_PREFIXCMP(commit_date, subtree_date) \
+ (prefixcmp(commit_date, subtree_date))
+
static struct int_node root_node;
+static struct int_node *cur_node;
+
static int initialized;
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
- unsigned int n);
+static void load_subtree(const unsigned char *sha1,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent, int n);
/*
* To find a leaf_node:
@@ -94,7 +200,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
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(l->val_sha1, l->key_sha1, l->key_sha1[19],
+ tree, NULL, (int) n);
free(l);
return note_tree_find(tree, n, key_sha1);
}
@@ -117,7 +224,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
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(l->val_sha1, l->key_sha1, l->key_sha1[19], tree,
+ NULL, (int) n);
free(l);
return note_tree_find(tree, n, key_sha1);
}
@@ -173,16 +281,28 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,
/* Free the entire notes data contained in the given tree */
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)) {
- case PTR_TYPE_INTERNAL:
- note_tree_free(CLR_PTR_TYPE(p));
- /* fall through */
- case PTR_TYPE_NOTE:
- case PTR_TYPE_SUBTREE:
- free(CLR_PTR_TYPE(p));
+ if (tree->magic == (void *) ~0) {
+ if (tree->prev) {
+ note_tree_free(tree->prev);
+ free(tree->prev);
+ }
+ if (tree->child) {
+ note_tree_free(tree->child);
+ free(tree->child);
+ }
+ }
+ else {
+ unsigned int i;
+ for (i = 0; i < 16; i++) {
+ void *p = tree->a[i];
+ switch(GET_PTR_TYPE(p)) {
+ case PTR_TYPE_INTERNAL:
+ note_tree_free(CLR_PTR_TYPE(p));
+ /* fall through */
+ case PTR_TYPE_NOTE:
+ case PTR_TYPE_SUBTREE:
+ free(CLR_PTR_TYPE(p));
+ }
}
}
}
@@ -215,29 +335,139 @@ 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)
+/*
+ * Parse year/month/date strings, and generate the corresponding period string
+ * for the given path entry:
+ * - prefix must follow one of these forms: "", "YYYY", "YYYY-MM"
+ * - path should follow one of these forms: "yYYYY", "yYYYYmMM", "yYYYYmMMdDD",
+ * "mMMdDD", "mMM" or "dDD"
+ * The resulting string (which follows the form "YYYY", "YYYY-MM" or
+ * "YYYY-MM-DD") is returned as a static string. If path is not valid in the
+ * given (prefix) context, NULL is returned.
+ */
+static const char *parse_period(const char *prefix, unsigned int prefix_len,
+ const char *path, unsigned int path_len)
+{
+ static char result[11];
+ char expect_type; /* y/m/d for year/month/day-based fanout */
+ unsigned int expect_len, value;
+ char *endptr, *target = result;
+
+ switch (prefix_len) {
+ case 0:
+ /* No prefix, expect year-based fanout in path */
+ expect_type = 'y';
+ expect_len = 4;
+ break;
+ case 4:
+ /* Year in prefix, expect month-based fanout in path */
+ expect_type = 'm';
+ expect_len = 2;
+ break;
+ case 7:
+ /* "YYYY-MM" in prefix, expect day-based fanout in path */
+ expect_type = 'd';
+ expect_len = 2;
+ break;
+ default:
+ die("Date-based notes tree loading invoked with invalid "
+ "prefix '%.*s'", prefix_len, prefix);
+ }
+
+ if (path[0] != expect_type) {
+ warning("Unexpected entry path in date-based notes tree: '%s' "
+ "(skipping)", path);
+ return NULL;
+ }
+ value = (unsigned int) strtoul(path + 1, &endptr, 10);
+ switch (expect_type) {
+ case 'y':
+ if (value < 1969 || value >= 3000) {
+ warning("Invalid year value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ case 'm':
+ if (value < 1 || value > 12) {
+ warning("Invalid month value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ case 'd':
+ if (value < 1 || value > 31) {
+ warning("Invalid day value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ }
+
+ if (prefix == result) {
+ target = result + prefix_len;
+ prefix = NULL;
+ prefix_len = 0;
+ }
+ prefix_len = snprintf(target, 11, "%.*s%s%0*u", prefix_len, prefix,
+ expect_len == 2 ? "-" : "", expect_len, value);
+ prefix_len += target - result;
+ assert(prefix_len < 11);
+
+ if (*endptr) /* there are more components in this path */
+ return parse_period(result, prefix_len, endptr,
+ path_len - (endptr - path));
+ return result;
+}
+
+static void load_date_subtree(struct tree_desc *tree_desc,
+ const char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent)
+{
+ struct name_entry entry;
+ struct int_node *cur_node = NULL;
+ struct int_node *new_node;
+
+ while (tree_entry(tree_desc, &entry)) {
+ const char *period = parse_period(
+ prefix, prefix_len, entry.path, strlen(entry.path));
+ if (!period)
+ continue;
+ if (tree_desc->size) /* this is not the last tree entry */
+ new_node = (struct int_node *)
+ xmalloc(sizeof(struct int_node));
+ else /* this is the last entry, store directly into node */
+ new_node = node;
+
+ new_node->magic = (void *) ~0;
+ new_node->child = NULL;
+ new_node->prev = cur_node;
+ new_node->parent = parent;
+ hashcpy(new_node->tree_sha1, entry.sha1);
+ strcpy(new_node->period, period);
+ cur_node = new_node;
+ }
+ assert(!cur_node || cur_node == node);
+}
+
+static void load_sha1_subtree(struct tree_desc *tree_desc,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, unsigned char n)
{
unsigned char commit_sha1[20];
- unsigned int prefix_len;
int status;
- void *buf;
- struct tree_desc desc;
struct name_entry entry;
- buf = fill_tree_descriptor(&desc, subtree->val_sha1);
- if (!buf)
- die("Could not read %s for notes-index",
- sha1_to_hex(subtree->val_sha1));
-
- prefix_len = subtree->key_sha1[19];
assert(prefix_len * 2 >= n);
- memcpy(commit_sha1, subtree->key_sha1, prefix_len);
- while (tree_entry(&desc, &entry)) {
+ memcpy(commit_sha1, prefix, prefix_len);
+ while (tree_entry(tree_desc, &entry)) {
int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
commit_sha1 + prefix_len, 20 - prefix_len);
- if (len < 0)
+ if (len < 0) {
+ warning("Invalid value in notes tree: '%s' (skipping)",
+ entry.path);
continue; /* entry.path is not a SHA1 sum. Skip */
+ }
len += prefix_len;
/*
@@ -258,6 +488,42 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
assert(!status);
}
}
+}
+
+static void load_subtree(const unsigned char *sha1,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent, int n)
+{
+ void *buf;
+ struct tree_desc desc;
+
+ buf = fill_tree_descriptor(&desc, sha1);
+ if (!buf)
+ die("Could not read notes subtree at %s", sha1_to_hex(sha1));
+ /*
+ * After fill_tree_descriptor(), we can peek at the first tree entry
+ * in desc.entry.
+ */
+ switch (desc.entry.path[0]) {
+ case 'd':
+ if (strlen(desc.entry.path) != 3)
+ break;
+ /* fall-through */
+ case 'm':
+ case 'y':
+ /* path cannot be a SHA1 fragment */
+ load_date_subtree(&desc, (const char *) prefix, prefix_len,
+ node, parent);
+ free(buf);
+ return;
+ }
+ if (n < 0) {
+ /* Arriving from a date-based subtree; reset prefix */
+ n = 0;
+ prefix = NULL;
+ prefix_len = 0;
+ }
+ load_sha1_subtree(&desc, prefix, prefix_len, node, n);
free(buf);
}
@@ -265,23 +531,81 @@ static void initialize_notes(const char *notes_ref_name)
{
unsigned char sha1[20], commit_sha1[20];
unsigned mode;
- struct leaf_node root_tree;
if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
get_tree_entry(commit_sha1, "", sha1, &mode))
return;
- hashclr(root_tree.key_sha1);
- hashcpy(root_tree.val_sha1, sha1);
- load_subtree(&root_tree, &root_node, 0);
+ load_subtree(sha1, NULL, 0, &root_node, NULL, 0);
+ cur_node = &root_node;
}
-static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+static unsigned char *lookup_notes(const struct commit *commit)
{
- struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
- if (found)
- return found->val_sha1;
- return NULL;
+ struct int_node *node = cur_node, *seen_node = cur_node;
+ struct leaf_node *found;
+ const char *short_date;
+
+ if (!node)
+ return NULL;
+
+ /* Convert commit->date to YYYY-MM-DD format */
+ short_date = show_date(commit->date, 0, DATE_SHORT);
+
+ while (node->magic == (void *) ~0) { /* date-based node */
+ int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->period);
+ if (cmp == 0) {
+ /* Search inside child node */
+ if (!node->child) {
+ /* Must unpack child node first */
+ node->child = (struct int_node *)
+ xcalloc(sizeof(struct int_node), 1);
+ load_subtree(node->tree_sha1,
+ (const unsigned char *) node->period,
+ strlen(node->period), node->child,
+ node, -1);
+ }
+ seen_node = node;
+ node = node->child;
+ }
+ else if (cmp > 0) {
+ /* Search in past node */
+ if (node->prev)
+ node = node->prev;
+ else
+ node = node->parent;
+ }
+ else {
+ /* Search in future node */
+ if (!node->parent) {
+ /* Restart from root_node */
+ seen_node = node;
+ node = &root_node;
+ }
+ else
+ node = node->parent;
+ }
+ if (!node || node == seen_node) {
+ /* We've been here before, give up search */
+ return NULL;
+ }
+ }
+ while (cur_node &&
+ SUBTREE_DATE_PREFIXCMP(cur_node->period, seen_node->period) < 0)
+ {
+ /*
+ * We're about to move cur_node backwards in history. We are
+ * unlikely to need this cur_node in the future, so free() it.
+ */
+ note_tree_free(cur_node->child);
+ cur_node->child = NULL;
+ cur_node = cur_node->parent;
+ }
+ cur_node = seen_node;
+
+ /* Drill down further with SHA1-based lookup */
+ found = note_tree_find(node, 0, commit->object.sha1);
+ return found ? found->val_sha1 : NULL;
}
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
@@ -303,7 +627,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
initialized = 1;
}
- sha1 = lookup_notes(commit->object.sha1);
+ sha1 = lookup_notes(commit);
if (!sha1)
return;
@@ -342,6 +666,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
void free_commit_notes()
{
+ cur_node = NULL;
note_tree_free(&root_node);
memset(&root_node, 0, sizeof(struct int_node));
initialized = 0;
--
1.6.4.304.g1365c.dirty
^ 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