Re: [PATCH v14 01/11] trailer: add data structures and basic functions

On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: +/* Get the length of buf from its beginning until its last alphanumeric character */ That makes it sound as if feeding abc%de#f@ to the function returns 3 for abc,

RE: Administrator Warning

From: Luis A. Beltran Sent: Tuesday, September 16, 2014 6:39 AM To: Luis A. Beltran Subject: Administrator Warning Help desk will undergo unscheduled system maintenance today in order to improve your account. The new Microsoft Outlook Web-access 2014 which will

[PATCH] Documentation/git-rebase.txt: make it explicit in the syntax there is no way to specify branch without upstream.

Current syntax description makes one wonder if there is any syntactic way to distinguish between branch and upstream so that one can specify branch but not upstream. The change makes it explicit that these arguments are in fact positional. Signed-off-by: Sergey Organov sorga...@gmail.com ---

[PATCH v3] pretty: add %D format specifier

Add a new format specifier, '%D' that is identical in behaviour to '%d', except that it does not include the ' (' prefix or ')' suffix provided by '%d'. Signed-off-by: Harry Jeffery ha...@exec64.co.uk --- Documentation/pretty-formats.txt | 6 -- log-tree.c | 24

Re: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

Jeff King p...@peff.net writes: The only cases that I can think of that would be a problem with this strictness are: 1. Somebody writes format-patch output to a file, reads in the mbox using another program, and then writes out the result (munging the mbox From line). And then

Re: Apparent bug in git-gc

From: Jeff King p...@peff.net I have an 11 GB repository. It passes git-fsck (though with a number of dangling objects). But when I run git-gc on it, the file refs/heads/master disappears. That's the expected behavior. Gc runs git pack-refs, which puts an entry into packed-refs and

Re: [PATCH] credential-cache: close stderr in daemon process

Jeff King p...@peff.net writes: Squash this in? Yeah, I did a crude one without _errno() while sending the report; will replace. Thanks. diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index c07a67c..c2f0049 100644 --- a/credential-cache--daemon.c +++

Re: [PATCH v5 00/23] Signed push

On Tue, Sep 16, 2014 at 12:24 AM, Junio C Hamano gits...@pobox.com wrote: A failing test has been added at the end for smart HTTP. It appears that somewhere in the callchain --signed is forgotten and the sending end not to send the certificate for some reason. If somebody with a fresh set of

Re: [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet

David Aguilar dav...@gmail.com writes: This patch now has the t1503 test case squashed into it. It was previously a separate patch, but it makes more sense for them to go together. Yes. diff --git a/refs.c b/refs.c index 2ce5d69..447e339 100644 --- a/refs.c +++ b/refs.c @@ -3108,7

Re: [PATCH] Documentation/git-rebase.txt: make it explicit in the syntax there is no way to specify branch without upstream.

Sergey Organov sorga...@gmail.com writes: Current syntax description makes one wonder if there is any syntactic way to distinguish between branch and upstream so that one can specify branch but not upstream. The change makes it explicit that these arguments are in fact positional. Makes

Re: [PATCH v3] pretty: add %D format specifier

Harry Jeffery ha...@exec64.co.uk writes: Add a new format specifier, '%D' that is identical in behaviour to '%d', except that it does not include the ' (' prefix or ')' suffix provided by '%d'. Signed-off-by: Harry Jeffery ha...@exec64.co.uk Thanks. @@ -196,20 +198,20 @@ void

Re: [PATCH v5 00/23] Signed push

Junio C Hamano gits...@pobox.com writes: A failing test has been added at the end for smart HTTP. It appears that somewhere in the callchain --signed is forgotten and the sending end not to send the certificate for some reason. If somebody with a fresh set of eyes can look into it, that

Re: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

Junio C Hamano gits...@pobox.com writes: I think you forgot to git add mbox.h. That being said, if we did go this route, I do not see any reason to share the code at all. This can be purely a mailinfo.c thing. OK. A reroll coming today when I find time. -- 8 -- From: Jeff King

Re: [PATCH 1/2] add macro REALLOCARRAY

Am 16.09.2014 um 05:04 schrieb Junio C Hamano: On Sun, Sep 14, 2014 at 9:55 AM, René Scharfe l@web.de wrote: +#define REALLOCARRAY(x, alloc) x = xrealloc((x), (alloc) * sizeof(*(x))) I have been wondering if x could be an expression that has an operator that binds weaker than the

[PATCH 1/2] add macro REALLOC_ARRAY

The macro ALLOC_GROW manages several aspects of dynamic memory allocations for arrays: It performs overprovisioning in order to avoid reallocations in future calls, updates the allocation size variable, multiplies the item size and thus allows users to simply specify the item count, performs the

[PATCH 2/2] use REALLOC_ARRAY for changing the allocation size of arrays

Signed-off-by: Rene Scharfe l@web.de --- attr.c | 3 +-- builtin/apply.c| 2 +- builtin/for-each-ref.c | 9 +++-- builtin/index-pack.c | 4 +--- builtin/log.c | 2 +- builtin/merge.c| 2 +- builtin/mv.c | 8

[PATCH v5 07/35] hold_lock_file_for_append(): release lock on errors

If there is an error copying the old contents to the lockfile, roll back the lockfile before exiting so that the lockfile is not held until process cleanup. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()

After commit_lock_file() is called, then the lock_file object is necessarily either committed or rolled back. So there is no need to call rollback_lock_file() again in either of these cases. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- config.c | 14 +++--- 1 file changed, 7

[PATCH v5 10/35] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

There are a few places that use these values, so define constants for them. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- cache.h| 4 lockfile.c | 11 ++- refs.c | 7 --- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cache.h b/cache.h index

[PATCH v5 31/35] trim_last_path_component(): replace last_path_elm()

Rewrite last_path_elm() to take a strbuf parameter and to trim off the last path name element in place rather than returning a pointer to the beginning of the last path name element. This simplifies the function a bit and makes it integrate better with its caller, which is now also strbuf-based.

[PATCH v5 12/35] prepare_index(): declare return value to be (const char *)

Declare the return value to be const to make it clear that we aren't giving callers permission to write over the string that it points at. (The return value is the filename field of a struct lock_file, which can be used by a signal handler at any time and therefore shouldn't be tampered with.)

