* Re: [msysGit] [PATCH] git grep: be careful to use mutices only when they are initialized
From: Pat Thoyts @ 2011-10-26 9:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: msysgit, gitster, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>
On 25 October 2011 18:25, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Rather nasty things happen when a mutex is not initialized but locked
> nevertheless. Now, when we're not running in a threaded manner, the mutex
> is not initialized, which is correct. But then we went and used the mutex
> anyway, which -- at least on Windows -- leads to a hard crash (ordinarily
> it would be called a segmentation fault, but in Windows speak it is an
> access violation).
>
> This problem was identified by our faithful tests when run in the msysGit
> environment.
I did not see this failure when running the tests on my machine. But
then threaded issues are often intermittent depending on load, number
of cores, phase of the moon, etc. You never said _which_ test either
although there are only 3 to try - most likey t7810-grep.sh
I was going to point out that it should be "mutexes" but I see it is
committed already :)
> To avoid having to wrap the line due to the 80 column limit, we use
So last century!
> the name "WHEN_THREADED" instead of "IF_USE_THREADS" because it is one
> character shorter. Which is all we need in this case.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> I looked around a bit but ran out of time to identify the reason why
> this was not caught earlier.
>
> builtin/grep.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 92eeada..e94c5fe 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -78,10 +78,11 @@ static pthread_mutex_t grep_mutex;
> /* Used to serialize calls to read_sha1_file. */
> static pthread_mutex_t read_sha1_mutex;
>
> -#define grep_lock() pthread_mutex_lock(&grep_mutex)
> -#define grep_unlock() pthread_mutex_unlock(&grep_mutex)
> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
> +#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0)
> +#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex))
> +#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex))
> +#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex))
> +#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex))
>
> /* Signalled when a new work_item is added to todo. */
> static pthread_cond_t cond_add;
> --
> 1.7.5.3.4540.g15f89
Works for me.
Pat.
^ permalink raw reply
* Out of memory error with git rebase
From: Hannu Koivisto @ 2011-10-26 9:21 UTC (permalink / raw)
To: git
Hi,
If 'git rebase origin/master' dies with an out of memory error
(probably due to a few of large binary files in the repository, the
largest being ~300MB and ~1GB in total in one directory), which
settings should be tweaked and how to get rid of the problem? I
tried...
[pack]
threads = 1
windowMemory = 64M
packSizeLimit = 512M
...based on some suggestions in the net but that was of no help.
I'm using git 1.7.5.1 in Cygwin and I also tried the latest master
branch version (which behaved identically).
--
Hannu
^ permalink raw reply
* Is there a place for benchmarking scripts?
From: Michael Haggerty @ 2011-10-26 9:50 UTC (permalink / raw)
To: git
I've been doing a lot of benchmarking of git performance in the presence
of lots of references. I've written a few scripts to automate the
benchmarking [1]. They are not beautiful and would require a couple of
local adjustments [2,3]. They are too time-consuming to be made part of
the usual test suite. I wouldn't want to commit to maintaining them.
But they have certainly been useful to me, and they generate readable
output [4].
My question is: would such benchmarking scripts be welcome within the
git project? If so, where should I put it? Is any benchmarking
code/framework already in use?
Michael
[1] Branch "refperf" at git://github.com/mhagger/git.git
[2] For example, one script checks out specified git revisions, merges
in the "refperf" branch to get the benchmarking code, then runs tests.
This script has to be adjusted with the local name of the "refperf"
branch (e.g., "refperf" vs. "origin/refperf" vs. ...)
[3] I wanted the tests to include cases with a cold disk cache and with
a warm disk cache. For the former, I have the script run "sync; sudo sh
-c 'echo 3 >/proc/sys/vm/drop_caches'" which obviously only works on
Linux and only when the user has password-less sudo permissions.
[4] Example (numbers are times in seconds):
=================================== ======== ======== ========
Test name [0] [1] [2]
=================================== ======== ======== ========
branch-loose-cold 3.32 3.25 0.55
branch-loose-warm 0.60 0.22 0.00
for-each-ref-loose-cold 3.71 3.46 3.45
for-each-ref-loose-warm 0.83 0.46 0.47
checkout-loose-cold 3.82 3.23 0.63
checkout-loose-warm 0.58 0.21 0.01
checkout-orphan-loose 0.58 0.20 0.00
checkout-from-detached-loose-cold 4.54 4.20 3.75
checkout-from-detached-loose-warm 1.41 1.07 0.63
branch-contains-loose-cold 4.04 3.71 3.67
branch-contains-loose-warm 0.93 0.57 0.56
pack-refs-loose 2.31 1.92 1.91
branch-packed-cold 0.53 0.50 0.49
branch-packed-warm 0.02 0.02 0.03
for-each-ref-packed-cold 1.01 0.92 0.94
for-each-ref-packed-warm 0.26 0.27 0.28
checkout-packed-cold 14.37 1.51 1.27
checkout-packed-warm 0.04 0.03 0.03
checkout-orphan-packed 0.02 0.02 0.03
checkout-from-detached-packed-cold 14.54 1.51 1.12
checkout-from-detached-packed-warm 13.85 0.82 0.46
branch-contains-packed-cold 1.03 1.04 1.01
branch-contains-packed-warm 0.37 0.38 0.39
clone-loose-cold 13.17 11.97 12.45
clone-loose-warm 6.86 5.40 5.83
fetch-nothing-loose 1.24 1.11 1.52
pack-refs 0.27 0.27 0.29
fetch-nothing-packed 1.23 1.11 1.52
clone-packed-cold 1.89 1.84 1.80
clone-packed-warm 0.62 0.63 0.70
fetch-everything-cold 12.85 13.07 13.40
fetch-everything-warm 6.53 6.92 6.88
filter-branch-warm 35383.91 15869.38 748.01
=================================== ======== ======== ========
[0] 703f05a (tag: v1.7.7) Git 1.7.7
Test repository created using: t/make-refperf-repo --commits 20000
--refs 10000 --shard
[1] 2c5c66b Merge branch 'jp/get-ref-dir-unsorted'
Test repository created using: t/make-refperf-repo --commits 20000
--refs 10000 --shard
[2] f579019 (ref-api-D) sort_ref_dir(): remove the recurse argument
Test repository created using: t/make-refperf-repo --commits 20000
--refs 10000 --shard
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] replace sha1 with another algorithm
From: Michael J Gruber @ 2011-10-26 9:59 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111026001237.GA22195@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 26.10.2011 02:12:
> SHA-1 is due to be cryptographically broken sometime in the
> next decade, with collision attacks becoming possible. But
> we don't have to wait! We can act now and replace it,
> treating us to all of the pain of a flag day without any
> delay!
>
> We could of course use the SHA-2 family, or wait for the
> upcoming SHA-3. But any good cryptographer knows that you
> should _never_ use a standard algorithm. It's always better
> to roll your own. After all, if _you_ can't break it, how
> could anyone else?
>
> Signed-off-by: Jeff King <peff@peff.net>
> Reviewed-by: Brandon Casey <drafnel@gmail.com>
> Mocked-by: Rick Balocca <richard.balocca@ericsson.com>
> Enjoyed-by: Elijah Newren <newren@gmail.com>
Awaited-by: Michael J Gruber <git@drmicha.warpmail.net>
Still remembering an earlier GitTogether's l33t l10n....
> ---
> block-sha1/sha1.h | 2 +-
> cache.h | 4 +++-
> sha1_file.c | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> index b864df6..49331e3 100644
> --- a/block-sha1/sha1.h
> +++ b/block-sha1/sha1.h
> @@ -19,4 +19,4 @@
> #define git_SHA_CTX blk_SHA_CTX
> #define git_SHA1_Init blk_SHA1_Init
> #define git_SHA1_Update blk_SHA1_Update
> -#define git_SHA1_Final blk_SHA1_Final
> +#define real_git_SHA1_Final blk_SHA1_Final
> diff --git a/cache.h b/cache.h
> index 2e6ad36..068062b 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -13,9 +13,11 @@
> #define git_SHA_CTX SHA_CTX
> #define git_SHA1_Init SHA1_Init
> #define git_SHA1_Update SHA1_Update
> -#define git_SHA1_Final SHA1_Final
> +#define real_git_SHA1_Final SHA1_Final
> #endif
>
> +void git_SHA1_Final(unsigned char out[20], git_SHA_CTX *ctx);
> +
> #include <zlib.h>
> typedef struct git_zstream {
> z_stream z;
> diff --git a/sha1_file.c b/sha1_file.c
> index 27f3b9b..23e0107 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2833,3 +2833,35 @@ void assert_sha1_type(const unsigned char *sha1, enum object_type expect)
> die("%s is not a valid '%s' object", sha1_to_hex(sha1),
> typename(expect));
> }
> +
> +static void xor_bytes(unsigned char *out, unsigned char *a, unsigned char *b,
> + unsigned n)
> +{
> + unsigned i;
> + for (i = 0; i < n; i++)
> + out[i] = a[i] ^ b[i];
> +}
> +
> +static void mix_hash(unsigned char *h, unsigned n)
> +{
> + unsigned char out[20];
unsigned char out[n];
;)
> + unsigned mid = n / 2;
> +
> + if (2*mid < n)
> + return;
> +
> + xor_bytes(out, h, h + mid, mid);
> + xor_bytes(out + mid, h + mid, h, mid);
> + memcpy(h, out, n);
> +
> + /* If a little bit of mixing is good, then a lot must be GREAT! */
> + mix_hash(h, mid);
> + mix_hash(h + mid, mid);
n a power of 2 anyone...
> +}
> +
> +void git_SHA1_Final(unsigned char out[20], git_SHA_CTX *ctx)
> +{
> + /* We build on top of the regular SHA1, but then "enhance" it. */
> + real_git_SHA1_Final(out, ctx);
> + mix_hash(out, 20);
> +}
>--
>1.7.7.troll
;)
^ permalink raw reply
* Re: Out of memory error with git rebase
From: Johannes Sixt @ 2011-10-26 10:55 UTC (permalink / raw)
To: Hannu Koivisto; +Cc: git
In-Reply-To: <83vcrc9kh7.fsf@kalahari.s2.org>
Am 26.10.2011 11:21, schrieb Hannu Koivisto:
> If 'git rebase origin/master' dies with an out of memory error
> (probably due to a few of large binary files in the repository, the
> largest being ~300MB and ~1GB in total in one directory), which
> settings should be tweaked and how to get rid of the problem? I
> tried...
Try 'git rebase -m origin/master'. Without -m, rebase uses
format-patch+am, i.e., assuming there are changes to the binary files
that are to be rebased, a binary patch file would have to be generated
and applied later. This is very likely where git bails out.
-- Hannes
^ permalink raw reply
* git merge successfully however this is still issue from logical perspective
From: Lynn Lin @ 2011-10-26 12:08 UTC (permalink / raw)
To: git
Hi all,
Do this case can happen? When I do merge from one branch to master
branch,merge successfully however from code logical perspective it
fails and It cause code compile (our project is using C++ language)
Thanks
Lynn
^ permalink raw reply
* Re: git merge successfully however this is still issue from logical perspective
From: Thomas Rast @ 2011-10-26 12:27 UTC (permalink / raw)
To: Lynn Lin; +Cc: git
In-Reply-To: <CAPgpnMSSOss=YxsMUZJ3E5TynDfHJG1i6PKitLBo_Tm7f=_+fQ@mail.gmail.com>
Lynn Lin wrote:
> Hi all,
> Do this case can happen? When I do merge from one branch to master
> branch,merge successfully however from code logical perspective it
> fails and It cause code compile (our project is using C++ language)
Sure. The easiest example is when one side of a merge renames foo()
to bar(), while the other side introduces another call to foo() in an
unrelated part of the code.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: git merge successfully however this is still issue from logical perspective
From: Lynn Lin @ 2011-10-26 12:29 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <201110261427.03585.trast@student.ethz.ch>
On Wed, Oct 26, 2011 at 8:27 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Lynn Lin wrote:
>> Hi all,
>> Do this case can happen? When I do merge from one branch to master
>> branch,merge successfully however from code logical perspective it
>> fails and It cause code compile (our project is using C++ language)
>
> Sure. The easiest example is when one side of a merge renames foo()
> to bar(), while the other side introduces another call to foo() in an
> unrelated part of the code.
Does this mean we shouldn't trust merge result ?
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
>
^ permalink raw reply
* Re: git merge successfully however this is still issue from logical perspective
From: Thomas Rast @ 2011-10-26 12:39 UTC (permalink / raw)
To: Lynn Lin; +Cc: git
In-Reply-To: <CAPgpnMR6_pRxMSLcdS=M4Cfj=dqk6KTXr3VGhk3LrnPHyv2waA@mail.gmail.com>
Lynn Lin wrote:
> On Wed, Oct 26, 2011 at 8:27 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> > Lynn Lin wrote:
> >> Hi all,
> >> Do this case can happen? When I do merge from one branch to master
> >> branch,merge successfully however from code logical perspective it
> >> fails and It cause code compile (our project is using C++ language)
> >
> > Sure. The easiest example is when one side of a merge renames foo()
> > to bar(), while the other side introduces another call to foo() in an
> > unrelated part of the code.
> Does this mean we shouldn't trust merge result ?
Depends what you mean by "trust".
If you expected a merge to understand what the changes on each side
were about, and then patch the code accordingly, no it can't do
that.[*] You are in no way guaranteed that the result of a merge is
correct or even compiles even if both sides of the merge were.
But after all git-merge is just a tool doing a well-defined operation:
textually merging the changes from both sides. AFAIK it could so far
always be trusted with doing *that* correctly.
[*] darcs can go half of the way by having more expressive
"changesets", but this can also fail. I think the general problem is
AI-complete.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Erik Faye-Lund @ 2011-10-26 13:08 UTC (permalink / raw)
To: msysgit; +Cc: git, johannes.schindelin, j.sixt
In-Reply-To: <16520370.401.1319598319120.JavaMail.geo-discussion-forums@prms22>
On Wed, Oct 26, 2011 at 5:05 AM, Atsushi Nakagawa <atnak@chejz.com> wrote:
> On Oct 25, 11:55 pm, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> [...]
>> +int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t
>> *attr)
>> +{
>> + InitializeCriticalSection(&mutex->cs);
>> + mutex->autoinit = 0;
>> + return 0;
>> +}
>> +
>> +int pthread_mutex_lock(pthread_mutex_t *mutex)
>> +{
>> + if (mutex->autoinit) {
>> + if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) !=
>> -1) {
> I'm making the assumption that mutex->autoinit starts off as 1 before things
> get multi-threaded..
> I've only looked at what's in the patch so I could be missing vital
> context.. Anyways, is there a reason why you made this
> "InterlockedCompareExchange(..., -1, 1) != -1" and not
> "InterlockedCompareExchange(..., -1, 1) == 1"?
No, not really.
> It looks to me the former adds a race condition after "if (mutex->autoinit)
> {". e.g. A second thread could reinitialize mutex->cs after the first
> thread has already entered EnterCriticalSection(...).
You are indeed correct, thanks for spotting :)
^ permalink raw reply
* Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Erik Faye-Lund @ 2011-10-26 13:16 UTC (permalink / raw)
To: Kyle Moffett; +Cc: Johannes Sixt, msysgit, git, johannes.schindelin
In-Reply-To: <CAGZ=bqJ7k5h_-62M3y-Jype4a7mOTe+FxeU13JreGj0mOnSRSg@mail.gmail.com>
On Wed, Oct 26, 2011 at 5:44 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
> On Tue, Oct 25, 2011 at 16:51, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Tue, Oct 25, 2011 at 10:07 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>> Am 25.10.2011 17:42, schrieb Erik Faye-Lund:
>>>> On Tue, Oct 25, 2011 at 5:28 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>>> Am 10/25/2011 16:55, schrieb Erik Faye-Lund:
>>>>>> +int pthread_mutex_lock(pthread_mutex_t *mutex)
>>>>>> +{
>>>>>> + if (mutex->autoinit) {
>>>>>> + if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) != -1) {
>>>>>> + pthread_mutex_init(mutex, NULL);
>>>>>> + mutex->autoinit = 0;
>>>>>> + } else
>>>>>> + while (mutex->autoinit != 0)
>>>>>> + ; /* wait for other thread */
>>>>>> + }
>>>>>
>>>>> The double-checked locking idiom. Very suspicious. Can you explain why it
>>>>> works in this case? Why are no Interlocked functions needed for the other
>>>>> accesses of autoinit? ("It is volatile" is the wrong answer to this last
>>>>> question, BTW.)
>>>>
>>>> I agree that it should look a bit suspicious; I'm generally skeptical
>>>> whenever I see 'volatile' in threading-code myself. But I think it's
>>>> the right answer in this case. "volatile" means that the compiler
>>>> cannot optimize away accesses, which is sufficient in this case.
>>>
>>> No, it is not, and it took me a train ride to see what's wrong. It has
>>> nothing to do with autoinit, but with all the other memory locations
>>> that are written. See here, with pthread_mutex_init() inlined:
>>>
>>> if (mutex->autoinit) {
>>>
>>> Assume two threads enter this block.
>>>
>>> if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) != -1) {
>>>
>>> Only one thread, A, say on CPU A, will enter this block.
>>>
>>> InitializeCriticalSection(&mutex->cs);
>>>
>>> Thread A writes some values. Note that there are no memory barriers
>>> involved here. Not that I know of or that they would be documented.
>>>
>>> mutex->autoinit = 0;
>>>
>>> And it writes another one. Thread A continues below to contend for the
>>> mutex it just initialized.
>>>
>>> } else
>>>
>>> Meanwhile, thread B, say on CPU B, spins in this loop:
>>>
>>> while (mutex->autoinit != 0)
>>> ; /* wait for other thread */
>>>
>>> When thread B arrives here, it sees the value of autoinit that thread A
>>> has written above.
>>>
>>> HOWEVER, when it continues, there is NO [*] guarantee that it will also
>>> see the values that InitializeCriticalSection() has written, because
>>> there were no memory barriers involved. When it continues, there is a
>>> chance that it calls EnterCriticalSection() with uninitialized values!
>>>
>>
>> Thanks for pointing this out, I completely forgot about write re-ordering.
>>
>> This is indeed a problem. So, shouldn't replacing "mutex->autoinit =
>> 0;" with "InterlockedExchange(&mutex->autoinit, 0)" solve the problem?
>> InterlockedExchange generates a full memory barrier:
>> http://msdn.microsoft.com/en-us/library/windows/desktop/ms683590(v=vs.85).aspx
>
> No, I'm afraid that won't solve the issue (at least in GCC, not sure about MSVC)
>
> A write barrier in one thread is only effective if it is paired with a
> read barrier in the other thread.
>
> Since there's no read barrier in the "while(mutex->autoinit != 0)",
> you don't have any guaranteed ordering.
>
> I guess if MSVC assumes that volatile reads imply barriers then it might work...
OK, so I should probably do something like this instead?
while (InterlockedCompareExchange(&mutex->autoinit, 0, 0) != 0)
; /* wait for other thread */
I really appreciate getting some extra eyes on this, thanks.
Concurrent programming is not my strong-suit (as this exercise has
shown) ;)
^ permalink raw reply
* Re: git-fixup-assigner.perl -- automatically decide where to "fixup!"
From: fREW Schmidt @ 2011-10-26 14:37 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <201012140309.59378.trast@student.ethz.ch>
On Mon, Dec 13, 2010 at 8:09 PM, Thomas Rast <trast@student.ethz.ch> wrote:
>
> While cleaning up the 'log -L' series I gathered a large number of
> little fixups, and decided it would be smart if git could
> automatically figure out where to put them.
>
> It works like this:
>
> * Split the diff by hunk. I'm using -U1 here for finer splits, but it
> could be tunable.
>
> * For each hunk, run blame to find out which commit's lines were
> affected.
>
> * Group the hunks by this commit, and output them with a suitable
> command to make a fixup.
>
> My git-fixup is
>
> $ g config alias.fixup
> !sh -c 'r=$1; git commit -m"fixup! $(git log -1 --pretty=%s $r)"' -
>
> so that is "suitable".
>
> You would run it with the changes unstaged in your tree as
>
> ./git-fixup-assigner.perl > fixups
>
> and can then review with 'less fixups', or run 'sh fixups' to commit
> them.
>
> It's certainly not perfect, notably the detection logic should ignore
> context, but it got the job done.
>
> --- 8< ---
> #!/usr/bin/perl
>
> use warnings;
> use strict;
>
> sub parse_hunk_header {
> my ($line) = @_;
> my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
> $line =~ /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/;
> $o_cnt = 1 unless defined $o_cnt;
> $n_cnt = 1 unless defined $n_cnt;
> return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
> }
>
> sub find_commit {
> my ($file, $begin, $end) = @_;
> my $blame;
> open($blame, '-|', 'git', '--no-pager', 'blame', 'HEAD', "-L$begin,$end", $file) or die;
> my %candidate;
> while (<$blame>) {
> $candidate{$1} += 1 if /^([0-9a-f]+)/;
> }
> close $blame or die;
> my @sorted = sort { $candidate{$b} <=> $candidate{$a} } keys %candidate;
> if (1 < scalar @sorted) {
> print STDERR "ambiguous split $file:$begin..$end\n";
> foreach my $c (@sorted) {
> print STDERR "\t$candidate{$c}\t$c\n";
> }
> }
> return $sorted[0];
> }
>
> my $diff;
> open($diff, '-|', 'git', '--no-pager', 'diff', '-U1') or die;
>
> my %by_commit;
> my @cur_hunk = ();
> my $cur_commit;
> my ($filename, $prefilename, $postfilename);
>
> while (<$diff>) {
> if (m{^diff --git ./(.*) ./\1$}) {
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
> $filename = $1;
> $prefilename = "./" . $1;
> $postfilename = "./" . $1;
> } elsif (m{^index}) {
> # ignore
> } elsif (m{^new file}) {
> $prefilename = '/dev/null';
> } elsif (m{^delete file}) {
> $postfilename = '/dev/null';
> } elsif (m{^--- $prefilename$}) {
> } elsif (m{^\+\+\+ $postfilename$}) {
> } elsif (m{^@@ }) {
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
> push @cur_hunk, $_;
> die "I don't handle this diff" if ($prefilename ne $postfilename);
> my ($o_ofs, $o_cnt, $n_ofs, $n_cnt)
> = parse_hunk_header($_);
> my $o_end = $o_ofs + $o_cnt - 1;
> $cur_commit = find_commit($filename, $o_ofs, $o_end);
> } elsif (m{^[-+ \\]}) {
> push @cur_hunk, $_;
> } else {
> die "unhandled diff line: '$_'";
> }
> }
>
> close $diff or die;
>
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
>
> print "#!/bin/sh\n\n";
>
> foreach my $commit (keys %by_commit) {
> print "git apply --cached <<EOF\n";
> foreach my $filename (keys %{$by_commit{$commit}}) {
> print "diff --git a/$filename b/$filename\n";
> print "--- a/$filename\n";
> print "+++ b/$filename\n";
> print @{$by_commit{$commit}{$filename}};
> }
> print "EOF\n\n";
> print "git fixup $commit\n\n";
> }
> --- >8 ---
This is super neat, but I'm having trouble getting it to work.
First, I made one small change:
- print "git fixup $commit\n\n";
+ print "git commit --fixup $commit\n\n";
To try it out I made a very simple change:
$ git diff
diff --git a/App/lib/MyApp/Controller/DashboardTemplates.pm
b/App/lib/MyApp/Controller/DashboardTemplates.pm
index aefdc3c..a53b534 100644
--- a/App/lib/MyApp/Controller/DashboardTemplates.pm
+++ b/App/lib/MyApp/Controller/DashboardTemplates.pm
@@ -13,7 +13,7 @@ cat_has $_ => ( is => 'rw' ) for qw(set);
sub base : Chained('/') PathPart('dashboard_templates') CaptureArgs(0) {
my ($self, $c) = @_;
- $self->set($c,
$c->model('DB')->schema->kiokudb_handle->lookup('dashboard
templates'));
+ $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
}
my $renderer = sub {
Then I tried it out
$ git fixup-assigner.pl > fixups && less fixups
#!/bin/sh
git apply --cached <<EOF
diff --git a/App/lib/MyApp/Controller/DashboardTemplates.pm
b/App/lib/MyApp/Controller/DashboardTemplates.pm
--- a/App/lib/MyApp/Controller/DashboardTemplates.pm
+++ b/App/lib/MyApp/Controller/DashboardTemplates.pm
@@ -15,3 +15,3 @@ sub base : Chained('/')
PathPart('dashboard_templates') CaptureArgs(0) {
my ($self, $c) = @_;
- $self->set($c,
$c->model('DB')->schema->kiokudb_handle->lookup('dashboard
templates'));
+ $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
}
EOF
git commit --fixup 7765cbd2
Looks fine to me. But then I try to use it:
$ git checkout . && sh fixups
error: patch failed: App/lib/MyApp/Controller/DashboardTemplates.pm:15
error: App/lib/MyApp/Controller/DashboardTemplates.pm: patch does not apply
Any ideas what I'm doing wrong?
--
fREW Schmidt
http://blog.afoolishmanifesto.com
^ permalink raw reply
* FYI: status of svn-fe speed
From: David Michael Barr @ 2011-10-26 15:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Dmitry Ivankov
Hi,
I talked a lot about low-level speed optimisation after the high-level
optimisation is finished this week. I merged my svn-fe-pu branch with
jch/next and tested with a 1000 rev dump of patches from git-core.
When not blocked, svn-fe can process such revs at ~5000 rev/s on my
laptop. This is far above what both svnadmin dump and git fast-import
presently achieve and might be deemed excessive. However, this
translates to low latency which is critical for parallelisation.
I had to use a 10 microsecond sampling interval to get an accurate
profile. These numbers have not been normalised but are the complete
set of symbols sampled.
2.2% parse_date_basic
1.5% match_string
1.1% svndump_read
1.1% 0x10000af70 [20.2KB]
0.4% strbuf_grow
0.4% strbuf_fread
0.4% strbuf_add
0.4% reset_rev_ctx
0.4% next_quote_pos
0.4% is_date
0.4% fast_export_data
0.4% end_revision
0.4% buffer_read_line
0.4% buffer_copy_bytes
I still think it ought to be a little faster. ;)
--
David Barr
^ permalink raw reply
* Re: [msysGit] [PATCH] git grep: be careful to use mutices only when they are initialized
From: Johannes Schindelin @ 2011-10-26 15:42 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: msysgit, gitster, git
In-Reply-To: <CALUzUxpVWHL8LyqYkYazxSxDr6i=kitACFfVRQsTxQHHYjiOyA@mail.gmail.com>
Hi Tay,
On Wed, 26 Oct 2011, Tay Ray Chuan wrote:
> On Wed, Oct 26, 2011 at 1:25 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Rather nasty things happen when a mutex is not initialized but locked
> > nevertheless. Now, when we're not running in a threaded manner, the
> > mutex is not initialized, which is correct. But then we went and used
> > the mutex anyway, which -- at least on Windows -- leads to a hard
> > crash (ordinarily it would be called a segmentation fault, but in
> > Windows speak it is an access violation).
> >
> > This problem was identified by our faithful tests when run in the
> > msysGit environment.
>
> May I ask which test are you talking about specifically?
It is t7810.
> I ask as I'm curious how this is triggered; git-grep works fine for me
> so far (1.7.6.msysgit.0.584.g2cbf)
That did not expose the error. The problem is exposed in msysGit's 'devel'
branch, though.
Ciao,
Johannes
^ permalink raw reply
* Re: FYI: status of svn-fe speed
From: Tay Ray Chuan @ 2011-10-26 16:18 UTC (permalink / raw)
To: David Michael Barr; +Cc: git, Jonathan Nieder, Dmitry Ivankov
In-Reply-To: <CAFfmPPNK+D=g5h7bdYmON++HE5jF_opYxKLobqjOosj--8+9FQ@mail.gmail.com>
On Wed, Oct 26, 2011 at 11:18 PM, David Michael Barr
<davidbarr@google.com> wrote:
> I talked a lot about low-level speed optimisation after the high-level
> optimisation is finished this week. I merged my svn-fe-pu branch with
> jch/next and tested with a 1000 rev dump of patches from git-core.
> When not blocked, svn-fe can process such revs at ~5000 rev/s on my
I had to rub my eyes when I read this - what???
Good job, then!
--
Cheers,
Ray Chuan
^ permalink raw reply
* [PATCH] Fix 'Cloning into' message
From: Richard Hartmann @ 2011-10-26 17:05 UTC (permalink / raw)
To: git; +Cc: Richard Hartmann
Without this patch,
git clone foo .
results in this:
Cloning into ....
done.
With it:
Cloning into '.'...
done.
---
builtin/clone.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 488f48e..efe8b6c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -577,9 +577,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (0 <= option_verbosity) {
if (option_bare)
- printf(_("Cloning into bare repository %s...\n"), dir);
+ printf(_("Cloning into bare repository '%s'...\n"), dir);
else
- printf(_("Cloning into %s...\n"), dir);
+ printf(_("Cloning into '%s'...\n"), dir);
}
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
--
1.7.7
^ permalink raw reply related
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 17:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Rather nasty things happen when a mutex is not initialized but locked
> nevertheless. Now, when we're not running in a threaded manner, the mutex
> is not initialized, which is correct.
Thanks; I wonder why pack-objects does not have the same issue, though.
^ permalink raw reply
* Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists
From: Junio C Hamano @ 2011-10-26 17:26 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CjJnO6rDVTV1tC9rWXP51LHBtUvNsgVWNfwC+HuNQ-6Q@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Now look from "git add" perspective, it does not really care where
> $GIT_DIR is.
> It assumes that $(pwd) is working directory's top. So it
Now you confused me.
Doesn't it use $GIT_DIR to find the index? And it decides that it is at
the top level because it is given GIT_DIR but not GIT_WORKING_TREE which
is how working tree discovery is defined.
> - reads content of current directory, it sees "clone2" as a directory
> - it descends in and see ".git" so "clone2" must be a git link
> - because clone2 is a separate repository (again $GIT_DIR is not
> consulted), "b" should be managed by "clone2"
> - so it stops.
>
> This is the only place I see a submodule (from the first glance) is
> actually top level repository. Yes I guess we can support this, but
> it's just too weird to be widely used in pratice..
Where did you get this idea that submodule is somehow involved in this test?
I do not see there is any submodules involved; the test uses two
repositories 1 and 2 that appear in the working tree of the main
repository test infrastructure created, but otherwise there is no
sub/super relation among these three, and there are many other tests with
"clone" or "mkdir+init" or "init <newdir>" in the main test repository.
The clone2 repository tracks a path without having a corresponding file in
its working tree (i.e. it has "b" but tracks "clone2/b") which I already
is said unusual, but unusual does not mean it is bad or we want to remove
a test that covers the unusual case to let a change that regresses the
case go unnoticed.
>> Running (cd clone2 && git add b) is a _more natural_ thing to do, and it
>> will result in a path "b" added to the clone2 repository, so that the
>> result is more useful if you are going to chdir to the repository and keep
>> working on it. But that does not mean the existing test is incorrect. It
>> does not just pass somehow but the test passes by design.
>>
>> I did not check if later tests look at the contents of commit "new2" to
>> make sure it contains "clone2/b", but if they do this change should break
>> such tests.
>>
>> So I am puzzled by this change; what is this trying to achieve?
So again, what is this change trying to achieve?
^ permalink raw reply
* Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
From: Junio C Hamano @ 2011-10-26 17:37 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CocoAiVx_PeaaX1oRZvmzfj9-z9JLJkE5unSRVtpGkNA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Current read_directory() treats given path separately from contents
> inside the path. If the given path has ".git", it's ok (but it'll stop
> at .git if during tree recursion). The new read_directory() does not
> make this exception, so when note-merge call
> read_directory(".git/NOTES_MERGE_WORKTREE"), read_directory() sees
> ".git" and stops immediately, assuming it's a gitlink.
When read_directory("where/ever") is called, what kind of paths does it
collect? Do the paths the function collects share "where/ever" as their
common prefix? I thought it collects the paths relative to whatever
top-level directory given to the function, so that "where/ever" could be
anything.
Why does it even have to look at the given path in the first place and
make a decision heavier than "can I opendir() and read from it"? In other
words, if you see read_directory("some/thing/.git/more/stuff") and find a
substring ".git/" in there, what "magic" gitlink handling does the code
have to do?
I do not think it matters for _this_ particular case, but I can for
example imagine an alternative implementation of a merge that uses
temporary working tree somewhere other than the main working tree, and one
of the natural "temporary" places such a feature in the future may want to
use is inside .git/ somewhere. If you are planning to close the door by
breaking the behaviour of read_directory(".git/some_where") for such
callers with this series, we need to be aware of it, and that is why I am
not satisfied by your explanation.
^ permalink raw reply
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Johannes Schindelin @ 2011-10-26 18:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: msysgit, git
In-Reply-To: <7vvcrb3c69.fsf@alter.siamese.dyndns.org>
Him
On Wed, 26 Oct 2011, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Rather nasty things happen when a mutex is not initialized but locked
> > nevertheless. Now, when we're not running in a threaded manner, the
> > mutex is not initialized, which is correct.
>
> Thanks; I wonder why pack-objects does not have the same issue, though.
Those tests do not fail here, so I did not investigate.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Stefan Naewe @ 2011-10-26 19:13 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Stefan Naewe
In-Reply-To: <CAJzBP5QYKOf4OUMm4vfVay=b7F_fHJf40JgzDAZZa7p0fxLpyA@mail.gmail.com>
Git for Windows comes with a bash that doesn't support process substitution.
It issues the following error when using git-completion.bash with
GIT_PS1_SHOWUPSTREAM set:
$ export GIT_PS1_SHOWUPSTREAM=1
sh.exe": cannot make pipe for process substitution: Function not implemented
sh.exe": cannot make pipe for process substitution: Function not implemented
sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
Replace the process substitution with a 'here string'.
Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
---
contrib/completion/git-completion.bash | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..0b3d47e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -110,6 +110,7 @@ __git_ps1_show_upstream ()
local upstream=git legacy="" verbose=""
# get some config options from git-config
+ output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
while read key value; do
case "$key" in
bash.showupstream)
@@ -125,7 +126,7 @@ __git_ps1_show_upstream ()
upstream=svn+git # default upstream is SVN if available, else git
;;
esac
- done < <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')
+ done <<< "$output"
# parse configuration values
for option in ${GIT_PS1_SHOWUPSTREAM}; do
--
1.7.7.1
^ permalink raw reply related
* Re: git-fixup-assigner.perl -- automatically decide where to "fixup!"
From: Thomas Rast @ 2011-10-26 19:40 UTC (permalink / raw)
To: fREW Schmidt; +Cc: git
In-Reply-To: <CADVrmKT1woYpJoe=L9sAbQ30TUw44zMc7y4WF=PMrT5Gj9kDNw@mail.gmail.com>
fREW Schmidt wrote:
>
> $ git fixup-assigner.pl > fixups && less fixups
> #!/bin/sh
>
> git apply --cached <<EOF
[...]
> - $self->set($c,
> $c->model('DB')->schema->kiokudb_handle->lookup('dashboard
> templates'));
> + $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
> }
> EOF
>
> git commit --fixup 7765cbd2
>
> Looks fine to me. But then I try to use it:
The shell will expand variables in the <<EOF section above. You can
fix that by changing it to output <<\EOF instead, which is clearly a
bug in the original script.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] replace sha1 with another algorithm
From: Junio C Hamano @ 2011-10-26 19:44 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111026001237.GA22195@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> +static void xor_bytes(unsigned char *out, unsigned char *a, unsigned char *b,
> + unsigned n)
> +{
> + unsigned i;
> + for (i = 0; i < n; i++)
> + out[i] = a[i] ^ b[i];
> +}
> +
> +static void mix_hash(unsigned char *h, unsigned n)
> +{
> + unsigned char out[20];
> + unsigned mid = n / 2;
> +
> + if (2*mid < n)
> + return;
> +
> + xor_bytes(out, h, h + mid, mid);
> + xor_bytes(out + mid, h + mid, h, mid);
> + memcpy(h, out, n);
> +
> + /* If a little bit of mixing is good, then a lot must be GREAT! */
> + mix_hash(h, mid);
> + mix_hash(h + mid, mid);
> +}
You seem to want to reduce the hash down to 5-bytes by duplicating the
same value on the left and right half, and duplicate that four times to
fill 20-byte buffer, but doesn't this look unnecessarily inefficient way
to achieve that?
^ permalink raw reply
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 20:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I looked around a bit but ran out of time to identify the reason why
> this was not caught earlier.
>
> builtin/grep.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 92eeada..e94c5fe 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -78,10 +78,11 @@ static pthread_mutex_t grep_mutex;
> /* Used to serialize calls to read_sha1_file. */
> static pthread_mutex_t read_sha1_mutex;
>
> -#define grep_lock() pthread_mutex_lock(&grep_mutex)
> -#define grep_unlock() pthread_mutex_unlock(&grep_mutex)
> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
> +#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0)
> +#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex))
> +#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex))
> +#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex))
> +#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex))
I think, from a quick glance, this is a good first step.
The remainder of this message are hints and random thoughts on potential
follow-up patches that may want to build on top of this patch for further
clean-ups (not specifically meant for Dscho but for other people on both
mailing lists).
- The patch makes the check for use_threads in lock_and_read_sha1_file()
redundant. The other user of read_sha1_lock/unlock in grep_object() can
take advantage of this change (see below).
- It makes me wonder if it is simpler to initialize mutexes even in
!use_threads case.
- Wouldn't the result be more readable to make these into static inline
functions?
- Could we lose "#ifndef NO_PTHREADS" inside grep_sha1(), grep_file(),
and possibly cmd_grep() functions and let the compiler optimize things
away under NO_PTHREADS compilation?
diff --git a/builtin/grep.c b/builtin/grep.c
index 7d0779f..60daa85 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -354,13 +354,9 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type
{
void *data;
- if (use_threads) {
- read_sha1_lock();
- data = read_sha1_file(sha1, type, size);
- read_sha1_unlock();
- } else {
- data = read_sha1_file(sha1, type, size);
- }
+ read_sha1_lock();
+ data = read_sha1_file(sha1, type, size);
+ read_sha1_unlock();
return data;
}
^ permalink raw reply related
* Re: [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Junio C Hamano @ 2011-10-26 20:02 UTC (permalink / raw)
To: Stefan Naewe; +Cc: spearce, git
In-Reply-To: <1319656389-9515-1-git-send-email-stefan.naewe@gmail.com>
Stefan Naewe <stefan.naewe@gmail.com> writes:
> Git for Windows comes with a bash that doesn't support process substitution.
> It issues the following error when using git-completion.bash with
> GIT_PS1_SHOWUPSTREAM set:
>
> $ export GIT_PS1_SHOWUPSTREAM=1
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
>
> Replace the process substitution with a 'here string'.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
Yuck, but I honestly shouldn't care about the yuckiness as this script is
inherently intimately dependent on bash anyway ;-).
> contrib/completion/git-completion.bash | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8648a36..0b3d47e 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -110,6 +110,7 @@ __git_ps1_show_upstream ()
> local upstream=git legacy="" verbose=""
>
> # get some config options from git-config
> + output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
> while read key value; do
> case "$key" in
> bash.showupstream)
> @@ -125,7 +126,7 @@ __git_ps1_show_upstream ()
> upstream=svn+git # default upstream is SVN if available, else git
> ;;
> esac
> - done < <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')
> + done <<< "$output"
>
> # parse configuration values
> for option in ${GIT_PS1_SHOWUPSTREAM}; do
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox