git/refs.c, branch v2.4.9

Merge branch 'mh/reporting-broken-refs-from-for-each-ref' into maint

2015-08-03T17:41:31Z

"git for-each-ref" reported "missing object" for 0{40} when it encounters a broken ref. The lack of object whose name is 0{40} is not the problem; the ref being broken is. * mh/reporting-broken-refs-from-for-each-ref: read_loose_refs(): treat NULL_SHA1 loose references as broken read_loose_refs(): simplify function logic for-each-ref: report broken references correctly t6301: new tests of for-each-ref error handling

read_loose_refs(): treat NULL_SHA1 loose references as broken

2015-06-08T17:35:41Z

NULL_SHA1 is used to indicate an "invalid object name" throughout our code (and the code of other git implementations), so it is vastly more likely that an on-disk reference was set to this value due to a software bug than that NULL_SHA1 is the legitimate SHA-1 of an actual object. Therefore, if a loose reference has the value NULL_SHA1, consider it to be broken. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

Merge branch 'mh/write-refs-sooner-2.4' into maint

2015-06-05T19:00:17Z

Multi-ref transaction support we merged a few releases ago unnecessarily kept many file descriptors open, risking to fail with resource exhaustion. This is for 2.4.x track. * mh/write-refs-sooner-2.4: ref_transaction_commit(): fix atomicity and avoid fd exhaustion ref_transaction_commit(): remove the local flags variable ref_transaction_commit(): inline call to write_ref_sha1() rename_ref(): inline calls to write_ref_sha1() from this function commit_ref_update(): new function, extracted from write_ref_sha1() write_ref_to_lockfile(): new function, extracted from write_ref_sha1() t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE update-ref: test handling large transactions properly ref_transaction_commit(): fix atomicity and avoid fd exhaustion ref_transaction_commit(): remove the local flags variable ref_transaction_commit(): inline call to write_ref_sha1() rename_ref(): inline calls to write_ref_sha1() from this function commit_ref_update(): new function, extracted from write_ref_sha1() write_ref_to_lockfile(): new function, extracted from write_ref_sha1() t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE update-ref: test handling large transactions properly

read_loose_refs(): simplify function logic

2015-06-03T18:44:25Z

Make it clearer that there are two possible ways to read the reference, but that we handle read errors uniformly regardless of which way it was read. This refactoring also makes the following change easier to implement. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

ref_transaction_commit(): fix atomicity and avoid fd exhaustion

2015-05-13T04:28:03Z

The old code was roughly for update in updates: acquire locks and check old_sha for update in updates: if changing value: write_ref_to_lockfile() commit_ref_update() for update in updates: if deleting value: unlink() rewrite packed-refs file for update in updates: if reference still locked: unlock_ref() This has two problems. Non-atomic updates ================== The atomicity of the reference transaction depends on all pre-checks being done in the first loop, before any changes have started being committed in the second loop. The problem is that write_ref_to_lockfile() (previously part of write_ref_sha1()), which is called from the second loop, contains two more checks: * It verifies that new_sha1 is a valid object * If the reference being updated is a branch, it verifies that new_sha1 points at a commit object (as opposed to a tag, tree, or blob). If either of these checks fails, the "transaction" is aborted during the second loop. But this might happen after some reference updates have already been permanently committed. In other words, the all-or-nothing promise of "git update-ref --stdin" could be violated. So these checks have to be moved to the first loop. File descriptor exhaustion ========================== The old code locked all of the references in the first loop, leaving all of the lockfiles open until later loops. Since we might be updating a lot of references, this could result in file descriptor exhaustion. The solution ============ After this patch, the code looks like for update in updates: acquire locks and check old_sha if changing value: write_ref_to_lockfile() else: close_ref() for update in updates: if changing value: commit_ref_update() for update in updates: if deleting value: unlink() rewrite packed-refs file for update in updates: if reference still locked: unlock_ref() This fixes both problems: 1. The pre-checks in write_ref_to_lockfile() are now done in the first loop, before any changes have been committed. If any of the checks fails, the whole transaction can now be rolled back correctly. 2. All lockfiles are closed in the first loop immediately after they are created (either by write_ref_to_lockfile() or by close_ref()). This means that there is never more than one open lockfile at a time, preventing file descriptor exhaustion. To simplify the bookkeeping across loops, add a new REF_NEEDS_COMMIT bit to update->flags, which keeps track of whether the corresponding lockfile needs to be committed, as opposed to just unlocked. (Since "struct ref_update" is internal to the refs module, this change is not visible to external callers.) This change fixes two tests in t1400. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

ref_transaction_commit(): remove the local flags variable

2015-05-13T04:28:03Z

Instead, work directly with update->flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the second loop instead of having to be recomputed. Plus, it was potentially confusing having both update->flags and flags, which sometimes had different values. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

ref_transaction_commit(): inline call to write_ref_sha1()

2015-05-13T04:28:03Z

That was the last caller, so delete function write_ref_sha1(). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

rename_ref(): inline calls to write_ref_sha1() from this function

2015-05-13T04:28:02Z

Most of what it does is unneeded from these call sites. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

commit_ref_update(): new function, extracted from write_ref_sha1()

2015-05-13T04:28:02Z

Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano

write_ref_to_lockfile(): new function, extracted from write_ref_sha1()

2015-05-13T04:28:02Z

This is the first step towards separating the checking and writing of the new reference value to committing the change. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano