* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 12:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Nicolas Pitre, kusmabite
In-Reply-To: <4AF2C4B9.10402@viscovery.net>
2009/11/5 Johannes Sixt <j.sixt@viscovery.net>:
> Elsewhere we use _beginthreadex(). What's the difference?
Oh my, I've just run through MSDN documentation and I'm wrongly using
CreateThread. I should have used _beginthread from the start, because
besides calling CreateThread it initializes local thread storage for
global C-runtime variables (errno etc.), which CreateThread does not
do. That could lead to very unpleasant consequences. I'll redo thread
creation to use _beginthreadex(). Thanks for that question!
> The pthread_cond_* functions are quite voluminous, but not performance
> critical. Could you please move them to pthread.c as well?
Ok, will do.
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Johannes Sixt @ 2009-11-05 12:27 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git, Nicolas Pitre, kusmabite
In-Reply-To: <1257416325-5605-1-git-send-email-ahaczewski@gmail.com>
Andrzej K. Haczewski schrieb:
> diff --git a/Makefile b/Makefile
> index bc039ac..30089a8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -453,6 +453,7 @@ LIB_H += commit.h
> LIB_H += compat/bswap.h
> LIB_H += compat/cygwin.h
> LIB_H += compat/mingw.h
> +LIB_H += compat/win32/pthread.h
> LIB_H += csum-file.h
> LIB_H += decorate.h
> LIB_H += delta.h
> @@ -971,15 +972,15 @@ 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
> AR = compat/vcbuild/scripts/lib.pl
> 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_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o
> + 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 =
What compiles compat/win32/pthread.c?
Please don't forget to add compat/win32/*.o to the clean target.
> +int pthread_create(pthread_t *thread, const void *unused,
> + void *(*start_routine)(void*), void *arg)
> +{
> + thread->arg = arg;
> + thread->handle = CreateThread(NULL, 0, win32_start_routine, thread, 0, NULL);
Elsewhere we use _beginthreadex(). What's the difference?
> +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 (!cond->sema)
> + die("");
> + 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)
> +{
> + /* 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 */
> + WaitForSingleObject(cond->sema, INFINITE))
> +
> + /* 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 0;
> +}
> +
> +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 : err_win_to_posix(GetLastError();
> + else
> + return 0;
> +}
The pthread_cond_* functions are quite voluminous, but not performance
critical. Could you please move them to pthread.c as well?
-- Hannes
^ permalink raw reply
* [PATCH] pre-commit.sample: Diff against the empty tree when HEAD is invalid
From: Björn Steinbrink @ 2009-11-05 10:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, git
This was already the case for the old "diff --check" call, but the new
one that checks whether there are any non-ascii file names was missing
it, making that check fail for root commits.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
templates/hooks--pre-commit.sample | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index 043970a..439eefd 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -7,6 +7,14 @@
#
# To enable this hook, rename this file to "pre-commit".
+if git-rev-parse --verify HEAD >/dev/null 2>&1
+then
+ against=HEAD
+else
+ # Initial commit: diff against an empty tree object
+ against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+fi
+
# If you want to allow non-ascii filenames set this variable to true.
allownonascii=$(git config hooks.allownonascii)
@@ -17,7 +25,7 @@ if [ "$allownonascii" != "true" ] &&
# Note that the use of brackets around a tr range is ok here, (it's
# even required, for portability to Solaris 10's /usr/bin/tr), since
# the square bracket bytes happen to fall in the designated range.
- test "$(git diff --cached --name-only --diff-filter=A -z |
+ test "$(git diff --cached --name-only --diff-filter=A -z $against |
LC_ALL=C tr -d '[ -~]\0')"
then
echo "Error: Attempt to add a non-ascii file name."
@@ -35,12 +43,4 @@ then
exit 1
fi
-if git-rev-parse --verify HEAD >/dev/null 2>&1
-then
- against=HEAD
-else
- # Initial commit: diff against an empty tree object
- against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-fi
-
exec git diff-index --check --cached $against --
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 10:37 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <4AF2A69F.1090802@lyx.org>
Abdelrazak Younes wrote:
> Abdelrazak Younes wrote:
> > Marco Costalba wrote:
> >>
> >>> uint qHash(const ShaString& s) { // fast path, called 6-7 times
> >>> per revision
> >>>
> >>>
> >>
> >> Function:
> >>
> >> uint qHash(const QByteArray&);
> >>
> >> is already defined in the Qt Core libraries, so I have a link
> >> error with your patch.
> >>
> >
> > By the way, this function of yours is not used anywhere AFAICS.
>
> Ok, now I understand that this is used by QHash, sorry.
>
> I have gcc-4.4.1 so maybe that's the reason why linking works in my
> case. But I don't which version of the qhash() function does take
> precedence...
FYI I just verified that your version does take precedence over the Qt one.
Anyway, if you like the patch, we could just inherit from QByteArray
instead of typedef it so that it links with your system.
Out of curiosity I just had a look at the two versions, yours:
********************************
// definition of an optimized sha hash function
static inline uint hexVal(const uchar* ch) {
return (*ch < 64 ? *ch - 48 : *ch - 87);
}
uint qHash(const ShaString& s) { // fast path, called 6-7 times per revision
const uchar* ch = reinterpret_cast<const uchar*>(s.data());
return (hexVal(ch ) << 24)
+ (hexVal(ch + 2) << 20)
+ (hexVal(ch + 4) << 16)
+ (hexVal(ch + 6) << 12)
+ (hexVal(ch + 8) << 8)
+ (hexVal(ch + 10) << 4)
+ hexVal(ch + 12);
}
********************************
And Qt's version:
********************************
static uint hash(const uchar *p, int n)
{
uint h = 0;
uint g;
while (n--) {
h = (h << 4) + *p++;
if ((g = (h & 0xf0000000)) != 0)
h ^= g >> 23;
h &= ~g;
}
return h;
}
uint qHash(const QBitArray &bitArray)
{
int m = bitArray.d.size() - 1;
uint result = hash(reinterpret_cast<const uchar
*>(bitArray.d.constData()), qMax(0, m));
// deal with the last 0 to 7 bits manually, because we can't trust that
// the padding is initialized to 0 in bitArray.d
int n = bitArray.size();
if (n & 0x7)
result = ((result << 4) + bitArray.d.at(m)) & ((1 << n) - 1);
return result;
}
********************************
I recompiled qgit with the Qt version and I didn't notice any
performance problem with a big repo (Qt).
Just tell me if this is not interesting to you and I'll shut up :-)
Abdel.
^ permalink raw reply
* [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 10:18 UTC (permalink / raw)
To: git; +Cc: Nicolas Pitre, kusmabite, Johannes Sixt, Andrzej K. Haczewski
In-Reply-To: <16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com>
This patch implements native to Windows subset of pthreads API used by Git.
It allows to remove Pthreads for Win32 dependency for msysgit and Cygwin.
The patch modifies Makefile only for MSVC (that's the environment I'm
capable of testing on), so it requires further corrections to compile
with MinGW or Cygwin.
Signed-off-by: Andrzej K. Haczewski <ahaczewski@gmail.com>
---
Makefile | 7 ++-
builtin-pack-objects.c | 31 ++++++++++--
compat/mingw.c | 2 +-
compat/mingw.h | 5 ++
compat/win32/pthread.c | 39 +++++++++++++++
compat/win32/pthread.h | 125 ++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 201 insertions(+), 8 deletions(-)
create mode 100644 compat/win32/pthread.c
create mode 100644 compat/win32/pthread.h
diff --git a/Makefile b/Makefile
index bc039ac..30089a8 100644
--- a/Makefile
+++ b/Makefile
@@ -453,6 +453,7 @@ LIB_H += commit.h
LIB_H += compat/bswap.h
LIB_H += compat/cygwin.h
LIB_H += compat/mingw.h
+LIB_H += compat/win32/pthread.h
LIB_H += csum-file.h
LIB_H += decorate.h
LIB_H += delta.h
@@ -971,15 +972,15 @@ 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
AR = compat/vcbuild/scripts/lib.pl
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_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o
+ 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..00594fd 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,
#ifdef THREADED_DELTA_SEARCH
-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t read_mutex;
#define read_lock() pthread_mutex_lock(&read_mutex)
#define read_unlock() pthread_mutex_unlock(&read_mutex)
-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t cache_mutex;
#define cache_lock() pthread_mutex_lock(&cache_mutex)
#define cache_unlock() pthread_mutex_unlock(&cache_mutex)
-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t progress_mutex;
#define progress_lock() pthread_mutex_lock(&progress_mutex)
#define progress_unlock() pthread_mutex_unlock(&progress_mutex)
@@ -1590,7 +1590,26 @@ struct thread_params {
unsigned *processed;
};
-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;
+static pthread_cond_t progress_cond;
+
+/*
+ * Mutex and conditional variable can't be statically-initialized on Windows.
+ */
+static void init_threaded_search()
+{
+ pthread_mutex_init(&read_mutex, NULL);
+ pthread_mutex_init(&cache_mutex, NULL);
+ pthread_mutex_init(&progress_mutex, NULL);
+ pthread_cond_init(&progress_cond, NULL);
+}
+
+static void cleanup_threaded_search()
+{
+ pthread_cond_destroy(&progress_cond);
+ pthread_mutex_destroy(&read_mutex);
+ pthread_mutex_destroy(&cache_mutex);
+ pthread_mutex_destroy(&progress_mutex);
+}
static void *threaded_find_deltas(void *arg)
{
@@ -1629,8 +1648,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
struct thread_params *p;
int i, ret, active_threads = 0;
+ init_threaded_search();
+
if (delta_search_threads <= 1) {
find_deltas(list, &list_size, window, depth, processed);
+ cleanup_threaded_search();
return;
}
if (progress > pack_to_stdout)
@@ -1745,6 +1767,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
active_threads--;
}
}
+ cleanup_threaded_search();
free(p);
}
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/mingw.h b/compat/mingw.h
index 6907345..7e25fb5 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -294,3 +294,8 @@ struct mingw_dirent
#define readdir(x) mingw_readdir(x)
struct dirent *mingw_readdir(DIR *dir);
#endif // !NO_MINGW_REPLACE_READDIR
+
+/*
+ * Used by Pthread API implementation for Windows
+ */
+extern int err_win_to_posix(DWORD winerr);
diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
new file mode 100644
index 0000000..a7ab7af
--- /dev/null
+++ b/compat/win32/pthread.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2009 Andrzej K. Haczewski <ahaczewski@gmail.com>
+ */
+
+#include "pthread.h"
+
+static DWORD __stdcall win32_start_routine(LPVOID arg)
+{
+ pthread_t *thread = arg;
+ thread->value = thread->start_routine(thread->arg);
+ return 0;
+}
+
+int pthread_create(pthread_t *thread, const void *unused,
+ void *(*start_routine)(void*), void *arg)
+{
+ thread->arg = arg;
+ thread->handle = CreateThread(NULL, 0, win32_start_routine, thread, 0, NULL);
+
+ if (!thread->handle)
+ return err_win_to_posix(GetLastError());
+ else
+ return 0;
+}
+
+int win32_pthread_join(pthread_t *thread, void **value_ptr)
+{
+ DWORD result = WaitForSingleObject(thread->handle, INFINITE);
+ switch (result) {
+ case WAIT_OBJECT_0:
+ if (value_ptr)
+ *value_ptr = thread.value;
+ return 0;
+ case WAIT_ABANDONED:
+ return EINVAL;
+ default:
+ return err_win_to_posix(GetLastError());
+ }
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
new file mode 100644
index 0000000..e50eb6b
--- /dev/null
+++ b/compat/win32/pthread.h
@@ -0,0 +1,125 @@
+/*
+ * 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>
+
+/*
+ * Implement simple condition variable for Windows threads, based on ACE
+ * implementation.
+ *
+ * See original implementation: http://bit.ly/1vkDjo
+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html
+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html
+ */
+typedef struct {
+ LONG waiters;
+ CRITICAL_SECTION waiters_lock;
+ HANDLE sema;
+} pthread_cond_t;
+
+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 (!cond->sema)
+ die("");
+ 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)
+{
+ /* 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 */
+ WaitForSingleObject(cond->sema, INFINITE))
+
+ /* 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 0;
+}
+
+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 : err_win_to_posix(GetLastError();
+ else
+ return 0;
+}
+
+#define pthread_mutex_t CRITICAL_SECTION
+
+#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
+
+typedef struct {
+ HANDLE handle;
+ void *(*start_routine)(void*);
+ void *arg;
+ void *value;
+} pthread_t;
+
+extern int pthread_create(pthread_t *thread, const void *unused,
+ void *(*start_routine)(void*), void *arg);
+
+/*
+ * To avoid the need of allocating struct, we use small macro wrapper to pass
+ * pointer to win32_pthread_join instead of using typedef struct {} *pthread_t
+ */
+#define pthread_join(a, b) win32_pthread_join(&(a), (b))
+
+extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
+
+#endif /* PTHREAD_H */
--
1.6.5.2
^ permalink raw reply related
* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 10:19 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <4AF2A538.7040303@lyx.org>
Abdelrazak Younes wrote:
> Marco Costalba wrote:
>>
>>> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
>>> revision
>>>
>>>
>>
>> Function:
>>
>> uint qHash(const QByteArray&);
>>
>> is already defined in the Qt Core libraries, so I have a link error
>> with your patch.
>>
>
> By the way, this function of yours is not used anywhere AFAICS.
Ok, now I understand that this is used by QHash, sorry.
I have gcc-4.4.1 so maybe that's the reason why linking works in my
case. But I don't which version of the qhash() function does take
precedence...
Abdel.
^ permalink raw reply
* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 10:13 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911050141t751d45a0r4e340fa0d10af366@mail.gmail.com>
Marco Costalba wrote:
>
>> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
>> revision
>>
>>
>
> Function:
>
> uint qHash(const QByteArray&);
>
> is already defined in the Qt Core libraries, so I have a link error
> with your patch.
>
By the way, this function of yours is not used anywhere AFAICS.
Abdel.
^ permalink raw reply
* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 9:50 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911050141t751d45a0r4e340fa0d10af366@mail.gmail.com>
Marco Costalba wrote:
> Hi Abdel,
>
> On Wed, Nov 4, 2009 at 15:56, Abdelrazak Younes <younes@lyx.org> wrote:
>
>> Hello Marco,
>>
>> While recompiling latest qgit4, I stumbled accross this. I am not quite sure
>> you used a QLatin1String instead of a QByteArray but the attached seems to
>> work fine...
>>
>>
>
> Unfortunatly I cannot say the same here ;-)
>
>
>
>> -class ShaString;
>> +typedef QByteArray ShaString;
>>
>
> ...... cut ......
>
>
>> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
>> revision
>>
>>
>
> Function:
>
> uint qHash(const QByteArray&);
>
> is already defined in the Qt Core libraries, so I have a link error
> with your patch.
>
Weird... it links just fine here... anyway this can be solved by
renaming your version. Or just using the Qt version if that does the
same thing ;-)
> BTW I don't think I have understood the reason of your patch. Do you
> have a compile error or something ?
>
No, I had some warnings so I looked at the code and I just thought that
QLatin1String was not appropriate here and overkill. And QByteArray
should be faster...
Anyway, this was just FYI, I don't think this patch is important at all :-)
Abdel.
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Erik Faye-Lund @ 2009-11-05 9:41 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: Nicolas Pitre, git, Johannes Sixt
In-Reply-To: <16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com>
On Thu, Nov 5, 2009 at 10:00 AM, Andrzej K. Haczewski
<ahaczewski@gmail.com> wrote:
>
> That way we don't need allocations to simulate pthread init/join API
Yay! By the way, I love the work you're doing here. Getting threaded
delta-searching on Windows is something I'm looking forward to :)
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [QGIT PATCH/RFC]
From: Marco Costalba @ 2009-11-05 9:41 UTC (permalink / raw)
To: Abdelrazak Younes; +Cc: git
In-Reply-To: <4AF19630.2070402@lyx.org>
Hi Abdel,
On Wed, Nov 4, 2009 at 15:56, Abdelrazak Younes <younes@lyx.org> wrote:
> Hello Marco,
>
> While recompiling latest qgit4, I stumbled accross this. I am not quite sure
> you used a QLatin1String instead of a QByteArray but the attached seems to
> work fine...
>
Unfortunatly I cannot say the same here ;-)
>-class ShaString;
>+typedef QByteArray ShaString;
...... cut ......
>
> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
> revision
>
Function:
uint qHash(const QByteArray&);
is already defined in the Qt Core libraries, so I have a link error
with your patch.
BTW I don't think I have understood the reason of your patch. Do you
have a compile error or something ?
Thanks
Marco
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 9:00 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: kusmabite, git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911042111270.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
> What about:
>
> typedef struct {
> HANDLE handle;
> void *(*start_routine)(void *);
> void *arg;
> } pthread_t;
>
> DWORD __stdcall windows_thread_start(LPVOID _self)
> {
> pthread_t *self = _self;
> void *ret = self->start_routine(self->arg);
> return (DWORD)ret;
> }
>
> static inline int pthread_create(pthread_t *thread, const void *unused,
> void *(*start_routine)(void *), void *arg)
> {
> thread->handle = CreateThread(NULL, 0, windows_thread_start,
> thread, 0, NULL);
> [...]
> }
The problem I see is not with pthread_init, but pthread_join. Here's
how it looks:
int pthread_join(pthread_t thread, void **value_ptr);
If pthread_t would be a struct, then we can't call pthread_join like
that... At least that's what I though yesterday, but maybe it can be
done like this:
int win32_pthread_join(pthread_t *thread, void **value_ptr)
{
[...]
}
#define pthread_join(a, b) win32_pthread_join(&(a), (b))
That way we don't need allocations to simulate pthread init/join API
> And thread creation is a relatively rare event compared to e.g. mutex
> lock/unlock, so the indirection shouldn't be noticeable. For the same
> reason, I also think that you could make pthread_create() and
> pthread_join() into a C file instead of being inlined which would reduce
> the code footprint at every call site, and allow for only one instance
> of windows_thread_start() which could then be made static.
Yeah, I'll factor that out to separate file.
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 8:51 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911041915120.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> Careful. At the beginning of the function you'll find:
>
> if (delta_search_threads <= 1) {
> find_deltas(list, &list_size, window, depth, processed);
> return;
> }
>
> That is, if we have thread support compiled in but we're told to use
> only one thread, then the bulk of the work splitting is bypassed
> entirely. Inside find_deltas() there will still be pthread_mutex_lock()
> and pthread_mutex_unlock() calls even if no threads are spawned.
Ah, I wasn't aware of that. Actually why would find_deltas lock if no
threads are used? Maybe, for non-threaded call to find_deltas, locking
could be factored out?
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 8:45 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911042039200.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> Please use die("CreateSemaphore() failed") in the failure case instead
> of returning success.
Sure, will do.
> However, my pthread_cond_init man page says:
And that is weird, because mine man page says:
[[[
pthread_cond_init, pthread_cond_signal, pthread_cond_broadcast, and
pthread_cond_wait never return an error code.
]]]
--
Andrzej
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #01; Wed, 04)
From: Johannes Sixt @ 2009-11-05 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <7vmy31a5p6.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> * rj/maint-simplify-cygwin-makefile (2009-10-27) 1 commit.
> - Makefile: merge two Cygwin configuration sections into one
>
> This is one of the most obviously correct bit from "Compiling on Cygwin
> using MSVC fails" topic I didn't really look at. If J6t is Ok with the
> series, I don't mind queueing the whole thing myself.
I'm actually expecting a resend of the remaining patches in the series
with updated commit messages and with the original patch 3/4 being first.
-- Hannes
^ permalink raw reply
* Re: Problem signing a tag
From: Joshua J. Kugler @ 2009-11-05 8:37 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Alex Riesen, git
In-Reply-To: <4AF28CE4.5000906@drmicha.warpmail.net>
On Wednesday 04 November 2009, Michael J Gruber said something like:
> > So, the docs consider 2 a fatal error, even though it appears it
> > isn't. It seems that
> > http://github.com/git/git/blob/a6dbf8814f433a7fbfa9cde6333c98019f6d
> >b1e4/builtin-tag.c#L202 needs to be patched to something along the
> > lines of:
> >
> > rv = finish_command(&gpg)
> > if ((rv && rv !=2) || !len || len < 0)
> >
> > Probably digging in to the gpg source code to figure out what
> > errors are and aren't fatal would be in order.
> >
> > Thanks again for your help! Glad to know what I needed to do to
> > sign my tags!
>
> Dig dig dig... gpg exits with 2 in a lot of cases, one would need to
> parse fd-error to find out more. But it also looks as if gpg exits
> normally with a good passphrase. So I tried, and at least with gpg
> 1.4.9 and git 1.6.5.2 I can sign tags with "use-agent" and without a
> running agent: I get asked for the passphrase (after reporting the
> agent MIA), and everything's fine.
>
> My gpg returns 0 in this case; it returns 2 only if I don't enter the
> passphrase. So, this seems to depend on the version of gpg. Or on
> entering the correct passphrase ;)
>
> Michael
That is weird. Because when working from the prompt (with agent MIA),
gpg 1.4.9, it would accept my pass phrase, and would print the
signature (either binary or ascii armored), but it will still exit with
2. I don't understand it. I'll pop on #gnupg tomorrow and ask about
it.
Thanks again for your help with this!
j
--
Joshua Kugler
Part-Time System Admin/Programmer
http://www.eeinternet.com
PGP Key: http://pgp.mit.edu/ ID 0x14EA086E
^ permalink raw reply
* Re: Problem signing a tag
From: Michael J Gruber @ 2009-11-05 8:29 UTC (permalink / raw)
To: Joshua J. Kugler; +Cc: Michael J Gruber, Alex Riesen, git
In-Reply-To: <200911040947.50226.joshua@eeinternet.com>
Joshua J. Kugler venit, vidit, dixit 04.11.2009 19:47:
> On Wednesday 04 November 2009, Michael J Gruber said something like:
>>> gpg: problem with the agent - disabling agent use
>>> error: gpg failed to sign the tag
>>> error: unable to sign the tag
>>> $ echo $?
>>> 128
>>>
>>> And when I sign at the prompt:
>>>
>>> $ gpg -sa
>>>
>>> You need a passphrase to unlock the secret key for
>>> user: "Joshua J. Kugler <joshua@azariah.com>"
>>> 1024-bit DSA key, ID 14EA086E, created 2009-08-09
>>>
>>> gpg: problem with the agent - disabling agent use
>>> Blah blah blah blah
>>> Blah blah blah blah
>>> $ echo $?
>>> 2
>>
>> [...]
>>
>> I assume you don't want to use gpg-agent, that should be the easy way
>> out.
>
> Well, I could, but I just haven't set it up. :)
>
>> If that helps you can put "--no-use-agent" in your gpg config.
>
> I commented out use-agent in the config. That worked. THANKS!
>
>> 2 is a non-fatal error, 128 a fatal one, btw.
>
> Well, the 2 was from running gpg alone, and 128 was from git erroring
> out. According to the gpg docs:
>
> "The program returns 0 if everything was fine, 1 if at least a signature
> was bad, and other error codes for fatal errors."
>
> So, the docs consider 2 a fatal error, even though it appears it isn't.
> It seems that
> http://github.com/git/git/blob/a6dbf8814f433a7fbfa9cde6333c98019f6db1e4/builtin-tag.c#L202
> needs to be patched to something along the lines of:
>
> rv = finish_command(&gpg)
> if ((rv && rv !=2) || !len || len < 0)
>
> Probably digging in to the gpg source code to figure out what errors are
> and aren't fatal would be in order.
>
> Thanks again for your help! Glad to know what I needed to do to sign my
> tags!
Dig dig dig... gpg exits with 2 in a lot of cases, one would need to
parse fd-error to find out more. But it also looks as if gpg exits
normally with a good passphrase. So I tried, and at least with gpg 1.4.9
and git 1.6.5.2 I can sign tags with "use-agent" and without a running
agent: I get asked for the passphrase (after reporting the agent MIA),
and everything's fine.
My gpg returns 0 in this case; it returns 2 only if I don't enter the
passphrase. So, this seems to depend on the version of gpg. Or on
entering the correct passphrase ;)
Michael
^ permalink raw reply
* Re: Automatically remote prune
From: John Tapsell @ 2009-11-05 8:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7viqdpemki.fsf@alter.siamese.dyndns.org>
2009/11/5 Junio C Hamano <gitster@pobox.com>:
> John Tapsell <johnflux@gmail.com> writes:
> "what the benefits are to give this information _in the 'branch' output_"
> was what I meant. From the part you omitted from my message:
I omitted it just because, imho, it's not what I 'care about'. I'm
not trying to help advanced users (Users that _want_ to keep
remotes/origin/* clean and users that _want_ to be careful to not lose
commits are both advanced users, imho). I'm just interested in
reducing confusion for non-advanced users. So either not-showing
removed remote branches by default, or showing them but marking them
as deleted.
> A better approach to please the first class of audience may be to
> introduce an option that tells fetch to cull tracking refs that are stale.
> Then "branch -r" output will not show stale refs and there is no place
> (nor need) to show [Deleted] labels.
If it's a non-default option, then it won't help the non-advanced users.
> Such an option won't be very useful for the second class of audience,
> though. For them we would need something else, and it would likely be an
> enhancement to "git remote".
Which still leaves confusion when viewing "git branch -r" since they
would show up there still.
John
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Johannes Sixt @ 2009-11-05 7:55 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1E1FD.1050102@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Junio C Hamano wrote:
>> Shouldn't this be solved by teaching the Makefile about this new "Cygwin
>> but using MSVC as compiler toolchain" combination?
>
> Yes. Err... see patch #3 :-P
A clarifiction: Junio talks about using the MSVC tools to build Cygwin
programs, that is, to merely substitute Cygwin's gcc by MSVC, but to still
link against cygwin's C runtime.
But this is not what this series is about.
When the "MSVC build" of git is made, then the MSVC compiler is used, and
this will always use Microsoft libraries in the resulting executables,
regardless of whether "make MSVC=1" was called from a "cygwin environment"
or from and "msys environment".
This series is about fixing "make MSVC=1" when it is run from a "cygwin
environment" by disentangling the MSVC and Cygwin configuration sections
in the Makefile.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 13/13] Add Python support library for remote helpers
From: Johan Herland @ 2009-11-05 7:55 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar
In-Reply-To: <1257364098-1685-14-git-send-email-srabbelier@gmail.com>
On Wednesday 04 November 2009, Sverre Rabbelier wrote:
> This patch introduces parts of a Python package called
> "git_remote_helpers" containing the building blocks for
> python helper.
> The actual remote helpers itself are NOT part of this patch.
>
> The patch includes the necessary Makefile additions to build and install
> the git_remote_cvs Python package along with the rest of Git.
There are a few typos in the above. How about:
This patch introduces parts of a Python package called
"git_remote_helpers" containing the building blocks for
remote helpers written in Python.
No actual remote helpers are part of this patch, the patch
only includes the common basics needed to start writing such
helpers.
> The patch includes the necessary Makefile additions to build and install
> the git_remote_cvs Python package along with the rest of Git.
s/git_remote_cvs/git_remote_helpers/
> This patch is based on Johan Herland's git_remote_cvs patch and
> has been improved by the following contributions:
> - David Aguilar: Lots of Python coding style fixes
> - David Aguilar: DESTDIR support in Makefile
>
> Cc: David Aguilar <davvid@gmail.com>
> Cc: Johan Herland <johan@herland.net>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
Otherwise it all looks good. Nice work!
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Jeff King @ 2009-11-05 7:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Tim Mazid, git
In-Reply-To: <7vhbtai2uy.fsf@alter.siamese.dyndns.org>
On Wed, Nov 04, 2009 at 10:03:49AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > 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.
>
> Isn't it quite different? What's in 'next' for 1.7.0 is to guess the
> user's intention when:
Sorry, yes, I just saw Sverre's comment and misread the original
proposal. Checking out "$remote/$branch" will still detach the HEAD,
and I don't think anybody has a previous proposal to change that.
> I think this is primarily because the way this DWIM is totally silent in
> the transcript is misleading. If you explain it the way I outlined above,
> I do not think there is any confusion. That is, there is no way for the
> user to get confused if the command sequence were like so:
>
> $ git branch -t foo origin/foo
> Branch foo set up to track remote branch foo from origin.
> $ git checkout foo
> 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.
>
> It could just be a matter of telling what we are doing a bit more
> explicitly when this DWIM kicks in. How about this?
>
> $ git checkout foo
> (first forking your own 'foo' from 'origin/foo')
> Branch foo set up to track remote branch foo from origin.
> Switched to a new branch 'foo'
This is much better than the current behavior, IMHO. It at least says
what is going on, so a user who actually reads the message will have a
chance of knowing what happened.
The devil's advocate argument is that the difference between the "branch
-t" and the DWIM is that in the former, the user intentionally asks for
a new branch, whereas in the latter, they must realize (by reading and
understanding) that a new branch has been created.
Maybe that difference isn't relevant, and people actually read and
understand everything git says. Maybe not. I dunno. I don't think we
have any real data yet on how people will perceive the feature over
time, and I suspect the only way to get it is to release with it and see
what happens.
> In any case, I do not think the DWIM would kick in when you try to detach
> at remote branch head. I did not check gitk code to find out the exact
> command line it uses, but I do not think it runs "checkout BRANCH". The
> command needs to be at least "checkout REMOTE/BRANCH" to work the way it
> does now with any released version of git, and I would not be surprised if
> paulus was cautious enough to have spelled it as "refs/REMOTE/BRANCH" to
> avoid any potential ambiguity issues.
Yes, I was confused when I wrote the original. I agree that "checkout
REMOTE/BRANCH" from the command line should still detach. If gitk wants
to prevent people accidentally detaching HEAD, the context menu for
remote branch boxes should probably detect remote branches and say
something like "Create local branch 'foo' from 'origin/foo'".
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
From: Johannes Sixt @ 2009-11-05 7:41 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1D8B9.9040603@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Johannes Sixt wrote:
>> It may be that I understand something incorrectly; but then I blame the
>> justification that you gave. In this case, it would be helpful to reword
>> the commit message, and perhaps add some results from your experiments.
>>
>
> The discussion which lead to this patch, including the experiments, can be
> found in the email thread starting here:
>
> http://thread.gmane.org/gmane.comp.version-control.git/129403
>
> (along with some other unrelated stuff; but it's not a long read :)
>
> In the above thread, Marius suggested API_WIN32, but I switched it around, since
> I thought it sounded better! I also thought about GIT_WIN32. Suggestions?
I suggested to treat WIN32 and _WIN32 as synonyms. The commit message
should summarize what you observed in your experiments.
But you can also tell me now why this is not possible. (I recall that your
report about the experiments was rather long; I don't have the time to
read and understand it again and to draw the correct conclusions.)
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Johannes Sixt @ 2009-11-05 7:35 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1D3A7.2070407@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Johannes Sixt wrote:
>> I understand that you run into this error if you define NO_MMAP in your
>> config.mak. I don't know whether we support NO_MMAP as a knob for to tweak
>> the builds on all platforms. If this is the case (Junio?), then your
>> justification must be updated.
>
> AFAICT, the only build to not support NO_MMAP is MSVC (on cygwin *or* msysGit).
> The solution was obvious and low impact, so why not remove this anomaly?
Sure, why not? But then the justification is different, as I said.
-- Hannes
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Johannes Sixt @ 2009-11-05 7:33 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git, Nicolas Pitre
In-Reply-To: <4AF214D5.6050202@gmail.com>
Andrzej K. Haczewski schrieb:
> This patch implements native to Windows subset of pthreads API used by Git.
> It allows to remove Pthreads for Win32 dependency for msysgit and cygwin.
>
> The patch modifies Makefile only for MSVC (that's the environment I'm
> capable of testing on), so it requires further corrections to compile
> with MinGW or Cygwin.
Looks quite good already.
In the next round, please squash this in.
diff --git a/Makefile b/Makefile
index bae1b40..6648d11 100644
--- a/Makefile
+++ b/Makefile
@@ -414,6 +414,7 @@ LIB_H += cache-tree.h
LIB_H += commit.h
LIB_H += compat/cygwin.h
LIB_H += compat/mingw.h
+LIB_H += compat/win32/pthread.h
LIB_H += csum-file.h
LIB_H += decorate.h
LIB_H += delta.h
^ permalink raw reply related
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-05 6:53 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911050016360.14365@iabervon.org>
On Thu, 5 Nov 2009, Daniel Barkalow wrote:
> On Thu, 5 Nov 2009, Sverre Rabbelier wrote:
>
> > Heya,
> >
> > On Wed, Nov 4, 2009 at 22:30, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> > >> On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > >> > That's not true for "git pull <url> <branch>"; we do want the remote ref,
> > >> > but it doesn't have a local peer.
> >
> > No, I don't think that's right, when doing a fetch we want to store
> > the refs somewhere, sure, but not under 'refs/heads/<branch>', perhaps
> > 'refs/hg/fetch/<branch>', either way, the current code does not work.
>
> I think you've still got things backwards. From the point of view of
> transport.c, refs/<vcs> is entirely opaque, and we never look at it. Those
> aren't local peers. They're a way for the helper to communicate to
> transport-helper.c. The user says: pull refs/heads/master of this hg
> repo. Transport.c tries to fetch refs/heads/master and get the sha1 to
> write into FETCH_HEAD or wherever. Transport-helper.c says "import
> refs/heads/master", and git-fast-import reads the resulting script and
> writes some ref that the helper tells it to write. Then transport-helper.c
> figures out where the ref was written, reads it, and updates the struct
> ref representing the remote info. Then builtin-fetch looks at the struct
> ref and writes it to the local repositories ref namespace or FETCH_HEAD.
>
> > >> >I think going straight to the refspec
> > >> > command is the right answer.
> > >>
> > >> Can you clarity what you mean with "the refspec command"?
> > >
> > > Whatever it is that lets the helper tell the transport code where in the
> > > helper's private namespace to look for refs. I'd been thinking the helper
> > > would advertize the "refspec" capability, and the transport code would
> > > call the "refspec" command in order to get the helper to report that; but
> > > then I actually only said that the helper reports refspec, and not
> > > proposed a name for the command.
> >
> > Currently I'm implementing so that it would work like this for the svn helper:
> >
> > $ echo capabilities | git remote-svn origin /path/to/hg/repo
> > import
> > refspec +refs/trunk:refs/svn/origin/trunk
> > refspec +refs/branches/*:refs/svn/origin/*
> >
> > That way we can put the refspec in the config file at clone time.
> >
> > Now I've been browsing through the builtin-fetch code, and it looks
> > like the main problem is going to be to apply this refspec at all.
> > I'll have a more extensive look tomorrow.
>
> This is entirely not what I think we should have. The config file should
> say refs/heads/*:refs/remotes/origin/* like it always does, because the
> transport will list the refs like refs/heads/* and refs/tags/* and return
> them like that.
>
> I'll see if I can write up an untested patch that does what I'm thinking
> of.
Here's a patch (on my original series, which doesn't seem to be in pu any
more, but should be floating around somewhere). Completely untested,
except that it compiles. The idea is that the helper will say something
like:
refspec refs/heads/master:refs/svn/origin/trunk
refspec refs/heads/*:refs/svn/origin/branches/*
refspec refs/tags/*:refs/svn/origin/tags/*
and transport-helper will use these patterns to figure out where to find
the correct value from the helper's private namespace when asked to fetch
refs/heads/topic.
-Daniel
commit 483836f6411d2317f24c7594c557fb01133508b6
Author: Daniel Barkalow <barkalow@iabervon.org>
Date: Thu Nov 5 01:39:02 2009 -0500
Allow helper to map private ref names into normal names
This allows a helper to say that, when it handles "import
refs/heads/topic", the script it outputs will actually write to
refs/svn/origin/branches/topic; therefore, transport-helper should
read it from the latter location after git-fast-import completes.
diff --git a/remote.c b/remote.c
index f0441c4..58d1a61 100644
--- a/remote.c
+++ b/remote.c
@@ -790,6 +790,24 @@ static int match_name_with_pattern(const char *key, const char *name,
return ret;
}
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+ const char *name)
+{
+ int i;
+ char *ret = NULL;
+ for (i = 0; i < nr_refspec; i++) {
+ struct refspec *refspec = refspecs + i;
+ if (refspec->pattern) {
+ if (match_name_with_pattern(refspec->src, name,
+ refspec->dst, &ret))
+ return ret;
+ } else if (!strcmp(refspec->src, name))
+ return strdup(refspec->dst);
+ }
+ return NULL;
+
+}
+
int remote_find_tracking(struct remote *remote, struct refspec *refspec)
{
int find_src = refspec->src == NULL;
diff --git a/remote.h b/remote.h
index ac0ce2f..c2f920b 100644
--- a/remote.h
+++ b/remote.h
@@ -91,6 +91,9 @@ void ref_remove_duplicates(struct ref *ref_map);
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+ const char *name);
+
int match_refs(struct ref *src, struct ref **dst,
int nr_refspec, const char **refspec, int all);
diff --git a/transport-helper.c b/transport-helper.c
index aa5ad3c..88573e7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -5,6 +5,7 @@
#include "commit.h"
#include "diff.h"
#include "revision.h"
+#include "remote.h"
struct helper_data
{
@@ -12,6 +13,9 @@ struct helper_data
struct child_process *helper;
unsigned fetch : 1;
unsigned import : 1;
+ /* These go from remote name (as in "list") to private name */
+ struct refspec *refspecs;
+ int refspec_nr;
};
static struct child_process *get_helper(struct transport *transport)
@@ -20,6 +24,9 @@ static struct child_process *get_helper(struct transport *transport)
struct strbuf buf = STRBUF_INIT;
struct child_process *helper;
FILE *file;
+ const char **refspecs = NULL;
+ int refspec_nr = 0;
+ int refspec_alloc = 0;
if (data->helper)
return data->helper;
@@ -53,6 +60,16 @@ static struct child_process *get_helper(struct transport *transport)
data->fetch = 1;
if (!strcmp(buf.buf, "import"))
data->import = 1;
+ if (!prefixcmp(buf.buf, "refspec ")) {
+ ALLOC_GROW(refspecs,
+ refspec_nr + 1,
+ refspec_alloc);
+ refspecs[refspec_nr++] = strdup(buf.buf + strlen("refspec "));
+ }
+ }
+ if (refspecs) {
+ data->refspec_nr = refspec_nr;
+ data->refspecs = parse_fetch_refspec(refspec_nr, refspecs);
}
return data->helper;
}
@@ -121,6 +138,7 @@ static int fetch_with_import(struct transport *transport,
{
struct child_process fastimport;
struct child_process *helper = get_helper(transport);
+ struct helper_data *data = transport->data;
int i;
struct ref *posn;
struct strbuf buf = STRBUF_INIT;
@@ -141,10 +159,16 @@ static int fetch_with_import(struct transport *transport,
finish_command(&fastimport);
for (i = 0; i < nr_heads; i++) {
+ char *private;
posn = to_fetch[i];
if (posn->status & REF_STATUS_UPTODATE)
continue;
- read_ref(posn->name, posn->old_sha1);
+ if (data->refspecs)
+ private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
+ else
+ private = strdup(posn->name);
+ read_ref(private, posn->old_sha1);
+ free(private);
}
return 0;
}
^ permalink raw reply related
* [PATCHv2 2/2] t1200: Make documentation and test agree
From: Stephen Boyd @ 2009-11-05 6:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1257402833-4741-1-git-send-email-bebarino@gmail.com>
There were some differences between t1200 and the gitcore-tutorial. Add
missing tests for manually merging two branches, and use the same
commands in both files.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
This is all new since v1
Documentation/gitcore-tutorial.txt | 20 ++++----
t/t1200-tutorial.sh | 97 ++++++++++++++++++++++++++++++++---
2 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index b3640c4..7bdf090 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -185,7 +185,7 @@ object is. git will tell you that you have a "blob" object (i.e., just a
regular file), and you can see the contents with
----------------
-$ git cat-file "blob" 557db03
+$ git cat-file blob 557db03
----------------
which will print out "Hello World". The object `557db03` is nothing
@@ -1188,7 +1188,7 @@ $ git show-branch
--
+ [mybranch] Some work.
* [master] Some fun.
-*+ [mybranch^] New day.
+*+ [mybranch^] Initial commit
------------
Now we are ready to experiment with the merge by hand.
@@ -1204,11 +1204,11 @@ $ mb=$(git merge-base HEAD mybranch)
The command writes the commit object name of the common ancestor
to the standard output, so we captured its output to a variable,
because we will be using it in the next step. By the way, the common
-ancestor commit is the "New day." commit in this case. You can
+ancestor commit is the "Initial commit" commit in this case. You can
tell it by:
------------
-$ git name-rev $mb
+$ git name-rev --name-only --tags $mb
my-first-tag
------------
@@ -1237,8 +1237,8 @@ inspect the index file with this command:
------------
$ git ls-files --stage
100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
@@ -1253,8 +1253,8 @@ To look at only non-zero stages, use `\--unmerged` flag:
------------
$ git ls-files --unmerged
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
@@ -1283,8 +1283,8 @@ the working tree.. This can be seen if you run `ls-files
------------
$ git ls-files --stage
100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index c57c9d5..299e724 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -47,8 +47,9 @@ test_expect_success 'tree' '
'
test_expect_success 'git diff-index -p HEAD' '
- echo "Initial commit" | \
- git commit-tree $(git write-tree) 2>&1 > .git/refs/heads/master &&
+ tree=$(git write-tree)
+ commit=$(echo "Initial commit" | git commit-tree $tree) &&
+ git update-ref HEAD $commit &&
git diff-index -p HEAD > diff.output &&
cmp diff.expect diff.output
'
@@ -131,16 +132,18 @@ Work, work, work
EOF
cat > show-branch.expect << EOF
-* [master] Merged "mybranch" changes.
+* [master] Merge work in mybranch
! [mybranch] Some work.
--
-- [master] Merged "mybranch" changes.
+- [master] Merge work in mybranch
*+ [mybranch] Some work.
+* [master^] Some fun.
EOF
test_expect_success 'git show-branch' '
- git commit -m "Merged \"mybranch\" changes." -i hello &&
- git show-branch --topo-order master mybranch > show-branch.output &&
+ git commit -m "Merge work in mybranch" -i hello &&
+ git show-branch --topo-order --more=1 master mybranch \
+ > show-branch.output &&
cmp show-branch.expect show-branch.output
'
@@ -160,10 +163,10 @@ test_expect_success 'git resolve' '
'
cat > show-branch2.expect << EOF
-! [master] Merged "mybranch" changes.
- * [mybranch] Merged "mybranch" changes.
+! [master] Merge work in mybranch
+ * [mybranch] Merge work in mybranch
--
--- [master] Merged "mybranch" changes.
+-- [master] Merge work in mybranch
EOF
test_expect_success 'git show-branch (part 2)' '
@@ -171,6 +174,82 @@ test_expect_success 'git show-branch (part 2)' '
cmp show-branch2.expect show-branch2.output
'
+cat > show-branch3.expect << EOF
+! [master] Merge work in mybranch
+ * [mybranch] Merge work in mybranch
+--
+-- [master] Merge work in mybranch
++* [master^2] Some work.
++* [master^] Some fun.
+EOF
+
+test_expect_success 'git show-branch (part 3)' '
+ git show-branch --topo-order --more=2 master mybranch \
+ > show-branch3.output &&
+ cmp show-branch3.expect show-branch3.output
+'
+
+test_expect_success 'rewind to "Some fun." and "Some work."' '
+ git checkout mybranch &&
+ git reset --hard master^2 &&
+ git checkout master &&
+ git reset --hard master^
+'
+
+cat > show-branch4.expect << EOF
+* [master] Some fun.
+ ! [mybranch] Some work.
+--
+ + [mybranch] Some work.
+* [master] Some fun.
+*+ [mybranch^] Initial commit
+EOF
+
+test_expect_success 'git show-branch (part 4)' '
+ git show-branch --topo-order > show-branch4.output &&
+ cmp show-branch4.expect show-branch4.output
+'
+
+test_expect_success 'manual merge' '
+ mb=$(git merge-base HEAD mybranch) &&
+ git name-rev --name-only --tags $mb > name-rev.output &&
+ test "my-first-tag" = $(cat name-rev.output) &&
+
+ git read-tree -m -u $mb HEAD mybranch
+'
+
+cat > ls-files.expect << EOF
+100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
+100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
+EOF
+
+test_expect_success 'git ls-files --stage' '
+ git ls-files --stage > ls-files.output &&
+ cmp ls-files.expect ls-files.output
+'
+
+cat > ls-files-unmerged.expect << EOF
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
+100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
+EOF
+
+test_expect_success 'git ls-files --unmerged' '
+ git ls-files --unmerged > ls-files-unmerged.output &&
+ cmp ls-files-unmerged.expect ls-files-unmerged.output
+'
+
+test_expect_success 'git-merge-index' '
+ test_must_fail git merge-index git-merge-one-file hello
+'
+
+test_expect_success 'git ls-files --stage (part 2)' '
+ git ls-files --stage > ls-files.output2 &&
+ cmp ls-files.expect ls-files.output2
+'
+
test_expect_success 'git repack' 'git repack'
test_expect_success 'git prune-packed' 'git prune-packed'
test_expect_success '-> only packed objects' '
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox