Git development
 help / color / mirror / Atom feed
* Re: [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits
From: Drew Northup @ 2011-10-25 22:35 UTC (permalink / raw)
  To: Jeff King
  Cc: Ted Ts'o, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <20111020155611.GB16114@sigill.intra.peff.net>


On Thu, 2011-10-20 at 11:56 -0400, Jeff King wrote:
> On Thu, Oct 20, 2011 at 09:14:55AM -0400, Ted Ts'o wrote:
> 
> > Another possibility is to warn if the commit messages are not NULL
> > terminated.
> 
> A minor nit, but it's not whether they are terminated with NUL, but
> rather whether they have embedded NUL. But yeah, this could maybe just
> be something fsck looks for.

God (or your deity of choice) forbid that anybody ever writes code that
saves raw UTF-16 as the commit message...

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: [PATCH] gitweb/Makefile: Remove static/gitweb.js in the clean target
From: Drew Northup @ 2011-10-25 22:58 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, Jakub Narebski, GIT Mailing-list
In-Reply-To: <4EA6EEA8.3000204@ramsay1.demon.co.uk>


On Tue, 2011-10-25 at 18:15 +0100, Ramsay Jones wrote:
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  gitweb/Makefile |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index 1c85b5f..4191c6b 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -185,7 +185,9 @@ install: all
>  ### Cleaning rules
>  
>  clean:
> -	$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
> +	$(RM) gitweb.cgi static/gitweb.js \
> +		static/gitweb.min.js static/gitweb.min.css \
> +		GITWEB-BUILD-OPTIONS
>  
>  .PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
>  

Forgive me for sounding a bit numb, but what does this fix? I don't see
it in the commit message.

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
From: Nguyen Thai Ngoc Duy @ 2011-10-26  0:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzkgo3m9b.fsf@alter.siamese.dyndns.org>

2011/10/26 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> notes_merge_commit() only needs to list all entries (non-recursively)
>> under a directory, which can be easily accomplished with
>> opendir/readdir and would be more lightweight than read_directory().
>>
>> read_directory() is designed to list paths inside a working
>> directory. Using it outside of its scope may lead to undesired effects.
>
> Technically isn't the directory structure this codepath looks at a working
> tree that has extract of a notes tree commit?

Yes it's like a secondary working tree, only for notes, if I read the
code correctly. The thing is this space is inside ".git".

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.

One could say we should keep current behavior, but I don't really see
it's worth the effort.
-- 
Duy

^ permalink raw reply

* [PATCH] replace sha1 with another algorithm
From: Jeff King @ 2011-10-26  0:12 UTC (permalink / raw)
  To: git

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>
---
 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 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);
+}
+
+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 related

* Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists
From: Nguyen Thai Ngoc Duy @ 2011-10-26  0:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3dk516p.fsf@alter.siamese.dyndns.org>

2011/10/26 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> In this case, "foo" is considered a submodule and bar, if added,
>> belongs to foo/.git. "git add" should only allow "git add foo" in this
>> case, but it passes somehow.
>
> I do not think the above description is correct.
>
> The test:
>
>  - populates the current directory;
>  - makes a clone in ./clone2;
>  - creates a file clone2/b;
>  - runs "git add clone2/b" with GIT_DIR set to clone2/.git, without
>   setting GIT_WORK_TREE nor having core.worktree in clone2/.git/config.
>
> The last step should add a path "clone2/b" to $GIT_DIR/index (which is
> clone2/.git/index in this case).  The clone2 is not a submodule to the top
> level repository in this case, but even if it were, that would not change
> the definition of what the command should do when GIT_DIR is set without
> GIT_WORK_TREE nor core.worktree in $GIT_DIR/config.

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

 - 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..

> 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?
-- 
Duy

^ permalink raw reply

* Re: [PATCH] gitweb/Makefile: Remove static/gitweb.js in the clean target
From: Jakub Narebski @ 2011-10-26  0:36 UTC (permalink / raw)
  To: Drew Northup; +Cc: Ramsay Jones, Junio C Hamano, GIT Mailing-list
In-Reply-To: <1319583484.10399.41.camel@drew-northup.unet.maine.edu>

