* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Junio C Hamano @ 2011-09-30 18:19 UTC (permalink / raw)
To: Pang Yan Han
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <20110930132903.GA1622@myhost>
Pang Yan Han <pangyanhan@gmail.com> writes:
> Sorry for asking, but do I need to reroll this with the fixup in
> origin/ph/push-to-delete-nothing ? Is the commit message fine especially
> in light of the changes in the fixup?
If you think that the result of squashing the fix-up commit into your
patch looks OK, I do not think there is a need to reroll. The patch title
may need to be shortened, but other than that I do not see anything
glaringly wrong in the commit log message that I cannot amend out.
Thanks.
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: René Scharfe @ 2011-09-30 18:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Martin Fick, Julian Phillips, Christian Couder, git,
Christian Couder, Thomas Rast
In-Reply-To: <7vfwjeotv1.fsf@alter.siamese.dyndns.org>
Am 30.09.2011 18:52, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Hi Martin,
>>
>> Am 29.09.2011 22:11, schrieb Martin Fick:
>>> Your patch works well for me. It achieves about the same
>>> gains as Julian's patch. Thanks!
>>
>> OK, and what happens if you apply the following patch on top of my first
>> one? It avoids going through all the refs a second time during cleanup,
>> at the cost of going through the list of all known objects. I wonder if
>> that's any faster in your case.
>> ...
>> static void describe_one_orphan(struct strbuf *sb, struct commit *commit)
>> @@ -690,8 +689,7 @@ static void orphaned_commit_warning(struct commit *commit)
>> else
>> describe_detached_head(_("Previous HEAD position was"), commit);
>>
>> - clear_commit_marks(commit, -1);
>> - for_each_ref(clear_commit_marks_from_one_ref, NULL);
>> + clear_commit_marks_for_all(ALL_REV_FLAGS);
>> }
>
> The function already clears all the flag bits from commits near the tip of
> all the refs (i.e. whatever commit it traverses until it gets to the fork
> point), so it cannot be reused in other contexts where the caller
>
> - first marks commit objects with some flag bits for its own purpose,
> unrelated to the "orphaned"-ness check;
> - calls this function to issue a warning; and then
> - use the flag it earlier set to do something useful.
>
> which requires "cleaning after yourself, by clearing only the bits you
> used without disturbing other bits that you do not use" pattern.
Yes, clear_commit_marks_for_all is a bit brutal. Callers could clear
specfic bits (e.g. SEEN|UNINTERESTING) instead of ALL_REV_FLAGS, though.
> It might be a better solution to not bother to clear the marks at all;
> would it break anything in this codepath?
Unfortunately, yes; the cleanup part was added by 5c08dc48 later, when
it become apparent that it's really needed.
However, since the patch only buys us a 5% speedup I'm not sure it's
worth it in its current form.
René
^ permalink raw reply
* Re: [RFC/PATCH]: reverse bisect v 2.0
From: Junio C Hamano @ 2011-09-30 18:13 UTC (permalink / raw)
To: Michal Vyskocil
Cc: git, Sverre Rabbelier, Johannes Sixt, Jeff King, Christian Couder
In-Reply-To: <20110930114220.GA742@zelva.suse.cz>
[administrivia: added people involved in the main discussion thread back
on CC line, and also added CCouder who seems to be fond of the command]
Michal Vyskocil <mvyskocil@suse.cz> writes:
> The bugfix command works like the previous git bisect start --reverse.
Does any released version of git have "bisect start --reverse"?
$ git bisect start --reverse
unrecognised option: '--reverse'
Let's suppose that we had a "git frotz" Porcelain subcommand, that used to
say "xyzzy" when all is well, but says "nitfol" these days. I released a
(bad) script that parses the output from the subcommand, and want to say
something like "This script only works with Git version after X", and to
find out the value of X, I need to bisect the history to find when "frotz"
started to say "nitfol".
I am not trying to find a commit that introduced a bug/regression to cause
the recent "frotz" to say "nitfol". Neither am I trying to find a fix that
corrected the earlier bogus output "xyzzy". No value judgement is involved
in this scenario.
I wonder if something along the following line would make the usage more
pleasant and self-explanatory:
$ git bisect start --used-to='git frotz says xyzzy' v0.99 master
Bisecting: 171 revisions left to test after this (roughly 8 steps)
$ ... build and then test ...
$ git bisect tested
You are trying to check: git frotz says xyzzy
Does the result satisify what you are trying to find [y/n]? yes
Saying 'yes' would be like saying 'good' and 'no' would be like saying
'bad' here.
When trying to find regression, you would say:
$ git bisect start --used-to='it works' v0.99 master
and you say 'yes' if it works (equivalent to 'good'), and 'no' if it does
not (equivalent to 'bad').
When trying to find a fix, you would say:
$ git bisect start --used-to='checkout $tree $path clobbers $path' v0.99 master
and you say 'yes' if it is still broken at the tested version (equivalent
to your reversed 'good'), and 'no' if the tested version contains a fix
(equivalent to your reversed 'bad').
I am not married to the name of the option, but --used-to='...' felt more
or less self-explanatory name to signal users what to describe in that
text. The condition "You are trying to check:" cue wants to remind the
user is "we used to do *this* but now we don't", and the command is trying
to see when it changed.
Thoughts?
^ permalink raw reply
* [PATCH] refs: Remove duplicates after sorting with qsort
From: Julian Phillips @ 2011-09-30 17:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: Julian Phillips, Martin Fick, Christian Couder, git,
Christian Couder, Thomas Rast, Michael Haggerty
In-Reply-To: <7vk48qouht.fsf@alter.siamese.dyndns.org>
The previous custom merge sort would drop duplicate entries as part of
the sort. It would also die if the duplicate entries had different
sha1 values. The standard library qsort doesn't do this, so we have
to do it manually afterwards.
Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---
On Fri, 30 Sep 2011 09:38:54 -0700, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> On 09/30/2011 01:48 AM, Junio C Hamano wrote:
>>> This version looks sane, although I have a suspicion that it may
>>> have
>>> some interaction with what Michael may be working on.
>>
>> Indeed, I have almost equivalent changes in the giant patch series
>> that
>> I am working on [1].
>
> Good; that was the primary thing I wanted to know. I want to take
> Julian's patch early but if the approach and data structures were
> drastically different from what you are cooking, that would force
> unnecessary reroll on your part, which I wanted to avoid.
>
> Thanks.
I had a quick look at Michael's code, and it reminded me that I had missed one
thing out. If we want to keep the duplicate detection & removal from the
original merge sort then this patch is needed on top of v3 of the binary search.
Though I never could figure out how duplicate refs were supposed to appear ... I
tested by editing packed-refs, but I assume that isn't "supported".
refs.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 4c01d79..cf080ee 100644
--- a/refs.c
+++ b/refs.c
@@ -77,7 +77,29 @@ static int ref_entry_cmp(const void *a, const void *b)
static void sort_ref_array(struct ref_array *array)
{
+ int i = 0, j = 1;
+
+ /* Nothing to sort unless there are at least two entries */
+ if (array->nr < 2)
+ return;
+
qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
+
+ /* Remove any duplicates from the ref_array */
+ for (; j < array->nr; j++) {
+ struct ref_entry *a = array->refs[i];
+ struct ref_entry *b = array->refs[j];
+ if (!strcmp(a->name, b->name)) {
+ if (hashcmp(a->sha1, b->sha1))
+ die("Duplicated ref, and SHA1s don't match: %s",
+ a->name);
+ warning("Duplicated ref: %s", a->name);
+ continue;
+ }
+ i++;
+ array->refs[i] = array->refs[j];
+ }
+ array->nr = i + 1;
}
static struct ref_entry *search_ref_array(struct ref_array *array, const char *name)
--
1.7.6.1
^ permalink raw reply related
* Re: [GUILT 1/6] Refuse to push corrupt patches
From: Josef 'Jeff' Sipek @ 2011-09-30 17:15 UTC (permalink / raw)
To: Alan Jenkins; +Cc: git
In-Reply-To: <1317219324-10319-1-git-send-email-alan.christopher.jenkins@googlemail.com>
Thanks for the patches. I looked them over and they look good. I'll give
them another thorough reading once I have a bit of time to apply them.
Jeff.
On Wed, Sep 28, 2011 at 03:15:19PM +0100, Alan Jenkins wrote:
> "guilt push" would treat corrupt patches as empty,
> because "git apply --numstat" prints nothing on stdout.
>
> (You do get an error message on stderr,
> but then guilt says "Patch applied" etc,
> and I didn't notice the earlier error message
> for quite some time.)
>
> Signed-off-by: Alan Jenkins <alan.christopher.jenkins@googlemail.com>
> ---
> guilt | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/guilt b/guilt
> index d1e17d4..51532f9 100755
> --- a/guilt
> +++ b/guilt
> @@ -611,7 +611,7 @@ push_patch()
> cd_to_toplevel
>
> # apply the patch if and only if there is something to apply
> - if [ `git apply --numstat "$p" | wc -l` -gt 0 ]; then
> + if [ `do_get_patch "$p" | wc -l` -gt 0 ]; then
> if [ "$bail_action" = abort ]; then
> reject=""
> fi
> --
> 1.7.4.1
>
--
Hegh QaQ law'
quvHa'ghach QaQ puS
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Junio C Hamano @ 2011-09-30 16:52 UTC (permalink / raw)
To: René Scharfe
Cc: Martin Fick, Julian Phillips, Christian Couder, git,
Christian Couder, Thomas Rast, Junio C Hamano
In-Reply-To: <4E8587E8.9070606@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Hi Martin,
>
> Am 29.09.2011 22:11, schrieb Martin Fick:
>> Your patch works well for me. It achieves about the same
>> gains as Julian's patch. Thanks!
>
> OK, and what happens if you apply the following patch on top of my first
> one? It avoids going through all the refs a second time during cleanup,
> at the cost of going through the list of all known objects. I wonder if
> that's any faster in your case.
> ...
> static void describe_one_orphan(struct strbuf *sb, struct commit *commit)
> @@ -690,8 +689,7 @@ static void orphaned_commit_warning(struct commit *commit)
> else
> describe_detached_head(_("Previous HEAD position was"), commit);
>
> - clear_commit_marks(commit, -1);
> - for_each_ref(clear_commit_marks_from_one_ref, NULL);
> + clear_commit_marks_for_all(ALL_REV_FLAGS);
> }
The function already clears all the flag bits from commits near the tip of
all the refs (i.e. whatever commit it traverses until it gets to the fork
point), so it cannot be reused in other contexts where the caller
- first marks commit objects with some flag bits for its own purpose,
unrelated to the "orphaned"-ness check;
- calls this function to issue a warning; and then
- use the flag it earlier set to do something useful.
which requires "cleaning after yourself, by clearing only the bits you
used without disturbing other bits that you do not use" pattern.
It might be a better solution to not bother to clear the marks at all;
would it break anything in this codepath?
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-30 16:41 UTC (permalink / raw)
To: git
Cc: Christian Couder, Thomas Rast, René Scharfe, Julian Phillips,
Michael Haggerty
In-Reply-To: <201109262056.04279.chriscool@tuxfamily.org>
> On Monday, September 26, 2011 12:56:04 pm Christian Couder
wrote:
> After "git pack-refs --all" I get:
OK. So many great improvements in ref scalability, thanks
everyone!
It is getting so good, that I had to take a step back and
re-evaluate what we consider good/bad. On doing so, I can't
help but think that fetches still need some improvement.
Fetches had the worst regression of all > 8days, so the
massive fix to bring it down to 7.5mins was awesome.
7-8mins sounded pretty good 2 weeks ago, especially when a
checkout took 5+ mins! but now that almost every other
operation has been sped up, that is starting to feel a bit
on the slow side still. My spidey sense tells me something
is still not quite right in the fetch path.
Here is some more data to backup my spidey sense: after all
the improvements, a noop fetch of all the changes (noop
meaning they are all already uptodate) takes around
3mins with a non gced (non packed refs) case. That same
noop only takes ~12s in the gced (packed ref case)!
I dug into this a bit further. I took a non gced and non
packed refs repo and this time instead of gcing it to get
packedrefs, I only ran the above git pack-refs --all so that
objects did not get gced. With this, the noop fetch was
also only around 12s. This confirmed that the non gced
objects are not interfering with the noop fetch, the problem
really is just the unpacked refs. Just to confirm that the
FS is not horribly slow, I did a "find .git/refs" and it
only takes about .4s for about 80Kresults!
So, while I understand that a full fetch will actually have
to transfer quite a bit of data, the noop fetch seems like
it is still suffering in the non gced (non packed ref case).
If that time were improved, I suspect that the full fetch
will improve at least by an equivalent amount, if not more.
Any thoughts?
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH v3] refs: Use binary search to lookup refs faster
From: Junio C Hamano @ 2011-09-30 16:38 UTC (permalink / raw)
To: Michael Haggerty
Cc: Julian Phillips, Martin Fick, Christian Couder, git,
Christian Couder, Thomas Rast
In-Reply-To: <4E85E07C.5070402@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 09/30/2011 01:48 AM, Junio C Hamano wrote:
>> This version looks sane, although I have a suspicion that it may have
>> some interaction with what Michael may be working on.
>
> Indeed, I have almost equivalent changes in the giant patch series that
> I am working on [1].
Good; that was the primary thing I wanted to know. I want to take
Julian's patch early but if the approach and data structures were
drastically different from what you are cooking, that would force
unnecessary reroll on your part, which I wanted to avoid.
Thanks.
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-30 16:09 UTC (permalink / raw)
To: René Scharfe
Cc: Julian Phillips, Christian Couder, git, Christian Couder,
Thomas Rast, Junio C Hamano
In-Reply-To: <4E8587E8.9070606@lsrfire.ath.cx>
On Friday, September 30, 2011 03:12:08 am René Scharfe
wrote:
> OK, and what happens if you apply the following patch on
> top of my first one? It avoids going through all the
> refs a second time during cleanup, at the cost of going
> through the list of all known objects. I wonder if
> that's any faster in your case.
This patch helps a bit more. It seems to shave about
another .5s off in packed and non packed case w or w/o
binary search.
-Martin
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 84e0cdc..a4b1003 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -596,15 +596,14 @@ static int
> add_pending_uninteresting_ref(const char *refname,
> return 0;
> }
>
> -static int clear_commit_marks_from_one_ref(const char
> *refname, - const unsigned
char *sha1,
> - int flags,
> - void *cb_data)
> +static void clear_commit_marks_for_all(unsigned int
> mark) {
> - struct commit *commit =
> lookup_commit_reference_gently(sha1, 1); - if (commit)
> - clear_commit_marks(commit, -1);
> - return 0;
> + unsigned int i, max = get_max_object_index();
> + for (i = 0; i < max; i++) {
> + struct object *object =
get_indexed_object(i);
> + if (object && object->type == OBJ_COMMIT)
> + object->flags &= ~mark;
> + }
> }
>
> static void describe_one_orphan(struct strbuf *sb,
> struct commit *commit) @@ -690,8 +689,7 @@ static void
> orphaned_commit_warning(struct commit *commit) else
> describe_detached_head(_("Previous HEAD
position
> was"), commit);
>
> - clear_commit_marks(commit, -1);
> - for_each_ref(clear_commit_marks_from_one_ref, NULL);
> + clear_commit_marks_for_all(ALL_REV_FLAGS);
> }
>
> static int switch_branches(struct checkout_opts *opts,
> struct branch_info *new) --
> To unsubscribe from this list: send the line "unsubscribe
> git" in the body of a message to
> majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH v3] refs: Use binary search to lookup refs faster
From: Martin Fick @ 2011-09-30 15:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: Julian Phillips, Christian Couder, git, Christian Couder,
Thomas Rast
In-Reply-To: <7vwrcqpuc7.fsf@alter.siamese.dyndns.org>
On Thursday, September 29, 2011 09:44:40 pm Junio C Hamano
wrote:
> Martin Fick <mfick@codeaurora.org> writes:
> > This works for me, however unfortunately, I cannot find
> > any scenarios where it improves anything over the
> > previous fix by René. :(
>
> Nevertheless, I would appreciate it if you can try this
> _without_ René's patch. This attempts to make
> resolve_ref() cheap for _any_ caller. René's patch
> avoids calling it in one specific callchain.
>
> They address different issues. René's patch is probably
> an independently good change (I haven't thought about
> the interactions with the topics in flight and its
> implications on the future direction), but would not
> help other/new callers that make many calls to
> resolve_ref().
Agreed. Here is what I am seeing without René's patch.
Checkout in NON packed ref repo takes about 20s, with patch
v3 of binary search, it takes about 11s (1s slower than
René's patch).
Checkout in packed ref repo takes about 5:30min, with patch
v3 of binary search, it takes about 10s (also 1s slower than
René's patch).
I'd say that's not bad, it seems like the 1s difference is
doing the search 60K+times (my tests don't quite scan the
full list), so the search seems to scale well with patch v3.
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* how to produce an index with smudged entries
From: Christian Halstrick @ 2011-09-30 15:33 UTC (permalink / raw)
To: git, Robin Rosenberg
I am trying to find out how native git handles the racy git problem. I
read https://raw.github.com/git/git/master/Documentation/technical/racy-git.txt.
But I cannot reproduce the behaviour described in this text.
I want to create an index which contains entries where the length of
the file was set to 0 to mark the entry as "racily clean". As I
understood the text you have to modify a file and execute 'git
update-index <file>' so fast that in the end the files modification
time is the same as the modification time of the file .git/index. That
should be sufficient that when persisting the index during the 'git
update-index' call git should detect that the files mod-time is
younger than the index mod time and should mark this entry "racily
clean" with a length of 0.
Here is the script with which I try to produce it but it always fails
to create an index with a smudged entry. Any ideas? (Btw: I would
assume that the first entry in the index is smduged when I find 4
zeros beginning at position 48. That's what I deduced from
https://github.com/gitster/git/blob/master/Documentation/technical/index-format.txt)
#!/bin/bash
git init
echo "initial" > foo
# wait for the next tick on the filesystem clock
touch foo
lastTouch=$(stat -c %Y foo)
while [ $lastTouch = $(stat -c %Y foo) ] ;do
touch foo
done
git add foo
# modify 'foo' and update the index
# (hopefully before the filesystem clock has increased)
# repeat this step until the filesystem timer increased and
# print the index everytime
lastTouch=$(stat -c %Y foo)
while [ $lastTouch = $(stat -c %Y foo) ] ;do
echo "modification #$n" > foo
git update-index foo
let "n += 1"
echo "after attempt $n:"
# dump 4 bytes of index beginning from pos 48.
# That should be the length of the first entry
od -j 48 -N 4 -x -A d .git/index
done
^ permalink raw reply
* Bug?: 'git log --find-copies' doesn't match 'git log --follow <rev> -- path/to/file'
From: Alexander Pepper @ 2011-09-30 15:32 UTC (permalink / raw)
To: git
Hello Again.
I'm not really sure, if this is a bug or if I am missing something, but the following is quite annoying:
$ git version
git version 1.7.6.3
$ git clone https://github.com/voldemort/voldemort.git
$ cd voldemort
$ git log --numstat --find-copies dd4e90f9
...
3 15 contrib/ec2-testing/src/java/voldemort/utils/{StopClusterException.java => ClusterOperation.java}
$ git log --numstat dd4e90f9 -- contrib/ec2-testing/src/java/voldemort/utils/ClusterOperation.java
...
23 0 contrib/ec2-testing/src/java/voldemort/utils/ClusterOperation.java
$ git log --numstat --follow dd4e90f9 -- contrib/ec2-testing/src/java/voldemort/utils/ClusterOperation.java
...
6 10 src/java/voldemort/annotations/concurrency/Immutable.java => contrib/ec2-testing/src/java/voldemort/utils/ClusterOperation.java
So git log with copy and rename detection on (--find-copies) tells me, that the file StopClusterException.java is copied to ClusterOperation.java. But If I ask git log for that specific file with --follow git claims a copy from Immutable.java to ClusterOperation.java!
I understand, that git doesn't record renames and copies, but only detects it afterwords. But at least I would expect, that git detects the same thing consistently between to (quite) alike flags.
I also tried adding "--find-copies" and "--find-copies-harder" to 'git log --numstat --follow dd4e90f9', but they also result in claiming Immutable.java as the origin of the copy.
Is there a flag to get consistent results between the two or is this really a bug?
Greetings from Berlin
Alex
^ permalink raw reply
* Re: [PATCH v3] refs: Use binary search to lookup refs faster
From: Michael Haggerty @ 2011-09-30 15:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Julian Phillips, Martin Fick, Christian Couder, git,
Christian Couder, Thomas Rast
In-Reply-To: <7v62karjv3.fsf@alter.siamese.dyndns.org>
On 09/30/2011 01:48 AM, Junio C Hamano wrote:
> This version looks sane, although I have a suspicion that it may have
> some interaction with what Michael may be working on.
Indeed, I have almost equivalent changes in the giant patch series that
I am working on [1]. The branch is very experimental. The tip
currently passes all the tests, but it has a known performance
regression in connection if "git fetch" is used to fetch many commits.
But before comparing ref-related optimizations, we have an *urgent* need
for a decent performance test suite. There are many slightly different
scenarios that have very different performance characteristics, and we
have to be sure that we are optimizing for the whole palette of
many-reference use cases. So I made an attempt at a kludgey but
somewhat flexible performance-testing script [2]. I don't know whether
something like this should be integrated into the git project, and if so
where; suggestions are welcome.
To run the tests, from the root of the git source tree:
make # make sure git is up-to-date
t/make-refperf-repo --help
t/make-refperf-repo [OPTIONS]
t/refperf
cat refperf.times # See the results
The default repo has 5k commits in a linear series with one reference on
each commit. (These numbers can both be adjusted.)
The reference namespace can be laid out a few ways:
* Many references in a single "directory" vs. sharded over many
"directories"
* In lexicographic order by commit, in reverse order, or "shuffled".
By default, the repo is written to "refperf-repo".
The time it takes to create the test repository is itself also an
interesting benchmark. For example, on the maint branch it is terribly
slow unless it is passed either the --pack-refs-interval=N (with N, say
100) or --no-replace-object option. I also noticed that if it is run like
t/make-refperf-repo --refs=5000 --commits=5000 \
--pack-refs-interval=100
(one ref per commit), git-pack-refs becomes precipitously and
dramatically slower after the 2000th commit.
I haven't had time yet for systematic benchmarks of other git versions.
See the refperf script to see what sorts of benchmarks that I have built
into it so far. The refperf test is non-destructive; it always copies
from "refperf-repo" to "refperf-repo-copy" and does its tests in the
copy; therefore a test repo can be reused. The timing data are written
to "refperf.times" and other output to "refperf.log".
Here are my refperf results for the "maint" branch on my notebook with
the default "make-refperf-repo" arguments (times in seconds):
3.36 git branch (cold)
0.01 git branch (warm)
0.04 git for-each-ref
3.08 git checkout (cold)
0.01 git checkout (warm)
0.00 git checkout --orphan (warm)
0.15 git checkout from detached orphan
0.12 git pack-refs
1.17 git branch (cold)
0.00 git branch (warm)
0.17 git for-each-ref
0.95 git checkout (cold)
0.00 git checkout (warm)
0.00 git checkout --orphan (warm)
0.21 git checkout from detached orphan
0.18 git branch -a --contains
7.67 git clone
0.06 git fetch (nothing)
0.01 git pack-refs
0.05 git fetch (nothing, packed)
0.10 git clone of a ref-packed repo
0.63 git fetch (everything)
Probably we should test with even more references than this, but this
test already shows that some commands are quite sluggish.
There are some more things that could be added, like:
* Branches vs. annotated tags
* References on the tips of branches in a more typical "branchy" repository.
* git describe --all
* git log --decorate
* git gc
* git filter-branch
(This has very different performance characteristics because it is a
script that invokes git many times.)
I suggest that we try to do systematic benchmarking of any changes that
we claim are performance optimizations and share before/after results in
the cover letter for the patch series.
Michael
[1] branch hierarchical-refs at git://github.com/mhagger/git.git
[2] branch refperf at git://github.com/mhagger/git.git
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Updated tag 'junio-gpg-pub' ?
From: Junio C Hamano @ 2011-09-30 15:21 UTC (permalink / raw)
To: Stefan Näwe; +Cc: Junio C. Hamano, Git List
In-Reply-To: <4E856676.3050209@atlas-elektronik.com>
Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:
> Junio,
>
> I haven't seen any announcement of this:
>
>> Fetching origin
>> >From http://github.com/gitster/git
>> - [tag update] junio-gpg-pub -> junio-gpg-pub
>
> Did you update your GPG key ?
I removed a user@my-old-isp.com from the list of uids contained within the
public key, and I updated that tag to point at a blob that records a new
export of the result. The "key" itself is not updated (it uses the same
crypto material). People who got my public key from the old tag should
still be able to run "verify-tag" and get the right result.
Thanks for noticing.
^ permalink raw reply
* Re: [PATCH/RFC] add lame win32 credential-helper
From: Erik Faye-Lund @ 2011-09-30 14:42 UTC (permalink / raw)
To: git; +Cc: jaysoffian, peff, gitster
In-Reply-To: <1316118324-6164-1-git-send-email-kusmabite@gmail.com>
On Thu, Sep 15, 2011 at 10:25 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>
> I got curious what a credential-helper that uses Windows'
> Credential Manager would look like; this is the result.
>
> Some parts of the code is heavily inspired by Jay Soffian's
> OSX-keychain work.
>
> Not that it's useful yet, since the core-git code for the
> credential-helper support doesn't compile on Windows. So
> it's not fully tested, I've only read the interface
> documentation and experimented with it from the command
> line.
>
...aaand with the following patch applied on top, it pass the
credential-helper test that peff wrote:
---8<---
Subject: [PATCH] fix helper to pass test
---
.../credential-wincred/git-credential-wincred.c | 87 +++++++++++---------
1 files changed, 47 insertions(+), 40 deletions(-)
diff --git a/contrib/credential-wincred/git-credential-wincred.c
b/contrib/credential-wincred/git-credential-wincred.c
index 072c002..278dd80 100644
--- a/contrib/credential-wincred/git-credential-wincred.c
+++ b/contrib/credential-wincred/git-credential-wincred.c
@@ -84,31 +84,35 @@ static void emit_user_pass(WCHAR *username, WCHAR *password)
wprintf(L"password=%s\n", password);
}
-static int find_credentials(WCHAR *target, WCHAR *username)
+static WCHAR *get_target(WCHAR *unique, WCHAR *username)
{
+ static WCHAR target_buf[4096];
+ _snwprintf(target_buf, sizeof(target_buf), L"gitcred:%s:%s",
+ unique, username);
+ return target_buf;
+}
+
+static int find_credentials(WCHAR *unique, WCHAR *username)
+{
+ int i;
WCHAR user_buf[256], pass_buf[256];
DWORD user_buf_size = sizeof(user_buf) - 1,
pass_buf_size = sizeof(pass_buf) - 1;
CREDENTIALW **creds, *cred = NULL;
DWORD num_creds;
- if (!CredEnumerateW(target, 0, &num_creds, &creds))
+ if (!CredEnumerateW(get_target(unique, username ? username : L"*"),
+ 0, &num_creds, &creds))
return -1;
- if (!username) {
- /* no username was specified, just pick the first one */
- cred = creds[0];
- } else {
- /* search for the first credential that matches username */
- int i;
- for (i = 0; i < num_creds; ++i)
- if (!wcscmp(username, creds[i]->UserName)) {
- cred = creds[i];
- break;
- }
- if (!cred)
- return -1;
- }
+ /* search for the first credential that matches username */
+ for (i = 0; i < num_creds; ++i)
+ if (!username || !wcscmp(username, creds[i]->UserName)) {
+ cred = creds[i];
+ break;
+ }
+ if (!cred)
+ return -1;
if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
@@ -126,9 +130,9 @@ static int find_credentials(WCHAR *target, WCHAR *username)
}
/* also saves the credentials if the user tells it to */
-static int ask_credentials(WCHAR *target, WCHAR *comment, WCHAR *username)
+static int ask_credentials(WCHAR *unique, WCHAR *comment, WCHAR *username)
{
- BOOL save = FALSE;
+ BOOL save = unique != NULL;
LPVOID auth_buf = NULL;
ULONG auth_buf_size = 0;
WCHAR user_buf[256], pass_buf[256];
@@ -140,7 +144,7 @@ static int ask_credentials(WCHAR *target, WCHAR
*comment, WCHAR *username)
ULONG package = 0;
CREDUI_INFOW info = {
sizeof(info), NULL,
- comment ? comment : target, L"Enter password", NULL
+ comment ? comment : unique, L"Enter password", NULL
};
if (username)
@@ -148,7 +152,7 @@ static int ask_credentials(WCHAR *target, WCHAR
*comment, WCHAR *username)
in_buf, &in_buf_size);
err = CredUIPromptForWindowsCredentialsW(&info, 0, &package,
in_buf, in_buf_size, &auth_buf, &auth_buf_size,
- &save, CREDUIWIN_GENERIC | CREDUIWIN_CHECKBOX);
+ &save, CREDUIWIN_GENERIC | (unique ? CREDUIWIN_CHECKBOX : 0));
if (err == ERROR_CANCELLED)
return 0;
if (err != ERROR_SUCCESS)
@@ -169,7 +173,7 @@ static int ask_credentials(WCHAR *target, WCHAR
*comment, WCHAR *username)
CREDENTIALW cred;
cred.Flags = 0;
cred.Type = CRED_TYPE_GENERIC;
- cred.TargetName = target;
+ cred.TargetName = get_target(unique, user_buf);
cred.Comment = comment;
cred.CredentialBlobSize = auth_buf_size;
cred.CredentialBlob = auth_buf;
@@ -184,17 +188,21 @@ static int ask_credentials(WCHAR *target, WCHAR
*comment, WCHAR *username)
return 0;
}
-static void delete_credentials(WCHAR *target, WCHAR *username)
+static void delete_credentials(WCHAR *unique, WCHAR *username)
{
- WCHAR temp[4096];
+ int i;
+ CREDENTIALW **creds;
+ DWORD num_creds;
- wcscpy(temp, target);
- if (username) {
- wcscat(temp, L"|");
- wcscat(temp, username);
- }
- if (!CredDeleteW(target, CRED_TYPE_GENERIC, 0))
- die("failed to delete credentials");
+ if (!CredEnumerateW(get_target(unique, username ? username : L"*"),
+ 0, &num_creds, &creds))
+ return;
+
+ for (i = 0; i < num_creds; ++i)
+ if (!CredDeleteW(get_target(unique, creds[i]->UserName),
+ CRED_TYPE_GENERIC, 0))
+ die("failed to delete credentials");
+ CredFree(creds);
}
int main(int argc, char *argv[])
@@ -232,9 +240,6 @@ int main(int argc, char *argv[])
die("Unrecognized argument `%s'; try --help", arg);
}
- if (!unique)
- die("Must specify --unique=TOKEN; try --help");
-
/* load DLLs */
advapi = LoadLibrary("advapi32.dll");
credui = LoadLibrary("credui.dll");
@@ -256,16 +261,18 @@ int main(int argc, char *argv[])
CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
!CredUIPromptForWindowsCredentialsW || !CredEnumerateW ||
- !CredPackAuthenticationBufferW || !CredFree || !CredDeleteW)
+ !CredFree || !CredPackAuthenticationBufferW || !CredDeleteW)
die("failed to load functions");
- if (reject) {
- delete_credentials(unique, username);
- return 0;
- }
+ if (unique) {
+ if (reject) {
+ delete_credentials(unique, username);
+ return 0;
+ }
- if (!find_credentials(unique, username))
- return 0;
+ if (!find_credentials(unique, username))
+ return 0;
+ }
if (!ask_credentials(unique, description, username))
return 0;
--
1.7.7.rc0.257.g9fefc
---8<---
^ permalink raw reply related
* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Pang Yan Han @ 2011-09-30 13:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7vk48sxn5g.fsf@alter.siamese.dyndns.org>
On Wed, Sep 28, 2011 at 04:28:11PM -0700, Junio C Hamano wrote:
> Pang Yan Han <pangyanhan@gmail.com> writes:
>
> > I am unable to reply this until much later, but are the tests in this patch ok?
> > It's the first time I'm writing test cases for git.
>
> I'll squash in a bit more updates and later publish the result.
> Thanks.
Hi Junio,
Thank you for correcting my many amateur mistakes!
Sorry for asking, but do I need to reroll this with the fixup in
origin/ph/push-to-delete-nothing ? Is the commit message fine especially
in light of the changes in the fixup?
Thanks.
^ permalink raw reply
* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Michael Witten @ 2011-09-30 13:23 UTC (permalink / raw)
To: Peter Shenkin; +Cc: git
In-Reply-To: <loom.20110930T041939-332@post.gmane.org>
On Fri, Sep 30, 2011 at 02:51, Peter Shenkin <shenkin@gmail.com> wrote:
> Perhaps it will be useful to say what would have been most
> helpful for me. In the current documentation for "fetch
> --tags", one sentence reads, "This flag lets all tags and
> their associated objects be downloaded." The following small
> modification would, IMO, be sufficient: "This flag causes all
> tags and their associated objects (only) to be downloaded."
Well, you're missing the fact that it not only causes those to
be downloaded, but it also causes the defaults to be ignored,
which is why you don't get the other stuff. You can still tell
git to fetch anything else you want in addition.
See here:
[PATCH v3] Docs: Clarify the --tags option of `git fetch'
Message-ID: <686c38876d5a4ad6bfac67ca77fe9bb3-mfwitten@gmail.com>
http://article.gmane.org/gmane.comp.version-control.git/181887
^ permalink raw reply
* [RFC/PATCH]: reverse bisect v 2.0
From: Michal Vyskocil @ 2011-09-30 11:42 UTC (permalink / raw)
To: git
In-Reply-To: <20110929142027.GA4936@zelva.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 5213 bytes --]
The bugfix command works like the previous git bisect start --reverse.
It switch the meaning of good(s) and bad from the default regression
search approach to the bugfix one.
I don't like adding more new subcommands into bisect, so I decided to
not add ideas I have found on this mailinglist, like 'git bisect
regression' or 'yes', 'no', 'fixed', 'unfixed' or whatever.
The git bisect start/bugfix good/bad/skip log replay and vizualize were
tested (however on simple linear example).
The missing points:
* git-bisect--helper has the "bad" hardcoded, so the commit fixing a
bug is reffered as a bad one
* in git bisect vizualize, the good commit is shown under refs/bad in
gitk (however that's the same problem if user reverse the usage of
good/bad itself).
* documentation and tests of course
Regards
Michal Vyskocil
---
bisect.c | 2 +-
git-bisect.sh | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/bisect.c b/bisect.c
index c7b7d79..2eb34db 100644
--- a/bisect.c
+++ b/bisect.c
@@ -768,7 +768,7 @@ static void handle_bad_merge_base(void)
fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n"
"git bisect cannot work properly in this case.\n"
- "Maybe you mistake good and bad revs?\n");
+ "Try git bisect bugfix to switch the default bisect logic.\n");
exit(1);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 2524060..6959cf8 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -4,7 +4,9 @@ USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
- reset bisect state and start bisection.
+ reset bisect state and start bisection to find a regression.
+git bisect bugfix [--no-checkout] [<good> [<bad>...]] [--] [<pathspec>...]
+ reset bisect state and start bisection to find a bugfix.
git bisect bad [<rev>]
mark <rev> a known-bad revision.
git bisect good [<rev>...]
@@ -33,6 +35,29 @@ OPTIONS_SPEC=
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+setup_bugfix_mode() {
+ /bin/touch "$GIT_DIR/BISECT_BUGFIX"
+}
+
+bisect_bugfix_mode() {
+ test -f "$GIT_DIR/BISECT_BUGFIX"
+}
+
+bisect_check_state() {
+
+ if bisect_bugfix_mode; then
+ if test "$1" = "good"; then
+ echo "bad"
+ return 0
+ elif test "$1" = "bad"; then
+ echo "good"
+ return 0
+ fi
+ fi
+
+ echo $1
+}
+
bisect_head()
{
if test -f "$GIT_DIR/BISECT_HEAD"
@@ -69,6 +94,15 @@ bisect_start() {
# Check for one bad and then some good revisions.
#
has_double_dash=0
+ #
+ # Exchange the internal meaning of good/bad allowing bisect to find
+ # a commit fixing a bug, not "only" the one causes a regression
+ #
+ cmd="start"
+ if test -n "$1" && test "$1" = "bugfix"; then
+ cmd="bugfix"
+ shift 1
+ fi
for arg; do
case "$arg" in --) has_double_dash=1; break ;; esac
done
@@ -99,10 +133,17 @@ bisect_start() {
die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
break
}
- case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
- esac
+ #if test $cmd = "bisect"; then
+ case $bad_seen in
+ 0) state='bad' ; bad_seen=1 ;;
+ *) state='good' ;;
+ esac
+ #else
+ # case $bad_seen in
+ # 0) state='good' ; bad_seen=1 ;;
+ # *) state='bad' ;;
+ # esac
+ #fi
eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
;;
@@ -169,7 +210,10 @@ bisect_start() {
} &&
git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
eval "$eval true" &&
- echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
+ echo "git bisect $cmd$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
+ if test $cmd = "bugfix"; then
+ setup_bugfix_mode || exit
+ fi
#
# Check if we can proceed to the next bisect state.
#
@@ -225,7 +269,7 @@ bisect_skip() {
bisect_state() {
bisect_autostart
- state=$1
+ state=$(bisect_check_state $1)
case "$#,$state" in
0,*)
die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
@@ -377,6 +421,7 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
+ rm -f "$GIT_DIR/BISECT_BUGFIX" &&
# Cleanup head-name if it got left by an old version of git-bisect
rm -f "$GIT_DIR/head-name" &&
git update-ref -d --no-deref BISECT_HEAD &&
@@ -401,7 +446,12 @@ bisect_replay () {
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
+ bugfix)
+ cmd="bisect_start 'bugfix' $rev"
+ setup_bugfix_mode || exit
+ eval "$cmd" ;;
good|bad|skip)
+ command=$(bisect_check_state $1)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -485,6 +535,8 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
+ bugfix)
+ bisect_start "bugfix" "$@" ;;
bad|good)
bisect_state "$cmd" "$@" ;;
skip)
--
1.7.6.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* [PATCH 2/2] use new Git::config_path() for aliasesfile
From: Cord Seele @ 2011-09-30 10:52 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Junio C Hamano, Eric Wong, Cord Seele
In-Reply-To: <1317379945-9355-1-git-send-email-cowose@gmail.com>
Signed-off-by: Cord Seele <cowose@gmail.com>
---
git-send-email.perl | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..f17f7b3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -225,7 +225,6 @@ my %config_settings = (
"cccmd" => \$cc_cmd,
"aliasfiletype" => \$aliasfiletype,
"bcc" => \@bcclist,
- "aliasesfile" => \@alias_files,
"suppresscc" => \@suppress_cc,
"envelopesender" => \$envelope_sender,
"multiedit" => \$multiedit,
@@ -234,6 +233,10 @@ my %config_settings = (
"assume8bitencoding" => \$auto_8bit_encoding,
);
+my %config_path_settings = (
+ "aliasesfile" => \@alias_files,
+);
+
# Help users prepare for 1.7.0
sub chain_reply_to {
if (defined $chain_reply_to &&
@@ -330,6 +333,11 @@ sub read_config {
$$target = Git::config_bool(@repo, "$prefix.$setting") unless (defined $$target);
}
+ foreach my $setting (keys %config_path_settings) {
+ my $target = $config_path_settings{$setting}->[0];
+ $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ }
+
foreach my $setting (keys %config_settings) {
my $target = $config_settings{$setting};
next if $setting eq "to" and defined $no_to;
--
1.7.6.4
^ permalink raw reply related
* [PATCH 1/2] Add Git::config_path()
From: Cord Seele @ 2011-09-30 10:52 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Junio C Hamano, Eric Wong, Cord Seele
In-Reply-To: <1317379945-9355-1-git-send-email-cowose@gmail.com>
Use --path option when calling 'git config' thus allow for pathname
expansion, e.g. a tilde.
Signed-off-by: Cord Seele <cowose@gmail.com>
---
perl/Git.pm | 32 ++++++++++++++++++++++++++++++++
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index a86ab70..c279bfb 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -627,6 +627,38 @@ sub config_bool {
};
}
+
+=item config_path ( VARIABLE )
+
+Retrieve the path configuration C<VARIABLE>. The return value
+is an expanded path or C<undef> if it's not defined.
+
+This currently wraps command('config') so it is not so fast.
+
+=cut
+
+sub config_path {
+ my ($self, $var) = _maybe_self(@_);
+
+ try {
+ my @cmd = ('config', '--path');
+ unshift @cmd, $self if $self;
+ if (wantarray) {
+ return command(@cmd, '--get-all', $var);
+ } else {
+ return command_oneline(@cmd, '--get', $var);
+ }
+ } catch Git::Error::Command with {
+ my $E = shift;
+ if ($E->value() == 1) {
+ # Key not found.
+ return undef;
+ } else {
+ throw $E;
+ }
+ };
+}
+
=item config_int ( VARIABLE )
Retrieve the integer configuration C<VARIABLE>. The return value
--
1.7.6.4
^ permalink raw reply related
* [PATCH v2] git-send-email: allow filename expansion
From: Cord Seele @ 2011-09-30 10:52 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Junio C Hamano, Eric Wong
In-Reply-To: <vpqty7wk9km.fsf@bauges.imag.fr>
OK, here's another try to get it more Git-like.
Filename expansion now works as far as I can tell. But I failed actually
using my mutt aliases (but also with plain v1.7.6.4). Since I'm not
familiar with perl-debugging could please someone more experienced have a look?
Thanks.
-- Cord
[PATCH 1/2] Add Git::config_path()
[PATCH 2/2] use new Git::config_path() for aliasesfile
^ permalink raw reply
* Re: [PATCH v3 0/4] port upload-archive to Windows
From: Jeff King @ 2011-09-30 10:46 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>
On Thu, Sep 29, 2011 at 10:59:19PM +0200, Erik Faye-Lund wrote:
> It's been a while, but here's another updated version of this
> series.
>
> The only change since last time is that the series has been made
> compatible with Peff's "when remote, disable some features"
> changes.
>
> Erik Faye-Lund (4):
> compat/win32/sys/poll.c: upgrade from upstream
> mingw: fix compilation of poll-emulation
> enter_repo: do not modify input
> upload-archive: use start_command instead of fork
Thanks. I can't comment on the earlier patches, but 4/4 looks sane to
me.
-Peff
^ permalink raw reply
* Re: [PATCH] show git tag output in pager
From: Jeff King @ 2011-09-30 10:42 UTC (permalink / raw)
To: Michal Vyskocil; +Cc: Matthieu Moy, git
In-Reply-To: <20110929093749.GB27152@zelva.suse.cz>
On Thu, Sep 29, 2011 at 11:37:49AM +0200, Michal Vyskocil wrote:
> On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:
> > The commit message should explain why this is needed, and in particular
> > why you prefer this to setting pager.tag in your ~/.gitconfig.
>
> Opps! I read a documentation, but I did not realize this works for all
> commands and not only for them calling setup_pager(). Then sorry, no
> change is needed.
I don't think you want to set pager.tag. It will invoke the pager for
all tag subcommands, including tag creation and deletion. It's not a
huge deal if your pager exits immediately when the input is less than a
page (which I think our default LESS settings will do). But I wouldn't
be surprised if it ends up confusing some program at some point.
I think instead, you want some way for commands to say "OK, I'm in a
subcommand that might or might not want a pager now".
Something like the (thoroughly not tested) patch below, which you can
use like:
git config pager.tag.list
diff --git a/builtin.h b/builtin.h
index 0e9da90..d0fe971 100644
--- a/builtin.h
+++ b/builtin.h
@@ -35,6 +35,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
extern int check_pager_config(const char *cmd);
+extern void try_subcommand_pager(const char *subcommand);
extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..bc45fe6 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -152,6 +152,7 @@ static int list_tags(const char **patterns, int lines,
filter.lines = lines;
filter.with_commit = with_commit;
+ try_subcommand_pager("tag.list");
for_each_tag_ref(show_reference, (void *) &filter);
return 0;
diff --git a/git.c b/git.c
index 8e34903..60fbf1e 100644
--- a/git.c
+++ b/git.c
@@ -64,6 +64,16 @@ static void commit_pager_choice(void) {
}
}
+void try_subcommand_pager(const char *subcommand)
+{
+ /* it's too late to turn off a running pager */
+ if (pager_in_use())
+ return;
+
+ use_pager = check_pager_config(subcommand);
+ commit_pager_choice();
+}
+
static int handle_options(const char ***argv, int *argc, int *envchanged)
{
const char **orig_argv = *argv;
Other programs like "git branch list" would probably want similar hooks.
For commands like "tag" and "branch" where there's really only one
operation that you would want to be paged (i.e., "list"), it's tempting
to also (or instead) make "pager.tag" only affect the useful command.
That's not too hard for internal commands like "tag" (patch below). But
I'm not sure how you would do it for something external like "stash";
from git.c's perspective, we don't know anything about stash or whether
it would want to respect pager config or not.
diff --git a/builtin/tag.c b/builtin/tag.c
index bc45fe6..bbdb135 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -152,7 +152,7 @@ static int list_tags(const char **patterns, int lines,
filter.lines = lines;
filter.with_commit = with_commit;
- try_subcommand_pager("tag.list");
+ try_subcommand_pager("tag");
for_each_tag_ref(show_reference, (void *) &filter);
return 0;
diff --git a/git.c b/git.c
index 60fbf1e..64a078d 100644
--- a/git.c
+++ b/git.c
@@ -276,6 +276,7 @@ static int handle_alias(int *argcp, const char ***argv)
* RUN_SETUP for reading from the configuration file.
*/
#define NEED_WORK_TREE (1<<3)
+#define NO_PAGER_CONFIG (1<<4)
struct cmd_struct {
const char *cmd;
@@ -299,7 +300,9 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
prefix = setup_git_directory_gently(&nongit_ok);
}
- if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY))
+ if (use_pager == -1 &&
+ p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&
+ !(p->option & NO_PAGER_CONFIG))
use_pager = check_pager_config(p->cmd);
if (use_pager == -1 && p->option & USE_PAGER)
use_pager = 1;
@@ -436,7 +439,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
- { "tag", cmd_tag, RUN_SETUP },
+ { "tag", cmd_tag, RUN_SETUP | NO_PAGER_CONFIG },
{ "tar-tree", cmd_tar_tree },
{ "unpack-file", cmd_unpack_file, RUN_SETUP },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-Peff
^ permalink raw reply related
* Re: [PATCH v2] Add a credential-helper for KDE
From: Jeff King @ 2011-09-30 10:21 UTC (permalink / raw)
To: Lukas Sandström; +Cc: Git Mailing List
In-Reply-To: <4E7605CA.7020204@gmail.com>
On Sun, Sep 18, 2011 at 04:52:58PM +0200, Lukas Sandström wrote:
> This Python script plugs into the credentials API
> of Git to ask the user for passwords with a nice
> KDE password dialog.
>
> The password is saved in the KWallet.
So I managed to play with this a bit tonight. Overall, it seems pretty
nice.
Initially, it seemed somewhat clumsy. It asked me to open the wallet
(using a password) each time git ran. Which is about as annoying as just
typing my git password each time. :)
The magic trick was to configure kwallet to "keep the wallet open for 10
minutes after the last use" instead of "close when no applications have
the wallet open". Since git runs as many small programs, kwallet has no
real idea of how long a git session is.
This is totally not a kwallet thing, and nothing to do with your helper.
But since the helper is so annoyingly useless without that config, it
might be worth mentioning it in a README.
> Right. Multiple usernames per "unique" context is supported in this version.
> I looked at the git-credential-storage helper when I wrote the first patch,
> which didn't have obvious support for multiple usernames per unique context.
This part passed my tests just fine. Very nice.
> +class CredentialHelper(KApplication):
> + def __init__(self, token, username = None, desc = None, reject = False):
> + super(CredentialHelper, self).__init__()
> + self.password = None
> + self.username = username
> + self.save_password = False
> + self.token = token
> + self.desc = desc
> +
> + if not self.token:
> + return
My tests complained about doing nothing when there is no token. As I've
mentioned elsewhere, this doesn't matter now (as git never invokes the
helper that way), but it would be nice to future-proof the helper by
just ignoring the wallet, but still doing the nice password dialog.
> + def open_wallet(self):
> + self.wallet = KWallet.Wallet.openWallet(
> + KWallet.Wallet.LocalWallet(), 0, KWallet.Wallet.Synchronous)
> + if not self.wallet.isOpen():
> + return None
> + if not self.wallet.hasFolder("GitCredentials"):
> + self.wallet.createFolder("GitCredentials")
> + self.wallet.setFolder("GitCredentials")
I peeked around the KWallet manager. There's a "passwords" folder in the
wallet, and I was surprised that the passwords didn't go there. But when
I tried using konqueror to store a password, I found that it also made
its own folder, and then stored a map within it for each URL.
So I'm not really sure if you're following kwallet best practices or
not, as I'm clearly confused about what the "passwords" folder is for.
;)
> + def check_wallet(self):
> + (res, data) = self.wallet.readMap(self.token)
So you're just using the token as a big blob. Which is how I
anticipated, but is the complete opposite of what OS X Keychain wants.
Which is leading me to think we should really just hand helpers both
forms: the information broken down by item (e.g., --host=github.com),
and a full URL (e.g., --url=https://github.com/). And then the helpers
can use whatever they like (where you would use "url" instead of the
current "unique").
-Peff
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: René Scharfe @ 2011-09-30 9:12 UTC (permalink / raw)
To: Martin Fick
Cc: Julian Phillips, Christian Couder, git, Christian Couder,
Thomas Rast, Junio C Hamano
In-Reply-To: <201109291411.06733.mfick@codeaurora.org>
Hi Martin,
Am 29.09.2011 22:11, schrieb Martin Fick:
> Your patch works well for me. It achieves about the same
> gains as Julian's patch. Thanks!
OK, and what happens if you apply the following patch on top of my first
one? It avoids going through all the refs a second time during cleanup,
at the cost of going through the list of all known objects. I wonder if
that's any faster in your case.
Thanks,
René
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 84e0cdc..a4b1003 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -596,15 +596,14 @@ static int add_pending_uninteresting_ref(const char *refname,
return 0;
}
-static int clear_commit_marks_from_one_ref(const char *refname,
- const unsigned char *sha1,
- int flags,
- void *cb_data)
+static void clear_commit_marks_for_all(unsigned int mark)
{
- struct commit *commit = lookup_commit_reference_gently(sha1, 1);
- if (commit)
- clear_commit_marks(commit, -1);
- return 0;
+ unsigned int i, max = get_max_object_index();
+ for (i = 0; i < max; i++) {
+ struct object *object = get_indexed_object(i);
+ if (object && object->type == OBJ_COMMIT)
+ object->flags &= ~mark;
+ }
}
static void describe_one_orphan(struct strbuf *sb, struct commit *commit)
@@ -690,8 +689,7 @@ static void orphaned_commit_warning(struct commit *commit)
else
describe_detached_head(_("Previous HEAD position was"), commit);
- clear_commit_marks(commit, -1);
- for_each_ref(clear_commit_marks_from_one_ref, NULL);
+ clear_commit_marks_for_all(ALL_REV_FLAGS);
}
static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
^ 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