[PATCH v5 18/35] commit_lock_file(): if close fails, roll back

If closing an open lockfile fails, then we cannot be sure of the contents of the lockfile, so there is nothing sensible to do but delete it. This change also leaves the lock_file object in a defined state in this error path (namely, unlocked). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu

[PATCH v5 21/35] dump_marks(): remove a redundant call to rollback_lock_file()

When commit_lock_file() fails, it now always calls rollback_lock_file() internally, so there is no need to call that function here. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- fast-import.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fast-import.c

[PATCH v5 17/35] commit_lock_file(): die() if called for unlocked lockfile object

It was previously a bug to call commit_lock_file() with a lock_file object that was not active (an illegal access would happen within the function). It was presumably never done, but this would be an easy programming error to overlook. So before continuing, do a consistency check that the

[PATCH v5 33/35] Rename LOCK_NODEREF to LOCK_NO_DEREF

This makes it harder to misread the name as LOCK_NODE_REF. Suggested-by: Torsten Bögershausen tbo...@web.de Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-lockfile.txt | 4 ++-- cache.h | 2 +- lockfile.c

[PATCH v5 25/35] try_merge_strategy(): remove redundant lock_file allocation

By the time the if block is entered, the lock_file instance from the main function block is no longer in use, so re-use that one instead of allocating a second one. Note that the lock variable in the if block shadowed the lock variable at function scope, so the only change needed is to remove the

[PATCH v5 23/35] lockfile: avoid transitory invalid states

Because remove_lock_file() can be called any time by the signal handler, it is important that any lock_file objects that are in the lock_file_list are always in a valid state. And since lock_file objects are often reused (but are never removed from lock_file_list), that means we have to be

[PATCH v5 35/35] get_locked_file_path(): new function

Add a function to return the path of the file that is locked by a lock_file object. This reduces the knowledge that callers have to have about the lock_file layout. Suggested-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Michael Haggerty mhag...@alum.mit.edu ---

[PATCH v5 01/35] unable_to_lock_die(): rename function from unable_to_lock_index_die()

This function is used for other things besides the index, so rename it accordingly. Suggested-by: Jeff King p...@peff.net Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-index.c | 2 +- cache.h| 2 +-

[PATCH v5 00/35] Lockfile correctness and refactoring

Next iteration of my lockfile fixes and refactoring. Thanks to Torsten Bögershausen, Junio, Peff, Ronnie Sahlberg, and Johannes Sixt for their comments about v4. I believe that this series addresses all of the comments from v1 [1], v2 [2], v3 [3], and v4 [4]. Changes since v4: * Rebase to

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

Document a couple more functions and the flags argument as used by hold_lock_file_for_update() and hold_lock_file_for_append(). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-lockfile.txt | 36 +--- 1 file changed, 33

