From b8591e07fde134fc07daa75fb545c77448f0ae29 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:09:08 +0100 Subject: [PATCH 01/22] fixup! do_git_config_sequence(): fall back to git_dir if commondir is NULL In preparation for applying the latest iteration of bw/config-h that was submitted to the Git mailing list, let's revert the original iteration. Signed-off-by: Johannes Schindelin --- config.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/config.c b/config.c index eb67c9b9cc8cc9..3461993f0af665 100644 --- a/config.c +++ b/config.c @@ -1676,8 +1676,6 @@ static int do_git_config_sequence(const struct config_options *opts, if (opts->commondir) repo_config = mkpathdup("%s/config", opts->commondir); - else if (opts->git_dir) - repo_config = mkpathdup("%s/config", opts->git_dir); else repo_config = NULL; From 0154b9c065801f9be515d916fa51e6a85f200d77 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 13 Jul 2017 22:55:36 +0200 Subject: [PATCH 02/22] squash! do_git_config_sequence(): fall back to git_dir if commondir is NULL config: report a bug if git_dir exists without commondir This did happen at some stage, and was fixed relatively quickly. Make sure that we detect very quickly, too, should that happen again. Signed-off-by: Johannes Schindelin --- config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config.c b/config.c index 3461993f0af665..efe67560b39873 100644 --- a/config.c +++ b/config.c @@ -1676,6 +1676,8 @@ static int do_git_config_sequence(const struct config_options *opts, if (opts->commondir) repo_config = mkpathdup("%s/config", opts->commondir); + else if (opts->git_dir) + BUG("git_dir without commondir"); else repo_config = NULL; From 17719e683f919b4d07a84aa68b7768b78bae0fe3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:15:14 +0100 Subject: [PATCH 03/22] fixup! http: when using Secure Channel, ignore sslCAInfo by default Revert this in preparation for applying the latest iteration of the patch. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 8 -------- http.c | 19 +------------------ 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 56a110446915ea..e107f4c1e24370 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2249,14 +2249,6 @@ http.schannelCheckRevoke:: certificate. This option is ignored if cURL lacks support for setting the relevant SSL option at runtime. -http.schannelUseSSLCAInfo:: - As of cURL v7.60.0, the Secure Channel backend can use the - certificate bundle provided via `http.sslCAInfo`, but that would - override the Windows Certificate Store. Since this is not desirable - by default, Git will tell cURL not to use that bundle by default - when the `schannel` backend was configured via `http.sslBackend`, - unless `http.schannelUseSSLCAInfo` overrides this behavior. - http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index 0ebf8f77a6d20b..2ce2cd447bf779 100644 --- a/http.c +++ b/http.c @@ -158,12 +158,6 @@ static char *cached_accept_language; static char *http_ssl_backend; static int http_schannel_check_revoke = 1; -/* - * With the backend being set to `schannel`, setting sslCAinfo would override - * the Certificate Store in cURL v7.60.0 and later, which is not what we want - * by default. - */ -static int http_schannel_use_ssl_cainfo; size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { @@ -323,11 +317,6 @@ static int http_options(const char *var, const char *value, void *cb) return 0; } - if (!strcmp("http.schannelusesslcainfo", var)) { - http_schannel_use_ssl_cainfo = git_config_bool(var, value); - return 0; - } - if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -880,13 +869,7 @@ static CURL *get_curl_handle(void) if (ssl_pinnedkey != NULL) curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey); #endif - if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && - !http_schannel_use_ssl_cainfo) { - curl_easy_setopt(result, CURLOPT_CAINFO, NULL); -#if LIBCURL_VERSION_NUM >= 0x073400 - curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL); -#endif - } else if (ssl_cainfo != NULL) + if (ssl_cainfo != NULL) curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo); if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) { From a36e5dca52a3780044668da3832e618ddf590496 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:15:41 +0100 Subject: [PATCH 04/22] fixup! http: add support for disabling SSL revocation checks in cURL Revert this in preparation for applying the latest iteration of the patch. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 8 -------- http.c | 17 ----------------- 2 files changed, 25 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e107f4c1e24370..c569e728593027 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2241,14 +2241,6 @@ http.sslBackend:: This option is ignored if cURL lacks support for choosing the SSL backend at runtime. -http.schannelCheckRevoke:: - Used to enforce or disable certificate revocation checks in cURL - when http.sslBackend is set to "schannel". Defaults to `true` if - unset. Only necessary to disable this if Git consistently errors - and the message is about checking the revocation status of a - certificate. This option is ignored if cURL lacks support for - setting the relevant SSL option at runtime. - http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index 2ce2cd447bf779..fedfb2a2070020 100644 --- a/http.c +++ b/http.c @@ -157,8 +157,6 @@ static char *cached_accept_language; static char *http_ssl_backend; -static int http_schannel_check_revoke = 1; - size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { size_t size = eltsize * nmemb; @@ -312,11 +310,6 @@ static int http_options(const char *var, const char *value, void *cb) return 0; } - if (!strcmp("http.schannelcheckrevoke", var)) { - http_schannel_check_revoke = git_config_bool(var, value); - return 0; - } - if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -818,16 +811,6 @@ static CURL *get_curl_handle(void) } #endif - if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && - !http_schannel_check_revoke) { -#if LIBCURL_VERSION_NUM >= 0x072c00 - curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); -#else - warning("CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\n" - "your curl version is too old (>= 7.44.0)"); -#endif - } - if (http_proactive_auth) init_curl_http_auth(result); From 69eadfb8354a1fa1bff405794fe54a0a001a6bf6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:15:51 +0100 Subject: [PATCH 05/22] fixup! http: add support for selecting SSL backends at runtime Revert this in preparation for applying the latest iteration of the patch. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 5 ----- http.c | 35 ----------------------------------- 2 files changed, 40 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index c569e728593027..eb66a119753726 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2236,11 +2236,6 @@ http.sslCAPath:: with when fetching or pushing over HTTPS. Can be overridden by the `GIT_SSL_CAPATH` environment variable. -http.sslBackend:: - Name of the SSL backend to use (e.g. "openssl" or "schannel"). - This option is ignored if cURL lacks support for choosing the SSL - backend at runtime. - http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index fedfb2a2070020..4162860ee31636 100644 --- a/http.c +++ b/http.c @@ -155,8 +155,6 @@ static struct active_request_slot *active_queue_head; static char *cached_accept_language; -static char *http_ssl_backend; - size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { size_t size = eltsize * nmemb; @@ -304,12 +302,6 @@ static int http_options(const char *var, const char *value, void *cb) curl_ssl_try = git_config_bool(var, value); return 0; } - if (!strcmp("http.sslbackend", var)) { - free(http_ssl_backend); - http_ssl_backend = xstrdup_or_null(value); - return 0; - } - if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -1003,33 +995,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) git_config(urlmatch_config_entry, &config); free(normalized_url); -#if LIBCURL_VERSION_NUM >= 0x073800 - if (http_ssl_backend) { - const curl_ssl_backend **backends; - struct strbuf buf = STRBUF_INIT; - int i; - - switch (curl_global_sslset(-1, http_ssl_backend, &backends)) { - case CURLSSLSET_UNKNOWN_BACKEND: - strbuf_addf(&buf, _("Unsupported SSL backend '%s'. " - "Supported SSL backends:"), - http_ssl_backend); - for (i = 0; backends[i]; i++) - strbuf_addf(&buf, "\n\t%s", backends[i]->name); - die("%s", buf.buf); - case CURLSSLSET_NO_BACKENDS: - die(_("Could not set SSL backend to '%s': " - "cURL was built without SSL backends"), - http_ssl_backend); - case CURLSSLSET_TOO_LATE: - die(_("Could not set SSL backend to '%s': already set"), - http_ssl_backend); - case CURLSSLSET_OK: - break; /* Okay! */ - } - } -#endif - if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) die("curl_global_init failed"); From 18dab43c05fd4aa7d5adb758aa7c36afd88a5297 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 Aug 2017 22:10:06 +0200 Subject: [PATCH 06/22] squash! http: add support for selecting SSL backends at runtime http: add support for selecting SSL backends at runtime As of version 7.56.0, curl supports being compiled with multiple SSL backends. This patch adds the Git side of that feature: by setting http.sslBackend to "openssl" or "schannel", Git for Windows can now choose the SSL backend at runtime. This comes in handy on Windows because Secure Channel ("schannel") is the native solution, accessing the Windows Credential Store, thereby allowing for enterprise-wide management of certificates. For historical reasons, Git for Windows needs to support OpenSSL still, as it has previously been the only supported SSL backend in Git for Windows for almost a decade. The patch has been carried in Git for Windows for over a year, and is considered mature. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 5 +++++ http.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index eb66a119753726..c569e728593027 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2236,6 +2236,11 @@ http.sslCAPath:: with when fetching or pushing over HTTPS. Can be overridden by the `GIT_SSL_CAPATH` environment variable. +http.sslBackend:: + Name of the SSL backend to use (e.g. "openssl" or "schannel"). + This option is ignored if cURL lacks support for choosing the SSL + backend at runtime. + http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index 4162860ee31636..fedfb2a2070020 100644 --- a/http.c +++ b/http.c @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head; static char *cached_accept_language; +static char *http_ssl_backend; + size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { size_t size = eltsize * nmemb; @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb) curl_ssl_try = git_config_bool(var, value); return 0; } + if (!strcmp("http.sslbackend", var)) { + free(http_ssl_backend); + http_ssl_backend = xstrdup_or_null(value); + return 0; + } + if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) git_config(urlmatch_config_entry, &config); free(normalized_url); +#if LIBCURL_VERSION_NUM >= 0x073800 + if (http_ssl_backend) { + const curl_ssl_backend **backends; + struct strbuf buf = STRBUF_INIT; + int i; + + switch (curl_global_sslset(-1, http_ssl_backend, &backends)) { + case CURLSSLSET_UNKNOWN_BACKEND: + strbuf_addf(&buf, _("Unsupported SSL backend '%s'. " + "Supported SSL backends:"), + http_ssl_backend); + for (i = 0; backends[i]; i++) + strbuf_addf(&buf, "\n\t%s", backends[i]->name); + die("%s", buf.buf); + case CURLSSLSET_NO_BACKENDS: + die(_("Could not set SSL backend to '%s': " + "cURL was built without SSL backends"), + http_ssl_backend); + case CURLSSLSET_TOO_LATE: + die(_("Could not set SSL backend to '%s': already set"), + http_ssl_backend); + case CURLSSLSET_OK: + break; /* Okay! */ + } + } +#endif + if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) die("curl_global_init failed"); From a67fd95dafbc94b320c64244967950daf452ac04 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 23 Jan 2018 13:20:55 +1100 Subject: [PATCH 07/22] squash! http: add support for disabling SSL revocation checks in cURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit http: add support for disabling SSL revocation checks in cURL This adds support for a new http.schannelCheckRevoke config value. This config value is only used if http.sslBackend is set to "schannel", which forces cURL to use the Windows Certificate Store when validating server certificates associated with a remote server. This config value should only be set to "false" if you are in an environment where revocation checks are blocked by the network, with no alternative options. This is only supported in cURL 7.44 or later. Note: originally, we wanted to call the config setting `http.schannel.checkRevoke`. This, however, does not work: the `http.*` config settings can be limited to specific URLs via `http..*` (and this feature would mistake `schannel` for a URL). Helped by Agustín Martín Barbero. Signed-off-by: Brendan Forster Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 8 ++++++++ http.c | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index c569e728593027..e107f4c1e24370 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2241,6 +2241,14 @@ http.sslBackend:: This option is ignored if cURL lacks support for choosing the SSL backend at runtime. +http.schannelCheckRevoke:: + Used to enforce or disable certificate revocation checks in cURL + when http.sslBackend is set to "schannel". Defaults to `true` if + unset. Only necessary to disable this if Git consistently errors + and the message is about checking the revocation status of a + certificate. This option is ignored if cURL lacks support for + setting the relevant SSL option at runtime. + http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index fedfb2a2070020..272584b16e5f7d 100644 --- a/http.c +++ b/http.c @@ -157,6 +157,8 @@ static char *cached_accept_language; static char *http_ssl_backend; +static int http_schannel_check_revoke = 1; + size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { size_t size = eltsize * nmemb; @@ -310,6 +312,11 @@ static int http_options(const char *var, const char *value, void *cb) return 0; } + if (!strcmp("http.schannelcheckrevoke", var)) { + http_schannel_check_revoke = git_config_bool(var, value); + return 0; + } + if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -811,6 +818,16 @@ static CURL *get_curl_handle(void) } #endif + if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && + !http_schannel_check_revoke) { +#if LIBCURL_VERSION_NUM >= 0x072c00 + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); +#else + warning("CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\n" + "your curl version is too old (< 7.44.0)"); +#endif + } + if (http_proactive_auth) init_curl_http_auth(result); From 850406a924d2f8c4cadb661d746f16d6bbb71e17 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 14 Jun 2018 14:01:58 +0200 Subject: [PATCH 08/22] squash! http: when using Secure Channel, ignore sslCAInfo by default http: when using Secure Channel, ignore sslCAInfo by default As of cURL v7.60.0, the Secure Channel backend can use the certificate bundle provided via `http.sslCAInfo`, but that would override the Windows Certificate Store. Since this is not desirable by default, let's tell Git to not ask cURL to use that bundle by default when the `schannel` backend was configured via `http.sslBackend`, unless `http.schannelUseSSLCAInfo` overrides this behavior. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 8 ++++++++ http.c | 19 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e107f4c1e24370..56a110446915ea 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2249,6 +2249,14 @@ http.schannelCheckRevoke:: certificate. This option is ignored if cURL lacks support for setting the relevant SSL option at runtime. +http.schannelUseSSLCAInfo:: + As of cURL v7.60.0, the Secure Channel backend can use the + certificate bundle provided via `http.sslCAInfo`, but that would + override the Windows Certificate Store. Since this is not desirable + by default, Git will tell cURL not to use that bundle by default + when the `schannel` backend was configured via `http.sslBackend`, + unless `http.schannelUseSSLCAInfo` overrides this behavior. + http.pinnedpubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with diff --git a/http.c b/http.c index 272584b16e5f7d..43e75ac583b428 100644 --- a/http.c +++ b/http.c @@ -158,6 +158,12 @@ static char *cached_accept_language; static char *http_ssl_backend; static int http_schannel_check_revoke = 1; +/* + * With the backend being set to `schannel`, setting sslCAinfo would override + * the Certificate Store in cURL v7.60.0 and later, which is not what we want + * by default. + */ +static int http_schannel_use_ssl_cainfo; size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { @@ -317,6 +323,11 @@ static int http_options(const char *var, const char *value, void *cb) return 0; } + if (!strcmp("http.schannelusesslcainfo", var)) { + http_schannel_use_ssl_cainfo = git_config_bool(var, value); + return 0; + } + if (!strcmp("http.minsessions", var)) { min_curl_sessions = git_config_int(var, value); #ifndef USE_CURL_MULTI @@ -869,7 +880,13 @@ static CURL *get_curl_handle(void) if (ssl_pinnedkey != NULL) curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey); #endif - if (ssl_cainfo != NULL) + if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && + !http_schannel_use_ssl_cainfo) { + curl_easy_setopt(result, CURLOPT_CAINFO, NULL); +#if LIBCURL_VERSION_NUM >= 0x073400 + curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL); +#endif + } else if (ssl_cainfo != NULL) curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo); if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) { From 07100ba0e0a6042dcd8a40e7ee3cc60eee72d1ff Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:26:31 +0100 Subject: [PATCH 09/22] fixup! Remove old code and macro-ize implementation In preparation for applying the patch that actually made it to the Git mailing list, let's revert this. Signed-off-by: Johannes Schindelin --- compat/win32/pthread.c | 184 +++++++++++++++++++++++++++++++++++++++++ compat/win32/pthread.h | 43 ++++++---- 2 files changed, 209 insertions(+), 18 deletions(-) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 2e7eead42cb008..27e6092781d854 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -56,3 +56,187 @@ pthread_t pthread_self(void) t.tid = GetCurrentThreadId(); return t; } + +#ifdef GIT_WIN_XP_SUPPORT + +int pthread_cond_init(pthread_cond_t *cond, const void *unused) +{ + cond->waiters = 0; + cond->was_broadcast = 0; + InitializeCriticalSection(&cond->waiters_lock); + + cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL); + if (!cond->sema) + die("CreateSemaphore() failed"); + + cond->continue_broadcast = CreateEvent(NULL, /* security */ + FALSE, /* auto-reset */ + FALSE, /* not signaled */ + NULL); /* name */ + if (!cond->continue_broadcast) + die("CreateEvent() failed"); + + return 0; +} + +int pthread_cond_destroy(pthread_cond_t *cond) +{ + CloseHandle(cond->sema); + CloseHandle(cond->continue_broadcast); + DeleteCriticalSection(&cond->waiters_lock); + return 0; +} + +int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex) +{ + int last_waiter; + + 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 - ignore return value */ + WaitForSingleObject(cond->sema, INFINITE); + + /* + * Decrease waiters count. If we are the last waiter, then we must + * notify the broadcasting thread that it can continue. + * But if we continued due to cond_signal, we do not have to do that + * because the signaling thread knows that only one waiter continued. + */ + EnterCriticalSection(&cond->waiters_lock); + cond->waiters--; + last_waiter = cond->was_broadcast && cond->waiters == 0; + LeaveCriticalSection(&cond->waiters_lock); + + if (last_waiter) { + /* + * cond_broadcast was issued while mutex was held. This means + * that all other waiters have continued, but are contending + * for the mutex at the end of this function because the + * broadcasting thread did not leave cond_broadcast, yet. + * (This is so that it can be sure that each waiter has + * consumed exactly one slice of the semaphor.) + * The last waiter must tell the broadcasting thread that it + * can go on. + */ + SetEvent(cond->continue_broadcast); + /* + * Now we go on to contend with all other waiters for + * the mutex. Auf in den Kampf! + */ + } + /* lock external mutex again */ + EnterCriticalSection(mutex); + + return 0; +} + +/* + * IMPORTANT: This implementation requires that pthread_cond_signal + * is called while the mutex is held that is used in the corresponding + * pthread_cond_wait calls! + */ +int pthread_cond_signal(pthread_cond_t *cond) +{ + int have_waiters; + + 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; +} + +/* + * DOUBLY IMPORTANT: This implementation requires that pthread_cond_broadcast + * is called while the mutex is held that is used in the corresponding + * pthread_cond_wait calls! + */ +int pthread_cond_broadcast(pthread_cond_t *cond) +{ + EnterCriticalSection(&cond->waiters_lock); + + if ((cond->was_broadcast = cond->waiters > 0)) { + /* wake up all waiters */ + ReleaseSemaphore(cond->sema, cond->waiters, NULL); + LeaveCriticalSection(&cond->waiters_lock); + /* + * At this point all waiters continue. Each one takes its + * slice of the semaphor. Now it's our turn to wait: Since + * the external mutex is held, no thread can leave cond_wait, + * yet. For this reason, we can be sure that no thread gets + * a chance to eat *more* than one slice. OTOH, it means + * that the last waiter must send us a wake-up. + */ + WaitForSingleObject(cond->continue_broadcast, INFINITE); + /* + * Since the external mutex is held, no thread can enter + * cond_wait, and, hence, it is safe to reset this flag + * without cond->waiters_lock held. + */ + cond->was_broadcast = 0; + } else { + LeaveCriticalSection(&cond->waiters_lock); + } + return 0; +} + +#else // GIT_WIN_XP_SUPPORT + +WINBASEAPI VOID WINAPI +InitializeConditionVariable(PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI VOID WINAPI +WakeConditionVariable(PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI VOID WINAPI +WakeAllConditionVariable(PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI WINBOOL WINAPI +SleepConditionVariableCS(PCONDITION_VARIABLE ConditionVariable, + PCRITICAL_SECTION CriticalSection, + DWORD dwMilliseconds); + +int pthread_cond_init(pthread_cond_t *cond, const void *unused) +{ + InitializeConditionVariable(cond); + return 0; +} + +int pthread_cond_destroy(pthread_cond_t *cond) +{ + return 0; +} + +int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex) +{ + SleepConditionVariableCS(cond, mutex, INFINITE); + return 0; +} + +int pthread_cond_signal(pthread_cond_t *cond) +{ + WakeConditionVariable(cond); + return 0; +} + +int pthread_cond_broadcast(pthread_cond_t *cond) +{ + WakeAllConditionVariable(cond); + return 0; +} + +#endif // GIT_WIN_XP_SUPPORT diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 11383eb5f4d823..10a9d45f07eadd 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -32,24 +32,31 @@ typedef int pthread_mutexattr_t; #define pthread_mutexattr_settype(a, t) 0 #define PTHREAD_MUTEX_RECURSIVE 0 -#define pthread_cond_t CONDITION_VARIABLE - -WINBASEAPI VOID WINAPI -InitializeConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI -WakeConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI -WakeAllConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI BOOL WINAPI -SleepConditionVariableCS(PCONDITION_VARIABLE ConditionVariable, - PCRITICAL_SECTION CriticalSection, - DWORD dwMilliseconds); - -#define pthread_cond_init(a,b) InitializeConditionVariable((a)) -#define pthread_cond_destroy(a) do {} while (0) -#define pthread_cond_wait(a,b) return_0(SleepConditionVariableCS((a), (b), INFINITE)) -#define pthread_cond_signal WakeConditionVariable -#define pthread_cond_broadcast WakeAllConditionVariable +#ifdef GIT_WIN_XP_SUPPORT +/* + * 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; + int was_broadcast; + CRITICAL_SECTION waiters_lock; + HANDLE sema; + HANDLE continue_broadcast; +} pthread_cond_t; +#else +typedef CONDITION_VARIABLE pthread_cond_t; +#endif + +extern int pthread_cond_init(pthread_cond_t *cond, const void *unused); +extern int pthread_cond_destroy(pthread_cond_t *cond); +extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex); +extern int pthread_cond_signal(pthread_cond_t *cond); +extern int pthread_cond_broadcast(pthread_cond_t *cond); /* * Simple thread creation implementation using pthread API From dba23ff7595efe917cacfdef32e2668d1033658a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:28:32 +0100 Subject: [PATCH 10/22] fixup! Format to 80 cols In preparation for applying the patch that actually made it to the Git mailing list, let's revert this. Signed-off-by: Johannes Schindelin --- compat/win32/pthread.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 27e6092781d854..80243b67cb8dc1 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -199,16 +199,10 @@ int pthread_cond_broadcast(pthread_cond_t *cond) #else // GIT_WIN_XP_SUPPORT -WINBASEAPI VOID WINAPI -InitializeConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI -WakeConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI -WakeAllConditionVariable(PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI WINBOOL WINAPI -SleepConditionVariableCS(PCONDITION_VARIABLE ConditionVariable, - PCRITICAL_SECTION CriticalSection, - DWORD dwMilliseconds); +WINBASEAPI VOID WINAPI InitializeConditionVariable (PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI VOID WINAPI WakeConditionVariable (PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI VOID WINAPI WakeAllConditionVariable (PCONDITION_VARIABLE ConditionVariable); +WINBASEAPI WINBOOL WINAPI SleepConditionVariableCS (PCONDITION_VARIABLE ConditionVariable, PCRITICAL_SECTION CriticalSection, DWORD dwMilliseconds); int pthread_cond_init(pthread_cond_t *cond, const void *unused) { From f7ca946ce4ea503c4628900fa18ae105020ed273 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:28:49 +0100 Subject: [PATCH 11/22] fixup! Implement pthread_cond_t with Win32 CONDITION_VARIABLE In preparation for applying the patch that actually made it to the Git mailing list, let's revert this. Signed-off-by: Johannes Schindelin --- compat/win32/pthread.c | 40 ---------------------------------------- compat/win32/pthread.h | 4 ---- 2 files changed, 44 deletions(-) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 80243b67cb8dc1..e18f5c6e2e55a4 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -57,8 +57,6 @@ pthread_t pthread_self(void) return t; } -#ifdef GIT_WIN_XP_SUPPORT - int pthread_cond_init(pthread_cond_t *cond, const void *unused) { cond->waiters = 0; @@ -196,41 +194,3 @@ int pthread_cond_broadcast(pthread_cond_t *cond) } return 0; } - -#else // GIT_WIN_XP_SUPPORT - -WINBASEAPI VOID WINAPI InitializeConditionVariable (PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI WakeConditionVariable (PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI VOID WINAPI WakeAllConditionVariable (PCONDITION_VARIABLE ConditionVariable); -WINBASEAPI WINBOOL WINAPI SleepConditionVariableCS (PCONDITION_VARIABLE ConditionVariable, PCRITICAL_SECTION CriticalSection, DWORD dwMilliseconds); - -int pthread_cond_init(pthread_cond_t *cond, const void *unused) -{ - InitializeConditionVariable(cond); - return 0; -} - -int pthread_cond_destroy(pthread_cond_t *cond) -{ - return 0; -} - -int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex) -{ - SleepConditionVariableCS(cond, mutex, INFINITE); - return 0; -} - -int pthread_cond_signal(pthread_cond_t *cond) -{ - WakeConditionVariable(cond); - return 0; -} - -int pthread_cond_broadcast(pthread_cond_t *cond) -{ - WakeAllConditionVariable(cond); - return 0; -} - -#endif // GIT_WIN_XP_SUPPORT diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 10a9d45f07eadd..1c164088fbb64d 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -32,7 +32,6 @@ typedef int pthread_mutexattr_t; #define pthread_mutexattr_settype(a, t) 0 #define PTHREAD_MUTEX_RECURSIVE 0 -#ifdef GIT_WIN_XP_SUPPORT /* * Implement simple condition variable for Windows threads, based on ACE * implementation. @@ -48,9 +47,6 @@ typedef struct { HANDLE sema; HANDLE continue_broadcast; } pthread_cond_t; -#else -typedef CONDITION_VARIABLE pthread_cond_t; -#endif extern int pthread_cond_init(pthread_cond_t *cond, const void *unused); extern int pthread_cond_destroy(pthread_cond_t *cond); From a0505d0a89b8e89f8854251d08ea484a5f539ea8 Mon Sep 17 00:00:00 2001 From: Loo Rong Jie Date: Thu, 22 Jun 2017 09:03:29 +0800 Subject: [PATCH 12/22] squash! Implement pthread_cond_t with Win32 CONDITION_VARIABLE win32: replace pthread_cond_*() with much simpler code The Win32 CONDITION_VARIABLE has better performance and is easier to maintain, as the code is a lot shorter now (the semantics of the CONDITION_VARIABLE matches the pthread_cond_t very well). Note: CONDITION_VARIABLE is not available in Windows XP and below, but the declared minimal Windows version required to build and run Git for Windows is Windows Vista (which is also beyond its end-of-life, but for less long than Windows XP), so that's okay. Signed-off-by: Loo Rong Jie Signed-off-by: Johannes Schindelin --- compat/win32/pthread.c | 138 ----------------------------------------- compat/win32/pthread.h | 28 +++------ 2 files changed, 7 insertions(+), 159 deletions(-) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index e18f5c6e2e55a4..2e7eead42cb008 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -56,141 +56,3 @@ pthread_t pthread_self(void) t.tid = GetCurrentThreadId(); return t; } - -int pthread_cond_init(pthread_cond_t *cond, const void *unused) -{ - cond->waiters = 0; - cond->was_broadcast = 0; - InitializeCriticalSection(&cond->waiters_lock); - - cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL); - if (!cond->sema) - die("CreateSemaphore() failed"); - - cond->continue_broadcast = CreateEvent(NULL, /* security */ - FALSE, /* auto-reset */ - FALSE, /* not signaled */ - NULL); /* name */ - if (!cond->continue_broadcast) - die("CreateEvent() failed"); - - return 0; -} - -int pthread_cond_destroy(pthread_cond_t *cond) -{ - CloseHandle(cond->sema); - CloseHandle(cond->continue_broadcast); - DeleteCriticalSection(&cond->waiters_lock); - return 0; -} - -int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex) -{ - int last_waiter; - - 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 - ignore return value */ - WaitForSingleObject(cond->sema, INFINITE); - - /* - * Decrease waiters count. If we are the last waiter, then we must - * notify the broadcasting thread that it can continue. - * But if we continued due to cond_signal, we do not have to do that - * because the signaling thread knows that only one waiter continued. - */ - EnterCriticalSection(&cond->waiters_lock); - cond->waiters--; - last_waiter = cond->was_broadcast && cond->waiters == 0; - LeaveCriticalSection(&cond->waiters_lock); - - if (last_waiter) { - /* - * cond_broadcast was issued while mutex was held. This means - * that all other waiters have continued, but are contending - * for the mutex at the end of this function because the - * broadcasting thread did not leave cond_broadcast, yet. - * (This is so that it can be sure that each waiter has - * consumed exactly one slice of the semaphor.) - * The last waiter must tell the broadcasting thread that it - * can go on. - */ - SetEvent(cond->continue_broadcast); - /* - * Now we go on to contend with all other waiters for - * the mutex. Auf in den Kampf! - */ - } - /* lock external mutex again */ - EnterCriticalSection(mutex); - - return 0; -} - -/* - * IMPORTANT: This implementation requires that pthread_cond_signal - * is called while the mutex is held that is used in the corresponding - * pthread_cond_wait calls! - */ -int pthread_cond_signal(pthread_cond_t *cond) -{ - int have_waiters; - - 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; -} - -/* - * DOUBLY IMPORTANT: This implementation requires that pthread_cond_broadcast - * is called while the mutex is held that is used in the corresponding - * pthread_cond_wait calls! - */ -int pthread_cond_broadcast(pthread_cond_t *cond) -{ - EnterCriticalSection(&cond->waiters_lock); - - if ((cond->was_broadcast = cond->waiters > 0)) { - /* wake up all waiters */ - ReleaseSemaphore(cond->sema, cond->waiters, NULL); - LeaveCriticalSection(&cond->waiters_lock); - /* - * At this point all waiters continue. Each one takes its - * slice of the semaphor. Now it's our turn to wait: Since - * the external mutex is held, no thread can leave cond_wait, - * yet. For this reason, we can be sure that no thread gets - * a chance to eat *more* than one slice. OTOH, it means - * that the last waiter must send us a wake-up. - */ - WaitForSingleObject(cond->continue_broadcast, INFINITE); - /* - * Since the external mutex is held, no thread can enter - * cond_wait, and, hence, it is safe to reset this flag - * without cond->waiters_lock held. - */ - cond->was_broadcast = 0; - } else { - LeaveCriticalSection(&cond->waiters_lock); - } - return 0; -} diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 1c164088fbb64d..c6cb8dd2190062 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -32,27 +32,13 @@ typedef int pthread_mutexattr_t; #define pthread_mutexattr_settype(a, t) 0 #define PTHREAD_MUTEX_RECURSIVE 0 -/* - * 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; - int was_broadcast; - CRITICAL_SECTION waiters_lock; - HANDLE sema; - HANDLE continue_broadcast; -} pthread_cond_t; - -extern int pthread_cond_init(pthread_cond_t *cond, const void *unused); -extern int pthread_cond_destroy(pthread_cond_t *cond); -extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex); -extern int pthread_cond_signal(pthread_cond_t *cond); -extern int pthread_cond_broadcast(pthread_cond_t *cond); +#define pthread_cond_t CONDITION_VARIABLE + +#define pthread_cond_init(a,b) InitializeConditionVariable((a)) +#define pthread_cond_destroy(a) do {} while (0) +#define pthread_cond_wait(a,b) return_0(SleepConditionVariableCS((a), (b), INFINITE)) +#define pthread_cond_signal WakeConditionVariable +#define pthread_cond_broadcast WakeAllConditionVariable /* * Simple thread creation implementation using pthread API From 2fb1d8b7cd5a49612f60dbc06fa736e46625f340 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:35:05 +0100 Subject: [PATCH 13/22] fixup! tests: explicitly use `git.exe` on Windows In preparation for applying the latest iteration that was submitted to the Git mailing list, let's revert this here patch. Signed-off-by: Johannes Schindelin --- Makefile | 1 - t/test-lib-functions.sh | 2 +- t/test-lib.sh | 13 ++++--------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 28d78688fa39fb..5a969f5830a410 100644 --- a/Makefile +++ b/Makefile @@ -2567,7 +2567,6 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+ @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+ - @echo X=\'$(X)\' >>$@+ ifdef TEST_OUTPUT_DIRECTORY @echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+ endif diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 01bbad2e19f9c2..849b14a7cf7be1 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -898,7 +898,7 @@ test_create_repo () { mkdir -p "$repo" ( cd "$repo" || error "Cannot setup test environment" - "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \ + "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \ "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled diff --git a/t/test-lib.sh b/t/test-lib.sh index 5bef52d71a7997..1f361886852549 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -49,23 +49,18 @@ export ASAN_OPTIONS : ${LSAN_OPTIONS=abort_on_error=1} export LSAN_OPTIONS -if test ! -f "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS -then - echo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).' - exit 1 -fi -. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS -export PERL_PATH SHELL_PATH - ################################################################ # It appears that people try to run tests without building... -test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git$X" >/dev/null || +test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null || if test $? != 1 then echo >&2 'error: you do not seem to have built git yet.' exit 1 fi +. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS +export PERL_PATH SHELL_PATH + # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. case "$GIT_TEST_TEE_STARTED, $* " in From 1f438ff32f985f2d42b0b83a095369ba89e10921 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:35:33 +0100 Subject: [PATCH 14/22] fixup! tests: do not require Git to be built when testing an installed Git In preparation for applying the latest iteration that was submitted to the Git mailing list, let's revert this here patch. Signed-off-by: Johannes Schindelin --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 1f361886852549..713eb718e68501 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -51,7 +51,7 @@ export LSAN_OPTIONS ################################################################ # It appears that people try to run tests without building... -test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null || +"$GIT_BUILD_DIR/git" >/dev/null if test $? != 1 then echo >&2 'error: you do not seem to have built git yet.' From c1f55c4fcd25672611292079f4f5f8ec9d8dc420 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:35:39 +0100 Subject: [PATCH 15/22] fixup! t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set In preparation for applying the latest iteration that was submitted to the Git mailing list, let's revert this here patch. Signed-off-by: Johannes Schindelin --- t/lib-gettext.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh index 9eb160c997a0f4..eec757f104708d 100644 --- a/t/lib-gettext.sh +++ b/t/lib-gettext.sh @@ -10,12 +10,7 @@ GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale" GIT_PO_PATH="$GIT_BUILD_DIR/po" export GIT_TEXTDOMAINDIR GIT_PO_PATH -if test -n "$GIT_TEST_INSTALLED" -then - . "$(git --exec-path)"/git-sh-i18n -else - . "$GIT_BUILD_DIR"/git-sh-i18n -fi +. "$GIT_BUILD_DIR"/git-sh-i18n if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON then From 69d262d0ec29619db289d16edf9d5a4f876e03f2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:35:42 +0100 Subject: [PATCH 16/22] fixup! tests: respect GIT_TEST_INSTALLED when initializing repositories In preparation for applying the latest iteration that was submitted to the Git mailing list, let's revert this here patch. Signed-off-by: Johannes Schindelin --- t/test-lib-functions.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 849b14a7cf7be1..4207af40777c69 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -898,8 +898,7 @@ test_create_repo () { mkdir -p "$repo" ( cd "$repo" || error "Cannot setup test environment" - "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \ - "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || + "$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled ) || exit From d64d6debd2dc6b78423f42b6bd76bcf9140f478f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Nov 2018 11:35:45 +0100 Subject: [PATCH 17/22] fixup! tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/ In preparation for applying the latest iteration that was submitted to the Git mailing list, let's revert this here patch. Signed-off-by: Johannes Schindelin --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 713eb718e68501..44288cbb598435 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -926,7 +926,7 @@ elif test -n "$GIT_TEST_INSTALLED" then GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) || error "Cannot run git from $GIT_TEST_INSTALLED." - PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH + PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH} else # normal case, use ../bin-wrappers only unless $with_dashes: git_bin_dir="$GIT_BUILD_DIR/bin-wrappers" From 7801f037afa954b5e89d58b780764fde7dac7982 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Jul 2017 22:17:20 +0200 Subject: [PATCH 18/22] squash! tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/ tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/ We really need to be able to find the test helpers... Really. This change was forgotten when we moved the test helpers into t/helper/ Signed-off-by: Johannes Schindelin --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb598435..713eb718e68501 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -926,7 +926,7 @@ elif test -n "$GIT_TEST_INSTALLED" then GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) || error "Cannot run git from $GIT_TEST_INSTALLED." - PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH + PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH} else # normal case, use ../bin-wrappers only unless $with_dashes: git_bin_dir="$GIT_BUILD_DIR/bin-wrappers" From faa7cccbc900056113422ddd6d526c3ce582c6b8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 20 Jul 2017 00:09:44 +0200 Subject: [PATCH 19/22] squash! tests: respect GIT_TEST_INSTALLED when initializing repositories tests: respect GIT_TEST_INSTALLED when initializing repositories It really makes very, very little sense to use a different git executable than the one the caller indicated via setting the environment variable GIT_TEST_INSTALLED. Signed-off-by: Johannes Schindelin --- t/test-lib-functions.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 4207af40777c69..849b14a7cf7be1 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -898,7 +898,8 @@ test_create_repo () { mkdir -p "$repo" ( cd "$repo" || error "Cannot setup test environment" - "$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || + "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \ + "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled ) || exit From fe37aea812d7f0c2dcd8b3875487b458560138a7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Jul 2017 23:32:36 +0200 Subject: [PATCH 20/22] squash! t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set It makes very, very little sense to test the built git-sh-i18n when the user asked specifically to test another one. Signed-off-by: Johannes Schindelin --- t/lib-gettext.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh index eec757f104708d..9eb160c997a0f4 100644 --- a/t/lib-gettext.sh +++ b/t/lib-gettext.sh @@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale" GIT_PO_PATH="$GIT_BUILD_DIR/po" export GIT_TEXTDOMAINDIR GIT_PO_PATH -. "$GIT_BUILD_DIR"/git-sh-i18n +if test -n "$GIT_TEST_INSTALLED" +then + . "$(git --exec-path)"/git-sh-i18n +else + . "$GIT_BUILD_DIR"/git-sh-i18n +fi if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON then From ce7b0f2e14c28877cc78229ea08fe88ced22d19f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Aug 2017 00:18:33 +0200 Subject: [PATCH 21/22] squash! tests: do not require Git to be built when testing an installed Git tests: do not require Git to be built when testing an installed Git We really only need the test helpers to be built in the worktree in that case, but that is not what we test for. On the other hand it is a perfect opportunity to verify that `GIT_TEST_INSTALLED` points to a working Git. So let's test the appropriate Git executable. While at it, also adjust the error message in the `GIT_TEST_INSTALLED` case. This patch is best viewed with `-w --patience`. Helped-by: Jeff King Signed-off-by: Johannes Schindelin --- t/test-lib.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 713eb718e68501..9b688d7698518e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -51,10 +51,15 @@ export LSAN_OPTIONS ################################################################ # It appears that people try to run tests without building... -"$GIT_BUILD_DIR/git" >/dev/null +"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git" >/dev/null if test $? != 1 then - echo >&2 'error: you do not seem to have built git yet.' + if test -n "$GIT_TEST_INSTALLED" + then + echo >&2 "error: there is no working Git at '$GIT_TEST_INSTALLED'" + else + echo >&2 'error: you do not seem to have built git yet.' + fi exit 1 fi From b094daa8750bb3ba014ab1d263d4362af31823cf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Nov 2016 09:40:12 -0700 Subject: [PATCH 22/22] squash! tests: explicitly use `git.exe` on Windows tests: explicitly use `git.exe` on Windows On Windows, when we refer to `/an/absolute/path/to/git`, it magically resolves `git.exe` at that location. Except if something of the name `git` exists next to that `git.exe`. So if we call `$BUILD_DIR/git`, it will find `$BUILD_DIR/git.exe` *only* if there is not, say, a directory called `$BUILD_DIR/git`. Such a directory, however, exists in Git for Windows when building with Visual Studio (our Visual Studio project generator defaults to putting the build files into a directory whose name is the base name of the corresponding `.exe`). In the bin-wrappers/* scripts, we already take pains to use `git.exe` rather than `git`, as this could pick up the wrong thing on Windows (i.e. if there exists a `git` file or directory in the build directory). Now we do the same in the tests' start-up code. This also helps when testing an installed Git, as there might be even more likely some stray file or directory in the way. Note: the only way we can record whether the `.exe` suffix is by writing it to the `GIT-BUILD-OPTIONS` file and sourcing it at the beginning of `t/test-lib.sh`. This is not a requirement introduced by this patch, but we move the call to be able to use the `$X` variable that holds the file extension, if any. Note also: the many, many calls to `git this` and `git that` are unaffected, as the regular PATH search will find the `.exe` files on Windows (and not be confused by a directory of the name `git` that is in one of the directories listed in the `PATH` variable), while `/path/to/git` would not, per se, know that it is looking for an executable and happily prefer such a directory. Signed-off-by: Johannes Schindelin --- Makefile | 1 + t/test-lib-functions.sh | 2 +- t/test-lib.sh | 13 +++++++++---- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830a410..28d78688fa39fb 100644 --- a/Makefile +++ b/Makefile @@ -2567,6 +2567,7 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+ @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+ + @echo X=\'$(X)\' >>$@+ ifdef TEST_OUTPUT_DIRECTORY @echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+ endif diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 849b14a7cf7be1..01bbad2e19f9c2 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -898,7 +898,7 @@ test_create_repo () { mkdir -p "$repo" ( cd "$repo" || error "Cannot setup test environment" - "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \ + "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \ "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled diff --git a/t/test-lib.sh b/t/test-lib.sh index 9b688d7698518e..60469ec7b0ef5e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -49,9 +49,17 @@ export ASAN_OPTIONS : ${LSAN_OPTIONS=abort_on_error=1} export LSAN_OPTIONS +if test ! -f "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS +then + echo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).' + exit 1 +fi +. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS +export PERL_PATH SHELL_PATH + ################################################################ # It appears that people try to run tests without building... -"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git" >/dev/null +"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null if test $? != 1 then if test -n "$GIT_TEST_INSTALLED" @@ -63,9 +71,6 @@ then exit 1 fi -. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS -export PERL_PATH SHELL_PATH - # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. case "$GIT_TEST_TEE_STARTED, $* " in