Commit Graph

40721 Commits

Author SHA1 Message Date
Junio C Hamano
c1e5ca90db Merge branch 'es/worktree-add'
Remove remaining cruft from  "git checkout --to", which
transitioned to "git worktree add".

* es/worktree-add:
  config: rename "gc.pruneWorktreesExpire" to "gc.worktreePruneExpire"
  Documentation/git-worktree: wordsmith worktree-related manpages
  Documentation/config: fix stale "git prune --worktree" reference
  Documentation/git-worktree: fix incorrect reference to file "locked"
  Documentation/git-worktree: consistently use term "linked working tree"
2015-08-12 14:09:55 -07:00
Junio C Hamano
71cc60070f Merge branch 'ad/bisect-cleanup'
Code and documentation clean-up to "git bisect".

* ad/bisect-cleanup:
  bisect: don't mix option parsing and non-trivial code
  bisect: simplify the addition of new bisect terms
  bisect: replace hardcoded "bad|good" by variables
  Documentation/bisect: revise overall content
  Documentation/bisect: move getting help section to the end
  bisect: correction of typo
2015-08-12 14:09:53 -07:00
Clemens Buchacher
dff6f280df git_open_noatime: return with errno=0 on success
In read_sha1_file_extended we die if read_object fails with a fatal
error. We detect a fatal error if errno is non-zero and is not
ENOENT. If the object could not be read because it does not exist,
this is not considered a fatal error and we want to return NULL.

Somewhere down the line, read_object calls git_open_noatime to open
a pack index file, for example. We first try open with O_NOATIME.
If O_NOATIME fails with EPERM, we retry without O_NOATIME. When the
second open succeeds, errno is however still set to EPERM from the
first attempt. When we finally determine that the object does not
exist, read_object returns NULL and read_sha1_file_extended dies
with a fatal error:

    fatal: failed to read object <sha1>: Operation not permitted

Fix this by resetting errno to zero before we call open again.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-12 13:56:19 -07:00
Paul Tan
b5e823594c am: let --signoff override --no-signoff
After resolving a conflicting patch, a user may wish to sign off the
patch to declare that the patch has been modified. As such, the user
will expect that running "git am --signoff --continue" will append the
signoff to the commit message.

However, the --signoff option is only taken into account during the
mail-parsing stage. If the --signoff option is set, then the signoff
will be appended to the commit message. Since the mail-parsing stage
comes before the patch application stage, the --signoff option, if
provided on the command-line when resuming, will have no effect at all.

We cannot move the append_signoff() call to the patch application stage
as the applypatch-msg hook and interactive mode, which run before patch
application, may expect the signoff to be there.

Fix this by taking note if the user explictly set the --signoff option
on the command-line, and append the signoff to the commit message when
resuming if so.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-12 10:33:47 -07:00
Paul Tan
852a171018 am: let command-line options override saved options
When resuming, git-am mistakenly ignores command-line options.

For instance, when a patch fails to apply with "git am patch",
subsequently running "git am --3way" would not cause git-am to fall
back on attempting a threeway merge.  This occurs because by default
the --3way option is saved as "false", and the saved am options are
loaded after the command-line options are parsed, thus overwriting
the command-line options when resuming.

Fix this by moving the am_load() function call before parse_options(),
so that command-line options will override the saved am options.

The purpose of supporting this use case is to enable users to "wiggle"
that one conflicting patch. As such, it is expected that the
command-line options do not affect subsequent applied patches. Implement
this by calling am_load() once we apply the conflicting patch
successfully.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-12 10:33:47 -07:00
Paul Tan
18d8c26930 test_terminal: redirect child process' stdin to a pty
When resuming, git-am detects if we are trying to feed it patches or not
by checking if stdin is a TTY.

However, the test library redirects stdin to /dev/null. This makes it
difficult, for instance, to test the behavior of "git am -3" when
resuming, as git-am will think we are trying to feed it patches and
error out.

Support this use case by extending test-terminal.perl to create a
pseudo-tty for the child process' standard input as well.

Note that due to the way the code is structured, the child's stdin
pseudo-tty will be closed when we finish reading from our stdin. This
means that in the common case, where our stdin is attached to /dev/null,
the child's stdin pseudo-tty will be closed immediately. Some operations
like isatty(), which git-am uses, require the file descriptor to be
open, and hence if the success of the command depends on such functions,
test_terminal's stdin should be redirected to a source with large amount
of data to ensure that the child's stdin is not closed, e.g.

	test_terminal git am --3way </dev/zero

Cc: Jonathan Nieder <jrnieder@gmail.com>
Cc: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-12 10:33:46 -07:00
David Turner
2c3aed1381 pseudoref: check return values from read_ref()
These codepaths attempt to compare the "expected" current value with
the actual current value, but did not check if we successfully read
the current value before comparison.

Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 15:52:20 -07:00
Johannes Sixt
b3325dfc64 t2019: skip test requiring '*' in a file name non Windows
A test case introduced by ae454f61 (Add tests for wildcard "path vs ref"
disambiguation) allocates a file named '*.c'. This does not work on
Windows, because the OS forbids file names containing wildcard
characters. The test case fails where the shell attempts to allocate the
file. Skip the test on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 15:17:30 -07:00
Johannes Sixt
fbf2fecf50 t7300-clean: require POSIXPERM for chmod 0 test
A test case introduced by 91479b9c (t7300: add tests to document
behavior of clean and nested git) uses 'chmod 0' to verify that a
subdirectory that has an unreadable .git file is not removed. This can
work only when the system pays attention to the permissions set with
'chmod'. Therefore, set the POSIXPERM prerequisite on the test case.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 15:17:07 -07:00
Jeff King
f4c3edc0b1 vreportf: avoid intermediate buffer
When we call "die(fmt, args...)", we end up in vreportf with
two pieces of information:

  1. The prefix "fatal: "

  2. The original fmt and va_list of args.

We format item (2) into a temporary buffer, and then fprintf
the prefix and the temporary buffer, along with a newline.
This has the unfortunate side effect of truncating any error
messages that are longer than 4096 bytes.

Instead, let's use separate calls for the prefix and
newline, letting us hand the item (2) directly to vfprintf.
This is essentially undoing d048a96 (print
warning/error/fatal messages in one shot, 2007-11-09), which
tried to have the whole output end up in a single `write`
call.

But we can address this instead by explicitly requesting
line-buffering for the output handle, and by making sure
that the buffer is empty before we start (so that outputting
the prefix does not cause a flush due to hitting the buffer
limit).

We may still break the output into two writes if the content
is larger than our buffer, but there's not much we can do
there; depending on the stdio implementation, that might
have happened even with a single fprintf call.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 14:29:36 -07:00
Jeff King
3b331e9267 vreportf: report to arbitrary filehandles
The vreportf function always goes to stderr, but run-command
wants child errors to go to the parent's original stderr. To
solve this, commit a5487dd duplicates the stderr fd and
installs die and error handlers to direct the output
appropriately (which later turned into the vwritef
function). This has two downsides, though:

  - we make multiple calls to write(), which contradicts the
    "write at once" logic from d048a96 (print
    warning/error/fatal messages in one shot, 2007-11-09).

  - the custom handlers basically duplicate the normal
    handlers.  They're only a few lines of code, but we
    should not have to repeat the magic "exit(128)", for
    example.

We can solve the first by using fdopen() on the duplicated
descriptor. We can't pass this to vreportf, but we could
introduce a new vreportf_to to handle it.

However, to fix the second problem, we instead introduce a
new "set_error_handle" function, which lets the normal
vreportf calls output to a handle besides stderr. Thus we
can get rid of our custom handlers entirely, and just ask
the regular handlers to output to our new descriptor.

And as vwritef has no more callers, it can just go away.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 14:24:50 -07:00
David Turner
b02e8595e2 notes: handle multiple worktrees
Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
find_shared_symref and die if we find one.  This prevents simultaneous
merges to the same notes branch from different worktrees.

Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 13:52:27 -07:00
David Turner
41af65651d worktrees: add find_shared_symref
Add a new function, find_shared_symref, which contains the heart of
die_if_checked_out, but works for any symref, not just HEAD.  Refactor
die_if_checked_out to use the same infrastructure as
find_shared_symref.

Soon, we will use find_shared_symref to protect notes merges in
worktrees.

