From b45219f179f4c00189e069425e31ce42547e423b Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Mon, 29 Mar 2021 10:08:44 -0700 Subject: [PATCH 01/27] Implement custom thread local storage for user of library --- include/git2/sys/custom_tls.h | 66 +++++++++++++++ src/custom_tls.c | 124 +++++++++++++++++++++++++++ src/custom_tls.h | 33 ++++++++ src/libgit2.c | 2 + src/unix/pthread.c | 73 ++++++++++++++++ src/unix/pthread.h | 15 +++- src/win32/thread.c | 17 +++- src/win32/thread.h | 2 + tests/threads/custom_tls.c | 155 ++++++++++++++++++++++++++++++++++ 9 files changed, 481 insertions(+), 6 deletions(-) create mode 100644 include/git2/sys/custom_tls.h create mode 100644 src/custom_tls.c create mode 100644 src/custom_tls.h create mode 100644 src/unix/pthread.c create mode 100644 tests/threads/custom_tls.c diff --git a/include/git2/sys/custom_tls.h b/include/git2/sys/custom_tls.h new file mode 100644 index 00000000000..73832206120 --- /dev/null +++ b/include/git2/sys/custom_tls.h @@ -0,0 +1,66 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_sys_custom_tls_h__ +#define INCLUDE_sys_custom_tls_h__ + +#include "git2/common.h" + +GIT_BEGIN_DECL + +/** + * Used to retrieve a pointer from a user of the library to pass to a newly + * created internal libgit2 thread. This should allow users of the library to + * establish a context that spans an internally threaded operation. This can + * useful for libraries that leverage callbacks used in an internally threaded + * routine. + */ +typedef void *GIT_CALLBACK(git_retrieve_tls_for_internal_thread_cb)(void); + +/** + * This callback will be called when a thread is exiting so that a user + * of the library can clean up their thread local storage. + */ +typedef void GIT_CALLBACK(git_set_tls_on_internal_thread_cb)(void *payload); + +/** + * This callback will be called when a thread is exiting so that a user + * of the library can clean up their thread local storage. + */ +typedef void GIT_CALLBACK(git_teardown_tls_on_internal_thread_cb)(void); + +/** + * Sets the callbacks for custom thread local storage used by internally + * created libgit2 threads. This allows users of the library an opportunity + * to set thread local storage for internal threads based on the creating + * thread. + * + * @param retrieve_storage_for_internal_thread Used to retrieve a pointer on + * a thread before spawning child + * threads. This pointer will be + * passed to set_storage_on_thread + * in the newly spawned threads. + * @param set_storage_on_thread When a thread is spawned internally in libgit2, + * whatever pointer was retrieved in the calling + * thread by retrieve_storage_for_internal_thread + * will be passed to this callback in the newly + * spawned thread. + * @param teardown_storage_on_thread Before an internally spawned thread exits, + * this method will be called allowing a user + * of the library an opportunity to clean up + * any thread local storage they set up on + * the internal thread. + * @return 0 on success, or an error code. (use git_error_last for information + * about the error) + */ +GIT_EXTERN(int) git_custom_tls_set_callbacks( + git_retrieve_tls_for_internal_thread_cb retrieve_storage_for_internal_thread, + git_set_tls_on_internal_thread_cb set_storage_on_thread, + git_teardown_tls_on_internal_thread_cb teardown_storage_on_thread); + +GIT_END_DECL + +#endif diff --git a/src/custom_tls.c b/src/custom_tls.c new file mode 100644 index 00000000000..e7380c19a1a --- /dev/null +++ b/src/custom_tls.c @@ -0,0 +1,124 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ + +#include "common.h" +#include "custom_tls.h" +#include "runtime.h" + +#ifdef GIT_THREADS + +#ifdef GIT_WIN32 +# include "win32/thread.h" +#else +# include "unix/pthread.h" +#endif + +struct git_custom_tls_callbacks { + git_retrieve_tls_for_internal_thread_cb retrieve_storage_for_internal_thread; + + git_set_tls_on_internal_thread_cb set_storage_on_thread; + + git_teardown_tls_on_internal_thread_cb teardown_storage_on_thread; + + git_rwlock lock; +}; + +struct git_custom_tls_callbacks git__custom_tls = { 0, 0, 0 }; + +static void git_custom_tls_global_shutdown(void) +{ + if (git_rwlock_wrlock(&git__custom_tls.lock) < 0) + return; + + git__custom_tls.retrieve_storage_for_internal_thread = 0; + git__custom_tls.set_storage_on_thread = 0; + git__custom_tls.teardown_storage_on_thread = 0; + + git_rwlock_wrunlock(&git__custom_tls.lock); + git_rwlock_free(&git__custom_tls.lock); +} + +int git_custom_tls__global_init(void) +{ + if (git_rwlock_init(&git__custom_tls.lock) < 0) + return -1; + + return git_runtime_shutdown_register(git_custom_tls_global_shutdown); +} + +int git_custom_tls_set_callbacks( + git_retrieve_tls_for_internal_thread_cb retrieve_storage_for_internal_thread, + git_set_tls_on_internal_thread_cb set_storage_on_thread, + git_teardown_tls_on_internal_thread_cb teardown_storage_on_thread) +{ + /* We want to ensure that all callbacks are set or not set in totality. + * It does not make sense to have a subset of callbacks set. + */ + assert((retrieve_storage_for_internal_thread && set_storage_on_thread && + teardown_storage_on_thread) || !(retrieve_storage_for_internal_thread && + set_storage_on_thread && teardown_storage_on_thread)); + + if (git_rwlock_wrlock(&git__custom_tls.lock) < 0) { + git_error_set(GIT_ERROR_OS, "failed to lock custom thread local storage"); + return -1; + } + + git__custom_tls.retrieve_storage_for_internal_thread = + retrieve_storage_for_internal_thread; + git__custom_tls.set_storage_on_thread = + set_storage_on_thread; + git__custom_tls.teardown_storage_on_thread = + teardown_storage_on_thread; + + git_rwlock_wrunlock(&git__custom_tls.lock); + return 0; +} + +int git_custom_tls__init(git_custom_tls *tls) +{ + if (git_rwlock_rdlock(&git__custom_tls.lock) < 0) { + git_error_set(GIT_ERROR_OS, "failed to lock custom thread local storage"); + return -1; + } + + /* We try to ensure that all 3 callbacks must be set or not set. + * It would not make sense to have a subset of the callbacks set. + */ + if (!git__custom_tls.retrieve_storage_for_internal_thread) { + tls->set_storage_on_thread = NULL; + tls->teardown_storage_on_thread = NULL; + tls->payload = NULL; + } else { + /* We set these on a struct so that if for whatever reason the opts are changed + * at least the opts will remain consistent for any given thread already in + * motion. + */ + tls->set_storage_on_thread = git__custom_tls.set_storage_on_thread; + tls->teardown_storage_on_thread = git__custom_tls.teardown_storage_on_thread; + tls->payload = git__custom_tls.retrieve_storage_for_internal_thread(); + } + + git_rwlock_rdunlock(&git__custom_tls.lock); + return 0; +} + +#else + +int git_custom_tls__global_init(void) +{ + return 0; +} + +int git_custom_tls_set_callbacks( + git_retrieve_tls_for_internal_thread_cb retrieve_storage_for_internal_thread, + git_set_tls_on_internal_thread_cb set_storage_on_thread, + git_teardown_tls_on_internal_thread_cb teardown_storage_on_thread) +{ + return 0; +} + +#endif diff --git a/src/custom_tls.h b/src/custom_tls.h new file mode 100644 index 00000000000..3166ec5e404 --- /dev/null +++ b/src/custom_tls.h @@ -0,0 +1,33 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_custom_tls_h__ +#define INCLUDE_custom_tls_h__ + +#include "common.h" +#include "git2/sys/custom_tls.h" + +int git_custom_tls__global_init(void); + +#ifdef GIT_THREADS + +typedef struct { + git_set_tls_on_internal_thread_cb set_storage_on_thread; + + git_teardown_tls_on_internal_thread_cb teardown_storage_on_thread; + + /** + * payload should be set on the thread that is spawning the child thread. + * This payload will be passed to set_storage_on_thread + */ + void *payload; +} git_custom_tls; + +int git_custom_tls__init(git_custom_tls *tls); + +#endif + +#endif diff --git a/src/libgit2.c b/src/libgit2.c index cc793b4587f..2aaa997351d 100644 --- a/src/libgit2.c +++ b/src/libgit2.c @@ -31,6 +31,7 @@ #include "transports/smart.h" #include "transports/http.h" #include "transports/ssh.h" +#include "custom_tls.h" #ifdef GIT_WIN32 # include "win32/w32_leakcheck.h" @@ -69,6 +70,7 @@ int git_libgit2_init(void) git_allocator_global_init, git_threadstate_global_init, git_threads_global_init, + git_custom_tls__global_init, git_hash_global_init, git_sysdir_global_init, git_filter_global_init, diff --git a/src/unix/pthread.c b/src/unix/pthread.c new file mode 100644 index 00000000000..796061b6cdd --- /dev/null +++ b/src/unix/pthread.c @@ -0,0 +1,73 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ + +#include "pthread.h" +#include "thread.h" +#include "runtime.h" + +git_tlsdata_key thread_handle; + +static void git_threads_global_shutdown(void) { + git_tlsdata_dispose(thread_handle); +} + +int git_threads_global_init(void) { + int error = git_tlsdata_init(&thread_handle, NULL); + if (error != 0) { + return error; + } + + return git_runtime_shutdown_register(git_threads_global_shutdown); +} + +static void *git_unix__threadproc(void *arg) +{ + void *result; + int error; + git_thread *thread = arg; + + error = git_tlsdata_set(thread_handle, thread); + if (error != 0) { + return NULL; + } + + if (thread->tls.set_storage_on_thread) { + thread->tls.set_storage_on_thread(thread->tls.payload); + } + + result = thread->proc(thread->param); + + if (thread->tls.teardown_storage_on_thread) { + thread->tls.teardown_storage_on_thread(); + } + + return result; +} + +int git_thread_create( + git_thread *thread, + void *(*start_routine)(void*), + void *arg) +{ + + thread->proc = start_routine; + thread->param = arg; + if (git_custom_tls__init(&thread->tls) < 0) + return -1; + + return pthread_create(&thread->thread, NULL, git_unix__threadproc, thread); +} + +void git_thread_exit(void *value) +{ + git_thread *thread = git_tlsdata_get(thread_handle); + + if (thread && thread->tls.teardown_storage_on_thread) + thread->tls.teardown_storage_on_thread(); + + return pthread_exit(value); +} diff --git a/src/unix/pthread.h b/src/unix/pthread.h index 55f4ae227ee..b34ac1b7c41 100644 --- a/src/unix/pthread.h +++ b/src/unix/pthread.h @@ -8,18 +8,25 @@ #ifndef INCLUDE_unix_pthread_h__ #define INCLUDE_unix_pthread_h__ +#include "../custom_tls.h" + typedef struct { pthread_t thread; + void *(*proc)(void *); + void *param; + git_custom_tls tls; } git_thread; -GIT_INLINE(int) git_threads_global_init(void) { return 0; } +int git_threads_global_init(void); -#define git_thread_create(git_thread_ptr, start_routine, arg) \ - pthread_create(&(git_thread_ptr)->thread, NULL, start_routine, arg) +int git_thread_create( + git_thread *thread, + void *(*start_routine)(void*), + void *arg); #define git_thread_join(git_thread_ptr, status) \ pthread_join((git_thread_ptr)->thread, status) #define git_thread_currentid() ((size_t)(pthread_self())) -#define git_thread_exit(retval) pthread_exit(retval) +void git_thread_exit(void *value); /* Git Mutex */ #define git_mutex pthread_mutex_t diff --git a/src/win32/thread.c b/src/win32/thread.c index f5cacd320d8..beccd183aeb 100644 --- a/src/win32/thread.c +++ b/src/win32/thread.c @@ -27,12 +27,19 @@ static DWORD fls_index; static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) { git_thread *thread = lpParameter; - /* Set the current thread for `git_thread_exit` */ FlsSetValue(fls_index, thread); + if (thread->tls.set_storage_on_thread) { + thread->tls.set_storage_on_thread(thread->tls.payload); + } + thread->result = thread->proc(thread->param); + if (thread->tls.teardown_storage_on_thread) { + thread->tls.teardown_storage_on_thread(); + } + return CLEAN_THREAD_EXIT; } @@ -72,6 +79,9 @@ int git_thread_create( thread->result = NULL; thread->param = arg; thread->proc = start_routine; + if (git_custom_tls__init(&thread->tls) < 0) + return -1; + thread->thread = CreateThread( NULL, 0, git_win32__threadproc, thread, 0, NULL); @@ -107,8 +117,11 @@ void git_thread_exit(void *value) { git_thread *thread = FlsGetValue(fls_index); - if (thread) + if (thread) { + if (thread->tls.teardown_storage_on_thread) + thread->tls.teardown_storage_on_thread(); thread->result = value; + } ExitThread(CLEAN_THREAD_EXIT); } diff --git a/src/win32/thread.h b/src/win32/thread.h index 8305036b4d6..582f0f6c079 100644 --- a/src/win32/thread.h +++ b/src/win32/thread.h @@ -9,6 +9,7 @@ #define INCLUDE_win32_thread_h__ #include "common.h" +#include "../custom_tls.h" #if defined (_MSC_VER) # define GIT_RESTRICT __restrict @@ -21,6 +22,7 @@ typedef struct { void *(*proc)(void *); void *param; void *result; + git_custom_tls tls; } git_thread; typedef CRITICAL_SECTION git_mutex; diff --git a/tests/threads/custom_tls.c b/tests/threads/custom_tls.c new file mode 100644 index 00000000000..cd4a059d98d --- /dev/null +++ b/tests/threads/custom_tls.c @@ -0,0 +1,155 @@ +#include "clar_libgit2.h" + +#include "thread_helpers.h" +#include "alloc.h" +#include "common.h" +#include "git2/sys/custom_tls.h" + +static int *test[2] = { NULL, NULL }; +static int num_threads_spawned = 0; + +#if defined(GIT_THREADS) && defined(GIT_WIN32) +static DWORD _fls_index; + +int init_thread_local_storage(void) +{ + if ((_fls_index = FlsAlloc(NULL)) == FLS_OUT_OF_INDEXES) + return -1; + + return 0; +} + +void cleanup_thread_local_storage(void) +{ + FlsFree(_fls_index); +} + +void *init_local_storage(void) { + test[num_threads_spawned] = git__calloc(1, sizeof(int)); + return test[num_threads_spawned++]; +} + +void init_tls(void *payload) { + int *i = payload; + (*i)++; + FlsSetValue(_fls_index, i); +} + +void teardown_tls(void) { + int *i = FlsGetValue(_fls_index); + (*i)++; +} + +#elif defined(GIT_THREADS) && defined(_POSIX_THREADS) +static pthread_key_t _tls_key; + +int init_thread_local_storage(void) +{ + return pthread_key_create(&_tls_key, NULL); +} + +void cleanup_thread_local_storage(void) +{ + pthread_key_delete(_tls_key); +} + +void *init_local_storage(void) { + test[num_threads_spawned] = git__calloc(1, sizeof(int)); + return test[num_threads_spawned++]; +} + +void init_tls(void *payload) { + int *i = payload; + (*i)++; + pthread_setspecific(_tls_key, i); +} + +void teardown_tls(void) { + int *i = pthread_getspecific(_tls_key); + (*i)++; +} + +#endif + +void test_threads_custom_tls__initialize(void) +{ +#ifdef GIT_THREADS + cl_git_pass(init_thread_local_storage()); + cl_git_pass(git_custom_tls_set_callbacks(init_local_storage, init_tls, teardown_tls)); + test[0] = NULL; + test[1] = NULL; + num_threads_spawned = 0; +#endif +} + +void test_threads_custom_tls__cleanup(void) +{ +#ifdef GIT_THREADS + cleanup_thread_local_storage(); + git_custom_tls_set_callbacks(NULL, NULL, NULL); + + git__free(test[0]); + test[0] = NULL; + + git__free(test[1]); + test[1] = NULL; +#endif +} + +#ifdef GIT_THREADS +static void *return_normally(void *param) +{ + return param; +} +#endif + +void test_threads_custom_tls__multiple_clean_exit(void) +{ +#ifndef GIT_THREADS + clar__skip(); +#else + git_thread thread1, thread2; + void *result; + + cl_git_pass(git_thread_create(&thread1, return_normally, (void *)424242)); + cl_git_pass(git_thread_create(&thread2, return_normally, (void *)232323)); + + cl_git_pass(git_thread_join(&thread1, &result)); + cl_assert_equal_sz(424242, (size_t)result); + cl_git_pass(git_thread_join(&thread2, &result)); + cl_assert_equal_sz(232323, (size_t)result); + + cl_assert_equal_i(2, *(test[0])); + cl_assert_equal_i(2, *(test[1])); +#endif +} + +#ifdef GIT_THREADS +static void *return_early(void *param) +{ + git_thread_exit(param); + assert(false); + return param; +} +#endif + +void test_threads_custom_tls__multiple_threads_use_exit(void) +{ +#ifndef GIT_THREADS + clar__skip(); +#else + git_thread thread1, thread2; + void *result; + + cl_git_pass(git_thread_create(&thread1, return_early, (void *)424242)); + cl_git_pass(git_thread_create(&thread2, return_early, (void *)232323)); + + cl_git_pass(git_thread_join(&thread1, &result)); + cl_assert_equal_sz(424242, (size_t)result); + cl_git_pass(git_thread_join(&thread2, &result)); + cl_assert_equal_sz(232323, (size_t)result); + + cl_assert_equal_i(2, *(test[0])); + cl_assert_equal_i(2, *(test[1])); +#endif +} From 7a76a33aeab85f7e975ab78de5dab2c99d66da32 Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Wed, 12 Aug 2020 13:51:26 -0700 Subject: [PATCH 02/27] checkout: cleanup duplication in checkout_create_the_new --- src/checkout.c | 47 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 3a171066b4e..535d2de1217 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1860,6 +1860,35 @@ static int checkout_remove_the_old( return 0; } +enum { + NO_SYMLINKS = 0, + SYMLINKS_ONLY = 1 +}; + +static int checkout_create_the_new_perform( + checkout_data *data, + unsigned int action, + git_diff_delta *delta, + unsigned int checkout_option) +{ + int error = 0; + if (action & CHECKOUT_ACTION__UPDATE_BLOB) { + if (checkout_option == NO_SYMLINKS && S_ISLNK(delta->new_file.mode)) + return 0; + + if (checkout_option == SYMLINKS_ONLY && !S_ISLNK(delta->new_file.mode)) + return 0; + + if ((error = checkout_blob(data, &delta->new_file)) < 0) + return error; + + data->completed_steps++; + report_progress(data, delta->new_file.path); + } + + return 0; +} + static int checkout_create_the_new( unsigned int *actions, checkout_data *data) @@ -1869,21 +1898,15 @@ static int checkout_create_the_new( size_t i; git_vector_foreach(&data->diff->deltas, i, delta) { - if (actions[i] & CHECKOUT_ACTION__UPDATE_BLOB && !S_ISLNK(delta->new_file.mode)) { - if ((error = checkout_blob(data, &delta->new_file)) < 0) - return error; - data->completed_steps++; - report_progress(data, delta->new_file.path); - } + if ((error = checkout_create_the_new_perform(data, actions[i], delta, + NO_SYMLINKS)) < 0) + return error; } git_vector_foreach(&data->diff->deltas, i, delta) { - if (actions[i] & CHECKOUT_ACTION__UPDATE_BLOB && S_ISLNK(delta->new_file.mode)) { - if ((error = checkout_blob(data, &delta->new_file)) < 0) - return error; - data->completed_steps++; - report_progress(data, delta->new_file.path); - } + if ((error = checkout_create_the_new_perform(data, actions[i], delta, + SYMLINKS_ONLY)) < 0) + return error; } return 0; From 07493cc435986f4802e6c43fb2728e922e110936 Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Mon, 29 Mar 2021 14:04:03 -0700 Subject: [PATCH 03/27] thread checkout: move checkout buffers to tls --- src/checkout.c | 71 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 535d2de1217..2b6af85472b 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -47,6 +47,12 @@ enum { (CHECKOUT_ACTION__UPDATE_BLOB | CHECKOUT_ACTION__REMOVE), }; +typedef struct { + git_buf target_path; + size_t target_len; + git_buf tmp; +} checkout_buffers; + typedef struct { git_repository *repo; git_iterator *target; @@ -61,9 +67,7 @@ typedef struct { git_vector update_conflicts; git_vector *update_reuc; git_vector *update_names; - git_buf target_path; - size_t target_len; - git_buf tmp; + git_tlsdata_key buffers; unsigned int strategy; int can_symlink; int respect_filemode; @@ -323,15 +327,16 @@ static int checkout_action_no_wd( static int checkout_target_fullpath( git_buf **out, checkout_data *data, const char *path) { - git_buf_truncate(&data->target_path, data->target_len); + checkout_buffers *buffers = git_tlsdata_get(data->buffers); + git_buf_truncate(&buffers->target_path, buffers->target_len); - if (path && git_buf_puts(&data->target_path, path) < 0) + if (path && git_buf_puts(&buffers->target_path, path) < 0) return -1; - if (git_path_validate_workdir_buf(data->repo, &data->target_path) < 0) + if (git_path_validate_workdir_buf(data->repo, &buffers->target_path) < 0) return -1; - *out = &data->target_path; + *out = &buffers->target_path; return 0; } @@ -368,6 +373,7 @@ static int checkout_action_wd_only( bool remove = false; git_checkout_notify_t notify = GIT_CHECKOUT_NOTIFY_NONE; const git_index_entry *wd = *wditem; + checkout_buffers *buffers = git_tlsdata_get(data->buffers); if (!git_pathspec__match( pathspec, wd->path, @@ -423,8 +429,8 @@ static int checkout_action_wd_only( /* copy the entry for issuing notification callback later */ git_index_entry saved_wd = *wd; - git_buf_sets(&data->tmp, wd->path); - saved_wd.path = data->tmp.ptr; + git_buf_sets(&buffers->tmp, wd->path); + saved_wd.path = buffers->tmp.ptr; error = git_iterator_advance_over( wditem, &untracked_state, workdir); @@ -1519,6 +1525,7 @@ static int blob_content_to_file( git_filter_list *fl = NULL; int fd; int error = 0; + checkout_buffers *buffers = git_tlsdata_get(data->buffers); GIT_ASSERT(hint_path != NULL); @@ -1536,7 +1543,7 @@ static int blob_content_to_file( } filter_session.attr_session = &data->attr_session; - filter_session.temp_buf = &data->tmp; + filter_session.temp_buf = &buffers->tmp; if (!data->opts.disable_filters && (error = git_filter_list__load( @@ -2088,6 +2095,7 @@ static int checkout_write_merge( git_filter_list *fl = NULL; git_filter_session filter_session = GIT_FILTER_SESSION_INIT; int error = 0; + checkout_buffers *buffers = git_tlsdata_get(data->buffers); if (data->opts.checkout_strategy & GIT_CHECKOUT_CONFLICT_STYLE_DIFF3) opts.flags |= GIT_MERGE_FILE_STYLE_DIFF3; @@ -2137,7 +2145,7 @@ static int checkout_write_merge( in_data.size = result.len; filter_session.attr_session = &data->attr_session; - filter_session.temp_buf = &data->tmp; + filter_session.temp_buf = &buffers->tmp; if ((error = git_filter_list__load( &fl, data->repo, NULL, result.path, @@ -2328,6 +2336,16 @@ static int checkout_extensions_update_index(checkout_data *data) return error; } +static void GIT_SYSTEM_CALL dispose_checkout_buffers(void *_buffers) { + checkout_buffers *buffers = _buffers; + if (buffers == NULL) + return; + + git_buf_dispose(&buffers->target_path); + git_buf_dispose(&buffers->tmp); + git__free(buffers); +} + static void checkout_data_clear(checkout_data *data) { if (data->opts_free_baseline) { @@ -2344,8 +2362,11 @@ static void checkout_data_clear(checkout_data *data) git__free(data->pfx); data->pfx = NULL; - git_buf_dispose(&data->target_path); - git_buf_dispose(&data->tmp); + if (data->buffers) { + dispose_checkout_buffers(git_tlsdata_get(data->buffers)); + git_tlsdata_set(data->buffers, NULL); + git_tlsdata_dispose(data->buffers); + } git_index_free(data->index); data->index = NULL; @@ -2377,6 +2398,7 @@ static int checkout_data_init( git_iterator *target, const git_checkout_options *proposed) { + checkout_buffers *buffers = NULL; int error = 0; git_repository *repo = git_iterator_owner(target); @@ -2525,22 +2547,37 @@ static int checkout_data_init( git_config_entry_free(conflict_style); } + if ((error = git_tlsdata_init(&data->buffers, dispose_checkout_buffers)) < 0) + goto cleanup; + + if ((buffers = git__malloc(sizeof(checkout_buffers))) == NULL) { + error = -1; + goto cleanup; + } + if ((error = git_pool_init(&data->pool, 1)) < 0 || (error = git_vector_init(&data->removes, 0, git__strcmp_cb)) < 0 || (error = git_vector_init(&data->remove_conflicts, 0, NULL)) < 0 || (error = git_vector_init(&data->update_conflicts, 0, NULL)) < 0 || - (error = git_buf_puts(&data->target_path, data->opts.target_directory)) < 0 || - (error = git_path_to_dir(&data->target_path)) < 0 || + (error = git_buf_init(&buffers->target_path, 0)) < 0 || + (error = git_buf_init(&buffers->tmp, 0)) < 0 || + (error = git_tlsdata_set(data->buffers, buffers)) < 0 || + (error = git_buf_puts(&buffers->target_path, data->opts.target_directory)) < 0 || + (error = git_path_to_dir(&buffers->target_path)) < 0 || (error = git_strmap_new(&data->mkdir_map)) < 0) goto cleanup; - data->target_len = git_buf_len(&data->target_path); + buffers->target_len = git_buf_len(&buffers->target_path); git_attr_session__init(&data->attr_session, data->repo); cleanup: - if (error < 0) + if (error < 0) { + if (data->buffers && buffers && git_tlsdata_get(data->buffers) == NULL) + dispose_checkout_buffers(buffers); + checkout_data_clear(data); + } return error; } From e1a1eaa10bab84d176e98eed4d076e5cfa53405a Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Wed, 12 Aug 2020 13:59:41 -0700 Subject: [PATCH 04/27] thread checkout: add locks around shared state --- src/checkout.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/checkout.c b/src/checkout.c index 2b6af85472b..944d12862a1 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -53,6 +53,11 @@ typedef struct { git_buf tmp; } checkout_buffers; +enum { + COMPLETED_STEPS_MUTEX_INITIALIZED = 1, + PERFDATA_MUTEX_INITIALIZED = 2 +}; + typedef struct { git_repository *repo; git_iterator *target; @@ -77,6 +82,9 @@ typedef struct { git_checkout_perfdata perfdata; git_strmap *mkdir_map; git_attr_session attr_session; + git_mutex completed_steps_mutex; + git_mutex perfdata_mutex; + unsigned int mutexes_initialized; } checkout_data; typedef struct { @@ -1430,9 +1438,11 @@ static int checkout_mkdir( error = git_futils_mkdir_relative( path, base, mode, flags, &mkdir_opts); + git_mutex_lock(&data->perfdata_mutex); data->perfdata.mkdir_calls += mkdir_opts.perfdata.mkdir_calls; data->perfdata.stat_calls += mkdir_opts.perfdata.stat_calls; data->perfdata.chmod_calls += mkdir_opts.perfdata.chmod_calls; + git_mutex_unlock(&data->perfdata_mutex); return error; } @@ -1452,7 +1462,9 @@ static int mkpath2file( return error; if (remove_existing) { + git_mutex_lock(&data->perfdata_mutex); data->perfdata.stat_calls++; + git_mutex_unlock(&data->perfdata_mutex); if (p_lstat(path, &st) == 0) { @@ -1572,7 +1584,9 @@ static int blob_content_to_file( return error; if (st) { + git_mutex_lock(&data->perfdata_mutex); data->perfdata.stat_calls++; + git_mutex_unlock(&data->perfdata_mutex); if ((error = p_stat(path, st)) < 0) { git_error_set(GIT_ERROR_OS, "failed to stat '%s'", path); @@ -1608,7 +1622,9 @@ static int blob_content_to_link( } if (!error) { + git_mutex_lock(&data->perfdata_mutex); data->perfdata.stat_calls++; + git_mutex_unlock(&data->perfdata_mutex); if ((error = p_lstat(path, st)) < 0) git_error_set(GIT_ERROR_CHECKOUT, "could not stat symlink %s", path); @@ -1653,7 +1669,9 @@ static int checkout_submodule_update_index( if (checkout_target_fullpath(&fullpath, data, file->path) < 0) return -1; + git_mutex_lock(&data->perfdata_mutex); data->perfdata.stat_calls++; + git_mutex_unlock(&data->perfdata_mutex); if (p_stat(fullpath->ptr, &st) < 0) { git_error_set( GIT_ERROR_CHECKOUT, "could not stat submodule %s\n", file->path); @@ -1721,7 +1739,9 @@ static int checkout_safe_for_update_only( { struct stat st; + git_mutex_lock(&data->perfdata_mutex); data->perfdata.stat_calls++; + git_mutex_unlock(&data->perfdata_mutex); if (p_lstat(path, &st) < 0) { /* if doesn't exist, then no error and no update */ @@ -1834,7 +1854,9 @@ static int checkout_remove_the_old( if (error < 0) return error; + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); report_progress(data, delta->old_file.path); if ((actions[i] & CHECKOUT_ACTION__UPDATE_BLOB) == 0 && @@ -1851,7 +1873,10 @@ static int checkout_remove_the_old( if (error < 0) return error; + + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); report_progress(data, str); if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0 && @@ -1889,7 +1914,9 @@ static int checkout_create_the_new_perform( if ((error = checkout_blob(data, &delta->new_file)) < 0) return error; + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); report_progress(data, delta->new_file.path); } @@ -1932,7 +1959,9 @@ static int checkout_create_submodules( if (error < 0) return error; + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); report_progress(data, delta->new_file.path); } } @@ -2280,7 +2309,9 @@ static int checkout_create_conflicts(checkout_data *data) if (error) break; + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); report_progress(data, conflict->ours ? conflict->ours->path : (conflict->theirs ? conflict->theirs->path : conflict->ancestor->path)); @@ -2298,7 +2329,9 @@ static int checkout_remove_conflicts(checkout_data *data) if (git_index_conflict_remove(data->index, conflict) < 0) return -1; + git_mutex_lock(&data->completed_steps_mutex); data->completed_steps++; + git_mutex_unlock(&data->completed_steps_mutex); } return 0; @@ -2375,6 +2408,12 @@ static void checkout_data_clear(checkout_data *data) data->mkdir_map = NULL; git_attr_session__free(&data->attr_session); + + if (data->mutexes_initialized & COMPLETED_STEPS_MUTEX_INITIALIZED) + git_mutex_free(&data->completed_steps_mutex); + + if (data->mutexes_initialized & PERFDATA_MUTEX_INITIALIZED) + git_mutex_free(&data->perfdata_mutex); } static int validate_target_directory(checkout_data *data) @@ -2416,6 +2455,16 @@ static int checkout_data_init( data->repo = repo; data->target = target; + if ((error = git_mutex_init(&data->completed_steps_mutex)) < 0) + goto cleanup; + + data->mutexes_initialized |= COMPLETED_STEPS_MUTEX_INITIALIZED; + + if ((error = git_mutex_init(&data->perfdata_mutex)) < 0) + goto cleanup; + + data->mutexes_initialized |= PERFDATA_MUTEX_INITIALIZED; + GIT_ERROR_CHECK_VERSION( proposed, GIT_CHECKOUT_OPTIONS_VERSION, "git_checkout_options"); From dc4e588be8565592d8f0b7cb3628a38d8b9b752b Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Fri, 9 Apr 2021 08:41:19 -0700 Subject: [PATCH 05/27] thread checkout: add locks around non thread-safe actions --- src/checkout.c | 61 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 944d12862a1..59bb9f4cee4 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -55,7 +55,9 @@ typedef struct { enum { COMPLETED_STEPS_MUTEX_INITIALIZED = 1, - PERFDATA_MUTEX_INITIALIZED = 2 + INDEX_MUTEX_INITIALIZED = 2, + MKPATH_MUTEX_INITIALIZED = 4, + PERFDATA_MUTEX_INITIALIZED = 8 }; typedef struct { @@ -83,6 +85,8 @@ typedef struct { git_strmap *mkdir_map; git_attr_session attr_session; git_mutex completed_steps_mutex; + git_mutex index_mutex; + git_mutex mkpath_mutex; git_mutex perfdata_mutex; unsigned int mutexes_initialized; } checkout_data; @@ -1451,15 +1455,20 @@ static int mkpath2file( checkout_data *data, const char *path, unsigned int mode) { struct stat st; - bool remove_existing = should_remove_existing(data); - unsigned int flags = + bool remove_existing; + unsigned int flags; + int error; + + git_mutex_lock(&data->mkpath_mutex); + + remove_existing = should_remove_existing(data); + flags = (remove_existing ? MKDIR_REMOVE_EXISTING : MKDIR_NORMAL) | GIT_MKDIR_SKIP_LAST; - int error; if ((error = checkout_mkdir( data, path, data->opts.target_directory, mode, flags)) < 0) - return error; + goto cleanup; if (remove_existing) { git_mutex_lock(&data->perfdata_mutex); @@ -1476,12 +1485,14 @@ static int mkpath2file( error = git_futils_rmdir_r(path, NULL, GIT_RMDIR_REMOVE_FILES); } else if (errno != ENOENT) { git_error_set(GIT_ERROR_OS, "failed to stat '%s'", path); - return GIT_EEXISTS; + error = GIT_EEXISTS; } else { git_error_clear(); } } +cleanup: + git_mutex_unlock(&data->mkpath_mutex); return error; } @@ -1557,12 +1568,19 @@ static int blob_content_to_file( filter_session.attr_session = &data->attr_session; filter_session.temp_buf = &buffers->tmp; - if (!data->opts.disable_filters && - (error = git_filter_list__load( + if (!data->opts.disable_filters) { + git_mutex_lock(&data->index_mutex); + + error = git_filter_list__load( &fl, data->repo, blob, hint_path, - GIT_FILTER_TO_WORKTREE, &filter_session))) { - p_close(fd); - return error; + GIT_FILTER_TO_WORKTREE, &filter_session); + + git_mutex_unlock(&data->index_mutex); + + if (error) { + p_close(fd); + return error; + } } /* setup the writer */ @@ -1818,8 +1836,11 @@ static int checkout_blob( data, &file->id, fullpath->ptr, file->path, file->mode, &st); /* update the index unless prevented */ - if (!error && (data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0) + if (!error && (data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0) { + git_mutex_lock(&data->index_mutex); error = checkout_update_index(data, file, &st); + git_mutex_unlock(&data->index_mutex); + } /* update the submodule data if this was a new .gitmodules file */ if (!error && strcmp(file->path, ".gitmodules") == 0) @@ -2412,6 +2433,12 @@ static void checkout_data_clear(checkout_data *data) if (data->mutexes_initialized & COMPLETED_STEPS_MUTEX_INITIALIZED) git_mutex_free(&data->completed_steps_mutex); + if (data->mutexes_initialized & INDEX_MUTEX_INITIALIZED) + git_mutex_free(&data->index_mutex); + + if (data->mutexes_initialized & MKPATH_MUTEX_INITIALIZED) + git_mutex_free(&data->mkpath_mutex); + if (data->mutexes_initialized & PERFDATA_MUTEX_INITIALIZED) git_mutex_free(&data->perfdata_mutex); } @@ -2460,6 +2487,16 @@ static int checkout_data_init( data->mutexes_initialized |= COMPLETED_STEPS_MUTEX_INITIALIZED; + if ((error = git_mutex_init(&data->index_mutex)) < 0) + goto cleanup; + + data->mutexes_initialized |= INDEX_MUTEX_INITIALIZED; + + if ((error = git_mutex_init(&data->mkpath_mutex)) < 0) + goto cleanup; + + data->mutexes_initialized |= MKPATH_MUTEX_INITIALIZED; + if ((error = git_mutex_init(&data->perfdata_mutex)) < 0) goto cleanup; From 07396892074d7d0e3bb88c1449bed83e67ab9fec Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Wed, 12 Aug 2020 14:30:00 -0700 Subject: [PATCH 06/27] thread checkout: stub indirection for threading --- src/checkout.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/checkout.c b/src/checkout.c index 59bb9f4cee4..735b545ca07 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1944,7 +1944,7 @@ static int checkout_create_the_new_perform( return 0; } -static int checkout_create_the_new( +static int checkout_create_the_new__single( unsigned int *actions, checkout_data *data) { @@ -1967,6 +1967,15 @@ static int checkout_create_the_new( return 0; } +static int checkout_create_the_new( + unsigned int *actions, + checkout_data *data) +{ +#ifdef GIT_THREADS +#endif + return checkout_create_the_new__single(actions, data); +} + static int checkout_create_submodules( unsigned int *actions, checkout_data *data) From 27f3c804d4c8dde110e9fd07f5ab829c85ba2717 Mon Sep 17 00:00:00 2001 From: Tyler Ang-Wanek Date: Mon, 29 Mar 2021 14:41:57 -0700 Subject: [PATCH 07/27] thread checkout: add threading to checkout_create_the_new --- src/checkout.c | 220 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 220 insertions(+) diff --git a/src/checkout.c b/src/checkout.c index 735b545ca07..d9afea980d2 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1967,11 +1967,231 @@ static int checkout_create_the_new__single( return 0; } +#ifdef GIT_THREADS + +typedef struct { + int error; + size_t index; + bool skipped; +} checkout_progress_pair; + +typedef struct { + git_thread thread; + const unsigned int *actions; + checkout_data *cd; + + git_cond *cond; + git_mutex *mutex; + + git_atomic32 *delta_index; + git_atomic32 *error; + git_vector *progress_pairs; +} thread_params; + +static void *checkout_create_the_new__thread(void *arg) +{ + thread_params *worker = arg; + size_t i; + checkout_buffers *buffers = git__malloc(sizeof(checkout_buffers)); + + // TODO if the thread fails to allocate, signal and have the parent thread check the return value + // TODO deduplicate this setup with checkout_data_init + git_buf_init(&buffers->target_path, 0); + git_buf_init(&buffers->tmp, 0); + git_tlsdata_set(worker->cd->buffers, buffers); + git_buf_puts(&buffers->target_path, worker->cd->opts.target_directory); + git_path_to_dir(&buffers->target_path); + buffers->target_len = git_buf_len(&buffers->target_path); + + while ((i = git_atomic32_add(worker->delta_index, 1)) < + git_vector_length(&worker->cd->diff->deltas)) { + checkout_progress_pair *progress_pair; + git_diff_delta *delta = git_vector_get(&worker->cd->diff->deltas, i); + + if (delta == NULL || git_atomic32_get(worker->error) != 0) + return NULL; + + progress_pair = (checkout_progress_pair *)git__malloc( + sizeof(checkout_progress_pair)); + if (progress_pair == NULL) { + git_atomic32_set(worker->error, -1); + git_cond_signal(worker->cond); + return NULL; + } + + /* We skip symlink operations, because we handle them + * in the main thread to avoid a symlink security flaw. + */ + if (!S_ISLNK(delta->new_file.mode) && + worker->actions[i] & CHECKOUT_ACTION__UPDATE_BLOB) { + /* We will retry failed operations in the calling thread to handle + * the case where might encounter a file locking error due to + * multithreading and name collisions. + */ + progress_pair->index = i; + progress_pair->error = checkout_blob(worker->cd, &delta->new_file); + progress_pair->skipped = false; + } else { + progress_pair->index = i; + progress_pair->error = 0; + progress_pair->skipped = true; + } + + git_mutex_lock(worker->mutex); + git_vector_insert(worker->progress_pairs, progress_pair); + git_cond_signal(worker->cond); + git_mutex_unlock(worker->mutex); + } + + return NULL; +} + +static int checkout_create_the_new__parallel( + unsigned int *actions, + checkout_data *data) +{ + thread_params *p; + size_t i, num_threads = git__online_cpus(), last_index = 0, current_index = 0, + num_deltas = git_vector_length(&data->diff->deltas); + int ret; + checkout_progress_pair *progress_pair; + git_atomic32 delta_index, error; + git_diff_delta *delta; + git_vector errored_pairs, progress_pairs, temp; + git_cond cond; + git_mutex mutex; + + if ( + (ret = git_vector_init(&progress_pairs, num_deltas, NULL)) < 0 || + (ret = git_vector_init(&errored_pairs, num_deltas, NULL)) < 0 || + (ret = git_vector_init(&temp, num_deltas, NULL)) < 0) + return ret; + + p = git__mallocarray(num_threads, sizeof(*p)); + GIT_ERROR_CHECK_ALLOC(p); + + git_cond_init(&cond); + git_mutex_init(&mutex); + git_mutex_lock(&mutex); + + git_atomic32_set(&delta_index, -1); + git_atomic32_set(&error, 0); + + /* Initialize worker threads */ + for (i = 0; i < num_threads; ++i) { + p[i].actions = actions; + p[i].cd = data; + p[i].cond = &cond; + p[i].mutex = &mutex; + p[i].error = &error; + p[i].delta_index = &delta_index; + p[i].progress_pairs = &progress_pairs; + } + + /* Start worker threads */ + for (i = 0; i < num_threads; ++i) { + ret = git_thread_create(&p[i].thread, checkout_create_the_new__thread, &p[i]); + + /* On error, we will cleanly exit any started worker threads, + * and then return with our error code */ + if (ret) { + git_atomic32_set(&error, -1); + git_error_set(GIT_ERROR_THREAD, "unable to create thread"); + git_mutex_unlock(&mutex); + /* Only clean up the number of threads we have started */ + num_threads = i; + ret = -1; + goto cleanup; + } + } + + while (last_index < num_deltas) { + if ((ret = git_atomic32_get(&error)) != 0) { + git_mutex_unlock(&mutex); + goto cleanup; + } + + current_index = git_vector_length(&progress_pairs); + + if (last_index == current_index) { + git_cond_wait(&cond, &mutex); + current_index = git_vector_length(&progress_pairs); + } + + git_vector_clear(&temp); + for (; last_index < current_index; ++last_index) { + progress_pair = git_vector_get(&progress_pairs, + last_index); + delta = git_vector_get(&data->diff->deltas, last_index); + + if (progress_pair->skipped) + continue; + + /* We will retry errored checkouts synchronously after all the workers + * complete + */ + if (progress_pair->error < 0) { + git_vector_insert(&errored_pairs, progress_pair); + continue; + } + + git_vector_insert(&temp, delta); + } + + git_mutex_unlock(&mutex); + + for (i = 0; i < git_vector_length(&temp); ++i) { + delta = git_vector_get(&temp, i); + data->completed_steps++; + report_progress(data, delta->new_file.path); + } + + git_mutex_lock(&mutex); + } + + git_mutex_unlock(&mutex); + + git_vector_foreach(&errored_pairs, i, progress_pair) { + delta = git_vector_get(&data->diff->deltas, progress_pair->index); + if ((ret = checkout_create_the_new_perform(data, actions[progress_pair->index], + delta, NO_SYMLINKS)) < 0) + goto cleanup; + } + + /* After we create everything else, we need to create all the symlinks + * to ensure that we don't accidentally write data through symlinks into + * the .git directory. + */ + git_vector_foreach(&data->diff->deltas, i, delta) { + if ((ret = checkout_create_the_new_perform(data, actions[i], delta, + SYMLINKS_ONLY)) < 0) + goto cleanup; + } + +cleanup: + for (i = 0; i < num_threads; ++i) { + git_thread_join(&p[i].thread, NULL); + } + + git__free(p); + git_vector_free(&errored_pairs); + git_vector_free(&temp); + git_vector_free_deep(&progress_pairs); + git_cond_free(&cond); + git_mutex_free(&mutex); + + return ret; +} + +#endif + static int checkout_create_the_new( unsigned int *actions, checkout_data *data) { #ifdef GIT_THREADS + if (git__online_cpus() > 1) + return checkout_create_the_new__parallel(actions, data); #endif return checkout_create_the_new__single(actions, data); } From 37caa8dc4db4c9b3eb98afab0cb90e645c682527 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 26 Feb 2022 12:56:43 -0500 Subject: [PATCH 08/27] meta: show build status for v1.3 branch --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0cbde525b5a..3e18800a4c5 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ libgit2 - the Git linkable library | Build Status | | | ------------ | - | | **main** branch CI builds | [![CI Build](https://github.com/libgit2/libgit2/workflows/CI%20Build/badge.svg?event=push)](https://github.com/libgit2/libgit2/actions?query=workflow%3A%22CI+Build%22+event%3Apush) | +| **v1.3 branch** CI builds | [![CI Build](https://github.com/libgit2/libgit2/workflows/CI%20Build/badge.svg?branch=maint%2Fv1.2&event=push)](https://github.com/libgit2/libgit2/actions?query=workflow%3A%22CI+Build%22+event%3Apush+branch%3Amaint%2Fv1.2) | | **v1.2 branch** CI builds | [![CI Build](https://github.com/libgit2/libgit2/workflows/CI%20Build/badge.svg?branch=maint%2Fv1.2&event=push)](https://github.com/libgit2/libgit2/actions?query=workflow%3A%22CI+Build%22+event%3Apush+branch%3Amaint%2Fv1.2) | -| **v1.1 branch** CI builds | [![CI Build](https://github.com/libgit2/libgit2/workflows/CI%20Build/badge.svg?branch=maint%2Fv1.1&event=push)](https://github.com/libgit2/libgit2/actions?query=workflow%3A%22CI+Build%22+event%3Apush+branch%3Amaint%2Fv1.1) | | **Nightly** builds | [![Nightly Build](https://github.com/libgit2/libgit2/workflows/Nightly%20Build/badge.svg)](https://github.com/libgit2/libgit2/actions?query=workflow%3A%22Nightly+Build%22) [![Coverity Scan Status](https://scan.coverity.com/projects/639/badge.svg)](https://scan.coverity.com/projects/639) | `libgit2` is a portable, pure C implementation of the Git core methods From 6b12762681bbaafa6fa999b512ba0ce376a84264 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 10 Jan 2022 21:25:05 -0500 Subject: [PATCH 09/27] online: test with https instead of git protocol GitHub is removing support for the unauthenticated git protocol; test with the https protocol. --- tests/fetchhead/fetchhead_data.h | 64 ++++++++++++++++---------------- tests/fetchhead/nonetwork.c | 12 +++--- tests/online/fetch.c | 9 +---- tests/online/fetchhead.c | 8 +--- tests/online/remotes.c | 2 +- 5 files changed, 43 insertions(+), 52 deletions(-) diff --git a/tests/fetchhead/fetchhead_data.h b/tests/fetchhead/fetchhead_data.h index c75b65b90f7..77c3220016b 100644 --- a/tests/fetchhead/fetchhead_data.h +++ b/tests/fetchhead/fetchhead_data.h @@ -1,48 +1,48 @@ #define FETCH_HEAD_WILDCARD_DATA_LOCAL \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ - "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of git://github.com/libgit2/TestGitRepository\n" \ - "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of git://github.com/libgit2/TestGitRepository\n" \ - "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of git://github.com/libgit2/TestGitRepository\n" + "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ + "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of https://github.com/libgit2/TestGitRepository\n" \ + "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of https://github.com/libgit2/TestGitRepository\n" \ + "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of https://github.com/libgit2/TestGitRepository\n" #define FETCH_HEAD_WILDCARD_DATA \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ - "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of git://github.com/libgit2/TestGitRepository\n" \ - "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of git://github.com/libgit2/TestGitRepository\n" \ - "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of git://github.com/libgit2/TestGitRepository\n" \ - "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e\tnot-for-merge\ttag 'nearly-dangling' of git://github.com/libgit2/TestGitRepository\n" + "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ + "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of https://github.com/libgit2/TestGitRepository\n" \ + "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of https://github.com/libgit2/TestGitRepository\n" \ + "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of https://github.com/libgit2/TestGitRepository\n" \ + "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e\tnot-for-merge\ttag 'nearly-dangling' of https://github.com/libgit2/TestGitRepository\n" #define FETCH_HEAD_WILDCARD_DATA2 \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ + "49322bb17d3acc9146f98c97d078513228bbf3c0\t\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ #define FETCH_HEAD_NO_MERGE_DATA \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ - "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of git://github.com/libgit2/TestGitRepository\n" \ - "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of git://github.com/libgit2/TestGitRepository\n" \ - "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of git://github.com/libgit2/TestGitRepository\n" \ - "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e\tnot-for-merge\ttag 'nearly-dangling' of git://github.com/libgit2/TestGitRepository\n" + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ + "d96c4e80345534eccee5ac7b07fc7603b56124cb\tnot-for-merge\ttag 'annotated_tag' of https://github.com/libgit2/TestGitRepository\n" \ + "55a1a760df4b86a02094a904dfa511deb5655905\tnot-for-merge\ttag 'blob' of https://github.com/libgit2/TestGitRepository\n" \ + "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of https://github.com/libgit2/TestGitRepository\n" \ + "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e\tnot-for-merge\ttag 'nearly-dangling' of https://github.com/libgit2/TestGitRepository\n" #define FETCH_HEAD_NO_MERGE_DATA2 \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ #define FETCH_HEAD_NO_MERGE_DATA3 \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" \ - "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of git://github.com/libgit2/TestGitRepository\n" \ - "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of git://github.com/libgit2/TestGitRepository\n" \ - "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of git://github.com/libgit2/TestGitRepository\n" \ + "0966a434eb1a025db6b71485ab63a3bfbea520b6\tnot-for-merge\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" \ + "49322bb17d3acc9146f98c97d078513228bbf3c0\tnot-for-merge\tbranch 'master' of https://github.com/libgit2/TestGitRepository\n" \ + "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1\tnot-for-merge\tbranch 'no-parent' of https://github.com/libgit2/TestGitRepository\n" \ + "8f50ba15d49353813cc6e20298002c0d17b0a9ee\tnot-for-merge\ttag 'commit_tree' of https://github.com/libgit2/TestGitRepository\n" \ #define FETCH_HEAD_EXPLICIT_DATA \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\t\tbranch 'first-merge' of git://github.com/libgit2/TestGitRepository\n" + "0966a434eb1a025db6b71485ab63a3bfbea520b6\t\tbranch 'first-merge' of https://github.com/libgit2/TestGitRepository\n" #define FETCH_HEAD_QUOTE_DATA \ - "0966a434eb1a025db6b71485ab63a3bfbea520b6\t\tbranch 'first's-merge' of git://github.com/libgit2/TestGitRepository\n" + "0966a434eb1a025db6b71485ab63a3bfbea520b6\t\tbranch 'first's-merge' of https://github.com/libgit2/TestGitRepository\n" diff --git a/tests/fetchhead/nonetwork.c b/tests/fetchhead/nonetwork.c index 6881af40a00..f0db5fd88bf 100644 --- a/tests/fetchhead/nonetwork.c +++ b/tests/fetchhead/nonetwork.c @@ -33,42 +33,42 @@ static void populate_fetchhead(git_vector *out, git_repository *repo) "49322bb17d3acc9146f98c97d078513228bbf3c0")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 1, "refs/heads/master", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_oid_fromstr(&oid, "0966a434eb1a025db6b71485ab63a3bfbea520b6")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 0, "refs/heads/first-merge", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_oid_fromstr(&oid, "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 0, "refs/heads/no-parent", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_oid_fromstr(&oid, "d96c4e80345534eccee5ac7b07fc7603b56124cb")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 0, "refs/tags/annotated_tag", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_oid_fromstr(&oid, "55a1a760df4b86a02094a904dfa511deb5655905")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 0, "refs/tags/blob", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_oid_fromstr(&oid, "8f50ba15d49353813cc6e20298002c0d17b0a9ee")); cl_git_pass(git_fetchhead_ref_create(&fetchhead_ref, &oid, 0, "refs/tags/commit_tree", - "git://github.com/libgit2/TestGitRepository")); + "https://github.com/libgit2/TestGitRepository")); cl_git_pass(git_vector_insert(out, fetchhead_ref)); cl_git_pass(git_fetchhead_write(repo, out)); diff --git a/tests/online/fetch.c b/tests/online/fetch.c index 67dfd69ed62..f7272f48e05 100644 --- a/tests/online/fetch.c +++ b/tests/online/fetch.c @@ -67,11 +67,6 @@ static void do_fetch(const char *url, git_remote_autotag_option_t flag, int n) git_remote_free(remote); } -void test_online_fetch__default_git(void) -{ - do_fetch("git://github.com/libgit2/TestGitRepository.git", GIT_REMOTE_DOWNLOAD_TAGS_AUTO, 6); -} - void test_online_fetch__default_http(void) { do_fetch("http://github.com/libgit2/TestGitRepository.git", GIT_REMOTE_DOWNLOAD_TAGS_AUTO, 6); @@ -84,7 +79,7 @@ void test_online_fetch__default_https(void) void test_online_fetch__no_tags_git(void) { - do_fetch("git://github.com/libgit2/TestGitRepository.git", GIT_REMOTE_DOWNLOAD_TAGS_NONE, 3); + do_fetch("https://github.com/libgit2/TestGitRepository.git", GIT_REMOTE_DOWNLOAD_TAGS_NONE, 3); } void test_online_fetch__no_tags_http(void) @@ -95,7 +90,7 @@ void test_online_fetch__no_tags_http(void) void test_online_fetch__fetch_twice(void) { git_remote *remote; - cl_git_pass(git_remote_create(&remote, _repo, "test", "git://github.com/libgit2/TestGitRepository.git")); + cl_git_pass(git_remote_create(&remote, _repo, "test", "https://github.com/libgit2/TestGitRepository.git")); cl_git_pass(git_remote_connect(remote, GIT_DIRECTION_FETCH, NULL, NULL, NULL)); cl_git_pass(git_remote_download(remote, NULL, NULL)); git_remote_disconnect(remote); diff --git a/tests/online/fetchhead.c b/tests/online/fetchhead.c index b6199313818..b4a6739a19b 100644 --- a/tests/online/fetchhead.c +++ b/tests/online/fetchhead.c @@ -5,7 +5,7 @@ #include "../fetchhead/fetchhead_data.h" #include "git2/clone.h" -#define LIVE_REPO_URL "git://github.com/libgit2/TestGitRepository" +#define LIVE_REPO_URL "https://github.com/libgit2/TestGitRepository" static git_repository *g_repo; static git_clone_options g_options; @@ -53,7 +53,6 @@ static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fet git_remote *remote; git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT; git_buf fetchhead_buf = GIT_BUF_INIT; - int equals = 0; git_strarray array, *active_refs = NULL; cl_git_pass(git_remote_lookup(&remote, g_repo, "origin")); @@ -70,11 +69,8 @@ static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fet cl_git_pass(git_futils_readbuffer(&fetchhead_buf, "./foo/.git/FETCH_HEAD")); - equals = (strcmp(fetchhead_buf.ptr, expected_fetchhead) == 0); - + cl_assert_equal_s(fetchhead_buf.ptr, expected_fetchhead); git_buf_dispose(&fetchhead_buf); - - cl_assert(equals); } void test_online_fetchhead__wildcard_spec(void) diff --git a/tests/online/remotes.c b/tests/online/remotes.c index f7fe4142ff4..887874d9285 100644 --- a/tests/online/remotes.c +++ b/tests/online/remotes.c @@ -1,6 +1,6 @@ #include "clar_libgit2.h" -#define URL "git://github.com/libgit2/TestGitRepository" +#define URL "https://github.com/libgit2/TestGitRepository" #define REFSPEC "refs/heads/first-merge:refs/remotes/origin/first-merge" static int remote_single_branch(git_remote **out, git_repository *repo, const char *name, const char *url, void *payload) From 670415a5896607fabda02bc79736437883b0903b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 22 Mar 2022 23:05:48 -0400 Subject: [PATCH 10/27] clone: update bitbucket tests --- tests/online/clone.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/online/clone.c b/tests/online/clone.c index 7d43c6a098f..35f1dc57f5d 100644 --- a/tests/online/clone.c +++ b/tests/online/clone.c @@ -8,9 +8,9 @@ #define LIVE_REPO_URL "http://github.com/libgit2/TestGitRepository" #define LIVE_EMPTYREPO_URL "http://github.com/libgit2/TestEmptyRepository" -#define BB_REPO_URL "https://libgit3@bitbucket.org/libgit2/testgitrepository.git" -#define BB_REPO_URL_WITH_PASS "https://libgit3:libgit3@bitbucket.org/libgit2/testgitrepository.git" -#define BB_REPO_URL_WITH_WRONG_PASS "https://libgit3:wrong@bitbucket.org/libgit2/testgitrepository.git" +#define BB_REPO_URL "https://libgit2-test@bitbucket.org/libgit2-test/testgitrepository.git" +#define BB_REPO_URL_WITH_PASS "https://libgit2-test:YT77Ppm2nq8w4TYjGS8U@bitbucket.org/libgit2-test/testgitrepository.git" +#define BB_REPO_URL_WITH_WRONG_PASS "https://libgit2-test:wrong@bitbucket.org/libgit2-test/testgitrepository.git" #define GOOGLESOURCE_REPO_URL "https://chromium.googlesource.com/external/github.com/sergi/go-diff" #define SSH_REPO_URL "ssh://github.com/libgit2/TestGitRepository" @@ -405,7 +405,7 @@ void test_online_clone__credentials(void) void test_online_clone__credentials_via_custom_headers(void) { - const char *creds = "libgit3:libgit3"; + const char *creds = "libgit2-test:YT77Ppm2nq8w4TYjGS8U"; git_buf auth = GIT_BUF_INIT; cl_git_pass(git_buf_puts(&auth, "Authorization: Basic ")); @@ -413,7 +413,7 @@ void test_online_clone__credentials_via_custom_headers(void) g_options.fetch_opts.custom_headers.count = 1; g_options.fetch_opts.custom_headers.strings = &auth.ptr; - cl_git_pass(git_clone(&g_repo, "https://bitbucket.org/libgit2/testgitrepository.git", "./foo", &g_options)); + cl_git_pass(git_clone(&g_repo, "https://bitbucket.org/libgit2-test/testgitrepository.git", "./foo", &g_options)); git_buf_dispose(&auth); } @@ -421,7 +421,7 @@ void test_online_clone__credentials_via_custom_headers(void) void test_online_clone__bitbucket_style(void) { git_credential_userpass_payload user_pass = { - "libgit3", "libgit3" + "libgit2-test", "YT77Ppm2nq8w4TYjGS8U" }; g_options.fetch_opts.callbacks.credentials = git_credential_userpass; @@ -435,7 +435,7 @@ void test_online_clone__bitbucket_style(void) void test_online_clone__bitbucket_uses_creds_in_url(void) { git_credential_userpass_payload user_pass = { - "libgit2", "wrong" + "libgit2-test", "wrong" }; g_options.fetch_opts.callbacks.credentials = git_credential_userpass; @@ -453,7 +453,7 @@ void test_online_clone__bitbucket_uses_creds_in_url(void) void test_online_clone__bitbucket_falls_back_to_specified_creds(void) { git_credential_userpass_payload user_pass = { - "libgit2", "libgit2" + "libgit2-test", "libgit2" }; g_options.fetch_opts.callbacks.credentials = git_credential_userpass; From 973d959abaa5faaddec6a4c27889ff5b980d2550 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sun, 10 Apr 2022 21:29:43 +0100 Subject: [PATCH 11/27] path: refactor ownership checks into current user and system Provide individual file ownership checks for both the current user and the system user, as well as a combined current user and system user check. --- src/config.c | 16 ++-- src/path.c | 231 ++++++++++++++++++++++++++++++++++------------ src/path.h | 22 +++-- tests/core/path.c | 25 +++++ 4 files changed, 223 insertions(+), 71 deletions(-) diff --git a/src/config.c b/src/config.c index 3251cd51fdb..489eee643a2 100644 --- a/src/config.c +++ b/src/config.c @@ -1118,16 +1118,20 @@ int git_config_find_system(git_buf *path) int git_config_find_programdata(git_buf *path) { int ret; + bool is_safe; - if ((ret = git_buf_sanitize(path)) < 0) + if ((ret = git_buf_sanitize(path)) < 0 || + (ret = git_sysdir_find_programdata_file(path, + GIT_CONFIG_FILENAME_PROGRAMDATA)) < 0 || + (ret = git_path_owner_is_system_or_current_user(&is_safe, path->ptr)) < 0) return ret; - ret = git_sysdir_find_programdata_file(path, - GIT_CONFIG_FILENAME_PROGRAMDATA); - if (ret != GIT_OK) - return ret; + if (!is_safe) { + git_error_set(GIT_ERROR_CONFIG, "programdata path has invalid ownership"); + return -1; + } - return git_path_validate_system_file_ownership(path->ptr); + return 0; } int git_config__global_location(git_buf *buf) diff --git a/src/path.c b/src/path.c index c444b31a771..b3bb0b40825 100644 --- a/src/path.c +++ b/src/path.c @@ -2024,78 +2024,195 @@ bool git_path_supports_symlinks(const char *dir) return supported; } -int git_path_validate_system_file_ownership(const char *path) +#ifdef GIT_WIN32 +static PSID *sid_dup(PSID sid) { -#ifndef GIT_WIN32 - GIT_UNUSED(path); - return GIT_OK; -#else - git_win32_path buf; - PSID owner_sid; - PSECURITY_DESCRIPTOR descriptor = NULL; - HANDLE token; - TOKEN_USER *info = NULL; - DWORD err, len; - int ret; + DWORD len; + PSID dup; - if (git_win32_path_from_utf8(buf, path) < 0) - return -1; + len = GetLengthSid(sid); - err = GetNamedSecurityInfoW(buf, SE_FILE_OBJECT, - OWNER_SECURITY_INFORMATION | - DACL_SECURITY_INFORMATION, - &owner_sid, NULL, NULL, NULL, &descriptor); + if ((dup = git__malloc(len)) == NULL) + return NULL; - if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) { - ret = GIT_ENOTFOUND; - goto cleanup; + if (!CopySid(len, dup, sid)) { + git_error_set(GIT_ERROR_OS, "could not duplicate sid"); + git__free(dup); + return NULL; } - if (err != ERROR_SUCCESS) { - git_error_set(GIT_ERROR_OS, "failed to get security information"); - ret = GIT_ERROR; - goto cleanup; - } + return dup; +} + +static int current_user_sid(PSID *out) +{ + TOKEN_USER *info = NULL; + HANDLE token = NULL; + DWORD len = 0; + int error = -1; - if (!IsValidSid(owner_sid)) { - git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is unknown"); - ret = GIT_ERROR; - goto cleanup; + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) { + git_error_set(GIT_ERROR_OS, "could not lookup process information"); + goto done; } - if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) || - IsWellKnownSid(owner_sid, WinLocalSystemSid)) { - ret = GIT_OK; - goto cleanup; - } - - /* Obtain current user's SID */ - if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) && - !GetTokenInformation(token, TokenUser, NULL, 0, &len)) { - info = git__malloc(len); - GIT_ERROR_CHECK_ALLOC(info); - if (!GetTokenInformation(token, TokenUser, info, len, &len)) { - git__free(info); - info = NULL; - } + if (GetTokenInformation(token, TokenUser, NULL, 0, &len) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + git_error_set(GIT_ERROR_OS, "could not lookup token metadata"); + goto done; } - /* - * If the file is owned by the same account that is running the current - * process, it's okay to read from that file. - */ - if (info && EqualSid(owner_sid, info->User.Sid)) - ret = GIT_OK; - else { - git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is not valid"); - ret = GIT_ERROR; + info = git__malloc(len); + GIT_ERROR_CHECK_ALLOC(info); + + if (!GetTokenInformation(token, TokenUser, info, len, &len)) { + git_error_set(GIT_ERROR_OS, "could not lookup current user"); + goto done; } + + if ((*out = sid_dup(info->User.Sid))) + error = 0; + +done: + if (token) + CloseHandle(token); + git__free(info); + return error; +} + +static int file_owner_sid(PSID *out, const char *path) +{ + git_win32_path path_w32; + PSECURITY_DESCRIPTOR descriptor = NULL; + PSID owner_sid; + DWORD ret; + int error = -1; + + if (git_win32_path_from_utf8(path_w32, path) < 0) + return -1; + + ret = GetNamedSecurityInfoW(path_w32, SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + &owner_sid, NULL, NULL, NULL, &descriptor); + + if (ret == ERROR_FILE_NOT_FOUND || ret == ERROR_PATH_NOT_FOUND) + error = GIT_ENOTFOUND; + else if (ret != ERROR_SUCCESS) + git_error_set(GIT_ERROR_OS, "failed to get security information"); + else if (!IsValidSid(owner_sid)) + git_error_set(GIT_ERROR_OS, "file owner is not valid"); + else if ((*out = sid_dup(owner_sid))) + error = 0; -cleanup: if (descriptor) LocalFree(descriptor); - return ret; -#endif + return error; } + +int git_path_owner_is_current_user(bool *out, const char *path) +{ + PSID owner_sid = NULL, user_sid = NULL; + int error = -1; + + if ((error = file_owner_sid(&owner_sid, path)) < 0 || + (error = current_user_sid(&user_sid)) < 0) + goto done; + + *out = EqualSid(owner_sid, user_sid); + error = 0; + +done: + git__free(owner_sid); + git__free(user_sid); + return error; +} + +int git_path_owner_is_system(bool *out, const char *path) +{ + PSID owner_sid; + + if (file_owner_sid(&owner_sid, path) < 0) + return -1; + + *out = IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) || + IsWellKnownSid(owner_sid, WinLocalSystemSid); + + git__free(owner_sid); + return 0; +} + +int git_path_owner_is_system_or_current_user(bool *out, const char *path) +{ + PSID owner_sid = NULL, user_sid = NULL; + int error = -1; + + if (file_owner_sid(&owner_sid, path) < 0) + goto done; + + if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) || + IsWellKnownSid(owner_sid, WinLocalSystemSid)) { + *out = 1; + error = 0; + goto done; + } + + if (current_user_sid(&user_sid) < 0) + goto done; + + *out = EqualSid(owner_sid, user_sid); + error = 0; + +done: + git__free(owner_sid); + git__free(user_sid); + return error; +} + +#else + +static int path_owner_is(bool *out, const char *path, uid_t *uids, size_t uids_len) +{ + struct stat st; + size_t i; + + *out = false; + + if (p_lstat(path, &st) != 0) { + if (errno == ENOENT) + return GIT_ENOTFOUND; + + git_error_set(GIT_ERROR_OS, "could not stat '%s'", path); + return -1; + } + + for (i = 0; i < uids_len; i++) { + if (uids[i] == st.st_uid) { + *out = true; + break; + } + } + + return 0; +} + +int git_path_owner_is_current_user(bool *out, const char *path) +{ + uid_t userid = geteuid(); + return path_owner_is(out, path, &userid, 1); +} + +int git_path_owner_is_system(bool *out, const char *path) +{ + uid_t userid = 0; + return path_owner_is(out, path, &userid, 1); +} + +int git_path_owner_is_system_or_current_user(bool *out, const char *path) +{ + uid_t userids[2] = { geteuid(), 0 }; + return path_owner_is(out, path, userids, 2); +} + +#endif diff --git a/src/path.h b/src/path.h index de6ec8ff248..699945bd70e 100644 --- a/src/path.h +++ b/src/path.h @@ -723,15 +723,21 @@ int git_path_normalize_slashes(git_buf *out, const char *path); bool git_path_supports_symlinks(const char *dir); /** - * Validate a system file's ownership - * * Verify that the file in question is owned by an administrator or system - * account, or at least by the current user. - * - * This function returns 0 if successful. If the file is not owned by any of - * these, or any other if there have been problems determining the file - * ownership, it returns -1. + * account. + */ +int git_path_owner_is_system(bool *out, const char *path); + +/** + * Verify that the file in question is owned by the current user; + */ + +int git_path_owner_is_current_user(bool *out, const char *path); + +/** + * Verify that the file in question is owned by an administrator or system + * account _or_ the current user; */ -int git_path_validate_system_file_ownership(const char *path); +int git_path_owner_is_system_or_current_user(bool *out, const char *path); #endif diff --git a/tests/core/path.c b/tests/core/path.c index eac3573fea7..321b492c92a 100644 --- a/tests/core/path.c +++ b/tests/core/path.c @@ -659,3 +659,28 @@ void test_core_path__git_path_is_file(void) cl_git_pass(git_path_is_gitfile("blob", 4, GIT_PATH_GITFILE_GITATTRIBUTES, GIT_PATH_FS_HFS)); cl_git_fail(git_path_is_gitfile("blob", 4, 3, GIT_PATH_FS_HFS)); } + +void test_core_path__validate_current_user_ownership(void) +{ + bool is_cur; + + cl_must_pass(p_mkdir("testdir", 0777)); + cl_git_pass(git_path_owner_is_current_user(&is_cur, "testdir")); + cl_assert_equal_i(is_cur, 1); + + cl_git_rewritefile("testfile", "This is a test file."); + cl_git_pass(git_path_owner_is_current_user(&is_cur, "testfile")); + cl_assert_equal_i(is_cur, 1); + +#ifdef GIT_WIN32 + cl_git_pass(git_path_owner_is_current_user(&is_cur, "C:\\")); + cl_assert_equal_i(is_cur, 0); + + cl_git_fail(git_path_owner_is_current_user(&is_cur, "c:\\path\\does\\not\\exist")); +#else + cl_git_pass(git_path_owner_is_current_user(&is_cur, "/")); + cl_assert_equal_i(is_cur, 0); + + cl_git_fail(git_path_owner_is_current_user(&is_cur, "/path/does/not/exist")); +#endif +} From 62d492dee448d98ef61d33680bbd7de614ce8fd8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 09:56:26 -0400 Subject: [PATCH 12/27] repo: ensure that repo dir is owned by current user Ensure that the repository directory is owned by the current user; this prevents us from opening configuration files that may have been created by an attacker. --- include/git2/errors.h | 1 + src/repository.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index de51582d58e..16712f988b9 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -58,6 +58,7 @@ typedef enum { GIT_EMISMATCH = -33, /**< Hashsum mismatch in object */ GIT_EINDEXDIRTY = -34, /**< Unsaved changes in the index would be overwritten */ GIT_EAPPLYFAIL = -35, /**< Patch application failed */ + GIT_EOWNER = -36 /**< The object is not owned by the current user */ } git_error_code; /** diff --git a/src/repository.c b/src/repository.c index 9b3e9c9e3b1..4dace9da972 100644 --- a/src/repository.c +++ b/src/repository.c @@ -482,6 +482,23 @@ static int read_gitfile(git_buf *path_out, const char *file_path) return error; } +static int validate_ownership(const char *repo_path) +{ + bool is_safe; + int error; + + if ((error = git_path_owner_is_current_user(&is_safe, repo_path)) < 0) + return (error == GIT_ENOTFOUND) ? 0 : error; + + if (is_safe) + return 0; + + git_error_set(GIT_ERROR_CONFIG, + "repository path '%s' is not owned by current user", + repo_path); + return GIT_EOWNER; +} + static int find_repo( git_buf *gitdir_path, git_buf *workdir_path, @@ -855,6 +872,7 @@ int git_repository_open_ext( gitlink = GIT_BUF_INIT, commondir = GIT_BUF_INIT; git_repository *repo = NULL; git_config *config = NULL; + const char *validation_path; int version = 0; if (flags & GIT_REPOSITORY_OPEN_FROM_ENV) @@ -903,16 +921,23 @@ int git_repository_open_ext( if ((error = check_extensions(config, version)) < 0) goto cleanup; - if ((flags & GIT_REPOSITORY_OPEN_BARE) != 0) + if ((flags & GIT_REPOSITORY_OPEN_BARE) != 0) { repo->is_bare = 1; - else { - + } else { if (config && ((error = load_config_data(repo, config)) < 0 || (error = load_workdir(repo, config, &workdir)) < 0)) goto cleanup; } + /* + * Ensure that the git directory is owned by the current user. + */ + validation_path = repo->is_bare ? repo->gitdir : repo->workdir; + + if ((error = validate_ownership(validation_path)) < 0) + goto cleanup; + cleanup: git_buf_dispose(&gitdir); git_buf_dispose(&workdir); From e4eabb030ebfdf818a116ad6e8ca9c0d0a3c9e92 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 23:47:01 -0400 Subject: [PATCH 13/27] fs_path: mock ownership checks Provide a mock for file ownership for testability. --- src/path.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/path.h | 14 ++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/path.c b/src/path.c index b3bb0b40825..02c4e5e04da 100644 --- a/src/path.c +++ b/src/path.c @@ -2024,6 +2024,13 @@ bool git_path_supports_symlinks(const char *dir) return supported; } +static git_path__mock_owner_t mock_owner = GIT_PATH_MOCK_OWNER_NONE; + +void git_path__set_owner(git_path__mock_owner_t owner) +{ + mock_owner = owner; +} + #ifdef GIT_WIN32 static PSID *sid_dup(PSID sid) { @@ -2116,6 +2123,11 @@ int git_path_owner_is_current_user(bool *out, const char *path) PSID owner_sid = NULL, user_sid = NULL; int error = -1; + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_CURRENT_USER); + return 0; + } + if ((error = file_owner_sid(&owner_sid, path)) < 0 || (error = current_user_sid(&user_sid)) < 0) goto done; @@ -2133,6 +2145,11 @@ int git_path_owner_is_system(bool *out, const char *path) { PSID owner_sid; + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_SYSTEM); + return 0; + } + if (file_owner_sid(&owner_sid, path) < 0) return -1; @@ -2148,6 +2165,12 @@ int git_path_owner_is_system_or_current_user(bool *out, const char *path) PSID owner_sid = NULL, user_sid = NULL; int error = -1; + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_SYSTEM || + mock_owner == GIT_PATH_MOCK_OWNER_CURRENT_USER); + return 0; + } + if (file_owner_sid(&owner_sid, path) < 0) goto done; @@ -2200,18 +2223,37 @@ static int path_owner_is(bool *out, const char *path, uid_t *uids, size_t uids_l int git_path_owner_is_current_user(bool *out, const char *path) { uid_t userid = geteuid(); + + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_CURRENT_USER); + return 0; + } + return path_owner_is(out, path, &userid, 1); } int git_path_owner_is_system(bool *out, const char *path) { uid_t userid = 0; + + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_SYSTEM); + return 0; + } + return path_owner_is(out, path, &userid, 1); } int git_path_owner_is_system_or_current_user(bool *out, const char *path) { uid_t userids[2] = { geteuid(), 0 }; + + if (mock_owner) { + *out = (mock_owner == GIT_PATH_MOCK_OWNER_SYSTEM || + mock_owner == GIT_PATH_MOCK_OWNER_CURRENT_USER); + return 0; + } + return path_owner_is(out, path, userids, 2); } diff --git a/src/path.h b/src/path.h index 699945bd70e..e0447d74800 100644 --- a/src/path.h +++ b/src/path.h @@ -722,6 +722,20 @@ int git_path_normalize_slashes(git_buf *out, const char *path); bool git_path_supports_symlinks(const char *dir); +typedef enum { + GIT_PATH_MOCK_OWNER_NONE = 0, /* do filesystem lookups as normal */ + GIT_PATH_MOCK_OWNER_SYSTEM = 1, + GIT_PATH_MOCK_OWNER_CURRENT_USER = 2, + GIT_PATH_MOCK_OWNER_OTHER = 3 +} git_path__mock_owner_t; + +/** + * Sets the mock ownership for files; subsequent calls to + * `git_path_owner_is_*` functions will return this data until cleared + * with `GIT_PATH_MOCK_OWNER_NONE`. + */ +void git_path__set_owner(git_path__mock_owner_t owner); + /** * Verify that the file in question is owned by an administrator or system * account. From caee92ee03694d33b4ce02b867696ca60c04fda2 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 17:07:20 +0100 Subject: [PATCH 14/27] repo: test configuration ownership validation Test that we prevent opening directories that are not owned by ourselves. --- tests/repo/config.c | 1 - tests/repo/open.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/repo/config.c b/tests/repo/config.c index 6ca31f550a7..93d9e65a30b 100644 --- a/tests/repo/config.c +++ b/tests/repo/config.c @@ -28,7 +28,6 @@ void test_repo_config__cleanup(void) cl_assert(!git_path_isdir("alternate")); cl_fixture_cleanup("empty_standard_repo"); - } void test_repo_config__can_open_global_when_there_is_no_file(void) diff --git a/tests/repo/open.c b/tests/repo/open.c index bd60c12c29b..c7e7a4ccf7b 100644 --- a/tests/repo/open.c +++ b/tests/repo/open.c @@ -7,9 +7,12 @@ void test_repo_open__cleanup(void) { cl_git_sandbox_cleanup(); + cl_fixture_cleanup("empty_standard_repo"); if (git_path_isdir("alternate")) git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES); + + git_path__set_owner(GIT_PATH_MOCK_OWNER_NONE); } void test_repo_open__bare_empty_repo(void) @@ -453,3 +456,35 @@ void test_repo_open__force_bare(void) git_repository_free(barerepo); } +void test_repo_open__validates_dir_ownership(void) +{ + git_repository *repo; + + cl_fixture_sandbox("empty_standard_repo"); + cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); + + /* When the current user owns the repo config, that's acceptable */ + git_path__set_owner(GIT_PATH_MOCK_OWNER_CURRENT_USER); + cl_git_pass(git_repository_open(&repo, "empty_standard_repo")); + git_repository_free(repo); + + /* When the system user owns the repo config, fail */ + git_path__set_owner(GIT_PATH_MOCK_OWNER_SYSTEM); + cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); + + /* When an unknown user owns the repo config, fail */ + git_path__set_owner(GIT_PATH_MOCK_OWNER_OTHER); + cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); +} + +void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) +{ + git_repository *repo; + + cl_fixture_sandbox("empty_standard_repo"); + cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); + + git_path__set_owner(GIT_PATH_MOCK_OWNER_OTHER); + cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); + +} From f68380665b05106520f1a13244564692bc16b424 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 13:04:26 -0400 Subject: [PATCH 15/27] repo: refactor global config loader function Pull the global configuration loader out of the symlink check so that it can be re-used. --- src/repository.c | 51 ++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/src/repository.c b/src/repository.c index 4dace9da972..e8ea1e75c5a 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1634,13 +1634,40 @@ static bool is_filesystem_case_insensitive(const char *gitdir_path) return is_insensitive; } -static bool are_symlinks_supported(const char *wd_path) +/* + * Return a configuration object with only the global and system + * configurations; no repository-level configuration. + */ +static int load_global_config(git_config **config) { - git_config *config = NULL; git_buf global_buf = GIT_BUF_INIT; git_buf xdg_buf = GIT_BUF_INIT; git_buf system_buf = GIT_BUF_INIT; git_buf programdata_buf = GIT_BUF_INIT; + int error; + + git_config_find_global(&global_buf); + git_config_find_xdg(&xdg_buf); + git_config_find_system(&system_buf); + git_config_find_programdata(&programdata_buf); + + error = load_config(config, NULL, + path_unless_empty(&global_buf), + path_unless_empty(&xdg_buf), + path_unless_empty(&system_buf), + path_unless_empty(&programdata_buf)); + + git_buf_dispose(&global_buf); + git_buf_dispose(&xdg_buf); + git_buf_dispose(&system_buf); + git_buf_dispose(&programdata_buf); + + return error; +} + +static bool are_symlinks_supported(const char *wd_path) +{ + git_config *config = NULL; int symlinks = 0; /* @@ -1651,19 +1678,9 @@ static bool are_symlinks_supported(const char *wd_path) * _not_ set, then we do not test or enable symlink support. */ #ifdef GIT_WIN32 - git_config_find_global(&global_buf); - git_config_find_xdg(&xdg_buf); - git_config_find_system(&system_buf); - git_config_find_programdata(&programdata_buf); - - if (load_config(&config, NULL, - path_unless_empty(&global_buf), - path_unless_empty(&xdg_buf), - path_unless_empty(&system_buf), - path_unless_empty(&programdata_buf)) < 0) - goto done; - - if (git_config_get_bool(&symlinks, config, "core.symlinks") < 0 || !symlinks) + if (load_global_config(&config) < 0 || + git_config_get_bool(&symlinks, config, "core.symlinks") < 0 || + !symlinks) goto done; #endif @@ -1671,10 +1688,6 @@ static bool are_symlinks_supported(const char *wd_path) goto done; done: - git_buf_dispose(&global_buf); - git_buf_dispose(&xdg_buf); - git_buf_dispose(&system_buf); - git_buf_dispose(&programdata_buf); git_config_free(config); return symlinks != 0; } From eb8c3e5dabdeaeb51d8fea39545b4c0c9ddff07a Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 15:18:44 -0400 Subject: [PATCH 16/27] repo: honor safe.directory during ownership checks Obey the `safe.directory` configuration variable if it is set in the global or system configuration. (Do not try to load this from the repository configuration - to avoid malicious repositories that then mark themselves as safe.) --- src/repository.c | 51 ++++++++++++++++++++--- tests/repo/open.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/repository.c b/src/repository.c index e8ea1e75c5a..ac2581167c6 100644 --- a/src/repository.c +++ b/src/repository.c @@ -64,6 +64,7 @@ static const struct { static int check_repositoryformatversion(int *version, git_config *config); static int check_extensions(git_config *config, int version); +static int load_global_config(git_config **config); #define GIT_COMMONDIR_FILE "commondir" #define GIT_GITDIR_FILE "gitdir" @@ -482,21 +483,61 @@ static int read_gitfile(git_buf *path_out, const char *file_path) return error; } +typedef struct { + const char *repo_path; + git_buf tmp; + bool is_safe; +} validate_ownership_data; + +static int validate_ownership_cb(const git_config_entry *entry, void *payload) +{ + validate_ownership_data *data = payload; + + if (strcmp(entry->value, "") == 0) + data->is_safe = false; + + if (git_path_prettify_dir(&data->tmp, entry->value, NULL) == 0 && + strcmp(data->tmp.ptr, data->repo_path) == 0) + data->is_safe = true; + + return 0; +} + static int validate_ownership(const char *repo_path) { + git_config *config = NULL; + validate_ownership_data data = { repo_path, GIT_BUF_INIT, false }; bool is_safe; int error; - if ((error = git_path_owner_is_current_user(&is_safe, repo_path)) < 0) - return (error == GIT_ENOTFOUND) ? 0 : error; + if ((error = git_path_owner_is_current_user(&is_safe, repo_path)) < 0) { + if (error == GIT_ENOTFOUND) + error = 0; - if (is_safe) - return 0; + goto done; + } + + if (is_safe) { + error = 0; + goto done; + } + + if (load_global_config(&config) == 0) { + error = git_config_get_multivar_foreach(config, "safe.directory", NULL, validate_ownership_cb, &data); + + if (!error && data.is_safe) + goto done; + } git_error_set(GIT_ERROR_CONFIG, "repository path '%s' is not owned by current user", repo_path); - return GIT_EOWNER; + error = GIT_EOWNER; + +done: + git_config_free(config); + git_buf_dispose(&data.tmp); + return error; } static int find_repo( diff --git a/tests/repo/open.c b/tests/repo/open.c index c7e7a4ccf7b..f23ba1c180f 100644 --- a/tests/repo/open.c +++ b/tests/repo/open.c @@ -3,16 +3,26 @@ #include "sysdir.h" #include +static git_buf config_path = GIT_BUF_INIT; + +void test_repo_open__initialize(void) +{ + cl_git_pass(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, &config_path)); +} void test_repo_open__cleanup(void) { cl_git_sandbox_cleanup(); cl_fixture_cleanup("empty_standard_repo"); + cl_fixture_cleanup("__global_config"); if (git_path_isdir("alternate")) git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES); git_path__set_owner(GIT_PATH_MOCK_OWNER_NONE); + + cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr)); + git_buf_dispose(&config_path); } void test_repo_open__bare_empty_repo(void) @@ -480,6 +490,9 @@ void test_repo_open__validates_dir_ownership(void) void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) { git_repository *repo; + git_buf config_path = GIT_BUF_INIT, + config_filename = GIT_BUF_INIT, + config_data = GIT_BUF_INIT; cl_fixture_sandbox("empty_standard_repo"); cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); @@ -487,4 +500,93 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) git_path__set_owner(GIT_PATH_MOCK_OWNER_OTHER); cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); + /* Add safe.directory options to the global configuration */ + git_buf_joinpath(&config_path, clar_sandbox_path(), "__global_config"); + cl_must_pass(p_mkdir(config_path.ptr, 0777)); + git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr); + + git_buf_joinpath(&config_filename, config_path.ptr, ".gitconfig"); + + git_buf_printf(&config_data, + "[foo]\n" \ + "\tbar = Foobar\n" \ + "\tbaz = Baz!\n" \ + "[safe]\n" \ + "\tdirectory = /non/existent/path\n" \ + "\tdirectory = /\n" \ + "\tdirectory = c:\\\\temp\n" \ + "\tdirectory = %s/%s\n" \ + "\tdirectory = /tmp\n" \ + "[bar]\n" \ + "\tfoo = barfoo\n", + clar_sandbox_path(), "empty_standard_repo"); + cl_git_rewritefile(config_filename.ptr, config_data.ptr); + + cl_git_pass(git_repository_open(&repo, "empty_standard_repo")); + git_repository_free(repo); + + git_buf_dispose(&config_path); + git_buf_dispose(&config_filename); + git_buf_dispose(&config_data); +} + +void test_repo_open__can_reset_safe_directory_list(void) +{ + git_repository *repo; + git_buf config_path = GIT_BUF_INIT, + config_filename = GIT_BUF_INIT, + config_data = GIT_BUF_INIT; + + cl_fixture_sandbox("empty_standard_repo"); + cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); + + git_path__set_owner(GIT_PATH_MOCK_OWNER_OTHER); + cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); + + /* Add safe.directory options to the global configuration */ + git_buf_joinpath(&config_path, clar_sandbox_path(), "__global_config"); + cl_must_pass(p_mkdir(config_path.ptr, 0777)); + git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr); + + git_buf_joinpath(&config_filename, config_path.ptr, ".gitconfig"); + + /* The blank resets our sandbox directory and opening fails */ + + git_buf_printf(&config_data, + "[foo]\n" \ + "\tbar = Foobar\n" \ + "\tbaz = Baz!\n" \ + "[safe]\n" \ + "\tdirectory = %s/%s\n" \ + "\tdirectory = \n" \ + "\tdirectory = /tmp\n" \ + "[bar]\n" \ + "\tfoo = barfoo\n", + clar_sandbox_path(), "empty_standard_repo"); + cl_git_rewritefile(config_filename.ptr, config_data.ptr); + + cl_git_fail(git_repository_open(&repo, "empty_standard_repo")); + + /* The blank resets tmp and allows subsequent declarations to succeed */ + + git_buf_clear(&config_data); + git_buf_printf(&config_data, + "[foo]\n" \ + "\tbar = Foobar\n" \ + "\tbaz = Baz!\n" \ + "[safe]\n" \ + "\tdirectory = /tmp\n" \ + "\tdirectory = \n" \ + "\tdirectory = %s/%s\n" \ + "[bar]\n" \ + "\tfoo = barfoo\n", + clar_sandbox_path(), "empty_standard_repo"); + cl_git_rewritefile(config_filename.ptr, config_data.ptr); + + cl_git_pass(git_repository_open(&repo, "empty_standard_repo")); + git_repository_free(repo); + + git_buf_dispose(&config_path); + git_buf_dispose(&config_filename); + git_buf_dispose(&config_data); } From b58e9053b43f8487b1bf523b2259f76cb868105d Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2022 21:31:25 -0400 Subject: [PATCH 17/27] repo: make ownership checks optional Introduce the `GIT_OPT_SET_OWNER_VALIDATION` option, so that users can disable repository ownership validation. --- include/git2/common.h | 12 +++++++++++- src/libgit2.c | 8 ++++++++ src/repository.c | 4 +++- src/repository.h | 1 + tests/clar_libgit2.c | 5 +++++ tests/clar_libgit2.h | 1 + tests/main.c | 1 + tests/repo/open.c | 10 ++++++++++ 8 files changed, 40 insertions(+), 2 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 2ee82902529..134ae602474 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -211,7 +211,9 @@ typedef enum { GIT_OPT_SET_ODB_PACKED_PRIORITY, GIT_OPT_SET_ODB_LOOSE_PRIORITY, GIT_OPT_GET_EXTENSIONS, - GIT_OPT_SET_EXTENSIONS + GIT_OPT_SET_EXTENSIONS, + GIT_OPT_GET_OWNER_VALIDATION, + GIT_OPT_SET_OWNER_VALIDATION } git_libgit2_opt_t; /** @@ -449,6 +451,14 @@ typedef enum { * > to support repositories with the `noop` extension but does want * > to support repositories with the `newext` extension. * + * opts(GIT_OPT_GET_OWNER_VALIDATION, int *enabled) + * > Gets the owner validation setting for repository + * > directories. + * + * opts(GIT_OPT_SET_OWNER_VALIDATION, int enabled) + * > Set that repository directories should be owned by the current + * > user. The default is to validate ownership. + * * @param option Option key * @param ... value to set the option * @return 0 on success, <0 on failure diff --git a/src/libgit2.c b/src/libgit2.c index cc793b4587f..dc73fba8ba5 100644 --- a/src/libgit2.c +++ b/src/libgit2.c @@ -390,6 +390,14 @@ int git_libgit2_opts(int key, ...) } break; + case GIT_OPT_GET_OWNER_VALIDATION: + *(va_arg(ap, int *)) = git_repository__validate_ownership; + break; + + case GIT_OPT_SET_OWNER_VALIDATION: + git_repository__validate_ownership = (va_arg(ap, int) != 0); + break; + default: git_error_set(GIT_ERROR_INVALID, "invalid option key"); error = -1; diff --git a/src/repository.c b/src/repository.c index ac2581167c6..cc69d9692a9 100644 --- a/src/repository.c +++ b/src/repository.c @@ -38,6 +38,7 @@ # include "win32/w32_util.h" #endif +bool git_repository__validate_ownership = true; bool git_repository__fsync_gitdir = false; static const struct { @@ -976,7 +977,8 @@ int git_repository_open_ext( */ validation_path = repo->is_bare ? repo->gitdir : repo->workdir; - if ((error = validate_ownership(validation_path)) < 0) + if (git_repository__validate_ownership && + (error = validate_ownership(validation_path)) < 0) goto cleanup; cleanup: diff --git a/src/repository.h b/src/repository.h index cbc160140f3..b0c326a14e4 100644 --- a/src/repository.h +++ b/src/repository.h @@ -34,6 +34,7 @@ #define GIT_DIR_SHORTNAME "GIT~1" extern bool git_repository__fsync_gitdir; +extern bool git_repository__validate_ownership; /** Cvar cache identifiers */ typedef enum { diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c index c4550c32a8b..3b2473cdc89 100644 --- a/tests/clar_libgit2.c +++ b/tests/clar_libgit2.c @@ -603,6 +603,11 @@ void cl_sandbox_set_search_path_defaults(void) git_buf_dispose(&path); } +void cl_sandbox_disable_ownership_validation(void) +{ + git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 0); +} + #ifdef GIT_WIN32 bool cl_sandbox_supports_8dot3(void) { diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h index e3b7bd9f83f..da3f4152458 100644 --- a/tests/clar_libgit2.h +++ b/tests/clar_libgit2.h @@ -222,6 +222,7 @@ void cl_fake_home(void); void cl_fake_home_cleanup(void *); void cl_sandbox_set_search_path_defaults(void); +void cl_sandbox_disable_ownership_validation(void); #ifdef GIT_WIN32 # define cl_msleep(x) Sleep(x) diff --git a/tests/main.c b/tests/main.c index 56751c28869..d879073a825 100644 --- a/tests/main.c +++ b/tests/main.c @@ -26,6 +26,7 @@ int main(int argc, char *argv[]) cl_global_trace_register(); cl_sandbox_set_search_path_defaults(); + cl_sandbox_disable_ownership_validation(); /* Run the test suite */ res = clar_test_run(); diff --git a/tests/repo/open.c b/tests/repo/open.c index f23ba1c180f..a2f006c0d6c 100644 --- a/tests/repo/open.c +++ b/tests/repo/open.c @@ -3,11 +3,13 @@ #include "sysdir.h" #include +static int validate_ownership = 0; static git_buf config_path = GIT_BUF_INIT; void test_repo_open__initialize(void) { cl_git_pass(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, &config_path)); + cl_git_pass(git_libgit2_opts(GIT_OPT_GET_OWNER_VALIDATION, &validate_ownership)); } void test_repo_open__cleanup(void) @@ -23,6 +25,8 @@ void test_repo_open__cleanup(void) cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr)); git_buf_dispose(&config_path); + + cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, validate_ownership)); } void test_repo_open__bare_empty_repo(void) @@ -470,6 +474,8 @@ void test_repo_open__validates_dir_ownership(void) { git_repository *repo; + cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 1)); + cl_fixture_sandbox("empty_standard_repo"); cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); @@ -494,6 +500,8 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) config_filename = GIT_BUF_INIT, config_data = GIT_BUF_INIT; + cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 1)); + cl_fixture_sandbox("empty_standard_repo"); cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); @@ -537,6 +545,8 @@ void test_repo_open__can_reset_safe_directory_list(void) config_filename = GIT_BUF_INIT, config_data = GIT_BUF_INIT; + cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 1)); + cl_fixture_sandbox("empty_standard_repo"); cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); From 1f39aacc4308e343890b77655e0215fff823e520 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Apr 2022 15:52:47 -0400 Subject: [PATCH 18/27] meta: update version numbers for v1.3.1 --- CMakeLists.txt | 2 +- include/git2/version.h | 4 ++-- package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3dccec3109a..893361e741f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,7 +13,7 @@ CMAKE_MINIMUM_REQUIRED(VERSION 3.5.1) -project(libgit2 VERSION "1.3.0" LANGUAGES C) +project(libgit2 VERSION "1.3.1" LANGUAGES C) # Add find modules to the path set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${libgit2_SOURCE_DIR}/cmake/") diff --git a/include/git2/version.h b/include/git2/version.h index 3503a62781a..738789dac2b 100644 --- a/include/git2/version.h +++ b/include/git2/version.h @@ -7,10 +7,10 @@ #ifndef INCLUDE_git_version_h__ #define INCLUDE_git_version_h__ -#define LIBGIT2_VERSION "1.3.0" +#define LIBGIT2_VERSION "1.3.1" #define LIBGIT2_VER_MAJOR 1 #define LIBGIT2_VER_MINOR 3 -#define LIBGIT2_VER_REVISION 0 +#define LIBGIT2_VER_REVISION 1 #define LIBGIT2_VER_PATCH 0 #define LIBGIT2_SOVERSION "1.3" diff --git a/package.json b/package.json index e2e672f9fe6..42f8a5c2ec9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "libgit2", - "version": "1.3.0", + "version": "1.3.1", "repo": "https://github.com/libgit2/libgit2", "description": " A cross-platform, linkable library implementation of Git that you can use in your application.", "install": "mkdir build && cd build && cmake .. && cmake --build ." From 23c24f80e362c5a6e3200a21d9617969b06c8957 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Apr 2022 15:54:26 -0400 Subject: [PATCH 19/27] meta: changelog for v1.3.1 --- docs/changelog.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index 8060874df0e..31c3bd0e7bc 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,3 +1,18 @@ +v1.3.1 +------ + +🔒 This is a security release to provide compatibility with git's changes to address [CVE 2022-24765](https://github.blog/2022-04-12-git-security-vulnerability-announced/). + +**libgit2 is not directly affected** by this vulnerability, because libgit2 does not directly invoke any executable. But we are providing these changes as a security release for any users that use libgit2 for repository discovery and then _also_ use git on that repository. In this release, we will now validate that the user opening the repository is the same user that owns the on-disk repository. This is to match git's behavior. + +In addition, we are providing several correctness fixes where invalid input can lead to a crash. These may prevent possible denial of service attacks. At this time there are not known exploits to these issues. + +Full list of changes: + +* Validate repository directory ownership (v1.3) by @ethomson in https://github.com/libgit2/libgit2/pull/6268 + +All users of the v1.3 release line are recommended to upgrade. + v1.3 ---- From 4b193b1410c563196f75d644ff6cefd5d6510f5d Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Fri, 6 May 2022 19:52:39 +0200 Subject: [PATCH 20/27] New checkout option: disabled_filters It is a string array of the filters you want to disable in checkout --- include/git2/checkout.h | 3 ++- src/checkout.c | 5 +++-- src/filter.c | 15 +++++++++++++++ src/filter.h | 1 + 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index c7aeee43120..787f50205c3 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -182,7 +182,7 @@ typedef enum { * notifications; don't update the working directory or index. */ GIT_CHECKOUT_DRY_RUN = (1u << 24), - + /** * THE FOLLOWING OPTIONS ARE NOT YET IMPLEMENTED */ @@ -339,6 +339,7 @@ typedef struct git_checkout_options { /** Payload passed to perfdata_cb */ void *perfdata_payload; + git_strarray disabled_filters; } git_checkout_options; #define GIT_CHECKOUT_OPTIONS_VERSION 1 diff --git a/src/checkout.c b/src/checkout.c index d9afea980d2..a8134f7f4f1 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1567,7 +1567,7 @@ static int blob_content_to_file( filter_session.attr_session = &data->attr_session; filter_session.temp_buf = &buffers->tmp; - + filter_session.disabled_filters = &data->opts.disabled_filters; if (!data->opts.disable_filters) { git_mutex_lock(&data->index_mutex); @@ -2425,6 +2425,7 @@ static int checkout_write_merge( filter_session.attr_session = &data->attr_session; filter_session.temp_buf = &buffers->tmp; + filter_session.disabled_filters = &data->opts.disabled_filters; if ((error = git_filter_list__load( &fl, data->repo, NULL, result.path, @@ -2998,7 +2999,7 @@ int git_checkout_iterator( if (data.strategy & GIT_CHECKOUT_DRY_RUN) goto cleanup; - + data.total_steps = counts[CHECKOUT_ACTION__REMOVE] + counts[CHECKOUT_ACTION__REMOVE_CONFLICT] + counts[CHECKOUT_ACTION__UPDATE_BLOB] + diff --git a/src/filter.c b/src/filter.c index 73497cb3024..b66b0343e0b 100644 --- a/src/filter.c +++ b/src/filter.c @@ -514,6 +514,8 @@ int git_filter_list__load( git_filter_session *filter_session) { int error = 0; + int i; + int disabled_filter_found; git_filter_list *fl = NULL; git_filter_source src = { 0 }; git_filter_entry *fe; @@ -541,6 +543,19 @@ int git_filter_list__load( if (!fdef || !fdef->filter) continue; + if (filter_session->disabled_filters) { + disabled_filter_found = 0; + for (i = 0; i < filter_session->disabled_filters->count; ++i) { + char *filter = filter_session->disabled_filters->strings[i]; + if (!strcmp(filter, fdef->filter_name)) { + disabled_filter_found = 1; + break; + } + } + if (disabled_filter_found) + continue; + } + if (fdef->nattrs > 0) { error = filter_list_check_attributes( &values, repo, diff --git a/src/filter.h b/src/filter.h index 241791276cd..9d25712f90e 100644 --- a/src/filter.h +++ b/src/filter.h @@ -20,6 +20,7 @@ typedef struct { git_filter_options options; git_attr_session *attr_session; git_buf *temp_buf; + git_strarray *disabled_filters; } git_filter_session; #define GIT_FILTER_SESSION_INIT {GIT_FILTER_OPTIONS_INIT, 0} From e78ee33f40daba15ccb44b3a7855e71ea5b7138c Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 18 May 2022 18:15:20 +0200 Subject: [PATCH 21/27] Fix degraded performance using GIT_USE_NSEC on repos cloned with GIT_USE_NSEC disabled --- src/index.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/index.h b/src/index.h index a365867d0bf..40e1f005549 100644 --- a/src/index.h +++ b/src/index.h @@ -84,6 +84,8 @@ GIT_INLINE(bool) git_index_time_eq(const git_index_time *one, const git_index_ti return false; #ifdef GIT_USE_NSEC + if (one->nanoseconds == 0 || two->nanoseconds == 0) + return true; if (one->nanoseconds != two->nanoseconds) return false; #endif @@ -109,6 +111,8 @@ GIT_INLINE(bool) git_index_entry_newer_than_index( return true; else if ((int32_t)index->stamp.mtime.tv_sec > entry->mtime.seconds) return false; + else if (entry->mtime.nanoseconds == 0 || index->stamp.mtime.tv_sec == 0) + return true; else return (uint32_t)index->stamp.mtime.tv_nsec <= entry->mtime.nanoseconds; #else From 3ad710a34524ac400fd14f59a50af81ca334abba Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Thu, 26 May 2022 15:52:32 +0200 Subject: [PATCH 22/27] Fix the GIT_USE_NSEC performance fix There is a typo, Instead comparing nanoseconds to zero the code is comparing seconds to zero --- src/index.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.h b/src/index.h index 40e1f005549..f6f813d9162 100644 --- a/src/index.h +++ b/src/index.h @@ -111,7 +111,7 @@ GIT_INLINE(bool) git_index_entry_newer_than_index( return true; else if ((int32_t)index->stamp.mtime.tv_sec > entry->mtime.seconds) return false; - else if (entry->mtime.nanoseconds == 0 || index->stamp.mtime.tv_sec == 0) + else if (entry->mtime.nanoseconds == 0 || index->stamp.mtime.tv_nsec == 0) return true; else return (uint32_t)index->stamp.mtime.tv_nsec <= entry->mtime.nanoseconds; From 8254d2e2aedde8edf6641cea729626d750591205 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 20 Jun 2022 09:26:55 +0200 Subject: [PATCH 23/27] Do not add the .gitignore file if it not existing --- src/ignore.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/ignore.c b/src/ignore.c index 9ead96ba67e..c3429b23334 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -370,13 +370,40 @@ int git_ignore__for_path( return error; } +char * concat_path_ign_file(git_buf *path) +{ + char *path_file = NULL; + + path_file = git__malloc(path->size + 11); + if (path_file == NULL) + return NULL; + + memcpy(path_file, path->ptr, path->size); + memcpy(path_file + path->size, GIT_IGNORE_FILE, 11); + + return path_file; +} + int git_ignore__push_dir(git_ignores *ign, const char *dir) { + char *file_ign = NULL; + struct stat st; + if (git_buf_joinpath(&ign->dir, ign->dir.ptr, dir) < 0) return -1; ign->depth++; + // Do not add the .gitignore file if it not existing + file_ign = concat_path_ign_file(&ign->dir); + if (file_ign == NULL) + return -1; + if (p_stat(file_ign, &st) < 0) { + free(file_ign); + return 0; + } + free(file_ign); + return push_ignore_file( ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE); } From 45f0e262f532a1bde03c8d35d8cdc0b471974a51 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 20 Jun 2022 09:30:47 +0200 Subject: [PATCH 24/27] iterator: don't stat directories Original commit: https://github.com/tiennou/libgit2/commit/5911878943ca8f576181babb8930af9825372dbb --- src/iterator.c | 73 ++++++++++++++++++++++++++++---------------------- src/path.c | 2 ++ src/path.h | 1 + 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index ce9f305ef28..647909e66d6 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1343,6 +1343,7 @@ static int filesystem_iterator_frame_push( struct stat statbuf; size_t path_len; int error; + bool submodule = false; if (iter->frames.size == FILESYSTEM_MAX_DEPTH) { git_error_set(GIT_ERROR_REPOSITORY, @@ -1410,50 +1411,58 @@ static int filesystem_iterator_frame_push( iter, frame_entry, path, path_len)) continue; + if (filesystem_iterator_is_dot_git(iter, path, path_len)) + continue; + /* TODO: don't need to stat if assume unchanged for this path and * we have an index, we can just copy the data out of it. */ - if ((error = git_path_diriter_stat(&statbuf, &diriter)) < 0) { - /* file was removed between readdir and lstat */ - if (error == GIT_ENOTFOUND) - continue; - - /* treat the file as unreadable */ - memset(&statbuf, 0, sizeof(statbuf)); - statbuf.st_mode = GIT_FILEMODE_UNREADABLE; - - error = 0; - } + if (diriter.d_type == DT_DIR && + !(error = filesystem_iterator_is_submodule(&submodule, iter, path, path_len)) && + !submodule) { + // It's a directory, no need to lstat it. + statbuf.st_mode = S_IFDIR; + } else if (error < 0) { + goto done; + } else { + if ((error = git_path_diriter_stat(&statbuf, &diriter)) < 0) { + /* file was removed between readdir and lstat */ + if (error == GIT_ENOTFOUND) + continue; - iter->base.stat_calls++; + /* treat the file as unreadable */ + memset(&statbuf, 0, sizeof(statbuf)); + statbuf.st_mode = GIT_FILEMODE_UNREADABLE; - /* Ignore wacky things in the filesystem */ - if (!S_ISDIR(statbuf.st_mode) && - !S_ISREG(statbuf.st_mode) && - !S_ISLNK(statbuf.st_mode) && - statbuf.st_mode != GIT_FILEMODE_UNREADABLE) - continue; + error = 0; + } - if (filesystem_iterator_is_dot_git(iter, path, path_len)) - continue; + iter->base.stat_calls++; - /* convert submodules to GITLINK and remove trailing slashes */ - if (S_ISDIR(statbuf.st_mode)) { - bool submodule = false; + /* Ignore wacky things in the filesystem */ + if (!S_ISDIR(statbuf.st_mode) && + !S_ISREG(statbuf.st_mode) && + !S_ISLNK(statbuf.st_mode) && + statbuf.st_mode != GIT_FILEMODE_UNREADABLE) + continue; - if ((error = filesystem_iterator_is_submodule(&submodule, - iter, path, path_len)) < 0) - goto done; + /* Ensure that the pathlist entry lines up with what we expected */ + if (dir_expected && !S_ISDIR(statbuf.st_mode)) + continue; - if (submodule) - statbuf.st_mode = GIT_FILEMODE_COMMIT; + /* convert submodules to GITLINK and remove trailing slashes */ + if (S_ISDIR(statbuf.st_mode)) { + if (!submodule) { + if ((error = filesystem_iterator_is_submodule(&submodule, + iter, path, path_len)) < 0) + goto done; + } + if (submodule) + statbuf.st_mode = GIT_FILEMODE_COMMIT; + } } - /* Ensure that the pathlist entry lines up with what we expected */ - else if (dir_expected) - continue; - if ((error = filesystem_iterator_entry_init(&entry, iter, new_frame, path, path_len, &statbuf, pathlist_match)) < 0) goto done; diff --git a/src/path.c b/src/path.c index 02c4e5e04da..ff9462e51eb 100644 --- a/src/path.c +++ b/src/path.c @@ -1430,6 +1430,8 @@ int git_path_diriter_next(git_path_diriter *diriter) if (git_buf_oom(&diriter->path)) return -1; + diriter->d_type = de->d_type; + return error; } diff --git a/src/path.h b/src/path.h index e0447d74800..176ea9e271c 100644 --- a/src/path.h +++ b/src/path.h @@ -482,6 +482,7 @@ struct git_path_diriter size_t parent_len; unsigned int flags; + unsigned char d_type; DIR *dir; From a4c112d3e7c277626aab0059a343cff9b8ebb960 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 20 Jun 2022 09:31:47 +0200 Subject: [PATCH 25/27] path: use fstatat instead of lstat Original commit: https://github.com/tiennou/libgit2/commit/bb2a754712eea952f8da24a30d6431f63f445174 --- src/path.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/path.c b/src/path.c index ff9462e51eb..86946a19b91 100644 --- a/src/path.c +++ b/src/path.c @@ -1469,7 +1469,11 @@ int git_path_diriter_stat(struct stat *out, git_path_diriter *diriter) GIT_ASSERT_ARG(out); GIT_ASSERT_ARG(diriter); - return git_path_lstat(diriter->path.ptr, out); + const char *fname; + fname = diriter->path.ptr + diriter->parent_len; + if (*fname == '/') ++fname; + + return fstatat(dirfd(diriter->dir), fname, out, AT_SYMLINK_NOFOLLOW); } void git_path_diriter_free(git_path_diriter *diriter) From 110e29cbbd58e24fc218356dc570b8915c058ba4 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 20 Jun 2022 15:32:35 +0200 Subject: [PATCH 26/27] iterator: replace O(N) skip-to-start with O(log N) Original commit: https://github.com/libgit2/libgit2/commit/8a565a7fb8bb0a15f3285e87b301bb157c86d8f8 iterator: sort same-dir entries by basename The current code sorts entries in a directory by their full paths. Full paths share long prefixes, which makes comparator slow, which makes sorting slow. TimSort, that libgit2 uses for sorting, performs insertion sort when there are fewer than 65 elements. Most directories have fewer than 65 entries. Insertion sort performs a lot of comparisons, so it's very important to make comparator cheap. --- src/iterator.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index 647909e66d6..945b7165071 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1012,6 +1012,7 @@ typedef struct { size_t path_len; iterator_pathlist_search_t match; git_oid id; + char *basename; char path[GIT_FLEX_ARRAY]; } filesystem_iterator_entry; @@ -1073,7 +1074,7 @@ static int filesystem_iterator_entry_cmp(const void *_a, const void *_b) const filesystem_iterator_entry *a = (const filesystem_iterator_entry *)_a; const filesystem_iterator_entry *b = (const filesystem_iterator_entry *)_b; - return git__strcmp(a->path, b->path); + return git__strcmp(a->basename, b->basename); } static int filesystem_iterator_entry_cmp_icase(const void *_a, const void *_b) @@ -1081,7 +1082,7 @@ static int filesystem_iterator_entry_cmp_icase(const void *_a, const void *_b) const filesystem_iterator_entry *a = (const filesystem_iterator_entry *)_a; const filesystem_iterator_entry *b = (const filesystem_iterator_entry *)_b; - return git__strcasecmp(a->path, b->path); + return git__strcasecmp(a->basename, b->basename); } #define FILESYSTEM_MAX_DEPTH 100 @@ -1292,6 +1293,7 @@ static int filesystem_iterator_entry_init( filesystem_iterator_frame *frame, const char *path, size_t path_len, + size_t basename_len, struct stat *statbuf, iterator_pathlist_search_t pathlist_match) { @@ -1315,6 +1317,7 @@ static int filesystem_iterator_entry_init( entry->match = pathlist_match; memcpy(entry->path, path, path_len); memcpy(&entry->st, statbuf, sizeof(struct stat)); + entry->basename = entry->path + (path_len - basename_len); /* Suffix directory paths with a '/' */ if (S_ISDIR(entry->st.st_mode)) @@ -1342,6 +1345,7 @@ static int filesystem_iterator_frame_push( filesystem_iterator_entry *entry; struct stat statbuf; size_t path_len; + size_t basename_len; int error; bool submodule = false; @@ -1463,8 +1467,10 @@ static int filesystem_iterator_frame_push( } } + basename_len = diriter.path.size - diriter.parent_len - 1; + if ((error = filesystem_iterator_entry_init(&entry, - iter, new_frame, path, path_len, &statbuf, pathlist_match)) < 0) + iter, new_frame, path, path_len, basename_len, &statbuf, pathlist_match)) < 0) goto done; git_vector_insert(&new_frame->entries, entry); @@ -2036,6 +2042,8 @@ typedef struct { git_buf tree_buf; bool skip_tree; + size_t end_idx; + const git_index_entry *entry; } index_iterator; @@ -2115,6 +2123,31 @@ static int index_iterator_advance( iter->base.flags |= GIT_ITERATOR_FIRST_ACCESS; + if (iter->end_idx == (size_t)-1) { + git_index_entry key; + + if (iter->base.start_len && !iterator__include_trees(&iter->base)) { + git_buf buf = GIT_BUF_INIT; + memset(&key, 0, sizeof(key)); + if (iter->base.start[iter->base.start_len - 1] == '/') { + git_buf_puts(&buf, iter->base.start); + buf.ptr[buf.size - 1] = 0; + key.path = buf.ptr; + } else { + key.path = iter->base.start; + } + git_vector_bsearch(&iter->next_idx, &iter->entries, &key); + git_buf_dispose(&buf); + } + + if (iter->base.end_len) { + key.path = iter->base.end; + if (!git_vector_bsearch(&iter->end_idx, &iter->entries, &key)) ++iter->end_idx; + } else { + iter->end_idx = iter->entries.length; + } + } + while (true) { if (iter->next_idx >= iter->entries.length) { error = GIT_ITEROVER; @@ -2135,7 +2168,7 @@ static int index_iterator_advance( continue; } - if (iterator_has_ended(&iter->base, entry->path)) { + if (iter->next_idx >= iter->end_idx && iterator_has_ended(&iter->base, entry->path)) { error = GIT_ITEROVER; break; } @@ -2223,6 +2256,7 @@ static int index_iterator_init(index_iterator *iter) iter->base.flags &= ~GIT_ITERATOR_FIRST_ACCESS; iter->next_idx = 0; iter->skip_tree = false; + iter->end_idx = -1; return 0; } From 8bbbfab23fce9886b882a05a04d14ac89dbc2aa8 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 20 Jun 2022 15:44:25 +0200 Subject: [PATCH 27/27] push ignore frames lazily Original commit: https://github.com/romkatv/libgit2/commit/e52594f4283bc5e2befe5a487ebfe2adf249c54f --- src/iterator.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index 945b7165071..1f7e76d399f 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1137,14 +1137,14 @@ static int filesystem_iterator_is_submodule( static void filesystem_iterator_frame_push_ignores( filesystem_iterator *iter, - filesystem_iterator_entry *frame_entry, + filesystem_iterator_frame *previous_frame, filesystem_iterator_frame *new_frame) { - filesystem_iterator_frame *previous_frame; - const char *path = frame_entry ? frame_entry->path : ""; - - if (!iterator__honor_ignores(&iter->base)) - return; + const char* path = ""; + if (previous_frame) { + path = filesystem_iterator_current_entry(previous_frame)->path; + assert(path && *path); + } if (git_ignore__lookup(&new_frame->is_ignored, &iter->ignores, path, GIT_DIR_FLAG_TRUE) < 0) { @@ -1153,13 +1153,13 @@ static void filesystem_iterator_frame_push_ignores( } /* if this is not the top level directory... */ - if (frame_entry) { + if (previous_frame) { const char *relative_path; previous_frame = filesystem_iterator_parent_frame(iter); /* push new ignores for files in this directory */ - relative_path = frame_entry->path + previous_frame->path_len; + relative_path = path + previous_frame->path_len; /* inherit ignored from parent if no rule specified */ if (new_frame->is_ignored <= GIT_IGNORE_NOTFOUND) @@ -1167,13 +1167,18 @@ static void filesystem_iterator_frame_push_ignores( git_ignore__push_dir(&iter->ignores, relative_path); } + assert((size_t)iter->ignores.depth <= iter->frames.size); } static void filesystem_iterator_frame_pop_ignores( filesystem_iterator *iter) { - if (iterator__honor_ignores(&iter->base)) - git_ignore__pop_dir(&iter->ignores); + if (iterator__honor_ignores(&iter->base)) { + assert((size_t)iter->ignores.depth <= iter->frames.size + 1); + if ((size_t)iter->ignores.depth == iter->frames.size + 1) { + git_ignore__pop_dir(&iter->ignores); + } + } } GIT_INLINE(bool) filesystem_iterator_examine_path( @@ -1359,6 +1364,7 @@ static int filesystem_iterator_frame_push( GIT_ERROR_CHECK_ALLOC(new_frame); memset(new_frame, 0, sizeof(filesystem_iterator_frame)); + new_frame->is_ignored = GIT_IGNORE_UNCHECKED; if (frame_entry) git_buf_joinpath(&root, iter->root, frame_entry->path); @@ -1389,9 +1395,6 @@ static int filesystem_iterator_frame_push( if ((error = git_pool_init(&new_frame->entry_pool, 1)) < 0) goto done; - /* check if this directory is ignored */ - filesystem_iterator_frame_push_ignores(iter, frame_entry, new_frame); - while ((error = git_path_diriter_next(&diriter)) == 0) { iterator_pathlist_search_t pathlist_match = ITERATOR_PATHLIST_FULL; bool dir_expected = false; @@ -1723,9 +1726,21 @@ GIT_INLINE(git_dir_flag) entry_dir_flag(git_index_entry *entry) static void filesystem_iterator_update_ignored(filesystem_iterator *iter) { + size_t i; filesystem_iterator_frame *frame; git_dir_flag dir_flag = entry_dir_flag(&iter->entry); + for (i = iter->frames.size; + i && iter->frames.ptr[i - 1].is_ignored == GIT_IGNORE_UNCHECKED; + --i) { + // empty body + } + + for (; i != iter->frames.size; ++i) { + frame = iter->frames.ptr + i; + filesystem_iterator_frame_push_ignores(iter, i ? frame - 1 : NULL, frame); + } + if (git_ignore__lookup(&iter->current_is_ignored, &iter->ignores, iter->entry.path, dir_flag) < 0) { git_error_clear();