git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Sep 16, 2014, 20:25 UTC
Message-ID
<20140916202525.GB29050@google.com>
In-Reply-To
<1410896036-12750-3-git-send-email-mhagger@alum.mit.edu>
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.
[...]
Show 12 quoted lines
> --- 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

Previous: Michael HaggertyNext: Michael Haggerty
Message 5 of 73 in “Lockfile correctness and refactoring”
  1. 00/35 Lockfile correctness and refactoringMichael Haggerty, Sep 16, 2014
  2. 01/35 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Sep 16, 2014
  3. Jonathan NiederSep 16, 2014
  4. 02/35 api-lockfile: expand the documentationMichael Haggerty, Sep 16, 2014
  5. Jonathan NiederSep 16, 2014
  6. Michael HaggertySep 22, 2014
  7. 03/35 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Sep 16, 2014
  8. Jonathan NiederSep 16, 2014
  9. 04/35 rollback_lock_file(): exit early if lock is not activeMichael Haggerty, Sep 16, 2014
  10. Jonathan NiederSep 16, 2014
  11. 05/35 rollback_lock_file(): set fd to -1Michael Haggerty, Sep 16, 2014
  12. Jonathan NiederSep 16, 2014
  13. Jonathan NiederSep 16, 2014
  14. Michael HaggertySep 17, 2014
  15. 06/35 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Sep 16, 2014
  16. Jonathan NiederSep 16, 2014
  17. 07/35 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Sep 16, 2014
  18. Jonathan NiederSep 16, 2014
  19. Michael HaggertySep 17, 2014
  20. 08/35 lock_file(): always add lock_file object to lock_file_listMichael Haggerty, Sep 16, 2014
  21. Jonathan NiederSep 16, 2014
  22. Michael HaggertySep 17, 2014
  23. Torsten BögershausenSep 18, 2014
  24. Michael HaggertySep 18, 2014
  25. 09/35 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Sep 16, 2014
  26. Jonathan NiederSep 16, 2014
  27. Michael HaggertySep 22, 2014
  28. Jonathan NiederSep 22, 2014
  29. 10/35 cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LENMichael Haggerty, Sep 16, 2014
  30. Jonathan NiederSep 16, 2014
  31. Michael HaggertySep 22, 2014
  32. 11/35 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Sep 16, 2014
  33. Jonathan NiederSep 16, 2014
  34. 12/35 prepare_index(): declare return value to be (const char *)Michael Haggerty, Sep 16, 2014
  35. Jonathan NiederSep 16, 2014
  36. 13/35 write_packed_entry_fn(): convert cb_data into a (const int *)Michael Haggerty, Sep 16, 2014
  37. 14/35 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Sep 16, 2014
  38. Jonathan NiederSep 16, 2014
  39. 15/35 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Sep 16, 2014
  40. Jonathan NiederSep 16, 2014
  41. 16/35 commit_lock_file(): inline temporary variableMichael Haggerty, Sep 16, 2014
  42. Jonathan NiederSep 16, 2014
  43. 17/35 commit_lock_file(): die() if called for unlocked lockfile objectMichael Haggerty, Sep 16, 2014
  44. Jonathan NiederSep 16, 2014
  45. 18/35 commit_lock_file(): if close fails, roll backMichael Haggerty, Sep 16, 2014
  46. Jonathan NiederSep 16, 2014
  47. Michael HaggertySep 23, 2014
  48. 19/35 commit_lock_file(): rollback lock file on failure to renameMichael Haggerty, Sep 16, 2014
  49. Jonathan NiederSep 16, 2014
  50. Michael HaggertySep 23, 2014
  51. 20/35 api-lockfile: document edge casesMichael Haggerty, Sep 16, 2014
  52. Jonathan NiederSep 16, 2014
  53. Michael HaggertySep 23, 2014
  54. 21/35 dump_marks(): remove a redundant call to rollback_lock_file()Michael Haggerty, Sep 16, 2014
  55. Jonathan NiederSep 16, 2014
  56. 22/35 git_config_set_multivar_in_file(): avoid call to rollback_lock_file()Michael Haggerty, Sep 16, 2014
  57. Jonathan NiederSep 16, 2014
  58. Michael HaggertySep 23, 2014
  59. 23/35 lockfile: avoid transitory invalid statesMichael Haggerty, Sep 16, 2014
  60. Jonathan NiederSep 16, 2014
  61. Michael HaggertySep 23, 2014
  62. 24/35 struct lock_file: declare some fields volatileMichael Haggerty, Sep 16, 2014
  63. 25/35 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Sep 16, 2014
  64. 26/35 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Sep 16, 2014
  65. 27/35 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Sep 16, 2014
  66. 28/35 Change lock_file::filename into a strbufMichael Haggerty, Sep 16, 2014
  67. 29/35 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Sep 16, 2014
  68. 30/35 resolve_symlink(): take a strbuf parameterMichael Haggerty, Sep 16, 2014
  69. 31/35 trim_last_path_component(): replace last_path_elm()Michael Haggerty, Sep 16, 2014
  70. 32/35 Extract a function commit_lock_file_to()Michael Haggerty, Sep 16, 2014
  71. 33/35 Rename LOCK_NODEREF to LOCK_NO_DEREFMichael Haggerty, Sep 16, 2014
  72. 34/35 lockfile.c: rename static functionsMichael Haggerty, Sep 16, 2014
  73. 35/35 get_locked_file_path(): new functionMichael Haggerty, Sep 16, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.