* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-02-04 21:31 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <20120127094145.GA2611@burratino>
On Fri, 27 Jan 2012 10:41:45 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Frans Klaver wrote:
>
>> Just for my understanding: before a command is executed, a pager
>> (less/more or so) is started? We want to avoid starting the pager if
>> we won't be able to execute the command?
>
> See [1] for an example of a recent patch touching the relevant
> code path.
>
> For example: if I run "git --paginate foo", foo is an alias for bar,
> and the "[pager] bar" configuration is set to point to "otherpager",
> then without this safety git launches the default pager in preparation
> for running git-foo, receives ENOENT from execvp("git-foo"), and then
> the pager has already been launched and it is too late to launch
> otherpager instead.
Took me a while to catch your drift, but if I understand correctly, you're
thinking using some of the code to find out if starting the pager is a
good idea or not. If I factor out the part that finds a command in PATH,
there's the helper that with a fair amount of certainty, will predict
whether 'git foo' will fail with ENOENT or not. It would fix a possible
problem that is currently there. Obviously the only case we can catch, is
the command not actually existing. Although it is just one of the cases
ENOENT can be returned for, I think it is the only one git actually cares
about when checking for it.
>> On Fri, Jan 27, 2012 at 9:48 AM, Jonathan Nieder <jrnieder@gmail.com>
>> wrote:
>
>>> I want to like (b), but the downside seems unacceptable.
>>
>> The downside being: having to figure out what execvp is going to do?
>> That would be tantamount to writing your own execvp.
>
> Exactly.
So as it seems, there are a few cases where we can fairly reliably predict
whether a command is or isn't going to be found. Unless I'm mistaken,
dashed externals are never shell built-ins and so we don't have to be able
to check for their existence. Then assuming that silent_exec_failure
really only cares about commands actually not existing, we can be fairly
naive about it. See if we can find it somewhere in PATH and if we can't
bail out. If we can, start the pager and everything execvp then returns
will be regarded a fatal error. In this case it would be a choice between
spawning the wrong pager, or having a quick browse through the file system.
> That's part of why I was really grateful to Hannes for the reminder to
> take a step back for a moment and consider whether it's worth it.
It may be a sensible reminder. I didn't understand that comment as such.
Maybe it's Hannes' style, I don't know.
> Maybe there's another way or a more targetted way to take care of the
> motivational original confusing scenario that leads to execvp errors.
> (By the way, can you remind me which one that was?)
Been thinking about it and I doubt it. To find out whether EACCES is
returned due to a PATH issue, you have to go through all of those PATH
entries. So while you're at it, there's a lot more you can check and most
of those checks are fairly trivial to do.
I think I've worked through all your review comments. I'll address Hannes'
comments, create an RFC series and see where we end up.
Junio, care to be CC'd in that?
Thanks,
Frans
> [1] http://thread.gmane.org/gmane.comp.version-control.git/179635
^ permalink raw reply
* Re: How to change the index for git status?
From: PJ Weisberg @ 2012-02-04 21:24 UTC (permalink / raw)
To: Peng Yu; +Cc: git
In-Reply-To: <CABrM6wngVDotJoe3Yi5cA_n=JWpLa+M35QEeoKqr1tsp5GD3ng@mail.gmail.com>
On Sat, Feb 4, 2012 at 12:08 PM, Peng Yu <pengyu.ut@gmail.com> wrote:
> Hi,
>
> When I run git status, it show the status compared against the branch
> upstream/develop. I want it to compare against origin/develop. I
> checked progit. But I don't see how to change the branch to compare
> to. Could anybody let me know how to do so? Thanks!
>
> ~/dvcs_src/craftyjs1/Crafty/build/api$ git status
> # On branch 2D_doc_dev
> # Your branch is ahead of 'upstream/develop' by 2 commits.
> #
> nothing to commit (working directory clean)
git branch --set-upstream 2D_doc_dev origin/develop
-PJ
Gehm's Corrollary to Clark's Law: Any technology distinguishable from
magic is insufficiently advanced.
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Jonathan Nieder @ 2012-02-04 21:15 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <CAMP44s1ZPQJzHzYj7e4Kj3Cu+qq0Q3uKrwsE=xS7BmmSqd3gSw@mail.gmail.com>
Felipe Contreras wrote:
> Title: mailmap: add simple name translation test
Thanks. I guess you think I'm stupid. I have no idea how I can
correct that assumption and help you to actually work with me to make
the code better. :/
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 21:13 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <CAMP44s3tmiPGgAUakUgoen2aJcsKw4CygtF5f=4x2dxNTrGbGA@mail.gmail.com>
Felipe Contreras wrote:
> On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Since I didn't receive a copy of the cover letter or patch 1, I don't
>> know what this is intended to test _for_. Good --- I can more easily
>> convey the reaction of future readers who do not necessarily know the
>> context in which the patch was written (and the commit message does
>> not seem to say).
[...]
> Look at the title:
> add 'git blame -e' tests
>
> s/blame/blame -e/
And? After copy/pasting this particular test with that substitution,
what do we get a test for? What class of problem is it supposed to
catch? I do not think a sentence or two is too much to ask for.
By the way, "I blindly copy/pasted" does not seem like a very sensible
excuse for writing meaningless code (such as the "# git blame" comment
line). Before the code contained one riddle. Afterwards it has two.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 21:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <20120204201218.GF22928@burratino>
On Sat, Feb 4, 2012 at 10:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Similar comments to the last patch apply here. This time the patch
> is even more mysterious, since it seems to touch a number of test
> assertions, even while I assume not all of them relate to whatever
> this is supposed to check for.
Title: mailmap: add simple name translation test
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 21:04 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <20120204201027.GE22928@burratino>
On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Since I didn't receive a copy of the cover letter or patch 1, I don't
> know what this is intended to test _for_. Good --- I can more easily
> convey the reaction of future readers who do not necessarily know the
> context in which the patch was written (and the commit message does
> not seem to say).
>
> Looking above, I see
>
> - a lone comment "git blame". What is it trying to tell me? I guess
> you copy/pasted it, but is there any purpose to it?
>
> - a test asserting the claim "Blame output (complex mapping)". This
> title is identical to the test before. I have no idea what this is
> about.
Look at the title:
add 'git blame -e' tests
s/blame/blame -e/
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Jakub Narebski @ 2012-02-04 20:59 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s2gLvXXCTTpAV78=DeJA9dSV793+bx=yJmns7vCwegagQ@mail.gmail.com>
On Sat, 4 Feb 2012, Felipe Contreras wrote:
>
> Typo: improved
Yeah, I have noticed this just as I have send it, but because it is in
_cover letter_ rather than in commit message...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Jonathan Nieder @ 2012-02-04 20:12 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <1328385024-6955-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Similar comments to the last patch apply here. This time the patch
is even more mysterious, since it seems to touch a number of test
assertions, even while I assume not all of them relate to whatever
this is supposed to check for.
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 20:10 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <1328385024-6955-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Thanks for writing tests. I think there is room for a few lines of
explanation above.
[...]
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
> test_cmp expect actual.fuzz
> '
>
> +# git blame
> +cat >expect <<\EOF
> +^OBJI (<author@example.com> DATE 1) one
> +OBJID (<some@dude.xx> DATE 2) two
> +OBJID (<other@author.xx> DATE 3) three
> +OBJID (<other@author.xx> DATE 4) four
> +OBJID (<santa.claus@northpole.xx> DATE 5) five
> +OBJID (<santa.claus@northpole.xx> DATE 6) six
> +OBJID (<cto@company.xx> DATE 7) seven
> +EOF
> +test_expect_success 'Blame output (complex mapping)' '
Since I didn't receive a copy of the cover letter or patch 1, I don't
know what this is intended to test _for_. Good --- I can more easily
convey the reaction of future readers who do not necessarily know the
context in which the patch was written (and the commit message does
not seem to say).
Looking above, I see
- a lone comment "git blame". What is it trying to tell me? I guess
you copy/pasted it, but is there any purpose to it?
- a test asserting the claim "Blame output (complex mapping)". This
title is identical to the test before. I have no idea what this is
about.
Puzzled,
Jonathan
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:08 UTC (permalink / raw)
To: git
In-Reply-To: <CABrM6wm2MochqgC0+FByDA-6nmo0XHtZy1FDXJRkc30+2eyiTw@mail.gmail.com>
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Felipe Contreras @ 2012-02-04 20:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <1328359648-29511-1-git-send-email-jnareb@gmail.com>
On Sat, Feb 4, 2012 at 2:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Typo: improved
--
Felipe Contreras
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:06 UTC (permalink / raw)
To: git
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* RE: Git performance results on a large repository
From: Joshua Redstone @ 2012-02-04 20:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
One more follow-on thought. I imagine that most consumers of git are nowhere near the scale of the test repo that I described. They may still enjoy benefit from efforts to improve git support for large repos. A few possible reasons:
1. The performance improvements should speed things up for smaller repos as well.
2. They may find their repos growing to a 'large scale' at some point in the future.
3. Any code cleanup as part of an effort to support git scalability is good for code base health and e.g., would facilitate future modifications that may more directly affect them.
Cheers,
Josh
________________________________________
From: Nguyen Thai Ngoc Duy [pclouds@gmail.com]
Sent: Friday, February 03, 2012 10:53 PM
To: Joshua Redstone
Cc: git@vger.kernel.org
Subject: Re: Git performance results on a large repository
On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
>
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
Have you tried "git update-index --assume-unchaged"? That should
reduce mass lstat() and hopefully improve the above numbers. The
interface is not exactly easy-to-use, but if it has significant gain,
then we can try to improve UI.
On the index size issue, ideally we should make minimum writes to
index instead of rewriting 191 MB index. An improvement we could do
now is to compress it, reduce disk footprint, thus disk I/O. If you
compress the index with gzip, how big is it?
--
Duy
^ permalink raw reply
* [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras, Jonathan Nieder,
Marius Storm-Olsen
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f6..b1bc521 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
test_cmp expect actual.fuzz
'
+# git blame
+cat >expect <<\EOF
+^OBJI (<author@example.com> DATE 1) one
+OBJID (<some@dude.xx> DATE 2) two
+OBJID (<other@author.xx> DATE 3) three
+OBJID (<other@author.xx> DATE 4) four
+OBJID (<santa.claus@northpole.xx> DATE 5) five
+OBJID (<santa.claus@northpole.xx> DATE 6) six
+OBJID (<cto@company.xx> DATE 7) seven
+EOF
+test_expect_success 'Blame output (complex mapping)' '
+ git blame -e one >actual &&
+ cp actual /tmp &&
+ cp internal_mailmap/.mailmap /tmp &&
+ fuzz_blame actual >actual.fuzz &&
+ test_cmp expect actual.fuzz
+'
+
test_done
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Marius Storm-Olsen,
Jim Meyering, Jonathan Nieder
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index b1bc521..589e39e 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -157,6 +157,9 @@ A U Thor <author@example.com> (1):
CTO <cto@company.xx> (1):
seventh
+Mr. Right <right@company.xx> (1):
+ eight
+
Other Author <other@author.xx> (2):
third
fourth
@@ -196,6 +199,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
test_tick &&
git commit --author "CTO <cto@coompany.xx>" -m seventh &&
+ echo eight >>one &&
+ git add one &&
+ test_tick &&
+ git commit --author "Wrong <right@company.xx>" -m eight &&
+
mkdir -p internal_mailmap &&
echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
@@ -204,6 +212,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
echo "Other Author <other@author.xx> <nick2@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
+ echo "Mr. Right <right@company.xx>" >> internal_mailmap/.mailmap &&
git shortlog -e HEAD >actual &&
test_cmp expect actual
@@ -212,6 +221,9 @@ test_expect_success 'Shortlog output (complex mapping)' '
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
+Author Wrong <right@company.xx> maps to Mr. Right <right@company.xx>
+Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
+
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
@@ -248,6 +260,7 @@ OBJID (Other Author DATE 4) four
OBJID (Santa Claus DATE 5) five
OBJID (Santa Claus DATE 6) six
OBJID (CTO DATE 7) seven
+OBJID (Mr. Right DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame one >actual &&
@@ -264,6 +277,7 @@ OBJID (<other@author.xx> DATE 4) four
OBJID (<santa.claus@northpole.xx> DATE 5) five
OBJID (<santa.claus@northpole.xx> DATE 6) six
OBJID (<cto@company.xx> DATE 7) seven
+OBJID (<right@company.xx> DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame -e one >actual &&
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 1/3] blame: fix email output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Brian Gianforcaro,
Marius Storm-Olsen, Junio C Hamano
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
An extra '>' is added in some cases (<spearce@spearce.org>>), for
example:
% git blame -e -L 947,+7 contrib/completion/git-completion.bash v1.7.9
The current code assumes map_user() *always* returns plain emails, but
that's not true; when there's no email substitution (only name),
map_user() would return 1, but don't touch the mail.
Also, add some tests.
This got broken by d20d654[1].
[1] Change current mailmap usage to do matching on both name and email
of author/committer.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/blame.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..dd69e51 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1403,10 +1403,13 @@ static void get_ac_line(const char *inbuf, const char *what,
* Now, convert both name and e-mail using mailmap
*/
if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
- /* Add a trailing '>' to email, since map_user returns plain emails
- Note: It already has '<', since we replace from mail+1 */
+ /*
+ * Add a trailing '>' to email, since map_user returns plain
+ * emails when it finds a matching mail.
+ * Note: It already has '<', since we replace from mail + 1
+ */
mailpos = memchr(mail, '\0', mail_len);
- if (mailpos && mailpos-mail < mail_len - 1) {
+ if (mailpos && mailpos-mail < mail_len - 1 && *(mailpos - 1) != '>') {
*mailpos = '>';
*(mailpos+1) = '\0';
}
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 0/3] blame: fix output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
The fix, and the tests.
Felipe Contreras (3):
blame: fix email output with mailmap
t: mailmap: add 'git blame -e' tests
t: mailmap: add simple name translation test
builtin/blame.c | 9 ++++++---
t/t4203-mailmap.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
--
1.7.9.1.g97f7d
^ permalink raw reply
* [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-04 19:45 UTC (permalink / raw)
To: git
Git includes protection against rewriting published history on the
receive side with fast-forward check by default (which can be
overridden) and various receive.deny* configuration variables,
including receive.denyNonFastForwards.
Nevertheless git users requested (among others in Git User's Survey)
more help on creation side, namely preventing rewriting parts of
history which was already made public (or at least warning that one is
about to rewrite published history). The "warn before/when rewriting
published history" answer in "17. Which of the following features would
you like to see implemented in git?" multiple-choice question in latest
Git User's Survey 2011[1] got 24% (1525) responses.
[1]: https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
So people would like for git to warn them about rewriting history before
they attempt a push and it turns out to not fast-forward.
What prompted this email is the fact that Mercurial includes support for
tracking which revisions (changesets) are safe to modify in its 2.1
latest version:
http://lwn.net/Articles/478795/
http://mercurial.selenic.com/wiki/WhatsNew
It does that by tracking so called "phase" of a changeset (revision).
http://mercurial.selenic.com/wiki/Phases
http://mercurial.selenic.com/wiki/PhasesDevel
http://www.logilab.org/blogentry/88203
http://www.logilab.org/blogentry/88219
http://www.logilab.org/blogentry/88259
While we don't have to play catch-up with Mercurial features, I think
something similar to what Mercurial has to warn about rewriting
published history (amend, rebase, perhaps even filter-branch) would
be nice to have. Perhaps even follow UI used by Mercurial, and/or
translating its implementation into git terms.
In Mercurial 2.1 there are three available phases: 'public' for
published commits, 'draft' for local un-published commits and
'secret' for local un-published commits which are not meant to
be published.
The phase of a changeset is always equal to or higher than the phase
of it's descendants, according to the following order:
public < draft < secret
Commits start life as 'draft', and move to 'public' on push.
Mercurial documentation talks about phase of a commit, which might
be a good UI, ut also about commits in 'public' phase being "immutable".
As commits in Git are immutable, and rewriting history is in fact
re-doing commits, this description should probably be changed.
While default "push matching" behavior makes it possible to have
"secret" commits, being able to explicitly mark commits as not for
publishing might be a good idea also for Git.
What do you think about this?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Felipe Contreras @ 2012-02-04 19:30 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120204182611.GA31091@sigill.intra.peff.net>
On Sat, Feb 4, 2012 at 8:26 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
>
>> In any case, the one to blame for the header corruption is git:
>> [...]
>> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>>
>> Notice the mail is wrong.
>
> Ugh. The fault lies in this code:
Yes, I found that as well.
> But that comment is wrong. If there's no email mapping needed, map_user
> will leave the "mail" buffer intact, in which case it will have the
> trailing ">" (because we feed the address with enclosing angle
> brackets). So while map_user tries to accept either "foo@example.com\0"
> and "foo@example.com>", it is up to the contents of the mailmap whether
> you get back something with the closing angle bracket or not. Which is a
> pretty error-prone interface.
>
> You can fix it with this:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 5a67c20..9b886fa 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
> /* Add a trailing '>' to email, since map_user returns plain emails
> Note: It already has '<', since we replace from mail+1 */
> mailpos = memchr(mail, '\0', mail_len);
> - if (mailpos && mailpos-mail < mail_len - 1) {
> + if (mailpos && mailpos-mail < mail_len - 1 &&
> + mailpos > mail && *(mailpos-1) != '>') {
> *mailpos = '>';
> *(mailpos+1) = '\0';
> }
Yes, I already did this. I'm writing tests for this right now, but I
think I found yet another bug...
> but it feels like the fix should go into map_user. I tried a few things,
> like "git log -1 --format=%aE", and couldn't find other code paths with
> this problem. So presumably they are all feeding email addresses without
> the closing ">" (so one option is to just say "map_user needs to get
> NUL-terminated strings).
Perhaps, but I though the idea was to make it efficient. I think the
above fix should be ok.
We should have tests for this though, to make sure it doesn't get
broken again. I'm on that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 0/9] respect binary attribute in grep
From: Pete Wyckoff @ 2012-02-04 19:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Thomas Rast, Conrad Irwin, git,
Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120202081747.GA10271@sigill.intra.peff.net>
I took a look at this series. It's nice. My worry was that the
extra open() of non-existent .gitattributes files in all the
directories would cause performance problems across networked
filesystems like NFS.
My usual (non-public) repository has order:
100k files
10k directories
and no files marked as binary. The grep string is such that it
is disk-bound, and not expected to match in any file (or binary):
"time ~/src/git/bin-wrappers/git grep unfindable-string".
With your change, there are 10k new open() calls looking for
.gitattributes in each directory, all of which return ENOENT.
This turns out to have an insignificant impact on performance due
to the much bigger time sink of stat()-ing all the files.
I think this happens to be true because the gitattributes lookups
run in parallel to all the file stat work, as the main thread
dispatches file work while doing its own gitattributes lookups.
It could be plausible that deep directory structures with few
grep-able files will suffer with this change. For example, many
big binary blobs in deep directory hierarchies, but also some
useful files here and there.
One could argue that with the use of .gitattributes to specify
which blobs should not be searched, this series makes this faster
by not having to to read the binary blobs at all. And I'd be
okay with that.
Just FYI that there may be a performance impact on certain
repositories.
-- Pete
^ permalink raw reply
* [PATCH/RFCv2 5/5] gitweb: Highlight matched part of shortened project description
From: Jakub Narebski @ 2012-02-04 18:56 UTC (permalink / raw)
To: git
In-Reply-To: <1328359648-29511-6-git-send-email-jnareb@gmail.com>
Previous commit make gitweb use esc_html_match_hl() to mark match in
the _whole_ description of a project when searching projects.
This commit makes gitweb highlight match in _shortened_ description,
based on match in whole description, using esc_html_match_hl_chopped()
subroutine.
If match is in removed (chopped) part, even partially, then trailing
"... " is highlighted.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version removes code duplication from v1.
It is still marked as RFC, because I am not sure if it is right way
to highlight match in shortened string, or if we better use full string,
or full string if match is in chopped part.
gitweb/gitweb.perl | 40 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c650268..f71afe0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1720,11 +1720,46 @@ sub esc_html_match_hl {
my ($str, $regexp) = @_;
return esc_html($str) unless defined $regexp;
+ return esc_html_match_hl_chopped($str, undef, $regexp);
+}
+
+
+# highlight match (if any) of shortened string, and escape HTML
+sub esc_html_match_hl_chopped {
+ my ($str, $chopped, $regexp) = @_;
+ return esc_html($chopped) unless defined $regexp;
+
my @matches;
while ($str =~ /$regexp/g) {
push @matches, [$-[0], $+[0]];
}
- return esc_html($str) unless @matches;
+ return esc_html(defined $chopped ? $chopped : $str) unless @matches;
+
+ # filter matches so that we mark chopped string, if it is present
+ if (defined $chopped) {
+ my $tail = "... "; # see chop_str
+ unless ($chopped =~ s/\Q$tail\E$//) {
+ $tail = '';
+ }
+ my $chop_len = length($chopped);
+ my $tail_len = length($tail);
+ my @filtered;
+
+ for my $m (@matches) {
+ if ($m->[0] > $chop_len) {
+ push @filtered, [ $chop_len, $chop_len + $tail_len ] if ($tail_len > 0);
+ last;
+ } elsif ($m->[1] > $chop_len) {
+ push @filtered, [ $m->[0], $chop_len + $tail_len ];
+ last;
+ }
+ push @filtered, $m;
+ }
+
+ # further operations are on chopped string
+ $str = $chopped . $tail;
+ @matches = @filtered;
+ }
my $out = '';
my $pos = 0;
@@ -5372,7 +5407,8 @@ sub git_project_list_rows {
"<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-class => "list", -title => $pr->{'descr_long'}},
$search_regexp
- ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+ ? esc_html_match_hl_chopped($pr->{'descr_long'},
+ $pr->{'descr'}, $search_regexp)
: esc_html($pr->{'descr'})) .
"</td>\n" .
"<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
--
1.7.9
^ permalink raw reply related
* [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-04 18:26 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s2QdJ4+qgg4fF5-DOWHx3Btd0pTivTT9s_E=qqxg16YLQ@mail.gmail.com>
On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
> In any case, the one to blame for the header corruption is git:
> [...]
> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>
> Notice the mail is wrong.
Ugh. The fault lies in this code:
$ sed -n 1405,1414p builtin/blame.c
if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
if (mailpos && mailpos-mail < mail_len - 1) {
*mailpos = '>';
*(mailpos+1) = '\0';
}
}
}
But that comment is wrong. If there's no email mapping needed, map_user
will leave the "mail" buffer intact, in which case it will have the
trailing ">" (because we feed the address with enclosing angle
brackets). So while map_user tries to accept either "foo@example.com\0"
and "foo@example.com>", it is up to the contents of the mailmap whether
you get back something with the closing angle bracket or not. Which is a
pretty error-prone interface.
You can fix it with this:
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..9b886fa 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
- if (mailpos && mailpos-mail < mail_len - 1) {
+ if (mailpos && mailpos-mail < mail_len - 1 &&
+ mailpos > mail && *(mailpos-1) != '>') {
*mailpos = '>';
*(mailpos+1) = '\0';
}
but it feels like the fix should go into map_user. I tried a few things,
like "git log -1 --format=%aE", and couldn't find other code paths with
this problem. So presumably they are all feeding email addresses without
the closing ">" (so one option is to just say "map_user needs to get
NUL-terminated strings).
-Peff
^ permalink raw reply related
* RE: Git performance results on a large repository
From: Joshua Redstone @ 2012-02-04 18:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
[ wanted to reply to my initial msg, but wasn't subscribed to the list at time of mailing, so replying to most recent post instead ]
Thanks to everyone for the questions and suggestions. I'll try to respond here. One high-level clarification - this synthetic repo for which I've reported perf times is representative of where we think we'll be in the future. Git is slow but marginally acceptable for today. We want to start planning now for any big changes we need to make going forward.
Evgeny Sazhin, Slinky and Ævar Arnfjörð Bjarmason suggested splitting up the repo into multiple, smaller repos. I indicated before that we have a lot of cross-dependencies. Our largest repo by number of files and commits is the repo containing the front-end server. It is a large code base in which the tight integration of various components results in many of the cross dependencies. We are working slowly to split things up more, for example into services, but that is a long-term process.
To get a bit abstract for a moment, in an ideal world, it doesn't seem like performance constraints of a source-control-system should dictate how we choose to structure our code. Ideally, seems like we should be able to choose to structure our code in whatever way we feel maximizes developer productivity. If development and code/release management seem easier in a single repo, than why not make an SCM that can handle it? This is one reason I've been leaning towards figuring out an SCM approach that can work well with our current practices rather than changing them as a prerequisite for good SCM performance.
Sam Vilain: Thanks for the pointer, i didn't realize that fast-import was bi-directional. I used it for generating the synthetic repo. Will look into using it the other way around. Though that still won't speed up things like git-blame, presumably? The sparse-checkout issue you mention is a good one. There is a good question of how to support quick checkout, branch switching, clone, push and so forth. I'll look into the approaches you suggest. One consideration is coming up with a high-leverage approach - i.e. not doing heavy dev work if we can avoid it. On the other hand, it would be nice if we (including the entire community :) ) improve git in areas that others that share similar issues benefit from as well.
Matt Graham: I don't have file stats at the moment. It's mostly code files, with a few larger data files here and there. We also don't do sparse checkouts, primarily because most people use git (whether on top of SVN or not), which doesn't support it.
Chris Lee: When I was building up the repo (e.g., doing lots of commits, before I started using fast-import), i noticed that flash was not much faster - stat'ing the whole repo takes a lot of kernel time, even with flash. My hunch is that we'd see similar issues with other operations, like git-blame.
Zeki Mokhtarzada: Dumping history I think would speed up operations for which we don't care about old history, like git-blame in which we only want to see recent modifications. We'd also need a good story for other kinds of operations. In my mental model of git scalability, I categorize git structures into three kinds: those for reasoning about history, those for the index and those for the working directory (yeah, I know these don't map precisely to actual on-disk things like the object store, including trees, etc.). One scaling approach we've been thinking of is to focus on each individually: develop a specialized thing to handle history commands efficiently (git-blame, git-log, git-diff, etc.), something to speed up or bypass the index, and something to make large changes to the working directly quickly.
Joey Hess: Separating the factors is a good suggestion. My hunch is that the various git operations test the performance issues in isolation. For example, git-status performance depends just on the number of files, not on the depth of history. On the other hand, my guess is that git-blame performance is more a function of the length of history rather than the number of files. Though, certainly with compression and indexing in pack files, I could imagine there being cross-effects between length of history and number of files. The git-status suggestion definitely helps when you know which directory you are concerned about. Often I'm lazy and stat the repo root so I trade-off slowness for being more sure I'm not missing anything.
@Joey, I think you're also touching on a good meta point which is that, there's probably no silver bullet here. If we want git to efficiently handle repos that are large across a number of dimensions (size, # commits, # files, etc.), there's multiple parts of git that would need enhancement of some form.
Nguyen Thai Ngoc Duy: At which point in the test flow should I insert git-update-index? I'm happy to try it out. Will compress index when I next get to a terminal. My guess is it'll compress a bunch. It's also conceivable that, if there were an external interface in git to attach other systems to efficiently report which files have changed (e.g., via file-system integration), it's possible that we could omit managing the index in many cases. I know that would be a big change, but the benefits are intriguing.
Cheers,
Josh
________________________________________
From: Nguyen Thai Ngoc Duy [pclouds@gmail.com]
Sent: Friday, February 03, 2012 10:53 PM
To: Joshua Redstone
Cc: git@vger.kernel.org
Subject: Re: Git performance results on a large repository
On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
>
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
Have you tried "git update-index --assume-unchaged"? That should
reduce mass lstat() and hopefully improve the above numbers. The
interface is not exactly easy-to-use, but if it has significant gain,
then we can try to improve UI.
On the index size issue, ideally we should make minimum writes to
index instead of rewriting 191 MB index. An improvement we could do
now is to compress it, reduce disk footprint, thus disk I/O. If you
compress the index with gzip, how big is it?
--
Duy
^ permalink raw reply
* [PATCH] send-email: add extra safetly in address sanitazion
From: Felipe Contreras @ 2012-02-04 16:32 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Brandon Casey, Uwe Kleine-König,
Brian Gernhardt, Robin H. Johnson,
Ævar Arnfjörð Bjarmason
Currently bad addresses like 'Foo Bar <foo@bar.com>>' will just be sent
verbatim -- that's not good; we should either error out, or sanitize
them.
The following patch adds extra sanitazion so the following
transformations are performed:
'Foo Bar <foo@bar.com>' -> 'Foo Bar <foo@bar.com>'
'"Foo Bar" <foo@bar.com>' -> '"Foo Bar" <foo@bar.com>'
'foo@bar.com' -> 'foo@bar.com'
'<foo@bar.com>' -> 'foo@bar.com'
'Foo Bar' -> 'Foo Bar'
'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
'"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
'<foo@bar.com>>' -> 'foo@bar.com'
Basically, we try to check that the address is in the form of
"Name <email>", and if not, assume it's "email". According to commit
155197e[1], the "prhase" should not be empty, so if it is, remove the
<>. Extra characters after the first ">" are ignored.
[1] send-email: rfc822 forbids using <address@domain> without a non-empty "phrase"
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-send-email.perl | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index ef30c55..19c600f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -889,15 +889,21 @@ sub is_rfc2047_quoted {
# use the simplest quoting being able to handle the recipient
sub sanitize_address {
my ($recipient) = @_;
- my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
+ my ($recipient_name, $recipient_addr);
+
+ if ($recipient =~ /^(.*?)\s*<(.*?)>/) {
+ ($recipient_name, $recipient_addr) = ($1, $2);
+ } else {
+ $recipient_addr = $recipient;
+ }
if (not $recipient_name) {
- return $recipient;
+ return $recipient_addr;
}
# if recipient_name is already quoted, do nothing
if (is_rfc2047_quoted($recipient_name)) {
- return $recipient;
+ return "$recipient_name <$recipient_addr>";
}
# rfc2047 is needed if a non-ascii char is included
@@ -912,7 +918,7 @@ sub sanitize_address {
$recipient_name = qq["$recipient_name"];
}
- return "$recipient_name $recipient_addr";
+ return "$recipient_name <$recipient_addr>";
}
--
1.7.9
^ permalink raw reply related
* [PATCH v2] branch: borrow --sort and --count from for-each-ref
From: Nguyễn Thái Ngọc Duy @ 2012-02-04 16:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326805907-19416-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Fix sorted refs being fnmatch()'d twice.
Documentation/git-branch.txt | 13 +++++++++
Makefile | 1 +
builtin/branch.c | 60 +++++++++++++++++++++++++++++++++--------
builtin/for-each-ref.c | 33 +++-------------------
builtin/for-each-ref.h | 32 ++++++++++++++++++++++
5 files changed, 99 insertions(+), 40 deletions(-)
create mode 100644 builtin/for-each-ref.h
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0427e80..b6f2826 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git branch' [--color[=<when>] | --no-color] [-r | -a]
[--list] [-v [--abbrev=<length> | --no-abbrev]]
+ [--count=<count>] [(--sort=<key>)...]
[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
'git branch' (-m | -M) [<oldbranch>] <newbranch>
@@ -192,6 +193,18 @@ start-point is either a local or remote-tracking branch.
The new name for an existing branch. The same restrictions as for
<branchname> apply.
+<count>::
+ By default the command shows all refs that match
+ `<pattern>`. This option makes it stop after showing
+ that many refs.
+
+<key>::
+ A field name to sort on. Prefix `-` to sort in descending
+ order of the value. When unspecified, `refname` is used. You
+ may use the --sort=<key> option multiple times, in which case
+ the last key becomes the primary key. See
+ linkgit:for-each-ref[1] for field name details.
+
Examples
--------
diff --git a/Makefile b/Makefile
index a782409..daf3e46 100644
--- a/Makefile
+++ b/Makefile
@@ -2108,6 +2108,7 @@ builtin/log.o builtin/shortlog.o: shortlog.h
builtin/prune.o builtin/reflog.o reachable.o: reachable.h
builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
builtin/tar-tree.o archive-tar.o: tar.h
+builtin/branch.o builtin/for-each-ref.o: builtin/for-each-ref.h
connect.o transport.o url.o http-backend.o: url.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
diff --git a/builtin/branch.c b/builtin/branch.c
index 7095718..97bf96c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -15,6 +15,7 @@
#include "branch.h"
#include "diff.h"
#include "revision.h"
+#include "for-each-ref.h"
static const char * const builtin_branch_usage[] = {
"git branch [options] [-r | -a] [--merged | --no-merged]",
@@ -30,6 +31,9 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static unsigned char head_sha1[20];
+static struct ref_sort *sort = NULL, **sort_tail = &sort;
+static int maxcount;
+
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -312,7 +316,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
if ((kind & ref_list->kinds) == 0)
return 0;
- if (!match_patterns(cb->pattern, refname))
+ if (cb->pattern && !match_patterns(cb->pattern, refname))
return 0;
commit = NULL;
@@ -510,10 +514,37 @@ static void show_detached(struct ref_list *ref_list)
}
}
-static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern)
+static int fetch_branches(struct ref_list *ref_list,
+ const char **pattern)
{
- int i;
struct append_ref_cb cb;
+ memset(&cb, 0, sizeof(cb));
+ cb.ref_list = ref_list;
+ if (sort) {
+ struct grab_ref_cbdata cbdata;
+ int i;
+ memset(&cbdata, 0, sizeof(cbdata));
+ cbdata.grab_pattern = pattern;
+ for_each_rawref(grab_single_ref, &cbdata);
+ sort_refs(sort, cbdata.grab_array, cbdata.grab_cnt);
+ for (i = 0; i < cbdata.grab_cnt; i++) {
+ struct refinfo *ri = cbdata.grab_array[i];
+ append_ref(ri->refname, ri->objectname, ri->flag, &cb);
+ }
+ } else {
+ cb.pattern = pattern;
+ for_each_rawref(append_ref, &cb);
+ qsort(ref_list->list, ref_list->index,
+ sizeof(struct ref_item), ref_cmp);
+ }
+ return cb.ret;
+}
+
+static int print_ref_list(int kinds, int detached, int verbose, int abbrev,
+ struct commit_list *with_commit,
+ const char **pattern)
+{
+ int i, ret;
struct ref_list ref_list;
memset(&ref_list, 0, sizeof(ref_list));
@@ -523,10 +554,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
ref_list.with_commit = with_commit;
if (merge_filter != NO_FILTER)
init_revisions(&ref_list.revs, NULL);
- cb.ref_list = &ref_list;
- cb.pattern = pattern;
- cb.ret = 0;
- for_each_rawref(append_ref, &cb);
+ ret = fetch_branches(&ref_list, pattern);
if (merge_filter != NO_FILTER) {
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
@@ -539,13 +567,13 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
ref_list.maxwidth = calc_maxwidth(&ref_list);
}
- qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
-
detached = (detached && (kinds & REF_LOCAL_BRANCH));
if (detached && match_patterns(pattern, "HEAD"))
show_detached(&ref_list);
- for (i = 0; i < ref_list.index; i++) {
+ if (!maxcount)
+ maxcount = ref_list.index;
+ for (i = 0; i < maxcount; i++) {
int current = !detached &&
(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
!strcmp(ref_list.list[i].name, head);
@@ -558,10 +586,10 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
free_ref_list(&ref_list);
- if (cb.ret)
+ if (ret)
error(_("some refs could not be read"));
- return cb.ret;
+ return ret;
}
static void rename_branch(const char *oldname, const char *newname, int force)
@@ -702,6 +730,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
parse_opt_with_commit, (intptr_t) "HEAD",
},
OPT__ABBREV(&abbrev),
+ OPT_CALLBACK(0 , "sort", sort_tail, "key",
+ "field name to sort on", &opt_parse_sort),
+ OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
OPT_GROUP("Specific git-branch actions:"),
OPT_SET_INT('a', "all", &kinds, "list both remote-tracking and local branches",
@@ -752,6 +783,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
0);
+ if (maxcount < 0) {
+ error("invalid --count argument: `%d'", maxcount);
+ usage_with_options(builtin_branch_usage, options);
+ }
+
if (!delete && !rename && !edit_description && argc == 0)
list = 1;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b01d76a..7b25c54 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
#include "quote.h"
#include "parse-options.h"
#include "remote.h"
+#include "for-each-ref.h"
/* Quoting styles */
#define QUOTE_NONE 0
@@ -19,25 +20,6 @@
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
- unsigned reverse : 1;
-};
-
-struct refinfo {
- char *refname;
- unsigned char objectname[20];
- int flag;
- const char *symref;
- struct atom_value *value;
-};
-
static struct {
const char *name;
cmp_type cmp_type;
@@ -765,17 +747,12 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
*v = &ref->value[atom];
}
-struct grab_ref_cbdata {
- struct refinfo **grab_array;
- const char **grab_pattern;
- int grab_cnt;
-};
-
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int grab_single_ref(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
@@ -858,7 +835,7 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
+void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
{
ref_sort = sort;
qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
@@ -953,7 +930,7 @@ static struct ref_sort *default_sort(void)
return sort;
}
-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_sort(const struct option *opt, const char *arg, int unset)
{
struct ref_sort **sort_tail = opt->value;
struct ref_sort *s;
diff --git a/builtin/for-each-ref.h b/builtin/for-each-ref.h
new file mode 100644
index 0000000..8542d66
--- /dev/null
+++ b/builtin/for-each-ref.h
@@ -0,0 +1,32 @@
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
+struct ref_sort {
+ struct ref_sort *next;
+ int atom; /* index into used_atom array */
+ unsigned reverse : 1;
+};
+
+struct refinfo {
+ char *refname;
+ unsigned char objectname[20];
+ int flag;
+ const char *symref;
+ struct atom_value *value;
+};
+
+struct grab_ref_cbdata {
+ struct refinfo **grab_array;
+ const char **grab_pattern;
+ int grab_cnt;
+};
+
+extern int grab_single_ref(const char *refname,
+ const unsigned char *sha1,
+ int flag, void *cb_data);
+extern int opt_parse_sort(const struct option *opt,
+ const char *arg, int unset);
+extern void sort_refs(struct ref_sort *sort,
+ struct refinfo **refs, int num_refs);
--
1.7.8.36.g69ee2
^ 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