Re: [PATCH v5 02/35] api-lockfile: expand the documentation

[Jonathan Nieder <jrnieder@gmail.com>]

Michael Haggerty wrote:

> Document a couple more functions and the flags argument as used by
> hold_lock_file_for_update() and hold_lock_file_for_append().

Thanks.

[...]
> --- a/Documentation/technical/api-lockfile.txt
> +++ b/Documentation/technical/api-lockfile.txt
> @@ -28,9 +28,39 @@ hold_lock_file_for_update::
>  	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.
> +	to the file.  The flags parameter is a combination of
> ++
> +--

Context: this document has structure

	lockfile API
	============

	Explanation of purpose (nice!).

	The functions
	-------------

	Quick descriptions of each of the four functions
	`hold_lock_file_for_update`, `commit_lock_file`,
	`rollback_lock_file`, `close_lock_file`.

	Reminder about lifetime of the lock_file structure.

	Description of cleanup convention (thou shalt either
	commit or roll back; if you forget to, the atexit
	handler will roll back for you).

	Long warning about the harder use cases.  The above
	"thou shalt" was a lie --- you can also
	close_lock_file if you know what you're doing
	[jn: why is that function part of the public API?].

What's nice about the existing structure is that you can get
a sense of how to use the API at a glance.  Would there be a
way to add this extra information while preserving that property?

E.g.:

	lockfile API
	============

	Nice brief explanation of purpose ("is this the API
	I want to use?"), as before.

	Calling sequence
	----------------

	The caller:

	* Allocates a variable `struct lock_file lock` in the bss
	section or heap.  Because the `lock_file` 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.

	* Attempts to create a lockfile by passing that variable and
	the filename of the final destination (e.g. `$GIT_DIR/index`)
	to `hold_lock_file_for_update` or `hold_lock_file_for_append`.
	+
	If the `die_on_error` flag is true, git dies if a lock is
	already taken for the file.

	* Writes new content for the destination file by writing to
	`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`.

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

	If you do not call one of `commit_lock_file`,
	`rollback_lock_file`, and `close_lock_file` and instead
	simply `exit(3)` from the program, an `atexit(3)` handler will
	close and remove the lockfile.

	You should never call `close(2)` on `lock->fd` yourself~
	Otherwise the ...

	Error handling
	--------------

	Functions return 0 on success, -1 on failure.  errno is?
	isn't? meaningful on error.

	... description of unable_to_lock_error and unable_to_lock_die
	here ...

	Flags
	-----

	LOCK_NODEREF::

		Usually symbolic links in the destination path are
		resolved and the lockfile is created by adding ".lock"
		to the resolved path.  If `LOCK_NODEREF` is set, then
		the lockfile is created by adding ".lock" to the path
		argument itself.

What is the user-visible effect of that flag?  When would I want
to pass that flag, and when wouldn't I?

	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 hold a lock file that is already taken will
		return -1 to the caller.

Sensible?
Jonathan