Signed-off-by: David Turner <dturner@twopensource.com>
Reviewed-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11 13:48:15 -07:00
Jeff King
f932729cc7 memoize common git-path "constant" files
One of the most common uses of git_path() is to pass a
constant, like git_path("MERGE_MSG"). This has two
drawbacks:

  1. The return value is a static buffer, and the lifetime
     is dependent on other calls to git_path, etc.

  2. There's no compile-time checking of the pathname. This
     is OK for a one-off (after all, we have to spell it
     correctly at least once), but many of these constant
     strings appear throughout the code.

This patch introduces a series of functions to "memoize"
these strings, which are essentially globals for the
lifetime of the program. We compute the value once, take
ownership of the buffer, and return the cached value for
subsequent calls.  cache.h provides a helper macro for
defining these functions as one-liners, and defines a few
common ones for global use.

Using a macro is a little bit gross, but it does nicely
document the purpose of the functions. If we need to touch
them all later (e.g., because we learned how to change the
git_dir variable at runtime, and need to invalidate all of
the stored values), it will be much easier to have the
complete list.

Note that the shared-global functions have separate, manual
declarations. We could do something clever with the macros
(e.g., expand it to a declaration in some places, and a
declaration _and_ a definition in path.c). But there aren't
that many, and it's probably better to stay away from
too-magical macros.

Likewise, if we abandon the C preprocessor in favor of
generating these with a script, we could get much fancier.
E.g., normalizing "FOO/BAR-BAZ" into "git_path_foo_bar_baz".
But the small amount of saved typing is probably not worth
the resulting confusion to readers who want to grep for the
function's definition.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:14 -07:00
Jeff King
0ea68e4296 get_repo_path: refactor path-allocation
The get_repo_path function calls mkpath() and then does some
non-trivial operations on it, like calling
is_git_directory() and read_gitfile(). These are actually
OK (they do not use more pathname static buffers
themselves), but it takes a fair bit of work to verify.

Let's use our own strbuf to store the path, and we can
simply reuse it for each iteration of the loop (we can even
avoid rewriting the beginning part, since we are trying a
series of suffixes).

To make the strbuf cleanup easier, we split out a thin
wrapper. As a bonus, this wrapper can factor out the
canonicalization that happens in all of the early-return
code paths.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:14 -07:00
Jeff King
03f2c7731b find_hook: keep our own static buffer
The find_hook function returns the results of git_path,
which is a static buffer shared by other path-related calls.
Returning such a buffer is slightly dangerous, because it
can be overwritten by seemingly unrelated functions.

Let's at least keep our _own_ static buffer, so you can
only get in trouble by calling find_hook in quick
succession, which is less likely to happen and more obvious
to notice.

While we're at it, let's add some documentation of the
function's limitations.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
470e28d4e1 refs.c: remove_empty_directories can take a strbuf
The first thing we do in this function is copy the input
into a strbuf. Of the 4 callers, 3 of them already have a
strbuf we could use. Let's just take the strbuf, and convert
the remaining caller to use a strbuf, rather than a raw
git_path. This is safer, anyway, as remove_dir_recursively
is a non-trivial function that might use the pathname
buffers itself (this is _probably_ OK, as the likely culprit
would be calling resolve_gitlink_ref, but we do not pass the
proper flags to ask it to avoid blowing away gitlinks).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
5f8ef5b848 refs.c: avoid git_path assignment in lock_ref_sha1_basic
Assigning the result of git_path is a bad pattern, because
it's not immediately obvious how long you expect the content
to stay valid (and it may be overwritten by subsequent
calls). Let's use a function-local strbuf here instead,
which we know is safe (we just have to remember to free it
in all code paths).

As a bonus, we get rid of a confusing variable-reuse
("ref_file" is used for two distinct purposes).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
d6549f3655 refs.c: avoid repeated git_path calls in rename_tmp_log
Because it's not safe to store the static-buffer results of
git_path for a long time, we end up formatting the same
filename over and over. We can fix this by using a
function-local strbuf to store the formatted pathname and
avoid repeating ourselves.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
54b418f698 refs.c: simplify strbufs in reflog setup and writing
Commit 1a83c24 (git_snpath(): retire and replace with
strbuf_git_path(), 2014-11-30) taught log_ref_setup and
log_ref_write_1 to take a strbuf parameter, rather than a
bare string. It then makes an alias to the strbuf's "buf"
field under the original name.

