* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Andrzej K. Haczewski @ 2009-11-04 14:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4AF175E8.7020400@viscovery.net>
2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
>> +static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)
>
> What's wrong with 'static inline int ...' (without the underscores)?
>
Forgot to answer. 'inline' is avaliable in C++ only (on MSVC at
least), while '__inline' is MS extension for C and C++.
--
Andrew
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Johannes Schindelin @ 2009-11-04 14:04 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git
In-Reply-To: <1257331059-26344-1-git-send-email-ahaczewski@gmail.com>
Hi,
On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 02f9246..c96d293 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -2327,6 +2327,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> #ifdef THREADED_DELTA_SEARCH
> if (!delta_search_threads) /* --threads=0 means autodetect */
> delta_search_threads = online_cpus();
> +
> +#ifdef _WIN32
This flies in the face of our endeavors to enhance readability by reducing
the number of #ifdef's, and at least guarding the #ifdef'ed parts behind
meaningful names rather than platform specifiers.
See for example THREADED_DELTA_SEARCH: it does not read "HAS_PTHREADS" or
some such.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Andrzej K. Haczewski @ 2009-11-04 13:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4AF175E8.7020400@viscovery.net>
2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> Please do not cull Cc list when you resend a patch, if possible.
Ok, will do. I was sending patch using git send-email and I just
forgot to copy Cc there. Still trying to BTW is there a way to
"reformat-patch" with new amended commit and then "resend-email"?
> After staring some time on the code, I have convinced myself that the
> pthread_cond_wait and pthread_cond_signal implementation will work *in our
> usage scenario* that has these preconditions:
But it is not impossible with that implementation. I based this
implementation on ACE (Adaptive Communication Environment, large C++
library) implementation of the same concepts. All I removed from their
implementation is cond_broadcast, since it's not used by Git. I'm sure
that ACE does the best job when it comes to threading primitives.
On resubmit I'll give more credit to ACE.
> - pthread_cond_signal is called while the mutex is held.
AFAIK that is a requirement for condition variable to be signaled
while holding the same mutex that other threads cond_wait on. I just
don't check that it is true, because Git is locking mutex.
> - We retest the condition after pthread_cond_wait returns.
>
> These conditions should be attached in BIG BOLD letters to this
> implementation; particularly, the last one.
That's also a known requirement for working with cond vars. Here's
excerpt from pthread_cond_wait man page:
When using condition variables there is always a boolean predicate
involving shared variables associated with each condition wait that is
true if the thread should proceed. Spurious wakeups from the
pthread_cond_wait() or pthread_cond_timedwait() functions may occur.
Since the return from pthread_cond_wait() or pthread_cond_timedwait()
does not imply anything about the value of this predicate, the
predicate should be re-evaluated upon such return.
> The subject is a bit misleading, IMHO. You are not porting the
> (p)threading code, but you are adding pthread_* function wrappers for Windows.
>
> Your patch adds whitespace-at-eol. Please use git show --check to see where.
>
> Please drop words from the commit message that do not make sense once this
> commit is in git's history. Look at existing commit messages to get a
> feeling for the style. Do write about "why" (motivation), "how" (design
> choices) and "how not" (dead ends that you tried).
>
Ok, thanks for pointing that out.
> I think it would be OK to drop '= PTHREAD_{MUTEX,COND}_INITIALIZER' and
> use *_init function calls without the #ifdef. Likewise for *_destroy.
Actually it won't save us many #ifdefs. There's one #ifdef for
initialization that could be saved, but then comes #ifdef for cleanup:
#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)
What you propose will remove one #ifdef _WIN32 for initialization, but
the cleanup will look almost the same:
#ifdef THREADED_DELTA_SEARCH
>
> -- Hannes
>
Thanks for awesome review, I'll fix all those returns and whitespaces
and resubmit.
--
Andrzej
^ permalink raw reply
* Re: thoughts on setting core.logAllRefUpdates default true for bare repos
From: Johannes Schindelin @ 2009-11-04 13:25 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: git
In-Reply-To: <slrnhf2uep.7d3.sitaramc@sitaramc.homelinux.net>
Hi,
On Wed, 4 Nov 2009, Sitaram Chamarty wrote:
> It seems to me an accidental push -f would cause some trouble on a bare
> repo, and would usually require a bit of grubbing around among the
> unreachable commits looking for the right one.
In one usage of mine, we have set said config value for exactly that
reason.
> What would be the downsides (other than some objects hanging
> around far longer, i.e., space issues) of setting the config
> variable core.logAllRefUpdates to be default true on bare
> repos.
I did not have time yet to investigate, but it seems that there are
problems with the permissions of shared bare repositories when activating
the reflogs.
With gitweb on a public site, there might be a problem when you pushed
some blob containing trade secrets accidentally, and try to scrub the
repository using "git gc" after a forced push.
That are the only downsides I can think of.
Ciao,
Dscho
^ permalink raw reply
* thoughts on setting core.logAllRefUpdates default true for bare repos
From: Sitaram Chamarty @ 2009-11-04 12:55 UTC (permalink / raw)
To: git
It seems to me an accidental push -f would cause some
trouble on a bare repo, and would usually require a bit of
grubbing around among the unreachable commits looking for
the right one.
What would be the downsides (other than some objects hanging
around far longer, i.e., space issues) of setting the config
variable core.logAllRefUpdates to be default true on bare
repos.
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Johannes Sixt @ 2009-11-04 12:39 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git
In-Reply-To: <1257331059-26344-1-git-send-email-ahaczewski@gmail.com>
Please do not cull Cc list when you resend a patch, if possible.
After staring some time on the code, I have convinced myself that the
pthread_cond_wait and pthread_cond_signal implementation will work *in our
usage scenario* that has these preconditions:
- There is no more than one thread waiting on any particular condition
variable instance.
- pthread_cond_signal is called while the mutex is held.
- We retest the condition after pthread_cond_wait returns.
These conditions should be attached in BIG BOLD letters to this
implementation; particularly, the last one.
On to your patch...
The subject is a bit misleading, IMHO. You are not porting the
(p)threading code, but you are adding pthread_* function wrappers for Windows.
Your patch adds whitespace-at-eol. Please use git show --check to see where.
Andrzej K. Haczewski schrieb:
> Here is slightly modified patch with more comments where explanations were
> requested (ie. non atomic release mutex and wait).
>
> The implementation of conditional variable is based on ACE.
>
> The patch needs testing from someone capable of compiling Git on Windows
> and running it with msysgit environment. I can confirm that it compiles
> cleanly on both Linux and Windows. I modified Makefile only for MSVC
> part, so if you'd like to compile it with mingw or cygwin, proper
> corrections have to be made. I aim for native MSVC compilation, that's
> why I did it like that. That's also the reason I don't like
> having Pthreads for Win32 dependency - it's faster to use native
> calls than depend on 3rd party wrapper library to do it for you
> (ie. pthreads for win32 does allocations to implement POSIX
> standard, and full-conformance isn't required by Git, since Git uses
> only small subset of pthreads).
>
> One more motivation I had for the patch: as I was reading through
> archives I had a feeling that Git aims to be as lightweight
> as possible, hence removing additional dependencies (even for
> Windows platform) seems sensible to me.
>
> Signed-off-by: Andrzej K. Haczewski <ahaczewski@gmail.com>
Please drop words from the commit message that do not make sense once this
commit is in git's history. Look at existing commit messages to get a
feeling for the style. Do write about "why" (motivation), "how" (design
choices) and "how not" (dead ends that you tried).
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 02f9246..c96d293 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -1592,7 +1592,7 @@ struct thread_params {
>
> static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;
>
> -static void *threaded_find_deltas(void *arg)
> +static THREAD_FUNC(threaded_find_deltas, arg)
> ...
> - return NULL;
> + THREAD_RETURN(NULL);
See Erik's and Paolo's comments.
> @@ -2327,6 +2327,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> #ifdef THREADED_DELTA_SEARCH
> if (!delta_search_threads) /* --threads=0 means autodetect */
> delta_search_threads = online_cpus();
> +
> +#ifdef _WIN32
> + /*
> + * Windows requires initialization of mutex (CRITICAL_SECTION)
> + * and conditional variable.
> + */
> + pthread_mutex_init(&read_mutex);
> + pthread_mutex_init(&cache_mutex);
> + pthread_mutex_init(&progress_mutex);
> + pthread_cond_init(&progress_cond, NULL);
> +#endif
I think it would be OK to drop '= PTHREAD_{MUTEX,COND}_INITIALIZER' and
use *_init function calls without the #ifdef. Likewise for *_destroy.
> +cleanup:
> +#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)
> + /* cleanup Windows threads thingies */
> + pthread_cond_destroy(&progress_cond);
> + pthread_mutex_destroy(&read_mutex);
> + pthread_mutex_destroy(&cache_mutex);
> + pthread_mutex_destroy(&progress_mutex);
> +#endif
> +
> return 0;
> }
> +
Drop this empty line at EOF.
> @@ -0,0 +1,143 @@
> +/*
> + * Header used to "adapt" pthread-based POSIX code to Windows API threads.
I think "adapt" is the right word here. You don't need to put it in quotes. ;)
> + *
> + * Copyright (C) 2009 Andrzej K. Haczewski <ahaczewski@gmail.com>
> + */
> +
> +#ifndef PTHREAD_H
> +#define PTHREAD_H
> +
> +#ifndef WIN32_LEAN_AND_MEAN
> +#define WIN32_LEAN_AND_MEAN
> +#endif
> +
> +#include <windows.h>
> +
> +/*
> + * don't include mingw.h for err_win_to_posix function - mingw.h doesn't
> + * have include-guards
So what? Is there an #include loop? Can't you add include guards?
> +static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)
What's wrong with 'static inline int ...' (without the underscores)?
> +{
> + cond->waiters = 0;
> +
> + InitializeCriticalSection(&cond->waiters_lock);
> +
> + cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
> + if (NULL == cond->sema)
> + return -1;
> + return 0;
In case of failure, the pthread_* functions return the error number, not
-1. Moreover, we write
if (!cond->sema)
return err_win_to_posix(GetLastError());
or
return cond->sema ? 0 : err_win_to_posix(GetLastError());
> +static __inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
> +{
> ...
> + /* let's wait */
> + if (0 != WaitForSingleObject(cond->sema, INFINITE))
> + ret = -1;
Mind the return value!
> +static __inline int pthread_cond_signal(pthread_cond_t *cond)
> +{
> ...
> + if (have_waiters)
> + return ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;
Return value again.
> +static __inline int pthread_create(pthread_t *t, const void *unused, DWORD (__stdcall *start_routine)(LPVOID), void *arg)
> +{
> + *t = CreateThread(NULL, 0, start_routine, arg, 0, NULL);
> +
> + if (NULL == *t) {
if (!*t)
> + errno = err_win_to_posix(GetLastError());
> + return -1;
Return value again. errno is not set.
> + } else {
> + errno = 0;
> + return 0;
> + }
> +}
> +
> +static __inline int pthread_join(pthread_t t, void **unused)
> +{
> ...
> + errno = err_win_to_posix(GetLastError());
> + return -1;
And again.
-- Hannes
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Paolo Bonzini @ 2009-11-04 11:23 UTC (permalink / raw)
To: git
In-Reply-To: <1257331059-26344-1-git-send-email-ahaczewski@gmail.com>
The condition variable implementation seems more complicated than
necessary. The mutex can be used to protect access to cond->waiters, so
waiters_lock is not necessary. On the other hand, it seems to me that
pthread_cond_signal should be the one that decrements the waiters count.
Otherwise, a loop like
while (pthread_cond_signal (cond, mutex));
will fill the semaphore with signals and the waiters will get lots of
spurious accesses.
static __inline int pthread_cond_wait(pthread_cond_t *cond,
CRITICAL_SECTION *mutex)
{
int ret = 0;
/* the mutex protects access to waiters count */
++cond->waiters;
/*
* Unlock external mutex and wait for signal.
* NOTE: cond->waiters > 0 now. If pthread_cond_signal
* is called after leaving mutex unlocked before we wait on
* semaphore, it will add a signal to the semaphore,
* and we'll happily go on with the wait. This would not
* happen with an event, for example.
*/
LeaveCriticalSection(mutex);
if (0 != WaitForSingleObject(cond->sema, INFINITE))
ret = -1;
EnterCriticalSection(mutex);
return ret;
}
static __inline int pthread_cond_signal(pthread_cond_t *cond)
{
/* the mutex protects access to waiters count */
if (cond->waiters > 0) {
--cond->waiters;
return ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;
} else
return 0;
}
Paolo
^ permalink raw reply
* Re: [PATCH v2] format-patch: autonumber by default
From: Eric W. Biederman @ 2009-11-04 11:20 UTC (permalink / raw)
To: Brian Gernhardt
Cc: Git List, Giuseppe Bilotta, Jakub Narebski, Johannes Schindelin,
Shawn O. Pearce, Andreas Ericsson
In-Reply-To: <20081002205539.GA36768@Hermes>
Brian Gernhardt <benji@silverinsanity.com> writes:
> format-patch is most commonly used for multiple patches at once when
> sending a patchset, in which case we want to number the patches; on
> the other hand, single patches are not usually expected to be
> numbered.
>
> In other words, the typical behavior expected from format-patch is the
> one obtained by enabling autonumber, so we set it to be the default.
>
> Users that want to disable numbering for a particular patchset can do
> so with the existing -N command-line switch. Users that want to
> change the default behavior can use the format.numbering config key.
>
> Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
> ---
Grumble. I updated git last night and this change just bit me.
Grumble.
Grumble.
It is probably a good change, but it was unexpected.
Grumble. Grumble. Grumble.
Eric
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Paolo Bonzini @ 2009-11-04 11:14 UTC (permalink / raw)
To: git
In-Reply-To: <40aa078e0911040250k55fa1920g6eee5657c6e35345@mail.gmail.com>
On 11/04/2009 11:50 AM, Erik Faye-Lund wrote:
> On Wed, Nov 4, 2009 at 11:37 AM, Andrzej K. Haczewski
> <ahaczewski@gmail.com> wrote:
>> +/*
>> + * Properly defines thread routine for Windows and POSIX
>> + */
>> +#ifndef NO_PTHREADS
>> +# ifndef _WIN32
>> +# define THREAD_FUNC(f, a) void *f(void *a)
>> +# define THREAD_RETURN(x) return (x)
>> +# else
>> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
>> +# define THREAD_RETURN(x) return (DWORD)(x);
>> +# endif
>> +#endif
>> +
>
> Seeing how THREAD_RETURN is only called with NULL-parameter, perhaps
> it would be better to do this?
>
> +/*
> + * Properly defines thread routine for Windows and POSIX
> + */
> +#ifndef NO_PTHREADS
> +# ifndef _WIN32
> +# define THREAD_FUNC(f, a) void *f(void *a)
> +# define THREAD_RETURN() return NULL;
> +# else
> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
> +# define THREAD_RETURN() return 0;
> +# endif
> +#endif
>> +
Even better, "return 0" is good under either platform (0 converts to
void *), and LPVOID is the same thing as void*, so you can just do
#ifndef _WIN32
# define THREAD_RET_TYPE DWORD __stdcall
#else
# define THREAD_RET_TYPE void *
#endif
Paolo
^ permalink raw reply
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Johannes Schindelin @ 2009-11-04 11:02 UTC (permalink / raw)
To: Joshua Jensen; +Cc: git
In-Reply-To: <4AF0E842.2010201@workspacewhiz.com>
Hi,
I do not appreciate at all that you culled me from the Cc: list.
On Tue, 3 Nov 2009, Joshua Jensen wrote:
> ----- Original Message -----
> From: Johannes Schindelin
> Date: 11/3/2009 4:38 PM
> > > #ifdef THREADED_DELTA_SEARCH
> > > -#include "thread-utils.h"
> > > -#include<pthread.h>
> > > +# include "thread-utils.h"
> > > +# ifndef _WIN32
> > > +# include<pthread.h>
> > > +# else
> > > +# include<winthread.h>
> > > +# endif
> > > #endif
> > >
> > >
> > It is unlikely that an #ifdef "contamination" of this extent will go
> > through easily, but I have a suggestion that may make your patch both
> > easier to read and more likely to be accepted into git.git: Try to
> > wrap the win32 calls into pthread-compatible function signatures.
> > Then you can add a compat/win32/pthread.h and not even touch core
> > files of git.git at all.
> >
> Pardon my ignorance, but is there a reason to not use Pthreads for
> Win32? http://sourceware.org/pthreads-win32/
Pthreads is a rather large dependency we do not really need.
Ciao,
Dscho
^ permalink raw reply
* Re: Problem with "From:" line on "git format-patch" generated patches
From: André Goddard Rosa @ 2009-11-04 10:59 UTC (permalink / raw)
To: Jeff King; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <20091104084903.GA1137@coredump.intra.peff.net>
On 11/4/09, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 03, 2009 at 04:06:39PM -0200, André Goddard Rosa wrote:
>
>> I'm not using any specific tool for inputting the git-format-patch,
>> but instead I'm sending the files generated by it through gmail as an
>> inlined patch in the email body.
>>
>> I like the convenience of format-patch for generating the patch files,
>> but in this case, formatting the header as rfc2047 is not necessary
>> and makes a funny/garbled output in my patch submission.
>>
>> Do you have a suggestion for my workflow?
>
> I don't think there's currently a way to turn off the rfc2047 from
> within format-patch. You can generate a single patch with the same
> format using:
>
> git log -1 -p --stat --summary \
> --pretty=tformat:'From: %an <%ae>%nDate: %aD%nSubject: [PATCH] %s%n%n%b'
>
> but it won't do nice things like putting one patch in each file.
>
> Probably it would make sense for format-patch to have an option to
> indicate that you are going to inline these patches into a different
> MUA. So drop the 'From' mbox header line, don't rfc2047 encode, and
> maybe some other behaviors. I do the same thing (including inline in
> mutt), but I just delete the unwanted lines manually, and fortunately my
> name doesn't contain any non-ascii characters. ;)
>
> -Peff
Hello, Peff!
It's good to know that I'm not alone on this. I think that should
be fairly easy, yes.
Thanks for helping!
Thank you all,
André
^ permalink raw reply
* Re: Accessing the reflog remotely
From: Tomas Carnecky @ 2009-11-04 10:58 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqljimpr95.fsf@bauges.imag.fr>
On Nov 4, 2009, at 10:35 AM, Matthieu Moy wrote:
> Hi,
>
> I guess the answer is "no", but I'll still ask in case ...
>
> Is it possible to access the reflog of a remote repository other than
> logging into this repository?
>
> My use-case is the following: I want to checkout "the last revision
> pushed in master on ssh://host/repo/ on day D at midnight" (to fetch
> the project of my students ;-) ). If it were locally, I'd do
>
> git checkout 'origin/master@{Nov 3 00:00:00}'
>
> But this tells me where _my_ local master was on that date (i.e. the
> last revision I had pulled).
Keep in mind that bare repos don't have reflogs by default, so unless
you enabled the reflog manually there is none.
tom
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Andrzej K. Haczewski @ 2009-11-04 10:56 UTC (permalink / raw)
To: kusmabite; +Cc: git
In-Reply-To: <40aa078e0911040250k55fa1920g6eee5657c6e35345@mail.gmail.com>
2009/11/4 Erik Faye-Lund <kusmabite@googlemail.com>:
>
> Seeing how THREAD_RETURN is only called with NULL-parameter, perhaps
> it would be better to do this?
>
> +/*
> + * Properly defines thread routine for Windows and POSIX
> + */
> +#ifndef NO_PTHREADS
> +# ifndef _WIN32
> +# define THREAD_FUNC(f, a) void *f(void *a)
> +# define THREAD_RETURN() return NULL;
> +# else
> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
> +# define THREAD_RETURN() return 0;
> +# endif
> +#endif
Good point. Should I resubmit the patch again?
Andrew
^ permalink raw reply
* Re: Problem with "From:" line on "git format-patch" generated patches
From: André Goddard Rosa @ 2009-11-04 10:55 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <20091103225556.GA20160@progeny.tock>
On 11/3/09, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi André,
>
> André Goddard Rosa wrote:
>
>>> I'm not using any specific tool for inputting the git-format-patch,
>>> but instead I'm sending the files generated by it through gmail as an
>>> inlined patch in the email body.
>>>
>>> I like the convenience of format-patch for generating the patch files,
>>> but in this case, formatting the header as rfc2047 is not necessary
>>> and makes a funny/garbled output in my patch submission.
>
> The header fields git format-patch outputs are just intended as a
> starting point for the header of your mailing. It is more convenient
> to receive an e-mail with
>
> Delivered-to: maintainer@example.com
> Received: [...]
> Message-ID: <patch.sender.0001@example.com>
> Date: Tue, 03 Nov 2009 16:33:54 -0600
> From: Patch Sender <patch.sender@example.com>
> Subject: [PATCH] Fix one bug, add another
> Content-Type: text/plain; charset=us-ascii
>
> Blah blah blah
>
> than one in which the content includes some useless metadata that was
> already in the header. So you should just strip the header out from
> the body before sending.
>
> There are three common exceptions: 1) you might want to send a patch
> written by someone else, 2) you might want to mark a patch as written
> before it was sent, and 3) some people like to receive patches as
> attachments rather than inlined in messages. For the first two cases,
> the solution is to include the header fields to change in the body:
>
> From: Patch Writer <patch.writer@example.com>
> Date: Wed, 01 Apr 1970 01:23:45 +0100
>
> Blah blah blah
> ---
> Hi,
>
> Patch Writer wrote this patch a while ago that might be
> relevant. It needed a straightforward one-line change to
> apply and is otherwise unchanged.
>
> What do you think?
> [...]
>
> For the last case, I think it is most common to send unchanged 'git
> format-patch' output. But only the From, Date, and Subject fields
> are actually needed.
>
> I am not sure how well 'git am' copes with non-ascii characters in
> the pseudo-header lines: I would have guessed it could handle them
> both rfc2047-encoded and not, but I have not tried.
>
>> I really would like continuing having the convenience of using a web
>> access to my gmail for sending the patches, so I just need a way to
>> format the patches which makes it easy submitting them later. I'd like
>> to avoid using any other email client for that, if possible.
>
> Here, there is another danger: the Gmail web interface does not
> consider your whitespace precious, so it is very prone to mangling
> patches (especially with long lines).
>
> Documentation/SubmittingPatches [1] has some advice:
>
> | Gmail
> | -----
> |
> | GMail does not appear to have any way to turn off line wrapping in the web
> | interface, so this will mangle any emails that you send. You can however
> | use any IMAP email client to connect to the google imap server, and
> forward
> | the emails through that. Just make sure to disable line wrapping in that
> | email client. Alternatively, use "git send-email" instead.
> |
> | Submitting properly formatted patches via Gmail is simple now that
> | IMAP support is available. First, edit your ~/.gitconfig to specify your
> | account settings:
> |
> | [imap]
> | folder = "[Gmail]/Drafts"
> | host = imaps://imap.gmail.com
> | user = user@gmail.com
> | pass = p4ssw0rd
> | port = 993
> | sslverify = false
> |
> | You might need to instead use: folder = "[Google Mail]/Drafts" if you get
> an error
> | that the "Folder doesn't exist".
> |
> | Next, ensure that your Gmail settings are correct. In "Settings" the
> | "Use Unicode (UTF-8) encoding for outgoing messages" should be checked.
> |
> | Once your commits are ready to send to the mailing list, run the following
> | command to send the patch emails to your Gmail Drafts folder.
> |
> | $ git format-patch -M --stdout origin/master | git imap-send
> |
> | Go to your Gmail account, open the Drafts folder, find the patch email,
> fill
> | in the To: and CC: fields and send away!
>
> Good luck.
>
> Hope that helps,
Hello, Jonatan!
Thanks for your insights, surely I understand what is reasoning behind it.
I expect this thread will be useful to others in the future. Here
goes another relevant reference about sending patches using web GUI of
gmail :
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt;h=a618efab7b156658be70b29c1a6a9b9c4093e0f5;hb=HEAD
Thanks a lot,
André
^ permalink raw reply
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Andrzej K. Haczewski @ 2009-11-04 10:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4AF13819.7050306@viscovery.net>
2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> Can't you just use the pthread package that is included in msysgit?
I don't like bloat, and msysgit is bloated. Sure, there are parts of
msysgit that are even heavier (bash, perl), but this will be removed
with further C'ification of scripts. I did what I thought could be
sensible for my first patch. I'm newbie after all.
>> + /* we're waiting... */
>> + EnterCriticalSection(&cond->waiters_lock);
>> + ++cond->waiters;
>> + LeaveCriticalSection(&cond->waiters_lock);
>> +
>> + /* unlock external mutex and wait for signal */
>> + LeaveCriticalSection(mutex);
>> + result = WaitForSingleObject(cond->sema, INFINITE);
>
> Releasing the mutex and entering the wait state as well as leaving the
> wait state and reacquiring the mutex should be atomic. Neither are in this
> implementation. You are not mentioning why you are implementing things
> like this and why this would be acceptable.
It's safe to do it like this here because we're serializing waiters
count and when signaling we make sure we have waiters before we
release semaphore. That implementation is based on ACE.
Andrew
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Erik Faye-Lund @ 2009-11-04 10:50 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git
In-Reply-To: <1257331059-26344-1-git-send-email-ahaczewski@gmail.com>
On Wed, Nov 4, 2009 at 11:37 AM, Andrzej K. Haczewski
<ahaczewski@gmail.com> wrote:
> +/*
> + * Properly defines thread routine for Windows and POSIX
> + */
> +#ifndef NO_PTHREADS
> +# ifndef _WIN32
> +# define THREAD_FUNC(f, a) void *f(void *a)
> +# define THREAD_RETURN(x) return (x)
> +# else
> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
> +# define THREAD_RETURN(x) return (DWORD)(x);
> +# endif
> +#endif
> +
Seeing how THREAD_RETURN is only called with NULL-parameter, perhaps
it would be better to do this?
+/*
+ * Properly defines thread routine for Windows and POSIX
+ */
+#ifndef NO_PTHREADS
+# ifndef _WIN32
+# define THREAD_FUNC(f, a) void *f(void *a)
+# define THREAD_RETURN() return NULL;
+# else
+# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
+# define THREAD_RETURN() return 0;
+# endif
+#endif
> +
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Automatically remote prune
From: John Tapsell @ 2009-11-04 10:42 UTC (permalink / raw)
To: Git List
Hi all,
Is there any particular reason why "git update" doesn't
automatically remove remote branches that have been removed on the
server? I keep getting questions about this from confused users.
If there's strong resistance against that (there usually is for
changing any default behaviour), could we at least mark deleted
branches?
So maybe:
$> git branch -r
origin/blah1 [Deleted]
origin/blah2
(Some branches have been deleted on the remote server. Use "git
remote prune origin" to remove these)
Or something.
John
^ permalink raw reply
* [PATCH] MSVC: port pthread code to native Windows threads
From: Andrzej K. Haczewski @ 2009-11-04 10:37 UTC (permalink / raw)
To: git; +Cc: Andrzej K. Haczewski
In-Reply-To: <1257283802-29726-1-git-send-email-ahaczewski@gmail.com>
Here is slightly modified patch with more comments where explanations were
requested (ie. non atomic release mutex and wait).
The implementation of conditional variable is based on ACE.
The patch needs testing from someone capable of compiling Git on Windows
and running it with msysgit environment. I can confirm that it compiles
cleanly on both Linux and Windows. I modified Makefile only for MSVC
part, so if you'd like to compile it with mingw or cygwin, proper
corrections have to be made. I aim for native MSVC compilation, that's
why I did it like that. That's also the reason I don't like
having Pthreads for Win32 dependency - it's faster to use native
calls than depend on 3rd party wrapper library to do it for you
(ie. pthreads for win32 does allocations to implement POSIX
standard, and full-conformance isn't required by Git, since Git uses
only small subset of pthreads).
One more motivation I had for the patch: as I was reading through
archives I had a feeling that Git aims to be as lightweight
as possible, hence removing additional dependencies (even for
Windows platform) seems sensible to me.
Signed-off-by: Andrzej K. Haczewski <ahaczewski@gmail.com>
---
Makefile | 4 +-
builtin-pack-objects.c | 29 +++++++++-
compat/mingw.c | 2 +-
compat/win32/pthread.h | 143 ++++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 13 ++++
preload-index.c | 4 +-
6 files changed, 187 insertions(+), 8 deletions(-)
create mode 100644 compat/win32/pthread.h
diff --git a/Makefile b/Makefile
index 521e8a5..450d8fe 100644
--- a/Makefile
+++ b/Makefile
@@ -939,7 +939,7 @@ ifdef MSVC
OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
NO_REGEX = YesPlease
NO_CURL = YesPlease
- NO_PTHREADS = YesPlease
+ THREADED_DELTA_SEARCH = YesPlease
BLK_SHA1 = YesPlease
CC = compat/vcbuild/scripts/clink.pl
@@ -947,7 +947,7 @@ ifdef MSVC
CFLAGS =
BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o
- COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\".exe\"
+ COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
lib =
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 02f9246..c96d293 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1592,7 +1592,7 @@ struct thread_params {
static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;
-static void *threaded_find_deltas(void *arg)
+static THREAD_FUNC(threaded_find_deltas, arg)
{
struct thread_params *me = arg;
@@ -1620,7 +1620,7 @@ static void *threaded_find_deltas(void *arg)
pthread_mutex_unlock(&me->mutex);
}
/* leave ->working 1 so that this doesn't get more work assigned */
- return NULL;
+ THREAD_RETURN(NULL);
}
static void ll_find_deltas(struct object_entry **list, unsigned list_size,
@@ -2327,6 +2327,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
#ifdef THREADED_DELTA_SEARCH
if (!delta_search_threads) /* --threads=0 means autodetect */
delta_search_threads = online_cpus();
+
+#ifdef _WIN32
+ /*
+ * Windows requires initialization of mutex (CRITICAL_SECTION)
+ * and conditional variable.
+ */
+ pthread_mutex_init(&read_mutex);
+ pthread_mutex_init(&cache_mutex);
+ pthread_mutex_init(&progress_mutex);
+ pthread_cond_init(&progress_cond, NULL);
+#endif
+
#endif
prepare_packed_git();
@@ -2345,7 +2357,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
stop_progress(&progress_state);
if (non_empty && !nr_result)
- return 0;
+ goto cleanup;
if (nr_result)
prepare_pack(window, depth);
write_pack_file();
@@ -2353,5 +2365,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
fprintf(stderr, "Total %"PRIu32" (delta %"PRIu32"),"
" reused %"PRIu32" (delta %"PRIu32")\n",
written, written_delta, reused, reused_delta);
+
+cleanup:
+#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)
+ /* cleanup Windows threads thingies */
+ pthread_cond_destroy(&progress_cond);
+ pthread_mutex_destroy(&read_mutex);
+ pthread_mutex_destroy(&cache_mutex);
+ pthread_mutex_destroy(&progress_mutex);
+#endif
+
return 0;
}
+
diff --git a/compat/mingw.c b/compat/mingw.c
index 6b5b5b2..f2e9f02 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -5,7 +5,7 @@
#include <shellapi.h>
-static int err_win_to_posix(DWORD winerr)
+int err_win_to_posix(DWORD winerr)
{
int error = ENOSYS;
switch(winerr) {
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
new file mode 100644
index 0000000..8f82d3c
--- /dev/null
+++ b/compat/win32/pthread.h
@@ -0,0 +1,143 @@
+/*
+ * Header used to "adapt" pthread-based POSIX code to Windows API threads.
+ *
+ * Copyright (C) 2009 Andrzej K. Haczewski <ahaczewski@gmail.com>
+ */
+
+#ifndef PTHREAD_H
+#define PTHREAD_H
+
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+
+#include <windows.h>
+
+/*
+ * don't include mingw.h for err_win_to_posix function - mingw.h doesn't
+ * have include-guards
+ */
+extern int err_win_to_posix(DWORD winerr);
+
+/* Implement simple condition variable for Windows threads, based on ACE implementation */
+typedef struct {
+ LONG waiters;
+ CRITICAL_SECTION waiters_lock;
+ HANDLE sema;
+} pthread_cond_t;
+
+#define PTHREAD_COND_INITIALIZER { 0, { 0 }, NULL }
+
+static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)
+{
+ cond->waiters = 0;
+
+ InitializeCriticalSection(&cond->waiters_lock);
+
+ cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
+ if (NULL == cond->sema)
+ return -1;
+ return 0;
+}
+
+static __inline int pthread_cond_destroy(pthread_cond_t *cond)
+{
+ CloseHandle(cond->sema);
+ cond->sema = NULL;
+
+ DeleteCriticalSection(&cond->waiters_lock);
+
+ return 0;
+}
+
+static __inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
+{
+ int ret = 0;
+
+ /* serialize access to waiters count */
+ EnterCriticalSection(&cond->waiters_lock);
+ ++cond->waiters;
+ LeaveCriticalSection(&cond->waiters_lock);
+
+ /*
+ * Unlock external mutex and wait for signal.
+ * NOTE: we've held mutex locked long enough to increment
+ * waiters count above, so there's no problem with
+ * leaving mutex unlocked before we wait on semaphore.
+ */
+ LeaveCriticalSection(mutex);
+
+ /* let's wait */
+ if (0 != WaitForSingleObject(cond->sema, INFINITE))
+ ret = -1;
+
+ /* we're done waiting, so make sure we decrease waiters count */
+ EnterCriticalSection(&cond->waiters_lock);
+ --cond->waiters;
+ LeaveCriticalSection(&cond->waiters_lock);
+
+ /* lock external mutex again */
+ EnterCriticalSection(mutex);
+
+ return ret;
+}
+
+static __inline int pthread_cond_signal(pthread_cond_t *cond)
+{
+ int have_waiters;
+
+ /* serialize access to waiters count */
+ EnterCriticalSection(&cond->waiters_lock);
+ have_waiters = cond->waiters > 0;
+ LeaveCriticalSection(&cond->waiters_lock);
+
+ /*
+ * Signal only when there are waiters
+ */
+ if (have_waiters)
+ return ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;
+ else
+ return 0;
+}
+
+#define pthread_t HANDLE
+#define pthread_mutex_t CRITICAL_SECTION
+
+#define PTHREAD_MUTEX_INITIALIZER { 0 }
+
+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))
+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))
+#define pthread_mutex_lock EnterCriticalSection
+#define pthread_mutex_unlock LeaveCriticalSection
+
+static __inline int pthread_create(pthread_t *t, const void *unused, DWORD (__stdcall *start_routine)(LPVOID), void *arg)
+{
+ *t = CreateThread(NULL, 0, start_routine, arg, 0, NULL);
+
+ if (NULL == *t) {
+ errno = err_win_to_posix(GetLastError());
+ return -1;
+ } else {
+ errno = 0;
+ return 0;
+ }
+}
+
+static __inline int pthread_join(pthread_t t, void **unused)
+{
+ DWORD result = WaitForSingleObject(t, INFINITE);
+ switch (result) {
+ case WAIT_OBJECT_0:
+ errno = 0;
+ return 0;
+ case WAIT_ABANDONED:
+ errno = EINVAL;
+ return -1;
+ default:
+ errno = err_win_to_posix(GetLastError());
+ return -1;
+ }
+}
+
+#endif /* PTHREAD_H */
+
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..202b90e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -464,4 +464,17 @@ void git_qsort(void *base, size_t nmemb, size_t size,
*/
int unlink_or_warn(const char *path);
+/*
+ * Properly defines thread routine for Windows and POSIX
+ */
+#ifndef NO_PTHREADS
+# ifndef _WIN32
+# define THREAD_FUNC(f, a) void *f(void *a)
+# define THREAD_RETURN(x) return (x)
+# else
+# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)
+# define THREAD_RETURN(x) return (DWORD)(x);
+# endif
+#endif
+
#endif
diff --git a/preload-index.c b/preload-index.c
index 9289933..ace10fe 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -28,7 +28,7 @@ struct thread_data {
int offset, nr;
};
-static void *preload_thread(void *_data)
+static THREAD_FUNC(preload_thread, _data)
{
int nr;
struct thread_data *p = _data;
@@ -59,7 +59,7 @@ static void *preload_thread(void *_data)
continue;
ce_mark_uptodate(ce);
} while (--nr > 0);
- return NULL;
+ THREAD_RETURN(NULL);
}
static void preload_index(struct index_state *index, const char **pathspec)
--
1.6.5.2
^ permalink raw reply related
* Re: Accessing the reflog remotely
From: Emanuele Aina @ 2009-11-04 10:16 UTC (permalink / raw)
To: git
In-Reply-To: <vpqljimpr95.fsf@bauges.imag.fr>
Matthieu Moy provò:
> git checkout 'origin/master@{Nov 3 00:00:00}'
>
> But this tells me where _my_ local master was on that date (i.e. the
> last revision I had pulled).
>
> So, the best I can think of is:
>
> ssh host 'cd /repo/ ; git tag final-version "master@{Nov 3 00:00:00}"'
> git fetch --tags
> git checkout tags/final-version
At least you can avoid the tag:
git checkout `ssh host 'GIT_DIR=/repo/ git rev-parse "master@{Nov 3 00:00:00}"'`
--
Buongiorno.
Complimenti per l'ottima scelta.
^ permalink raw reply
* Accessing the reflog remotely
From: Matthieu Moy @ 2009-11-04 9:35 UTC (permalink / raw)
To: git
Hi,
I guess the answer is "no", but I'll still ask in case ...
Is it possible to access the reflog of a remote repository other than
logging into this repository?
My use-case is the following: I want to checkout "the last revision
pushed in master on ssh://host/repo/ on day D at midnight" (to fetch
the project of my students ;-) ). If it were locally, I'd do
git checkout 'origin/master@{Nov 3 00:00:00}'
But this tells me where _my_ local master was on that date (i.e. the
last revision I had pulled).
So, the best I can think of is:
ssh host 'cd /repo/ ; git tag final-version "master@{Nov 3 00:00:00}"'
git fetch --tags
git checkout tags/final-version
Is there a better way?
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Tim Mazid @ 2009-11-04 9:08 UTC (permalink / raw)
To: git
In-Reply-To: <20091104072709.GC24263@coredump.intra.peff.net>
Jeff King wrote:
>
> On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:
>
>> On Wed, Nov 4, 2009 at 07:17, Tim Mazid <timmazid@hotmail.com> wrote:
>> > So instead of invoking 'git checkout REMOTE/BRANCH', do 'git checkout
>> -b
>> > BRANCH REMOTE/BRANCH'.
>>
>> Automagically doing 'git checkout -t remote/branch' when asked to do
>> 'git checkout remote/branch' was suggested earlier on the list and I
>> think there was even a patch that implemented it, not sure what the
>> outcome of the series was. I do remember that Peff was annoyed by it
>> at the GitTogether though so it might be a bad idea.
>
> It's in 'next' now. And for the record, my complaint about its behavior
> turned out to be partially because I was an idiot. I am still not
> convinced that we won't later regret leaving the stale local branch
> sitting around, or that users won't find it confusing to see:
>
> $ git checkout foo
> Branch foo set up to track remote branch foo from origin.
> Switched to a new branch 'foo'
>
> ... time passes ...
>
> $ git checkout foo
> Switched to branch 'foo'
> Your branch is behind 'origin/foo' by 1 commit, and can be
> fast-forwarded.
>
Hm. I actually meant inside gitk, not git itself. As in, when you click
inside gitk and try to checkout a remote, it automatically creates a
tracking branch and checks THAT out instead, whereas command-line git works
the same way.
Does that even make sense? :P
--
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3943894.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* What happened to [PATCH] Support "core.excludesfile = ~/.gitignore" ?
From: Matthieu Moy @ 2009-11-04 8:56 UTC (permalink / raw)
To: git, quarl
Hi,
I found a patch to manage ~ in core.excludesfile here:
http://thread.gmane.org/gmane.comp.version-control.git/93250
It seems the patch was abandonned by its author near here:
http://thread.gmane.org/gmane.comp.version-control.git/93250/focus=94318
Was there any particular reason for dropping the patch other than lack
of time? If not, I'll try to resurect it.
(I have core.excludesfile = /home/moy/.gitignore hardcoded, and I have
to edit it each time I teach Git to a colleague, that would be so much
better to have the same value "~/.gitignore" for everybody ...)
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Problem with "From:" line on "git format-patch" generated patches
From: Jeff King @ 2009-11-04 8:49 UTC (permalink / raw)
To: André Goddard Rosa; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <b8bf37780911031006q4bc4c487xd9db86eb0fa531e7@mail.gmail.com>
On Tue, Nov 03, 2009 at 04:06:39PM -0200, André Goddard Rosa wrote:
> I'm not using any specific tool for inputting the git-format-patch,
> but instead I'm sending the files generated by it through gmail as an
> inlined patch in the email body.
>
> I like the convenience of format-patch for generating the patch files,
> but in this case, formatting the header as rfc2047 is not necessary
> and makes a funny/garbled output in my patch submission.
>
> Do you have a suggestion for my workflow?
I don't think there's currently a way to turn off the rfc2047 from
within format-patch. You can generate a single patch with the same
format using:
git log -1 -p --stat --summary \
--pretty=tformat:'From: %an <%ae>%nDate: %aD%nSubject: [PATCH] %s%n%n%b'
but it won't do nice things like putting one patch in each file.
Probably it would make sense for format-patch to have an option to
indicate that you are going to inline these patches into a different
MUA. So drop the 'From' mbox header line, don't rfc2047 encode, and
maybe some other behaviors. I do the same thing (including inline in
mutt), but I just delete the unwanted lines manually, and fortunately my
name doesn't contain any non-ascii characters. ;)
-Peff
^ permalink raw reply
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Michael Wookey @ 2009-11-04 8:48 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Andrzej K. Haczewski, git
In-Reply-To: <4AF13819.7050306@viscovery.net>
2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> Andrzej K. Haczewski schrieb:
>
>> +static __inline int win32_cond_init(win32_cond_t *cond)
>> +{
>> + cond->waiters = 0;
>> +
>> + InitializeCriticalSection(&cond->waiters_lock);
>> +
>> + cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
>
> Wouldn't an Event object be lighter-weight? (I'm only guessing.)
Both events and semaphores resolve to wait-able kernel objects; so
neither is "lighter-weight" than the other.
^ permalink raw reply
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Johannes Sixt @ 2009-11-04 8:24 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git
In-Reply-To: <16cee31f0911032344m3263730l607c02eb4e9adef5@mail.gmail.com>
[please don't cull Cc list on this ML]
Andrzej K. Haczewski schrieb:
>> Pardon my ignorance, but is there a reason to not use Pthreads for Win32?
>> http://sourceware.org/pthreads-win32/
>>
>
> Not using pthreads on Windows makes Git:
> 1. faster on that platform
I believe this only if you present hard numbers. My guess is that (for
example) packing objects with two threads is still faster with a slow
pthreads emulation than without threading at all.
> 2. not depend on Pthreads for Win32
Why is this an advantage?
> IMHO that makes Git one step closer to become native on Windows, and
> is a sensible step.
Emulating pthreads on Windows with all its facets is an extremely
difficult task. If exact POSIX conformance is needed, I would choose an
existing package over doing it myself at any time.
Granted, we don't need the esoteric parts (cancelation points), which
would simplify the emulation a lot. But, as I pointed out in my other
mail, even a pthread_cond_wait() is not that trivial to implement with the
Windows API.
-- Hannes
^ 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