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@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html