This made the original diff much shorter, but the resulting
code is more complicated that it needs to be. Since we've
aliased the pointer, we drop our reference to the strbuf to
ensure we don't accidentally change it. But if we simply
drop our alias and use "logfile.buf" directly, we do not
have to worry about this aliasing. It's a larger diff, but
the resulting code is simpler.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
07e3070d2a path.c: drop git_path_submodule
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
f5b2dec165 refs.c: remove extra git_path calls from read_loose_refs
In iterating over the loose refs in "refs/foo/", we keep a
running strbuf with "refs/foo/one", "refs/foo/two", etc. But
we also need to access these files in the filesystem, as
".git/refs/foo/one", etc. For this latter purpose, we make a
series of independent calls to git_path(). These are safe
(we only use the result to call stat()), but assigning the
result of git_path is a suspicious pattern that we'd rather
avoid.

This patch keeps a running buffer with ".git/refs/foo/", and
we can just append/reset each directory element as we loop.
This matches how we handle the refnames. It should also be
more efficient, as we do not keep formatting the same
".git/refs/foo" prefix (which can be arbitrarily deep).

Technically we are dropping a call to strbuf_cleanup() on
each generated filename, but that's OK; it wasn't doing
anything, as we are putting in single-level names we read
from the filesystem (so it could not possibly be cleaning up
cruft like "./" in this instance).

A clever reader may also note that the running refname
buffer ("refs/foo/") is actually a subset of the filesystem
path buffer (".git/refs/foo/"). We could get by with one
buffer, indexing the length of $GIT_DIR when we want the
refname. However, having tried this, the resulting code
actually ends up a little more confusing, and the efficiency
improvement is tiny (and almost certainly dwarfed by the
system calls we are making).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:13 -07:00
Jeff King
b21a5d6605 remote.c: drop extraneous local variable from migrate_file
It's an anti-pattern to assign the result of git_path to a
variable, since other calls may reuse our buffer. In this
case, we feed the result to unlink_or_warn immediately
afterwards, so it's OK. However, it's nice to avoid
assignment entirely, which makes it more obvious that
there's no bug.

We can just pass the result directly to unlink_or_warn,
which is a known-simple function. As a bonus, the code flow
is a little more obvious, as we eliminate an extra
conditional (a reader does not have to wonder any more
"under which circumstances is 'path' set?").

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:12 -07:00
Jeff King
e3cf230324 prefer mkpathdup to mkpath in assignments
As with the previous commit to git_path, assigning the
result of mkpath is suspicious, since it is not clear
whether we will still depend on the value after it may have
been overwritten by subsequent calls. This patch converts
low-hanging fruit to use mkpathdup instead of mkpath (with
the downside that we must remember to free the result).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:12 -07:00
Jeff King
fcd12db6af prefer git_pathdup to git_path in some possibly-dangerous cases
Because git_path uses a static buffer that is shared with
calls to git_path, mkpath, etc, it can be dangerous to
assign the result to a variable or pass it to a non-trivial
function. The value may change unexpectedly due to other
calls.

None of the cases changed here has a known bug, but they're
worth converting away from git_path because:

  1. It's easy to use git_pathdup in these cases.

  2. They use constructs (like assignment) that make it
     hard to tell whether they're safe or not.

The extra malloc overhead should be trivial, as an
allocation should be an order of magnitude cheaper than a
system call (which we are clearly about to make, since we
are constructing a filename). The real cost is that we must
remember to free the result.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:37:12 -07:00
Jeff King
77b9b1d13a add_to_alternates_file: don't add duplicate entries
The add_to_alternates_file function blindly uses
hold_lock_file_for_append to copy the existing contents, and
then adds the new line to it. This has two minor problems:

  1. We might add duplicate entries, which are ugly and
     inefficient.

  2. We do not check that the file ends with a newline, in
     which case we would bogusly append to the final line.
     This is quite unlikely in practice, though, as we call
     this function only from git-clone, so presumably we are
     the only writers of the file (and we always add a
     newline).

