git/trace.c, branch v2.13.2

trace: do not fall back to stderr

2016-08-05T16:28:17Z

If the trace code cannot open a specified file, or does not understand the contents of the GIT_TRACE variable, it falls back to printing trace output to stderr. This is an attempt to be helpful, but in practice it just ends up annoying. The user was trying to get the output to go somewhere else, so spewing it to stderr does not really accomplish that. And as it's intended for debugging, they can presumably re-run the command with their error corrected. So instead of falling back, this patch disables bogus trace keys for the rest of the program, just as we do for write errors. We can drop the "Defaulting to..." part of the error message entirely; after seeing "cannot open '/foo'", the user can assume that tracing is skipped. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: disable key after write error

2016-08-05T16:28:16Z

If we get a write error writing to a trace descriptor, the error isn't likely to go away if we keep writing. Instead, you'll just get the same error over and over. E.g., try: GIT_TRACE_PACKET=42 git ls-remote >/dev/null You don't really need to see: warning: unable to write trace for GIT_TRACE_PACKET: Bad file descriptor hundreds of times. We could fallback to tracing to stderr, as we do in the error code-path for open(), but there's not much point. If the user fed us a bogus descriptor, they're probably better off fixing their invocation. And if they didn't, and we saw a transient error (e.g., ENOSPC writing to a file), it probably doesn't help anybody to have half of the trace in a file, and half on stderr. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: correct variable name in write() error message

2016-08-05T16:28:16Z

Our error message for write() always mentions GIT_TRACE, even though we may be writing for a different variable entirely. It's also not quite accurate to say "fd given by GIT_TRACE environment variable", as we may hit this error based on a filename the user put in the variable (we do complain and switch to stderr if the file cannot be opened, but it's still possible to hit a write() error on the descriptor later). So let's fix those things, and switch to our more usual "unable to do X: Y" format for the error. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: cosmetic fixes for error messages

2016-08-05T16:28:16Z

The error messages for the trace code are often multi-line; the first line gets a nice "warning:", but the rest are left-aligned. Let's give them an indentation to make sure they stand out as a unit. While we're here, let's also downcase the first letter of each error (our usual style), and break up a long line of advice (since we're already using multiple lines, one more doesn't hurt). We also replace "What does 'foo' for GIT_TRACE mean?". While cute, it's probably a good idea to give more context, and follow our usual styles. So it's now "unknown trace value for 'GIT_TRACE': foo". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: use warning() for printing trace errors

2016-08-05T16:27:34Z

Right now we just fprintf() straight to stderr, which can make the output hard to distinguish. It would be helpful to give it one of our usual prefixes like "error:", "warning:", etc. It doesn't make sense to use error() here, as the trace code is "optional" debugging code. If something goes wrong, we should warn the user, but saying "error" implies the actual git operation had a problem. So warning() is the only sane choice. Note that this does end up calling warn_routine() to do the formatting. This is probably a good thing, since they are clearly trying to hook messages before they make it to stderr. However, it also means that in theory somebody who tries to trace from their warn_routine() could cause a loop. This seems rather unlikely in practice (we've never even overridden the default warn_builtin routine before, and recent discussions to do so would just install a noop routine). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: stop using write_or_whine_pipe()

2016-08-04T20:33:27Z

The write_or_whine_pipe function does two things: 1. it checks for EPIPE and converts it into a signal death 2. it prints a message to stderr on error The first thing does not help us, and actively hurts. Generally we would simply die from SIGPIPE in this case, unless somebody has taken the time to ignore SIGPIPE for the whole process. And if they _did_ do that, it seems rather silly for the trace code, which otherwise takes pains to continue even in the face of errors (e.g., by not using write_or_die!), to take down the whole process for one specific type of error. Nor does the second thing help us; it just makes it harder to write our error message, because we have to feed bits of it as an argument to write_or_whine_pipe(). Translators never get to see the full message, and it's hard for us to customize it. Let's switch to just using write_in_full() and writing our own error string. For now, the error is identical to what write_or_whine_pipe() would say, but now that it's more under our control, we can improve it in future patches. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: handle NULL argument in trace_disable()

2016-08-04T20:33:27Z

All of the trace functions treat a NULL key as a synonym for the default GIT_TRACE key. Except for trace_disable(), which will segfault. Fortunately, this can't cause any bugs, as the function has no callers. But rather than drop it, let's fix the bug, as I plan to add a caller. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

trace: use strbuf for quote_crnl output

2015-09-25T17:18:18Z

When we output GIT_TRACE_SETUP paths, we quote any meta-characters. But our buffer to hold the result is only PATH_MAX bytes, and we could double the size of the input path (if every character needs quoting). We could use a 2*PATH_MAX buffer, if we assume the input will never be more than PATH_MAX. But it's easier still to just switch to a strbuf and not worry about whether the input can exceed PATH_MAX or not. The original copied the "p2" pointer to "p1", advancing both. Since this gets rid of "p1", let's also drop "p2", whose name is now confusing. We can just advance the original "path" pointer. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

pkt-line: support tracing verbatim pack contents

2015-06-16T20:24:22Z

When debugging the pack protocol, it is sometimes useful to store the verbatim pack that we sent or received on the wire. Looking at the on-disk result is often not helpful for a few reasons: 1. If the operation is a clone, we destroy the repo on failure, leaving nothing on disk. 2. If the pack is small, we unpack it immediately, and the full pack never hits the disk. 3. If we feed the pack to "index-pack --fix-thin", the resulting pack has the extra delta bases added to it. We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACKFILE to dump the verbatim packfile. There are a few other positive fallouts that come from rearranging this code: - We currently disable the packet trace after seeing the PACK header, even though we may get human-readable lines on other sidebands; now we include them in the trace. - We currently try to print "PACK ..." in the trace to indicate that the packfile has started. But because we disable packet tracing, we never printed this line. We will now do so. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'nd/multiple-work-trees'

2015-05-11T21:23:39Z

A replacement for contrib/workdir/git-new-workdir that does not rely on symbolic links and make sharing of objects and refs safer by making the borrowee and borrowers aware of each other. * nd/multiple-work-trees: (41 commits) prune --worktrees: fix expire vs worktree existence condition t1501: fix test with split index t2026: fix broken &&-chain t2026 needs procondition SANITY git-checkout.txt: a note about multiple checkout support for submodules checkout: add --ignore-other-wortrees checkout: pass whole struct to parse_branchname_arg instead of individual flags git-common-dir: make "modules/" per-working-directory directory checkout: do not fail if target is an empty directory t2025: add a test to make sure grafts is working from a linked checkout checkout: don't require a work tree when checking out into a new one git_path(): keep "info/sparse-checkout" per work-tree count-objects: report unused files in $GIT_DIR/worktrees/... gc: support prune --worktrees gc: factor out gc.pruneexpire parsing code gc: style change -- no SP before closing parenthesis checkout: clean up half-prepared directories in --to mode checkout: reject if the branch is already checked out elsewhere prune: strategies for linked checkouts checkout: support checking out into a new working directory ...