aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Merge branch 'kb/perf-trace'Junio C Hamano2014-07-22
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * kb/perf-trace: api-trace.txt: add trace API documentation progress: simplify performance measurement by using getnanotime() wt-status: simplify performance measurement by using getnanotime() git: add performance tracing for git's main() function to debug scripts trace: add trace_performance facility to debug performance issues trace: add high resolution timer function to debug performance issues trace: add 'file:line' to all trace output trace: move code around, in preparation to file:line output trace: add current timestamp to all trace output trace: disable additional trace output for unit tests trace: add infrastructure to augment trace output with additional info sha1_file: change GIT_TRACE_PACK_ACCESS logging to use trace API Documentation/git.txt: improve documentation of 'GIT_TRACE*' variables trace: improve trace performance trace: remove redundant printf format attribute trace: consistently name the format parameter trace: move trace declarations from cache.h to new trace.h
| * api-trace.txt: add trace API documentationKarsten Blees2014-07-13
| | | | | | | | | | Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * progress: simplify performance measurement by using getnanotime()Karsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | Calculating duration from a single uint64_t is simpler than from a struct timeval. Change throughput measurement from gettimeofday() to getnanotime(). Also calculate misec only if needed, and change integer division to integer multiplication + shift, which should be slightly faster. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * wt-status: simplify performance measurement by using getnanotime()Karsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | Calculating duration from a single uint64_t is simpler than from a struct timeval. Change performance measurement for 'advice.statusuoption' from gettimeofday() to getnanotime(). Also initialize t_begin to prevent uninitialized variable warning. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * git: add performance tracing for git's main() function to debug scriptsKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use trace_performance to measure and print execution time and command line arguments of the entire main() function. In constrast to the shell's 'time' utility, which measures total time of the parent process, this logs all involved git commands recursively. This is particularly useful to debug performance issues of scripted commands (i.e. which git commands were called with which parameters, and how long did they execute). Due to git's deliberate use of exit(), the implementation uses an atexit routine rather than just adding trace_performance_since() at the end of main(). Usage example: > GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list Creates a log file like this: 23:57:38.638765 trace.c:405 performance: 0.000310107 s: git command: 'git' 'rev-parse' '--git-dir' 23:57:38.644387 trace.c:405 performance: 0.000261759 s: git command: 'git' 'rev-parse' '--show-toplevel' 23:57:38.646207 trace.c:405 performance: 0.000304468 s: git command: 'git' 'config' '--get-colorbool' 'color.interactive' 23:57:38.648491 trace.c:405 performance: 0.000241667 s: git command: 'git' 'config' '--get-color' 'color.interactive.help' 'red bold' 23:57:38.650465 trace.c:405 performance: 0.000243063 s: git command: 'git' 'config' '--get-color' '' 'reset' 23:57:38.654850 trace.c:405 performance: 0.025126313 s: git command: 'git' 'stash' 'list' Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: add trace_performance facility to debug performance issuesKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add trace_performance and trace_performance_since macros that print a duration and an optional printf-formatted text to the file specified in environment variable GIT_TRACE_PERFORMANCE. These macros, in conjunction with getnanotime(), are intended to simplify performance measurements from within the application (i.e. profiling via manual instrumentation, rather than using an external profiling tool). Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable impact on performance, so that test code for well known time killers may be shipped in release builds. Alternatively, a developer could provide an additional performance patch (not meant for master) that allows reviewers to reproduce performance tests more easily, e.g. on other platforms or using their own repositories. Usage examples: Simple use case (measure one code section): uint64_t start = getnanotime(); /* code section to measure */ trace_performance_since(start, "foobar"); Complex use case (measure repetitive code sections): uint64_t t = 0; for (;;) { /* ignore */ t -= getnanotime(); /* code section to measure */ t += getnanotime(); /* ignore */ } trace_performance(t, "frotz"); Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: add high resolution timer function to debug performance issuesKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a getnanotime() function that returns nanoseconds since 01/01/1970 as unsigned 64-bit integer (i.e. overflows in july 2554). This is easier to work with than e.g. struct timeval or struct timespec. Basing the timer on the epoch allows using the results with other time-related APIs. To simplify adaption to different platforms, split the implementation into a common getnanotime() and a platform-specific highres_nanos() function. The common getnanotime() function handles errors, falling back to gettimeofday() if highres_nanos() isn't implemented or doesn't work. getnanotime() is also responsible for normalizing to the epoch. The offset to the system clock is calculated only once on initialization, i.e. manually setting the system clock has no impact on the timer (except if the fallback gettimeofday() is in use). Git processes are typically short lived, so we don't need to handle clock drift. The highres_nanos() function returns monotonically increasing nanoseconds relative to some arbitrary point in time (e.g. system boot), or 0 on failure. Providing platform-specific implementations should be relatively easy, e.g. adapting to clock_gettime() as defined by the POSIX realtime extensions is seven lines of code. This version includes highres_nanos() implementations for: * Linux: using clock_gettime(CLOCK_MONOTONIC) * Windows: using QueryPerformanceCounter() Todo: * enable clock_gettime() on more platforms * add Mac OSX version, e.g. using mach_absolute_time + mach_timebase_info Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: add 'file:line' to all trace outputKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is useful to see where trace output came from. Add 'const char *file, int line' parameters to the printing functions and rename them to *_fl. Add trace_printf* and trace_strbuf macros resolving to the *_fl functions and let the preprocessor fill in __FILE__ and __LINE__. As the trace_printf* functions take a variable number of arguments, this requires variadic macros (i.e. '#define foo(...) foo_impl(__VA_ARGS__)'. Though part of C99, it is unclear whether older compilers support this. Thus keep the old functions and only enable variadic macros for GNUC and MSVC 2005+ (_MSC_VER 1400). This has the nice side effect that the old C-style declarations serve as documentation how the macros are to be used. Print 'file:line ' as prefix to each trace line. Align the remaining trace output at column 40 to accommodate 18 char file names + 4 digit line number (currently there are 30 *.c files of length 18 and just 11 of 19). Trace output from longer source files (e.g. builtin/receive-pack.c) will not be aligned. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: move code around, in preparation to file:line outputKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | No functional changes, just move stuff around so that the next patch isn't that ugly... Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: add current timestamp to all trace outputKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | This is useful to tell apart trace output of separate test runs. It can also be used for basic, coarse-grained performance analysis. Note that the accuracy is tainted by writing to the trace file, and you have to calculate the deltas yourself (which is next to impossible if multiple threads or processes are involved). Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: disable additional trace output for unit testsKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | Some unit-tests use trace output to verify internal state, and unstable output such as timestamps and line numbers are not useful there. Disable additional trace output if GIT_TRACE_BARE is set. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: add infrastructure to augment trace output with additional infoKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To be able to add a common prefix or suffix to all trace output (e.g. a timestamp or file:line of the caller), factor out common setup and cleanup tasks of the trace* functions. When adding a common prefix, it makes sense that the output of each trace call starts on a new line. Add '\n' in case the caller forgot. Note that this explicitly limits trace output to line-by-line, it is no longer possible to trace-print just part of a line. Until now, this was just an implicit assumption (trace-printing part of a line worked, but messed up the trace file if multiple threads or processes were involved). Thread-safety / inter-process-safety is also the reason why we need to do the prefixing and suffixing in memory rather than issuing multiple write() calls. Write_or_whine_pipe() / xwrite() is atomic unless the size exceeds MAX_IO_SIZE (8MB, see wrapper.c). In case of trace_strbuf, this costs an additional string copy (which should be irrelevant for performance in light of actual file IO). While we're at it, rename trace_strbuf's 'buf' argument, which suggests that the function is modifying the buffer. Trace_strbuf() currently is the only trace API that can print arbitrary binary data (without barfing on '%' or stopping at '\0'), so 'data' seems more appropriate. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * sha1_file: change GIT_TRACE_PACK_ACCESS logging to use trace APIKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | This changes GIT_TRACE_PACK_ACCESS functionality as follows: * supports the same options as GIT_TRACE (e.g. printing to stderr) * no longer supports relative paths * appends to the trace file rather than overwriting Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Documentation/git.txt: improve documentation of 'GIT_TRACE*' variablesKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | Separate GIT_TRACE description into what it prints and how to configure where trace output is printed to. Change other GIT_TRACE_* descriptions to refer to GIT_TRACE. Add descriptions for GIT_TRACE_SETUP and GIT_TRACE_SHALLOW. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: improve trace performanceKarsten Blees2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The trace API currently rechecks the environment variable and reopens the trace file on every API call. This has the ugly side effect that errors (e.g. file cannot be opened, or the user specified a relative path) are also reported on every call. Performance can be improved by about factor three by remembering the environment state and keeping the file open. Replace the 'const char *key' parameter in the API with a pointer to a 'struct trace_key' that bundles the environment variable name with additional, trace-internal state. Change the call sites of these APIs to use a static 'struct trace_key' instead of a string constant. In trace.c::get_trace_fd(), save and reuse the file descriptor in 'struct trace_key'. Add a 'trace_disable()' API, so that packet_trace() can cleanly disable tracing when it encounters packed data (instead of using unsetenv()). Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: remove redundant printf format attributeKarsten Blees2014-06-17
| | | | | | | | | | | | | | | | trace_printf_key() is the only non-static function that duplicates the printf format attribute in the .c file, remove it for consistency. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: consistently name the format parameterKarsten Blees2014-06-17
| | | | | | | | | | | | | | | | | | The format parameter to trace_printf functions is sometimes abbreviated 'fmt'. Rename to 'format' everywhere (consistent with POSIX' printf specification). Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * trace: move trace declarations from cache.h to new trace.hKarsten Blees2014-06-17
| | | | | | | | | | | | | | | | | | Also include direct dependencies (strbuf.h and git-compat-util.h for __attribute__) so that trace.h can be used independently of cache.h, e.g. in test programs. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'maint'Junio C Hamano2014-07-21
|\ \ | | | | | | | | | | | | | | | * maint: use xmemdupz() to allocate copies of strings given by start and length use xcalloc() to allocate zero-initialized memory
| * | use xmemdupz() to allocate copies of strings given by start and lengthRené Scharfe2014-07-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use xmemdupz() to allocate the memory, copy the data and make sure to NUL-terminate the result, all in one step. The resulting code is shorter, doesn't contain the constants 1 and '\0', and avoids duplicating function parameters. For blame, the last copied byte (o->file.ptr[o->file.size]) is always set to NUL by fake_working_tree_commit() or read_sha1_file(), so no information is lost by the conversion to using xmemdupz(). Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | use xcalloc() to allocate zero-initialized memoryRené Scharfe2014-07-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | Use xcalloc() instead of xmalloc() followed by memset() to allocate and zero out memory because it's shorter and avoids duplicating the function parameters. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Ninth batch for 2.1Junio C Hamano2014-07-21
| | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'rs/unify-is-branch'Junio C Hamano2014-07-21
|\ \ \ | | | | | | | | | | | | | | | | * rs/unify-is-branch: refs.c: add a public is_branch function
| * | | refs.c: add a public is_branch functionRonnie Sahlberg2014-07-16
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | Both refs.c and fsck.c have their own private copies of the is_branch function. Delete the is_branch function from fsck.c and make the version in refs.c public. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'kb/avoid-fchmod-for-now'Junio C Hamano2014-07-21
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replaces the only two uses of fchmod() with chmod() because the former does not work on Windows port and because luckily we can. * kb/avoid-fchmod-for-now: config: use chmod() instead of fchmod()
| * | | config: use chmod() instead of fchmod()Karsten Blees2014-07-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There is no fchmod() on native Windows platforms (MinGW and MSVC), and the equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista. Use chmod() instead. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sk/mingw-uni-fix'Junio C Hamano2014-07-21
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | * sk/mingw-uni-fix: Win32: Unicode file name support (dirent) Win32: Unicode file name support (except dirent)
| * | | | Win32: Unicode file name support (dirent)Karsten Blees2014-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Changes opendir/readdir to use Windows Unicode APIs and convert between UTF-8/UTF-16. Removes parameter checks that are already covered by xutftowcs_path. This changes detection of ENAMETOOLONG from MAX_PATH - 2 to MAX_PATH (matching is_dir_empty in mingw.c). If name + "/*" or the resulting absolute path is too long, FindFirstFile fails and errno is set through err_win_to_posix. Increases the size of dirent.d_name to accommodate the full WIN32_FIND_DATA.cFileName converted to UTF-8 (UTF-16 to UTF-8 conversion may grow by factor three in the worst case). Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Stepan Kasal <kasal@ucw.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | Win32: Unicode file name support (except dirent)Karsten Blees2014-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replaces Windows "ANSI" APIs dealing with file- or path names with their Unicode equivalent, adding UTF-8/UTF-16LE conversion as necessary. The dirent API (opendir/readdir/closedir) is updated in a separate commit. Adds trivial wrappers for access, chmod and chdir. Adds wrapper for mktemp (needed for both mkstemp and mkdtemp). The simplest way to convert a repository with legacy-encoded (e.g. Cp1252) file names to UTF-8 ist to checkout with an old msysgit version and "git add --all & git commit" with the new version. Includes a fix for bug reported by John Chen: On Windows XP (not Win7), directories cannot be deleted while a find handle is open, causing "Deletion of directory '...' failed. Should I try again?" prompts. Prior to this commit, these failures were silently ignored due to strbuf_free in is_dir_empty resetting GetLastError to ERROR_SUCCESS. Close the find handle in is_dir_empty so that git doesn't block deletion of the directory even after all other applications have released it. Reported-by: John Chen <john0312@gmail.com> Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Stepan Kasal <kasal@ucw.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'ek/alt-odb-entry-fix'Junio C Hamano2014-07-21
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | * ek/alt-odb-entry-fix: sha1_file: do not add own object directory as alternate
| * | | | | sha1_file: do not add own object directory as alternateEphrim Khong2014-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When adding alternate object directories, we try not to add the directory of the current repository to avoid cycles. Unfortunately, that test was broken, since it compared an absolute with a relative path. Signed-off-by: Ephrim Khong <dr.khong@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'kb/hashmap-updates'Junio C Hamano2014-07-21
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * kb/hashmap-updates: hashmap: add string interning API hashmap: add simplified hashmap_get_from_hash() API hashmap: improve struct hashmap member documentation hashmap: factor out getting a hash code from a SHA1
| * | | | | | hashmap: add string interning APIKarsten Blees2014-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Interning short strings with high probability of duplicates can reduce the memory footprint and speed up comparisons. Add strintern() and memintern() APIs that use a hashmap to manage the pool of unique, interned strings. Note: strintern(getenv()) could be used to sanitize git's use of getenv(), in case we ever encounter a platform where a call to getenv() invalidates previous getenv() results (which is allowed by POSIX). Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hashmap: add simplified hashmap_get_from_hash() APIKarsten Blees2014-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Hashmap entries are typically looked up by just a key. The hashmap_get() API expects an initialized entry structure instead, to support compound keys. This flexibility is currently only needed by find_dir_entry() in name-hash.c (and compat/win32/fscache.c in the msysgit fork). All other (currently five) call sites of hashmap_get() have to set up a near emtpy entry structure, resulting in duplicate code like this: struct hashmap_entry keyentry; hashmap_entry_init(&keyentry, hash(key)); return hashmap_get(map, &keyentry, key); Add a hashmap_get_from_hash() API that allows hashmap lookups by just specifying the key and its hash code, i.e.: return hashmap_get_from_hash(map, hash(key), key); Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hashmap: improve struct hashmap member documentationKarsten Blees2014-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hashmap: factor out getting a hash code from a SHA1Karsten Blees2014-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Copying the first bytes of a SHA1 is duplicated in six places, however, the implications (the actual value would depend on the endianness of the platform) is documented only once. Add a properly documented API for this. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'jk/remote-curl-squelch-extra-errors'Junio C Hamano2014-07-21
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/remote-curl-squelch-extra-errors: remote-curl: mark helper-protocol errors more clearly remote-curl: use error instead of fprintf(stderr) remote-curl: do not complain on EOF from parent git
| * | | | | | | remote-curl: mark helper-protocol errors more clearlyJeff King2014-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we encounter an error in remote-curl, we generally just report it to stderr. There is no need for the user to care that the "could not connect to server" error was generated by git-remote-https rather than a function in the parent git-fetch process. However, when the error is in the protocol between git and the helper, it makes sense to clearly identify which side is complaining. These cases shouldn't ever happen, but when they do, we can make them less confusing by being more verbose. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | remote-curl: use error instead of fprintf(stderr)Jeff King2014-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We usually prefix our error messages with "error: ", but many error messages from remote-curl are simply printed with fprintf. This can make the output a little harder to read (especially because such message may be intermingled with errors from the parent git process). There is no reason to avoid error(), as we are already calling it many places (in addition to libgit.a functions which use it). While we're adjusting the messages, we can also drop the capitalization which makes them unlike other git error messages. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | remote-curl: do not complain on EOF from parent gitJeff King2014-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The parent git process is supposed to send us an empty line to indicate that the conversation is over. However, the parent process may die() if there is a problem with the operation (e.g., we try to fetch a ref that does not exist). In this case, it produces a useful message, but then remote-curl _also_ produces an unhelpful message: $ git pull origin matser fatal: couldn't find remote ref matser Unexpected end of command stream The "right" way to fix this is to teach the parent git to always cleanly close the connection to the helper, letting it know that we are done. Implementing that is rather clunky, though, as it would involve either replacing die() operations with returning errors up the stack (until we disconnect the transport), or adding an atexit handler to clean up any transport helpers left open. It's much simpler to just suppress the EOF message in remote-curl. It was not added to address any real-world situation in the first place, but rather a "we should probably report unexpected things" suggestion[1]. It is the parent git which drives the operation, and whose exit value actually matters. If the parent dies, then the helper has no need to complain (except as a debugging aid). In the off chance that the pipe is closed without the parent dying, it can still notice the non-zero exit code. [1] http://article.gmane.org/gmane.comp.version-control.git/176036 Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'rs/ref-transaction-0'Junio C Hamano2014-07-21
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Early part of the "ref transaction" topic. * rs/ref-transaction-0: refs.c: change ref_transaction_update() to do error checking and return status refs.c: remove the onerr argument to ref_transaction_commit update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument for errors refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback
| * | | | | | | | refs.c: change ref_transaction_update() to do error checking and return statusRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update ref_transaction_update() do some basic error checking and return non-zero on error. Update all callers to check ref_transaction_update() for error. There are currently no conditions in _update that will return error but there will be in the future. Add an err argument that will be updated on failure. In future patches we will start doing both locking and checking for name conflicts in _update instead of _commit at which time this function will start returning errors for these conditions. Also check for BUGs during update and die(BUG:...) if we are calling _update with have_old but the old_sha1 pointer is NULL. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: remove the onerr argument to ref_transaction_commitRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr argument any more. Remove the onerr argument from the ref_transaction_commit signature. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | update-ref: use err argument to get error from ref_transaction_commitRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is returned to print a log message if/after the transaction fails. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: make update_ref_write update a strbuf on failureRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change update_ref_write to also update an error strbuf on failure. This makes the error available to ref_transaction_commit callers if the transaction failed due to update_ref_sha1/write_ref_sha1 failures. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: make ref_update_reject_duplicates take a strbuf argument for errorsRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make ref_update_reject_duplicates return any error that occurs through a new strbuf argument. This means that when a transaction commit fails in this function we will now be able to pass a helpful error message back to the caller. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: log_ref_write should try to return meaningful errnoRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Making errno from write_ref_sha1() meaningful, which should fix * a bug in "git checkout -b" where it prints strerror(errno)  despite errno possibly being zero or clobbered * a bug in "git fetch"'s s_update_ref, which trusts the result of an  errno == ENOTDIR check to detect D/F conflicts Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: make resolve_ref_unsafe set errno to something meaningful on errorRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Making errno when returning from resolve_ref_unsafe() meaningful, which should fix * a bug in lock_ref_sha1_basic, where it assumes EISDIR means it failed due to a directory being in the way Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: commit_packed_refs to return a meaningful errno on failureRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Making errno when returning from commit_packed_refs() meaningful, which should fix * a bug in "git clone" where it prints strerror(errno) based on errno, despite errno possibly being zero and potentially having been clobbered by that point * the same kind of bug in "git pack-refs" and prepares for repack_without_refs() to get a meaningful error message when commit_packed_refs() fails without falling into the same bug. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | | | | | | refs.c: make remove_empty_directories always set errno to something saneRonnie Sahlberg2014-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Making errno when returning from remove_empty_directories() more obviously meaningful, which should provide some peace of mind for people auditing lock_ref_sha1_basic. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>