From 93b42728952487941bc2b0b78a7f0080b7b3d600 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Tue, 9 Jun 2015 14:38:30 -0700 Subject: [PATCH 01/31] 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 5c55ddd1a..994eed5e1 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 cdb0b514d..2b1dedc01 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 3f20bfd31..fc6337adc 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 b2abbe6a6..8c336e3ef 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 000000000..a778f4164 --- /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 000000000..40ca60d53 --- /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 000000000..f1e589531 --- /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 000000000..b457d2e8a --- /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 ae582d1cb..aaeeb7810 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 56326da1c..f67c8ffbc 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 000000000..c00c1b774 --- /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/31] 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 f1e589531..15af3dcb7 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 b457d2e8a..21170bd2f 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/31] 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 b3ab5397e..0abbc7f06 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 c1adcc662..44f627880 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 f702568bf..df45ad236 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 526f91f5525707b2a948e6ff5a59665d92370cab Mon Sep 17 00:00:00 2001 From: Linquize Date: Wed, 1 Jul 2015 14:58:13 +0800 Subject: [PATCH 04/31] Fix 8.3 filename tests failure when 8.3 is disabled --- src/repository.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/repository.c b/src/repository.c index 82d998124..08f4baa20 100644 --- a/src/repository.c +++ b/src/repository.c @@ -867,7 +867,9 @@ static int reserved_names_add8dot3(git_repository *repo, const char *path) { char *name = git_win32_path_8dot3_name(path); const char *def = GIT_DIR_SHORTNAME; + const char *def_dot_git = DOT_GIT; size_t name_len, def_len = CONST_STRLEN(GIT_DIR_SHORTNAME); + size_t def_dot_git_len = CONST_STRLEN(DOT_GIT); git_buf *buf; if (!name) @@ -875,7 +877,8 @@ static int reserved_names_add8dot3(git_repository *repo, const char *path) name_len = strlen(name); - if (name_len == def_len && memcmp(name, def, def_len) == 0) { + if ((name_len == def_len && memcmp(name, def, def_len) == 0) || + (name_len == def_dot_git_len && memcmp(name, def_dot_git, def_dot_git_len) == 0)) { git__free(name); return 0; } From 63924435a103d34315ad9a4851d84b2b052aca38 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 1 Jul 2015 09:40:11 -0500 Subject: [PATCH 05/31] 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 852000bba..735f1fe27 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 5fd8d5566..baf1515d6 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 e25d37c35..70c4fa382 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 493d26c80..fd1cd271c 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 000000000..2c80212be --- /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 000000000..13cfb23ae --- /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 000000000..ba826b5dc --- /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 34065968ed3c386b875ee050b8ed34d9968e2d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 1 Jul 2015 17:49:07 +0200 Subject: [PATCH 06/31] submodule: completely remove reload_all The function was removed, but its declaration and changelog entry about its removal were forgotten. The comment in the test doesn't make any sense as the function doesn't exist anymore, so get rid of it as well. --- CHANGELOG.md | 3 +++ include/git2/submodule.h | 11 ----------- tests/submodule/nosubs.c | 2 -- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1406085d..418055ca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -177,6 +177,9 @@ v0.22 + 1 * `git_submodule_save()` has been removed. The submodules are no longer configured via the objects. +* `git_submodule_reload_all()` has been removed as we no longer cache + submodules. + ### Breaking API changes * `git_smart_subtransport_cb` now has a `param` parameter. diff --git a/include/git2/submodule.h b/include/git2/submodule.h index cbafccd1b..689fe4b64 100644 --- a/include/git2/submodule.h +++ b/include/git2/submodule.h @@ -583,17 +583,6 @@ GIT_EXTERN(int) git_submodule_open( */ GIT_EXTERN(int) git_submodule_reload(git_submodule *submodule, int force); -/** - * Reread all submodule info. - * - * Call this to reload all cached submodule information for the repo. - * - * @param repo The repository to reload submodule data for - * @param force Force full reload even if the data doesn't seem out of date - * @return 0 on success, <0 on error - */ -GIT_EXTERN(int) git_submodule_reload_all(git_repository *repo, int force); - /** * Get the status for a submodule. * diff --git a/tests/submodule/nosubs.c b/tests/submodule/nosubs.c index 538825c1c..8a73dc11a 100644 --- a/tests/submodule/nosubs.c +++ b/tests/submodule/nosubs.c @@ -71,8 +71,6 @@ void test_submodule_nosubs__add_and_delete(void) git_submodule *sm; git_buf buf = GIT_BUF_INIT; - /* note lack of calls to git_submodule_reload_all - this *should* work */ - cl_git_fail(git_submodule_lookup(NULL, repo, "libgit2")); cl_git_fail(git_submodule_lookup(NULL, repo, "submodules/libgit2")); From e8e848a8dad92b6542b16449d29f4a5535fa7c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 1 Jul 2015 21:10:40 +0200 Subject: [PATCH 07/31] submodule: add failing test for loading the wrong submodule When two submodules are fairly similar, we may end up loading the wrong one. --- tests/submodule/lookup.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c index cddbdcfc2..4d40e2279 100644 --- a/tests/submodule/lookup.c +++ b/tests/submodule/lookup.c @@ -269,3 +269,26 @@ void test_submodule_lookup__just_added(void) refute_submodule_exists(g_repo, "sm_just_added_head", GIT_EEXISTS); } +/* Test_App and Test_App2 are fairly similar names, make sure we load the right one */ +void test_submodule_lookup__prefix_name(void) +{ + git_submodule *sm; + + cl_git_rewritefile("submod2/.gitmodules", + "[submodule \"Test_App\"]\n" + " path = Test_App\n" + " url = ../Test_App\n" + "[submodule \"Test_App2\"]\n" + " path = Test_App2\n" + " url = ../Test_App\n"); + + cl_git_pass(git_submodule_lookup(&sm, g_repo, "Test_App")); + cl_assert_equal_s("Test_App", git_submodule_name(sm)); + + git_submodule_free(sm); + + cl_git_pass(git_submodule_lookup(&sm, g_repo, "Test_App2")); + cl_assert_equal_s("Test_App2", git_submodule_name(sm)); + + git_submodule_free(sm); +} From e0af3cb30c10f65f6db802f64a3389f73c0292b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 1 Jul 2015 21:15:06 +0200 Subject: [PATCH 08/31] submodule: correctly delimit the keys to use for lookup The regex we use to look at the gitmodules file does not correctly delimit the name of submodule which we want to look up and puts '.*' straight after the name, maching on any submodule which has the seeked submodule as a prefix of its name. Add the missing '\.' in the regex so we want a full stop to exist both before and after the submodule name. --- src/submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/submodule.c b/src/submodule.c index 17e1a3561..fb3d4bf1e 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1385,7 +1385,7 @@ int git_submodule_reload(git_submodule *sm, int force) git_buf_sets(&path, "submodule\\."); git_buf_text_puts_escape_regex(&path, sm->name); - git_buf_puts(&path, ".*"); + git_buf_puts(&path, "\\..*"); if (git_buf_oom(&path)) { error = -1; From 79306d8d59b3a7abeac379005f41fa9c63cca50b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jul 2015 10:18:40 -0500 Subject: [PATCH 09/31] filter::stream: free the filter sanely Don't use the filter's free callback to free the actual data structure holding the filter, as we may not always actually initialize it (the test may be skipped). --- tests/filter/stream.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/filter/stream.c b/tests/filter/stream.c index 603f19494..6bf540ce7 100644 --- a/tests/filter/stream.c +++ b/tests/filter/stream.c @@ -25,6 +25,7 @@ void test_filter_stream__cleanup(void) g_repo = NULL; git_filter_unregister("compress"); + git__free(compress_filter); } #define CHUNKSIZE 10240 @@ -123,11 +124,6 @@ static int compress_filter_stream_init( return 0; } -static void compress_filter_free(git_filter *f) -{ - git__free(f); -} - git_filter *create_compress_filter(void) { git_filter *filter = git__calloc(1, sizeof(git_filter)); @@ -136,7 +132,6 @@ git_filter *create_compress_filter(void) filter->version = GIT_FILTER_VERSION; filter->attributes = "+compress"; filter->stream = compress_filter_stream_init; - filter->shutdown = compress_filter_free; return filter; } From dd6b24b19a02a5402b237e9fa9ba29cfee5b6ccf Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jul 2015 10:36:15 -0500 Subject: [PATCH 10/31] iterator_walk: cast away constness for free --- src/iterator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index 2c3a87af0..a312afb3a 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1921,8 +1921,8 @@ int git_iterator_walk( } done: - git__free(iterator_item); - git__free(cur_items); + git__free((git_index_entry **)iterator_item); + git__free((git_index_entry **)cur_items); if (error == GIT_ITEROVER) error = 0; From a94d3e68bf64c6acb89db1e14c78cfdc4b39ba0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 3 Jul 2015 14:34:30 +0200 Subject: [PATCH 11/31] filter: add docs for streaming filters These functions are available on the public API but don't have any documentation, so they don't appear on the API reference. Fix that. --- include/git2/filter.h | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/include/git2/filter.h b/include/git2/filter.h index 1828903e4..436a0f3c8 100644 --- a/include/git2/filter.h +++ b/include/git2/filter.h @@ -137,7 +137,13 @@ GIT_EXTERN(int) git_filter_list_apply_to_data( git_buf *in); /** - * Apply filter list to the contents of a file on disk + * Apply a filter list to the contents of a file on disk + * + * @param out buffer into which to store the filtered file + * @param filters the list of filters to apply + * @param repo the repository in which to perform the filtering + * @param path the path of the file to filter, a relative path will be + * taken as relative to the workdir */ GIT_EXTERN(int) git_filter_list_apply_to_file( git_buf *out, @@ -146,24 +152,51 @@ GIT_EXTERN(int) git_filter_list_apply_to_file( const char *path); /** - * Apply filter list to the contents of a blob + * Apply a filter list to the contents of a blob + * + * @param out buffer into which to store the filtered file + * @param filters the list of filters to apply + * @param blob the blob to filter */ GIT_EXTERN(int) git_filter_list_apply_to_blob( git_buf *out, git_filter_list *filters, git_blob *blob); +/** + * Apply a filter list to an arbitrary buffer as a stream + * + * @param filters the list of filters to apply + * @param data the buffer to filter + * @param target the stream into which the data will be written + */ GIT_EXTERN(int) git_filter_list_stream_data( git_filter_list *filters, git_buf *data, git_writestream *target); +/** + * Apply a filter list to a file as a stream + * + * @param filters the list of filters to apply + * @param repo the repository in which to perform the filtering + * @param path the path of the file to filter, a relative path will be + * taken as relative to the workdir + * @param target the stream into which the data will be written + */ GIT_EXTERN(int) git_filter_list_stream_file( git_filter_list *filters, git_repository *repo, const char *path, git_writestream *target); +/** + * Apply a filter list to a blob as a stream + * + * @param filters the list of filters to apply + * @param blob the blob to filter + * @param target the stream into which the data will be written + */ GIT_EXTERN(int) git_filter_list_stream_blob( git_filter_list *filters, git_blob *blob, 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 12/31] 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 418055ca3..b824a66da 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 13/31] 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 c2e0162d8..a0216e9c8 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 14/31] 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 a0216e9c8..5510379d4 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 15/31] 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 958252e9f..4df7c6b7c 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 16/31] 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 9131a1fa1..6a85224b4 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 17/31] 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 7f461abff..2fd90d066 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 18/31] 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 7ccf0f937..9f83bd842 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 19/31] 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 | 18 ++++++++++------ 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, 138 insertions(+), 77 deletions(-) diff --git a/src/blame_git.c b/src/blame_git.c index e863efe2e..f481426aa 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 cb8b235b5..e2f1e892b 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_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 XDL_PATCH_NORMAL '-' -#define XDL_PATCH_REVERSE '+' -#define XDL_PATCH_MODEMASK ((1 << 8) - 1) -#define XDL_PATCH_IGNOREBSPACE (1 << 8) +#define XDF_PATIENCE_DIFF (1 << 5) +#define XDF_HISTOGRAM_DIFF (1 << 6) +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF) +#define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK) + +#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 84aa0fcfe..2358a2d63 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 7a92ea9c4..8b81206c9 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 e3e63d902..750a97294 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 */ - for (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next) - if (xch->i1 - (xchp->i1 + xchp->chg1) > max_common) + /* 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 = 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 c2e2e8302..d29710770 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 c84812893..500d8112a 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 84e424672..b11e59817 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 fdd7d0263..04e1a1ab2 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 e419f4f72..63a22c630 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 bb7bdee49..30f2a30ac 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 714719a89..8f952a8e6 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 20/31] 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 0aae4da6a..fa6599190 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 2eeebd565..fd55eac0a 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 6534bdbbe..63421fcf7 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 958252e9f..1bd4f1421 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 10e19166c..33b6c5c38 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 83af137f8..0b0d4bac3 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 da047d690..0c43c4b0b 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 21/31] 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 2358a2d63..0620e5fff 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 750a97294..600fd1fdd 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 22/31] 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 ba826b5dc..8031ad3d3 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 23/31] 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 0aae4da6a..5e15a2121 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 8031ad3d3..999b33653 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 24/31] 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 818ac833b..71eb583d5 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 25/31] 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 5c55ddd1a..d3fe48f89 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 4ee1fdc2a..3191aeee2 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 26/31] 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 4d40e2279..9b2b3aa19 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,10 +104,27 @@ 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); } void test_submodule_lookup__lookup_even_with_unborn_head(void) 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 27/31] 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 fb3d4bf1e..892c98304 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 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 28/31] 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 67444cb4a..b536959fa 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 29/31] 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 67444cb4a..6be12406b 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 2f3ad1563..ca5173c0d 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 a94d509d3..4b3acbcce 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 9d6252ea8..863ac8f2d 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 8da7b4f7f..17536c030 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 acf89442a..fcb1112ac 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 dd4426475..1ed292be5 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 d214c9fa5..9ccbd8085 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 21cd1a615..b305ba1e3 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 dabc47a09..b14af44a9 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 33710f403..f81471424 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 cec3569f250379aaa9aa4cec4fb49d4a78d6ee11 Mon Sep 17 00:00:00 2001 From: Fallso Date: Tue, 14 Jul 2015 15:33:56 +0100 Subject: [PATCH 30/31] 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 c3baf411a..dc9b2f09e 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 31/31] 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 e47f00881..444fe5276 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 */