[PATCH v5 13/35] write_packed_entry_fn(): convert cb_data into a (const int *)

This makes it obvious that we have no plans to change the integer pointed to, which is actually the fd field from a struct lock_file. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1

[PATCH v5 04/35] rollback_lock_file(): exit early if lock is not active

Eliminate a layer of nesting. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- lockfile.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lockfile.c b/lockfile.c index a548e08..49179d8 100644 ---

[PATCH v5 14/35] lock_file(): exit early if lockfile cannot be opened

This is a bit easier to read than the old version, which nested part of the non-error code in an if block. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- lockfile.c | 23 +++ 1 file changed, 11 insertions(+), 12

[PATCH v5 03/35] rollback_lock_file(): do not clear filename redundantly

It is only necessary to clear the lock_file's filename field if it was not already clear. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- lockfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lockfile.c

[PATCH v5 19/35] commit_lock_file(): rollback lock file on failure to rename

If rename() fails, call rollback_lock_file() to delete the lock file (in case it is still present) and reset the filename field to the empty string so that the lockfile object is left in a valid state. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 18 +++--- 1

[PATCH v5 16/35] commit_lock_file(): inline temporary variable

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lockfile.c b/lockfile.c index 1c85b18..9e1cdd2 100644 --- a/lockfile.c +++ b/lockfile.c @@ -311,12 +311,14 @@ int reopen_lock_file(struct lock_file *lk)

[PATCH v5 28/35] Change lock_file::filename into a strbuf

For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then

[PATCH v5 15/35] remove_lock_file(): call rollback_lock_file()

It does just what we need. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lockfile.c b/lockfile.c index 911f123..1c85b18 100644 --- a/lockfile.c +++ b/lockfile.c @@ -68,12 +68,8 @@ static void

[PATCH v5 06/35] lockfile: unlock file if lockfile permissions cannot be adjusted

