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

2014-09-22 Thread Michael Haggerty
On 09/16/2014 10:25 PM, Jonathan Nieder wrote:
 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

OK, in the next reroll I will revise the documentation pretty
thoroughly. Please let me know what you think.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-09-16 Thread Jonathan Nieder
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html