* Re: [PATCH] git-mailinfo: Fix getting the subject from the body
From: Lukas Sandström @ 2008-07-12 21:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Don Zickus
In-Reply-To: <7v3amfxx3a.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Lukas Sandström <lukass@etek.chalmers.se> writes:
>
>> "Subject: " isn't in the static array "header", and thus
>> memcmp("Subject: ", header[i], 7) will never match.
>>
>> Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
>> ---
>>
>> This has been broken since 2007-03-12, with commit
>> 87ab799234639c26ea10de74782fa511cb3ca606
>> so it might not be very important.
>>
>> builtin-mailinfo.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
>> index 962aa34..2d1520f 100644
>> --- a/builtin-mailinfo.c
>> +++ b/builtin-mailinfo.c
>> @@ -334,7 +334,7 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
>> return 1;
>> if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
>> for (i = 0; header[i]; i++) {
>> - if (!memcmp("Subject: ", header[i], 9)) {
>> + if (!memcmp("Subject", header[i], 7)) {
>> if (! handle_header(line, hdr_data[i], 0)) {
>> return 1;
>> }
>
> Actually, I do not think your patch alone makes any difference, and the
> original code looks somewhat bogus. If there is no "Subject: " in the
> same section of the message (either in e-mail header in which case
> hdr_data == p_hdr_data[], or in the message body part in which case
> hdr_data == s_hdr_data[]), hdr_data[1] will be NULL, because the only
> place that allocates the storage for the data is the first loop of this
> function that deals with real-RFC2822-header-looking lines.
>
> You'd probably need something like this on top of your patch to actually
> activate the code.
Right, I noticed that too. It's fixed in the strbuf conversion, I think.
Lukas Sandström <lukass@etek.chalmers.se> wrote:
> After looking at this part some more, I see that there is no guarantee
> that hdr_data[i] != NULL in this codepath, and then we won't use the
> subject anyway.
I'll be hiking the next week, in case you wonder why I'm not responding.
I'll try to get another version of the patches out before I leave.
/Lukas
^ permalink raw reply
* Re: git-rebase eats empty commits
From: Stephan Beyer @ 2008-07-12 22:12 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <g4vrrm$g35$1@ger.gmane.org>
Hi,
Michael J Gruber wrote:
> "git commit" allows empty commits with the "--allow-empty" option, i.e.
> commits which introduce no change at all. This is sometimes useful for
> keeping a log of untracked work related to tracked content.
>
> "git rebase" removes empty commits, for the good reason that rebasing
> may make certain commits obsolete; but I don't want that in the case
> mentioned above. Is there any way to specify "--preserve-empty" or
> similar?
First I can speak for git-sequencer: there is no such thing as a
"preserve empty" option, but currently, when you are picking a commit
that has already been applied so that no changes occur, it will pause.
(It will not pause if it is a fast-forward.)
Yet, I was unsure if this is a "correct" behavior, but it seemed to be
useful, because you can inspect the situation.
In my mind, the same should happen with an empty commit, so I tested it:
1. It pauses.
2. In that pause I only need to run "git commit --allow-empty" and I have
the picked empty patch with that commit message.
So if this behavior is kept, there is no such need for such an option.
Now I'm checking it with the old rebase-i (I'm always referring to
git-rebase--interactive as rebase-i) and exactly the same behavior
occurs.
But rebase is not rebase-i.
So I've also checked both, pure rebase and rebase-m: then the empty commit
is lost.
To sum up, use rebase -i and when it's pausing, do "git commit --allow-empty"
and then "git rebase --continue" and you have what you want.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [HACK] t/test-lib.sh HACK: Add -s/--show-hack to test suite.
From: Stephan Beyer @ 2008-07-12 22:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807071540310.18205@racer>
Hi,
I'm just cleaning up my inbox and I've seen I've not yet replied to your
mail.
Johannes Schindelin wrote:
> Hi,
>
> On Mon, 7 Jul 2008, Stephan Beyer wrote:
>
> > > > This option realizes a stupid hack that tries to run the test cases
> > > > line by line (separated by &&).
> > >
> > > In what way is that better than "sh -x t????-*.sh"?
> >
> > Your suggestion is more like "./t????-*.sh -v" instead of -s, at least
> > on bash and dash here.
>
> No, I meant without "-v".
Me, too.
I've written something different:
"sh -x" is a great thing and does exactly what it should on simple scripts
containing:
foo &&
bar &&
baz
But for a test case in the git test suite it does not work, unfortunately.
(Tested on bash, dash and zsh.)
The information I get from
sh -x ./t????-*.sh
is like the information I get from invoking
./t????-*.sh -v
but less eye-pleasing.
And ./t????-*.sh -s (using this patch) shows me something like:
Testing:
foo
Testing:
bar
* FAIL: blabla
So that I what *command* of the test case fails.
But perhaps I am just doing something wrong.
> > But I didn't know the -x flag and it seems that this could be used in
> > test-lib.sh to make the hack faster, more robust and less hacky ;-)
>
> It would obsolete your hack, I suggest. Obviously, you haven't tried it
> yet.
The obvious is wrong.
I would be very happy to obsolete my slow and error-prone hack, but
currently I have not seen a good alternative.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* [PATCH] Add a new test for git-merge-resolve
From: Miklos Vajna @ 2008-07-12 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1215888130-3260-1-git-send-email-vmiklos@frugalware.org>
Actually this is a simple test, just to ensure merge-resolve properly
calls read-tree. read-tree itself already has more complex tests.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
This one adds tests in a new file; it tests merge-resolve as well, just
as my previous patch, but I though it's better if I just add the
multiple-base merge one to the end of the existing criss-cross tests.
So this is not a replacement, both are sent for inclusion.
t/t7605-merge-resolve.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 46 insertions(+), 0 deletions(-)
create mode 100755 t/t7605-merge-resolve.sh
diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
new file mode 100755
index 0000000..ee21a10
--- /dev/null
+++ b/t/t7605-merge-resolve.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing the resolve strategy.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo c0 > c0.c &&
+ git add c0.c &&
+ git commit -m c0 &&
+ git tag c0 &&
+ echo c1 > c1.c &&
+ git add c1.c &&
+ git commit -m c1 &&
+ git tag c1 &&
+ git reset --hard c0 &&
+ echo c2 > c2.c &&
+ git add c2.c &&
+ git commit -m c2 &&
+ git tag c2 &&
+ git reset --hard c0 &&
+ echo c3 > c2.c &&
+ git add c2.c &&
+ git commit -m c3 &&
+ git tag c3
+'
+
+test_expect_success 'merge c1 to c2' '
+ git reset --hard c1 &&
+ git merge -s resolve c2 &&
+ test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
+ test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
+ test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
+ git diff --exit-code &&
+ test -f c0.c &&
+ test -f c1.c &&
+ test -f c2.c
+'
+
+test_expect_success 'merge c2 to c3 (fails)' '
+ git reset --hard c2 &&
+ test_must_fail git merge -s resolve c3
+'
+test_done
--
1.5.6.2.450.g8d367.dirty
^ permalink raw reply related
* Re: [GitStats] Bling bling or some statistics on the git.git repository
From: Sverre Rabbelier @ 2008-07-12 22:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailinglist, David Symonds
In-Reply-To: <alpine.DEB.1.00.0807120028280.8950@racer>
On Sat, Jul 12, 2008 at 1:33 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Fair enough. As long as you use a language that is easy to prototype
> quickly and dirtily in. Such as Python.
It turned out to work really nicely, especially since I already had a
parser for diffs ;).
> Let's go for it, then!
My first attempt at something usable is at [0].
> That is why I was suggesting using the diff tool with munged input to find
> out what works best.
I used python's difflib so that I didn't have to write out to a file.
> When that is done, I'll turn it into C.
Goodluckwiththat ;D.
[0] http://repo.or.cz/w/git-stats.git?a=commit;h=ac2a192f1dcec387e96f2a7eb5d66a38401b0722
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* (no subject)
From: Kevin Phair @ 2008-07-12 22:31 UTC (permalink / raw)
To: git
subscribe
^ permalink raw reply
* Re: [RFC PATCH] Fix quadratic performance in rewrite_one.
From: Linus Torvalds @ 2008-07-12 22:55 UTC (permalink / raw)
To: Alexander N. Gavrilov; +Cc: git, Junio C Hamano
In-Reply-To: <200807122200.58187.angavrilov@gmail.com>
On Sat, 12 Jul 2008, Alexander N. Gavrilov wrote:
>
> Parent commits are usually older than their children. Thus,
> on each iteration of the loop in rewrite_one, add_parents_to_list
> traverses all commits previously processed by the loop.
> It performs very poorly in case of very long rewrite chains.
Good call, but you don't seem to invalidate the cache when we remove
things from the list.
The top of the limit_list() loop does that "get top entry from list, an
free it", and I'm not seeing you invalidating the cache if that entry that
just got free'd happened to be the cache entry?
Or did I miss some reason why that couldn't happen?
Linus
^ permalink raw reply
* Re: git-svn and svn properties on upstream
From: Peter Harris @ 2008-07-13 0:07 UTC (permalink / raw)
To: Avery Pennarun; +Cc: gnuruandstuff, git
In-Reply-To: <32541b130807121430ia85a3c8u3efc0b5a8591eb44@mail.gmail.com>
On Sat, Jul 12, 2008 at 5:30 PM, Avery Pennarun wrote:
>
> I guess in an ideal world there'd be a way to track svn properties in
> the git history, but I have no idea where such things should probably
> go.
.gitattributes?
svn:eol-style translates roughly into crlf. svn:keywords translates
(partially) into ident.
git appears to ignore unrecognized entries in .gitattributes. The rest
can be stored there untranslated (or lightly translated - ':' is not
legal for .gitattributes names, if I read the comments correctly).
Peter Harris
^ permalink raw reply
* Re: [HACK] t/test-lib.sh HACK: Add -s/--show-hack to test suite.
From: Johannes Schindelin @ 2008-07-13 0:25 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git
In-Reply-To: <20080712222212.GC22323@leksak.fem-net>
Hi,
On Sun, 13 Jul 2008, Stephan Beyer wrote:
> Johannes Schindelin wrote:
>
> > On Mon, 7 Jul 2008, Stephan Beyer wrote:
> >
> > > > > This option realizes a stupid hack that tries to run the test
> > > > > cases line by line (separated by &&).
> > > >
> > > > In what way is that better than "sh -x t????-*.sh"?
> > >
> > > Your suggestion is more like "./t????-*.sh -v" instead of -s, at
> > > least on bash and dash here.
> >
> > No, I meant without "-v".
>
> Me, too.
>
> I've written something different: "sh -x" is a great thing and does
> exactly what it should on simple scripts containing:
>
> foo &&
> bar &&
> baz
>
> But for a test case in the git test suite it does not work, unfortunately.
Huh?
When I run "sh -x t*.sh", it lists _every_ command that was executed in
the script, and even more: it shows me the result, too! If there was a
function that was called, it prefixes an additional "+" so that I can
follow recursion better.
All in all, "sh -x" is a very versatile tool.
As far as I undertood your commit message, your HACK would have done the
first part: list what commands were executed. This is less than what "sh
-x" does.
Please clarify,
Dscho
^ permalink raw reply
* Re: [GitStats] Bling bling or some statistics on the git.git repository
From: Johannes Schindelin @ 2008-07-13 0:29 UTC (permalink / raw)
To: sverre; +Cc: Git Mailinglist, David Symonds
In-Reply-To: <bd6139dc0807121536u41591b1cv3258505b9a742851@mail.gmail.com>
Hi,
On Sun, 13 Jul 2008, Sverre Rabbelier wrote:
> On Sat, Jul 12, 2008 at 1:33 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > When that is done, I'll turn it into C.
>
> Goodluckwiththat ;D.
>
> [0]
> http://repo.or.cz/w/git-stats.git?a=commit;h=ac2a192f1dcec387e96f2a7eb5d66a38401b0722
Can you back that up with examples before I go and spend my time on
turning that into C, so I can see that it is what I want, first?
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH] Fix quadratic performance in rewrite_one.
From: Alexander Gavrilov @ 2008-07-13 0:37 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.LFD.1.10.0807121550320.2959@woody.linux-foundation.org>
On Sunday 13 July 2008 02:55:27 Linus Torvalds wrote:
> On Sat, 12 Jul 2008, Alexander N. Gavrilov wrote:
> > Parent commits are usually older than their children. Thus,
> > on each iteration of the loop in rewrite_one, add_parents_to_list
> > traverses all commits previously processed by the loop.
> > It performs very poorly in case of very long rewrite chains.
>
> Good call, but you don't seem to invalidate the cache when we remove
> things from the list.
The cache is local to rewrite_one, and is invalidated by exiting from that
function. Other users of add_parents_to_list just pass NULL as cache_ptr,
thus causing insert_by_date_cached to degenerate into a simple
insert_by_date.
> The top of the limit_list() loop does that "get top entry from list, an
> free it", and I'm not seeing you invalidating the cache if that entry that
> just got free'd happened to be the cache entry?
This type of workflow can be expected to keep the list relatively short
(roughly limited by the number of simultaneously existing branches); and if
it is already long, new entries will probably be added near the beginning
anyway, so there doesn't seem to be any need to use caching.
rewrite_one() is special in that it can sometimes walk through thousands of
commits at once and put them all into the list -- i.e. it is bound not by the
width of the history, but by its length.
Alexander
^ permalink raw reply
* Re: git pull is slow
From: Shawn O. Pearce @ 2008-07-13 1:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Hennig, Nicolas Pitre, Andreas Ericsson, git
In-Reply-To: <alpine.DEB.1.00.0807121546590.8950@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sat, 12 Jul 2008, Stephan Hennig wrote:
> >
> > Thanks for having a look at this! What does "problem with the pack"
> > mean? Do you think it is a Git problem (client or server side?) or just
> > a misconfiguration?
>
> I thought that the blobs in the pack are just too similar. That makes for
> a good compression, since you get many relatively small deltas. But it
> also makes for a lot of work to reconstruct the blobs.
>
> I suspected that you run out of space for the cache holding some
> reconstructed blobs (to prevent reconstructing all of them from scratch).
...
> Whoa. As you can see, your puny little 3.3 megabyte pack is blown to a
> full 555 megabyte in RAM.
...
> I expect this to touch the resolve_delta() function of index-pack.c in a
> major way, though.
Yea, that's going to be ugly. The "cache" you speak of above is held
on the call stack as resolv_delta() recurses through the delta chain
to reconstruct objects and generate their SHA-1s. There isn't a way to
release these objects when memory gets low so your worst case scenario
is a 100M+ blob with a delta chain of 50 or more - that will take you
5G of memory to pass through index-pack.
jgit isn't any better here.
What we need to do is maintain a list of the objects we are holding
on the call stack, and reduce ones up near the top when memory
gets low. Then upon recursing back up we can just recreate the
object if we had to throw it out. The higher up on the stack the
object is, the less likely we are to need it in the near future.
The more that I think about this, the easier it sounds to implement.
I may try to look at it a little later this evening.
> P.S.: It seems that "git verify-pack -v" only shows the sizes of the
> deltas. Might be interesting to some to show the unpacked _full_ size,
> too.
It wouldn't be very difficult to get that unpacked size. We just have
to deflate enough of the delta to see the delta header and obtain the
inflated object size from that. Unfortunately there is not an API in
sha1_file.c to offer that information to callers.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/2 (for GIT-GUI)] git-gui: MERGE_RR lives in .git/ directly with newer Git versions
From: Shawn O. Pearce @ 2008-07-13 1:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Kalle Olavi Niemitalo, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807121556480.8950@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Now that MERGE_RR was moved out of .git/rr-cache/, we have to delete
> it somewhere else. Just in case somebody wants to use a newer git-gui
> with an older Git, the file .git/rr-cache/MERGE_RR is removed, too (if
> it exists).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ack, I can pull this into git-gui.git. But I want to make sure
Junio is going to take 1/2 into git.git.
> git-gui/lib/merge.tcl | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/lib/merge.tcl b/lib/merge.tcl
> index cc26b07..5c01875 100644
> --- a/lib/merge.tcl
> +++ b/lib/merge.tcl
> @@ -257,6 +257,7 @@ proc _reset_wait {fd} {
>
> catch {file delete [gitdir MERGE_HEAD]}
> catch {file delete [gitdir rr-cache MERGE_RR]}
> + catch {file delete [gitdir MERGE_RR]}
> catch {file delete [gitdir SQUASH_MSG]}
> catch {file delete [gitdir MERGE_MSG]}
> catch {file delete [gitdir GITGUI_MSG]}
> --
--
Shawn.
^ permalink raw reply
* Re: [RFH] Finding all commits that touch the same files as a specific commit
From: Junio C Hamano @ 2008-07-13 1:24 UTC (permalink / raw)
To: sverre; +Cc: Git Mailinglist
In-Reply-To: <bd6139dc0807120858vc058451lb10933b5225c8521@mail.gmail.com>
"Sverre Rabbelier" <alturin@gmail.com> writes:
> Currently I do the following:
> $git diff-tree --name-status --no-commit-id -r <hash>
> To get all the files touched by the commit, I do:
> $git rev-list HEAD -- all the returned paths here
> This works perfectly, except when the subtree merge strategy is used,
> since in that case I get (example from git.git):
> $ git diff-tree --name-status --no-commit-id -r
> 5821988f97b827f6ba81dfeebff932067c88ba6c
> M git-gui.sh
> M lib/diff.tcl
> $ git rev-list HEAD -- git-gui.sh lib/diff.tcl
> $
>
> Now it was noticed on #git that git log has a --follow argument which
> -does- catch the rename, but it only works on one file at a time. So,
> my question is this:
> How do I find all commits that touch the same files as a specific commit?
> I have described my current approach above, which does not work when
> the subtree merge strategy is used. I am not stuck to this approach
> though, if someone comes up with a better way to do this than with
> 'git diff-tree' / 'git rev-list' I'm fine by that. I provided with my
> current approach in the hope that someone comes up with a similar
> solution which means I'll have to edit less ;).
First of all, a bad news that everybody should have known since day 1 when
the --follow option was introduced. It merely is a cute hack that works
most of the time in trivial histories. The data structure it uses cannot
reliably follow renames if you have any nontrivial history.
Revision traversal machinery has a single list of pathspecs to filter the
results with, and in the usual traversal, the list never changes. That is
why you would need to give a list of three pathspecs upfront, like this:
git log -- arch/i386 arch/x86 arch/x86_64
to get the whole picture of how things are consolidated into a single
arch/x86 hierarchy over time from originally two hierarchies. The
revision traversal works by simplifying away commits that do not touch
path that match any of the given pathspecs, so giving the "current" path
(i.e. arch/x86) is not sufficient.
The --follow changes the behaviour slightly. When you have this history:
---o---o---o---x---x---x
where a file you are interested in (say, arch/i386/kernel/reboot.c)
existed in the past in 'o' commits, but was renamed to something else
(say, arch/x86/kernel/reboot.c) in newer 'x' commits, you would start
following from the tip of the history like this:
git log --follow arch/x86/kernel/reboot.c
And the machinery traverses down the history, showing only the commits
that touch the given path. An interesting thing happens, however, when it
hits the earliest 'x' commit and realizes that its parent 'o' does not
have that path. It runs the rename detection there, realizes the path it
is interested in corresponds to a different path in the parent, and
_updates_ the pathspec to the old name. I.e. it will from that point on
behaves as if you started digging from the tip of this history:
---o---o---o
with a different pathspec:
git log --follow arch/i386/kernel/reboot.c
This works as long as your history is trivial, but in real life, the world
is not linear.
x---x---x---x
/ /
----o---o---o
If commits 'x' have git-gui/git-gui.sh and commits 'o' have git-gui.sh at
the root level, you would start digging from the tip with --follow:
git log --follow git-gui/git-gui.sh
When it hits the rightmost merge 'x', it realizes the changes to the file
came from lower history and switches the pathspec to "git-gui.sh" at the
root level (the commits that have already been traversed are marked with
uppercase latters here).
x---x---X---X
/ /
----o---o---O
Switching the pathspec from "git-gui/git-gui.sh" to "git-gui.sh" is fine
for the purpose of traversing the 'o' history down, but there is a
problem. Remember I said there is a _single_ list of pathspecs the
revision traversal machinery keeps track of? If you switch that single
list to "git-gui.sh", it means you completely forget that you were
following "git-gui/git-gui.sh". You cannot follow the upper history
anymore.
In order to follow renames reliably in a merge heavy history, you need to
keep track of the pathname the file you are interested in appears as _in
each commit_. As you traverse down the history, you pass down the
pathname to the parent you visit, so while you are traversing from 'x' to
earlier 'x', you will keep following "git-gui/git-gui.sh", while you
traverse down to 'o', you will inspect "git-gui.sh".
The data structure the revision traversal machinery uses does not support
this "path-per-commit" natively.
This is the reason "git-blame" uses its own traversal engine. It keeps
track of <commit, path> pairs so that it can mark which line came from
what path in what commit. When copy/move detection are used, we can even
notice that the contents we are interested in came from more than one file
in the same commits, and the data structure supports it (i.e. it is not
just a pointer to a single string from "struct commit").
For the purpose of "git log" traversal and the "file renames" people
usually talk about, this is overkill; you should however be able to
backport the basic idea to revision machinery, if you really cared.
In a real history, "file rename" is a very ill defined concept and is not
always useful in practice. I did a fairly detailed analysis on one
real-world history more than two years ago, which is found here:
http://thread.gmane.org/gmane.comp.version-control.git/13746/focus=13769
In our own "git.git" history, the evolution of what finally landed in
revision.c is interesting. The interesting part of content movement never
involved any file renames --- only bits and pieces migrated over across
many files. That is not something "file rename tracking", even with an
extension to the revision traversal machinery to keep one path per commit
to record the file you are interested in, can ever give meaningful
explanation of the history. You need a lot more fine grained "blame"
traversal machinery for that.
^ permalink raw reply
* [PATCH] bash completion: Improve responsiveness of git-log completion
From: Shawn O. Pearce @ 2008-07-13 2:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Petr Baudis
Junio noticed the bash completion has been taking a long time lately.
Petr Baudis tracked it down to 72e5e989b ("bash: Add space after
unique command name is completed."). Tracing the code showed
we spent significant time inside of this loop within __gitcomp,
due to the string copying overhead.
[28.146109654] _git common over
[28.164791148] gitrefs in
[28.280302268] gitrefs dir out
[28.300939737] gitcomp in
[28.308378112] gitcomp pre-case
* [28.313407453] gitcomp iter in
* [28.701270296] gitcomp iter out
[28.713370786] out normal
Since __git_refs avoids this string copying by forking and using
echo we use the same trick here when we need to finish generating
the names for the caller.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Does this make things better? Or worse? I'm not seeing a huge
difference on my own system. Maybe its too fast these days...
contrib/completion/git-completion.bash | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 27332ed..61581fe 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -114,9 +114,20 @@ __git_ps1 ()
fi
}
+__gitcomp_1 ()
+{
+ local c IFS=' '$'\t'$'\n'
+ for c in $1; do
+ case "$c$2" in
+ --*=*) printf %s$'\n' "$c$2" ;;
+ *.) printf %s$'\n' "$c$2" ;;
+ *) printf %s$'\n' "$c$2 " ;;
+ esac
+ done
+}
+
__gitcomp ()
{
- local all c s=$'\n' IFS=' '$'\t'$'\n'
local cur="${COMP_WORDS[COMP_CWORD]}"
if [ $# -gt 2 ]; then
cur="$3"
@@ -124,21 +135,14 @@ __gitcomp ()
case "$cur" in
--*=)
COMPREPLY=()
- return
;;
*)
- for c in $1; do
- case "$c$4" in
- --*=*) all="$all$c$4$s" ;;
- *.) all="$all$c$4$s" ;;
- *) all="$all$c$4 $s" ;;
- esac
- done
+ local IFS=$'\n'
+ COMPREPLY=($(compgen -P "$2" \
+ -W "$(__gitcomp_1 "$1" "$4")" \
+ -- "$cur"))
;;
esac
- IFS=$s
- COMPREPLY=($(compgen -P "$2" -W "$all" -- "$cur"))
- return
}
__git_heads ()
--
1.5.6.2.393.g45096
^ permalink raw reply related
* Re: [PATCH] cherry: cache patch-ids to avoid repeating work
From: Geoffrey Irving @ 2008-07-13 3:14 UTC (permalink / raw)
To: Junio C Hamano, git@vger.kernel.org; +Cc: Johannes Schindelin
In-Reply-To: <7vej60jln6.fsf@gitster.siamese.dyndns.org>
Add cached-sha-map.[ch] implementing a persistent hash map from sha1 to
sha1. The map is read with mmap, and completely rewritten if any entries
change. It would be good to add incremental update to handle the usual case
where only a few entries change.
This structure is used by patch-ids.c to cache the mapping from commit to
patch-id into $GIT_DIR/patch-id-cache. In the one case I've tested so far,
this speeds up the second invocation of git-cherry by two orders of
magnitude. The caching can be disabled by setting cherry.cachepatchids to
false.
Original code cannibalized from Johannes Schindelin's notes-index structure.
Signed-off-by: Geoffrey Irving <irving@naml.us>
---
Here's an updated version that avoids infinite loops and adds a sha1
checksum of the header. It is still vastly more likely that this code
will return incorrect results due to disk corruption than that the old
version would infinite loop. If we want to be even more paranoid, we
could add a checksum for every 511 entries, but I'm hoping that isn't
required. :)
Your version of the infinite loop avoidance didn't quite work, since
I'm already using every 32 bit return value in find_helper.
I also fixed the 4/3 check to not overflow.
Documentation/config.txt | 5 +
Makefile | 2 +
builtin-log.c | 12 ++
cached-sha1-map.c | 293 ++++++++++++++++++++++++++++++++++++++++++++++
cached-sha1-map.h | 45 +++++++
patch-ids.c | 26 ++++-
patch-ids.h | 2 +
7 files changed, 384 insertions(+), 1 deletions(-)
create mode 100644 cached-sha1-map.c
create mode 100644 cached-sha1-map.h
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 838794d..02b8113 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -468,6 +468,11 @@ browser.<tool>.path::
browse HTML help (see '-w' option in linkgit:git-help[1]) or a
working repository in gitweb (see linkgit:git-instaweb[1]).
+cherry.cachepatchids::
+ If true, linkgit:git-cherry will store a cache of computed patch-ids
+ in $GIT_DIR/patch-id-cache in order to make repeated invocations faster.
+ Defaults to true.
+
clean.requireForce::
A boolean to make git-clean do nothing unless given -f
or -n. Defaults to true.
diff --git a/Makefile b/Makefile
index 4796565..f7360e1 100644
--- a/Makefile
+++ b/Makefile
@@ -356,6 +356,7 @@ LIB_H += pack-refs.h
LIB_H += pack-revindex.h
LIB_H += parse-options.h
LIB_H += patch-ids.h
+LIB_H += cached-sha1-map.h
LIB_H += path-list.h
LIB_H += pkt-line.h
LIB_H += progress.h
@@ -436,6 +437,7 @@ LIB_OBJS += pager.o
LIB_OBJS += parse-options.o
LIB_OBJS += patch-delta.o
LIB_OBJS += patch-ids.o
+LIB_OBJS += cached-sha1-map.o
LIB_OBJS += path-list.o
LIB_OBJS += path.o
LIB_OBJS += pkt-line.o
diff --git a/builtin-log.c b/builtin-log.c
index 430d876..fbfefbd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1081,6 +1081,16 @@ static int add_pending_commit(const char *arg,
struct rev_info *revs, int flags)
return -1;
}
+static int git_cherry_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, "cherry.cachepatchids")) {
+ cache_patch_ids = git_config_bool(var, value);
+ return 0;
+ }
+
+ return 0;
+}
+
static const char cherry_usage[] =
"git-cherry [-v] <upstream> [<head>] [<limit>]";
int cmd_cherry(int argc, const char **argv, const char *prefix)
@@ -1094,6 +1104,8 @@ int cmd_cherry(int argc, const char **argv,
const char *prefix)
const char *limit = NULL;
int verbose = 0;
+ git_config(git_cherry_config, NULL);
+
if (argc > 1 && !strcmp(argv[1], "-v")) {
verbose = 1;
argc--;
diff --git a/cached-sha1-map.c b/cached-sha1-map.c
new file mode 100644
index 0000000..9cf7252
--- /dev/null
+++ b/cached-sha1-map.c
@@ -0,0 +1,293 @@
+#include "cached-sha1-map.h"
+
+union cached_sha1_map_header {
+ struct {
+ char signature[4]; /* CS1M */
+ uint32_t version;
+ uint32_t count;
+ uint32_t size;
+ uint32_t pad; /* pad to 20 bytes */
+ } u;
+ /* pad header out to 40 bytes. As a consistency
+ * check, pad.value stores the sha1 of pad.key. */
+ struct cached_sha1_entry pad;
+};
+
+static const char *signature = "CS1M";
+static const uint32_t version = 1;
+
+static int init_empty_map(struct cached_sha1_map *cache, uint32_t size)
+{
+ cache->count = 0;
+ cache->size = size;
+ cache->initialized = 1;
+ cache->mmapped = 0;
+ cache->dirty = 1;
+
+ cache->entries = calloc(size, sizeof(struct cached_sha1_entry));
+ if (!cache->entries) {
+ warning("failed to allocate empty map of size %"PRIu32" for %s",
+ size, git_path(cache->filename));
+ cache->size = 0;
+ cache->dirty = 0;
+ return -1;
+ }
+ return 0;
+}
+
+static int grow_map(struct cached_sha1_map *cache)
+{
+ struct cached_sha1_map new_cache;
+ uint32_t i;
+
+ if (cache->size * 2 == 0) {
+ warning("%s overflowed, so resetting to empty",
+ git_path(cache->filename));
+ return init_empty_map(cache, 64);
+ }
+
+ /* allocate cache with twice the size */
+ new_cache.filename = cache->filename;
+ if (init_empty_map(&new_cache, cache->size * 2)) {
+ warning("failed to grow %s to size %"PRIu32,
+ git_path(cache->filename), cache->size * 2);
+ return init_empty_map(cache, 64);
+ }
+
+ /* reinsert all entries */
+ for (i = 0; i < cache->size; i++)
+ if (!is_null_sha1(cache->entries[i].key))
+ set_cached_sha1_entry(&new_cache,
+ cache->entries[i].key, cache->entries[i].value);
+ /* finish */
+ free_cached_sha1_map(cache);
+ *cache = new_cache;
+ return 0;
+}
+
+/* Any errors that occur result in the cache being initialized to empty */
+static int init_cached_sha1_map(struct cached_sha1_map *cache)
+{
+ int fd;
+ union cached_sha1_map_header header;
+ const char *filename;
+ size_t map_size;
+ SHA_CTX ctx;
+
+ if (cache->initialized)
+ return cache->size ? 0 : -1;
+
+ filename = git_path(cache->filename);
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ if (errno != ENOENT)
+ warning("failed to read '%s': %s", filename,
+ strerror(errno));
+ goto empty;
+ }
+
+ if (read_in_full(fd, &header, sizeof(header)) != sizeof(header)) {
+ warning("cannot read %s header", filename);
+ goto empty;
+ }
+
+ if (memcmp(header.u.signature, signature, 4)) {
+ warning("%s has invalid header", filename);
+ goto empty;
+ }
+
+ if (ntohl(header.u.version) != version) {
+ warning("%s has unrecognized version %"PRIu32, filename,
+ ntohl(header.u.version));
+ goto empty;
+ }
+
+ cache->count = ntohl(header.u.count);
+ cache->size = ntohl(header.u.size);
+
+ if (cache->size & (cache->size-1)) {
+ warning("%s is corrupt: size %"PRIu32" is not a power of two",
+ filename, cache->size);
+ goto empty;
+ }
+
+ if (cache->count >= cache->size) {
+ warning("%s is corrupt: count %"PRIu32" >= size %"PRIu32,
+ filename, cache->count, cache->size);
+ goto empty;
+ }
+
+ SHA1_Init(&ctx);
+ SHA1_Update(&ctx, header.pad.key, 20);
+ SHA1_Final(header.pad.key, &ctx); /* reuse pad.key to store its sha1 */
+ if (hashcmp(header.pad.key, header.pad.value)) {
+ warning("%s header has invalid sha1", filename);
+ goto empty;
+ }
+
+ cache->dirty = 0;
+ cache->initialized = 1;
+ cache->mmapped = 1;
+
+ /* mmap entire file so that file / memory blocks are aligned */
+ map_size = sizeof(struct cached_sha1_entry) * (cache->size + 1);
+ cache->entries = mmap(NULL, map_size,
+ PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+ if (cache->entries == MAP_FAILED) {
+ /* this is just a cache, so don't free pack memory and retry */
+ warning("%s mmap failed: %s", filename, strerror(errno));
+ goto empty;
+ }
+ cache->entries += 1; /* skip header */
+ return 0;
+
+empty:
+ if (fd >= 0)
+ close(fd);
+ return init_empty_map(cache, 64);
+}
+
+int write_cached_sha1_map(struct cached_sha1_map *cache)
+{
+ union cached_sha1_map_header header;
+ struct lock_file update_lock;
+ int fd;
+ size_t map_size;
+ const char *filename;
+ SHA_CTX ctx;
+
+ if (!cache->initialized || !cache->dirty)
+ return 0;
+
+ filename = git_path(cache->filename);
+ fd = hold_lock_file_for_update(&update_lock, filename, 0);
+
+ if (fd < 0)
+ {
+ warning("could not construct %s", filename);
+ return -1;
+ }
+
+ /* initialize header */
+ memcpy(header.u.signature, signature, 4);
+ header.u.version = htonl(version);
+ header.u.count = htonl(cache->count);
+ header.u.size = htonl(cache->size);
+ header.u.pad = 0; /* make header deterministic */
+
+ /* compute header sha1 */
+ SHA1_Init(&ctx);
+ SHA1_Update(&ctx, header.pad.key, 20);
+ SHA1_Final(header.pad.value, &ctx);
+
+ map_size = sizeof(struct cached_sha1_entry) * cache->size;
+ if (write_in_full(fd, &header, sizeof(header)) != sizeof(header)
+ || write_in_full(fd, cache->entries, map_size) != map_size)
+ {
+ warning("could not write %s", filename);
+ return -1;
+ }
+
+ if (commit_lock_file(&update_lock) < 0)
+ {
+ warning("could not write %s", filename);
+ return -1;
+ }
+
+ cache->dirty = 0;
+ return 0;
+}
+
+void free_cached_sha1_map(struct cached_sha1_map *cache)
+{
+ if (!cache->initialized)
+ return;
+
+ if (cache->mmapped)
+ munmap(cache->entries - 1,
+ sizeof(struct cached_sha1_entry) * (cache->size + 1));
+ else
+ free(cache->entries);
+}
+
+/* The fact that size is a power of two means count-1 <= INT32_MAX, so it
+ * is safe to return signed integers here. */
+static int32_t get_hash_index(const unsigned char *sha1)
+{
+ /* this is alignment safe since 40 is a multiple of 4 */
+ return ntohl(*(uint32_t*)sha1);
+}
+
+/*
+ * Returns the index if the entry exists, and the complemented index of
+ * the next free entry otherwise. If the hash is full, returns the
+ * complement of a nonfree entry and sets count = size (this happens
+ * only if the file is corrupt).
+ */
+static int32_t find_helper(struct cached_sha1_map *cache,
+ const unsigned char *key)
+{
+ int32_t i, mask, full;
+
+ mask = cache->size - 1;
+ i = get_hash_index(key) & mask;
+ full = (i-1) & mask;
+
+ for (; ; i = (i+1) & mask) {
+ if (!hashcmp(key, cache->entries[i].key))
+ return i;
+ else if (is_null_sha1(cache->entries[i].key) || i == full)
+ return ~i;
+ if (i == full) {
+ cache->count = cache->size; /* fix count */
+ return ~1;
+ }
+ }
+}
+
+int get_cached_sha1_entry(struct cached_sha1_map *cache,
+ const unsigned char *key, unsigned char *value)
+{
+ int32_t i;
+
+ if (init_cached_sha1_map(cache))
+ return -1;
+
+ i = find_helper(cache, key);
+ if(i < 0)
+ return -1;
+
+ /* entry found, return value */
+ hashcpy(value, cache->entries[i].value);
+ return 0;
+}
+
+int set_cached_sha1_entry(struct cached_sha1_map *cache,
+ const unsigned char *key, const unsigned char *value)
+{
+ int32_t i;
+ struct cached_sha1_entry *entry;
+
+ if (init_cached_sha1_map(cache))
+ return -1;
+
+ i = find_helper(cache, key);
+
+ if (i < 0) { /* write new entry */
+ entry = cache->entries + ~i;
+ hashcpy(entry->key, key);
+ hashcpy(entry->value, value);
+ cache->count++;
+ cache->dirty = 1;
+ } else { /* overwrite existing entry */
+ entry = cache->entries + i;
+ if (hashcmp(value, entry->value)) {
+ hashcpy(entry->value, value);
+ cache->dirty = 1;
+ }
+ }
+
+ if (cache->count >= cache->size/4*3)
+ return grow_map(cache);
+ return 0;
+}
diff --git a/cached-sha1-map.h b/cached-sha1-map.h
new file mode 100644
index 0000000..296c17c
--- /dev/null
+++ b/cached-sha1-map.h
@@ -0,0 +1,45 @@
+#ifndef CACHED_SHA1_MAP_H
+#define CACHED_SHA1_MAP_H
+
+#include "cache.h"
+
+/*
+ * A cached-sha1-map is a file storing a hash map from sha1 to sha1.
+ *
+ * The file is mmap'ed, updated in memory during operation, and flushed
+ * back to disk when freed. Currently the entire file is rewritten for
+ * any change. This could be a significant bottleneck for common uses,
+ * so it would be good to fix this later if possible.
+ *
+ * The performance of a hash map depends highly on a good hashing
+ * algorithm, to avoid collisions. Lucky us! SHA-1 is a pretty good
+ * hashing algorithm.
+ */
+
+struct cached_sha1_entry {
+ unsigned char key[20];
+ unsigned char value[20];
+};
+
+struct cached_sha1_map {
+ const char *filename; /* relative to GIT_DIR */
+
+ /* rest is for internal use */
+ uint32_t count, size;
+ unsigned int initialized : 1;
+ unsigned int dirty : 1;
+ unsigned int mmapped : 1;
+ struct cached_sha1_entry *entries; /* pointer to mmap'ed memory + 1 */
+};
+
+extern int get_cached_sha1_entry(struct cached_sha1_map *cache,
+ const unsigned char *key,unsigned char *value);
+
+extern int set_cached_sha1_entry(struct cached_sha1_map *cache,
+ const unsigned char *key, const unsigned char *value);
+
+extern int write_cached_sha1_map(struct cached_sha1_map *cache);
+
+extern void free_cached_sha1_map(struct cached_sha1_map *cache);
+
+#endif
diff --git a/patch-ids.c b/patch-ids.c
index 3be5d31..663ffee 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -2,17 +2,36 @@
#include "diff.h"
#include "commit.h"
#include "patch-ids.h"
+#include "cached-sha1-map.h"
+
+int cache_patch_ids = 1;
+static struct cached_sha1_map patch_id_cache;
static int commit_patch_id(struct commit *commit, struct diff_options *options,
unsigned char *sha1)
{
+ int ret;
+
+ /* pull patch-id out of the cache if possible */
+ patch_id_cache.filename = "patch-id-cache";
+ if (cache_patch_ids && !get_cached_sha1_entry(&patch_id_cache,
+ commit->object.sha1, sha1))
+ return 0;
+
if (commit->parents)
diff_tree_sha1(commit->parents->item->object.sha1,
commit->object.sha1, "", options);
else
diff_root_tree_sha1(commit->object.sha1, "", options);
diffcore_std(options);
- return diff_flush_patch_id(options, sha1);
+ ret = diff_flush_patch_id(options, sha1);
+ if (ret)
+ return ret;
+
+ /* record commit, patch-id pair in cache */
+ if (cache_patch_ids)
+ set_cached_sha1_entry(&patch_id_cache, commit->object.sha1, sha1);
+ return 0;
}
static uint32_t take2(const unsigned char *id)
@@ -136,6 +155,11 @@ int free_patch_ids(struct patch_ids *ids)
next = patches->next;
free(patches);
}
+
+ /* write cached patch-ids and ignore any errors that arise
+ * (e.g. if the repository is write protected) */
+ if (cache_patch_ids)
+ write_cached_sha1_map(&patch_id_cache);
return 0;
}
diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..c0ebdc1 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -18,4 +18,6 @@ int free_patch_ids(struct patch_ids *);
struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
struct patch_id *has_commit_patch_id(struct commit *, struct patch_ids *);
+extern int cache_patch_ids;
+
#endif /* PATCH_IDS_H */
--
1.5.6.2.256.g33ad.dirty
^ permalink raw reply related
* Re: [PATCH] bash completion: Improve responsiveness of git-log completion
From: Petr Baudis @ 2008-07-13 4:02 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20080713023742.GA31760@spearce.org>
On Sun, Jul 13, 2008 at 02:37:42AM +0000, Shawn O. Pearce wrote:
> Junio noticed the bash completion has been taking a long time lately.
> Petr Baudis tracked it down to 72e5e989b ("bash: Add space after
> unique command name is completed."). Tracing the code showed
> we spent significant time inside of this loop within __gitcomp,
> due to the string copying overhead.
>
> [28.146109654] _git common over
> [28.164791148] gitrefs in
> [28.280302268] gitrefs dir out
> [28.300939737] gitcomp in
> [28.308378112] gitcomp pre-case
> * [28.313407453] gitcomp iter in
> * [28.701270296] gitcomp iter out
> [28.713370786] out normal
>
> Since __git_refs avoids this string copying by forking and using
> echo we use the same trick here when we need to finish generating
> the names for the caller.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
I spent quite some time trying to optimize the run by heavy use of bash
arrays (either just in __gitcomp() and at later stage reusing
__git_refs() array within __gitcomp()), but it turned out I have made
few simple mistakes and in the end, the array is slower than Shawn's
approach - when reusing the array, the difference is even bigger.
I think that if a[i]=x is slower than echo x, bash is doing something
real weird, but that seems to be the case.
Someone might find my simple and noisy benchmarker useful:
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 27332ed..31f97c1 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -45,6 +45,18 @@
# git@vger.kernel.org
#
+
+benchmark_last=0
+benchmark_delta=0
+benchmark ()
+{
+ #return
+ local now=$(date +%S%N)
+ benchmark_diff=$(echo "($now-$benchmark_last)/1000000-($benchmark_delta)"|bc)
+ echo "[+${benchmark_diff}ms] $*" >&2
+ benchmark_last=$now
+}
+
__gitdir ()
{
if [ -z "$1" ]; then
@@ -116,6 +128,7 @@ __git_ps1 ()
__gitcomp ()
{
+ benchmark "gitcomp start"
local all c s=$'\n' IFS=' '$'\t'$'\n'
local cur="${COMP_WORDS[COMP_CWORD]}"
if [ $# -gt 2 ]; then
@@ -138,6 +151,7 @@ __gitcomp ()
esac
IFS=$s
COMPREPLY=($(compgen -P "$2" -W "$all" -- "$cur"))
+ benchmark "gitcomp end"
return
}
@@ -185,12 +199,16 @@ __git_tags ()
__git_refs ()
{
+ benchmark "gitrefs in"
local cmd i is_hash=y dir="$(__gitdir "$1")"
if [ -d "$dir" ]; then
if [ -e "$dir/HEAD" ]; then echo HEAD; fi
- for i in $(git --git-dir="$dir" \
+ benchmark "gitrefs dir a"
+ local a=($(git --git-dir="$dir" \
for-each-ref --format='%(refname)' \
- refs/tags refs/heads refs/remotes); do
+ refs/tags refs/heads refs/remotes))
+ benchmark "gitrefs dir r"
+ for i in "${a[@]}"; do
case "$i" in
refs/tags/*) echo "${i#refs/tags/}" ;;
refs/heads/*) echo "${i#refs/heads/}" ;;
@@ -198,6 +216,7 @@ __git_refs ()
*) echo "$i" ;;
esac
done
+ benchmark "gitrefs dir out"
return
fi
for i in $(git ls-remote "$dir" 2>/dev/null); do
@@ -210,6 +229,7 @@ __git_refs ()
n,*) is_hash=y; echo "$i" ;;
esac
done
+ benchmark "gitrefs out"
}
__git_refs2 ()
@@ -324,7 +344,9 @@ __git_complete_revlist ()
__gitcomp "$cur."
;;
*)
+ benchmark "revlist simple in"
__gitcomp "$(__git_refs)"
+ benchmark "revlist simple out"
;;
esac
}
@@ -756,6 +778,8 @@ _git_log ()
{
__git_has_doubledash && return
+ benchmark "doubledash over"
+
local cur="${COMP_WORDS[COMP_CWORD]}"
case "$cur" in
--pretty=*)
@@ -791,6 +815,7 @@ _git_log ()
return
;;
esac
+ benchmark "gitcomp over"
__git_complete_revlist
}
@@ -1303,6 +1328,9 @@ _git ()
{
local i c=1 command __git_dir
+ benchmark "in"; benchmark_delta=0
+ benchmark "in netto"; benchmark_delta=$benchmark_diff
+
while [ $c -lt $COMP_CWORD ]; do
i="${COMP_WORDS[c]}"
case "$i" in
@@ -1314,6 +1342,8 @@ _git ()
c=$((++c))
done
+ #[ "$__git_dir" ] || __git_dir="$(__gitdir)"
+
if [ -z "$command" ]; then
case "${COMP_WORDS[COMP_CWORD]}" in
--*=*) COMPREPLY=() ;;
@@ -1330,12 +1360,16 @@ _git ()
;;
*) __gitcomp "$(__git_commands) $(__git_aliases)" ;;
esac
+
+ benchmark "out cut $(date +%S.%N)"
return
fi
local expansion=$(__git_aliased_command "$command")
[ "$expansion" ] && command="$expansion"
+ benchmark "mark common"
+
case "$command" in
am) _git_am ;;
add) _git_add ;;
@@ -1374,6 +1408,8 @@ _git ()
whatchanged) _git_log ;;
*) COMPREPLY=() ;;
esac
+
+ benchmark "out normal"
}
_gitk ()
^ permalink raw reply related
* Q: How to tag individual files
From: Poojan Wagh @ 2008-07-13 4:21 UTC (permalink / raw)
To: git
The crash course at: http://git.or.cz/course/svn.html says:
> You usually tag commits but if you want, you can tag files (or trees, but that's a bit low-level) as well.
I've done a bit of searching, and I can't for the life of me figure out
how to tag an individual file. Help from the group would be much
appreciated. Thanks.
--
Poojan Wagh
http://www.circuitdesign.info
^ permalink raw reply
* Re: Q: How to tag individual files
From: Shawn O. Pearce @ 2008-07-13 5:35 UTC (permalink / raw)
To: Poojan Wagh; +Cc: git
In-Reply-To: <g5bvte$52j$1@ger.gmane.org>
Poojan Wagh <poojanwagh@gmail.com> wrote:
> The crash course at: http://git.or.cz/course/svn.html says:
>
>> You usually tag commits but if you want, you can tag files (or trees,
>> but that's a bit low-level) as well.
>
> I've done a bit of searching, and I can't for the life of me figure out
> how to tag an individual file. Help from the group would be much
> appreciated. Thanks.
You don't tag individual files. Git is snapshot based. You tag
the entire project at once.
The comment you quoted was talking about tagging a single listing of
blobs (a tree) or a single blob (stream of bytes). This can be handy
in rare cases. In the linux kernel for example the prior history
was stored by tagging the tree of that prior history, as there was
no git based history preceeding that snapshot, so there were no
commits to tag. In git.git Junio publishes his GnuPG public key
as a tagged blob, making it available to validate signed tag objects.
So you can't tag individual files. The comment is a bit misleading
and gets far too low-level for an otherwise high-level conversion
guide.
--
Shawn.
^ permalink raw reply
* Re: Q: How to tag individual files
From: Poojan Wagh @ 2008-07-13 6:14 UTC (permalink / raw)
To: git
In-Reply-To: <20080713053537.GA32746@spearce.org>
Thanks, Shawn, for the clarification. Your explanation--together with
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#the-object-database--
makes it clear.
So, to summarize (with the intent of fixing any misunderstanding on my
part): You can tag the contents of files (blobs), but not files per se,
because:
1. Git doesn't store files as an object as such. It stores the contents
of files ("blobs")
2. Git stores directories (which define their constituent file names and
contents=>blobs) as "tree" objects. So, the association between a file's
name and a its contents occurs in a tree object.
One thing I notice about this structure is that a tree object can at
least theoretically give me what I want: the ability to have files
organized with labels (a la gmail) rather than folders--I would like to
be able to have a single file show up under many different labels rather
than mutually exclusive folders/directories. I know that this won't
happen automatically, but it's nice to know that I *could* do it if I
wanted to.
Thanks!
P.S. I also found http://www.gitcasts.com/posts/browsing-git-objects
useful for as illustration of all the object structure.
Shawn O. Pearce wrote:
> Poojan Wagh <poojanwagh@gmail.com> wrote:
>> The crash course at: http://git.or.cz/course/svn.html says:
>>
>>> You usually tag commits but if you want, you can tag files (or trees,
>>> but that's a bit low-level) as well.
>> I've done a bit of searching, and I can't for the life of me figure out
>> how to tag an individual file. Help from the group would be much
>> appreciated. Thanks.
>
> You don't tag individual files. Git is snapshot based. You tag
> the entire project at once.
>
> The comment you quoted was talking about tagging a single listing of
> blobs (a tree) or a single blob (stream of bytes). This can be handy
> in rare cases. In the linux kernel for example the prior history
> was stored by tagging the tree of that prior history, as there was
> no git based history preceeding that snapshot, so there were no
> commits to tag. In git.git Junio publishes his GnuPG public key
> as a tagged blob, making it available to validate signed tag objects.
>
> So you can't tag individual files. The comment is a bit misleading
> and gets far too low-level for an otherwise high-level conversion
> guide.
>
^ permalink raw reply
* Re: [PATCH] git-bisect: use dash-less form on git bisect log
From: Christian Couder @ 2008-07-13 6:19 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <1215792089-27201-1-git-send-email-vmiklos@frugalware.org>
Le vendredi 11 juillet 2008, Miklos Vajna a écrit :
> Given that users are supposed to type 'git bisect' now, make the output
> of 'git bisect log' consistent with this.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Fri, Jul 11, 2008 at 08:39:14AM +0200, Christian Couder
<chriscool@tuxfamily.org> wrote:
> > Wouldn't it be better if bisect_replay could read old logs?
> >
> > Maybe with something like:
> >
> > + while read git bisect command rev
> > do
> > - test "$bisect" = "git-bisect" || continue
> > + test "$git $bisect" = "git bisect" -o "$git" =
> > "git-bisect" || continue
> > + if test "$git" = "git-bisect"; then
> > + rev="$command"
> > + command="$bisect"
> > + fi
>
> You are right, here is an updated patch.
Your patch looks good. But there is also the problem that new bisect logs
will not be understood by old git. So I wonder if a patch to only read the
new log should be first commited to the "maint" branch.
And sorry but I will be on vacation without Internet access for one week
starting really soon now, so I will not be able to do anything soon, nor
answer to any mails.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-13 6:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Michael Haggerty, Jeff King, git
In-Reply-To: <200807110036.17504.chriscool@tuxfamily.org>
Le vendredi 11 juillet 2008, Christian Couder a écrit :
> Le jeudi 10 juillet 2008, Junio C Hamano a écrit :
> > Christian Couder <chriscool@tuxfamily.org> writes:
> > > Yeah, in that case...
> > >
> > >> The whole idea of "bisect" relies on that idea, that any ancestor of
> > >> a good commit is good. Otherwise you'd have to check the commits
> > >> one by one, not in a bisecting manner.
> >
> > Didn't we already discuss this at length?
>
> Yes, the thread is there:
>
> http://thread.gmane.org/gmane.comp.version-control.git/86951
>
> > > No, you just need to check that the merge bases between the bad rev
> > > on one side and each good rev on the other side are good too. And if
> > > that is the case, then you can be sure that bisection will point to a
> > > first bad commit.
> > >
> > > So the choice is between a simple and fast but not fully reliable
> > > bisect, or a more complex and slower but fully reliable bisect.
> >
> > I have not looked at your implementation, but I do think:
> >
> > - The current one is not "fully reliable"; the user needs to know what
> > he is doing. You might call it "prone to user errors".
>
> I agree.
>
> > - "Test this merge-base before going forward, please" will add
> > typically only one round of check (if you have more merge bases between
> > good and bad, you need to test all of them are good to be sure), so it
> > is not "slower nor more complex".
>
> By "slower" I meant that it would need more rounds of check on average.
> By "more complex" I meant that more code is needed.
>
> And I think you are right, all the merge bases need to be tested so I
> will send a patch on top of the patch discussed here.
Ok, here is an untested patch that should check all merge bases. I don't
have time right now to add tests and a good commit message but I will do
that when I come back from vacancy in about one week. So please consider it
as just a RFC.
Thanks,
Christian.
-----8<------------
[PATCH] bisect: check all merge bases instead of only one
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 50f912f..ca16609 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -375,7 +375,6 @@ Warning: the merge base between $_bad and $_g must be
skipped.
So we cannot be sure the first bad commit is between $_mb and $_bad.
We continue anyway.
EOF
- mark_merge_base_ok "$_bad" "$_g"
}
check_merge_bases() {
@@ -384,19 +383,21 @@ check_merge_bases() {
_skip="$3"
for _g in $_good; do
is_merge_base_ok "$_bad" "$_g" && continue
- _mb=$(git merge-base $_g $_bad)
- if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
- mark_merge_base_ok "$_bad" "$_g"
- elif test "$_mb" = "$_bad"; then
- handle_bad_merge_base "$_bad" "$_g"
- elif is_among "$_mb" "$_skip"; then
- handle_skipped_merge_base "$_bad" "$_g" "_mb"
- else
- mark_testing_merge_base "$_mb"
- checkout "$_mb" "a merge base must be tested"
- checkout_done=1
- break
- fi
+ for _mb in $(git merge-base --all $_g $_bad); do
+ if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
+ continue
+ elif test "$_mb" = "$_bad"; then
+ handle_bad_merge_base "$_bad" "$_g"
+ elif is_among "$_mb" "$_skip"; then
+ handle_skipped_merge_base "$_bad" "$_g" "_mb"
+ else
+ mark_testing_merge_base "$_mb"
+ checkout "$_mb" "a merge base must be tested"
+ checkout_done=1
+ return
+ fi
+ done
+ mark_merge_base_ok "$_bad" "$_g"
done
}
--
1.5.6.2.221.gf54e0.dirty
^ permalink raw reply related
* Q: how to remove or move submodules?
From: Rob Sanheim @ 2008-07-13 6:38 UTC (permalink / raw)
To: git
Hello
I don't see any info on what the 'right' way is to delete or move
submodules around in any of the docs. Normally I end up just hacking
my way through it and hand modifying the .gitmodules file, but this
seems wrong. Is there a recommended way?
thanks,
Rob
^ permalink raw reply
* [PATCH/Test] Build in merge is broken
From: Sverre Hvammen Johansen @ 2008-07-13 8:13 UTC (permalink / raw)
To: git
This test case shows breakage of build in merge. This have been
bisected to 1c7b76be Build in merge.
---
Great that we now are introducing merge in C. Great job Miklos.
However, it is broken as this patch shows. This have been
bisected to 1c7b76be Build in merge.
The test case when run will record the parents that were asked for which is
fine. However, only c2 is recorded as a parent in the commit object. Both
c1 and c2 should have been recorded. The merge is otherwise working
correctly.
t/t7600-merge.sh | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 16f4608..a96a497 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -465,4 +465,15 @@ test_expect_success 'merge log message' '
test_debug 'gitk --all'
+test_expect_success 'merge c1 with c0, c2, c0, and c1' '
+ git reset --hard c1 &&
+ git config branch.master.mergeoptions "" &&
+ test_tick &&
+ git merge c0 c2 c0 c1 &&
+ verify_merge file result.1-5 &&
+ verify_parents $c1 $c2
+'
+
+test_debug 'gitk --all'
+
test_done
--
1.5.5.54.gc6550
^ permalink raw reply related
* Re: [PATCH 3/3 FIXED] help (Windows): Display HTML in default browser using Windows' shell API
From: Junio C Hamano @ 2008-07-13 8:58 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Steffen Prohaska, git, Johannes Schindelin
In-Reply-To: <1215895277.487916ed43464@webmail.nextra.at>
Johannes Sixt <johannes.sixt@telecom.at> writes:
> Zitat von Junio C Hamano <gitster@pobox.com>:
>
>> Steffen Prohaska <prohaska@zib.de> writes:
>>
>> >> Do you mean to have that printf() or is it a leftover debugging
>> >> statement?
>> >
>> > I mean to have it.
>>
>> Ok, I was just checking. Unless other Windows users complain, will apply
>> as-is. As you might guess, I am completely neutral on this one.
>
> I'm working on followups to this series, and it turns out to be more
> convenient to have system_path() in exec_cmd.c instead of path.c.
> It'll make sense if I resend the series with an updated version of 1/3
> (instead of a patch that merely moves the function around).
Ok, will drop these three patches and wait for replacement from yours to
appear, and then we will see which ones to apply.
^ permalink raw reply
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