Merge branch 'mh/lockfile'

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
  ...

Documentation/technical/api-lockfile.txt

@@ -3,72 +3,206 @@ lockfile API
 
 The lockfile API serves two purposes:
 
-* Mutual exclusion.  When we write out a new index file, first
-  we create a new file `$GIT_DIR/index.lock`, write the new
-  contents into it, and rename it to the final destination
-  `$GIT_DIR/index`.  We try to create the `$GIT_DIR/index.lock`
-  file with O_EXCL so that we can notice and fail when somebody
-  else is already trying to update the index file.
-
-* Automatic cruft removal.  After we create the "lock" file, we
-  may decide to `die()`, and we would want to make sure that we
-  remove the file that has not been committed to its final
-  destination.  This is done by remembering the lockfiles we
-  created in a linked list and cleaning them up from an
-  `atexit(3)` handler.  Outstanding lockfiles are also removed
-  when the program dies on a signal.
+* Mutual exclusion and atomic file updates. When we want to change a
+  file, we create a lockfile `<filename>.lock`, write the new file
+  contents into it, and then rename the lockfile to its final
+  destination `<filename>`. We create the `<filename>.lock` file with
+  `O_CREAT|O_EXCL` so that we can notice and fail if somebody else has
+  already locked the file, then atomically rename the lockfile to its
+  final destination to commit the changes and unlock the file.
+
+* Automatic cruft removal. If the program exits after we lock a file
+  but before the changes have been committed, we want to make sure
+  that we remove the lockfile. This is done by remembering the
+  lockfiles we have created in a linked list and setting up an
+  `atexit(3)` handler and a signal handler that clean up the
+  lockfiles. This mechanism ensures that outstanding lockfiles are
+  cleaned up if the program exits (including when `die()` is called)
+  or if the program dies on a signal.
+
+Please note that lockfiles only block other writers. Readers do not
+block, but they are guaranteed to see either the old contents of the
+file or the new contents of the file (assuming that the filesystem
+implements `rename(2)` atomically).
+
+
+Calling sequence
+----------------
+
+The caller:
+
+* Allocates a `struct lock_file` either as a static variable or on the
+  heap, initialized to zeros. Once you use the structure to call the
+  `hold_lock_file_*` family of functions, it belongs to the lockfile
+  subsystem and its storage must remain valid throughout the life of
+  the program (i.e. you cannot use an on-stack variable to hold this
+  structure).
+
+* Attempts to create a lockfile by passing that variable and the path
+  of the final destination (e.g. `$GIT_DIR/index`) to
+  `hold_lock_file_for_update` or `hold_lock_file_for_append`.
+
+* Writes new content for the destination file by writing to the file
+  descriptor returned by those functions (also available via
+  `lock->fd`).
+
+When finished writing, the caller can:
+
+* Close the file descriptor and rename the lockfile to its final
+  destination by calling `commit_lock_file` or `commit_lock_file_to`.
+
+* Close the file descriptor and remove the lockfile by calling
+  `rollback_lock_file`.
+
+* Close the file descriptor without removing or renaming the lockfile
+  by calling `close_lock_file`, and later call `commit_lock_file`,
+  `commit_lock_file_to`, `rollback_lock_file`, or `reopen_lock_file`.
+
+Even after the lockfile is committed or rolled back, the `lock_file`
+object must not be freed or altered by the caller. However, it may be
+reused; just pass it to another call of `hold_lock_file_for_update` or
+`hold_lock_file_for_append`.
+
+If the program exits before you have called one of `commit_lock_file`,
+`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an
+`atexit(3)` handler will close and remove the lockfile, rolling back
+any uncommitted changes.
+
+If you need to close the file descriptor you obtained from a
+`hold_lock_file_*` function yourself, do so by calling
+`close_lock_file`. You should never call `close(2)` yourself!
+Otherwise the `struct lock_file` structure would still think that the
+file descriptor needs to be closed, and a commit or rollback would
+result in duplicate calls to `close(2)`. Worse yet, if you `close(2)`
+and then later open another file descriptor for a completely different
+purpose, then a commit or rollback might close that unrelated file
+descriptor.
+
+
+Error handling
+--------------
+
+The `hold_lock_file_*` functions return a file descriptor on success
+or -1 on failure (unless `LOCK_DIE_ON_ERROR` is used; see below). On
+errors, `errno` describes the reason for failure. Errors can be
+reported by passing `errno` to one of the following helper functions:
+
+unable_to_lock_message::
+
+	Append an appropriate error message to a `strbuf`.
+
+unable_to_lock_error::
+
+	Emit an appropriate error message using `error()`.
+
+unable_to_lock_die::
+
+	Emit an appropriate error message and `die()`.
+
+Similarly, `commit_lock_file`, `commit_lock_file_to`, and
+`close_lock_file` return 0 on success. On failure they set `errno`
+appropriately, do their best to roll back the lockfile, and return -1.
+
+
+Flags
+-----
+
+The following flags can be passed to `hold_lock_file_for_update` or
+`hold_lock_file_for_append`:
+
+LOCK_NO_DEREF::
+
+	Usually symbolic links in the destination path are resolved
+	and the lockfile is created by adding ".lock" to the resolved
+	path. If `LOCK_NO_DEREF` is set, then the lockfile is created
+	by adding ".lock" to the path argument itself. This option is
+	used, for example, when locking a symbolic reference, which
+	for backwards-compatibility reasons can be a symbolic link
+	containing the name of the referred-to-reference.
+
+LOCK_DIE_ON_ERROR::
+
+	If a lock is already taken for the file, `die()` with an error
+	message. If this option is not specified, trying to lock a
+	file that is already locked returns -1 to the caller.
 
 
 The functions
 -------------
 
 hold_lock_file_for_update::
 
-	Take a pointer to `struct lock_file`, the filename of
-	the final destination (e.g. `$GIT_DIR/index`) and a flag
-	`die_on_error`.  Attempt to create a lockfile for the
-	destination and return the file descriptor for writing
-	to the file.  If `die_on_error` flag is true, it dies if
-	a lock is already taken for the file; otherwise it
-	returns a negative integer to the caller on failure.
+	Take a pointer to `struct lock_file`, the path of the file to
+	be locked (e.g. `$GIT_DIR/index`) and a flags argument (see
+	above). Attempt to create a lockfile for the destination and
+	return the file descriptor for writing to the file.
+
+hold_lock_file_for_append::
+
+	Like `hold_lock_file_for_update`, but before returning copy
+	the existing contents of the file (if any) to the lockfile and
+	position its write pointer at the end of the file.
+
+get_locked_file_path::
+
+	Return the path of the file that is locked by the specified
+	lock_file object. The caller must free the memory.
 
 commit_lock_file::
 
-	Take a pointer to the `struct lock_file` initialized
-	with an earlier call to `hold_lock_file_for_update()`,
-	close the file descriptor and rename the lockfile to its
-	final destination.  Returns 0 upon success, a negative
-	value on failure to close(2) or rename(2).
+	Take a pointer to the `struct lock_file` initialized with an
+	earlier call to `hold_lock_file_for_update` or
+	`hold_lock_file_for_append`, close the file descriptor, and
+	rename the lockfile to its final destination. Return 0 upon
+	success. On failure, roll back the lock file and return -1,
+	with `errno` set to the value from the failing call to
+	`close(2)` or `rename(2)`. It is a bug to call
+	`commit_lock_file` for a `lock_file` object that is not
+	currently locked.
+
+commit_lock_file_to::
+
+	Like `commit_lock_file()`, except that it takes an explicit
+	`path` argument to which the lockfile should be renamed. The
+	`path` must be on the same filesystem as the lock file.
 
 rollback_lock_file::
 
-	Take a pointer to the `struct lock_file` initialized
-	with an earlier call to `hold_lock_file_for_update()`,
-	close the file descriptor and remove the lockfile.
+	Take a pointer to the `struct lock_file` initialized with an
+	earlier call to `hold_lock_file_for_update` or
+	`hold_lock_file_for_append`, close the file descriptor and
+	remove the lockfile. It is a NOOP to call
+	`rollback_lock_file()` for a `lock_file` object that has
+	already been committed or rolled back.
 
 close_lock_file::
-	Take a pointer to the `struct lock_file` initialized
-	with an earlier call to `hold_lock_file_for_update()`,
-	and close the file descriptor.  Returns 0 upon success,
-	a negative value on failure to close(2).
-
-Because the structure is used in an `atexit(3)` handler, its
-storage has to stay throughout the life of the program.  It
-cannot be an auto variable allocated on the stack.
-
-Call `commit_lock_file()` or `rollback_lock_file()` when you are
-done writing to the file descriptor.  If you do not call either
-and simply `exit(3)` from the program, an `atexit(3)` handler
-will close and remove the lockfile.
-
-If you need to close the file descriptor you obtained from
-`hold_lock_file_for_update` function yourself, do so by calling
-`close_lock_file()`.  You should never call `close(2)` yourself!
-Otherwise the `struct
-lock_file` structure still remembers that the file descriptor
-needs to be closed, and a later call to `commit_lock_file()` or
-`rollback_lock_file()` will result in duplicate calls to
-`close(2)`.  Worse yet, if you `close(2)`, open another file
-descriptor for completely different purpose, and then call
-`commit_lock_file()` or `rollback_lock_file()`, they may close
-that unrelated file descriptor.
+
+	Take a pointer to the `struct lock_file` initialized with an
+	earlier call to `hold_lock_file_for_update` or
+	`hold_lock_file_for_append`, and close the file descriptor.
+	Return 0 upon success. On failure to `close(2)`, return a
+	negative value and roll back the lock file. Usually
+	`commit_lock_file`, `commit_lock_file_to`, or
+	`rollback_lock_file` should eventually be called if
+	`close_lock_file` succeeds.
+
+reopen_lock_file::
+
+	Re-open a lockfile that has been closed (using
+	`close_lock_file`) but not yet committed or rolled back. This
+	can be used to implement a sequence of operations like the
+	following:
+
+	* Lock file.
+
+	* Write new contents to lockfile, then `close_lock_file` to
+	  cause the contents to be written to disk.
+
+	* Pass the name of the lockfile to another program to allow it
+	  (and nobody else) to inspect the contents you wrote, while
+	  still holding the lock yourself.
+
+	* `reopen_lock_file` to reopen the lockfile. Make further
+	  updates to the contents.
+
+	* `commit_lock_file` to make the final version permanent.

builtin/add.c

@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "lockfile.h"
 #include "dir.h"
 #include "pathspec.h"
 #include "exec_cmd.h"

builtin/apply.c

@@ -7,6 +7,7 @@
  *
  */
 #include "cache.h"
+#include "lockfile.h"
 #include "cache-tree.h"
 #include "quote.h"
 #include "blob.h"

builtin/checkout-index.c

@@ -5,7 +5,7 @@
  *
  */
 #include "builtin.h"
-#include "cache.h"
+#include "lockfile.h"
 #include "quote.h"
 #include "cache-tree.h"
 #include "parse-options.h"

builtin/checkout.c

@@ -1,5 +1,5 @@
-#include "cache.h"
 #include "builtin.h"
+#include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
 #include "commit.h"

builtin/clone.c

@@ -9,6 +9,7 @@
  */
 
 #include "builtin.h"
+#include "lockfile.h"
 #include "parse-options.h"
 #include "fetch-pack.h"
 #include "refs.h"

builtin/commit.c

@@ -6,6 +6,7 @@
  */
 
 #include "cache.h"
+#include "lockfile.h"
 #include "cache-tree.h"
 #include "color.h"
 #include "dir.h"
@@ -315,8 +316,8 @@ static void refresh_cache_or_die(int refresh_flags)
 		die_resolve_conflict("commit");
 }
 
-static char *prepare_index(int argc, const char **argv, const char *prefix,
-			   const struct commit *current_head, int is_status)
+static const char *prepare_index(int argc, const char **argv, const char *prefix,
+				 const struct commit *current_head, int is_status)
 {
 	struct string_list partial;
 	struct pathspec pathspec;
@@ -341,7 +342,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 			die(_("unable to create temporary index"));
 
 		old_index_env = getenv(INDEX_ENVIRONMENT);
-		setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
+		setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);
 
 		if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
 			die(_("interactive add failed"));
@@ -352,7 +353,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 			unsetenv(INDEX_ENVIRONMENT);
 
 		discard_cache();
-		read_cache_from(index_lock.filename);
+		read_cache_from(index_lock.filename.buf);
 		if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
 			if (reopen_lock_file(&index_lock) < 0)
 				die(_("unable to write index file"));
@@ -362,7 +363,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 			warning(_("Failed to update main cache tree"));
 
 		commit_style = COMMIT_NORMAL;
-		return index_lock.filename;
+		return index_lock.filename.buf;
 	}
 
 	/*
@@ -385,7 +386,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
 			die(_("unable to write new_index file"));
 		commit_style = COMMIT_NORMAL;
-		return index_lock.filename;
+		return index_lock.filename.buf;
 	}
 
 	/*
@@ -472,9 +473,9 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		die(_("unable to write temporary index file"));
 
 	discard_cache();
-	read_cache_from(false_lock.filename);
+	read_cache_from(false_lock.filename.buf);
 
-	return false_lock.filename;
+	return false_lock.filename.buf;
 }
 
 static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,

builtin/describe.c

@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "lockfile.h"
 #include "commit.h"
 #include "tag.h"
 #include "refs.h"

builtin/diff.c

@@ -4,6 +4,7 @@
  * Copyright (c) 2006 Junio C Hamano
  */
 #include "cache.h"
+#include "lockfile.h"
 #include "color.h"
 #include "commit.h"
 #include "blob.h"

builtin/gc.c

@@ -11,7 +11,7 @@
  */
 
 #include "builtin.h"
-#include "cache.h"
+#include "lockfile.h"
 #include "parse-options.h"
 #include "run-command.h"
 #include "sigchain.h"