From 93b42728952487941bc2b0b78a7f0080b7b3d600 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Tue, 9 Jun 2015 14:38:30 -0700 Subject: [PATCH 01/73] Include stacktrace summary in memory leak output. --- CMakeLists.txt | 1 + src/common.h | 4 + src/global.c | 18 +- src/util.h | 13 +- src/win32/w32_crtdbg_stacktrace.c | 343 ++++++++++++++++++++++++++++++ src/win32/w32_crtdbg_stacktrace.h | 93 ++++++++ src/win32/w32_stack.c | 180 ++++++++++++++++ src/win32/w32_stack.h | 132 ++++++++++++ tests/clar_libgit2_trace.c | 19 ++ tests/main.c | 19 -- tests/trace/windows/stacktrace.c | 151 +++++++++++++ 11 files changed, 947 insertions(+), 26 deletions(-) create mode 100644 src/win32/w32_crtdbg_stacktrace.c create mode 100644 src/win32/w32_crtdbg_stacktrace.h create mode 100644 src/win32/w32_stack.c create mode 100644 src/win32/w32_stack.h create mode 100644 tests/trace/windows/stacktrace.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c55ddd1a2b..994eed5e11e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -336,6 +336,7 @@ IF (MSVC) IF (MSVC_CRTDBG) SET(CRT_FLAG_DEBUG "${CRT_FLAG_DEBUG} /DGIT_MSVC_CRTDBG") + SET(CMAKE_C_STANDARD_LIBRARIES "${CMAKE_C_STANDARD_LIBRARIES}" "Dbghelp.lib") ENDIF() # /Zi - Create debugging information diff --git a/src/common.h b/src/common.h index cdb0b514d6f..2b1dedc0146 100644 --- a/src/common.h +++ b/src/common.h @@ -46,6 +46,10 @@ # ifdef GIT_THREADS # include "win32/pthread.h" # endif +# if defined(GIT_MSVC_CRTDBG) +# include "win32/w32_stack.h" +# include "win32/w32_crtdbg_stacktrace.h" +# endif #else diff --git a/src/global.c b/src/global.c index 3f20bfd3195..fc6337adc0b 100644 --- a/src/global.c +++ b/src/global.c @@ -11,7 +11,10 @@ #include "git2/global.h" #include "git2/sys/openssl.h" #include "thread-utils.h" - +#if defined(GIT_MSVC_CRTDBG) +#include "win32/w32_stack.h" +#include "win32/w32_crtdbg_stacktrace.h" +#endif git_mutex git__mwindow_mutex; @@ -225,6 +228,11 @@ int git_libgit2_init(void) /* Only do work on a 0 -> 1 transition of the refcount */ if ((ret = git_atomic_inc(&git__n_inits)) == 1) { +#if defined(GIT_MSVC_CRTDBG) + git_win32__crtdbg_stacktrace_init(); + git_win32__stack_init(); +#endif + if (synchronized_threads_init() < 0) ret = -1; } @@ -254,9 +262,15 @@ int git_libgit2_shutdown(void) while (InterlockedCompareExchange(&_mutex, 1, 0)) { Sleep(0); } /* Only do work on a 1 -> 0 transition of the refcount */ - if ((ret = git_atomic_dec(&git__n_inits)) == 0) + if ((ret = git_atomic_dec(&git__n_inits)) == 0) { synchronized_threads_shutdown(); +#if defined(GIT_MSVC_CRTDBG) + git_win32__crtdbg_stacktrace_cleanup(); + git_win32__stack_cleanup(); +#endif + } + /* Exit the lock */ InterlockedExchange(&_mutex, 0); diff --git a/src/util.h b/src/util.h index b2abbe6a652..8c336e3ef8e 100644 --- a/src/util.h +++ b/src/util.h @@ -35,6 +35,7 @@ */ #include #include +#include "win32/w32_crtdbg_stacktrace.h" #endif #include "common.h" @@ -62,23 +63,24 @@ #define CONST_STRLEN(x) ((sizeof(x)/sizeof(x[0])) - 1) #if defined(GIT_MSVC_CRTDBG) + GIT_INLINE(void *) git__crtdbg__malloc(size_t len, const char *file, int line) { - void *ptr = _malloc_dbg(len, _NORMAL_BLOCK, file, line); + void *ptr = _malloc_dbg(len, _NORMAL_BLOCK, git_win32__crtdbg_stacktrace(1,file), line); if (!ptr) giterr_set_oom(); return ptr; } GIT_INLINE(void *) git__crtdbg__calloc(size_t nelem, size_t elsize, const char *file, int line) { - void *ptr = _calloc_dbg(nelem, elsize, _NORMAL_BLOCK, file, line); + void *ptr = _calloc_dbg(nelem, elsize, _NORMAL_BLOCK, git_win32__crtdbg_stacktrace(1,file), line); if (!ptr) giterr_set_oom(); return ptr; } GIT_INLINE(char *) git__crtdbg__strdup(const char *str, const char *file, int line) { - char *ptr = _strdup_dbg(str, _NORMAL_BLOCK, file, line); + char *ptr = _strdup_dbg(str, _NORMAL_BLOCK, git_win32__crtdbg_stacktrace(1,file), line); if (!ptr) giterr_set_oom(); return ptr; } @@ -118,7 +120,7 @@ GIT_INLINE(char *) git__crtdbg__substrdup(const char *start, size_t n, const cha GIT_INLINE(void *) git__crtdbg__realloc(void *ptr, size_t size, const char *file, int line) { - void *new_ptr = _realloc_dbg(ptr, size, _NORMAL_BLOCK, file, line); + void *new_ptr = _realloc_dbg(ptr, size, _NORMAL_BLOCK, git_win32__crtdbg_stacktrace(1,file), line); if (!new_ptr) giterr_set_oom(); return new_ptr; } @@ -126,8 +128,9 @@ GIT_INLINE(void *) git__crtdbg__realloc(void *ptr, size_t size, const char *file GIT_INLINE(void *) git__crtdbg__reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line) { size_t newsize; + return GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize) ? - NULL : _realloc_dbg(ptr, newsize, _NORMAL_BLOCK, file, line); + NULL : _realloc_dbg(ptr, newsize, _NORMAL_BLOCK, git_win32__crtdbg_stacktrace(1,file), line); } GIT_INLINE(void *) git__crtdbg__mallocarray(size_t nelem, size_t elsize, const char *file, int line) diff --git a/src/win32/w32_crtdbg_stacktrace.c b/src/win32/w32_crtdbg_stacktrace.c new file mode 100644 index 00000000000..a778f4164ad --- /dev/null +++ b/src/win32/w32_crtdbg_stacktrace.c @@ -0,0 +1,343 @@ +/* + * 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. + */ + +#if defined(GIT_MSVC_CRTDBG) +#include "w32_stack.h" +#include "w32_crtdbg_stacktrace.h" + +#define CRTDBG_STACKTRACE__UID_LEN (15) + +/** + * The stacktrace of an allocation can be distilled + * to a unique id based upon the stackframe pointers + * and ignoring any size arguments. We will use these + * UIDs as the (char const*) __FILE__ argument we + * give to the CRT malloc routines. + */ +typedef struct { + char uid[CRTDBG_STACKTRACE__UID_LEN + 1]; +} git_win32__crtdbg_stacktrace__uid; + +/** + * All mallocs with the same stacktrace will be de-duped + * and aggregated into this row. + */ +typedef struct { + git_win32__crtdbg_stacktrace__uid uid; /* must be first */ + git_win32__stack__raw_data raw_data; + unsigned int count_allocs; /* times this alloc signature seen since init */ + unsigned int count_allocs_at_last_checkpoint; /* times since last mark */ + unsigned int transient_count_leaks; /* sum of leaks */ +} git_win32__crtdbg_stacktrace__row; + +static CRITICAL_SECTION g_crtdbg_stacktrace_cs; + +/** + * CRTDBG memory leak tracking takes a "char const * const file_name" + * and stores the pointer in the heap data (instead of allocing a copy + * for itself). Normally, this is not a problem, since we usually pass + * in __FILE__. But I'm going to lie to it and pass in the address of + * the UID in place of the file_name. Also, I do not want to alloc the + * stacktrace data (because we are called from inside our alloc routines). + * Therefore, I'm creating a very large static pool array to store row + * data. This also eliminates the temptation to realloc it (and move the + * UID pointers). + * + * And to efficiently look for duplicates we need an index on the rows + * so we can bsearch it. Again, without mallocing. + * + * If we observe more than MY_ROW_LIMIT unique malloc signatures, we + * fall through and use the traditional __FILE__ processing and don't + * try to de-dup them. If your testing hits this limit, just increase + * it and try again. + */ + +#define MY_ROW_LIMIT (1024 * 1024) +static git_win32__crtdbg_stacktrace__row g_cs_rows[MY_ROW_LIMIT]; +static git_win32__crtdbg_stacktrace__row *g_cs_index[MY_ROW_LIMIT]; + +static unsigned int g_cs_end = MY_ROW_LIMIT; +static unsigned int g_cs_ins = 0; /* insertion point == unique allocs seen */ +static unsigned int g_count_total_allocs = 0; /* number of allocs seen */ +static unsigned int g_transient_count_total_leaks = 0; /* number of total leaks */ +static unsigned int g_transient_count_dedup_leaks = 0; /* number of unique leaks */ +static bool g_limit_reached = false; /* had allocs after we filled row table */ + +static unsigned int g_checkpoint_id = 0; /* to better label leak checkpoints */ +static bool g_transient_leaks_since_mark = false; /* payload for hook */ + +/** + * Compare function for bsearch on g_cs_index table. + */ +static int row_cmp(const void *v1, const void *v2) +{ + git_win32__stack__raw_data *d1 = (git_win32__stack__raw_data*)v1; + git_win32__crtdbg_stacktrace__row *r2 = (git_win32__crtdbg_stacktrace__row *)v2; + + return (git_win32__stack_compare(d1, &r2->raw_data)); +} + +/** + * Unique insert the new data into the row and index tables. + * We have to sort by the stackframe data itself, not the uid. + */ +static git_win32__crtdbg_stacktrace__row * insert_unique( + const git_win32__stack__raw_data *pdata) +{ + size_t pos; + if (git__bsearch(g_cs_index, g_cs_ins, pdata, row_cmp, &pos) < 0) { + /* Append new unique item to row table. */ + memcpy(&g_cs_rows[g_cs_ins].raw_data, pdata, sizeof(*pdata)); + sprintf(g_cs_rows[g_cs_ins].uid.uid, "##%08lx", g_cs_ins); + + /* Insert pointer to it into the proper place in the index table. */ + if (pos < g_cs_ins) + memmove(&g_cs_index[pos+1], &g_cs_index[pos], (g_cs_ins - pos)*sizeof(g_cs_index[0])); + g_cs_index[pos] = &g_cs_rows[g_cs_ins]; + + g_cs_ins++; + } + + g_cs_index[pos]->count_allocs++; + + return g_cs_index[pos]; +} + +/** + * Hook function to receive leak data from the CRT. (This includes + * both ":()" data, but also each of the + * various headers and fields. + * + * Scan this for the special "##" UID forms that we substituted + * for the "". Map back to the row data and + * increment its leak count. + * + * See https://msdn.microsoft.com/en-us/library/74kabxyx.aspx + * + * We suppress the actual crtdbg output. + */ +static int __cdecl report_hook(int nRptType, char *szMsg, int *retVal) +{ + static int hook_result = TRUE; /* FALSE to get stock dump; TRUE to suppress. */ + unsigned int pos; + + *retVal = 0; /* do not invoke debugger */ + + if ((szMsg[0] != '#') || (szMsg[1] != '#')) + return hook_result; + + if (sscanf(&szMsg[2], "%08lx", &pos) < 1) + return hook_result; + if (pos >= g_cs_ins) + return hook_result; + + if (g_transient_leaks_since_mark) { + if (g_cs_rows[pos].count_allocs == g_cs_rows[pos].count_allocs_at_last_checkpoint) + return hook_result; + } + + g_cs_rows[pos].transient_count_leaks++; + + if (g_cs_rows[pos].transient_count_leaks == 1) + g_transient_count_dedup_leaks++; + + g_transient_count_total_leaks++; + + return hook_result; +} + +/** + * Write leak data to all of the various places we need. + * We force the caller to sprintf() the message first + * because we want to avoid fprintf() because it allocs. + */ +static void my_output(const char *buf) +{ + fwrite(buf, strlen(buf), 1, stderr); + OutputDebugString(buf); +} + +/** + * For each row with leaks, dump a stacktrace for it. + */ +static void dump_summary(const char *label) +{ + unsigned int k; + char buf[10 * 1024]; + + if (g_transient_count_total_leaks == 0) + return; + + fflush(stdout); + fflush(stderr); + my_output("\n"); + + if (g_limit_reached) { + sprintf(buf, + "LEAK SUMMARY: de-dup row table[%d] filled. Increase MY_ROW_LIMIT.\n", + MY_ROW_LIMIT); + my_output(buf); + } + + if (!label) + label = ""; + + if (g_transient_leaks_since_mark) { + sprintf(buf, "LEAK CHECKPOINT %d: leaks %d unique %d: %s\n", + g_checkpoint_id, g_transient_count_total_leaks, g_transient_count_dedup_leaks, label); + my_output(buf); + } else { + sprintf(buf, "LEAK SUMMARY: TOTAL leaks %d de-duped %d: %s\n", + g_transient_count_total_leaks, g_transient_count_dedup_leaks, label); + my_output(buf); + } + my_output("\n"); + + for (k = 0; k < g_cs_ins; k++) { + if (g_cs_rows[k].transient_count_leaks > 0) { + sprintf(buf, "LEAK: %s leaked %d of %d times:\n", + g_cs_rows[k].uid.uid, + g_cs_rows[k].transient_count_leaks, + g_cs_rows[k].count_allocs); + my_output(buf); + + if (git_win32__stack_format( + buf, sizeof(buf), &g_cs_rows[k].raw_data, + NULL, NULL) >= 0) { + my_output(buf); + } + + my_output("\n"); + } + } + + fflush(stderr); +} + +void git_win32__crtdbg_stacktrace_init(void) +{ + InitializeCriticalSection(&g_crtdbg_stacktrace_cs); + + EnterCriticalSection(&g_crtdbg_stacktrace_cs); + + _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); + + _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); + _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); + _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); + + _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR); + _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR); + _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR); + + LeaveCriticalSection(&g_crtdbg_stacktrace_cs); +} + +int git_win32__crtdbg_stacktrace__dump( + git_win32__crtdbg_stacktrace_options opt, + const char *label) +{ + _CRT_REPORT_HOOK old; + unsigned int k; + int r = 0; + +#define IS_BIT_SET(o,b) (((o) & (b)) != 0) + + bool b_set_mark = IS_BIT_SET(opt, GIT_WIN32__CRTDBG_STACKTRACE__SET_MARK); + bool b_leaks_since_mark = IS_BIT_SET(opt, GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK); + bool b_leaks_total = IS_BIT_SET(opt, GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_TOTAL); + bool b_quiet = IS_BIT_SET(opt, GIT_WIN32__CRTDBG_STACKTRACE__QUIET); + + if (b_leaks_since_mark && b_leaks_total) { + giterr_set(GITERR_INVALID, "Cannot combine LEAKS_SINCE_MARK and LEAKS_TOTAL."); + return GIT_ERROR; + } + if (!b_set_mark && !b_leaks_since_mark && !b_leaks_total) { + giterr_set(GITERR_INVALID, "Nothing to do."); + return GIT_ERROR; + } + + EnterCriticalSection(&g_crtdbg_stacktrace_cs); + + if (b_leaks_since_mark || b_leaks_total) { + /* All variables with "transient" in the name are per-dump counters + * and reset before each dump. This lets us handle checkpoints. + */ + g_transient_count_total_leaks = 0; + g_transient_count_dedup_leaks = 0; + for (k = 0; k < g_cs_ins; k++) { + g_cs_rows[k].transient_count_leaks = 0; + } + } + + g_transient_leaks_since_mark = b_leaks_since_mark; + + old = _CrtSetReportHook(report_hook); + _CrtDumpMemoryLeaks(); + _CrtSetReportHook(old); + + if (b_leaks_since_mark || b_leaks_total) { + r = g_transient_count_dedup_leaks; + + if (!b_quiet) + dump_summary(label); + } + + if (b_set_mark) { + for (k = 0; k < g_cs_ins; k++) { + g_cs_rows[k].count_allocs_at_last_checkpoint = g_cs_rows[k].count_allocs; + } + + g_checkpoint_id++; + } + + LeaveCriticalSection(&g_crtdbg_stacktrace_cs); + + return r; +} + +void git_win32__crtdbg_stacktrace_cleanup(void) +{ + /* At shutdown/cleanup, dump cummulative leak info + * with everything since startup. This might generate + * extra noise if the caller has been doing checkpoint + * dumps, but it might also eliminate some false + * positives for resources previously reported during + * checkpoints. + */ + git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_TOTAL, + "CLEANUP"); + + DeleteCriticalSection(&g_crtdbg_stacktrace_cs); +} + +const char *git_win32__crtdbg_stacktrace(int skip, const char *file) +{ + git_win32__stack__raw_data new_data; + git_win32__crtdbg_stacktrace__row *row; + const char * result = file; + + if (git_win32__stack_capture(&new_data, skip+1) < 0) + return result; + + EnterCriticalSection(&g_crtdbg_stacktrace_cs); + + if (g_cs_ins < g_cs_end) { + row = insert_unique(&new_data); + result = row->uid.uid; + } else { + g_limit_reached = true; + } + + g_count_total_allocs++; + + LeaveCriticalSection(&g_crtdbg_stacktrace_cs); + + return result; +} +#endif diff --git a/src/win32/w32_crtdbg_stacktrace.h b/src/win32/w32_crtdbg_stacktrace.h new file mode 100644 index 00000000000..40ca60d5370 --- /dev/null +++ b/src/win32/w32_crtdbg_stacktrace.h @@ -0,0 +1,93 @@ +/* + * 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_w32_crtdbg_stacktrace_h__ +#define INCLUDE_w32_crtdbg_stacktrace_h__ + +#if defined(GIT_MSVC_CRTDBG) + +/** + * Initialize our memory leak tracking and de-dup data structures. + * This should ONLY be called by git_libgit2_init(). + */ +void git_win32__crtdbg_stacktrace_init(void); + +/** + * Shutdown our memory leak tracking and dump summary data. + * This should ONLY be called by git_libgit2_shutdown(). + * + * We explicitly call _CrtDumpMemoryLeaks() during here so + * that we can compute summary data for the leaks. We print + * the stacktrace of each unique leak. + * + * This cleanup does not happen if the app calls exit() + * without calling the libgit2 shutdown code. + * + * This info we print here is independent of any automatic + * reporting during exit() caused by _CRTDBG_LEAK_CHECK_DF. + * Set it in your app if you also want traditional reporting. + */ +void git_win32__crtdbg_stacktrace_cleanup(void); + +/** + * Checkpoint options. + */ +typedef enum git_win32__crtdbg_stacktrace_options { + /** + * Set checkpoint marker. + */ + GIT_WIN32__CRTDBG_STACKTRACE__SET_MARK = (1 << 0), + + /** + * Dump leaks since last checkpoint marker. + * May not be combined with __LEAKS_TOTAL. + * + * Note that this may generate false positives for global TLS + * error state and other global caches that aren't cleaned up + * until the thread/process terminates. So when using this + * around a region of interest, also check the final (at exit) + * dump before digging into leaks reported here. + */ + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK = (1 << 1), + + /** + * Dump leaks since init. May not be combined + * with __LEAKS_SINCE_MARK. + */ + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_TOTAL = (1 << 2), + + /** + * Suppress printing during dumps. + * Just return leak count. + */ + GIT_WIN32__CRTDBG_STACKTRACE__QUIET = (1 << 3), + +} git_win32__crtdbg_stacktrace_options; + +/** + * Checkpoint memory state and/or dump unique stack traces of + * current memory leaks. + * + * @return number of unique leaks (relative to requested starting + * point) or error. + */ +GIT_EXTERN(int) git_win32__crtdbg_stacktrace__dump( + git_win32__crtdbg_stacktrace_options opt, + const char *label); + +/** + * Construct stacktrace and append it to the global buffer. + * Return pointer to start of this string. On any error or + * lack of buffer space, just return the given file buffer + * so it will behave as usual. + * + * This should ONLY be called by our internal memory allocations + * routines. + */ +const char *git_win32__crtdbg_stacktrace(int skip, const char *file); + +#endif +#endif diff --git a/src/win32/w32_stack.c b/src/win32/w32_stack.c new file mode 100644 index 00000000000..f1e589531f0 --- /dev/null +++ b/src/win32/w32_stack.c @@ -0,0 +1,180 @@ +/* + * 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. + */ + +#if defined(GIT_MSVC_CRTDBG) +#include "Windows.h" +#include "Dbghelp.h" +#include "win32/posix.h" +#include "w32_stack.h" +#include "hash.h" + +/** + * This is supposedly defined in WinBase.h (from Windows.h) but there were linker issues. + */ +USHORT WINAPI RtlCaptureStackBackTrace(ULONG, ULONG, PVOID*, PULONG); + +static bool g_win32_stack_initialized = false; +static HANDLE g_win32_stack_process = INVALID_HANDLE_VALUE; +static git_win32__stack__aux_cb_alloc g_aux_cb_alloc = NULL; +static git_win32__stack__aux_cb_lookup g_aux_cb_lookup = NULL; + +int git_win32__stack__set_aux_cb( + git_win32__stack__aux_cb_alloc cb_alloc, + git_win32__stack__aux_cb_lookup cb_lookup) +{ + g_aux_cb_alloc = cb_alloc; + g_aux_cb_lookup = cb_lookup; + + return 0; +} + +void git_win32__stack_init(void) +{ + if (!g_win32_stack_initialized) { + g_win32_stack_process = GetCurrentProcess(); + SymSetOptions(SYMOPT_LOAD_LINES); + SymInitialize(g_win32_stack_process, NULL, TRUE); + g_win32_stack_initialized = true; + } +} + +void git_win32__stack_cleanup(void) +{ + if (g_win32_stack_initialized) { + SymCleanup(g_win32_stack_process); + g_win32_stack_process = INVALID_HANDLE_VALUE; + g_win32_stack_initialized = false; + } +} + +int git_win32__stack_capture(git_win32__stack__raw_data *pdata, int skip) +{ + if (!g_win32_stack_initialized) { + giterr_set(GITERR_INVALID, "git_win32_stack not initialized."); + return GIT_ERROR; + } + + memset(pdata, 0, sizeof(*pdata)); + pdata->nr_frames = RtlCaptureStackBackTrace( + skip+1, GIT_WIN32__STACK__MAX_FRAMES, pdata->frames, NULL); + + /* If an "aux" data provider was registered, ask it to capture + * whatever data it needs and give us an "aux_id" to it so that + * we can refer to it later when reporting. + */ + if (g_aux_cb_alloc) + (g_aux_cb_alloc)(&pdata->aux_id); + + return 0; +} + +int git_win32__stack_compare( + git_win32__stack__raw_data *d1, + git_win32__stack__raw_data *d2) +{ + return memcmp(d1, d2, sizeof(d1)); +} + +int git_win32__stack_format( + char *pbuf, int buf_len, + const git_win32__stack__raw_data *pdata, + const char *prefix, const char *suffix) +{ +#define MY_MAX_FILENAME 255 + + /* SYMBOL_INFO has char FileName[1] at the end. The docs say to + * to malloc it with extra space for your desired max filename. + * We use a union to do the same thing without mallocing. + */ + struct { + SYMBOL_INFO symbol; + char extra[MY_MAX_FILENAME + 1]; + } s; + + IMAGEHLP_LINE64 line; + int buf_used = 0; + unsigned int k; + char detail[MY_MAX_FILENAME * 2]; /* filename plus space for function name and formatting */ + int detail_len; + + if (!g_win32_stack_initialized) { + giterr_set(GITERR_INVALID, "git_win32_stack not initialized."); + return GIT_ERROR; + } + + if (!prefix) + prefix = "\t"; + if (!suffix) + suffix = "\n"; + + memset(pbuf, 0, buf_len); + + memset(&s, 0, sizeof(s)); + s.symbol.MaxNameLen = MY_MAX_FILENAME; + s.symbol.SizeOfStruct = sizeof(SYMBOL_INFO); + + memset(&line, 0, sizeof(line)); + line.SizeOfStruct = sizeof(IMAGEHLP_LINE64); + + for (k=0; k < pdata->nr_frames; k++) { + DWORD64 frame_k = (DWORD64)pdata->frames[k]; + DWORD dwUnused; + + if (SymFromAddr(g_win32_stack_process, frame_k, 0, &s.symbol) && + SymGetLineFromAddr64(g_win32_stack_process, frame_k, &dwUnused, &line)) { + const char *pslash; + const char *pfile; + + pslash = strrchr(line.FileName, '\\'); + pfile = ((pslash) ? (pslash+1) : line.FileName); + p_snprintf(detail, sizeof(detail), "%s%s:%d> %s%s", + prefix, pfile, line.LineNumber, s.symbol.Name, suffix); + } else { + /* This happens when we cross into another module. + * For example, in CLAR tests, this is typically + * the CRT startup code. Just print an unknown + * frame and continue. + */ + p_snprintf(detail, sizeof(detail), "%s??%s", prefix, suffix); + } + detail_len = strlen(detail); + + if (buf_len < (buf_used + detail_len + 1)) { + /* we don't have room for this frame in the buffer, so just stop. */ + break; + } + + memcpy(&pbuf[buf_used], detail, detail_len); + buf_used += detail_len; + } + + /* If an "aux" data provider was registered, ask it to append its detailed + * data to the end of ours using the "aux_id" it gave us when this de-duped + * item was created. + */ + if (g_aux_cb_lookup) + (g_aux_cb_lookup)(pdata->aux_id, &pbuf[buf_used], (buf_len - buf_used - 1)); + + return GIT_OK; +} + +int git_win32__stack( + char * pbuf, int buf_len, + int skip, + const char *prefix, const char *suffix) +{ + git_win32__stack__raw_data data; + int error; + + if ((error = git_win32__stack_capture(&data, skip)) < 0) + return error; + if ((error = git_win32__stack_format(pbuf, buf_len, &data, prefix, suffix)) < 0) + return error; + return 0; +} + +#endif diff --git a/src/win32/w32_stack.h b/src/win32/w32_stack.h new file mode 100644 index 00000000000..b457d2e8ac6 --- /dev/null +++ b/src/win32/w32_stack.h @@ -0,0 +1,132 @@ +/* + * 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_w32_stack_h__ +#define INCLUDE_w32_stack_h__ + +#if defined(GIT_MSVC_CRTDBG) + +/** + * This type defines a callback to be used to augment a C stacktrace + * with "aux" data. This can be used, for example, to allow LibGit2Sharp + * (or other interpreted consumer libraries) to give us C# stacktrace + * data for the PInvoke. + * + * This callback will be called during crtdbg-instrumented allocs. + * + * @param aux_id [out] A returned "aux_id" representing a unique + * (de-duped at the C# layer) stacktrace. + */ +typedef void (*git_win32__stack__aux_cb_alloc)(unsigned int *aux_id); + +/** + * This type defines a callback to be used to augment the output of + * a stacktrace. This will be used to request the C# layer format + * the C# stacktrace associated with "aux_id" into the provided + * buffer. + * + * This callback will be called during leak reporting. + * + * @param aux_id The "aux_id" key associated with a stacktrace. + * @param aux_msg A buffer where a formatted message should be written. + * @param aux_msg_len The size of the buffer. + */ +typedef void (*git_win32__stack__aux_cb_lookup)(unsigned int aux_id, char *aux_msg, unsigned int aux_msg_len); + +/** + * Register an "aux" data provider to augment our C stacktrace data. + * + * This can be used, for example, to allow LibGit2Sharp (or other + * interpreted consumer libraries) to give us the C# stacktrace of + * the PInvoke. + * + * If you choose to use this feature, it should be registered during + * initialization and not changed for the duration of the process. + */ +GIT_EXTERN(int) git_win32__stack__set_aux_cb( + git_win32__stack__aux_cb_alloc cb_alloc, + git_win32__stack__aux_cb_lookup cb_lookup); + +/** + * Maximum number of stackframes to record for a + * single stacktrace. + */ +#define GIT_WIN32__STACK__MAX_FRAMES 30 + +/** + * Wrapper containing the raw unprocessed stackframe + * data for a single stacktrace and any "aux_id". + */ +typedef struct { + void *frames[GIT_WIN32__STACK__MAX_FRAMES]; + unsigned int nr_frames; + unsigned int aux_id; +} git_win32__stack__raw_data; + + +/** + * Load symbol table data. This should be done in the primary + * thread at startup (under a lock if there are other threads + * active). + */ +void git_win32__stack_init(void); + +/** + * Cleanup symbol table data. This should be done in the + * primary thead at shutdown (under a lock if there are other + * threads active). + */ +void git_win32__stack_cleanup(void); + + +/** + * Capture raw stack trace data for the current process/thread. + * + * @param skip Number of initial frames to skip. Pass 0 to + * begin with the caller of this routine. Pass 1 to begin + * with its caller. And so on. + */ +int git_win32__stack_capture(git_win32__stack__raw_data *pdata, int skip); + +/** + * Compare 2 raw stacktraces with the usual -1,0,+1 result. + * This includes any "aux_id" values in the comparison, so that + * our de-dup is also "aux" context relative. + */ +int git_win32__stack_compare( + git_win32__stack__raw_data *d1, + git_win32__stack__raw_data *d2); + +/** + * Format raw stacktrace data into buffer WITHOUT using any mallocs. + * + * @param prefix String written before each frame; defaults to "\t". + * @param suffix String written after each frame; defaults to "\n". + */ +int git_win32__stack_format( + char *pbuf, int buf_len, + const git_win32__stack__raw_data *pdata, + const char *prefix, const char *suffix); + +/** + * Convenience routine to capture and format stacktrace into + * a buffer WITHOUT using any mallocs. This is primarily a + * wrapper for testing. + * + * @param skip Number of initial frames to skip. Pass 0 to + * begin with the caller of this routine. Pass 1 to begin + * with its caller. And so on. + * @param prefix String written before each frame; defaults to "\t". + * @param suffix String written after each frame; defaults to "\n". + */ +int git_win32__stack( + char * pbuf, int buf_len, + int skip, + const char *prefix, const char *suffix); + +#endif /* GIT_MSVC_CRTDBG */ +#endif /* INCLUDE_w32_stack_h__ */ diff --git a/tests/clar_libgit2_trace.c b/tests/clar_libgit2_trace.c index ae582d1cb4a..aaeeb78106c 100644 --- a/tests/clar_libgit2_trace.c +++ b/tests/clar_libgit2_trace.c @@ -142,9 +142,28 @@ void _cl_trace_cb__event_handler( switch (ev) { case CL_TRACE__SUITE_BEGIN: git_trace(GIT_TRACE_TRACE, "\n\n%s\n%s: Begin Suite", HR, suite_name); +#if 0 && defined(GIT_MSVC_CRTDBG) + git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__SET_MARK, + suite_name); +#endif break; case CL_TRACE__SUITE_END: +#if 0 && defined(GIT_MSVC_CRTDBG) + /* As an example of checkpointing, dump leaks within this suite. + * This may generate false positives for things like the global + * TLS error state and maybe the odb cache since they aren't + * freed until the global shutdown and outside the scope of this + * set of tests. + * + * This may under-report if the test itself uses a checkpoint. + * See tests/trace/windows/stacktrace.c + */ + git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + suite_name); +#endif git_trace(GIT_TRACE_TRACE, "\n\n%s: End Suite\n%s", suite_name, HR); break; diff --git a/tests/main.c b/tests/main.c index 56326da1c10..f67c8ffbc73 100644 --- a/tests/main.c +++ b/tests/main.c @@ -1,10 +1,3 @@ - -#if defined(GIT_MSVC_CRTDBG) -/* Enable MSVC CRTDBG memory leak reporting. See src/util.h for details. */ -#include -#include -#endif - #include "clar_libgit2.h" #include "clar_libgit2_trace.h" @@ -16,18 +9,6 @@ int main(int argc, char *argv[]) { int res; -#if defined(GIT_MSVC_CRTDBG) - _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); - - _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); - _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); - _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE); - - _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR); - _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR); - _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR); -#endif - clar_test_init(argc, argv); git_libgit2_init(); diff --git a/tests/trace/windows/stacktrace.c b/tests/trace/windows/stacktrace.c new file mode 100644 index 00000000000..c00c1b77448 --- /dev/null +++ b/tests/trace/windows/stacktrace.c @@ -0,0 +1,151 @@ +#include "clar_libgit2.h" +#include "win32/w32_stack.h" + +#if defined(GIT_MSVC_CRTDBG) +static void a(void) +{ + char buf[10000]; + + cl_assert(git_win32__stack(buf, sizeof(buf), 0, NULL, NULL) == 0); + +#if 0 + fprintf(stderr, "Stacktrace from [%s:%d]:\n%s\n", __FILE__, __LINE__, buf); +#endif +} + +static void b(void) +{ + a(); +} + +static void c(void) +{ + b(); +} +#endif + +void test_trace_windows_stacktrace__basic(void) +{ +#if defined(GIT_MSVC_CRTDBG) + c(); +#endif +} + + +void test_trace_windows_stacktrace__leaks(void) +{ +#if defined(GIT_MSVC_CRTDBG) + void * p1; + void * p2; + void * p3; + void * p4; + int before, after; + int leaks; + int error; + + /* remember outstanding leaks due to set setup + * and set mark/checkpoint. + */ + before = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_TOTAL | + GIT_WIN32__CRTDBG_STACKTRACE__SET_MARK, + NULL); + + p1 = git__malloc(5); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p1"); + cl_assert((leaks == 1)); + + p2 = git__malloc(5); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p1,p2"); + cl_assert((leaks == 2)); + + p3 = git__malloc(5); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p1,p2,p3"); + cl_assert((leaks == 3)); + + git__free(p2); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p1,p3"); + cl_assert((leaks == 2)); + + /* move the mark. only new leaks should appear afterwards */ + error = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__SET_MARK, + NULL); + cl_assert((error == 0)); + + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "not_p1,not_p3"); + cl_assert((leaks == 0)); + + p4 = git__malloc(5); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p4,not_p1,not_p3"); + cl_assert((leaks == 1)); + + git__free(p1); + git__free(p3); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "p4"); + cl_assert((leaks == 1)); + + git__free(p4); + leaks = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_SINCE_MARK, + "end"); + cl_assert((leaks == 0)); + + /* confirm current absolute leaks count matches beginning value. */ + after = git_win32__crtdbg_stacktrace__dump( + GIT_WIN32__CRTDBG_STACKTRACE__QUIET | + GIT_WIN32__CRTDBG_STACKTRACE__LEAKS_TOTAL, + "total"); + cl_assert((before == after)); +#endif +} + +#if defined(GIT_MSVC_CRTDBG) +static void aux_cb_alloc__1(unsigned int *aux_id) +{ + static unsigned int aux_counter = 0; + + *aux_id = aux_counter++; +} + +static void aux_cb_lookup__1(unsigned int aux_id, char *aux_msg, unsigned int aux_msg_len) +{ + p_snprintf(aux_msg, aux_msg_len, "\tQQ%08x\n", aux_id); +} + +#endif + +void test_trace_windows_stacktrace__aux1(void) +{ +#if defined(GIT_MSVC_CRTDBG) + git_win32__stack__set_aux_cb(aux_cb_alloc__1, aux_cb_lookup__1); + c(); + c(); + c(); + c(); + git_win32__stack__set_aux_cb(NULL, NULL); +#endif +} From 827b954ef400f3c488500ad6c8a57246223dde9c Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Sun, 28 Jun 2015 06:56:02 -0400 Subject: [PATCH 02/73] Reserve aux_id 0; sort leaks by aux_id. Fix cmp. --- src/win32/w32_stack.c | 26 +++++++++++++++++++------- src/win32/w32_stack.h | 12 +++++++++--- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/win32/w32_stack.c b/src/win32/w32_stack.c index f1e589531f0..15af3dcb7de 100644 --- a/src/win32/w32_stack.c +++ b/src/win32/w32_stack.c @@ -76,7 +76,7 @@ int git_win32__stack_compare( git_win32__stack__raw_data *d1, git_win32__stack__raw_data *d2) { - return memcmp(d1, d2, sizeof(d1)); + return memcmp(d1, d2, sizeof(*d1)); } int git_win32__stack_format( @@ -88,7 +88,6 @@ int git_win32__stack_format( /* SYMBOL_INFO has char FileName[1] at the end. The docs say to * to malloc it with extra space for your desired max filename. - * We use a union to do the same thing without mallocing. */ struct { SYMBOL_INFO symbol; @@ -152,12 +151,25 @@ int git_win32__stack_format( buf_used += detail_len; } - /* If an "aux" data provider was registered, ask it to append its detailed - * data to the end of ours using the "aux_id" it gave us when this de-duped - * item was created. + /* "aux_id" 0 is reserved to mean no aux data. This is needed to handle + * allocs that occur before the aux callbacks were registered. */ - if (g_aux_cb_lookup) - (g_aux_cb_lookup)(pdata->aux_id, &pbuf[buf_used], (buf_len - buf_used - 1)); + if (pdata->aux_id > 0) { + p_snprintf(detail, sizeof(detail), "%saux_id: %d%s", + prefix, pdata->aux_id, suffix); + detail_len = strlen(detail); + if ((buf_used + detail_len + 1) < buf_len) { + memcpy(&pbuf[buf_used], detail, detail_len); + buf_used += detail_len; + } + + /* If an "aux" data provider is still registered, ask it to append its detailed + * data to the end of ours using the "aux_id" it gave us when this de-duped + * item was created. + */ + if (g_aux_cb_lookup) + (g_aux_cb_lookup)(pdata->aux_id, &pbuf[buf_used], (buf_len - buf_used - 1)); + } return GIT_OK; } diff --git a/src/win32/w32_stack.h b/src/win32/w32_stack.h index b457d2e8ac6..21170bd2f81 100644 --- a/src/win32/w32_stack.h +++ b/src/win32/w32_stack.h @@ -19,7 +19,8 @@ * This callback will be called during crtdbg-instrumented allocs. * * @param aux_id [out] A returned "aux_id" representing a unique - * (de-duped at the C# layer) stacktrace. + * (de-duped at the C# layer) stacktrace. "aux_id" 0 is reserved + * to mean no aux stacktrace data. */ typedef void (*git_win32__stack__aux_cb_alloc)(unsigned int *aux_id); @@ -60,11 +61,16 @@ GIT_EXTERN(int) git_win32__stack__set_aux_cb( /** * Wrapper containing the raw unprocessed stackframe * data for a single stacktrace and any "aux_id". + * + * I put the aux_id first so leaks will be sorted by it. + * So, for example, if a specific callstack in C# leaks + * a repo handle, all of the pointers within the associated + * repo pointer will be grouped together. */ typedef struct { - void *frames[GIT_WIN32__STACK__MAX_FRAMES]; - unsigned int nr_frames; unsigned int aux_id; + unsigned int nr_frames; + void *frames[GIT_WIN32__STACK__MAX_FRAMES]; } git_win32__stack__raw_data; From ccef5adb63bdba7f5182aec9f0bdc83a2887d9d1 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Latour Date: Tue, 30 Jun 2015 09:30:20 -0700 Subject: [PATCH 03/73] Added git_diff_index_to_index() --- include/git2/diff.h | 19 +++++++++++++++++++ src/diff.c | 25 +++++++++++++++++++++++++ tests/diff/index.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/include/git2/diff.h b/include/git2/diff.h index b3ab5397e21..0abbc7f06f8 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -835,6 +835,25 @@ GIT_EXTERN(int) git_diff_tree_to_workdir_with_index( git_tree *old_tree, const git_diff_options *opts); /**< can be NULL for defaults */ +/** + * Create a diff with the difference between two index objects. + * + * The first index will be used for the "old_file" side of the delta and the + * second index will be used for the "new_file" side of the delta. + * + * @param diff Output pointer to a git_diff pointer to be allocated. + * @param repo The repository containing the indexes. + * @param old_index A git_index object to diff from. + * @param new_index A git_index object to diff to. + * @param opts Structure with options to influence diff or NULL for defaults. + */ +GIT_EXTERN(int) git_diff_index_to_index( + git_diff **diff, + git_repository *repo, + git_index *old_index, + git_index *new_index, + const git_diff_options *opts); /**< can be NULL for defaults */ + /** * Merge one diff into another. * diff --git a/src/diff.c b/src/diff.c index c1adcc662c1..44f62788086 100644 --- a/src/diff.c +++ b/src/diff.c @@ -1421,6 +1421,31 @@ int git_diff_tree_to_workdir_with_index( return error; } +int git_diff_index_to_index( + git_diff **diff, + git_repository *repo, + git_index *old_index, + git_index *new_index, + const git_diff_options *opts) +{ + int error = 0; + + assert(diff && old_index && new_index); + + DIFF_FROM_ITERATORS( + git_iterator_for_index( + &a, old_index, GIT_ITERATOR_DONT_IGNORE_CASE, pfx, pfx), + git_iterator_for_index( + &b, new_index, GIT_ITERATOR_DONT_IGNORE_CASE, pfx, pfx) + ); + + /* if index is in case-insensitive order, re-sort deltas to match */ + if (!error && (old_index->ignore_case || new_index->ignore_case)) + diff_set_ignore_case(*diff, true); + + return error; +} + size_t git_diff_num_deltas(const git_diff *diff) { assert(diff); diff --git a/tests/diff/index.c b/tests/diff/index.c index f702568bfb6..df45ad236bf 100644 --- a/tests/diff/index.c +++ b/tests/diff/index.c @@ -268,3 +268,35 @@ void test_diff_index__not_in_head_conflicted(void) git_index_free(index); git_tree_free(a); } + +void test_diff_index__to_index(void) +{ + const char *a_commit = "26a125ee1bf"; /* the current HEAD */ + git_tree *old_tree; + git_index *old_index; + git_index *new_index; + git_diff *diff; + diff_expects exp; + + cl_git_pass(git_index_new(&old_index)); + old_tree = resolve_commit_oid_to_tree(g_repo, a_commit); + cl_git_pass(git_index_read_tree(old_index, old_tree)); + + cl_git_pass(git_repository_index(&new_index, g_repo)); + + cl_git_pass(git_diff_index_to_index(&diff, g_repo, old_index, new_index, NULL)); + + memset(&exp, 0, sizeof(diff_expects)); + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_binary_cb, diff_hunk_cb, diff_line_cb, &exp)); + cl_assert_equal_i(8, exp.files); + cl_assert_equal_i(3, exp.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(2, exp.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(3, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_CONFLICTED]); + + git_diff_free(diff); + git_index_free(new_index); + git_index_free(old_index); + git_tree_free(old_tree); +} From 63924435a103d34315ad9a4851d84b2b052aca38 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 1 Jul 2015 09:40:11 -0500 Subject: [PATCH 04/73] filters: custom filters with wildcard attributes Allow custom filters with wildcard attributes, so that clients can support some random `filter=foo` in a .gitattributes and look up the corresponding smudge/clean commands in the configuration file. --- CHANGELOG.md | 4 + include/git2/sys/filter.h | 5 +- src/filter.c | 7 +- tests/filter/custom.c | 107 +------------------- tests/filter/custom_helpers.c | 108 ++++++++++++++++++++ tests/filter/custom_helpers.h | 18 ++++ tests/filter/wildcard.c | 181 ++++++++++++++++++++++++++++++++++ 7 files changed, 323 insertions(+), 107 deletions(-) create mode 100644 tests/filter/custom_helpers.c create mode 100644 tests/filter/custom_helpers.h create mode 100644 tests/filter/wildcard.c diff --git a/CHANGELOG.md b/CHANGELOG.md index 852000bba61..735f1fe27cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,10 @@ support for HTTPS connections insead of OpenSSL. * If libcurl is installed, we will use it to connect to HTTP(S) servers. +* Custom filters can now be registered with wildcard attributes, for + example `filter=*`. Consumers should examine the attributes parameter + of the `check` function for details. + ### API additions * The `git_merge_options` gained a `file_flags` member. diff --git a/include/git2/sys/filter.h b/include/git2/sys/filter.h index 5fd8d5566d8..baf1515d62c 100644 --- a/include/git2/sys/filter.h +++ b/include/git2/sys/filter.h @@ -240,7 +240,10 @@ typedef void (*git_filter_cleanup_fn)( * for this filter (e.g. "eol crlf text"). If the attribute name is bare, * it will be simply loaded and passed to the `check` callback. If it has * a value (i.e. "name=value"), the attribute must match that value for - * the filter to be applied. + * the filter to be applied. The value may be a wildcard (eg, "name=*"), + * in which case the filter will be invoked for any value for the given + * attribute name. See the attribute parameter of the `check` callback + * for the attribute value that was specified. * * The `initialize`, `shutdown`, `check`, `apply`, and `cleanup` callbacks * are all documented above with the respective function pointer typedefs. diff --git a/src/filter.c b/src/filter.c index e25d37c353c..70c4fa3824b 100644 --- a/src/filter.c +++ b/src/filter.c @@ -433,8 +433,11 @@ static int filter_list_check_attributes( want_type = git_attr_value(want); found_type = git_attr_value(strs[i]); - if (want_type != found_type || - (want_type == GIT_ATTR_VALUE_T && strcmp(want, strs[i]))) + if (want_type != found_type) + error = GIT_ENOTFOUND; + else if (want_type == GIT_ATTR_VALUE_T && + strcmp(want, strs[i]) && + strcmp(want, "*")) error = GIT_ENOTFOUND; } diff --git a/tests/filter/custom.c b/tests/filter/custom.c index 493d26c80ca..fd1cd271c7f 100644 --- a/tests/filter/custom.c +++ b/tests/filter/custom.c @@ -5,6 +5,7 @@ #include "buf_text.h" #include "git2/sys/filter.h" #include "git2/sys/repository.h" +#include "custom_helpers.h" /* going TO_WORKDIR, filters are executed low to high * going TO_ODB, filters are executed high to low @@ -12,8 +13,6 @@ #define BITFLIP_FILTER_PRIORITY -1 #define REVERSE_FILTER_PRIORITY -2 -#define VERY_SECURE_ENCRYPTION(b) ((b) ^ 0xff) - #ifdef GIT_WIN32 # define NEWLINE "\r\n" #else @@ -27,6 +26,8 @@ static char workdir_data[] = "trivially" NEWLINE "scrambled." NEWLINE; +#define REVERSED_DATA_LEN 51 + /* Represents the data above scrambled (bits flipped) after \r\n -> \n * conversion, then bytewise reversed */ @@ -63,107 +64,6 @@ void test_filter_custom__cleanup(void) g_repo = NULL; } -static int bitflip_filter_apply( - git_filter *self, - void **payload, - git_buf *to, - const git_buf *from, - const git_filter_source *source) -{ - const unsigned char *src = (const unsigned char *)from->ptr; - unsigned char *dst; - size_t i; - - GIT_UNUSED(self); GIT_UNUSED(payload); - - /* verify that attribute path match worked as expected */ - cl_assert_equal_i( - 0, git__strncmp("hero", git_filter_source_path(source), 4)); - - if (!from->size) - return 0; - - cl_git_pass(git_buf_grow(to, from->size)); - - dst = (unsigned char *)to->ptr; - - for (i = 0; i < from->size; i++) - dst[i] = VERY_SECURE_ENCRYPTION(src[i]); - - to->size = from->size; - - return 0; -} - -static void bitflip_filter_free(git_filter *f) -{ - git__free(f); -} - -static git_filter *create_bitflip_filter(void) -{ - git_filter *filter = git__calloc(1, sizeof(git_filter)); - cl_assert(filter); - - filter->version = GIT_FILTER_VERSION; - filter->attributes = "+bitflip"; - filter->shutdown = bitflip_filter_free; - filter->apply = bitflip_filter_apply; - - return filter; -} - - -static int reverse_filter_apply( - git_filter *self, - void **payload, - git_buf *to, - const git_buf *from, - const git_filter_source *source) -{ - const unsigned char *src = (const unsigned char *)from->ptr; - const unsigned char *end = src + from->size; - unsigned char *dst; - - GIT_UNUSED(self); GIT_UNUSED(payload); GIT_UNUSED(source); - - /* verify that attribute path match worked as expected */ - cl_assert_equal_i( - 0, git__strncmp("hero", git_filter_source_path(source), 4)); - - if (!from->size) - return 0; - - cl_git_pass(git_buf_grow(to, from->size)); - - dst = (unsigned char *)to->ptr + from->size - 1; - - while (src < end) - *dst-- = *src++; - - to->size = from->size; - - return 0; -} - -static void reverse_filter_free(git_filter *f) -{ - git__free(f); -} - -static git_filter *create_reverse_filter(const char *attrs) -{ - git_filter *filter = git__calloc(1, sizeof(git_filter)); - cl_assert(filter); - - filter->version = GIT_FILTER_VERSION; - filter->attributes = attrs; - filter->shutdown = reverse_filter_free; - filter->apply = reverse_filter_apply; - - return filter; -} - static void register_custom_filters(void) { static int filters_registered = 0; @@ -186,7 +86,6 @@ static void register_custom_filters(void) } } - void test_filter_custom__to_odb(void) { git_filter_list *fl; diff --git a/tests/filter/custom_helpers.c b/tests/filter/custom_helpers.c new file mode 100644 index 00000000000..2c80212be09 --- /dev/null +++ b/tests/filter/custom_helpers.c @@ -0,0 +1,108 @@ +#include "clar_libgit2.h" +#include "posix.h" +#include "filter.h" +#include "buf_text.h" +#include "git2/sys/filter.h" + +#define VERY_SECURE_ENCRYPTION(b) ((b) ^ 0xff) + +int bitflip_filter_apply( + git_filter *self, + void **payload, + git_buf *to, + const git_buf *from, + const git_filter_source *source) +{ + const unsigned char *src = (const unsigned char *)from->ptr; + unsigned char *dst; + size_t i; + + GIT_UNUSED(self); GIT_UNUSED(payload); + + /* verify that attribute path match worked as expected */ + cl_assert_equal_i( + 0, git__strncmp("hero", git_filter_source_path(source), 4)); + + if (!from->size) + return 0; + + cl_git_pass(git_buf_grow(to, from->size)); + + dst = (unsigned char *)to->ptr; + + for (i = 0; i < from->size; i++) + dst[i] = VERY_SECURE_ENCRYPTION(src[i]); + + to->size = from->size; + + return 0; +} + +static void bitflip_filter_free(git_filter *f) +{ + git__free(f); +} + +git_filter *create_bitflip_filter(void) +{ + git_filter *filter = git__calloc(1, sizeof(git_filter)); + cl_assert(filter); + + filter->version = GIT_FILTER_VERSION; + filter->attributes = "+bitflip"; + filter->shutdown = bitflip_filter_free; + filter->apply = bitflip_filter_apply; + + return filter; +} + + +int reverse_filter_apply( + git_filter *self, + void **payload, + git_buf *to, + const git_buf *from, + const git_filter_source *source) +{ + const unsigned char *src = (const unsigned char *)from->ptr; + const unsigned char *end = src + from->size; + unsigned char *dst; + + GIT_UNUSED(self); GIT_UNUSED(payload); GIT_UNUSED(source); + + /* verify that attribute path match worked as expected */ + cl_assert_equal_i( + 0, git__strncmp("hero", git_filter_source_path(source), 4)); + + if (!from->size) + return 0; + + cl_git_pass(git_buf_grow(to, from->size)); + + dst = (unsigned char *)to->ptr + from->size - 1; + + while (src < end) + *dst-- = *src++; + + to->size = from->size; + + return 0; +} + +static void reverse_filter_free(git_filter *f) +{ + git__free(f); +} + +git_filter *create_reverse_filter(const char *attrs) +{ + git_filter *filter = git__calloc(1, sizeof(git_filter)); + cl_assert(filter); + + filter->version = GIT_FILTER_VERSION; + filter->attributes = attrs; + filter->shutdown = reverse_filter_free; + filter->apply = reverse_filter_apply; + + return filter; +} diff --git a/tests/filter/custom_helpers.h b/tests/filter/custom_helpers.h new file mode 100644 index 00000000000..13cfb23ae2f --- /dev/null +++ b/tests/filter/custom_helpers.h @@ -0,0 +1,18 @@ +#include "git2/sys/filter.h" + +extern git_filter *create_bitflip_filter(void); +extern git_filter *create_reverse_filter(const char *attr); + +extern int bitflip_filter_apply( + git_filter *self, + void **payload, + git_buf *to, + const git_buf *from, + const git_filter_source *source); + +extern int reverse_filter_apply( + git_filter *self, + void **payload, + git_buf *to, + const git_buf *from, + const git_filter_source *source); diff --git a/tests/filter/wildcard.c b/tests/filter/wildcard.c new file mode 100644 index 00000000000..ba826b5dc95 --- /dev/null +++ b/tests/filter/wildcard.c @@ -0,0 +1,181 @@ +#include "clar_libgit2.h" +#include "posix.h" +#include "blob.h" +#include "filter.h" +#include "buf_text.h" +#include "git2/sys/filter.h" +#include "git2/sys/repository.h" +#include "custom_helpers.h" + +static git_repository *g_repo = NULL; + +static git_filter *create_wildcard_filter(); + +#define DATA_LEN 32 + +static unsigned char input[] = { + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, +}; + +static unsigned char reversed[] = { + 0x1f, 0x1e, 0x1d, 0x1c, 0x1b, 0x1a, 0x19, 0x18, + 0x17, 0x16, 0x15, 0x14, 0x13, 0x12, 0x11, 0x10, + 0x0f, 0x0e, 0x0d, 0x0c, 0x0b, 0x0a, 0x09, 0x08, + 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01, 0x00, +}; + +static unsigned char flipped[] = { + 0xff, 0xfe, 0xfd, 0xfc, 0xfb, 0xfa, 0xf9, 0xf8, + 0xf7, 0xf6, 0xf5, 0xf4, 0xf3, 0xf2, 0xf1, 0xf0, + 0xef, 0xee, 0xed, 0xec, 0xeb, 0xea, 0xe9, 0xe8, + 0xe7, 0xe6, 0xe5, 0xe4, 0xe3, 0xe2, 0xe1, 0xe0, +}; + +void test_filter_wildcard__initialize(void) +{ + cl_git_pass(git_filter_register( + "wildcard", create_wildcard_filter(), GIT_FILTER_DRIVER_PRIORITY)); + + g_repo = cl_git_sandbox_init("empty_standard_repo"); + + cl_git_rewritefile( + "empty_standard_repo/.gitattributes", + "* binary\n" + "hero-flip-* filter=wcflip\n" + "hero-reverse-* filter=wcreverse\n" + "none-* filter=unregistered\n"); +} + +void test_filter_wildcard__cleanup(void) +{ + cl_git_pass(git_filter_unregister("wildcard")); + + cl_git_sandbox_cleanup(); + g_repo = NULL; +} + +static int wildcard_filter_check( + git_filter *self, + void **payload, + const git_filter_source *src, + const char **attr_values) +{ + if (strcmp(attr_values[0], "wcflip") == 0 || + strcmp(attr_values[0], "wcreverse") == 0) { + *payload = git__strdup(attr_values[0]); + GITERR_CHECK_ALLOC(*payload); + return 0; + } + + return GIT_PASSTHROUGH; +} + +static int wildcard_filter_apply( + git_filter *self, + void **payload, + git_buf *to, + const git_buf *from, + const git_filter_source *source) +{ + const char *filtername = *payload; + + if (filtername && strcmp(filtername, "wcflip") == 0) + return bitflip_filter_apply(self, payload, to, from, source); + else if (filtername && strcmp(filtername, "wcreverse") == 0) + return reverse_filter_apply(self, payload, to, from, source); + + cl_fail("Unexpected attribute"); + return GIT_PASSTHROUGH; +} + +static int wildcard_filter_cleanup(git_filter *self, void *payload) +{ + git__free(payload); + return 0; +} + +static void wildcard_filter_free(git_filter *f) +{ + git__free(f); +} + +static git_filter *create_wildcard_filter() +{ + git_filter *filter = git__calloc(1, sizeof(git_filter)); + cl_assert(filter); + + filter->version = GIT_FILTER_VERSION; + filter->attributes = "filter=*"; + filter->check = wildcard_filter_check; + filter->apply = wildcard_filter_apply; + filter->cleanup = wildcard_filter_cleanup; + filter->shutdown = wildcard_filter_free; + + return filter; +} + +void test_filter_wildcard__reverse(void) +{ + git_filter_list *fl; + git_buf in = GIT_BUF_INIT, out = GIT_BUF_INIT; + + cl_git_pass(git_filter_list_load( + &fl, g_repo, NULL, "hero-reverse-foo", GIT_FILTER_TO_ODB, 0)); + + cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); + + cl_assert_equal_i(DATA_LEN, out.size); + + cl_assert_equal_i( + 0, memcmp(reversed, out.ptr, out.size)); + + git_filter_list_free(fl); + git_buf_free(&out); + git_buf_free(&in); +} + +void test_filter_wildcard__flip(void) +{ + git_filter_list *fl; + git_buf in = GIT_BUF_INIT, out = GIT_BUF_INIT; + + cl_git_pass(git_filter_list_load( + &fl, g_repo, NULL, "hero-flip-foo", GIT_FILTER_TO_ODB, 0)); + + cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); + + cl_assert_equal_i(DATA_LEN, out.size); + + cl_assert_equal_i( + 0, memcmp(flipped, out.ptr, out.size)); + + git_filter_list_free(fl); + git_buf_free(&out); + git_buf_free(&in); +} + +void test_filter_wildcard__none(void) +{ + git_filter_list *fl; + git_buf in = GIT_BUF_INIT, out = GIT_BUF_INIT; + + cl_git_pass(git_filter_list_load( + &fl, g_repo, NULL, "none-foo", GIT_FILTER_TO_ODB, 0)); + + cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); + + cl_assert_equal_i(DATA_LEN, out.size); + + cl_assert_equal_i( + 0, memcmp(input, out.ptr, out.size)); + + git_filter_list_free(fl); + git_buf_free(&out); + git_buf_free(&in); +} From e069c621bdd62e603b048eb536f5a978a905b310 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jul 2015 09:25:48 -0500 Subject: [PATCH 05/73] git__getenv: utf-8 aware env reader Introduce `git__getenv` which is a UTF-8 aware `getenv` everywhere. Make `cl_getenv` use this to keep consistent memory handling around return values (free everywhere, as opposed to only some platforms). --- src/remote.c | 27 +++++++---- src/sysdir.c | 30 ++++++++---- src/util.c | 48 +++++++++++++++++++ src/util.h | 5 ++ src/win32/posix_w32.c | 2 + tests/clar_libgit2.c | 43 +++++++++-------- tests/clar_libgit2.h | 1 + tests/core/env.c | 20 ++++---- tests/core/ftruncate.c | 2 +- tests/filter/stream.c | 2 +- tests/online/clone.c | 103 +++++++++++++++++++++-------------------- tests/online/push.c | 23 ++++++--- tests/repo/init.c | 2 +- 13 files changed, 199 insertions(+), 109 deletions(-) diff --git a/src/remote.c b/src/remote.c index f31fc150a7c..d31e1b89ecb 100644 --- a/src/remote.c +++ b/src/remote.c @@ -153,7 +153,7 @@ static int get_check_cert(int *out, git_repository *repo) * most specific to least specific. */ /* GIT_SSL_NO_VERIFY environment variable */ - if ((val = getenv("GIT_SSL_NO_VERIFY")) != NULL) + if ((val = p_getenv("GIT_SSL_NO_VERIFY")) != NULL) return git_config_parse_bool(out, val); /* http.sslVerify config setting */ @@ -759,7 +759,7 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur { git_config *cfg; git_config_entry *ce = NULL; - const char *val = NULL; + git_buf val = GIT_BUF_INIT; int error; assert(remote); @@ -789,7 +789,7 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur return error; if (ce && ce->value) { - val = ce->value; + *proxy_url = git__strdup(ce->value); goto found; } } @@ -797,19 +797,28 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur /* http.proxy config setting */ if ((error = git_config__lookup_entry(&ce, cfg, "http.proxy", false)) < 0) return error; + if (ce && ce->value) { - val = ce->value; + *proxy_url = git__strdup(ce->value); goto found; } /* HTTP_PROXY / HTTPS_PROXY environment variables */ - val = use_ssl ? getenv("HTTPS_PROXY") : getenv("HTTP_PROXY"); + error = git__getenv(&val, use_ssl ? "HTTPS_PROXY" : "HTTP_PROXY"); -found: - if (val && val[0]) { - *proxy_url = git__strdup(val); - GITERR_CHECK_ALLOC(*proxy_url); + if (error < 0) { + if (error == GIT_ENOTFOUND) { + giterr_clear(); + error = 0; + } + + return error; } + + *proxy_url = git_buf_detach(&val); + +found: + GITERR_CHECK_ALLOC(*proxy_url); git_config_entry_free(ce); return 0; diff --git a/src/sysdir.c b/src/sysdir.c index cd94a8b5754..2795de4918f 100644 --- a/src/sysdir.c +++ b/src/sysdir.c @@ -29,7 +29,14 @@ static int git_sysdir_guess_global_dirs(git_buf *out) #ifdef GIT_WIN32 return git_win32__find_global_dirs(out); #else - return git_buf_sets(out, getenv("HOME")); + int error = git__getenv(out, "HOME"); + + if (error == GIT_ENOTFOUND) { + giterr_clear(); + error = 0; + } + + return error; #endif } @@ -38,15 +45,22 @@ static int git_sysdir_guess_xdg_dirs(git_buf *out) #ifdef GIT_WIN32 return git_win32__find_xdg_dirs(out); #else - const char *env = NULL; + git_buf env = GIT_BUF_INIT; + int error; - if ((env = getenv("XDG_CONFIG_HOME")) != NULL) - return git_buf_joinpath(out, env, "git"); - else if ((env = getenv("HOME")) != NULL) - return git_buf_joinpath(out, env, ".config/git"); + if ((error = git__getenv(&env, "XDG_CONFIG_HOME")) == 0) + error = git_buf_joinpath(out, env.ptr, "git"); - git_buf_clear(out); - return 0; + if (error == GIT_ENOTFOUND && (error = git__getenv(&env, "HOME")) == 0) + error = git_buf_joinpath(out, env.ptr, ".config/git"); + + if (error == GIT_ENOTFOUND) { + giterr_clear(); + error = 0; + } + + git_buf_free(&env); + return error; #endif } diff --git a/src/util.c b/src/util.c index c62826420f6..b08b2b884ee 100644 --- a/src/util.c +++ b/src/util.c @@ -10,6 +10,10 @@ #include #include "posix.h" +#ifdef GIT_WIN32 +# include "win32/buffer.h" +#endif + #ifdef _MSC_VER # include #endif @@ -765,3 +769,47 @@ int git__utf8_iterate(const uint8_t *str, int str_len, int32_t *dst) *dst = uc; return length; } + +#ifdef GIT_WIN32 +int git__getenv(git_buf *out, const char *name) +{ + wchar_t *wide_name = NULL, *wide_value = NULL; + DWORD value_len; + int error = -1; + + git_buf_clear(out); + + if (git__utf8_to_16_alloc(&wide_name, name) < 0) + return -1; + + if ((value_len = GetEnvironmentVariableW(wide_name, NULL, 0)) > 0) { + wide_value = git__malloc(value_len * sizeof(wchar_t)); + GITERR_CHECK_ALLOC(wide_value); + + value_len = GetEnvironmentVariableW(wide_name, wide_value, value_len); + } + + if (value_len) + error = git_buf_put_w(out, wide_value, value_len); + else if (GetLastError() == ERROR_ENVVAR_NOT_FOUND) + error = GIT_ENOTFOUND; + else + giterr_set(GITERR_OS, "could not read environment variable '%s'", name); + + git__free(wide_name); + git__free(wide_value); + return error; +} +#else +int git__getenv(git_buf *out, const char *name) +{ + const char *val = getenv(name); + + git_buf_clear(out); + + if (!val) + return GIT_ENOTFOUND; + + return git_buf_puts(out, val); +} +#endif diff --git a/src/util.h b/src/util.h index b2abbe6a652..458b0db41fa 100644 --- a/src/util.h +++ b/src/util.h @@ -7,6 +7,9 @@ #ifndef INCLUDE_util_h__ #define INCLUDE_util_h__ +#include "git2/buffer.h" +#include "buffer.h" + #if defined(GIT_MSVC_CRTDBG) /* Enable MSVC CRTDBG memory leak reporting. * @@ -596,4 +599,6 @@ GIT_INLINE(double) git__timer(void) #endif +extern int git__getenv(git_buf *out, const char *name); + #endif /* INCLUDE_util_h__ */ diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 504562b0e3e..c909af6cc04 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -11,6 +11,8 @@ #include "utf-conv.h" #include "repository.h" #include "reparse.h" +#include "global.h" +#include "buffer.h" #include #include #include diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c index dabc47a0925..f73fc5b0091 100644 --- a/tests/clar_libgit2.c +++ b/tests/clar_libgit2.c @@ -58,31 +58,38 @@ void cl_git_rmfile(const char *filename) cl_must_pass(p_unlink(filename)); } -#ifdef GIT_WIN32 - -#include "win32/utf-conv.h" - char *cl_getenv(const char *name) { - wchar_t *wide_name, *wide_value; - char *utf8_value = NULL; - DWORD value_len; + git_buf out = GIT_BUF_INIT; + int error = git__getenv(&out, name); - cl_assert(git__utf8_to_16_alloc(&wide_name, name) >= 0); + cl_assert(error >= 0 || error == GIT_ENOTFOUND); - value_len = GetEnvironmentVariableW(wide_name, NULL, 0); + if (error == GIT_ENOTFOUND) + return NULL; - if (value_len) { - cl_assert(wide_value = git__malloc(value_len * sizeof(wchar_t))); - cl_assert(GetEnvironmentVariableW(wide_name, wide_value, value_len)); - cl_assert(git__utf16_to_8_alloc(&utf8_value, wide_value) >= 0); - git__free(wide_value); + if (out.size == 0) { + char *dup = git__strdup(""); + cl_assert(dup); + + return dup; } - git__free(wide_name); - return utf8_value; + return git_buf_detach(&out); } +bool cl_is_env_set(const char *name) +{ + char *env = cl_getenv(name); + bool result = (env != NULL); + git__free(env); + return result; +} + +#ifdef GIT_WIN32 + +#include "win32/utf-conv.h" + int cl_setenv(const char *name, const char *value) { wchar_t *wide_name, *wide_value = NULL; @@ -138,10 +145,6 @@ int cl_rename(const char *source, const char *dest) #else #include -char *cl_getenv(const char *name) -{ - return getenv(name); -} int cl_setenv(const char *name, const char *value) { diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h index 9ab0da4f602..d7e6353026f 100644 --- a/tests/clar_libgit2.h +++ b/tests/clar_libgit2.h @@ -119,6 +119,7 @@ bool cl_is_chmod_supported(void); /* Environment wrappers */ char *cl_getenv(const char *name); +bool cl_is_env_set(const char *name); int cl_setenv(const char *name, const char *value); /* Reliable rename */ diff --git a/tests/core/env.c b/tests/core/env.c index 293b786db1c..ee08258a658 100644 --- a/tests/core/env.c +++ b/tests/core/env.c @@ -30,14 +30,8 @@ static char *home_values[] = { void test_core_env__initialize(void) { int i; - for (i = 0; i < NUM_VARS; ++i) { - const char *original = cl_getenv(env_vars[i]); -#ifdef GIT_WIN32 - env_save[i] = (char *)original; -#else - env_save[i] = original ? git__strdup(original) : NULL; -#endif - } + for (i = 0; i < NUM_VARS; ++i) + env_save[i] = cl_getenv(env_vars[i]); } static void set_global_search_path_from_env(void) @@ -77,12 +71,14 @@ static void setenv_and_check(const char *name, const char *value) char *check; cl_git_pass(cl_setenv(name, value)); - check = cl_getenv(name); - cl_assert_equal_s(value, check); -#ifdef GIT_WIN32 + + if (value) + cl_assert_equal_s(value, check); + else + cl_assert(check == NULL); + git__free(check); -#endif } void test_core_env__0(void) diff --git a/tests/core/ftruncate.c b/tests/core/ftruncate.c index 21981d677b1..2f4729fc2f0 100644 --- a/tests/core/ftruncate.c +++ b/tests/core/ftruncate.c @@ -10,7 +10,7 @@ static int fd = -1; void test_core_ftruncate__initialize(void) { - if (!cl_getenv("GITTEST_INVASIVE_FS_SIZE")) + if (!cl_is_env_set("GITTEST_INVASIVE_FS_SIZE")) cl_skip(); cl_must_pass((fd = p_open(filename, O_CREAT | O_RDWR, 0644))); diff --git a/tests/filter/stream.c b/tests/filter/stream.c index 603f1949476..9228911b6df 100644 --- a/tests/filter/stream.c +++ b/tests/filter/stream.c @@ -214,7 +214,7 @@ void test_filter_stream__smallfile(void) /* optionally write a 500 MB file through the compression stream */ void test_filter_stream__bigfile(void) { - if (!cl_getenv("GITTEST_INVASIVE_FS_SIZE")) + if (!cl_is_env_set("GITTEST_INVASIVE_FS_SIZE")) cl_skip(); test_stream(51200); diff --git a/tests/online/clone.c b/tests/online/clone.c index e63cf55f1a8..225b3abe2e8 100644 --- a/tests/online/clone.c +++ b/tests/online/clone.c @@ -17,6 +17,15 @@ static git_repository *g_repo; static git_clone_options g_options; +static char *_remote_url = NULL; +static char *_remote_user = NULL; +static char *_remote_pass = NULL; +static char *_remote_ssh_pubkey = NULL; +static char *_remote_ssh_privkey = NULL; +static char *_remote_ssh_passphrase = NULL; +static char *_remote_ssh_fingerprint = NULL; + + void test_online_clone__initialize(void) { git_checkout_options dummy_opts = GIT_CHECKOUT_OPTIONS_INIT; @@ -29,6 +38,14 @@ void test_online_clone__initialize(void) g_options.checkout_opts = dummy_opts; g_options.checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE; g_options.fetch_opts = dummy_fetch; + + _remote_url = cl_getenv("GITTEST_REMOTE_URL"); + _remote_user = cl_getenv("GITTEST_REMOTE_USER"); + _remote_pass = cl_getenv("GITTEST_REMOTE_PASS"); + _remote_ssh_pubkey = cl_getenv("GITTEST_REMOTE_SSH_PUBKEY"); + _remote_ssh_privkey = cl_getenv("GITTEST_REMOTE_SSH_KEY"); + _remote_ssh_passphrase = cl_getenv("GITTEST_REMOTE_SSH_PASSPHRASE"); + _remote_ssh_fingerprint = cl_getenv("GITTEST_REMOTE_SSH_FINGERPRINT"); } void test_online_clone__cleanup(void) @@ -38,6 +55,14 @@ void test_online_clone__cleanup(void) g_repo = NULL; } cl_fixture_cleanup("./foo"); + + git__free(_remote_url); + git__free(_remote_user); + git__free(_remote_pass); + git__free(_remote_ssh_pubkey); + git__free(_remote_ssh_privkey); + git__free(_remote_ssh_passphrase); + git__free(_remote_ssh_fingerprint); } void test_online_clone__network_full(void) @@ -202,15 +227,12 @@ static int cred_failure_cb( void test_online_clone__cred_callback_failure_return_code_is_tunnelled(void) { - const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); - - if (!remote_url || !remote_user) + if (!_remote_url || !_remote_user) clar__skip(); g_options.fetch_opts.callbacks.credentials = cred_failure_cb; - cl_git_fail_with(-172, git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_fail_with(-172, git_clone(&g_repo, _remote_url, "./foo", &g_options)); } static int cred_count_calls_cb(git_cred **cred, const char *url, const char *user, @@ -233,17 +255,15 @@ static int cred_count_calls_cb(git_cred **cred, const char *url, const char *use void test_online_clone__cred_callback_called_again_on_auth_failure(void) { - const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); size_t counter = 0; - if (!remote_url || !remote_user) + if (!_remote_url || !_remote_user) clar__skip(); g_options.fetch_opts.callbacks.credentials = cred_count_calls_cb; g_options.fetch_opts.callbacks.payload = &counter; - cl_git_fail_with(GIT_EUSER, git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_fail_with(GIT_EUSER, git_clone(&g_repo, _remote_url, "./foo", &g_options)); cl_assert_equal_i(3, counter); } @@ -269,22 +289,22 @@ void test_online_clone__credentials(void) /* Remote URL environment variable must be set. * User and password are optional. */ - const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); git_cred_userpass_payload user_pass = { - cl_getenv("GITTEST_REMOTE_USER"), - cl_getenv("GITTEST_REMOTE_PASS") + _remote_user, + _remote_pass }; - if (!remote_url) return; + if (!_remote_url) + clar__skip(); - if (cl_getenv("GITTEST_REMOTE_DEFAULT")) { + if (cl_is_env_set("GITTEST_REMOTE_DEFAULT")) { g_options.fetch_opts.callbacks.credentials = cred_default; } else { g_options.fetch_opts.callbacks.credentials = git_cred_userpass; g_options.fetch_opts.callbacks.payload = &user_pass; } - cl_git_pass(git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_pass(git_clone(&g_repo, _remote_url, "./foo", &g_options)); git_repository_free(g_repo); g_repo = NULL; cl_fixture_cleanup("./foo"); } @@ -335,18 +355,15 @@ void test_online_clone__can_cancel(void) static int cred_cb(git_cred **cred, const char *url, const char *user_from_url, unsigned int allowed_types, void *payload) { - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); - const char *pubkey = cl_getenv("GITTEST_REMOTE_SSH_PUBKEY"); - const char *privkey = cl_getenv("GITTEST_REMOTE_SSH_KEY"); - const char *passphrase = cl_getenv("GITTEST_REMOTE_SSH_PASSPHRASE"); - GIT_UNUSED(url); GIT_UNUSED(user_from_url); GIT_UNUSED(payload); if (allowed_types & GIT_CREDTYPE_USERNAME) - return git_cred_username_new(cred, remote_user); + return git_cred_username_new(cred, _remote_user); if (allowed_types & GIT_CREDTYPE_SSH_KEY) - return git_cred_ssh_key_new(cred, remote_user, pubkey, privkey, passphrase); + return git_cred_ssh_key_new(cred, + _remote_user, _remote_ssh_pubkey, + _remote_ssh_privkey, _remote_ssh_passphrase); giterr_set(GITERR_NET, "unexpected cred type"); return -1; @@ -417,13 +434,10 @@ void test_online_clone__ssh_with_paths(void) 2, }; - const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); - #ifndef GIT_SSH clar__skip(); #endif - if (!remote_url || !remote_user || strncmp(remote_url, "ssh://", 5) != 0) + if (!_remote_url || !_remote_user || strncmp(_remote_url, "ssh://", 5) != 0) clar__skip(); g_options.remote_cb = custom_remote_ssh_with_paths; @@ -431,10 +445,10 @@ void test_online_clone__ssh_with_paths(void) g_options.fetch_opts.callbacks.credentials = cred_cb; g_options.fetch_opts.callbacks.payload = &arr; - cl_git_fail(git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_fail(git_clone(&g_repo, _remote_url, "./foo", &g_options)); arr.strings = good_paths; - cl_git_pass(git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_pass(git_clone(&g_repo, _remote_url, "./foo", &g_options)); } static int cred_foo_bar(git_cred **cred, const char *url, const char *username_from_url, @@ -460,15 +474,13 @@ int ssh_certificate_check(git_cert *cert, int valid, const char *host, void *pay { git_cert_hostkey *key; git_oid expected = {{0}}, actual = {{0}}; - const char *expected_str; GIT_UNUSED(valid); GIT_UNUSED(payload); - expected_str = cl_getenv("GITTEST_REMOTE_SSH_FINGERPRINT"); - cl_assert(expected_str); + cl_assert(_remote_ssh_fingerprint); - cl_git_pass(git_oid_fromstrp(&expected, expected_str)); + cl_git_pass(git_oid_fromstrp(&expected, _remote_ssh_fingerprint)); cl_assert_equal_i(GIT_CERT_HOSTKEY_LIBSSH2, cert->cert_type); key = (git_cert_hostkey *) cert; @@ -477,9 +489,9 @@ int ssh_certificate_check(git_cert *cert, int valid, const char *host, void *pay * the type. Here we abuse the fact that both hashes fit into * our git_oid type. */ - if (strlen(expected_str) == 32 && key->type & GIT_CERT_SSH_MD5) { + if (strlen(_remote_ssh_fingerprint) == 32 && key->type & GIT_CERT_SSH_MD5) { memcpy(&actual.id, key->hash_md5, 16); - } else if (strlen(expected_str) == 40 && key->type & GIT_CERT_SSH_SHA1) { + } else if (strlen(_remote_ssh_fingerprint) == 40 && key->type & GIT_CERT_SSH_SHA1) { memcpy(&actual, key->hash_sha1, 20); } else { cl_fail("Cannot find a usable SSH hash"); @@ -496,7 +508,7 @@ void test_online_clone__ssh_cert(void) { g_options.fetch_opts.callbacks.certificate_check = ssh_certificate_check; - if (!cl_getenv("GITTEST_REMOTE_SSH_FINGERPRINT")) + if (!_remote_ssh_fingerprint) cl_skip(); cl_git_fail_with(GIT_EUSER, git_clone(&g_repo, "ssh://localhost/foo", "./foo", &g_options)); @@ -525,22 +537,17 @@ static char *read_key_file(const char *path) static int ssh_memory_cred_cb(git_cred **cred, const char *url, const char *user_from_url, unsigned int allowed_types, void *payload) { - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); - const char *pubkey_path = cl_getenv("GITTEST_REMOTE_SSH_PUBKEY"); - const char *privkey_path = cl_getenv("GITTEST_REMOTE_SSH_KEY"); - const char *passphrase = cl_getenv("GITTEST_REMOTE_SSH_PASSPHRASE"); - GIT_UNUSED(url); GIT_UNUSED(user_from_url); GIT_UNUSED(payload); if (allowed_types & GIT_CREDTYPE_USERNAME) - return git_cred_username_new(cred, remote_user); + return git_cred_username_new(cred, _remote_user); if (allowed_types & GIT_CREDTYPE_SSH_KEY) { - char *pubkey = read_key_file(pubkey_path); - char *privkey = read_key_file(privkey_path); + char *pubkey = read_key_file(_remote_ssh_pubkey); + char *privkey = read_key_file(_remote_ssh_privkey); - int ret = git_cred_ssh_key_memory_new(cred, remote_user, pubkey, privkey, passphrase); + int ret = git_cred_ssh_key_memory_new(cred, _remote_user, pubkey, privkey, _remote_ssh_passphrase); if (privkey) free(privkey); @@ -555,19 +562,15 @@ static int ssh_memory_cred_cb(git_cred **cred, const char *url, const char *user void test_online_clone__ssh_memory_auth(void) { - const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); - const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); - const char *privkey = cl_getenv("GITTEST_REMOTE_SSH_KEY"); - #ifndef GIT_SSH_MEMORY_CREDENTIALS clar__skip(); #endif - if (!remote_url || !remote_user || !privkey || strncmp(remote_url, "ssh://", 5) != 0) + if (!_remote_url || !_remote_user || !_remote_ssh_privkey || strncmp(_remote_url, "ssh://", 5) != 0) clar__skip(); g_options.fetch_opts.callbacks.credentials = ssh_memory_cred_cb; - cl_git_pass(git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_git_pass(git_clone(&g_repo, _remote_url, "./foo", &g_options)); } void test_online_clone__url_with_no_path_returns_EINVALIDSPEC(void) diff --git a/tests/online/push.c b/tests/online/push.c index 6cd444320d9..0b0892c97ab 100644 --- a/tests/online/push.c +++ b/tests/online/push.c @@ -9,16 +9,16 @@ static git_repository *_repo; -static char *_remote_url; +static char *_remote_url = NULL; -static char *_remote_ssh_key; -static char *_remote_ssh_pubkey; -static char *_remote_ssh_passphrase; +static char *_remote_user = NULL; +static char *_remote_pass = NULL; -static char *_remote_user; -static char *_remote_pass; +static char *_remote_ssh_key = NULL; +static char *_remote_ssh_pubkey = NULL; +static char *_remote_ssh_passphrase = NULL; -static char *_remote_default; +static char *_remote_default = NULL; static int cred_acquire_cb(git_cred **, const char *, const char *, unsigned int, void *); @@ -355,6 +355,7 @@ void test_online_push__initialize(void) git_oid_fromstr(&_tag_tag, "eea4f2705eeec2db3813f2430829afce99cd00b5"); /* Remote URL environment variable must be set. User and password are optional. */ + _remote_url = cl_getenv("GITTEST_REMOTE_URL"); _remote_user = cl_getenv("GITTEST_REMOTE_USER"); _remote_pass = cl_getenv("GITTEST_REMOTE_PASS"); @@ -406,6 +407,14 @@ void test_online_push__cleanup(void) git_remote_free(_remote); _remote = NULL; + git__free(_remote_url); + git__free(_remote_user); + git__free(_remote_pass); + git__free(_remote_ssh_key); + git__free(_remote_ssh_pubkey); + git__free(_remote_ssh_passphrase); + git__free(_remote_default); + /* Freed by cl_git_sandbox_cleanup */ _repo = NULL; diff --git a/tests/repo/init.c b/tests/repo/init.c index 525020f5a4b..929d7418004 100644 --- a/tests/repo/init.c +++ b/tests/repo/init.c @@ -713,7 +713,7 @@ void test_repo_init__at_filesystem_root(void) git_buf root = GIT_BUF_INIT; int root_len; - if (!cl_getenv("GITTEST_INVASIVE_FS_STRUCTURE")) + if (!cl_is_env_set("GITTEST_INVASIVE_FS_STRUCTURE")) cl_skip(); root_len = git_path_root(sandbox); From 159061a8ce206b694448313a84387600408f6029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 6 Jul 2015 16:23:44 +0200 Subject: [PATCH 06/73] Update CHANGELOG with the release number --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 418055ca319..b824a66da3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,13 @@ -v0.22 + 1 +v0.23 + 1 +------- + +### Changes or improvements + +### API additions + +### API removals + +v0.23 ------ ### Changes or improvements From d41b8ed083a3e529a8a16db513087266e4424a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 6 Jul 2015 18:32:31 +0200 Subject: [PATCH 07/73] travis: update the homebrew db We need to make sure we are asking for the current version of packages, or we might get 404s from the download service. --- script/install-deps-osx.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/script/install-deps-osx.sh b/script/install-deps-osx.sh index c2e0162d839..a0216e9c837 100755 --- a/script/install-deps-osx.sh +++ b/script/install-deps-osx.sh @@ -2,4 +2,5 @@ set -x +brew update brew install libssh2 cmake From a0bdfe32412b687910e809cced2d31d1e76918fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 6 Jul 2015 18:42:39 +0200 Subject: [PATCH 08/73] travis: don't install CMake on OS X Homebrew will error out because it's already installed. --- script/install-deps-osx.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/install-deps-osx.sh b/script/install-deps-osx.sh index a0216e9c837..5510379d41d 100755 --- a/script/install-deps-osx.sh +++ b/script/install-deps-osx.sh @@ -3,4 +3,4 @@ set -x brew update -brew install libssh2 cmake +brew install libssh2 From febc8c4612b6be1d891f658e50f3f70cc5dc5945 Mon Sep 17 00:00:00 2001 From: Tony Kelman Date: Tue, 7 Jul 2015 06:55:05 -0400 Subject: [PATCH 09/73] Fix undefined reference with old versions of openssl Versions prior to 0.9.8f did not have this function, rhel/centos5 are still on a heavily backported version of 0.9.8e and theoretically supported until March 2017 Without this ifdef, I get the following link failure: ``` CMakeFiles/libgit2_clar.dir/src/openssl_stream.c.o: In function `openssl_connect': openssl_stream.c:(.text+0x45a): undefined reference to `SSL_set_tlsext_host_name' collect2: error: ld returned 1 exit status make[6]: *** [libgit2_clar] Error 1 ``` --- src/openssl_stream.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 958252e9f66..4df7c6b7c45 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -324,7 +324,9 @@ int openssl_connect(git_stream *stream) SSL_set_bio(st->ssl, bio, bio); /* specify the host in case SNI is needed */ +#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME SSL_set_tlsext_host_name(st->ssl, st->host); +#endif if ((ret = SSL_connect(st->ssl)) <= 0) return ssl_set_error(st->ssl, ret); From 01e031d921820efe9fa849d6b6a8848f341f2df4 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 7 Jul 2015 12:40:15 -0500 Subject: [PATCH 10/73] meta: Add Antoine Pelisse to hall-of-fame list Antoine Pelisse has kindly allowed his contributions to core git to be used under the libgit2 license. --- git.git-authors | 1 + 1 file changed, 1 insertion(+) diff --git a/git.git-authors b/git.git-authors index 9131a1fa126..6a85224b466 100644 --- a/git.git-authors +++ b/git.git-authors @@ -39,6 +39,7 @@ ok Adam Simpkins (http transport) ok Adrian Johnson ok Alexey Shumkin ok Andreas Ericsson +ok Antoine Pelisse ok Boyd Lynn Gerber ok Brandon Casey ok Brian Downing From ae8f7260ecbfc64e49d77ac2e86122971a560670 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 7 Jul 2015 16:59:14 -0500 Subject: [PATCH 11/73] merge_files: don't add trailing newlines When invoked with three files that each lack a trailing newline, the merge result should also lack a trailing newline. --- tests/merge/files.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/merge/files.c b/tests/merge/files.c index 7f461abff67..2fd90d0666f 100644 --- a/tests/merge/files.c +++ b/tests/merge/files.c @@ -249,3 +249,42 @@ void test_merge_files__automerge_whitespace_change(void) git_merge_file_result_free(&result); } + +void test_merge_files__doesnt_add_newline(void) +{ + git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT, + ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; + git_merge_file_result result = {0}; + const char *expected = "Zero\n1\n2\n3\n4\n5 XXX\n6 YYY\n7\n8\n9\nTen"; + + ancestor.ptr = "0\n1\n2\n3\n4\n5 XXX\n6YYY\n7\n8\n9\n10"; + ancestor.size = strlen(ancestor.ptr); + ancestor.path = "testfile.txt"; + ancestor.mode = 0100755; + + ours.ptr = "Zero\n1\n2\n3\n4\n5 XXX\n6 YYY\n7\n8\n9\n10"; + ours.size = strlen(ours.ptr); + ours.path = "testfile.txt"; + ours.mode = 0100755; + + theirs.ptr = "0\n1\n2\n3\n4\n5 XXX\n6 YYY\n7\n8\n9\nTen"; + theirs.size = strlen(theirs.ptr); + theirs.path = "testfile.txt"; + theirs.mode = 0100755; + + opts.flags |= GIT_MERGE_FILE_IGNORE_WHITESPACE_CHANGE; + cl_git_pass(git_merge_file(&result, &ancestor, &ours, &theirs, &opts)); + + cl_assert_equal_i(1, result.automergeable); + + cl_assert_equal_s("testfile.txt", result.path); + cl_assert_equal_i(0100755, result.mode); + + cl_assert_equal_i(strlen(expected), result.len); + cl_assert_equal_strn(expected, result.ptr, result.len); + + git_merge_file_result_free(&result); +} + From 43ce8cb52e86373711e8ec361c3052c8c39e7a2c Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 7 Jul 2015 16:46:20 -0500 Subject: [PATCH 12/73] revert: correct test that added trailing newline --- tests/revert/workdir.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/revert/workdir.c b/tests/revert/workdir.c index 7ccf0f937da..9f83bd8424b 100644 --- a/tests/revert/workdir.c +++ b/tests/revert/workdir.c @@ -334,16 +334,18 @@ void test_revert_workdir__again_after_edit_two(void) cl_assert(merge_test_index(repo_index, merge_index_entries, 3)); cl_git_pass(git_futils_readbuffer(&diff_buf, "revert/file.txt")); - cl_assert(strcmp(diff_buf.ptr, "a\n" \ - "<<<<<<< HEAD\n" \ - "=======\n" \ - "a\n" \ - ">>>>>>> parent of 97e52d5... Revert me\n" \ - "a\n" \ - "a\n" \ - "a\n" \ - "a\n" \ - "ab\n") == 0); + cl_assert_equal_s( + "a\n" \ + "<<<<<<< HEAD\n" \ + "=======\n" \ + "a\n" \ + ">>>>>>> parent of 97e52d5... Revert me\n" \ + "a\n" \ + "a\n" \ + "a\n" \ + "a\n" \ + "ab", + diff_buf.ptr); git_commit_free(revert_commit); git_commit_free(head_commit); From 234ca40a89fe1b2181ef56300f1f1261b5cb2836 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 7 Jul 2015 16:46:48 -0500 Subject: [PATCH 13/73] xdiff: upgrade to core git 2.4.5 Upgrade xdiff to version used in core git 2.4.5 (0df0541). Corrects an issue where an LF is added at EOF while applying an unrelated change (ba31180), cleans up some unused code (be89977 and e5b0662), and provides an improved callback to avoid leaking internal (to xdiff) structures (467d348). This also adds some additional functionality that we do not yet take advantage of, namely the ability to ignore changes whose lines are all blank (36617af). --- src/blame_git.c | 25 +++++++++------------ src/xdiff/xdiff.h | 16 ++++++++------ src/xdiff/xdiffi.c | 48 +++++++++++++++++++++++++++++++++++++---- src/xdiff/xdiffi.h | 1 + src/xdiff/xemit.c | 49 ++++++++++++++++++++++++++++++++++++------ src/xdiff/xemit.h | 2 +- src/xdiff/xhistogram.c | 2 +- src/xdiff/xmerge.c | 4 ++-- src/xdiff/xpatience.c | 2 +- src/xdiff/xprepare.c | 21 +++++++++--------- src/xdiff/xutils.c | 42 +++++++++++------------------------- src/xdiff/xutils.h | 1 + 12 files changed, 137 insertions(+), 76 deletions(-) diff --git a/src/blame_git.c b/src/blame_git.c index e863efe2e08..f481426aa85 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -304,21 +304,16 @@ static void blame_chunk( } static int my_emit( - xdfenv_t *xe, - xdchange_t *xscr, - xdemitcb_t *ecb, - xdemitconf_t const *xecfg) + long start_a, long count_a, + long start_b, long count_b, + void *cb_data) { - xdchange_t *xch = xscr; - GIT_UNUSED(xe); - GIT_UNUSED(xecfg); - while (xch) { - blame_chunk_cb_data *d = ecb->priv; - blame_chunk(d->blame, d->tlno, d->plno, xch->i2, d->target, d->parent); - d->plno = xch->i1 + xch->chg1; - d->tlno = xch->i2 + xch->chg2; - xch = xch->next; - } + blame_chunk_cb_data *d = (blame_chunk_cb_data *)cb_data; + + blame_chunk(d->blame, d->tlno, d->plno, start_b, d->target, d->parent); + d->plno = start_a + count_a; + d->tlno = start_b + count_b; + return 0; } @@ -352,7 +347,7 @@ static int diff_hunks(mmfile_t file_a, mmfile_t file_b, void *cb_data) xdemitconf_t xecfg = {0}; xdemitcb_t ecb = {0}; - xecfg.emit_func = (void(*)(void))my_emit; + xecfg.hunk_func = my_emit; ecb.priv = cb_data; trim_common_tail(&file_a, &file_b, 0); diff --git a/src/xdiff/xdiff.h b/src/xdiff/xdiff.h index cb8b235b50f..e2f1e892b19 100644 --- a/src/xdiff/xdiff.h +++ b/src/xdiff/xdiff.h @@ -32,14 +32,14 @@ extern "C" { #define XDF_IGNORE_WHITESPACE (1 << 2) #define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3) #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4) +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL) + #define XDF_PATIENCE_DIFF (1 << 5) #define XDF_HISTOGRAM_DIFF (1 << 6) -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL) +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF) +#define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK) -#define XDL_PATCH_NORMAL '-' -#define XDL_PATCH_REVERSE '+' -#define XDL_PATCH_MODEMASK ((1 << 8) - 1) -#define XDL_PATCH_IGNOREBSPACE (1 << 8) +#define XDF_IGNORE_BLANK_LINES (1 << 7) #define XDL_EMIT_FUNCNAMES (1 << 0) #define XDL_EMIT_COMMON (1 << 1) @@ -88,13 +88,17 @@ typedef struct s_xdemitcb { typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv); +typedef int (*xdl_emit_hunk_consume_func_t)(long start_a, long count_a, + long start_b, long count_b, + void *cb_data); + typedef struct s_xdemitconf { long ctxlen; long interhunkctxlen; unsigned long flags; find_func_t find_func; void *find_func_priv; - void (*emit_func)(void); + xdl_emit_hunk_consume_func_t hunk_func; } xdemitconf_t; typedef struct s_bdiffparam { diff --git a/src/xdiff/xdiffi.c b/src/xdiff/xdiffi.c index 84aa0fcfe4b..2358a2d6326 100644 --- a/src/xdiff/xdiffi.c +++ b/src/xdiff/xdiffi.c @@ -328,10 +328,10 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdalgoenv_t xenv; diffdata_t dd1, dd2; - if (xpp->flags & XDF_PATIENCE_DIFF) + if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) return xdl_do_patience_diff(mf1, mf2, xpp, xe); - if (xpp->flags & XDF_HISTOGRAM_DIFF) + if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) return xdl_do_histogram_diff(mf1, mf2, xpp, xe); if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) { @@ -394,6 +394,7 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1, xch->i2 = i2; xch->chg1 = chg1; xch->chg2 = chg2; + xch->ignore = 0; return xch; } @@ -538,13 +539,49 @@ void xdl_free_script(xdchange_t *xscr) { } } +static int xdl_call_hunk_func(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, + xdemitconf_t const *xecfg) +{ + xdchange_t *xch, *xche; + + for (xch = xscr; xch; xch = xche->next) { + xche = xdl_get_hunk(&xch, xecfg); + if (!xch) + break; + if (xecfg->hunk_func(xch->i1, xche->i1 + xche->chg1 - xch->i1, + xch->i2, xche->i2 + xche->chg2 - xch->i2, + ecb->priv) < 0) + return -1; + } + return 0; +} + +static void xdl_mark_ignorable(xdchange_t *xscr, xdfenv_t *xe, long flags) +{ + xdchange_t *xch; + + for (xch = xscr; xch; xch = xch->next) { + int ignore = 1; + xrecord_t **rec; + long i; + + rec = &xe->xdf1.recs[xch->i1]; + for (i = 0; i < xch->chg1 && ignore; i++) + ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags); + + rec = &xe->xdf2.recs[xch->i2]; + for (i = 0; i < xch->chg2 && ignore; i++) + ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags); + + xch->ignore = ignore; + } +} int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb) { xdchange_t *xscr; xdfenv_t xe; - emit_func_t ef = xecfg->emit_func ? - (emit_func_t)xecfg->emit_func : xdl_emit_diff; + emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff; if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) { @@ -558,6 +595,9 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; } if (xscr) { + if (xpp->flags & XDF_IGNORE_BLANK_LINES) + xdl_mark_ignorable(xscr, &xe, xpp->flags); + if (ef(&xe, xscr, ecb, xecfg) < 0) { xdl_free_script(xscr); diff --git a/src/xdiff/xdiffi.h b/src/xdiff/xdiffi.h index 7a92ea9c4d8..8b81206c9af 100644 --- a/src/xdiff/xdiffi.h +++ b/src/xdiff/xdiffi.h @@ -41,6 +41,7 @@ typedef struct s_xdchange { struct s_xdchange *next; long i1, i2; long chg1, chg2; + int ignore; } xdchange_t; diff --git a/src/xdiff/xemit.c b/src/xdiff/xemit.c index e3e63d90234..750a9729484 100644 --- a/src/xdiff/xemit.c +++ b/src/xdiff/xemit.c @@ -56,16 +56,51 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t * /* * Starting at the passed change atom, find the latest change atom to be included * inside the differential hunk according to the specified configuration. + * Also advance xscr if the first changes must be discarded. */ -xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) { - xdchange_t *xch, *xchp; +xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg) +{ + xdchange_t *xch, *xchp, *lxch; long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen; + long max_ignorable = xecfg->ctxlen; + unsigned long ignored = 0; /* number of ignored blank lines */ + + /* remove ignorable changes that are too far before other changes */ + for (xchp = *xscr; xchp && xchp->ignore; xchp = xchp->next) { + xch = xchp->next; + + if (xch == NULL || + xch->i1 - (xchp->i1 + xchp->chg1) >= max_ignorable) + *xscr = xch; + } + + if (*xscr == NULL) + return NULL; + + lxch = *xscr; - for (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next) - if (xch->i1 - (xchp->i1 + xchp->chg1) > max_common) + for (xchp = *xscr, xch = xchp->next; xch; xchp = xch, xch = xch->next) { + long distance = xch->i1 - (xchp->i1 + xchp->chg1); + if (distance > max_common) break; - return xchp; + if (distance < max_ignorable && (!xch->ignore || lxch == xchp)) { + lxch = xch; + ignored = 0; + } else if (distance < max_ignorable && xch->ignore) { + ignored += xch->chg2; + } else if (lxch != xchp && + xch->i1 + ignored - (lxch->i1 + lxch->chg1) > max_common) { + break; + } else if (!xch->ignore) { + lxch = xch; + ignored = 0; + } else { + ignored += xch->chg2; + } + } + + return lxch; } @@ -144,7 +179,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, return xdl_emit_common(xe, xscr, ecb, xecfg); for (xch = xscr; xch; xch = xche->next) { - xche = xdl_get_hunk(xch, xecfg); + xche = xdl_get_hunk(&xch, xecfg); + if (!xch) + break; s1 = XDL_MAX(xch->i1 - xecfg->ctxlen, 0); s2 = XDL_MAX(xch->i2 - xecfg->ctxlen, 0); diff --git a/src/xdiff/xemit.h b/src/xdiff/xemit.h index c2e2e830273..d29710770ce 100644 --- a/src/xdiff/xemit.h +++ b/src/xdiff/xemit.h @@ -27,7 +27,7 @@ typedef int (*emit_func_t)(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, xdemitconf_t const *xecfg); -xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg); +xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg); int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, xdemitconf_t const *xecfg); diff --git a/src/xdiff/xhistogram.c b/src/xdiff/xhistogram.c index c84812893a4..500d8112a9d 100644 --- a/src/xdiff/xhistogram.c +++ b/src/xdiff/xhistogram.c @@ -258,7 +258,7 @@ static int fall_back_to_classic_diff(struct histindex *index, int line1, int count1, int line2, int count2) { xpparam_t xpp; - xpp.flags = index->xpp->flags & ~XDF_HISTOGRAM_DIFF; + xpp.flags = index->xpp->flags & ~XDF_DIFF_ALGORITHM_MASK; return xdl_fall_back_diff(index->env, &xpp, line1, count1, line2, count2); diff --git a/src/xdiff/xmerge.c b/src/xdiff/xmerge.c index 84e42467228..b11e5981792 100644 --- a/src/xdiff/xmerge.c +++ b/src/xdiff/xmerge.c @@ -245,11 +245,11 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1, dest ? dest + size : NULL); /* Postimage from side #1 */ if (m->mode & 1) - size += xdl_recs_copy(xe1, m->i1, m->chg1, 1, + size += xdl_recs_copy(xe1, m->i1, m->chg1, (m->mode & 2), dest ? dest + size : NULL); /* Postimage from side #2 */ if (m->mode & 2) - size += xdl_recs_copy(xe2, m->i2, m->chg2, 1, + size += xdl_recs_copy(xe2, m->i2, m->chg2, 0, dest ? dest + size : NULL); } else continue; diff --git a/src/xdiff/xpatience.c b/src/xdiff/xpatience.c index fdd7d0263f5..04e1a1ab2a8 100644 --- a/src/xdiff/xpatience.c +++ b/src/xdiff/xpatience.c @@ -288,7 +288,7 @@ static int fall_back_to_classic_diff(struct hashmap *map, int line1, int count1, int line2, int count2) { xpparam_t xpp; - xpp.flags = map->xpp->flags & ~XDF_PATIENCE_DIFF; + xpp.flags = map->xpp->flags & ~XDF_DIFF_ALGORITHM_MASK; return xdl_fall_back_diff(map->env, &xpp, line1, count1, line2, count2); diff --git a/src/xdiff/xprepare.c b/src/xdiff/xprepare.c index e419f4f7260..63a22c630e5 100644 --- a/src/xdiff/xprepare.c +++ b/src/xdiff/xprepare.c @@ -181,7 +181,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ if (!(recs = (xrecord_t **) xdl_malloc(narec * sizeof(xrecord_t *)))) goto abort; - if (xpp->flags & XDF_HISTOGRAM_DIFF) + if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) hbits = hsize = 0; else { hbits = xdl_hashbits((unsigned int) narec); @@ -209,8 +209,8 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ crec->ha = hav; recs[nrec++] = crec; - if (!(xpp->flags & XDF_HISTOGRAM_DIFF) && - xdl_classify_record(pass, cf, rhash, hbits, crec) < 0) + if ((XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && + xdl_classify_record(pass, cf, rhash, hbits, crec) < 0) goto abort; } } @@ -273,16 +273,15 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, * (nrecs) will be updated correctly anyway by * xdl_prepare_ctx(). */ - sample = xpp->flags & XDF_HISTOGRAM_DIFF ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1; + sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF + ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1); enl1 = xdl_guess_lines(mf1, sample) + 1; enl2 = xdl_guess_lines(mf2, sample) + 1; - if (!(xpp->flags & XDF_HISTOGRAM_DIFF) && - xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) { - + if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF && + xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) return -1; - } if (xdl_prepare_ctx(1, mf1, enl1, xpp, &cf, &xe->xdf1) < 0) { @@ -296,9 +295,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; } - if (!(xpp->flags & XDF_PATIENCE_DIFF) && - !(xpp->flags & XDF_HISTOGRAM_DIFF) && - xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { + if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && + (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && + xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { xdl_free_ctx(&xe->xdf2); xdl_free_ctx(&xe->xdf1); diff --git a/src/xdiff/xutils.c b/src/xdiff/xutils.c index bb7bdee4925..30f2a30ac71 100644 --- a/src/xdiff/xutils.c +++ b/src/xdiff/xutils.c @@ -120,35 +120,6 @@ void *xdl_cha_alloc(chastore_t *cha) { return data; } - -void *xdl_cha_first(chastore_t *cha) { - chanode_t *sncur; - - if (!(cha->sncur = sncur = cha->head)) - return NULL; - - cha->scurr = 0; - - return (char *) sncur + sizeof(chanode_t) + cha->scurr; -} - - -void *xdl_cha_next(chastore_t *cha) { - chanode_t *sncur; - - if (!(sncur = cha->sncur)) - return NULL; - cha->scurr += cha->isize; - if (cha->scurr == sncur->icurr) { - if (!(sncur = cha->sncur = sncur->next)) - return NULL; - cha->scurr = 0; - } - - return (char *) sncur + sizeof(chanode_t) + cha->scurr; -} - - long xdl_guess_lines(mmfile_t *mf, long sample) { long nl = 0, size, tsize = 0; char const *data, *cur, *top; @@ -170,6 +141,19 @@ long xdl_guess_lines(mmfile_t *mf, long sample) { return nl + 1; } +int xdl_blankline(const char *line, long size, long flags) +{ + long i; + + if (!(flags & XDF_WHITESPACE_FLAGS)) + return (size <= 1); + + for (i = 0; i < size && XDL_ISSPACE(line[i]); i++) + ; + + return (i == size); +} + int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags) { int i1, i2; diff --git a/src/xdiff/xutils.h b/src/xdiff/xutils.h index 714719a89cb..8f952a8e627 100644 --- a/src/xdiff/xutils.h +++ b/src/xdiff/xutils.h @@ -34,6 +34,7 @@ void *xdl_cha_alloc(chastore_t *cha); void *xdl_cha_first(chastore_t *cha); void *xdl_cha_next(chastore_t *cha); long xdl_guess_lines(mmfile_t *mf, long sample); +int xdl_blankline(const char *line, long size, long flags); int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags); unsigned long xdl_hash_record(char const **data, char const *top, long flags); unsigned int xdl_hashbits(unsigned int size); From 79698030b090b017aea6d33fb3f02d3fcb046738 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 29 Jun 2015 22:51:18 +0000 Subject: [PATCH 14/73] git_cert: child types use proper base type --- CHANGELOG.md | 4 ++++ include/git2/transport.h | 35 ++++++++++++++--------------------- src/curl_stream.c | 14 +++++++------- src/openssl_stream.c | 5 +++-- src/stransport_stream.c | 2 +- src/transports/ssh.c | 4 ++-- src/transports/winhttp.c | 2 +- 7 files changed, 32 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aae4da6a18..fa659919053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ v0.23 + 1 ### API removals +### Breaking API changes + +* `git_cert` descendent types now have a proper `parent` member + v0.23 ------ diff --git a/include/git2/transport.h b/include/git2/transport.h index 2eeebd56589..fd55eac0ab0 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -37,39 +37,32 @@ typedef enum { * Hostkey information taken from libssh2 */ typedef struct { + git_cert parent; + /** - * Type of certificate. Here to share the header with - * `git_cert`. + * A hostkey type from libssh2, either + * `GIT_CERT_SSH_MD5` or `GIT_CERT_SSH_SHA1` */ - git_cert_t cert_type; - /** - * A hostkey type from libssh2, either - * `GIT_CERT_SSH_MD5` or `GIT_CERT_SSH_SHA1` - */ git_cert_ssh_t type; - /** - * Hostkey hash. If type has `GIT_CERT_SSH_MD5` set, this will - * have the MD5 hash of the hostkey. - */ + /** + * Hostkey hash. If type has `GIT_CERT_SSH_MD5` set, this will + * have the MD5 hash of the hostkey. + */ unsigned char hash_md5[16]; - /** - * Hostkey hash. If type has `GIT_CERT_SSH_SHA1` set, this will - * have the SHA-1 hash of the hostkey. - */ - unsigned char hash_sha1[20]; + /** + * Hostkey hash. If type has `GIT_CERT_SSH_SHA1` set, this will + * have the SHA-1 hash of the hostkey. + */ + unsigned char hash_sha1[20]; } git_cert_hostkey; /** * X.509 certificate information */ typedef struct { - /** - * Type of certificate. Here to share the header with - * `git_cert`. - */ - git_cert_t cert_type; + git_cert parent; /** * Pointer to the X.509 certificate data */ diff --git a/src/curl_stream.c b/src/curl_stream.c index 6534bdbbe78..63421fcf7a8 100644 --- a/src/curl_stream.c +++ b/src/curl_stream.c @@ -67,9 +67,9 @@ static int curls_certificate(git_cert **out, git_stream *stream) /* No information is available, can happen with SecureTransport */ if (certinfo->num_of_certs == 0) { - s->cert_info.cert_type = GIT_CERT_NONE; - s->cert_info.data = NULL; - s->cert_info.len = 0; + s->cert_info.parent.cert_type = GIT_CERT_NONE; + s->cert_info.data = NULL; + s->cert_info.len = 0; return 0; } @@ -85,11 +85,11 @@ static int curls_certificate(git_cert **out, git_stream *stream) s->cert_info_strings.strings = (char **) strings.contents; s->cert_info_strings.count = strings.length; - s->cert_info.cert_type = GIT_CERT_STRARRAY; - s->cert_info.data = &s->cert_info_strings; - s->cert_info.len = strings.length; + s->cert_info.parent.cert_type = GIT_CERT_STRARRAY; + s->cert_info.data = &s->cert_info_strings; + s->cert_info.len = strings.length; - *out = (git_cert *) &s->cert_info; + *out = &s->cert_info.parent; return 0; } diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 958252e9f66..1bd4f1421c0 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -358,11 +358,12 @@ int openssl_certificate(git_cert **out, git_stream *stream) return -1; } - st->cert_info.cert_type = GIT_CERT_X509; + st->cert_info.parent.cert_type = GIT_CERT_X509; st->cert_info.data = encoded_cert; st->cert_info.len = len; - *out = (git_cert *)&st->cert_info; + *out = &st->cert_info.parent; + return 0; } diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 10e19166cd3..33b6c5c384c 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -108,7 +108,7 @@ int stransport_certificate(git_cert **out, git_stream *stream) return -1; } - st->cert_info.cert_type = GIT_CERT_X509; + st->cert_info.parent.cert_type = GIT_CERT_X509; st->cert_info.data = (void *) CFDataGetBytePtr(st->der_data); st->cert_info.len = CFDataGetLength(st->der_data); diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 83af137f8b9..0b0d4bac360 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -525,10 +525,10 @@ static int _git_ssh_setup_conn( goto done; if (t->owner->certificate_check_cb != NULL) { - git_cert_hostkey cert = { 0 }, *cert_ptr; + git_cert_hostkey cert = {{ 0 }}, *cert_ptr; const char *key; - cert.cert_type = GIT_CERT_HOSTKEY_LIBSSH2; + cert.parent.cert_type = GIT_CERT_HOSTKEY_LIBSSH2; key = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_SHA1); if (key != NULL) { diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index da047d690c7..0c43c4b0b9a 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -228,7 +228,7 @@ static int certificate_check(winhttp_stream *s, int valid) } giterr_clear(); - cert.cert_type = GIT_CERT_X509; + cert.parent.cert_type = GIT_CERT_X509; cert.data = cert_ctx->pbCertEncoded; cert.len = cert_ctx->cbCertEncoded; error = t->owner->certificate_check_cb((git_cert *) &cert, valid, t->connection_data.host, t->owner->cred_acquire_payload); From a3c00cd8e36b303f0753412d12203d68af8c07e4 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 10 Jul 2015 09:21:59 -0500 Subject: [PATCH 15/73] xdiff: cleanup some warnings --- src/xdiff/xdiffi.c | 2 ++ src/xdiff/xemit.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/xdiff/xdiffi.c b/src/xdiff/xdiffi.c index 2358a2d6326..0620e5fff4c 100644 --- a/src/xdiff/xdiffi.c +++ b/src/xdiff/xdiffi.c @@ -544,6 +544,8 @@ static int xdl_call_hunk_func(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, { xdchange_t *xch, *xche; + (void)xe; + for (xch = xscr; xch; xch = xche->next) { xche = xdl_get_hunk(&xch, xecfg); if (!xch) diff --git a/src/xdiff/xemit.c b/src/xdiff/xemit.c index 750a9729484..600fd1fddb8 100644 --- a/src/xdiff/xemit.c +++ b/src/xdiff/xemit.c @@ -90,7 +90,7 @@ xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg) } else if (distance < max_ignorable && xch->ignore) { ignored += xch->chg2; } else if (lxch != xchp && - xch->i1 + ignored - (lxch->i1 + lxch->chg1) > max_common) { + xch->i1 + ignored - (lxch->i1 + lxch->chg1) > (unsigned long)max_common) { break; } else if (!xch->ignore) { lxch = xch; From bae467aec4de8bb7d0c10a3a11e585ab6d05dd8f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 10 Jul 2015 09:25:20 -0500 Subject: [PATCH 16/73] wildcard filters: clean up some warnings in tests --- tests/filter/wildcard.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/filter/wildcard.c b/tests/filter/wildcard.c index ba826b5dc95..8031ad3d380 100644 --- a/tests/filter/wildcard.c +++ b/tests/filter/wildcard.c @@ -9,7 +9,7 @@ static git_repository *g_repo = NULL; -static git_filter *create_wildcard_filter(); +static git_filter *create_wildcard_filter(void); #define DATA_LEN 32 @@ -63,6 +63,9 @@ static int wildcard_filter_check( const git_filter_source *src, const char **attr_values) { + GIT_UNUSED(self); + GIT_UNUSED(src); + if (strcmp(attr_values[0], "wcflip") == 0 || strcmp(attr_values[0], "wcreverse") == 0) { *payload = git__strdup(attr_values[0]); @@ -91,8 +94,9 @@ static int wildcard_filter_apply( return GIT_PASSTHROUGH; } -static int wildcard_filter_cleanup(git_filter *self, void *payload) +static void wildcard_filter_cleanup(git_filter *self, void *payload) { + GIT_UNUSED(self); git__free(payload); return 0; } @@ -125,7 +129,7 @@ void test_filter_wildcard__reverse(void) cl_git_pass(git_filter_list_load( &fl, g_repo, NULL, "hero-reverse-foo", GIT_FILTER_TO_ODB, 0)); - cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_buf_put(&in, (char *)input, DATA_LEN)); cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); cl_assert_equal_i(DATA_LEN, out.size); @@ -146,7 +150,7 @@ void test_filter_wildcard__flip(void) cl_git_pass(git_filter_list_load( &fl, g_repo, NULL, "hero-flip-foo", GIT_FILTER_TO_ODB, 0)); - cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_buf_put(&in, (char *)input, DATA_LEN)); cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); cl_assert_equal_i(DATA_LEN, out.size); @@ -167,7 +171,7 @@ void test_filter_wildcard__none(void) cl_git_pass(git_filter_list_load( &fl, g_repo, NULL, "none-foo", GIT_FILTER_TO_ODB, 0)); - cl_git_pass(git_buf_put(&in, input, DATA_LEN)); + cl_git_pass(git_buf_put(&in, (char *)input, DATA_LEN)); cl_git_pass(git_filter_list_apply_to_data(&out, fl, &in)); cl_assert_equal_i(DATA_LEN, out.size); From 9a99ca7b2108e3c3ca483d695cf0f46de386630a Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 10 Jul 2015 09:25:45 -0500 Subject: [PATCH 17/73] wildcard filters: move CHANGELOG message to 0.23+1 --- CHANGELOG.md | 8 ++++---- tests/filter/wildcard.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aae4da6a18..5e15a21216b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ v0.23 + 1 ### Changes or improvements +* Custom filters can now be registered with wildcard attributes, for + example `filter=*`. Consumers should examine the attributes parameter + of the `check` function for details. + ### API additions ### API removals @@ -88,10 +92,6 @@ v0.23 * If libcurl is installed, we will use it to connect to HTTP(S) servers. -* Custom filters can now be registered with wildcard attributes, for - example `filter=*`. Consumers should examine the attributes parameter - of the `check` function for details. - ### API additions * The `git_merge_options` gained a `file_flags` member. diff --git a/tests/filter/wildcard.c b/tests/filter/wildcard.c index 8031ad3d380..999b33653ac 100644 --- a/tests/filter/wildcard.c +++ b/tests/filter/wildcard.c @@ -98,7 +98,6 @@ static void wildcard_filter_cleanup(git_filter *self, void *payload) { GIT_UNUSED(self); git__free(payload); - return 0; } static void wildcard_filter_free(git_filter *f) @@ -106,7 +105,7 @@ static void wildcard_filter_free(git_filter *f) git__free(f); } -static git_filter *create_wildcard_filter() +static git_filter *create_wildcard_filter(void) { git_filter *filter = git__calloc(1, sizeof(git_filter)); cl_assert(filter); From 9c0331026b71ed801baa969c308598d59c6891c7 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 30 Jun 2015 13:41:01 +0000 Subject: [PATCH 18/73] khash: add eol so picky compilers stop warning --- src/khash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/khash.h b/src/khash.h index 818ac833bf1..71eb583d589 100644 --- a/src/khash.h +++ b/src/khash.h @@ -619,4 +619,4 @@ typedef const char *kh_cstr_t; #define KHASH_MAP_INIT_STR(name, khval_t) \ KHASH_INIT(name, kh_cstr_t, khval_t, 1, kh_str_hash_func, kh_str_hash_equal) -#endif /* __AC_KHASH_H */ \ No newline at end of file +#endif /* __AC_KHASH_H */ From 37c84dc58f43c609ddadff3f9ff25c1f5b52a746 Mon Sep 17 00:00:00 2001 From: Tony Kelman Date: Sun, 5 Jul 2015 10:07:48 -0400 Subject: [PATCH 19/73] Increase required version of cmake to 2.8 --- CMakeLists.txt | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c55ddd1a2b..d3fe48f89fa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,7 +12,7 @@ # > cmake --build . --target install PROJECT(libgit2 C) -CMAKE_MINIMUM_REQUIRED(VERSION 2.6) +CMAKE_MINIMUM_REQUIRED(VERSION 2.8) CMAKE_POLICY(SET CMP0015 NEW) # Add find modules to the path diff --git a/README.md b/README.md index 4ee1fdc2ad1..3191aeee2c6 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ Under Unix-like systems, like Linux, \*BSD and Mac OS X, libgit2 expects `pthrea they should be installed by default on all systems. Under Windows, libgit2 uses the native Windows API for threading. -The `libgit2` library is built using [CMake]() (version 2.6 or newer) on all platforms. +The `libgit2` library is built using [CMake]() (version 2.8 or newer) on all platforms. On most systems you can build the library using the following commands From a34c4f8dcee7e985444be9f5e02b627a55c16bbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 11 Jul 2015 13:32:57 +0200 Subject: [PATCH 20/73] submdule: reproduce double-reporting of a submodule in foreach When we rename a submodule, we should be merging two sets of information based on whether their path is the same. We currently only deduplicate on equal name, which causes us to double-report. --- tests/submodule/lookup.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c index 4d40e227908..9b2b3aa1964 100644 --- a/tests/submodule/lookup.c +++ b/tests/submodule/lookup.c @@ -1,6 +1,7 @@ #include "clar_libgit2.h" #include "submodule_helpers.h" #include "git2/sys/repository.h" +#include "repository.h" #include "fileops.h" static git_repository *g_repo = NULL; @@ -103,8 +104,25 @@ static int sm_lookup_cb(git_submodule *sm, const char *name, void *payload) void test_submodule_lookup__foreach(void) { + git_config *cfg; sm_lookup_data data; + + memset(&data, 0, sizeof(data)); + cl_git_pass(git_submodule_foreach(g_repo, sm_lookup_cb, &data)); + cl_assert_equal_i(8, data.count); + memset(&data, 0, sizeof(data)); + + /* Change the path for a submodule so it doesn't match the name */ + cl_git_pass(git_config_open_ondisk(&cfg, "submod2/.gitmodules")); + + cl_git_pass(git_config_set_string(cfg, "submodule.smchangedindex.path", "sm_changed_index")); + cl_git_pass(git_config_set_string(cfg, "submodule.smchangedindex.url", "../submod2_target")); + cl_git_pass(git_config_delete_entry(cfg, "submodule.sm_changed_index.path")); + cl_git_pass(git_config_delete_entry(cfg, "submodule.sm_changed_index.url")); + + git_config_free(cfg); + cl_git_pass(git_submodule_foreach(g_repo, sm_lookup_cb, &data)); cl_assert_equal_i(8, data.count); } From 08c2d3e97c0ee45222b93c214d0b752d0c52ee2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 11 Jul 2015 18:31:28 +0200 Subject: [PATCH 21/73] submodule: lookup the submodule by path if available If we get the path from the gitmodules file, look up the submodule we're interested in by path, rather then by name. Otherwise we might get duplicate results. --- src/submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/submodule.c b/src/submodule.c index fb3d4bf1e52..892c983041a 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1647,7 +1647,7 @@ static int submodule_load_from_config( } else { khiter_t pos; git_strmap *map = data->map; - pos = git_strmap_lookup_index(map, name.ptr); + pos = git_strmap_lookup_index(map, path ? path : name.ptr); if (git_strmap_valid_index(map, pos)) { sm = git_strmap_value_at(map, pos); } else { From 8a52ed7a482935c74dbb24358e21811dfa6d91c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 11 Jul 2015 18:51:36 +0200 Subject: [PATCH 22/73] errors: add EDIRECTORY This is to be returned when the operation which the user asked for is not possible to do on a directory. --- include/git2/errors.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/git2/errors.h b/include/git2/errors.h index a81aa05d9ce..e189e55f17e 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -48,6 +48,7 @@ typedef enum { GIT_EEOF = -20, /**< Unexpected EOF */ GIT_EINVALID = -21, /**< Invalid operation or input */ GIT_EUNCOMMITTED = -22, /**< Uncommitted changes in index prevented operation */ + GIT_EDIRECTORY = -23, /**< The operation is not valid for a directory */ GIT_PASSTHROUGH = -30, /**< Internal only */ GIT_ITEROVER = -31, /**< Signals end of iteration with iterator */ From 0d98af0911ebf55ba64cc0c9e17a3d450c77a9be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 11 Jul 2015 19:03:38 +0200 Subject: [PATCH 23/73] blob: fail to create a blob from a dir with EDIRECTORY This also affects `git_index_add_bypath()` by providing a better error message and a specific error code when a directory is passed. --- src/blob.c | 6 ++++++ tests/index/bypath.c | 23 +++++++++++++++++++++++ tests/submodule/add.c | 1 + 3 files changed, 30 insertions(+) create mode 100644 tests/index/bypath.c diff --git a/src/blob.c b/src/blob.c index 07c4d92c84b..ad0f4ac62aa 100644 --- a/src/blob.c +++ b/src/blob.c @@ -185,6 +185,12 @@ int git_blob__create_from_paths( (error = git_repository_odb(&odb, repo)) < 0) goto done; + if (S_ISDIR(st.st_mode)) { + giterr_set(GITERR_ODB, "cannot create blob from '%s'; it is a directory", content_path); + error = GIT_EDIRECTORY; + goto done; + } + if (out_st) memcpy(out_st, &st, sizeof(st)); diff --git a/tests/index/bypath.c b/tests/index/bypath.c new file mode 100644 index 00000000000..08c9934ea0d --- /dev/null +++ b/tests/index/bypath.c @@ -0,0 +1,23 @@ +#include "clar_libgit2.h" +#include "repository.h" +#include "../submodule/submodule_helpers.h" + +static git_repository *g_repo; +static git_index *g_idx; + +void test_index_bypath__initialize(void) +{ + g_repo = setup_fixture_submod2(); + cl_git_pass(git_repository_index__weakptr(&g_idx, g_repo)); +} + +void test_index_bypath__cleanup(void) +{ + g_repo = NULL; + g_idx = NULL; +} + +void test_index_bypath__add_directory(void) +{ + cl_git_fail_with(GIT_EDIRECTORY, git_index_add_bypath(g_idx, "just_a_dir")); +} diff --git a/tests/submodule/add.c b/tests/submodule/add.c index 01625d3aa50..c3b3e63648d 100644 --- a/tests/submodule/add.c +++ b/tests/submodule/add.c @@ -4,6 +4,7 @@ #include "submodule_helpers.h" #include "config/config_helpers.h" #include "fileops.h" +#include "repository.h" static git_repository *g_repo = NULL; From 247d27c2c6868e808191e6090056ecece6da30c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 11 Jul 2015 19:41:03 +0200 Subject: [PATCH 24/73] index: allow add_bypath to update submodules Similarly to how git itself does it, allow the index update operation to stage a change in a submodule's HEAD. --- src/index.c | 24 ++++++++++++++++++++++-- tests/index/bypath.c | 12 ++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/index.c b/src/index.c index 5ce5522f8aa..501498e9e07 100644 --- a/src/index.c +++ b/src/index.c @@ -1236,9 +1236,29 @@ int git_index_add_bypath(git_index *index, const char *path) assert(index && path); - if ((ret = index_entry_init(&entry, index, path)) < 0 || - (ret = index_insert(index, &entry, 1, false)) < 0) + if ((ret = index_entry_init(&entry, index, path)) == 0) + ret = index_insert(index, &entry, 1, false); + + /* If we were given a directory, let's see if it's a submodule */ + if (ret < 0 && ret != GIT_EDIRECTORY) + return ret; + + if (ret == GIT_EDIRECTORY) { + git_submodule *sm; + git_error_state err; + + giterr_capture(&err, ret); + + ret = git_submodule_lookup(&sm, INDEX_OWNER(index), path); + if (ret == GIT_ENOTFOUND) + return giterr_restore(&err); + else + git__free(err.error_msg.message); + + ret = git_submodule_add_to_index(sm, false); + git_submodule_free(sm); return ret; + } /* Adding implies conflict was resolved, move conflict entries to REUC */ if ((ret = index_conflict_to_reuc(index, path)) < 0 && ret != GIT_ENOTFOUND) diff --git a/tests/index/bypath.c b/tests/index/bypath.c index 08c9934ea0d..ddb766a22d1 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -21,3 +21,15 @@ void test_index_bypath__add_directory(void) { cl_git_fail_with(GIT_EDIRECTORY, git_index_add_bypath(g_idx, "just_a_dir")); } + +void test_index_bypath__add_submodule(void) +{ + unsigned int status; + const char *sm_name = "sm_changed_head"; + + cl_git_pass(git_submodule_status(&status, g_repo, sm_name, 0)); + cl_assert_equal_i(GIT_SUBMODULE_STATUS_WD_MODIFIED, status & GIT_SUBMODULE_STATUS_WD_MODIFIED); + cl_git_pass(git_index_add_bypath(g_idx, sm_name)); + cl_git_pass(git_submodule_status(&status, g_repo, sm_name, 0)); + cl_assert_equal_i(0, status & GIT_SUBMODULE_STATUS_WD_MODIFIED); +} From 4de7f3bfc3ad9b2b22620cbe012a70c1c11b3b17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 12 Jul 2015 13:28:03 +0200 Subject: [PATCH 25/73] filter: make sure to close the stream even on error When the stream list init or write fail, we must also make sure to close the stream, as that's the function contract. --- src/filter.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/filter.c b/src/filter.c index 70c4fa3824b..60473e4e12c 100644 --- a/src/filter.c +++ b/src/filter.c @@ -950,18 +950,20 @@ int git_filter_list_stream_data( { git_vector filter_streams = GIT_VECTOR_INIT; git_writestream *stream_start; - int error = 0; + int error = 0, close_error; git_buf_sanitize(data); - if ((error = stream_list_init( - &stream_start, &filter_streams, filters, target)) == 0 && - (error = - stream_start->write(stream_start, data->ptr, data->size)) == 0) - error = stream_start->close(stream_start); + if ((error = stream_list_init(&stream_start, &filter_streams, filters, target)) < 0) + goto out; + error = stream_start->write(stream_start, data->ptr, data->size); + +out: + close_error = stream_start->close(stream_start); stream_list_free(&filter_streams); - return error; + /* propagate the stream init or write error */ + return error < 0 ? error : close_error; } int git_filter_list_stream_blob( From 01d0c02dbaa8856c4e2481ab1435bdf7df58690a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 12 Jul 2015 19:08:06 +0200 Subject: [PATCH 26/73] refdb: delete a ref's reflog upon deletion Removing a reflog upon ref deletion is something which only some backends might wish to do. Backends which are database-backed may wish to archive a reflog, log-based ones may not need to do anything. --- CHANGELOG.md | 4 ++++ include/git2/sys/refdb_backend.h | 5 +++-- src/branch.c | 13 +------------ src/refdb_fs.c | 7 +++++++ tests/refs/branches/delete.c | 2 ++ 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0a819ed796..243b696d723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ v0.23 + 1 * `git_cert` descendent types now have a proper `parent` member +* It is the responsibility fo the refdb backend to decide what to do + with the reflog on ref deletion. The file-based backend must delete + it, a database-backed one may wish to archive it. + v0.23 ------ diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index d943e550f46..9f2a99b7ebb 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -103,8 +103,9 @@ struct git_refdb_backend { const git_signature *who, const char *message); /** - * Deletes the given reference from the refdb. A refdb implementation - * must provide this function. + * Deletes the given reference (and if necessary its reflog) + * from the refdb. A refdb implementation must provide this + * function. */ int (*del)(git_refdb_backend *backend, const char *ref_name, const git_oid *old_id, const char *old_target); diff --git a/src/branch.c b/src/branch.c index 791d551067e..0dcc14c297e 100644 --- a/src/branch.c +++ b/src/branch.c @@ -155,18 +155,7 @@ int git_branch_delete(git_reference *branch) git_reference_owner(branch), git_buf_cstr(&config_section), NULL) < 0) goto on_error; - if (git_reference_delete(branch) < 0) - goto on_error; - - if ((error = git_reflog_delete(git_reference_owner(branch), git_reference_name(branch))) < 0) { - if (error == GIT_ENOTFOUND) { - giterr_clear(); - error = 0; - } - goto on_error; - } - - error = 0; + error = git_reference_delete(branch); on_error: git_buf_free(&config_section); diff --git a/src/refdb_fs.c b/src/refdb_fs.c index e1a77f3ff8d..55d535eb4b3 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -63,6 +63,8 @@ typedef struct refdb_fs_backend { uint32_t direach_flags; } refdb_fs_backend; +static int refdb_reflog_fs__delete(git_refdb_backend *_backend, const char *name); + static int packref_cmp(const void *a_, const void *b_) { const struct packref *a = a_, *b = b_; @@ -1217,6 +1219,11 @@ static int refdb_fs_backend__delete( if ((error = loose_lock(&file, backend, ref_name)) < 0) return error; + if ((error = refdb_reflog_fs__delete(_backend, ref_name)) < 0) { + git_filebuf_cleanup(&file); + return error; + } + return refdb_fs_backend__delete_tail(_backend, &file, ref_name, old_id, old_target); } diff --git a/tests/refs/branches/delete.c b/tests/refs/branches/delete.c index 343ff0f5084..8807db23197 100644 --- a/tests/refs/branches/delete.c +++ b/tests/refs/branches/delete.c @@ -132,6 +132,8 @@ void test_refs_branches_delete__removes_reflog(void) cl_git_pass(git_branch_delete(branch)); git_reference_free(branch); + cl_assert_equal_i(false, git_reference_has_log(repo, "refs/heads/track-local")); + /* Reading a nonexistant reflog creates it, but it should be empty */ cl_git_pass(git_reflog_read(&log, repo, "refs/heads/track-local")); cl_assert_equal_i(0, git_reflog_entrycount(log)); From 6c7e86e158f1294a4f98c470aa42118fc85924fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 12 Jul 2015 19:41:01 +0200 Subject: [PATCH 27/73] examples: modernise the fetch example Under normal conditions, git_remote_fetch() should be the only function used to perform a fetch. Don't let the example lead people astray. --- examples/network/fetch.c | 101 +++++++++++---------------------------- 1 file changed, 27 insertions(+), 74 deletions(-) diff --git a/examples/network/fetch.c b/examples/network/fetch.c index 67444cb4aae..b536959fab1 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -23,32 +23,6 @@ static int progress_cb(const char *str, int len, void *data) return 0; } -static void *download(void *ptr) -{ - struct dl_data *data = (struct dl_data *)ptr; - - // Connect to the remote end specifying that we want to fetch - // information from it. - if (git_remote_connect(data->remote, GIT_DIRECTION_FETCH, &data->fetch_opts->callbacks) < 0) { - data->ret = -1; - goto exit; - } - - // Download the packfile and index it. This function updates the - // amount of received data and the indexer stats which lets you - // inform the user about progress. - if (git_remote_download(data->remote, NULL, data->fetch_opts) < 0) { - data->ret = -1; - goto exit; - } - - data->ret = 0; - -exit: - data->finished = 1; - return &data->ret; -} - /** * This function gets called for each remote-tracking branch that gets * updated. The message we output depends on whether it's a new one or @@ -73,6 +47,24 @@ static int update_cb(const char *refname, const git_oid *a, const git_oid *b, vo return 0; } +/** + * This gets called during the download and indexing. Here we show + * processed and total objects in the pack and the amount of received + * data. Most frontends will probably want to show a percentage and + * the download rate. + */ +static int transfer_progress_cb(const git_transfer_progress *stats, void *payload) +{ + if (stats->received_objects == stats->total_objects) { + printf("Resolving deltas %d/%d\r", + stats->indexed_deltas, stats->total_deltas); + } else if (stats->total_objects > 0) { + printf("Received %d/%d objects (%d) in %" PRIuZ " bytes\r", + stats->received_objects, stats->total_objects, + stats->indexed_objects, stats->received_bytes); + } +} + /** Entry point for this command */ int fetch(git_repository *repo, int argc, char **argv) { @@ -80,9 +72,6 @@ int fetch(git_repository *repo, int argc, char **argv) const git_transfer_progress *stats; struct dl_data data; git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT; -#ifndef _WIN32 - pthread_t worker; -#endif if (argc < 2) { fprintf(stderr, "usage: %s fetch \n", argv[-1]); @@ -99,49 +88,23 @@ int fetch(git_repository *repo, int argc, char **argv) // Set up the callbacks (only update_tips for now) fetch_opts.callbacks.update_tips = &update_cb; fetch_opts.callbacks.sideband_progress = &progress_cb; + fetch_opts.callbacks.transfer_progress = transfer_progress_cb; fetch_opts.callbacks.credentials = cred_acquire_cb; - // Set up the information for the background worker thread - data.remote = remote; - data.fetch_opts = &fetch_opts; - data.ret = 0; - data.finished = 0; - - stats = git_remote_stats(remote); - -#ifdef _WIN32 - download(&data); -#else - pthread_create(&worker, NULL, download, &data); - - // Loop while the worker thread is still running. Here we show processed - // and total objects in the pack and the amount of received - // data. Most frontends will probably want to show a percentage and - // the download rate. - do { - usleep(10000); - - if (stats->received_objects == stats->total_objects) { - printf("Resolving deltas %d/%d\r", - stats->indexed_deltas, stats->total_deltas); - } else if (stats->total_objects > 0) { - printf("Received %d/%d objects (%d) in %" PRIuZ " bytes\r", - stats->received_objects, stats->total_objects, - stats->indexed_objects, stats->received_bytes); - } - } while (!data.finished); - - if (data.ret < 0) - goto on_error; - - pthread_join(worker, NULL); -#endif + /** + * Perform the fetch with the configured refspecs from the + * config. Update the reflog for the updated references with + * "fetch". + */ + if (git_remote_fetch(remote, NULL, &fetch_opts, "fetch") < 0) + return -1; /** * If there are local objects (we got a thin pack), then tell * the user how many objects we saved from having to cross the * network. */ + stats = git_remote_stats(remote); if (stats->local_objects > 0) { printf("\rReceived %d/%d objects in %zu bytes (used %d local objects)\n", stats->indexed_objects, stats->total_objects, stats->received_bytes, stats->local_objects); @@ -150,16 +113,6 @@ int fetch(git_repository *repo, int argc, char **argv) stats->indexed_objects, stats->total_objects, stats->received_bytes); } - // Disconnect the underlying connection to prevent from idling. - git_remote_disconnect(remote); - - // Update the references in the remote's namespace to point to the - // right commits. This may be needed even if there was no packfile - // to download, which can happen e.g. when the branches have been - // changed but all the needed objects are available locally. - if (git_remote_update_tips(remote, &fetch_opts.callbacks, 1, fetch_opts.download_tags, NULL) < 0) - return -1; - git_remote_free(remote); return 0; From 768f8be31c3fac1b0ed8f4d49cf7176a30586443 Mon Sep 17 00:00:00 2001 From: Matthew Plough Date: Tue, 30 Jun 2015 19:00:41 -0400 Subject: [PATCH 28/73] Fix #3094 - improve use of portable size_t/ssize_t format specifiers. The header src/cc-compat.h defines portable format specifiers PRIuZ, PRIdZ, and PRIxZ. The original report highlighted the need to use these specifiers in examples/network/fetch.c. For this commit, I checked all C source and header files not in deps/ and transitioned to the appropriate format specifier where appropriate. --- examples/network/fetch.c | 4 ++-- src/cache.c | 8 ++++---- src/checkout.c | 4 ++-- src/merge.c | 6 +++--- src/rebase.c | 4 ++-- src/stash.c | 2 +- src/transports/http.c | 2 +- src/transports/smart_pkt.c | 2 +- tests/blame/blame_helpers.c | 2 +- tests/clar_libgit2.c | 4 ++-- tests/merge/merge_helpers.c | 2 +- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/examples/network/fetch.c b/examples/network/fetch.c index 67444cb4aae..6be12406b8e 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -143,10 +143,10 @@ int fetch(git_repository *repo, int argc, char **argv) * network. */ if (stats->local_objects > 0) { - printf("\rReceived %d/%d objects in %zu bytes (used %d local objects)\n", + printf("\rReceived %d/%d objects in %" PRIuZ " bytes (used %d local objects)\n", stats->indexed_objects, stats->total_objects, stats->received_bytes, stats->local_objects); } else{ - printf("\rReceived %d/%d objects in %zu bytes\n", + printf("\rReceived %d/%d objects in %" PRIuZ "bytes\n", stats->indexed_objects, stats->total_objects, stats->received_bytes); } diff --git a/src/cache.c b/src/cache.c index 2f3ad156316..ca5173c0d91 100644 --- a/src/cache.c +++ b/src/cache.c @@ -50,16 +50,16 @@ void git_cache_dump_stats(git_cache *cache) if (kh_size(cache->map) == 0) return; - printf("Cache %p: %d items cached, %d bytes\n", - cache, kh_size(cache->map), (int)cache->used_memory); + printf("Cache %p: %d items cached, %"PRIdZ" bytes\n", + cache, kh_size(cache->map), cache->used_memory); kh_foreach_value(cache->map, object, { char oid_str[9]; - printf(" %s%c %s (%d)\n", + printf(" %s%c %s (%"PRIuZ")\n", git_object_type2string(object->type), object->flags == GIT_CACHE_STORE_PARSED ? '*' : ' ', git_oid_tostr(oid_str, sizeof(oid_str), &object->oid), - (int)object->size + object->size ); }); } diff --git a/src/checkout.c b/src/checkout.c index a94d509d34f..4b3acbcce59 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1299,8 +1299,8 @@ static int checkout_get_actions( if (counts[CHECKOUT_ACTION__CONFLICT] > 0 && (data->strategy & GIT_CHECKOUT_ALLOW_CONFLICTS) == 0) { - giterr_set(GITERR_CHECKOUT, "%d %s checkout", - (int)counts[CHECKOUT_ACTION__CONFLICT], + giterr_set(GITERR_CHECKOUT, "%"PRIuZ" %s checkout", + counts[CHECKOUT_ACTION__CONFLICT], counts[CHECKOUT_ACTION__CONFLICT] == 1 ? "conflict prevents" : "conflicts prevent"); error = GIT_ECONFLICT; diff --git a/src/merge.c b/src/merge.c index 9d6252ea81a..863ac8f2d11 100644 --- a/src/merge.c +++ b/src/merge.c @@ -79,7 +79,7 @@ int merge_bases_many(git_commit_list **out, git_revwalk **walk_out, git_reposito unsigned int i; if (length < 2) { - giterr_set(GITERR_INVALID, "At least two commits are required to find an ancestor. Provided 'length' was %u.", length); + giterr_set(GITERR_INVALID, "At least two commits are required to find an ancestor. Provided 'length' was %" PRIuZ ".", length); return -1; } @@ -185,7 +185,7 @@ int git_merge_base_octopus(git_oid *out, git_repository *repo, size_t length, co assert(out && repo && input_array); if (length < 2) { - giterr_set(GITERR_INVALID, "At least two commits are required to find an ancestor. Provided 'length' was %u.", length); + giterr_set(GITERR_INVALID, "At least two commits are required to find an ancestor. Provided 'length' was %" PRIuZ ".", length); return -1; } @@ -2451,7 +2451,7 @@ int git_merge__check_result(git_repository *repo, git_index *index_new) goto done; if ((conflicts = index_conflicts + wd_conflicts) > 0) { - giterr_set(GITERR_MERGE, "%d uncommitted change%s would be overwritten by merge", + giterr_set(GITERR_MERGE, "%" PRIuZ " uncommitted change%s would be overwritten by merge", conflicts, (conflicts != 1) ? "s" : ""); error = GIT_ECONFLICT; } diff --git a/src/rebase.c b/src/rebase.c index 8da7b4f7f0f..17536c03052 100644 --- a/src/rebase.c +++ b/src/rebase.c @@ -436,7 +436,7 @@ static int rebase_setupfiles_merge(git_rebase *rebase) size_t i; int error = 0; - if ((error = rebase_setupfile(rebase, END_FILE, -1, "%d\n", git_array_size(rebase->operations))) < 0 || + if ((error = rebase_setupfile(rebase, END_FILE, -1, "%" PRIuZ "\n", git_array_size(rebase->operations))) < 0 || (error = rebase_setupfile(rebase, ONTO_NAME_FILE, -1, "%s\n", rebase->onto_name)) < 0) goto done; @@ -789,7 +789,7 @@ static int rebase_next_merge( normalize_checkout_options_for_apply(&checkout_opts, rebase, current_commit); if ((error = git_indexwriter_init_for_operation(&indexwriter, rebase->repo, &checkout_opts.checkout_strategy)) < 0 || - (error = rebase_setupfile(rebase, MSGNUM_FILE, -1, "%d\n", rebase->current+1)) < 0 || + (error = rebase_setupfile(rebase, MSGNUM_FILE, -1, "%" PRIuZ "\n", rebase->current+1)) < 0 || (error = rebase_setupfile(rebase, CURRENT_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, current_idstr)) < 0 || (error = git_merge_trees(&index, rebase->repo, parent_tree, head_tree, current_tree, NULL)) < 0 || (error = git_merge__check_result(rebase->repo, index)) < 0 || diff --git a/src/stash.c b/src/stash.c index acf89442add..fcb1112acc7 100644 --- a/src/stash.c +++ b/src/stash.c @@ -770,7 +770,7 @@ static int ensure_clean_index(git_repository *repo, git_index *index) goto done; if (git_diff_num_deltas(index_diff) > 0) { - giterr_set(GITERR_STASH, "%d uncommitted changes exist in the index", + giterr_set(GITERR_STASH, "%" PRIuZ " uncommitted changes exist in the index", git_diff_num_deltas(index_diff)); error = GIT_EUNCOMMITTED; } diff --git a/src/transports/http.c b/src/transports/http.c index dd4426475a2..1ed292be5fe 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -511,7 +511,7 @@ static int write_chunk(git_stream *io, const char *buffer, size_t len) git_buf buf = GIT_BUF_INIT; /* Chunk header */ - git_buf_printf(&buf, "%X\r\n", (unsigned)len); + git_buf_printf(&buf, "%" PRIxZ "\r\n", len); if (git_buf_oom(&buf)) return -1; diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index d214c9fa535..9ccbd80852d 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -523,7 +523,7 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca if (len > 0xffff) { giterr_set(GITERR_NET, - "Tried to produce packet with invalid length %d", len); + "Tried to produce packet with invalid length %" PRIuZ, len); return -1; } diff --git a/tests/blame/blame_helpers.c b/tests/blame/blame_helpers.c index 21cd1a6151e..b305ba1e317 100644 --- a/tests/blame/blame_helpers.c +++ b/tests/blame/blame_helpers.c @@ -4,7 +4,7 @@ void hunk_message(size_t idx, const git_blame_hunk *hunk, const char *fmt, ...) { va_list arglist; - printf("Hunk %zd (line %d +%d): ", idx, + printf("Hunk %"PRIuZ" (line %d +%d): ", idx, hunk->final_start_line_number, hunk->lines_in_hunk-1); va_start(arglist, fmt); diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c index dabc47a0925..b14af44a974 100644 --- a/tests/clar_libgit2.c +++ b/tests/clar_libgit2.c @@ -483,8 +483,8 @@ void clar__assert_equal_file( for (pos = 0; pos < bytes && expected_data[pos] == buf[pos]; ++pos) /* find differing byte offset */; p_snprintf( - buf, sizeof(buf), "file content mismatch at byte %d", - (int)(total_bytes + pos)); + buf, sizeof(buf), "file content mismatch at byte %"PRIdZ, + (ssize_t)(total_bytes + pos)); p_close(fd); clar__fail(file, line, path, buf, 1); } diff --git a/tests/merge/merge_helpers.c b/tests/merge/merge_helpers.c index 33710f4038e..f8147142410 100644 --- a/tests/merge/merge_helpers.c +++ b/tests/merge/merge_helpers.c @@ -110,7 +110,7 @@ void merge__dump_index_entries(git_vector *index_entries) size_t i; const git_index_entry *index_entry; - printf ("\nINDEX [%d]:\n", (int)index_entries->length); + printf ("\nINDEX [%"PRIuZ"]:\n", index_entries->length); for (i = 0; i < index_entries->length; i++) { index_entry = index_entries->contents[i]; From aa51fa1e03df9a30c8c37b168758dd34dd75f353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 13 Jul 2015 08:39:35 +0200 Subject: [PATCH 29/73] submodule: add failing test for backslash in url --- tests/submodule/lookup.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c index 4d40e227908..38f06abbc4d 100644 --- a/tests/submodule/lookup.c +++ b/tests/submodule/lookup.c @@ -133,6 +133,29 @@ void test_submodule_lookup__lookup_even_with_missing_index(void) test_submodule_lookup__simple_lookup(); /* baseline should still pass */ } +void test_submodule_lookup__backslashes(void) +{ + git_config *cfg; + git_submodule *sm; + git_repository *subrepo; + git_buf buf = GIT_BUF_INIT; + const char *backslashed_path = "..\\submod2_target"; + + cl_git_pass(git_config_open_ondisk(&cfg, "submod2/.gitmodules")); + cl_git_pass(git_config_set_string(cfg, "submodule.sm_unchanged.url", backslashed_path)); + git_config_free(cfg); + + cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_unchanged")); + cl_assert_equal_s(backslashed_path, git_submodule_url(sm)); + cl_git_pass(git_submodule_open(&subrepo, sm)); + + cl_git_pass(git_submodule_resolve_url(&buf, g_repo, backslashed_path)); + + git_buf_free(&buf); + git_submodule_free(sm); + git_repository_free(subrepo); +} + static void baseline_tests(void) { /* small baseline that should work even if we change the index or make From f00f005bad3d27bb5c23ae5d7187622b940c5d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 13 Jul 2015 09:08:32 +0200 Subject: [PATCH 30/73] submodule: normalize slashes in resolve_url Our path functions expect to work with slashes, so convert a path with backslashes into one with slashes at the top of the function. --- src/submodule.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/submodule.c b/src/submodule.c index fb3d4bf1e52..e4469efa34f 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -781,11 +781,25 @@ const char *git_submodule_url(git_submodule *submodule) int git_submodule_resolve_url(git_buf *out, git_repository *repo, const char *url) { int error = 0; + git_buf normalized = GIT_BUF_INIT; assert(out && repo && url); git_buf_sanitize(out); + if (strchr(url, '\\')) { + char *p; + if ((error = git_buf_puts(&normalized, url)) < 0) + return error; + + for (p = normalized.ptr; *p; p++) { + if (*p == '\\') + *p = '/'; + } + + url = normalized.ptr; + } + if (git_path_is_relative(url)) { if (!(error = get_url_base(out, repo))) error = git_path_apply_relative(out, url); @@ -796,6 +810,7 @@ int git_submodule_resolve_url(git_buf *out, git_repository *repo, const char *ur error = -1; } + git_buf_free(&normalized); return error; } From a58854a0311316a66fda363e83665ab942d81ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 13 Jul 2015 17:11:19 +0200 Subject: [PATCH 31/73] submodule, path: extract slash conversion Extract the backslash-to-slash conversion into a helper function. --- src/path.c | 16 ++++++++++++++++ src/path.h | 5 +++++ src/submodule.c | 10 +++------- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/path.c b/src/path.c index 2558058dd8b..16283cab2a4 100644 --- a/src/path.c +++ b/src/path.c @@ -1671,3 +1671,19 @@ bool git_path_isvalid( return verify_component(repo, start, (c - start), flags); } + +int git_path_normalize_slashes(git_buf *out, const char *path) +{ + int error; + char *p; + + if ((error = git_buf_puts(out, path)) < 0) + return error; + + for (p = out->ptr; *p; p++) { + if (*p == '\\') + *p = '/'; + } + + return 0; +} diff --git a/src/path.h b/src/path.h index 5927a53812c..adb76865e98 100644 --- a/src/path.h +++ b/src/path.h @@ -591,4 +591,9 @@ extern bool git_path_isvalid( const char *path, unsigned int flags); +/** + * Convert any backslashes into slashes + */ +int git_path_normalize_slashes(git_buf *out, const char *path); + #endif diff --git a/src/submodule.c b/src/submodule.c index e4469efa34f..f16d3c5ab48 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -787,19 +787,15 @@ int git_submodule_resolve_url(git_buf *out, git_repository *repo, const char *ur git_buf_sanitize(out); + /* We do this in all platforms in case someone on Windows created the .gitmodules */ if (strchr(url, '\\')) { - char *p; - if ((error = git_buf_puts(&normalized, url)) < 0) + if ((error = git_path_normalize_slashes(&normalized, url)) < 0) return error; - for (p = normalized.ptr; *p; p++) { - if (*p == '\\') - *p = '/'; - } - url = normalized.ptr; } + if (git_path_is_relative(url)) { if (!(error = get_url_base(out, repo))) error = git_path_apply_relative(out, url); From cec3569f250379aaa9aa4cec4fb49d4a78d6ee11 Mon Sep 17 00:00:00 2001 From: Fallso Date: Tue, 14 Jul 2015 15:33:56 +0100 Subject: [PATCH 32/73] Fix macro redefinition warning --- src/thread-utils.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/thread-utils.c b/src/thread-utils.c index c3baf411ad3..dc9b2f09e93 100644 --- a/src/thread-utils.c +++ b/src/thread-utils.c @@ -8,7 +8,9 @@ #include "thread-utils.h" #ifdef _WIN32 +#ifndef WIN32_LEAN_AND_MEAN # define WIN32_LEAN_AND_MEAN +#endif # include #elif defined(hpux) || defined(__hpux) || defined(_hpux) # include From 37996d474b658f7428b2195023da82a9b480f307 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 20 Jul 2015 09:21:36 -0400 Subject: [PATCH 33/73] Document git_fetch_options struct and fix typo. git_fetch_options was missing from the API docs because it lacked a documentation comment above the struct declaration. I used the git_checkout_options docstring as a template. Also fixes a typo in git_remote_prune_refs (remote, not reamote). --- include/git2/remote.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/git2/remote.h b/include/git2/remote.h index e47f00881ba..444fe5276de 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -511,6 +511,14 @@ typedef enum { GIT_REMOTE_DOWNLOAD_TAGS_ALL, } git_remote_autotag_option_t; +/** + * Fetch options structure. + * + * Zero out for defaults. Initialize with `GIT_FETCH_OPTIONS_INIT` macro to + * correctly set the `version` field. E.g. + * + * git_fetch_options opts = GIT_FETCH_OPTIONS_INIT; + */ typedef struct { int version; @@ -739,7 +747,7 @@ GIT_EXTERN(int) git_remote_prune_refs(const git_remote *remote); * stored here for further processing by the caller. Always free this * strarray on successful return. * @param repo the repository in which to rename - * @param name the current name of the reamote + * @param name the current name of the remote * @param new_name the new name the remote should bear * @return 0, GIT_EINVALIDSPEC, GIT_EEXISTS or an error code */ From 318bb763e9e896472e2aa16ebe8cbf2463804e19 Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Tue, 21 Jul 2015 23:36:39 +0200 Subject: [PATCH 34/73] Make libgit2 work on Windows Vista again (fixes issue #3316) Signed-off-by: Sven Strickroth --- src/path.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/path.c b/src/path.c index 2558058dd8b..3e63f06d48a 100644 --- a/src/path.c +++ b/src/path.c @@ -12,6 +12,7 @@ #include "win32/posix.h" #include "win32/buffer.h" #include "win32/w32_util.h" +#include "win32/version.h" #else #include #endif @@ -1085,7 +1086,7 @@ int git_path_direach( #if defined(GIT_WIN32) && !defined(__MINGW32__) /* Using _FIND_FIRST_EX_LARGE_FETCH may increase performance in Windows 7 - * and better. Prior versions will ignore this. + * and better. */ #ifndef FIND_FIRST_EX_LARGE_FETCH # define FIND_FIRST_EX_LARGE_FETCH 2 @@ -1099,6 +1100,10 @@ int git_path_diriter_init( git_win32_path path_filter; git_buf hack = {0}; + static int is_win7_or_later = -1; + if (is_win7_or_later < 0) + is_win7_or_later = git_has_win32_version(6, 1, 0); + assert(diriter && path); memset(diriter, 0, sizeof(git_path_diriter)); @@ -1122,11 +1127,11 @@ int git_path_diriter_init( diriter->handle = FindFirstFileExW( path_filter, - FindExInfoBasic, + is_win7_or_later ? FindExInfoBasic : FindExInfoStandard, &diriter->current, FindExSearchNameMatch, NULL, - FIND_FIRST_EX_LARGE_FETCH); + is_win7_or_later ? FIND_FIRST_EX_LARGE_FETCH : 0); if (diriter->handle == INVALID_HANDLE_VALUE) { giterr_set(GITERR_OS, "Could not open directory '%s'", path); From cf198fdf2a044d2e2f0675c2c6b1cd9cdbcf4fcf Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 22 Jul 2015 10:51:38 -0400 Subject: [PATCH 35/73] Increment `git__n_inits` before doing `init_once`. Fixes #3318. --- src/global.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/global.c b/src/global.c index 3f20bfd3195..cb2242405ca 100644 --- a/src/global.c +++ b/src/global.c @@ -330,8 +330,8 @@ int git_libgit2_init(void) { int ret; - pthread_once(&_once_init, init_once); ret = git_atomic_inc(&git__n_inits); + pthread_once(&_once_init, init_once); return init_error ? init_error : ret; } From 668053befecfd0979ff790bc9a2a3c308c38a9be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 24 Jul 2015 18:44:29 +0200 Subject: [PATCH 36/73] filebuf: failing test for leaving the lockfile when failing to rename When we fail to rename, we currently leave the lockfile laying around. This shows that behaviour. --- tests/core/filebuf.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/core/filebuf.c b/tests/core/filebuf.c index 5a3e7510f48..9fd52b179ba 100644 --- a/tests/core/filebuf.c +++ b/tests/core/filebuf.c @@ -124,3 +124,30 @@ void test_core_filebuf__umask(void) cl_must_pass(p_unlink(test)); } +void test_core_filebuf__rename_error(void) +{ + git_filebuf file = GIT_FILEBUF_INIT; + char *dir = "subdir", *test = "subdir/test", *test_lock = "subdir/test.lock"; + int fd; + +#ifndef GIT_WIN32 + cl_skip(); +#endif + + cl_git_pass(p_mkdir(dir, 0666)); + cl_git_mkfile(test, "dummy content"); + fd = p_open(test, O_RDONLY); + cl_assert(fd > 0); + cl_git_pass(git_filebuf_open(&file, test, 0, 0666)); + + cl_git_pass(git_filebuf_printf(&file, "%s\n", "libgit2 rocks")); + + cl_assert_equal_i(true, git_path_exists(test_lock)); + + cl_git_fail(git_filebuf_commit(&file)); + p_close(fd); + + git_filebuf_cleanup(&file); + cl_assert_equal_i(false, git_path_exists(test_lock)); + +} From 19d9beb7ffbde3e171c17fc3544d508304a30fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 24 Jul 2015 19:22:41 +0200 Subject: [PATCH 37/73] filebuf: remove lockfile upon rename errors When we have an error renaming the lockfile, we need to make sure that we remove it upon cleanup. For this, we need to keep track of whether we opened the file and whether the rename succeeded. If we did create the lockfile but the rename did not succeed, we remove the lockfile. This won't protect against all errors, but the most common ones (target file is open) does get handled. --- src/filebuf.c | 7 ++++++- src/filebuf.h | 2 ++ tests/core/filebuf.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/filebuf.c b/src/filebuf.c index 848ac343bc5..838f4b4d265 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -101,7 +101,7 @@ void git_filebuf_cleanup(git_filebuf *file) if (file->fd_is_open && file->fd >= 0) p_close(file->fd); - if (file->fd_is_open && file->path_lock && git_path_exists(file->path_lock)) + if (file->created_lock && !file->did_rename && file->path_lock && git_path_exists(file->path_lock)) p_unlink(file->path_lock); if (file->compute_digest) { @@ -258,6 +258,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode goto cleanup; } file->fd_is_open = true; + file->created_lock = true; /* No original path */ file->path_original = NULL; @@ -281,6 +282,8 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode /* open the file for locking */ if ((error = lock_file(file, flags, mode)) < 0) goto cleanup; + + file->created_lock = true; } return 0; @@ -340,6 +343,8 @@ int git_filebuf_commit(git_filebuf *file) goto on_error; } + file->did_rename = true; + git_filebuf_cleanup(file); return 0; diff --git a/src/filebuf.h b/src/filebuf.h index 2bd18dc35a7..f4d255b0a42 100644 --- a/src/filebuf.h +++ b/src/filebuf.h @@ -44,6 +44,8 @@ struct git_filebuf { size_t buf_size, buf_pos; git_file fd; bool fd_is_open; + bool created_lock; + bool did_rename; bool do_not_buffer; int last_error; }; diff --git a/tests/core/filebuf.c b/tests/core/filebuf.c index 9fd52b179ba..3f7dc856919 100644 --- a/tests/core/filebuf.c +++ b/tests/core/filebuf.c @@ -148,6 +148,6 @@ void test_core_filebuf__rename_error(void) p_close(fd); git_filebuf_cleanup(&file); - cl_assert_equal_i(false, git_path_exists(test_lock)); + cl_assert_equal_i(false, git_path_exists(test_lock)); } From 12786e0f7c0e6ea23b1483f8732ca23b367bfd8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 26 Jul 2015 17:19:22 +0200 Subject: [PATCH 38/73] iterator: skip over errors in diriter init An error here will typically mean that the directory was removed between the time we iterated the parent and the time we wanted to visit it in which case we should ignore it. Other kinds of errors such as permissions (or transient errors) also better dealt with by pretending we didn't see it. --- src/iterator.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/iterator.c b/src/iterator.c index a312afb3a1a..cf51a340db8 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1027,8 +1027,11 @@ static int dirload_with_stat( strncomp = (flags & GIT_PATH_DIR_IGNORE_CASE) != 0 ? git__strncasecmp : git__strncmp; - if ((error = git_path_diriter_init(&diriter, dirpath, flags)) < 0) + /* Any error here is equivalent to the dir not existing, skip over it */ + if ((error = git_path_diriter_init(&diriter, dirpath, flags)) < 0) { + error = GIT_ENOTFOUND; goto done; + } while ((error = git_path_diriter_next(&diriter)) == 0) { if ((error = git_path_diriter_fullpath(&path, &path_len, &diriter)) < 0) From 0e391d8526032008a53fd3e8f7c6795d59ebdb5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 27 Jul 2015 13:31:06 +0200 Subject: [PATCH 39/73] iterator: adjust unreadable-dir test to new behaviour We don't want the iterator to make us stop whenever we hit an unreadable dir. We should instead move over to the next item. --- tests/repo/iterator.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/repo/iterator.c b/tests/repo/iterator.c index 26e8954fe3a..bb2d3a1867b 100644 --- a/tests/repo/iterator.c +++ b/tests/repo/iterator.c @@ -928,7 +928,7 @@ void test_repo_iterator__fs2(void) git_iterator_free(i); } -void test_repo_iterator__fs_preserves_error(void) +void test_repo_iterator__unreadable_dir(void) { git_iterator *i; const git_index_entry *e; @@ -951,10 +951,6 @@ void test_repo_iterator__fs_preserves_error(void) cl_git_pass(git_iterator_advance(&e, i)); /* a */ cl_git_fail(git_iterator_advance(&e, i)); /* b */ - cl_assert(giterr_last()); - cl_assert(giterr_last()->message != NULL); - /* skip 'c/' empty directory */ - cl_git_pass(git_iterator_advance(&e, i)); /* d */ cl_assert_equal_i(GIT_ITEROVER, git_iterator_advance(&e, i)); cl_must_pass(p_chmod("empty_standard_repo/r/b", 0777)); From 41808d04705644b8af566c9039a3ddcbb3257c99 Mon Sep 17 00:00:00 2001 From: Ben Chatelain Date: Mon, 27 Jul 2015 14:46:50 -0600 Subject: [PATCH 40/73] Fix @param names in doc comments --- include/git2/repository.h | 2 +- include/git2/sys/odb_backend.h | 2 +- include/git2/sys/refdb_backend.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/git2/repository.h b/include/git2/repository.h index ce56fef0f48..cf268ef85b6 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -745,7 +745,7 @@ GIT_EXTERN(int) git_repository_ident(const char **name, const char **email, cons * * @param repo the repository to configure * @param name the name to use for the reflog entries - * @param name the email to use for the reflog entries + * @param email the email to use for the reflog entries */ GIT_EXTERN(int) git_repository_set_ident(git_repository *repo, const char *name, const char *email); diff --git a/include/git2/sys/odb_backend.h b/include/git2/sys/odb_backend.h index 0a51c6dba4a..fe102ff3cab 100644 --- a/include/git2/sys/odb_backend.h +++ b/include/git2/sys/odb_backend.h @@ -93,7 +93,7 @@ struct git_odb_backend { * Initializes a `git_odb_backend` with default values. Equivalent to * creating an instance with GIT_ODB_BACKEND_INIT. * - * @param opts the `git_odb_backend` struct to initialize. + * @param backend the `git_odb_backend` struct to initialize. * @param version Version the struct; pass `GIT_ODB_BACKEND_VERSION` * @return Zero on success; -1 on failure. */ diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index d943e550f46..8b004a7e027 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -175,7 +175,7 @@ struct git_refdb_backend { * Initializes a `git_refdb_backend` with default values. Equivalent to * creating an instance with GIT_REFDB_BACKEND_INIT. * - * @param opts the `git_refdb_backend` struct to initialize + * @param backend the `git_refdb_backend` struct to initialize * @param version Version of struct; pass `GIT_REFDB_BACKEND_VERSION` * @return Zero on success; -1 on failure. */ From f90fbb8d22c77fbc4d207ef68a9fd445e68602db Mon Sep 17 00:00:00 2001 From: Ben Chatelain Date: Mon, 27 Jul 2015 17:42:08 -0600 Subject: [PATCH 41/73] Use correct Doxygen trailing comment syntax --- include/git2/sys/diff.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/git2/sys/diff.h b/include/git2/sys/diff.h index 034d5c47830..aa6fed757ae 100644 --- a/include/git2/sys/diff.h +++ b/include/git2/sys/diff.h @@ -38,7 +38,7 @@ GIT_EXTERN(int) git_diff_print_callback__to_buf( const git_diff_delta *delta, const git_diff_hunk *hunk, const git_diff_line *line, - void *payload); /*< payload must be a `git_buf *` */ + void *payload); /**< payload must be a `git_buf *` */ /** * Diff print callback that writes to stdio FILE handle. @@ -58,7 +58,7 @@ GIT_EXTERN(int) git_diff_print_callback__to_file_handle( const git_diff_delta *delta, const git_diff_hunk *hunk, const git_diff_line *line, - void *payload); /*< payload must be a `FILE *` */ + void *payload); /**< payload must be a `FILE *` */ /** @@ -66,8 +66,8 @@ GIT_EXTERN(int) git_diff_print_callback__to_file_handle( */ typedef struct { unsigned int version; - size_t stat_calls; /*< Number of stat() calls performed */ - size_t oid_calculations; /*< Number of ID calculations */ + size_t stat_calls; /**< Number of stat() calls performed */ + size_t oid_calculations; /** Date: Mon, 27 Jul 2015 18:28:29 -0600 Subject: [PATCH 42/73] Add -Wdocumentation flag if supported --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 20e3272f9b8..a15ce7dbd7b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -413,6 +413,7 @@ ELSE () SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") ENDIF () + ADD_C_FLAG_IF_SUPPORTED(-Wdocumentation) ADD_C_FLAG_IF_SUPPORTED(-Wno-missing-field-initializers) ADD_C_FLAG_IF_SUPPORTED(-Wstrict-aliasing=2) ADD_C_FLAG_IF_SUPPORTED(-Wstrict-prototypes) From 08afd227dfd3ee56accd6d6ded3889f34c5329fc Mon Sep 17 00:00:00 2001 From: Ben Chatelain Date: Mon, 27 Jul 2015 18:32:55 -0600 Subject: [PATCH 43/73] Fix remaining documentation warnings --- include/git2/sys/config.h | 2 +- include/git2/sys/diff.h | 2 +- src/path.h | 2 +- src/push.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/git2/sys/config.h b/include/git2/sys/config.h index b5b7df15f07..044e34417ed 100644 --- a/include/git2/sys/config.h +++ b/include/git2/sys/config.h @@ -76,7 +76,7 @@ struct git_config_backend { * Initializes a `git_config_backend` with default values. Equivalent to * creating an instance with GIT_CONFIG_BACKEND_INIT. * - * @param opts the `git_config_backend` struct to initialize. + * @param backend the `git_config_backend` struct to initialize. * @param version Version of struct; pass `GIT_CONFIG_BACKEND_VERSION` * @return Zero on success; -1 on failure. */ diff --git a/include/git2/sys/diff.h b/include/git2/sys/diff.h index aa6fed757ae..aefd7b9973f 100644 --- a/include/git2/sys/diff.h +++ b/include/git2/sys/diff.h @@ -67,7 +67,7 @@ GIT_EXTERN(int) git_diff_print_callback__to_file_handle( typedef struct { unsigned int version; size_t stat_calls; /**< Number of stat() calls performed */ - size_t oid_calculations; /** Date: Sun, 26 Jul 2015 21:12:00 +0200 Subject: [PATCH 44/73] error: store the error messages in a reusable buffer Instead of allocating a brand new buffer for each error string we want to store, we can use a per-thread buffer to store the error string and re-use the underlying storage. We already use the buffer to format the string, so this mostly makes that more direct. --- src/errors.c | 50 +++++++++++++++++++++++++++++++++----------------- src/global.c | 14 ++++++++------ src/global.h | 1 + 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/errors.c b/src/errors.c index 7a260058699..56fad009fb0 100644 --- a/src/errors.c +++ b/src/errors.c @@ -18,19 +18,30 @@ static git_error g_git_oom_error = { GITERR_NOMEMORY }; -static void set_error(int error_class, char *string) +static void set_error_from_buffer(int error_class) { git_error *error = &GIT_GLOBAL->error_t; + git_buf *buf = &GIT_GLOBAL->error_buf; - if (error->message != string) - git__free(error->message); - - error->message = string; + error->message = buf->ptr; error->klass = error_class; GIT_GLOBAL->last_error = error; } +static void set_error(int error_class, char *string) +{ + git_buf *buf = &GIT_GLOBAL->error_buf; + + git_buf_clear(buf); + if (string) { + git_buf_puts(buf, string); + git__free(string); + } + + set_error_from_buffer(error_class); +} + void giterr_set_oom(void) { GIT_GLOBAL->last_error = &g_git_oom_error; @@ -38,27 +49,28 @@ void giterr_set_oom(void) void giterr_set(int error_class, const char *string, ...) { - git_buf buf = GIT_BUF_INIT; va_list arglist; #ifdef GIT_WIN32 DWORD win32_error_code = (error_class == GITERR_OS) ? GetLastError() : 0; #endif int error_code = (error_class == GITERR_OS) ? errno : 0; + git_buf *buf = &GIT_GLOBAL->error_buf; + git_buf_clear(buf); if (string) { va_start(arglist, string); - git_buf_vprintf(&buf, string, arglist); + git_buf_vprintf(buf, string, arglist); va_end(arglist); if (error_class == GITERR_OS) - git_buf_PUTS(&buf, ": "); + git_buf_PUTS(buf, ": "); } if (error_class == GITERR_OS) { #ifdef GIT_WIN32 char * win32_error = git_win32_get_error_message(win32_error_code); if (win32_error) { - git_buf_puts(&buf, win32_error); + git_buf_puts(buf, win32_error); git__free(win32_error); SetLastError(0); @@ -66,26 +78,29 @@ void giterr_set(int error_class, const char *string, ...) else #endif if (error_code) - git_buf_puts(&buf, strerror(error_code)); + git_buf_puts(buf, strerror(error_code)); if (error_code) errno = 0; } - if (!git_buf_oom(&buf)) - set_error(error_class, git_buf_detach(&buf)); + if (!git_buf_oom(buf)) + set_error_from_buffer(error_class); } void giterr_set_str(int error_class, const char *string) { - char *message; + git_buf *buf = &GIT_GLOBAL->error_buf; assert(string); - message = git__strdup(string); + if (!string) + return; - if (message) - set_error(error_class, message); + git_buf_clear(buf); + git_buf_puts(buf, string); + if (!git_buf_oom(buf)) + set_error_from_buffer(error_class); } int giterr_set_regex(const regex_t *regex, int error_code) @@ -119,13 +134,14 @@ void giterr_clear(void) int giterr_detach(git_error *cpy) { git_error *error = GIT_GLOBAL->last_error; + git_buf *buf = &GIT_GLOBAL->error_buf; assert(cpy); if (!error) return -1; - cpy->message = error->message; + cpy->message = git_buf_detach(buf); cpy->klass = error->klass; error->message = NULL; diff --git a/src/global.c b/src/global.c index 37a47bd27ad..3d37ee4dec2 100644 --- a/src/global.c +++ b/src/global.c @@ -279,18 +279,19 @@ int git_libgit2_shutdown(void) git_global_st *git__global_state(void) { - void *ptr; + git_global_st *ptr; assert(git_atomic_get(&git__n_inits) > 0); if ((ptr = TlsGetValue(_tls_index)) != NULL) return ptr; - ptr = git__malloc(sizeof(git_global_st)); + ptr = git__calloc(1, sizeof(git_global_st)); if (!ptr) return NULL; - memset(ptr, 0x0, sizeof(git_global_st)); + git_buf_init(&ptr->error_buf, 0); + TlsSetValue(_tls_index, ptr); return ptr; } @@ -378,18 +379,18 @@ int git_libgit2_shutdown(void) git_global_st *git__global_state(void) { - void *ptr; + git_global_st *ptr; assert(git_atomic_get(&git__n_inits) > 0); if ((ptr = pthread_getspecific(_tls_key)) != NULL) return ptr; - ptr = git__malloc(sizeof(git_global_st)); + ptr = git__calloc(1, sizeof(git_global_st)); if (!ptr) return NULL; - memset(ptr, 0x0, sizeof(git_global_st)); + git_buf_init(&ptr->error_buf, 0); pthread_setspecific(_tls_key, ptr); return ptr; } @@ -407,6 +408,7 @@ int git_libgit2_init(void) ssl_inited = 1; } + git_buf_init(&__state.error_buf, 0); return git_atomic_inc(&git__n_inits); } diff --git a/src/global.h b/src/global.h index fdad6ba8994..37e909ac6f7 100644 --- a/src/global.h +++ b/src/global.h @@ -14,6 +14,7 @@ typedef struct { git_error *last_error; git_error error_t; + git_buf error_buf; char oid_fmt[GIT_OID_HEXSZ+1]; } git_global_st; From 6d8f3a5162f3160069a2cbafeb44e9ffd4ebe77c Mon Sep 17 00:00:00 2001 From: Ben Chatelain Date: Tue, 28 Jul 2015 08:28:33 -0600 Subject: [PATCH 45/73] Better param docs --- src/push.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/push.h b/src/push.h index 53890f54b28..a847ee0d080 100644 --- a/src/push.h +++ b/src/push.h @@ -83,7 +83,7 @@ int git_push_add_refspec(git_push *push, const char *refspec); * Update remote tips after a push * * @param push The push object - * @param callbacks The identity to use when updating reflogs + * @param callbacks the callbacks to use for this connection * * @return 0 or an error code */ @@ -100,6 +100,7 @@ int git_push_update_tips(git_push *push, const git_remote_callbacks *callbacks); * order to find out which updates were accepted or rejected. * * @param push The push object + * @param callbacks the callbacks to use for this connection * * @return 0 or an error code */ @@ -117,6 +118,7 @@ int git_push_finish(git_push *push, const git_remote_callbacks *callbacks); * * @param push The push object * @param cb The callback to call on each object + * @param data The payload passed to the callback * * @return 0 on success, non-zero callback return value, or error code */ From 31a76374a9bea7d61fec3741d46f09872a8bbb2a Mon Sep 17 00:00:00 2001 From: Anders Borum Date: Wed, 29 Jul 2015 22:23:00 +0200 Subject: [PATCH 46/73] case-insensitive check for WWW-Authenticate header Fixes issue #3338 --- src/transports/http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/http.c b/src/transports/http.c index 1ed292be5fe..e3d90de11a4 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -255,7 +255,7 @@ static int on_header_ready(http_subtransport *t) GITERR_CHECK_ALLOC(t->content_type); } } - else if (!strcmp("WWW-Authenticate", git_buf_cstr(name))) { + else if (!strcasecmp("WWW-Authenticate", git_buf_cstr(name))) { char *dup = git__strdup(git_buf_cstr(value)); GITERR_CHECK_ALLOC(dup); From c369b37919114f4976b7a06107506e95916e5dd0 Mon Sep 17 00:00:00 2001 From: Stefan Widgren Date: Fri, 31 Jul 2015 16:23:11 +0200 Subject: [PATCH 47/73] Remove extra semicolon outside of a function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this change, compiling with gcc and pedantic generates warning: ISO C does not allow extra ‘;’ outside of a function. --- src/describe.c | 2 +- src/indexer.c | 2 +- src/pack-objects.c | 2 +- src/pack.c | 4 ++-- src/revwalk.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/describe.c b/src/describe.c index 5aea927c1ad..48f04e85837 100644 --- a/src/describe.c +++ b/src/describe.c @@ -19,7 +19,7 @@ #include "vector.h" #include "repository.h" -GIT__USE_OIDMAP; +GIT__USE_OIDMAP /* Ported from https://github.com/git/git/blob/89dde7882f71f846ccd0359756d27bebc31108de/builtin/describe.c */ diff --git a/src/indexer.c b/src/indexer.c index 512addf48bf..9aa092556b0 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -18,7 +18,7 @@ #include "oidmap.h" #include "zstream.h" -GIT__USE_OIDMAP; +GIT__USE_OIDMAP extern git_mutex git__mwindow_mutex; diff --git a/src/pack-objects.c b/src/pack-objects.c index e287e33069a..c4c061a3a9f 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -41,7 +41,7 @@ struct pack_write_context { git_transfer_progress *stats; }; -GIT__USE_OIDMAP; +GIT__USE_OIDMAP #ifdef GIT_THREADS diff --git a/src/pack.c b/src/pack.c index cd652672119..45dd4d5be8e 100644 --- a/src/pack.c +++ b/src/pack.c @@ -16,8 +16,8 @@ #include -GIT__USE_OFFMAP; -GIT__USE_OIDMAP; +GIT__USE_OFFMAP +GIT__USE_OIDMAP static int packfile_open(struct git_pack_file *p); static git_off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n); diff --git a/src/revwalk.c b/src/revwalk.c index 6acc5d034be..dcdd97915aa 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -14,7 +14,7 @@ #include "git2/revparse.h" #include "merge.h" -GIT__USE_OIDMAP; +GIT__USE_OIDMAP git_commit_list_node *git_revwalk__commit_lookup( git_revwalk *walk, const git_oid *oid) From 63e5b5512226cfbea24bcf5d74b68cbc89197447 Mon Sep 17 00:00:00 2001 From: Linquize Date: Wed, 29 Jul 2015 00:08:37 +0800 Subject: [PATCH 48/73] index: add test for adding an old-style submodule to index --- tests/index/bypath.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/index/bypath.c b/tests/index/bypath.c index ddb766a22d1..b587a9ce8bd 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -33,3 +33,10 @@ void test_index_bypath__add_submodule(void) cl_git_pass(git_submodule_status(&status, g_repo, sm_name, 0)); cl_assert_equal_i(0, status & GIT_SUBMODULE_STATUS_WD_MODIFIED); } + +void test_index_bypath__add_submodule_old_style(void) +{ + const char *sm_name = "not-submodule"; + + cl_git_pass(git_index_add_bypath(g_idx, sm_name)); +} From b426ac90a958176131b9ace6a4676bc3ee1ace4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Aug 2015 19:52:25 +0200 Subject: [PATCH 49/73] index: test that an unregistered submodule gets staged When we pass the path of a repository to `_bypath()`, we should behave like git and stage it as a `_COMMIT` regardless of whether it is registered a a submodule. --- tests/index/bypath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/index/bypath.c b/tests/index/bypath.c index b587a9ce8bd..9706a8833e2 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -34,9 +34,15 @@ void test_index_bypath__add_submodule(void) cl_assert_equal_i(0, status & GIT_SUBMODULE_STATUS_WD_MODIFIED); } -void test_index_bypath__add_submodule_old_style(void) +void test_index_bypath__add_submodule_unregistered(void) { const char *sm_name = "not-submodule"; + const char *sm_head = "68e92c611b80ee1ed8f38314ff9577f0d15b2444"; + const git_index_entry *entry; cl_git_pass(git_index_add_bypath(g_idx, sm_name)); + + cl_assert(entry = git_index_get_bypath(g_idx, sm_name, 0)); + cl_assert_equal_s(sm_head, git_oid_tostr_s(&entry->id)); + cl_assert_equal_s(sm_name, entry->path); } From ea961abf244002f067699341734dce83f3b8b7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Aug 2015 19:53:53 +0200 Subject: [PATCH 50/73] index: stage an unregistered submodule as well We previously added logic to `_add_bypath()` to update a submodule. Go further and stage the submodule even if it's not registered to behave like git. --- src/index.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 5 deletions(-) diff --git a/src/index.c b/src/index.c index 501498e9e07..73f0b3d2669 100644 --- a/src/index.c +++ b/src/index.c @@ -1228,6 +1228,45 @@ int git_index_add_frombuffer( return 0; } +static int add_repo_as_submodule(git_index_entry **out, git_index *index, const char *path) +{ + git_repository *sub; + git_buf abspath = GIT_BUF_INIT; + git_repository *repo = INDEX_OWNER(index); + git_reference *head; + git_index_entry *entry; + struct stat st; + int error; + + if (index_entry_create(&entry, INDEX_OWNER(index), path) < 0) + return -1; + + if ((error = git_buf_joinpath(&abspath, git_repository_workdir(repo), path)) < 0) + return error; + + if ((error = p_stat(abspath.ptr, &st)) < 0) { + giterr_set(GITERR_OS, "failed to stat repository dir"); + return -1; + } + + git_index_entry__init_from_stat(entry, &st, !index->distrust_filemode); + + if ((error = git_repository_open(&sub, abspath.ptr)) < 0) + return error; + + if ((error = git_repository_head(&head, sub)) < 0) + return error; + + git_oid_cpy(&entry->id, git_reference_target(head)); + entry->mode = GIT_FILEMODE_COMMIT; + + git_reference_free(head); + git_repository_free(sub); + git_buf_free(&abspath); + + *out = entry; + return 0; +} int git_index_add_bypath(git_index *index, const char *path) { @@ -1252,12 +1291,26 @@ int git_index_add_bypath(git_index *index, const char *path) ret = git_submodule_lookup(&sm, INDEX_OWNER(index), path); if (ret == GIT_ENOTFOUND) return giterr_restore(&err); - else - git__free(err.error_msg.message); - ret = git_submodule_add_to_index(sm, false); - git_submodule_free(sm); - return ret; + git__free(err.error_msg.message); + + /* + * EEXISTS means that there is a repository at that path, but it's not known + * as a submodule. We add its HEAD as an entry and don't register it. + */ + if (ret == GIT_EEXISTS) { + if ((ret = add_repo_as_submodule(&entry, index, path)) < 0) + return ret; + + if ((ret = index_insert(index, &entry, 1, false)) < 0) + return ret; + } else if (ret < 0) { + return ret; + } else { + ret = git_submodule_add_to_index(sm, false); + git_submodule_free(sm); + return ret; + } } /* Adding implies conflict was resolved, move conflict entries to REUC */ From ac728c248361df6ab8c23d8c5cfece7391c871db Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 3 Aug 2015 07:38:07 +0100 Subject: [PATCH 51/73] Handle ssh:// and git:// urls containing a '~' character. For such a path '/~/...' the leading '/' is stripped so the server will get a path starting with '~' and correctly handle it. --- src/transports/git.c | 2 ++ src/transports/ssh.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/transports/git.c b/src/transports/git.c index 7e0a474148b..52de92d0969 100644 --- a/src/transports/git.c +++ b/src/transports/git.c @@ -50,6 +50,8 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url) } repo = delim; + if (repo[1] == '~') + ++repo; delim = strchr(url, ':'); if (delim == NULL) diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 0b0d4bac360..8f5a7164b81 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -66,6 +66,8 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url) if (!git__prefixcmp(url, prefix_ssh)) { url = url + strlen(prefix_ssh); repo = strchr(url, '/'); + if (repo && repo[1] == '~') + ++repo; } else { repo = strchr(url, ':'); if (repo) repo++; From 5ef4b86015309c157b20260905cb5d0c9bb47ca8 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Thu, 23 Jul 2015 13:16:19 +0100 Subject: [PATCH 52/73] Add failing test for capture/restore oom error --- tests/core/errors.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/core/errors.c b/tests/core/errors.c index a06ec4abc4e..6aceb30fd74 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -110,6 +110,29 @@ void test_core_errors__restore(void) cl_assert_equal_s("Foo: bar", giterr_last()->message); } +void test_core_errors__restore_oom(void) +{ + git_error_state err_state = {0}; + const char *static_message = NULL; + + giterr_clear(); + + giterr_set_oom(); /* internal fn */ + static_message = giterr_last()->message; + + cl_assert_equal_i(-1, giterr_capture(&err_state, -1)); + + cl_assert(giterr_last() == NULL); + + cl_assert_(err_state.error_msg.message != static_message, "pointer to static buffer exposed"); + + giterr_restore(&err_state); + + cl_assert(giterr_last()->klass == GITERR_NOMEMORY); + + giterr_clear(); +} + static int test_arraysize_multiply(size_t nelem, size_t size) { size_t out; From c2f17bda074b2e5718456aed75fedd2196c8dc94 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Thu, 23 Jul 2015 13:17:08 +0100 Subject: [PATCH 53/73] Ensure static oom error message not detached Error messages that are detached are assumed to be dynamically allocated. Passing a pointer to the static oom error message can cause an attempt to free the static buffer later. This change checks if the oom error message is about to be detached and detaches a copy instead. --- src/errors.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/errors.c b/src/errors.c index 7a260058699..979602a2a9a 100644 --- a/src/errors.c +++ b/src/errors.c @@ -125,10 +125,14 @@ int giterr_detach(git_error *cpy) if (!error) return -1; - cpy->message = error->message; + if (error == &g_git_oom_error) { + cpy->message = git__strdup(error->message); + } else { + cpy->message = error->message; + error->message = NULL; + } cpy->klass = error->klass; - error->message = NULL; giterr_clear(); return 0; From 25dbcf34993cad3cdc3981f1ed394d3374fb640f Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Mon, 27 Jul 2015 09:59:07 +0100 Subject: [PATCH 54/73] Make giterr_detach no longer public --- include/git2/errors.h | 12 ------------ src/errors.c | 2 +- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index e189e55f17e..4698366d852 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -113,18 +113,6 @@ GIT_EXTERN(const git_error *) giterr_last(void); */ GIT_EXTERN(void) giterr_clear(void); -/** - * Get the last error data and clear it. - * - * This copies the last error into the given `git_error` struct - * and returns 0 if the copy was successful, leaving the error - * cleared as if `giterr_clear` had been called. - * - * If there was no existing error in the library, -1 will be returned - * and the contents of `cpy` will be left unmodified. - */ -GIT_EXTERN(int) giterr_detach(git_error *cpy); - /** * Set the error message string for this thread. * diff --git a/src/errors.c b/src/errors.c index 979602a2a9a..95c62176c73 100644 --- a/src/errors.c +++ b/src/errors.c @@ -116,7 +116,7 @@ void giterr_clear(void) #endif } -int giterr_detach(git_error *cpy) +static int giterr_detach(git_error *cpy) { git_error *error = GIT_GLOBAL->last_error; From 0fcfb60dc4f5e6cfd91c902d844f5d8665a5c1a7 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Mon, 27 Jul 2015 10:10:18 +0100 Subject: [PATCH 55/73] Make giterr_restore aware of g_git_oom_error Allow restoring a previously captured oom error, by detecting when the captured message pointer points to the static oom error message. This means there is no need to strdup the message in giterr_detach. --- src/errors.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/errors.c b/src/errors.c index 95c62176c73..f10430c1016 100644 --- a/src/errors.c +++ b/src/errors.c @@ -125,14 +125,12 @@ static int giterr_detach(git_error *cpy) if (!error) return -1; - if (error == &g_git_oom_error) { - cpy->message = git__strdup(error->message); - } else { - cpy->message = error->message; - error->message = NULL; - } + cpy->message = error->message; cpy->klass = error->klass; + if (error != &g_git_oom_error) { + error->message = NULL; + } giterr_clear(); return 0; @@ -153,8 +151,13 @@ int giterr_capture(git_error_state *state, int error_code) int giterr_restore(git_error_state *state) { - if (state && state->error_code && state->error_msg.message) - set_error(state->error_msg.klass, state->error_msg.message); + if (state && state->error_code && state->error_msg.message) { + if (state->error_msg.message == g_git_oom_error.message) { + giterr_set_oom(); + } else { + set_error(state->error_msg.klass, state->error_msg.message); + } + } else giterr_clear(); From 988ea59443b71f4a07b19fff837ccaa1659dbcc0 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Mon, 27 Jul 2015 10:13:49 +0100 Subject: [PATCH 56/73] Test: check restored oom error points to static buffer --- tests/core/errors.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/core/errors.c b/tests/core/errors.c index 6aceb30fd74..25bbbd5f6dc 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -113,22 +113,22 @@ void test_core_errors__restore(void) void test_core_errors__restore_oom(void) { git_error_state err_state = {0}; - const char *static_message = NULL; + const git_error *oom_error = NULL; giterr_clear(); giterr_set_oom(); /* internal fn */ - static_message = giterr_last()->message; + oom_error = giterr_last(); + cl_assert(oom_error); cl_assert_equal_i(-1, giterr_capture(&err_state, -1)); cl_assert(giterr_last() == NULL); - cl_assert_(err_state.error_msg.message != static_message, "pointer to static buffer exposed"); - giterr_restore(&err_state); cl_assert(giterr_last()->klass == GITERR_NOMEMORY); + cl_assert_(giterr_last() == oom_error, "static oom error not restored"); giterr_clear(); } From ef4857c2b3d4a61fd1d840199afc92eaf2e15345 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 3 Aug 2015 16:50:27 -0500 Subject: [PATCH 57/73] errors: tighten up git_error_state OOMs a bit more When an error state is an OOM, make sure that we treat is specially and do not try to free it. --- src/clone.c | 4 +-- src/common.h | 13 +++++--- src/errors.c | 72 ++++++++++++++++++++++++++------------------- src/index.c | 6 ++-- src/iterator.c | 4 +-- tests/core/errors.c | 33 +++++++++++++++++---- 6 files changed, 86 insertions(+), 46 deletions(-) diff --git a/src/clone.c b/src/clone.c index 070daf94dd2..6b4b7ae530c 100644 --- a/src/clone.c +++ b/src/clone.c @@ -440,14 +440,14 @@ int git_clone( if (error != 0) { git_error_state last_error = {0}; - giterr_capture(&last_error, error); + giterr_state_capture(&last_error, error); git_repository_free(repo); repo = NULL; (void)git_futils_rmdir_r(local_path, NULL, rmdir_flags); - giterr_restore(&last_error); + giterr_state_restore(&last_error); } *out = repo; diff --git a/src/common.h b/src/common.h index 2b1dedc0146..6dca36fbd4d 100644 --- a/src/common.h +++ b/src/common.h @@ -141,20 +141,25 @@ void giterr_system_set(int code); * Structure to preserve libgit2 error state */ typedef struct { - int error_code; + int error_code; + unsigned int oom : 1; git_error error_msg; } git_error_state; /** * Capture current error state to restore later, returning error code. - * If `error_code` is zero, this does nothing and returns zero. + * If `error_code` is zero, this does not clear the current error state. + * You must either restore this error state, or free it. */ -int giterr_capture(git_error_state *state, int error_code); +extern int giterr_state_capture(git_error_state *state, int error_code); /** * Restore error state to a previous value, returning saved error code. */ -int giterr_restore(git_error_state *state); +extern int giterr_state_restore(git_error_state *state); + +/** Free an error state. */ +extern void giterr_state_free(git_error_state *state); /** * Check a versioned structure for validity diff --git a/src/errors.c b/src/errors.c index 973326c00cd..91acc354146 100644 --- a/src/errors.c +++ b/src/errors.c @@ -131,53 +131,65 @@ void giterr_clear(void) #endif } -static int giterr_detach(git_error *cpy) +const git_error *giterr_last(void) +{ + return GIT_GLOBAL->last_error; +} + +int giterr_state_capture(git_error_state *state, int error_code) { git_error *error = GIT_GLOBAL->last_error; - git_buf *buf = &GIT_GLOBAL->error_buf; + git_buf *error_buf = &GIT_GLOBAL->error_buf; - assert(cpy); + memset(state, 0, sizeof(git_error_state)); - if (!error) - return -1; + if (!error_code) + return 0; + + state->error_code = error_code; + state->oom = (error == &g_git_oom_error); - cpy->message = git_buf_detach(buf); - cpy->klass = error->klass; + if (error) { + state->error_msg.klass = error->klass; - if (error != &g_git_oom_error) { - error->message = NULL; + if (state->oom) + state->error_msg.message = g_git_oom_error.message; + else + state->error_msg.message = git_buf_detach(error_buf); } - giterr_clear(); - return 0; + giterr_clear(); + return error_code; } -const git_error *giterr_last(void) +int giterr_state_restore(git_error_state *state) { - return GIT_GLOBAL->last_error; -} + int ret = 0; -int giterr_capture(git_error_state *state, int error_code) -{ - state->error_code = error_code; - if (error_code) - giterr_detach(&state->error_msg); - return error_code; -} + giterr_clear(); -int giterr_restore(git_error_state *state) -{ - if (state && state->error_code && state->error_msg.message) { - if (state->error_msg.message == g_git_oom_error.message) { + if (state && state->error_msg.message) { + if (state->oom) giterr_set_oom(); - } else { + else set_error(state->error_msg.klass, state->error_msg.message); - } + + ret = state->error_code; + memset(state, 0, sizeof(git_error_state)); } - else - giterr_clear(); - return state ? state->error_code : 0; + return ret; +} + +void giterr_state_free(git_error_state *state) +{ + if (!state) + return; + + if (!state->oom) + git__free(state->error_msg.message); + + memset(state, 0, sizeof(git_error_state)); } int giterr_system_last(void) diff --git a/src/index.c b/src/index.c index 73f0b3d2669..e424698bb96 100644 --- a/src/index.c +++ b/src/index.c @@ -1286,13 +1286,13 @@ int git_index_add_bypath(git_index *index, const char *path) git_submodule *sm; git_error_state err; - giterr_capture(&err, ret); + giterr_state_capture(&err, ret); ret = git_submodule_lookup(&sm, INDEX_OWNER(index), path); if (ret == GIT_ENOTFOUND) - return giterr_restore(&err); + return giterr_state_restore(&err); - git__free(err.error_msg.message); + giterr_state_free(&err); /* * EEXISTS means that there is a repository at that path, but it's not known diff --git a/src/iterator.c b/src/iterator.c index cf51a340db8..900ffdcaa8b 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1120,7 +1120,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi) if (error < 0) { git_error_state last_error = { 0 }; - giterr_capture(&last_error, error); + giterr_state_capture(&last_error, error); /* these callbacks may clear the error message */ fs_iterator__free_frame(ff); @@ -1128,7 +1128,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi) /* next time return value we skipped to */ fi->base.flags &= ~GIT_ITERATOR_FIRST_ACCESS; - return giterr_restore(&last_error); + return giterr_state_restore(&last_error); } if (ff->entries.length == 0) { diff --git a/tests/core/errors.c b/tests/core/errors.c index 25bbbd5f6dc..ab18951a667 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -93,23 +93,44 @@ void test_core_errors__restore(void) giterr_clear(); cl_assert(giterr_last() == NULL); - cl_assert_equal_i(0, giterr_capture(&err_state, 0)); + cl_assert_equal_i(0, giterr_state_capture(&err_state, 0)); memset(&err_state, 0x0, sizeof(git_error_state)); giterr_set(42, "Foo: %s", "bar"); - cl_assert_equal_i(-1, giterr_capture(&err_state, -1)); + cl_assert_equal_i(-1, giterr_state_capture(&err_state, -1)); cl_assert(giterr_last() == NULL); giterr_set(99, "Bar: %s", "foo"); - giterr_restore(&err_state); + giterr_state_restore(&err_state); cl_assert_equal_i(42, giterr_last()->klass); cl_assert_equal_s("Foo: bar", giterr_last()->message); } +void test_core_errors__free_state(void) +{ + git_error_state err_state = {0}; + + giterr_clear(); + + giterr_set(42, "Foo: %s", "bar"); + cl_assert_equal_i(-1, giterr_state_capture(&err_state, -1)); + + giterr_set(99, "Bar: %s", "foo"); + + giterr_state_free(&err_state); + + cl_assert_equal_i(99, giterr_last()->klass); + cl_assert_equal_s("Bar: foo", giterr_last()->message); + + giterr_state_restore(&err_state); + + cl_assert(giterr_last() == NULL); +} + void test_core_errors__restore_oom(void) { git_error_state err_state = {0}; @@ -121,11 +142,13 @@ void test_core_errors__restore_oom(void) oom_error = giterr_last(); cl_assert(oom_error); - cl_assert_equal_i(-1, giterr_capture(&err_state, -1)); + cl_assert_equal_i(-1, giterr_state_capture(&err_state, -1)); cl_assert(giterr_last() == NULL); + cl_assert_equal_i(GITERR_NOMEMORY, err_state.error_msg.klass); + cl_assert_equal_s("Out of memory", err_state.error_msg.message); - giterr_restore(&err_state); + giterr_state_restore(&err_state); cl_assert(giterr_last()->klass == GITERR_NOMEMORY); cl_assert_(giterr_last() == oom_error, "static oom error not restored"); From eba784d24d12ad7243708d97d80427bbdc9fd2b7 Mon Sep 17 00:00:00 2001 From: John Haley Date: Wed, 5 Aug 2015 10:19:06 -0700 Subject: [PATCH 58/73] Fix duplicate basenames to support older VS With Visual Studio versions 2008 and older they ignore the full path to files and only check the basename of the file to find a collision. Additionally, having duplicate basenames can break other build tools like GYP. This fixes https://github.com/libgit2/libgit2/issues/3356 --- src/path.c | 2 +- src/util.c | 2 +- src/win32/{buffer.c => w32_buffer.c} | 3 +-- src/win32/{buffer.h => w32_buffer.h} | 0 4 files changed, 3 insertions(+), 4 deletions(-) rename src/win32/{buffer.c => w32_buffer.c} (98%) rename src/win32/{buffer.h => w32_buffer.h} (100%) diff --git a/src/path.c b/src/path.c index 8317aaaa7ca..9ce5d297894 100644 --- a/src/path.c +++ b/src/path.c @@ -10,7 +10,7 @@ #include "repository.h" #ifdef GIT_WIN32 #include "win32/posix.h" -#include "win32/buffer.h" +#include "win32/w32_buffer.h" #include "win32/w32_util.h" #include "win32/version.h" #else diff --git a/src/util.c b/src/util.c index b08b2b884ee..b3929bca232 100644 --- a/src/util.c +++ b/src/util.c @@ -11,7 +11,7 @@ #include "posix.h" #ifdef GIT_WIN32 -# include "win32/buffer.h" +# include "win32/w32_buffer.h" #endif #ifdef _MSC_VER diff --git a/src/win32/buffer.c b/src/win32/w32_buffer.c similarity index 98% rename from src/win32/buffer.c rename to src/win32/w32_buffer.c index 74950189e10..9122baaa684 100644 --- a/src/win32/buffer.c +++ b/src/win32/w32_buffer.c @@ -6,7 +6,7 @@ */ #include "common.h" -#include "buffer.h" +#include "w32_buffer.h" #include "../buffer.h" #include "utf-conv.h" @@ -52,4 +52,3 @@ int git_buf_put_w(git_buf *buf, const wchar_t *string_w, size_t len_w) buf->ptr[buf->size] = '\0'; return 0; } - diff --git a/src/win32/buffer.h b/src/win32/w32_buffer.h similarity index 100% rename from src/win32/buffer.h rename to src/win32/w32_buffer.h From c27b4afcdd80f5a45d7120044bf3d78272181abb Mon Sep 17 00:00:00 2001 From: Slava Karpenko Date: Thu, 6 Aug 2015 11:06:17 +0300 Subject: [PATCH 59/73] Forcing libssh2 lib location OS X may have libssh2 in diff locations, so CHECK_LIBRARY_EXISTS may check the wrong lib; forcing it to use a found directory. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a15ce7dbd7b..b6a8b6d4ec3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -286,7 +286,7 @@ IF (LIBSSH2_FOUND) #SET(LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS} ${LIBSSH2_LDFLAGS}") SET(SSH_LIBRARIES ${LIBSSH2_LIBRARIES}) - CHECK_LIBRARY_EXISTS("${LIBSSH2_LIBRARIES}" libssh2_userauth_publickey_frommemory "" HAVE_LIBSSH2_MEMORY_CREDENTIALS) + CHECK_LIBRARY_EXISTS("${LIBSSH2_LIBRARIES}" libssh2_userauth_publickey_frommemory "${LIBSSH2_LIBRARY_DIRS}" HAVE_LIBSSH2_MEMORY_CREDENTIALS) IF (HAVE_LIBSSH2_MEMORY_CREDENTIALS) ADD_DEFINITIONS(-DGIT_SSH_MEMORY_CREDENTIALS) ENDIF() From dc0351893accfc94911cf7067f2b96736675a419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 6 Aug 2015 13:02:35 +0200 Subject: [PATCH 60/73] curl: use the most secure auth method for the proxy When curl uses a proxy, it will only use Basic unless we prompt it to try to use the most secure on it has available. This is something which git did recently, and it seems like a good idea. --- src/curl_stream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/curl_stream.c b/src/curl_stream.c index 63421fcf7a8..798bd5a5255 100644 --- a/src/curl_stream.c +++ b/src/curl_stream.c @@ -220,6 +220,7 @@ int git_curl_stream_new(git_stream **out, const char *host, const char *port) curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1); curl_easy_setopt(handle, CURLOPT_CERTINFO, 1); curl_easy_setopt(handle, CURLOPT_HTTPPROXYTUNNEL, 1); + curl_easy_setopt(handle, CURLOPT_PROXYAUTH, CURLAUTH_ANY); /* curl_easy_setopt(handle, CURLOPT_VERBOSE, 1); */ From a879276783de5cc2c82543a9f930337f000aa8e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 11 Aug 2015 20:44:19 +0200 Subject: [PATCH 61/73] remote: add failing test for a mirror refspec While we download the remote's remote-tracking branches, we don't download the tag. This points to the tag auto-follow rules interfering with the refspec. --- tests/network/fetchlocal.c | 39 +++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/network/fetchlocal.c b/tests/network/fetchlocal.c index 06ee3dd36a1..17c8f26e333 100644 --- a/tests/network/fetchlocal.c +++ b/tests/network/fetchlocal.c @@ -369,17 +369,46 @@ void test_network_fetchlocal__clone_into_mirror(void) { git_clone_options opts = GIT_CLONE_OPTIONS_INIT; git_repository *repo; - git_reference *head; + git_reference *ref; opts.bare = true; opts.remote_cb = remote_mirror_cb; cl_git_pass(git_clone(&repo, cl_git_fixture_url("testrepo.git"), "./foo.git", &opts)); - cl_git_pass(git_reference_lookup(&head, repo, "HEAD")); - cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head)); - cl_assert_equal_s("refs/heads/master", git_reference_symbolic_target(head)); + cl_git_pass(git_reference_lookup(&ref, repo, "HEAD")); + cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(ref)); + cl_assert_equal_s("refs/heads/master", git_reference_symbolic_target(ref)); + + git_reference_free(ref); + cl_git_pass(git_reference_lookup(&ref, repo, "refs/remotes/test/master")); + + git_reference_free(ref); + git_repository_free(repo); + cl_fixture_cleanup("./foo.git"); +} - git_reference_free(head); +void test_network_fetchlocal__all_refs(void) +{ + git_repository *repo; + git_remote *remote; + git_reference *ref; + char *allrefs = "+refs/*:refs/*"; + git_strarray refspecs = { + &allrefs, + 1, + }; + + cl_git_pass(git_repository_init(&repo, "./foo.git", true)); + cl_git_pass(git_remote_create_anonymous(&remote, repo, cl_git_fixture_url("testrepo.git"))); + cl_git_pass(git_remote_fetch(remote, &refspecs, NULL, NULL)); + + cl_git_pass(git_reference_lookup(&ref, repo, "refs/remotes/test/master")); + git_reference_free(ref); + + cl_git_pass(git_reference_lookup(&ref, repo, "refs/tags/test")); + git_reference_free(ref); + + git_remote_free(remote); git_repository_free(repo); cl_fixture_cleanup("./foo.git"); } From e3e017d4839893cb6e5339b063afafa60b56bf0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 11 Aug 2015 22:51:53 +0200 Subject: [PATCH 62/73] remote: don't confuse tag auto-follow rules with refspec matching When we're looking to update a tag, we can't stop if the tag auto-follow rules don't say to update it. The tag might still match the refspec we were given. --- src/remote.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/remote.c b/src/remote.c index d31e1b89ecb..7404bf49f53 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1334,11 +1334,13 @@ static int update_tips_for_spec( for (; i < refs->length; ++i) { head = git_vector_get(refs, i); autotag = 0; + git_buf_clear(&refname); /* Ignore malformed ref names (which also saves us from tag^{} */ if (!git_reference_is_valid_name(head->name)) continue; + /* If we have a tag, see if the auto-follow rules say to update it */ if (git_refspec_src_matches(&tagspec, head->name)) { if (tagopt != GIT_REMOTE_DOWNLOAD_TAGS_NONE) { @@ -1348,10 +1350,11 @@ static int update_tips_for_spec( git_buf_clear(&refname); if (git_buf_puts(&refname, head->name) < 0) goto on_error; - } else { - continue; } - } else if (git_refspec_src_matches(spec, head->name)) { + } + + /* If we didn't want to auto-follow the tag, check if the refspec matches */ + if (!autotag && git_refspec_src_matches(spec, head->name)) { if (spec->dst) { if (git_refspec_transform(&refname, spec, head->name) < 0) goto on_error; @@ -1365,7 +1368,10 @@ static int update_tips_for_spec( continue; } - } else { + } + + /* If we still don't have a refname, we don't want it */ + if (git_buf_len(&refname) == 0) { continue; } From 3ce9e4d23572718deeab438ce149013eece57371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 1 Jun 2015 08:45:15 +0200 Subject: [PATCH 63/73] config: write the modified file to memory Instead of writing into the filebuf directly, make the functions to write the modified config file write into a buffer which can then be dumped into the lockfile for committing. This allows us to re-use the same code for modifying a locked configuration, as we can simply skip the last step of dumping the data to disk. --- src/config_file.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 52a5376bd9a..fd03c200f2a 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -1602,7 +1602,7 @@ static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct re return config_parse(reader, NULL, read_on_variable, NULL, NULL, &parse_data); } -static int write_section(git_filebuf *file, const char *key) +static int write_section(git_buf *fbuf, const char *key) { int result; const char *dot; @@ -1626,7 +1626,7 @@ static int write_section(git_filebuf *file, const char *key) if (git_buf_oom(&buf)) return -1; - result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size); + result = git_buf_put(fbuf, git_buf_cstr(&buf), buf.size); git_buf_free(&buf); return result; @@ -1651,7 +1651,7 @@ static const char *quotes_for_value(const char *value) } struct write_data { - git_filebuf *file; + git_buf *buf; unsigned int in_section : 1, preg_replaced : 1; const char *section; @@ -1662,10 +1662,10 @@ struct write_data { static int write_line(struct write_data *write_data, const char *line, size_t line_len) { - int result = git_filebuf_write(write_data->file, line, line_len); + int result = git_buf_put(write_data->buf, line, line_len); if (!result && line_len && line[line_len-1] != '\n') - result = git_filebuf_printf(write_data->file, "\n"); + result = git_buf_printf(write_data->buf, "\n"); return result; } @@ -1676,7 +1676,7 @@ static int write_value(struct write_data *write_data) int result; q = quotes_for_value(write_data->value); - result = git_filebuf_printf(write_data->file, + result = git_buf_printf(write_data->buf, "\t%s = %s%s%s\n", write_data->name, q, write_data->value, q); /* If we are updating a single name/value, we're done. Setting `value` @@ -1782,7 +1782,7 @@ static int write_on_eof(struct reader **reader, void *data) * value. */ if ((!write_data->preg || !write_data->preg_replaced) && write_data->value) { - if ((result = write_section(write_data->file, write_data->section)) == 0) + if ((result = write_section(write_data->buf, write_data->section)) == 0) result = write_value(write_data); } @@ -1797,6 +1797,7 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p int result; char *section, *name, *ldot; git_filebuf file = GIT_FILEBUF_INIT; + git_buf buf = GIT_BUF_INIT; struct reader *reader = git_array_get(cfg->readers, 0); struct write_data write_data; @@ -1827,7 +1828,7 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p name = ldot + 1; section = git__strndup(key, ldot - key); - write_data.file = &file; + write_data.buf = &buf; write_data.section = section; write_data.in_section = 0; write_data.preg_replaced = 0; @@ -1839,10 +1840,13 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p git__free(section); if (result < 0) { + git_buf_free(&buf); git_filebuf_cleanup(&file); goto done; } + git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf)); + /* refresh stats - if this errors, then commit will error too */ (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); From b1667039640ba3464ea0e2d13ad28c9244d80b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 1 Jun 2015 19:17:03 +0200 Subject: [PATCH 64/73] config: implement basic transactional support When a configuration file is locked, any updates made to it will be done to the in-memory copy of the file. This allows for multiple updates to happen while we hold the lock, preventing races during complex config-file manipulation. --- include/git2/sys/config.h | 14 ++++++ src/config_file.c | 93 ++++++++++++++++++++++++++++++++++----- src/config_file.h | 10 +++++ tests/config/write.c | 46 +++++++++++++++++++ 4 files changed, 151 insertions(+), 12 deletions(-) diff --git a/include/git2/sys/config.h b/include/git2/sys/config.h index 044e34417ed..4dad6da42e1 100644 --- a/include/git2/sys/config.h +++ b/include/git2/sys/config.h @@ -67,6 +67,20 @@ struct git_config_backend { int (*iterator)(git_config_iterator **, struct git_config_backend *); /** Produce a read-only version of this backend */ int (*snapshot)(struct git_config_backend **, struct git_config_backend *); + /** + * Lock this backend. + * + * Prevent any writes to the data store backing this + * backend. Any updates must not be visible to any other + * readers. + */ + int (*lock)(struct git_config_backend *); + /** + * Unlock the data store backing this backend. If success is + * true, the changes should be committed, otherwise rolled + * back. + */ + int (*unlock)(struct git_config_backend *, int success); void (*free)(struct git_config_backend *); }; #define GIT_CONFIG_BACKEND_VERSION 1 diff --git a/src/config_file.c b/src/config_file.c index fd03c200f2a..a3fec1b349a 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -105,6 +105,10 @@ typedef struct { git_array_t(struct reader) readers; + bool locked; + git_filebuf locked_buf; + git_buf locked_content; + char *file_path; } diskfile_backend; @@ -685,6 +689,42 @@ static int config_snapshot(git_config_backend **out, git_config_backend *in) return git_config_file__snapshot(out, b); } +static int config_lock(git_config_backend *_cfg) +{ + diskfile_backend *cfg = (diskfile_backend *) _cfg; + int error; + + if ((error = git_filebuf_open(&cfg->locked_buf, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) + return error; + + error = git_futils_readbuffer(&cfg->locked_content, cfg->file_path); + if (error < 0 && error != GIT_ENOTFOUND) { + git_filebuf_cleanup(&cfg->locked_buf); + return error; + } + + cfg->locked = true; + return 0; + +} + +static int config_unlock(git_config_backend *_cfg, int success) +{ + diskfile_backend *cfg = (diskfile_backend *) _cfg; + int error = 0; + + if (success) { + git_filebuf_write(&cfg->locked_buf, cfg->locked_content.ptr, cfg->locked_content.size); + error = git_filebuf_commit(&cfg->locked_buf); + } + + git_filebuf_cleanup(&cfg->locked_buf); + git_buf_free(&cfg->locked_content); + cfg->locked = false; + + return error; +} + int git_config_file__ondisk(git_config_backend **out, const char *path) { diskfile_backend *backend; @@ -706,6 +746,8 @@ int git_config_file__ondisk(git_config_backend **out, const char *path) backend->header.parent.del_multivar = config_delete_multivar; backend->header.parent.iterator = config_iterator_new; backend->header.parent.snapshot = config_snapshot; + backend->header.parent.lock = config_lock; + backend->header.parent.unlock = config_unlock; backend->header.parent.free = backend_free; *out = (git_config_backend *)backend; @@ -750,6 +792,21 @@ static int config_delete_readonly(git_config_backend *cfg, const char *name) return config_error_readonly(); } +static int config_lock_readonly(git_config_backend *_cfg) +{ + GIT_UNUSED(_cfg); + + return config_error_readonly(); +} + +static int config_unlock_readonly(git_config_backend *_cfg, int success) +{ + GIT_UNUSED(_cfg); + GIT_UNUSED(success); + + return config_error_readonly(); +} + static void backend_readonly_free(git_config_backend *_backend) { diskfile_backend *backend = (diskfile_backend *)_backend; @@ -803,6 +860,8 @@ int git_config_file__snapshot(git_config_backend **out, diskfile_backend *in) backend->header.parent.del = config_delete_readonly; backend->header.parent.del_multivar = config_delete_multivar_readonly; backend->header.parent.iterator = config_iterator_new; + backend->header.parent.lock = config_lock_readonly; + backend->header.parent.unlock = config_unlock_readonly; backend->header.parent.free = backend_readonly_free; *out = (git_config_backend *)backend; @@ -1801,15 +1860,19 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p struct reader *reader = git_array_get(cfg->readers, 0); struct write_data write_data; - /* Lock the file */ - if ((result = git_filebuf_open( - &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) { + if (cfg->locked) { + result = git_buf_puts(&reader->buffer, git_buf_cstr(&cfg->locked_content)); + } else { + /* Lock the file */ + if ((result = git_filebuf_open( + &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) { git_buf_free(&reader->buffer); return result; - } + } - /* We need to read in our own config file */ - result = git_futils_readbuffer(&reader->buffer, cfg->file_path); + /* We need to read in our own config file */ + result = git_futils_readbuffer(&reader->buffer, cfg->file_path); + } /* Initialise the reading position */ if (result == GIT_ENOTFOUND) { @@ -1840,20 +1903,26 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p git__free(section); if (result < 0) { - git_buf_free(&buf); git_filebuf_cleanup(&file); goto done; } - git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf)); + if (cfg->locked) { + size_t len = buf.asize; + /* Update our copy with the modified contents */ + git_buf_free(&cfg->locked_content); + git_buf_attach(&cfg->locked_content, git_buf_detach(&buf), len); + } else { + git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf)); - /* refresh stats - if this errors, then commit will error too */ - (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); + /* refresh stats - if this errors, then commit will error too */ + (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); - result = git_filebuf_commit(&file); - git_buf_free(&reader->buffer); + result = git_filebuf_commit(&file); + } done: + git_buf_free(&buf); git_buf_free(&reader->buffer); return result; } diff --git a/src/config_file.h b/src/config_file.h index 0d8bf740f54..1c52892c328 100644 --- a/src/config_file.h +++ b/src/config_file.h @@ -55,6 +55,16 @@ GIT_INLINE(int) git_config_file_foreach_match( return git_config_backend_foreach_match(cfg, regexp, fn, data); } +GIT_INLINE(int) git_config_file_lock(git_config_backend *cfg) +{ + return cfg->lock(cfg); +} + +GIT_INLINE(int) git_config_file_unlock(git_config_backend *cfg, int success) +{ + return cfg->unlock(cfg, success); +} + extern int git_config_file_normalize_section(char *start, char *end); #endif diff --git a/tests/config/write.c b/tests/config/write.c index 2e7b8182ac5..5446b95c31f 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -1,6 +1,9 @@ #include "clar_libgit2.h" #include "buffer.h" #include "fileops.h" +#include "git2/sys/config.h" +#include "config_file.h" +#include "config.h" void test_config_write__initialize(void) { @@ -630,3 +633,46 @@ void test_config_write__to_file_with_only_comment(void) git_buf_free(&result); } +void test_config_write__locking(void) +{ + git_config_backend *cfg, *cfg2; + git_config_entry *entry; + const char *filename = "locked-file"; + + /* Open the config and lock it */ + cl_git_mkfile(filename, "[section]\n\tname = value\n"); + cl_git_pass(git_config_file__ondisk(&cfg, filename)); + cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_assert_equal_s("value", entry->value); + git_config_entry_free(entry); + cl_git_pass(git_config_file_lock(cfg)); + + /* Change entries in the locked backend */ + cl_git_pass(git_config_file_set_string(cfg, "section.name", "other value")); + cl_git_pass(git_config_file_set_string(cfg, "section2.name3", "more value")); + + /* We can see that the file we read from hasn't changed */ + cl_git_pass(git_config_file__ondisk(&cfg2, filename)); + cl_git_pass(git_config_file_open(cfg2, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg2, "section.name")); + cl_assert_equal_s("value", entry->value); + git_config_entry_free(entry); + cl_git_fail_with(GIT_ENOTFOUND, git_config_file_get_string(&entry, cfg2, "section2.name3")); + git_config_file_free(cfg2); + + git_config_file_unlock(cfg, true); + git_config_file_free(cfg); + + /* Now that we've unlocked it, we should see both updates */ + cl_git_pass(git_config_file__ondisk(&cfg, filename)); + cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_assert_equal_s("other value", entry->value); + git_config_entry_free(entry); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section2.name3")); + cl_assert_equal_s("more value", entry->value); + git_config_entry_free(entry); + + git_config_file_free(cfg); +} From 36f784b538c4b27f7b52427d2cfce06c535abba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 1 Jun 2015 20:02:23 +0200 Subject: [PATCH 65/73] config: expose locking via the main API This lock/unlock pair allows for the cller to lock a configuration file to avoid concurrent operations. It also allows for a transactional approach to updating a configuration file. If multiple updates must be made atomically, they can be done while the config is locked. --- CHANGELOG.md | 10 ++++++++++ include/git2/config.h | 27 +++++++++++++++++++++++++++ src/config.c | 31 +++++++++++++++++++++++++++++++ tests/config/write.c | 41 ++++++++++++++++++++++------------------- 4 files changed, 90 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 243b696d723..4442d0a3f3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ v0.23 + 1 ### API additions +* `git_config_lock()` and `git_config_unlock()` have been added, which + allow for transactional/atomic complex updates to the configuration, + removing the opportunity for concurrent operations and not + committing any changes until the unlock. + + ### API removals ### Breaking API changes @@ -19,6 +25,10 @@ v0.23 + 1 with the reflog on ref deletion. The file-based backend must delete it, a database-backed one may wish to archive it. +* `git_config_backend` has gained two entries. `lock` and `unlock` + with which to implement the transactional/atomic semantics for the + configuration backend. + v0.23 ------ diff --git a/include/git2/config.h b/include/git2/config.h index 6d3fdb0c2eb..2550f8edbd2 100644 --- a/include/git2/config.h +++ b/include/git2/config.h @@ -689,6 +689,33 @@ GIT_EXTERN(int) git_config_backend_foreach_match( void *payload); +/** + * Lock the backend with the highest priority + * + * Locking disallows anybody else from writing to that backend. Any + * updates made after locking will not be visible to a reader until + * the file is unlocked. + * + * @param cfg the configuration in which to lock + * @return 0 or an error code + */ +GIT_EXTERN(int) git_config_lock(git_config *cfg); + +/** + * Unlock the backend with the highest priority + * + * Unlocking will allow other writers to updat the configuration + * file. Optionally, any changes performed since the lock will be + * applied to the configuration. + * + * @param cfg the configuration + * @param commit boolean which indicates whether to commit any changes + * done since locking + * @return 0 or an error code + */ +GIT_EXTERN(int) git_config_unlock(git_config *cfg, int commit); + + /** @} */ GIT_END_DECL #endif diff --git a/src/config.c b/src/config.c index 77cf573e657..937d00dcb6e 100644 --- a/src/config.c +++ b/src/config.c @@ -1144,6 +1144,37 @@ int git_config_open_default(git_config **out) return error; } +int git_config_lock(git_config *cfg) +{ + git_config_backend *file; + file_internal *internal; + + internal = git_vector_get(&cfg->files, 0); + if (!internal || !internal->file) { + giterr_set(GITERR_CONFIG, "cannot lock; the config has no backends/files"); + return -1; + } + file = internal->file; + + return file->lock(file); +} + +int git_config_unlock(git_config *cfg, int commit) +{ + git_config_backend *file; + file_internal *internal; + + internal = git_vector_get(&cfg->files, 0); + if (!internal || !internal->file) { + giterr_set(GITERR_CONFIG, "cannot lock; the config has no backends/files"); + return -1; + } + + file = internal->file; + + return file->unlock(file, commit); +} + /*********** * Parsers ***********/ diff --git a/tests/config/write.c b/tests/config/write.c index 5446b95c31f..e43c26bd9c1 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -635,44 +635,47 @@ void test_config_write__to_file_with_only_comment(void) void test_config_write__locking(void) { - git_config_backend *cfg, *cfg2; + git_config *cfg, *cfg2; git_config_entry *entry; const char *filename = "locked-file"; /* Open the config and lock it */ cl_git_mkfile(filename, "[section]\n\tname = value\n"); - cl_git_pass(git_config_file__ondisk(&cfg, filename)); - cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); - cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_git_pass(git_config_open_ondisk(&cfg, filename)); + cl_git_pass(git_config_get_entry(&entry, cfg, "section.name")); cl_assert_equal_s("value", entry->value); git_config_entry_free(entry); - cl_git_pass(git_config_file_lock(cfg)); + cl_git_pass(git_config_lock(cfg)); /* Change entries in the locked backend */ - cl_git_pass(git_config_file_set_string(cfg, "section.name", "other value")); - cl_git_pass(git_config_file_set_string(cfg, "section2.name3", "more value")); + cl_git_pass(git_config_set_string(cfg, "section.name", "other value")); + cl_git_pass(git_config_set_string(cfg, "section2.name3", "more value")); /* We can see that the file we read from hasn't changed */ - cl_git_pass(git_config_file__ondisk(&cfg2, filename)); - cl_git_pass(git_config_file_open(cfg2, GIT_CONFIG_LEVEL_APP)); - cl_git_pass(git_config_file_get_string(&entry, cfg2, "section.name")); + cl_git_pass(git_config_open_ondisk(&cfg2, filename)); + cl_git_pass(git_config_get_entry(&entry, cfg2, "section.name")); cl_assert_equal_s("value", entry->value); git_config_entry_free(entry); - cl_git_fail_with(GIT_ENOTFOUND, git_config_file_get_string(&entry, cfg2, "section2.name3")); - git_config_file_free(cfg2); + cl_git_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg2, "section2.name3")); + git_config_free(cfg2); - git_config_file_unlock(cfg, true); - git_config_file_free(cfg); + /* And we also get the old view when we read from the locked config */ + cl_git_pass(git_config_get_entry(&entry, cfg, "section.name")); + cl_assert_equal_s("value", entry->value); + git_config_entry_free(entry); + cl_git_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg, "section2.name3")); + + git_config_unlock(cfg, true); + git_config_free(cfg); /* Now that we've unlocked it, we should see both updates */ - cl_git_pass(git_config_file__ondisk(&cfg, filename)); - cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); - cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_git_pass(git_config_open_ondisk(&cfg, filename)); + cl_git_pass(git_config_get_entry(&entry, cfg, "section.name")); cl_assert_equal_s("other value", entry->value); git_config_entry_free(entry); - cl_git_pass(git_config_file_get_string(&entry, cfg, "section2.name3")); + cl_git_pass(git_config_get_entry(&entry, cfg, "section2.name3")); cl_assert_equal_s("more value", entry->value); git_config_entry_free(entry); - git_config_file_free(cfg); + git_config_free(cfg); } From 5340d63d3815ddbd1a7e1b5b9628fce10089e8a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 12 Jul 2015 12:50:23 +0200 Subject: [PATCH 66/73] config: perform unlocking via git_transaction This makes the API for commiting or discarding changes the same as for references. --- CHANGELOG.md | 9 ++++----- include/git2/config.h | 23 +++++++---------------- src/config.c | 8 ++++++-- src/config.h | 15 +++++++++++++++ src/transaction.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/transaction.h | 14 ++++++++++++++ tests/config/write.c | 7 ++++--- 7 files changed, 92 insertions(+), 26 deletions(-) create mode 100644 src/transaction.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 4442d0a3f3f..5dd4b34474a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,10 @@ v0.23 + 1 ### API additions -* `git_config_lock()` and `git_config_unlock()` have been added, which - allow for transactional/atomic complex updates to the configuration, - removing the opportunity for concurrent operations and not - committing any changes until the unlock. - +* `git_config_lock()` has been added, which allow for + transactional/atomic complex updates to the configuration, removing + the opportunity for concurrent operations and not committing any + changes until the unlock. ### API removals diff --git a/include/git2/config.h b/include/git2/config.h index 2550f8edbd2..05c3ad622bb 100644 --- a/include/git2/config.h +++ b/include/git2/config.h @@ -696,25 +696,16 @@ GIT_EXTERN(int) git_config_backend_foreach_match( * updates made after locking will not be visible to a reader until * the file is unlocked. * - * @param cfg the configuration in which to lock - * @return 0 or an error code - */ -GIT_EXTERN(int) git_config_lock(git_config *cfg); - -/** - * Unlock the backend with the highest priority + * You can apply the changes by calling `git_transaction_commit()` + * before freeing the transaction. Either of these actions will unlock + * the config. * - * Unlocking will allow other writers to updat the configuration - * file. Optionally, any changes performed since the lock will be - * applied to the configuration. - * - * @param cfg the configuration - * @param commit boolean which indicates whether to commit any changes - * done since locking + * @param tx the resulting transaction, use this to commit or undo the + * changes + * @param cfg the configuration in which to lock * @return 0 or an error code */ -GIT_EXTERN(int) git_config_unlock(git_config *cfg, int commit); - +GIT_EXTERN(int) git_config_lock(git_transaction **tx, git_config *cfg); /** @} */ GIT_END_DECL diff --git a/src/config.c b/src/config.c index 937d00dcb6e..2df1fc80e30 100644 --- a/src/config.c +++ b/src/config.c @@ -1144,8 +1144,9 @@ int git_config_open_default(git_config **out) return error; } -int git_config_lock(git_config *cfg) +int git_config_lock(git_transaction **out, git_config *cfg) { + int error; git_config_backend *file; file_internal *internal; @@ -1156,7 +1157,10 @@ int git_config_lock(git_config *cfg) } file = internal->file; - return file->lock(file); + if ((error = file->lock(file)) < 0) + return error; + + return git_transaction_config_new(out, cfg); } int git_config_unlock(git_config *cfg, int commit) diff --git a/src/config.h b/src/config.h index f257cc90faf..ba745331a02 100644 --- a/src/config.h +++ b/src/config.h @@ -88,4 +88,19 @@ extern int git_config__cvar( */ int git_config_lookup_map_enum(git_cvar_t *type_out, const char **str_out, const git_cvar_map *maps, size_t map_n, int enum_val); + +/** + * Unlock the backend with the highest priority + * + * Unlocking will allow other writers to updat the configuration + * file. Optionally, any changes performed since the lock will be + * applied to the configuration. + * + * @param cfg the configuration + * @param commit boolean which indicates whether to commit any changes + * done since locking + * @return 0 or an error code + */ +GIT_EXTERN(int) git_config_unlock(git_config *cfg, int commit); + #endif diff --git a/src/transaction.c b/src/transaction.c index e8331891cb7..e9639bf970e 100644 --- a/src/transaction.c +++ b/src/transaction.c @@ -12,6 +12,7 @@ #include "pool.h" #include "reflog.h" #include "signature.h" +#include "config.h" #include "git2/transaction.h" #include "git2/signature.h" @@ -20,6 +21,12 @@ GIT__USE_STRMAP +typedef enum { + TRANSACTION_NONE, + TRANSACTION_REFS, + TRANSACTION_CONFIG, +} transaction_t; + typedef struct { const char *name; void *payload; @@ -39,13 +46,29 @@ typedef struct { } transaction_node; struct git_transaction { + transaction_t type; git_repository *repo; git_refdb *db; + git_config *cfg; git_strmap *locks; git_pool pool; }; +int git_transaction_config_new(git_transaction **out, git_config *cfg) +{ + git_transaction *tx; + assert(out && cfg); + + tx = git__calloc(1, sizeof(git_transaction)); + GITERR_CHECK_ALLOC(tx); + + tx->type = TRANSACTION_CONFIG; + tx->cfg = cfg; + *out = tx; + return 0; +} + int git_transaction_new(git_transaction **out, git_repository *repo) { int error; @@ -71,6 +94,7 @@ int git_transaction_new(git_transaction **out, git_repository *repo) if ((error = git_repository_refdb(&tx->db, repo)) < 0) goto on_error; + tx->type = TRANSACTION_REFS; memcpy(&tx->pool, &pool, sizeof(git_pool)); tx->repo = repo; *out = tx; @@ -305,6 +329,14 @@ int git_transaction_commit(git_transaction *tx) assert(tx); + if (tx->type == TRANSACTION_CONFIG) { + error = git_config_unlock(tx->cfg, true); + git_config_free(tx->cfg); + tx->cfg = NULL; + + return error; + } + for (pos = kh_begin(tx->locks); pos < kh_end(tx->locks); pos++) { if (!git_strmap_has_data(tx->locks, pos)) continue; @@ -332,6 +364,16 @@ void git_transaction_free(git_transaction *tx) assert(tx); + if (tx->type == TRANSACTION_CONFIG) { + if (tx->cfg) { + git_config_unlock(tx->cfg, false); + git_config_free(tx->cfg); + } + + git__free(tx); + return; + } + /* start by unlocking the ones we've left hanging, if any */ for (pos = kh_begin(tx->locks); pos < kh_end(tx->locks); pos++) { if (!git_strmap_has_data(tx->locks, pos)) diff --git a/src/transaction.h b/src/transaction.h new file mode 100644 index 00000000000..780c068303e --- /dev/null +++ b/src/transaction.h @@ -0,0 +1,14 @@ +/* + * 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_transaction_h__ +#define INCLUDE_transaction_h__ + +#include "common.h" + +int git_transaction_config_new(git_transaction **out, git_config *cfg); + +#endif diff --git a/tests/config/write.c b/tests/config/write.c index e43c26bd9c1..3d9b1a16aa4 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -637,6 +637,7 @@ void test_config_write__locking(void) { git_config *cfg, *cfg2; git_config_entry *entry; + git_transaction *tx; const char *filename = "locked-file"; /* Open the config and lock it */ @@ -645,7 +646,7 @@ void test_config_write__locking(void) cl_git_pass(git_config_get_entry(&entry, cfg, "section.name")); cl_assert_equal_s("value", entry->value); git_config_entry_free(entry); - cl_git_pass(git_config_lock(cfg)); + cl_git_pass(git_config_lock(&tx, cfg)); /* Change entries in the locked backend */ cl_git_pass(git_config_set_string(cfg, "section.name", "other value")); @@ -665,8 +666,8 @@ void test_config_write__locking(void) git_config_entry_free(entry); cl_git_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg, "section2.name3")); - git_config_unlock(cfg, true); - git_config_free(cfg); + cl_git_pass(git_transaction_commit(tx)); + git_transaction_free(tx); /* Now that we've unlocked it, we should see both updates */ cl_git_pass(git_config_open_ondisk(&cfg, filename)); From b0b2c72274efb660e63fcb8f476222bcccb6c680 Mon Sep 17 00:00:00 2001 From: Dan Leehr Date: Thu, 13 Aug 2015 22:52:52 -0400 Subject: [PATCH 67/73] Fix bug in git_smart__push: push_transfer_progress cb is never called The conditional checked cbs->transfer_progress then used the value in cbs->push_transfer_progress. In both cases it should be push_transfer_progress --- src/transports/smart_protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 0920f2eef04..1d46d4bc97a 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -957,7 +957,7 @@ int git_smart__push(git_transport *transport, git_push *push, const git_remote_c packbuilder_payload.pb = push->pb; - if (cbs && cbs->transfer_progress) { + if (cbs && cbs->push_transfer_progress) { packbuilder_payload.cb = cbs->push_transfer_progress; packbuilder_payload.cb_payload = cbs->payload; } From 11bca2d265d7a1136ddf3e34a39263eba6320e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 15 Aug 2015 18:15:23 +0200 Subject: [PATCH 68/73] http: propagate the credentials callback's error code When we ask for credentials, the user may choose to return EUSER to indicate that an error has happened on its end and it wants to be given back control. We must therefore pass that back to the user instead of mentioning that it was on_headers_complete() that returned an error code. Since we can, we return the exact error code from the user (other than PASSTHROUGH) since it doesn't cost anything, though using other error codes aren't recommended. --- src/transports/http.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/transports/http.c b/src/transports/http.c index e3d90de11a4..87f3ee816a8 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -36,6 +36,8 @@ static const char *post_verb = "POST"; #define PARSE_ERROR_GENERIC -1 #define PARSE_ERROR_REPLAY -2 +/** Look at the user field */ +#define PARSE_ERROR_EXT -3 #define CHUNK_SIZE 4096 @@ -78,6 +80,7 @@ typedef struct { git_vector www_authenticate; enum last_cb last_cb; int parse_error; + int error; unsigned parse_finished : 1; /* Authentication */ @@ -351,7 +354,8 @@ static int on_headers_complete(http_parser *parser) if (error == GIT_PASSTHROUGH) { no_callback = 1; } else if (error < 0) { - return PARSE_ERROR_GENERIC; + t->error = error; + return t->parse_error = PARSE_ERROR_EXT; } else { assert(t->cred); @@ -712,6 +716,10 @@ static int http_stream_read( goto replay; } + if (t->parse_error == PARSE_ERROR_EXT) { + return t->error; + } + if (t->parse_error < 0) return -1; From e451cd5c0350aa70e05df44bbf5cbdc0e6504f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 15 Aug 2015 18:46:38 +0200 Subject: [PATCH 69/73] diff: don't error out on an invalid regex When parsing user-provided regex patterns for functions, we must not fail to provide a diff just because a pattern is not well formed. Ignore it instead. --- src/diff_driver.c | 13 ++++++------- tests/diff/drivers.c | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/diff_driver.c b/src/diff_driver.c index 9d133710337..bc3518991c8 100644 --- a/src/diff_driver.c +++ b/src/diff_driver.c @@ -97,8 +97,7 @@ static int diff_driver_add_patterns( for (scan = regex_str; scan; scan = end) { /* get pattern to fill in */ if ((pat = git_array_alloc(drv->fn_patterns)) == NULL) { - error = -1; - break; + return -1; } pat->flags = regex_flags; @@ -117,10 +116,9 @@ static int diff_driver_add_patterns( break; if ((error = regcomp(&pat->re, buf.ptr, regex_flags)) != 0) { - /* if regex fails to compile, warn? fail? */ - error = giterr_set_regex(&pat->re, error); - regfree(&pat->re); - break; + /* + * TODO: issue a warning + */ } } @@ -128,7 +126,8 @@ static int diff_driver_add_patterns( (void)git_array_pop(drv->fn_patterns); /* release last item */ git_buf_free(&buf); - return error; + /* We want to ignore bad patterns, so return success regardless */ + return 0; } static int diff_driver_xfuncname(const git_config_entry *entry, void *payload) diff --git a/tests/diff/drivers.c b/tests/diff/drivers.c index e3a0014dbe1..42af38a9a44 100644 --- a/tests/diff/drivers.c +++ b/tests/diff/drivers.c @@ -250,3 +250,29 @@ void test_diff_drivers__builtins(void) git_buf_free(&expected); git_vector_free(&files); } + +void test_diff_drivers__invalid_pattern(void) +{ + git_config *cfg; + git_index *idx; + git_diff *diff; + git_patch *patch; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + + g_repo = cl_git_sandbox_init("userdiff"); + cl_git_mkfile("userdiff/.gitattributes", "*.storyboard diff=storyboard\n"); + + cl_git_pass(git_repository_config__weakptr(&cfg, g_repo)); + cl_git_pass(git_config_set_string(cfg, "diff.storyboard.xfuncname", "")); + + cl_git_mkfile("userdiff/dummy.storyboard", ""); + cl_git_pass(git_repository_index__weakptr(&idx, g_repo)); + cl_git_pass(git_index_add_bypath(idx, "dummy.storyboard")); + cl_git_mkfile("userdiff/dummy.storyboard", "some content\n"); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + cl_git_pass(git_patch_from_diff(&patch, diff, 0)); + + git_patch_free(patch); + git_diff_free(diff); +} From 26ea28f32bb93d262f39d04af72dc19a93593bc5 Mon Sep 17 00:00:00 2001 From: Leo Yang Date: Mon, 17 Aug 2015 15:18:47 -0400 Subject: [PATCH 70/73] =?UTF-8?q?Fix=20build=20warning:=20implicit=20decla?= =?UTF-8?q?ration=20of=20function=20=E2=80=98git=5Ftransaction=5Fconfig=5F?= =?UTF-8?q?new=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config.c b/src/config.c index 2df1fc80e30..f0b2c3a61b0 100644 --- a/src/config.c +++ b/src/config.c @@ -13,6 +13,7 @@ #include "vector.h" #include "buf_text.h" #include "config_file.h" +#include "transaction.h" #if GIT_WIN32 # include #endif From 47ed7e5acd5fe9fd8dfc1a6b9aac7603a50da25c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 18 Aug 2015 20:55:59 +0200 Subject: [PATCH 71/73] transport: provide a way to get the callbacks libgit2 implementations of smart subtransports can simply reach through the structure, but external implementors cannot. Add these two functions as a way for the smart subtransports to get the callbacks as set by the user. --- include/git2/sys/transport.h | 22 ++++++++++++++++++++++ src/transports/smart.c | 14 ++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/git2/sys/transport.h b/include/git2/sys/transport.h index 867fbcbced8..4a75b08327e 100644 --- a/include/git2/sys/transport.h +++ b/include/git2/sys/transport.h @@ -211,6 +211,28 @@ GIT_EXTERN(int) git_transport_smart( git_remote *owner, /* (git_smart_subtransport_definition *) */ void *payload); +/** + * Call the certificate check for this transport. + * + * @param transport a smart transport + * @param cert the certificate to pass to the caller + * @param valid whether we believe the certificate is valid + * @param hostname the hostname we connected to + * @return the return value of the callback + */ +GIT_EXTERN(int) git_transport_smart_certificate_check(git_transport *transport, git_cert *cert, int valid, const char *hostname); + +/** + * Call the credentials callback for this transport + * + * @param out the pointer where the creds are to be stored + * @param transport a smart transport + * @param user the user we saw on the url (if any) + * @param methods available methods for authentication + * @return the return value of the callback + */ +GIT_EXTERN(int) git_transport_smart_credentials(git_cred **out, git_transport *transport, const char *user, int methods); + /* *** End of base transport interface *** *** Begin interface for subtransports for the smart transport *** diff --git a/src/transports/smart.c b/src/transports/smart.c index 85a49e54349..31a2dec7bc9 100644 --- a/src/transports/smart.c +++ b/src/transports/smart.c @@ -372,6 +372,20 @@ static int ref_name_cmp(const void *a, const void *b) return strcmp(ref_a->head.name, ref_b->head.name); } +int git_transport_smart_certificate_check(git_transport *transport, git_cert *cert, int valid, const char *hostname) +{ + transport_smart *t = (transport_smart *)transport; + + return t->certificate_check_cb(cert, valid, hostname, t->message_cb_payload); +} + +int git_transport_smart_credentials(git_cred **out, git_transport *transport, const char *user, int methods) +{ + transport_smart *t = (transport_smart *)transport; + + return t->cred_acquire_cb(out, t->url, user, methods, t->cred_acquire_payload); +} + int git_transport_smart(git_transport **out, git_remote *owner, void *param) { transport_smart *t; From 57af0b928ec8dd12c443c9479973ad3e39f258b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 19 Aug 2015 00:46:28 +0200 Subject: [PATCH 72/73] cred: add a free function wrapper --- include/git2/transport.h | 11 +++++++++++ src/transports/cred.c | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/include/git2/transport.h b/include/git2/transport.h index fd55eac0ab0..0ec24169942 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -307,6 +307,17 @@ GIT_EXTERN(int) git_cred_ssh_key_memory_new( const char *privatekey, const char *passphrase); + +/** + * Free a credential. + * + * This is only necessary if you own the object; that is, if you are a + * transport. + * + * @param cred the object to free + */ +GIT_EXTERN(void) git_cred_free(git_cred *cred); + /** * Signature of a function which acquires a credential object. * diff --git a/src/transports/cred.c b/src/transports/cred.c index 044b2a262f2..49ede48bf58 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -378,3 +378,11 @@ int git_cred_username_new(git_cred **cred, const char *username) *cred = (git_cred *) c; return 0; } + +void git_cred_free(git_cred *cred) +{ + if (!cred) + return; + + cred->free(cred); +} From b445940e2d891a72a7d623bc7efa7649e19863e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 19 Aug 2015 12:53:31 +0200 Subject: [PATCH 73/73] CMake: fall back to OpenSSL on older OS X Starting at OS X 10.8, the Security framework offers some functions which are unified across OS X and iOS. These are the functions that we use. Older versions of OS X do not have these functions and we fail to compile. In these situations, fall back to using OpenSSL for our TLS stream instead. --- CMakeLists.txt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b6a8b6d4ec3..6c03c718c7e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -152,8 +152,18 @@ STRING(REGEX REPLACE "^.*LIBGIT2_SOVERSION ([0-9]+)$" "\\1" LIBGIT2_SOVERSION "$ INCLUDE_DIRECTORIES(src include) IF (SECURITY_FOUND) - MESSAGE("-- Found Security ${SECURITY_DIRS}") - LIST(APPEND LIBGIT2_PC_LIBS "-framework Security") + # OS X 10.7 and older do not have some functions we use, fall back to OpenSSL there + CHECK_LIBRARY_EXISTS("${SECURITY_DIRS}" SSLCreateContext "Security/SecureTransport.h" HAVE_NEWER_SECURITY) + IF (HAVE_NEWER_SECURITY) + MESSAGE("-- Found Security ${SECURITY_DIRS}") + LIST(APPEND LIBGIT2_PC_LIBS "-framework Security") + ELSE() + MESSAGE("-- Security framework is too old, falling back to OpenSSL") + SET(SECURITY_FOUND "NO") + SET(SECURITY_DIRS "") + SET(SECURITY_DIR "") + SET(USE_OPENSSL "ON") + ENDIF() ENDIF() IF (COREFOUNDATION_FOUND)