Drew Northup napisał:
> On Tue, 2011-10-25 at 18:15 +0100, Ramsay Jones wrote:
> > Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> > ---
> >  gitweb/Makefile |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/gitweb/Makefile b/gitweb/Makefile
> > index 1c85b5f..4191c6b 100644
> > --- a/gitweb/Makefile
> > +++ b/gitweb/Makefile
> > @@ -185,7 +185,9 @@ install: all
> >  ### Cleaning rules
> >  
> >  clean:
> > -	$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
> > +	$(RM) gitweb.cgi static/gitweb.js \
> > +		static/gitweb.min.js static/gitweb.min.css \
> > +		GITWEB-BUILD-OPTIONS
> >  
> >  .PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
> >  
> 
> Forgive me for sounding a bit numb, but what does this fix? I don't see
> it in the commit message.

gitweb.js is nowadays a generated file.  Though that bit should be
in commit message...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Kyle Moffett @ 2011-10-26  3:44 UTC (permalink / raw)
  To: kusmabite; +Cc: Johannes Sixt, msysgit, git, johannes.schindelin
In-Reply-To: <CABPQNSY6-j7iNagsJc3WKVZ94=yZHdfBswA-v0XY7vH+RxyjYQ@mail.gmail.com>

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...

Cheers,
Kyle Moffett

-- 
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] completion: fix issue with process substitution not working on Git for Windows
From: Stefan Näwe @ 2011-10-26  6:52 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: spearce, git, gitster
In-Reply-To: <4EA71E8C.8010704@kdbg.org>

Am 25. Oktober 2011 22:39 schrieb Johannes Sixt <j6t@kdbg.org>:
> Am 25.10.2011 20:01, schrieb Stefan Naewe:
>> 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 simple "echo $var | while...".
>>
>> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
>> ---
>>  contrib/completion/git-completion.bash |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 8648a36..926db80 100755
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -110,6 +110,8 @@ __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 ')"
>> +     echo "$output" | \
>>       while read key value; do
>>               case "$key" in
>>               bash.showupstream)
>> @@ -125,7 +127,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
>>
>>       # parse configuration values
>>       for option in ${GIT_PS1_SHOWUPSTREAM}; do
>
> Are you sure that the result still works as intended? The while loop
> sets a few variables. When you place it in a pipe, the loop runs in a
> subshell, and subsequent code will not see the modified values. Unless
> bash knows how to optimize away the subshell, that is.

I doesn't work in the 'git svn' case, I guess.

> OTOH, when you use while ...; do ...; done < <(...), the while loop is
> not in a subshell.
>
> An alternative is to use:  while ...; do ...; done <<< "$output"

I'll try that.

> BTW, you don't need to protect the end-of-line with a backslash if the
> line ends with the pipe symbol.

OK. Will do.

> -- Hannes

Thanks,
  Stefan
-- 
----------------------------------------------------------------
python -c "print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')"

^ permalink raw reply

* Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Atsushi Nakagawa @ 2011-10-26  3:05 UTC (permalink / raw)
  To: msysgit; +Cc: git, johannes.schindelin, j.sixt
In-Reply-To: <1319554509-6532-1-git-send-email-kusmabite@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

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"?

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(...).

> +                       pthread_mutex_init(mutex, NULL);
> +                       mutex->autoinit = 0;
> +               } else
> +                       while (mutex->autoinit != 0)
> +                               ; /* wait for other thread */
> +       }
> +
> +       EnterCriticalSection(&mutex->cs);
> +       return 0;
> +}
> [...]

-- 
Atsushi Nakagawa


[-- Attachment #2: Type: text/html, Size: 2272 bytes --]

^ permalink raw reply

* Re: [msysGit] [PATCH] git grep: be careful to use mutices only when they are initialized
From: Tay Ray Chuan @ 2011-10-26  9:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: msysgit, gitster, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>

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?

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)

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Recovery after interrupted repack
From: Hannu Koivisto @ 2011-10-26  9:15 UTC (permalink / raw)
  To: git

Hi,

I interrupted git repack -a -f -d and now most commands say "fatal:
bad object HEAD".  Additionally an attempt to run git repack again
says a bunch of "error: refs/heads/foo does not point to a valid
object!" messages.

git fsck prints a huge list of "missing blob <sha-1>" lines.

I'm using a Cygwin build of git 1.7.5.1 (I also have a build of the
latest master available).

-- 
Hannu

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox