From: Michael Haggerty <mhagger@alum•mit.edu>
To: Jonathan Nieder <jrnieder@gmail•com>
Cc: "Junio C Hamano" <gitster@pobox•com>,
"Johannes Sixt" <j6t@kdbg•org>,
"Torsten Bögershausen" <tboegi@web•de>,
"Jeff King" <peff@peff•net>,
"Ronnie Sahlberg" <sahlberg@google•com>,
git@vger•kernel.org
Subject: Re: [PATCH v5 02/35] api-lockfile: expand the documentation
Date: Mon, 22 Sep 2014 16:13:39 +0200 [thread overview]
Message-ID: <54202E93.4050707@alum.mit.edu> (raw)
In-Reply-To: <20140916202525.GB29050@google.com>
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
mhagger@alum•mit.edu
next prev parent reply other threads:[~2014-09-22 14:13 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-16 19:33 [PATCH v5 00/35] Lockfile correctness and refactoring Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 01/35] unable_to_lock_die(): rename function from unable_to_lock_index_die() Michael Haggerty
2014-09-16 19:52 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 02/35] api-lockfile: expand the documentation Michael Haggerty
2014-09-16 20:25 ` Jonathan Nieder
2014-09-22 14:13 ` Michael Haggerty [this message]
2014-09-16 19:33 ` [PATCH v5 03/35] rollback_lock_file(): do not clear filename redundantly Michael Haggerty
2014-09-16 20:37 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 04/35] rollback_lock_file(): exit early if lock is not active Michael Haggerty
2014-09-16 20:38 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 05/35] rollback_lock_file(): set fd to -1 Michael Haggerty
2014-09-16 20:38 ` Jonathan Nieder
2014-09-16 20:39 ` Jonathan Nieder
2014-09-17 15:02 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 06/35] lockfile: unlock file if lockfile permissions cannot be adjusted Michael Haggerty
2014-09-16 20:42 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 07/35] hold_lock_file_for_append(): release lock on errors Michael Haggerty
2014-09-16 20:48 ` Jonathan Nieder
2014-09-17 15:39 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 08/35] lock_file(): always add lock_file object to lock_file_list Michael Haggerty
2014-09-16 20:57 ` Jonathan Nieder
2014-09-17 16:10 ` Michael Haggerty
2014-09-18 4:32 ` Torsten Bögershausen
2014-09-18 7:47 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects Michael Haggerty
2014-09-16 21:03 ` Jonathan Nieder
2014-09-22 15:20 ` Michael Haggerty
2014-09-22 22:34 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 10/35] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN Michael Haggerty
2014-09-16 21:05 ` Jonathan Nieder
2014-09-22 15:25 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 11/35] delete_ref_loose(): don't muck around in the lock_file's filename Michael Haggerty
2014-09-16 21:11 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 12/35] prepare_index(): declare return value to be (const char *) Michael Haggerty
2014-09-16 21:17 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 13/35] write_packed_entry_fn(): convert cb_data into a (const int *) Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 14/35] lock_file(): exit early if lockfile cannot be opened Michael Haggerty
2014-09-16 22:12 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 15/35] remove_lock_file(): call rollback_lock_file() Michael Haggerty
2014-09-16 22:13 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 16/35] commit_lock_file(): inline temporary variable Michael Haggerty
2014-09-16 22:16 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 17/35] commit_lock_file(): die() if called for unlocked lockfile object Michael Haggerty
2014-09-16 22:17 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 18/35] commit_lock_file(): if close fails, roll back Michael Haggerty
2014-09-16 22:19 ` Jonathan Nieder
2014-09-23 11:30 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 19/35] commit_lock_file(): rollback lock file on failure to rename Michael Haggerty
2014-09-16 22:22 ` Jonathan Nieder
2014-09-23 11:57 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 20/35] api-lockfile: document edge cases Michael Haggerty
2014-09-16 22:23 ` Jonathan Nieder
2014-09-23 12:56 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 21/35] dump_marks(): remove a redundant call to rollback_lock_file() Michael Haggerty
2014-09-16 22:24 ` Jonathan Nieder
2014-09-16 19:33 ` [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid " Michael Haggerty
2014-09-16 22:28 ` Jonathan Nieder
2014-09-23 13:08 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 23/35] lockfile: avoid transitory invalid states Michael Haggerty
2014-09-16 22:45 ` Jonathan Nieder
2014-09-23 13:40 ` Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 24/35] struct lock_file: declare some fields volatile Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 25/35] try_merge_strategy(): remove redundant lock_file allocation Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 26/35] try_merge_strategy(): use a statically-allocated lock_file object Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 27/35] commit_lock_file(): use a strbuf to manage temporary space Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 28/35] Change lock_file::filename into a strbuf Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 29/35] resolve_symlink(): use a strbuf for internal scratch space Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 30/35] resolve_symlink(): take a strbuf parameter Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 31/35] trim_last_path_component(): replace last_path_elm() Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 32/35] Extract a function commit_lock_file_to() Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 33/35] Rename LOCK_NODEREF to LOCK_NO_DEREF Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 34/35] lockfile.c: rename static functions Michael Haggerty
2014-09-16 19:33 ` [PATCH v5 35/35] get_locked_file_path(): new function Michael Haggerty
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54202E93.4050707@alum.mit.edu \
--to=mhagger@alum$(echo .)mit.edu \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=j6t@kdbg$(echo .)org \
--cc=jrnieder@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=sahlberg@google$(echo .)com \
--cc=tboegi@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox