* mh/lockfile-stdio:
commit_packed_refs(): reimplement using fdopen_lock_file()
dump_marks(): reimplement using fdopen_lock_file()
fdopen_lock_file(): access a lockfile using stdio
The lockfile API and its users have been cleaned up.
* mh/lockfile: (38 commits)
lockfile.h: extract new header file for the functions in lockfile.c
hold_locked_index(): move from lockfile.c to read-cache.c
hold_lock_file_for_append(): restore errno before returning
get_locked_file_path(): new function
lockfile.c: rename static functions
lockfile: rename LOCK_NODEREF to LOCK_NO_DEREF
commit_lock_file_to(): refactor a helper out of commit_lock_file()
trim_last_path_component(): replace last_path_elm()
resolve_symlink(): take a strbuf parameter
resolve_symlink(): use a strbuf for internal scratch space
lockfile: change lock_file::filename into a strbuf
commit_lock_file(): use a strbuf to manage temporary space
try_merge_strategy(): use a statically-allocated lock_file object
try_merge_strategy(): remove redundant lock_file allocation
struct lock_file: declare some fields volatile
lockfile: avoid transitory invalid states
git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
dump_marks(): remove a redundant call to rollback_lock_file()
api-lockfile: document edge cases
commit_lock_file(): rollback lock file on failure to rename
...
"git daemon" (with NO_IPV6 build configuration) used to incorrectly
use the hostname even when gethostbyname() reported that the given
hostname is not found.
When running a required clean filter, we do not have to mmap the
original before feeding the filter. Instead, stream the file
contents directly to the filter and process its output.
* sp/stream-clean-filter:
sha1_file: don't convert off_t to size_t too early to avoid potential die()
convert: stream from fd to required clean filter to reduce used address space
copy_fd(): do not close the input file descriptor
mmap_limit: introduce GIT_MMAP_LIMIT to allow testing expected mmap size
memory_limit: use git_env_ulong() to parse GIT_ALLOC_LIMIT
config.c: add git_env_ulong() to parse environment variable
convert: drop arguments other than 'path' from would_convert_to_git()
Allow "git push" request to be signed, so that it can be verified and
audited, using the GPG signature of the person who pushed, that the
tips of branches at a public repository really point the commits
the pusher wanted to, without having to "trust" the server.
* jc/push-cert: (24 commits)
receive-pack::hmac_sha1(): copy the entire SHA-1 hash out
signed push: allow stale nonce in stateless mode
signed push: teach smart-HTTP to pass "git push --signed" around
signed push: fortify against replay attacks
signed push: add "pushee" header to push certificate
signed push: remove duplicated protocol info
send-pack: send feature request on push-cert packet
receive-pack: GPG-validate push certificates
push: the beginning of "git push --signed"
pack-protocol doc: typofix for PKT-LINE
gpg-interface: move parse_signature() to where it should be
gpg-interface: move parse_gpg_output() to where it should be
send-pack: clarify that cmds_sent is a boolean
send-pack: refactor inspecting and resetting status and sending commands
send-pack: rename "new_refs" to "need_pack_data"
receive-pack: factor out capability string generation
send-pack: factor out capability string generation
send-pack: always send capabilities
send-pack: refactor decision to send update per ref
send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
...
Some MUAs mangled a line in a message that begins with "From " to
">From " when writing to a mailbox file and feeding such an input to
"git am" used to lose such a line.
* jk/mbox-from-line:
mailinfo: work around -Wstring-plus-int warning
mailinfo: make ">From" in-body header check more robust
The output file hasn't been created at this point, yet, so there is no
need to delete it when exiting early.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new function, fdopen_lock_file(), which returns a FILE pointer
open to the lockfile. If a stream is open on a lock_file object, it is
closed using fclose() on commit, rollback, or close_lock_file().
This change will allow callers to use stdio to write to a lockfile
without having to muck around in the internal representation of the
lock_file object (callers will be rewritten in upcoming commits).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lockfile.h: extract new header file for the functions in lockfile.c
Move the interface declaration for the functions in lockfile.c from
cache.h to a new file, lockfile.h. Add #includes where necessary (and
remove some redundant includes of cache.h by files that already
include builtin.h).
Move the documentation of the lock_file state diagram from lockfile.c
to the new header file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function to return the path of the file that is locked by a
lock_file object. This reduces the knowledge that callers have to have
about the lock_file layout.
Suggested-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it harder to misread the name as LOCK_NODE_REF.
Suggested-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit_lock_file_to(): refactor a helper out of commit_lock_file()
commit_locked_index(), when writing to an alternate index file,
duplicates (poorly) the code in commit_lock_file(). And anyway, it
shouldn't have to know so much about the internal workings of lockfile
objects. So extract a new function commit_lock_file_to() that does the
work common to the two functions, and call it from both
commit_lock_file() and commit_locked_index().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rewrite last_path_elm() to take a strbuf parameter and to trim off the
last path name element in place rather than returning a pointer to the
beginning of the last path name element. This simplifies the function
a bit and makes it integrate better with its caller, which is now also
strbuf-based. Rename the function accordingly and a bit less tersely.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change resolve_symlink() to take a strbuf rather than a string as
parameter. This simplifies the code and removes an arbitrary pathname
length restriction. It also means that lock_file's filename field no
longer needs to be initialized to a large size.
Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lockfile: change lock_file::filename into a strbuf
For now, we still make sure to allocate at least PATH_MAX characters
for the strbuf because resolve_symlink() doesn't know how to expand
the space for its return value. (That will be fixed in a moment.)
Another alternative would be to just use a strbuf as scratch space in
lock_file() but then store a pointer to the naked string in struct
lock_file. But lock_file objects are often reused. By reusing the
same strbuf, we can avoid having to reallocate the string most times
when a lock_file object is reused.
Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
try_merge_strategy(): use a statically-allocated lock_file object
Even the one lockfile object needn't be allocated each time the
function is called. Instead, define one statically-allocated
lock_file object and reuse it for every call.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
By the time the "if" block is entered, the lock_file instance from the
main function block is no longer in use, so re-use that one instead of
allocating a second one.
Note that the "lock" variable in the "if" block shadowed the "lock"
variable at function scope, so the only change needed is to remove the
inner definition.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function remove_lock_file_on_signal() is used as a signal handler.
It is not realistic to make the signal handler conform strictly to the
C standard, which is very restrictive about what a signal handler is
allowed to do. But let's increase the likelihood that it will work:
The lock_file_list global variable and several fields from struct
lock_file are used by the signal handler. Declare those values
"volatile" to (1) force the main process to write the values to RAM
promptly, and (2) prevent updates to these fields from being reordered
in a way that leaves an opportunity for a jump to the signal handler
while the object is in an inconsistent state.
We don't mark the filename field volatile because that would prevent
the use of strcpy(), and it is anyway unlikely that a compiler
re-orders a strcpy() call across other expressions. So in practice it
should be possible to get away without "volatile" in the "filename"
case.
Suggested-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Because remove_lock_file() can be called any time by the signal
handler, it is important that any lock_file objects that are in the
lock_file_list are always in a valid state. And since lock_file
objects are often reused (but are never removed from lock_file_list),
that means we have to be careful whenever mutating a lock_file object
to always keep it in a well-defined state.
This was formerly not the case, because part of the state was encoded
by setting lk->filename to the empty string vs. a valid filename. It
is wrong to assume that this string can be updated atomically; for
example, even
strcpy(lk->filename, value)
is unsafe. But the old code was even more reckless; for example,
strcpy(lk->filename, path);
if (!(flags & LOCK_NODEREF))
resolve_symlink(lk->filename, max_path_len);
strcat(lk->filename, ".lock");
During the call to resolve_symlink(), lk->filename contained the name
of the file that was being locked, not the name of the lockfile. If a
signal were raised during that interval, then the signal handler would
have deleted the valuable file!
We could probably continue to use the filename field to encode the
state by being careful to write characters 1..N-1 of the filename
first, and then overwrite the NUL at filename[0] with the first
character of the filename, but that would be awkward and error-prone.
So, instead of using the filename field to determine whether the
lock_file object is active, add a new field "lock_file::active" for
this purpose. Be careful to set this field only when filename really
contains the name of a file that should be deleted on cleanup.
Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
After commit_lock_file() is called, then the lock_file object is
necessarily either committed or rolled back. So there is no need to
call rollback_lock_file() again in either of these cases.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
dump_marks(): remove a redundant call to rollback_lock_file()
When commit_lock_file() fails, it now always calls
rollback_lock_file() internally, so there is no need to call that
function here.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Document the behavior of commit_lock_file() when it fails, namely
that it rolls back the lock_file object and sets errno
appropriately.
* Document the behavior of rollback_lock_file() when called for a
lock_file object that has already been committed or rolled back,
namely that it is a NOOP.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit_lock_file(): rollback lock file on failure to rename
If rename() fails, call rollback_lock_file() to delete the lock file
(in case it is still present) and reset the filename field to the
empty string so that the lockfile object is left in a valid state.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If closing an open lockfile fails, then we cannot be sure of the
contents of the lockfile, so there is nothing sensible to do but
delete it. This change also insures that the lock_file object is left
in a defined state in this error path (namely, unlocked).
The only caller that is ultimately affected by this change is
try_merge_strategy() -> write_locked_index(), which can call
close_lock_file() via various execution paths. This caller uses a
static lock_file object which previously could have been reused after
a failed close_lock_file() even though it was still in locked state.
This change causes the lock_file object to be unlocked on failure,
thus fixing this error-handling path.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit_lock_file(): die() if called for unlocked lockfile object
It was previously a bug to call commit_lock_file() with a lock_file
object that was not active (an illegal access would happen within the
function). It was presumably never done, but this would be an easy
programming error to overlook. So before continuing, do a consistency
check that the lock_file object really is locked.
Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lock_file(): exit early if lockfile cannot be opened
This is a bit easier to read than the old version, which nested part
of the non-error code in an "if" block.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
prepare_index(): declare return value to be (const char *)
Declare the return value to be const to make it clear that we aren't
giving callers permission to write over the string that it points at.
(The return value is the filename field of a struct lock_file, which
can be used by a signal handler at any time and therefore shouldn't be
tampered with.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
delete_ref_loose(): don't muck around in the lock_file's filename
It's bad manners. Especially since there could be a signal during the
call to unlink_or_warn(), in which case the signal handler will see
the wrong filename and delete the reference file, leaving the lockfile
behind.
So make our own copy to work with.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lockfile.c: document the various states of lock_file objects
Document the valid states of lock_file objects, how they get into each
state, and how the state is encoded in the object's fields.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lock_file(): always initialize and register lock_file object
The purpose of this change is to make the state diagram for
lock_file objects simpler and deterministic.
If locking fails, lock_file() sometimes leaves the lock_file object
partly initialized, but sometimes not. It sometimes registers the
object in lock_file_list, but sometimes not. This makes the state
diagram for lock_file objects effectively indeterministic and hard
to reason about. A future patch will also change the filename field
into a strbuf, which needs more involved initialization, so it will
become even more important that the state of a lock_file object is
well-defined after a failed attempt to lock.
The ambiguity doesn't currently have any ill effects, because
lock_file objects cannot be removed from the lock_file_list anyway.
But to make it easier to document and reason about the code, make
this behavior consistent: *always* initialize the lock_file object
and *always* register it in lock_file_list the first time it is
used, regardless of whether an error occurs.
While we're at it, make sure that all of the lock_file fields are
initialized to values appropriate for an unlocked object; the caller
is only responsible for making sure that on_list is set to zero before
the first time it is used.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
hold_lock_file_for_append(): release lock on errors
If there is an error copying the old contents to the lockfile, roll
back the lockfile before exiting so that the lockfile is not held
until process cleanup.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
lockfile: unlock file if lockfile permissions cannot be adjusted
If the call to adjust_shared_perm() fails, lock_file returns -1, which
to the caller looks like any other failure to lock the file. So in
this case, roll back the lockfile before returning so that the lock
file is deleted immediately and the lockfile object is left in a
predictable state (namely, unlocked). Previously, the lockfile was
retained until process cleanup in this situation.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When rolling back the lockfile, call close_lock_file() so that the
lock_file's fd field gets set back to -1. This keeps the lock_file
object in a valid state, which is important because these objects are
allowed to be reused. It also makes it unnecessary to check whether
the file has already been closed, because close_lock_file() takes care
of that.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rollback_lock_file(): exit early if lock is not active
Eliminate a layer of nesting.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rollback_lock_file(): do not clear filename redundantly
It is only necessary to clear the lock_file's filename field if it was
not already clear.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
close_lock_file(): exit (successfully) if file is already closed
Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Document a couple more functions and the flags argument as used by
hold_lock_file_for_update() and hold_lock_file_for_append().
Reorganize the document to make it more accessible.
Helped-by: Jonathan Nieder <jrnieder@gmail.com> Helped-by: Junio Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
unable_to_lock_die(): rename function from unable_to_lock_index_die()
This function is used for other things besides the index, so rename it
accordingly.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the user-supplied hostname can't be found then we should not use it.
We already avoid doing that in the non-NO_IPV6 case by checking if the
return value of getaddrinfo() is zero (success). Do the same in the
NO_IPV6 case and make sure the return value of gethostbyname() isn't
NULL before dereferencing this pointer.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the first 18 bytes of the SHA1's of all entries are the same then
sha1_pos() dies and reports that the lower and upper limits of the
binary search were the same that this wasn't supposed to happen. This
is wrong because the remaining two bytes could still differ.
Furthermore: It wouldn't be a problem if they actually were the same,
i.e. if all entries have the same SHA1. The code already handles
duplicates just fine. Simply remove the erroneous check.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Initialize the AWK variable c to make sure the shell variable
subtree_count always contains a numerical value, in order to keep the
subsequently called printf happy.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git merge-recursive" had a small bug that could have made it
mishandle "one side deleted, the other side did not touch it" in a
rare corner case, where the other side actually did touch to cause
the blob object names to be different but both blobs before and
after the change normalize to the same (e.g. correcting mistake to
check in a blob with CRLF line endings by replacing it with another
blob that records the same contents with LF line endings).
We used to get confused when a process called us with SIGPIPE
ignored; we do want to die with SIGPIPE when the output is not
read by default, and do ignore the signal when appropriate.
* pr/use-default-sigpipe-setting:
mingw.h: add dummy functions for sigset_t operations
unblock and unignore SIGPIPE
* so/rebase-doc:
Documentation/git-rebase.txt: <upstream> must be given to specify <branch>
Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op
Some MUAs mangled a line in a message that begins with "From " to
">From " when writing to a mailbox file and feeding such an input
to "git am" used to lose such a line.
* jk/mbox-from-line:
mailinfo: work around -Wstring-plus-int warning
mailinfo: make ">From" in-body header check more robust
"rev-parse --verify --quiet $name" is meant to quietly exit with a
non-zero status when $name is not a valid object name, but still
gave error messages in some cases.
* da/rev-parse-verify-quiet:
stash: prefer --quiet over shell redirection of the standard error stream
refs: make rev-parse --quiet actually quiet
t1503: use test_must_be_empty
Documentation: a note about stdout for git rev-parse --verify --quiet
The pretty-format specifier "%d", which expanded to " (tagname)"
for a tagged commit, gained a cousin "%D" that just gives the
"tagname" without frills.
* hj/pretty-naked-decoration:
pretty: add %D format specifier
Documentation/git-rebase.txt: document when --fork-point is auto-enabled
Running "git rebase" without giving a specific commit with respect
to which the operation is done enables --fork-point mode, while
telling the command to rebase with respect to a specific commit,
i.e. "git rebase <upstream>" does not.
This was not mentioned in the DESCRIPTION section of the manual
page, even though the case of omitted <upstream> was otherwise
discussed. That in turn made actual behavior of vanilla "git
rebase" hardly discoverable.
While we are at it, clarify the --fork-point description itself as
well.
Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use write_script to create the helper "askpass" script, instead of
hand-creating it with hardcoded "#!/bin/sh" to make sure we use the
shell the user told us to use.
Signed-off-by: Ben Walton <bdwalton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'maint' of git://github.com/git-l10n/git-po into maint
* 'maint' of git://github.com/git-l10n/git-po:
l10n: de.po: use comma before "um"
l10n: de.po: change Email to E-Mail
po/TEAMS: add new member to German translation team