If the call to adjust_shared_perm() fails, lock_file returns -1, which to the caller looks like any other failure to lock the file. So in this case, roll back the lockfile before returning so that the lock file is deleted immediately and the lockfile object is left in a predictable state (namely,

[PATCH v5 11/35] delete_ref_loose(): don't muck around in the lock_file's filename

It's bad manners. Especially since there could be a signal during the call to unlink_or_warn(), in which case the signal handler will see the wrong filename and delete the reference file, leaving the lockfile behind. So make our own copy to work with. Signed-off-by: Michael Haggerty

[PATCH v5 26/35] try_merge_strategy(): use a statically-allocated lock_file object

Even the one lockfile object needn't be allocated each time the function is called. Instead, define one statically-allocated lock_file object and reuse it for every call. Suggested-by: Jeff King p...@peff.net Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/merge.c | 14

[PATCH v5 30/35] resolve_symlink(): take a strbuf parameter

Change resolve_symlink() to take a strbuf rather than a string as parameter. This simplifies the code and removes an arbitrary pathname length restriction. It also means that lock_file's filename field no longer needs to be initialized to a large size. Helped-by: Torsten Bögershausen

[PATCH v5 20/35] api-lockfile: document edge cases

* Document the behavior of commit_lock_file() when it fails, namely that it rolls back the lock_file object and sets errno appropriately. * Document the behavior of rollback_lock_file() when called for a lock_file object that has already been committed or rolled back, namely that it is a

[PATCH v5 27/35] commit_lock_file(): use a strbuf to manage temporary space

Avoid relying on the filename length restrictions that are currently checked by lock_file(). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lockfile.c b/lockfile.c index a09b4a3..59c822d 100644

[PATCH v5 34/35] lockfile.c: rename static functions

* remove_lock_file() - remove_lock_files() * remove_lock_file_on_signal() - remove_lock_files_on_signal() Suggested-by: Torsten Bögershausen tbo...@web.de Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff

[PATCH v5 08/35] lock_file(): always add lock_file object to lock_file_list

It used to be that if locking failed, lock_file() usually did not register the lock_file object in lock_file_list but sometimes it did. This confusion was compounded if lock_file() was called via hold_lock_file_for_append(), which has its own failure modes. The ambiguity didn't have any ill

[PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

Document the valid states of lock_file objects, how they get into each state, and how the state is encoded in the object's fields. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- lockfile.c | 57

[PATCH v5 29/35] resolve_symlink(): use a strbuf for internal scratch space

Aside from shortening and simplifying the code, this removes another place where the path name length is arbitrarily limited. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git

[PATCH v5 32/35] Extract a function commit_lock_file_to()

commit_locked_index(), when writing to an alternate index file, duplicates (poorly) the code in commit_lock_file(). And anyway, it shouldn't have to know so much about the internal workings of lockfile objects. So extract a new function commit_lock_file_to() that does the work common to the two

[PATCH v5 24/35] struct lock_file: declare some fields volatile

The function remove_lock_file_on_signal() is used as a signal handler. It is not realistic to make the signal handler conform strictly to the C standard, which is very restrictive about what a signal handler is allowed to do. But let's increase the likelihood that it will work: The

Re: [PATCH v5 01/35] unable_to_lock_die(): rename function from unable_to_lock_index_die()

Michael Haggerty wrote: This function is used for other things besides the index, so rename it accordingly. Makes sense. [...] builtin/update-index.c | 2 +- cache.h| 2 +- lockfile.c | 6 +++--- refs.c | 2 +- 4 files changed, 6

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

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 @@

Re: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

On Tue, Sep 16, 2014 at 11:41:08AM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: I think you forgot to git add mbox.h. That being said, if we did go this route, I do not see any reason to share the code at all. This can be purely a mailinfo.c thing. OK. A

Re: [PATCH v5 00/23] Signed push

On Tuesday, September 16, 2014, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: A failing test has been added at the end for smart HTTP. It appears that somewhere in the callchain --signed is forgotten and the sending end not to send the certificate for

Re: [PATCH v5 03/35] rollback_lock_file(): do not clear filename redundantly

Michael Haggerty wrote: It is only necessary to clear the lock_file's filename field if it was not already clear. [...] --- a/lockfile.c +++ b/lockfile.c @@ -276,6 +276,6 @@ void rollback_lock_file(struct lock_file *lk) if (lk-fd = 0) close(lk-fd);

Re: [PATCH v5 04/35] rollback_lock_file(): exit early if lock is not active

Michael Haggerty wrote: Eliminate a layer of nesting. Oh --- guess I should have been more patient. :) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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

Re: [PATCH v5 05/35] rollback_lock_file(): set fd to -1

Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -276,7 +276,7 @@ void rollback_lock_file(struct lock_file *lk) return; if (lk-fd = 0) - close(lk-fd); + close_lock_file(lk); Doesn't need to be guarded by the 'if'. -- To

Re: [PATCH v5 05/35] rollback_lock_file(): set fd to -1

Jonathan Nieder wrote: Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -276,7 +276,7 @@ void rollback_lock_file(struct lock_file *lk) return; if (lk-fd = 0) -close(lk-fd); +close_lock_file(lk); Doesn't need to be guarded by the

Re: [PATCH v5 06/35] lockfile: unlock file if lockfile permissions cannot be adjusted

Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to

Re: [PATCH v5 07/35] hold_lock_file_for_append(): release lock on errors

Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -219,13 +219,13 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags) if (errno != ENOENT) { if (flags LOCK_DIE_ON_ERROR)

Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -4,6 +4,63 @@ #include cache.h #include sigchain.h +/* + * File write-locks as used by Git. + * + * When a file at $FILENAME needs to be written, it is done as + * follows: This overlaps a lot with the API doc, which makes

Re: [PATCH v5 10/35] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

Michael Haggerty wrote: There are a few places that use these values, so define constants for them. Seems like a symptom of the API leaving out a useful helper (e.g., something that strips off the lock suffix and returns a memdupz'd filename). [...] --- a/cache.h +++ b/cache.h @@ -570,6

Re: [PATCH v5 11/35] delete_ref_loose(): don't muck around in the lock_file's filename

Michael Haggerty wrote: It's bad manners. Especially since there could be a signal during the call to unlink_or_warn(), in which case the signal handler will see the wrong filename and delete the reference file, leaving the lockfile behind. So make our own copy to work with. Nice. Could

Re: [PATCH v5 12/35] prepare_index(): declare return value to be (const char *)

Michael Haggerty wrote: Declare the return value to be const to make it clear that we aren't giving callers permission to write over the string that it points at. (The return value is the filename field of a struct lock_file, which can be used by a signal handler at any time and therefore

Re: [PATCH] unblock and unignore SIGPIPE

Patrick Reynolds patrick.reyno...@github.com writes: Blocked and ignored signals -- but not caught signals -- are inherited across exec. Some callers with sloppy signal-handling behavior can call git with SIGPIPE blocked or ignored, even non-deterministically. When SIGPIPE is blocked or

Re: [PATCH v5 14/35] lock_file(): exit early if lockfile cannot be opened

Michael Haggerty wrote: lockfile.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More

Re: [PATCH v5 15/35] remove_lock_file(): call rollback_lock_file()

Michael Haggerty wrote: lockfile.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) Nice. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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

Re: [PATCH v5 16/35] commit_lock_file(): inline temporary variable

Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -311,12 +311,14 @@ int reopen_lock_file(struct lock_file *lk) int commit_lock_file(struct lock_file *lk) { char result_file[PATH_MAX]; - size_t i; + if (lk-fd = 0 close_lock_file(lk)) return

Re: [PATCH v5 17/35] commit_lock_file(): die() if called for unlocked lockfile object

Michael Haggerty wrote: --- a/lockfile.c +++ b/lockfile.c @@ -312,6 +312,9 @@ int commit_lock_file(struct lock_file *lk) { char result_file[PATH_MAX]; + if (!lk-filename[0]) + die(BUG: attempt to commit unlocked object); Sure, this is fine instead of an assert()

Re: [PATCH v5 18/35] commit_lock_file(): if close fails, roll back

Michael Haggerty wrote: If closing an open lockfile fails, then we cannot be sure of the contents of the lockfile Is that true? It seems more like a bug in close_lock_file: if it fails, perhaps it should either set lk-fd back to fd or unlink the lockfile itself. What do other callers do on

Re: [PATCH v5 19/35] commit_lock_file(): rollback lock file on failure to rename

Michael Haggerty wrote: If rename() fails, call rollback_lock_file() to delete the lock file (in case it is still present) and reset the filename field to the empty string so that the lockfile object is left in a valid state. Can you spell out more what the goal is? Is the idea to keep the

Re: [PATCH v5 20/35] api-lockfile: document edge cases

Michael Haggerty wrote: * Document the behavior of commit_lock_file() when it fails, namely that it rolls back the lock_file object and sets errno appropriately. * Document the behavior of rollback_lock_file() when called for a lock_file object that has already been committed or

Re: [PATCH v5 21/35] dump_marks(): remove a redundant call to rollback_lock_file()

Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- fast-import.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to

Re: [RFC] allowing hooks to ignore input?

Johannes Sixt j...@kdbg.org writes: I think this is a good move. Hooks are written by users, who sometimes are not clueful enough. Thanks for a sanity check. I do not think it is about cluefulness in this particular case. A rule that is not meaningfully enforced by reliably failing offenders

Re: [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()

Michael Haggerty wrote: After commit_lock_file() is called, then the lock_file object is necessarily either committed or rolled back. So there is no need to call rollback_lock_file() again in either of these cases. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu This seems to involve

Re: [PATCH v5 23/35] lockfile: avoid transitory invalid states

Michael Haggerty wrote: We could probably continue to use the filename field to encode the state by being careful to write characters 1..N-1 of the filename first, and then overwrite the NUL at filename[0] with the first character of the filename, but that would be awkward and error-prone.

Re: [PATCH] compat-util: add _DEFAULT_SOURCE define

Hello, On (09/15/14 12:02), Junio C Hamano wrote: Date: Mon, 15 Sep 2014 12:02:33 -0700 From: Junio C Hamano gits...@pobox.com To: Sergey Senozhatsky sergey.senozhat...@gmail.com Cc: git@vger.kernel.org Subject: Re: [PATCH] compat-util: add _DEFAULT_SOURCE define User-Agent: Gnus/5.13 (Gnus

Re: [PATCH v3] pretty: add %D format specifier

Junio C Hamano gits...@pobox.com writes: +test_expect_success 'clean log decoration' ' +git log --no-walk --tags --pretty=%H %D --decorate=full actual +cat EOF expected +$head1 tag: refs/tags/tag2 +$head2 tag: refs/tags/message-one +$old_head1 tag: refs/tags/message-two +EOF ...

[PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist

Make rev-parse --verify --quiet ref@{1.year.ago} when the reflog does not go back that far succeed silently with --quiet. Signed-off-by: David Aguilar dav...@gmail.com --- sha1_name.c | 19 --- t/t1503-rev-parse-verify.sh | 10 ++ 2 files changed, 22

[PATCH 1/2] fixup! refs: make rev-parse --quiet actually quiet

--- This is a fixup for da/rev-parse-verify-quiet in pu We now exit(128) and handle the Log for XXX only has DDD entries case. refs.c | 2 +- sha1_name.c | 3 +++ t/t1503-rev-parse-verify.sh | 10 +- 3 files changed, 13 insertions(+), 2 deletions(-)