Instead of using hold_lock_file_for_append, let's copy the
file line by line, which ensures all records are properly
terminated. If we see an extra line, we can simply abort the
update (there is no point in even copying the rest, as we
know that it would be identical to the original).

As a bonus, we also get rid of some calls to the
static-buffer mkpath and git_path functions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:15:42 -07:00
Jeff King
bc192300c4 t5700: modernize style
The early part of this test is rather old, and does not
follow our usual style guidelines. In particular:

  - the tests liberally chdir, and expect out-of-test "cd"
    commands to return them to a sane state

  - test commands aren't indented at all

  - there are a lot of minor formatting nits, like the
    opening quote of the test block on the wrong line,
    spaces after ">", etc

This patch fixes the style issues, and uses a few helper
functions, along with subshells and "git -C", to avoid
changing the cwd of the main script.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:15:42 -07:00
Jeff King
f5895fd399 cache.h: complete set of git_path_submodule helpers
The git_path function has "git_pathdup" and
"strbuf_git_path" variants, but git_submodule_path only
comes in the dangerous, static-buffer variant. That makes
refactoring callers to use the safer functions hard (since
they don't exist).

Since we're already using a strbuf behind the scenes, it's
easy to expose all three of these interfaces with thin
wrappers.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:15:41 -07:00
Jeff King
69ddd231fe cache.h: clarify documentation for git_path, et al
The comment above these functions actually describes
sha1_file_name, and comes from the very first revision of
git. Commit 723c31f (Add "git_path()" and "head_ref()"
helper functions., 2005-07-05) added git_path, pushing the
comment away from the function it describes; later commits
added more functions in this block.

Let's fix the comment to describe these related functions in
more detail. Let's also make sure to point out their safer
alternatives (and move those alternatives below, which makes
more sense when reading the file).

Note that we do not need to move the existing comment to
sha1_file_name.  Commit d40d535 (sha1_file.c: document a
bunch of functions defined in the file, 2014-02-21) already
added a much more descriptive comment to it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 15:15:41 -07:00
Michael Haggerty
6e122b449b setup_temporary_shallow(): use tempfile module
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
f6ecc62dbf write_shared_index(): use tempfile module
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
99397152a3 register_tempfile(): new function to handle an existing temporary file
Allow an existing file to be registered with the tempfile-handling
infrastructure; in particular, arrange for it to be deleted on program
exit. This can be used if the temporary file has to be created in a
more complicated way than just open(). For example:

* If the file itself needs to be created via the lockfile API
* If it is not a regular file (e.g., a socket)

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
354ab11206 tempfile: add several functions for creating temporary files
Add several functions for creating temporary files with
automatically-generated names, analogous to mkstemps(), but also
arranging for the files to be deleted on program exit.

The functions are named according to a pattern depending how they
operate. They will be used to replace many places in the code where
temporary files are created and cleaned up ad-hoc.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
7eba6ce5c7 prepare_tempfile_object(): new function, extracted from create_tempfile()
This makes the next step easier.

The old code used to use "path" to set the initial length of
tempfile->filename. This was not helpful because path was usually
relative whereas the value stored to filename will be absolute. So
just initialize the length to 0.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
1a9d15db25 tempfile: a new module for handling temporary files
A lot of work went into defining the state diagram for lockfiles and
ensuring correct, race-resistant cleanup in all circumstances.

Most of that infrastructure can be applied directly to *any* temporary
file. So extract a new "tempfile" module from the "lockfile" module.
Reimplement lockfile on top of tempfile.

Subsequent commits will add more users of the new module.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
9c77381d6a commit_lock_file(): use get_locked_file_path()
First beef up the sanity checking in get_locked_file_path() to match
that in commit_lock_file(). Then rewrite commit_lock_file() to use
get_locked_file_path() for its pathname computation.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
b4fb09e4da lockfile: add accessor get_lock_file_path()
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
c99a4c2db3 lockfile: add accessors get_lock_file_fd() and get_lock_file_fp()
We are about to move those members, so change client code to read them
through accessor functions.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
e54c347c1c create_bundle(): duplicate file descriptor to avoid closing it twice
write_pack_data() passes bundle_fd to start_command() to be used as
the stdout of pack-objects. But start_command() closes its stdout if
it is > 1. This is a problem if bundle_fd is the fd of a lock_file,
because commit_lock_file() will also try to close the fd.

So the old code suppressed commit_lock_file()'s usual behavior of
closing the file descriptor by setting the lock_file object's fd field
to -1.

But this is not really kosher. Code here shouldn't be mutating fields
within the lock_file object.

Instead, duplicate the file descriptor before passing it to
write_pack_data(). Then that function can close its copy without
closing the copy held in the lock_file object.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Michael Haggerty
2db69de81d lockfile: move documentation to lockfile.h and lockfile.c
Rearrange/rewrite it somewhat to fit its new environment.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:57:14 -07:00
Jim Hill
3ebbd00cf3 strbuf_read(): skip unnecessary strbuf_grow() at eof
The loop in strbuf_read() uses xread() repeatedly while extending
the strbuf until the call returns zero.  If the buffer is
sufficiently large to begin with, this results in xread()
returning the remainder of the file to the end (returning
non-zero), the loop extending the strbuf, and then making another
call to xread() to have it return zero.

By using read_in_full(), we can tell when the read reached the end
of file: when it returns less than was requested, it's eof.  This
way we can avoid an extra iteration that allocates an extra 8kB
that is never used.

Signed-off-by: Jim Hill <gjthill@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:51:13 -07:00
Junio C Hamano
cb5add5868 sha1_file.c: rename move_temp_to_file() to finalize_object_file()
Since 5a688fe4 ("core.sharedrepository = 0mode" should set, not
loosen, 2009-03-25), we kept reminding ourselves:

    NEEDSWORK: this should be renamed to finalize_temp_file() as
    "moving" is only a part of what it does, when no patch between
    master to pu changes the call sites of this function.

without doing anything about it.  Let's do so.

The purpose of this function was not to move but to finalize.  The
detail of the primarily implementation of finalizing was to link the
temporary file to its final name and then to unlink, which wasn't
even "moving".  The alternative implementation did "move" by calling
rename(2), which is a fun tangent.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 11:10:37 -07:00
Patrick Steinhardt
adef9561f0 clone: abort if no dir name could be guessed
Due to various components of the URI being stripped off it may
happen that we fail to guess a directory name. We currently error
out with a message that it is impossible to create the working
tree '' in such cases. Instead, error out early with a sensible
error message hinting that a directory name should be specified
manually on the command line.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 11:02:11 -07:00
Patrick Steinhardt
92722efec0 clone: do not use port number as dir name
If the URI contains a port number and the URI's path component is
empty we fail to guess a sensible directory name. E.g. cloning a
repository 'ssh://example.com:2222/' we guess a directory name
'2222' where we would want the hostname only, e.g. 'example.com'.

We need to take care to not drop trailing port-like numbers in
certain cases. E.g. when cloning a repository 'foo/bar:2222.git'
we want to guess the directory name '2222' instead of 'bar'.
Thus, we have to first check the stripped URI for path separators
and only strip port numbers if there are path separators present.
This heuristic breaks when cloning a repository 'bar:2222.git',
though.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 11:02:07 -07:00
Patrick Steinhardt
e895986727 clone: do not include authentication data in guessed dir
If the URI contains authentication data and the URI's path
component is empty, we fail to guess a sensible directory name.
E.g. cloning a repository 'ssh://user:password@example.com/' we
guess a directory name 'password@example.com' where we would want
the hostname only, e.g. 'example.com'.

The naive way of just adding '@' as a path separator would break
cloning repositories like 'foo/bar@baz.git' (which would
currently become 'bar@baz' but would then become 'baz' only).
Instead fix this by first dropping the scheme and then greedily
scanning for an '@' sign until we find the first path separator.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 11:01:08 -07:00
Jeff King
db2e220447 clone: use computed length in guess_dir_name
Commit 7e837c6 (clone: simplify string handling in
guess_dir_name(), 2015-07-09) changed clone to use
strip_suffix instead of hand-rolled pointer manipulation.
However, strip_suffix will strip from the end of a
NUL-terminated string, and we may have already stripped some
characters (like directory separators, or "/.git"). This
leads to commands like:

  git clone host:foo.git/

failing to strip the ".git".

We must instead convert our pointer arithmetic into a
computed length and feed that to strip_suffix_mem, which will
then reduce the length further for us.

It would be nicer if we could drop the pointer manipulation
entirely, and just continually strip using strip_suffix. But
that doesn't quite work for two reasons:

  1. The early suffixes we're stripping are not constant; we
     need to look for is_dir_sep, which could be one of
     several characters.

  2. Mid-way through the stripping we compute the pointer
     "start", which shows us the beginning of the pathname.
     Which really give us two lengths to work with: the
     offset from the start of the string, and from the start
     of the path. By using pointers for the early part, we
     can just compute the length from "start" when we need
     it.

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 11:01:05 -07:00
Jeff King
d6a31e08cd clone: add tests for output directory
When we run "git clone $url", clone guesses from the $url
what to name the local output directory. We don't have any
test coverage of this, so let's add some basic tests.

This reveals a few problems:

  - cloning "foo.git/" does not properly remove the ".git";
    this is a recent regression from 7e837c6 (clone:
    simplify string handling in guess_dir_name(), 2015-07-09)

  - likewise, cloning foo/.git does not seem to handle the
    bare case (we should end up in foo.git, but we try to
    use foo/.git on the local end), which also comes from
    7e837c6.

  - cloning the root is not very smart about URL parsing,
    and usernames and port numbers may end up in the
    directory name

All of these tests are marked as failures.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 10:59:30 -07:00
SZEDER Gábor
905f2036d0 completion: list variable names reliably with 'git config --name-only'
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:

  $ git config --get branch.b.description
  Branch description to fool the completion script with a
  second line containing dot . and equals = characters.
  $ git config --unset <TAB>
  ...
  second line containing dot . and equals
  ...

The completion script runs 'git config --list' and processes its output
to strip the values and keep only the variable names.  It does so by
looking for lines containing '.' and '=' and outputting everything
before the '=', which was fooled by my multi-line branch description.

A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
lines in its output are simply stripped after the first space, so
subsequent lines don't even have to contain '.' and '=' to fool the
completion script.

Use the new '--name-only' option added in the previous commit to list
config variable names reliably in both cases, without error-prone post
processing.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 10:34:40 -07:00
SZEDER Gábor
578625fa91 config: add '--name-only' option to list only variable names
'git config' can only show values or name-value pairs, so if a shell
script needs the names of set config variables it has to run 'git config
--list' or '--get-regexp' and parse the output to separate config
variable names from their values.  However, such a parsing can't cope
with multi-line values.  Though 'git config' can produce null-terminated
output for newline-safe parsing, that's of no use in such a case, becase
shells can't cope with null characters.

Even our own bash completion script suffers from these issues.

Help the completion script, and shell scripts in general, by introducing
the '--name-only' option to modify the output of '--list' and
'--get-regexp' to list only the names of config variables, so they don't
have to perform error-prone post processing to separate variable names
from their values anymore.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 10:33:58 -07:00
Jeff King
9b5fe78b34 test-lib: disable trace when test is not verbose
The "-x" test-script option turns on the shell's "-x"
tracing, which can help show why a particular test is
failing. Unfortunately, this can create false negatives in
some tests if they invoke a shell function with its stderr
redirected. t5512.10 is such a test, as it does:

    test_must_fail git ls-remote refs*master >actual 2>&1 &&
    test_cmp exp actual

The "actual" file gets the "-x" trace for the test_must_fail
function, which prevents it from matching the expected
output.

There's no way to avoid this without managing the
trace flag inside each sub-function, which isn't really a
workable solution. But unless you specifically care about
t5512.10, we can work around it by enabling tracing only for
the specific tests we want.

You can already do:

    ./t5512-ls-remote.sh -x --verbose-only=16

to see the trace only for a specific test. But that doesn't
_disable_ the tracing in the other tests; it just sends it
to /dev/null. However, there's no point in generating a
trace that the user won't see, so we can simply disable
tracing whenever it doesn't have a matching verbose flag.

The normal case of just "./t5512-ls-remote.sh -x" stays the
same, as "-x" already implies "--verbose" (and
"--verbose-only" overrides "--verbose", which is why this
works at all). And for our test, we need only check
$verbose, as maybe_setup_verbose will have already
set that flag based on the $verbose_only list).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-07 11:52:46 -07:00