summaryrefslogtreecommitdiff
path: root/refs.c
Commit message (Collapse)AuthorAgeFilesLines
* pseudoref: check return values from read_ref()dt/refs-pseudoDavid Turner2015-08-111-2/+5
| | | | | | | | | These codepaths attempt to compare the "expected" current value with the actual current value, but did not check if we successfully read the current value before comparison. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pseudorefs: create and use pseudoref update and delete functionsDavid Turner2015-07-311-8/+93
| | | | | | | | | | | Pseudorefs should not be updated through the ref transaction API, because alternate ref backends still need to store pseudorefs in GIT_DIR (instead of wherever they store refs). Instead, change update_ref and delete_ref to call pseudoref-specific functions. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: add ref_type functionDavid Turner2015-07-311-0/+26
| | | | | | | | | | | | Add a function ref_type, which categorizes refs as per-worktree, pseudoref, or normal ref. Later, we will use this in refs.c to treat pseudorefs specially. Alternate ref backends may use it to treat both pseudorefs and per-worktree refs differently. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: add REF_FORCE_CREATE_REFLOG flagDavid Turner2015-07-211-13/+21
| | | | | | | | | | | | Add a flag to allow forcing the creation of a reflog even if the ref name and core.logAllRefUpdates setting would not ordinarily cause ref creation. In a moment, we will use this to add options to git tag and git update-ref to force reflog creation. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: new public ref function: safe_create_reflogDavid Turner2015-07-211-4/+20
| | | | | | | | | | | | | | | | | | The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: break out check for reflog autocreationDavid Turner2015-07-211-5/+11
| | | | | | | This is just for clarity. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add err arguments to reflog functionsDavid Turner2015-07-211-55/+74
| | | | | | | | | | | | | | | | | | | | Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Some error messages are slightly reordered. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'mh/reporting-broken-refs-from-for-each-ref'Junio C Hamano2015-06-241-7/+22
|\ | | | | | | | | | | | | | | | | | | | | | | "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 brokenmh/reporting-broken-refs-from-for-each-refMichael Haggerty2015-06-081-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | 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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * read_loose_refs(): simplify function logicMichael Haggerty2015-06-031-7/+12
| | | | | | | | | | | | | | | | | | | | | | 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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'mh/verify-lock-error-report'Junio C Hamano2015-06-111-14/+26
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bring consistency to error reporting mechanism used in "refs" API. * mh/verify-lock-error-report: ref_transaction_commit(): do not capitalize error messages verify_lock(): do not capitalize error messages verify_lock(): report errors via a strbuf verify_lock(): on errors, let the caller unlock the lock verify_lock(): return 0/-1 rather than struct ref_lock *
| * | ref_transaction_commit(): do not capitalize error messagesmh/verify-lock-error-reportMichael Haggerty2015-05-271-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | Our convention is for error messages to start with a lower-case letter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | verify_lock(): do not capitalize error messagesMichael Haggerty2015-05-271-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | Our convention is for error messages to start with a lower-case letter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | verify_lock(): report errors via a strbufMichael Haggerty2015-05-271-7/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of writing error messages directly to stderr, write them to a "strbuf *err". The caller, lock_ref_sha1_basic(), uses this error reporting convention with all the other callees, and reports its error this way to its callers. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | verify_lock(): on errors, let the caller unlock the lockMichael Haggerty2015-05-271-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | The caller already knows how to do it, so always do it in the same place. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | verify_lock(): return 0/-1 rather than struct ref_lock *Michael Haggerty2015-05-271-7/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Its return value wasn't conveying any extra information, but it made the reader wonder whether the ref_lock that it returned might be different than the one that was passed to it. So change the function to the traditional "return 0 on success or a negative value on error". Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | struct ref_lock: convert old_sha1 member to object_idMichael Haggerty2015-05-251-12/+12
| | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | warn_if_dangling_symref(): convert local variable "junk" to object_idMichael Haggerty2015-05-251-3/+3
| | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | each_ref_fn_adapter(): remove adapterMichael Haggerty2015-05-251-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All of the callers of the for_each_ref family of functions have now been rewritten to work with object_ids, so this adapter is no longer needed. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | each_ref_fn: change to take an object_id parameterMichael Haggerty2015-05-251-16/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change typedef each_ref_fn to take a "const struct object_id *oid" parameter instead of "const unsigned char *sha1". To aid this transition, implement an adapter that can be used to wrap old-style functions matching the old typedef, which is now called "each_ref_sha1_fn"), and make such functions callable via the new interface. This requires the old function and its cb_data to be wrapped in a "struct each_ref_fn_sha1_adapter", and that object to be used as the cb_data for an adapter function, each_ref_fn_adapter(). This is an enormous diff, but most of it consists of simple, mechanical changes to the sites that call any of the "for_each_ref" family of functions. Subsequent to this change, the call sites can be rewritten one by one to use the new interface. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | refs: convert struct ref_entry to use struct object_idbrian m. carlson2015-05-251-22/+22
|/ / | | | | | | | | | | Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'mh/lockfile-retry'Junio C Hamano2015-05-221-1/+11
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | Instead of dying immediately upon failing to obtain a lock, retry after a short while with backoff. * mh/lockfile-retry: lock_packed_refs(): allow retries when acquiring the packed-refs lock lockfile: allow file locking to be retried with a timeout
| * | lock_packed_refs(): allow retries when acquiring the packed-refs lockmh/lockfile-retryMichael Haggerty2015-05-141-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, there is only one attempt to acquire any lockfile, and if the lock is held by another process, the locking attempt fails immediately. This is not such a limitation for loose reference files. First, they don't take long to rewrite. Second, most reference updates have a known "old" value, so if another process is updating a reference at the same moment that we are trying to lock it, then probably the expected "old" value will not longer be valid, and the update will fail anyway. But these arguments do not hold for packed-refs: * The packed-refs file can be large and take significant time to rewrite. * Many references are stored in a single packed-refs file, so it could be that the other process was changing a different reference than the one that we are interested in. Therefore, it is much more likely for there to be spurious lock conflicts in connection to the packed-refs file, resulting in unnecessary command failures. So, if the first attempt to lock the packed-refs file fails, continue retrying for a configurable length of time before giving up. The default timeout is 1 second. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'mh/ref-directory-file'Junio C Hamano2015-05-221-111/+198
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ref API did not handle cases where 'refs/heads/xyzzy/frotz' is removed at the same time as 'refs/heads/xyzzy' is added (or vice versa) very well. * mh/ref-directory-file: reflog_expire(): integrate lock_ref_sha1_basic() errors into ours ref_transaction_commit(): delete extra "the" from error message ref_transaction_commit(): provide better error messages rename_ref(): integrate lock_ref_sha1_basic() errors into ours lock_ref_sha1_basic(): improve diagnostics for ref D/F conflicts lock_ref_sha1_basic(): report errors via a "struct strbuf *err" verify_refname_available(): report errors via a "struct strbuf *err" verify_refname_available(): rename function refs: check for D/F conflicts among refs created in a transaction ref_transaction_commit(): use a string_list for detecting duplicates is_refname_available(): use dirname in first loop struct nonmatching_ref_data: store a refname instead of a ref_entry report_refname_conflict(): inline function entry_matches(): inline function is_refname_available(): convert local variable "dirname" to strbuf is_refname_available(): avoid shadowing "dir" variable is_refname_available(): revamp the comments t1404: new tests of ref D/F conflicts within transactions
| * | | reflog_expire(): integrate lock_ref_sha1_basic() errors into oursmh/ref-directory-file-3mh/ref-directory-fileMichael Haggerty2015-05-111-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that lock_ref_sha1_basic() gives us back its error messages via a strbuf, incorporate its error message into our error message rather than emitting two separate error messages. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | ref_transaction_commit(): delete extra "the" from error messageMichael Haggerty2015-05-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While we are in the area, let's remove a superfluous definite article from the error message that is emitted when the reference cannot be locked. This improves how it reads and makes it a bit shorter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | ref_transaction_commit(): provide better error messagesMichael Haggerty2015-05-111-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that lock_ref_sha1_basic() gives us back its error messages via a strbuf, incorporate its error message into our error message rather than emitting one error messages to stderr immediately and returning a second to our caller. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | rename_ref(): integrate lock_ref_sha1_basic() errors into oursMichael Haggerty2015-05-111-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that lock_ref_sha1_basic() gives us back its error messages via a strbuf, incorporate its error message into our error message rather than emitting two separate error messages. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | lock_ref_sha1_basic(): improve diagnostics for ref D/F conflictsMichael Haggerty2015-05-111-4/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If there is a failure to lock a reference that is likely caused by a D/F conflict (e.g., trying to lock "refs/foo/bar" when reference "refs/foo" already exists), invoke verify_refname_available() to try to generate a more helpful error message. That function might not detect an error. For example, some non-reference file might be blocking the deletion of an otherwise-empty directory tree, or there might be a race with another process that just deleted the offending reference. In such cases, generate the strerror-based error message like before. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | lock_ref_sha1_basic(): report errors via a "struct strbuf *err"Michael Haggerty2015-05-111-16/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For now, change the callers to spew the error to stderr like before. But soon we will change them to incorporate the reason for the failure into their own error messages. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | verify_refname_available(): report errors via a "struct strbuf *err"Michael Haggerty2015-05-111-20/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It shouldn't be spewing errors directly to stderr. For now, change its callers to spew the errors to stderr. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | verify_refname_available(): rename functionMichael Haggerty2015-05-111-17/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename is_refname_available() to verify_refname_available() and change its return value from 1 for success to 0 for success, to be consistent with our error-handling convention. In a moment it will also get a "struct strbuf *err" parameter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | refs: check for D/F conflicts among refs created in a transactionMichael Haggerty2015-05-111-62/+94
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If two references that D/F conflict (e.g., "refs/foo" and "refs/foo/bar") are created in a single transaction, the old code discovered the problem only after the "commit" phase of ref_transaction_commit() had already begun. This could leave some references updated and others not, which violates the promise of atomicity. Instead, check for such conflicts during the "locking" phase: * Teach is_refname_available() to take an "extras" parameter that can contain extra reference names with which the specified refname must not conflict. * Change lock_ref_sha1_basic() to take an "extras" parameter, which it passes through to is_refname_available(). * Change ref_transaction_commit() to pass "affected_refnames" to lock_ref_sha1_basic() as its "extras" argument. This change fixes a test case in t1404. This code is a bit stricter than it needs to be. We could conceivably allow reference "refs/foo/bar" to be created in the same transaction as "refs/foo" is deleted (or vice versa). But that would be complicated to implement, because it is not possible to lock "refs/foo/bar" while "refs/foo" exists as a loose reference, but on the other hand we don't want to delete some references before adding others (because that could leave a gap during which required objects are unreachable). There is also a complication that reflog files' paths can conflict. Any less-strict implementation would probably require tricks like the packing of all references before the start of the real transaction, or the use of temporary intermediate reference names. So for now let's accept too-strict checks. Some reference update transactions will be rejected unnecessarily, but they will be rejected in their entirety rather than leaving the repository in an intermediate state, as would happen now. Please note that there is still one kind of D/F conflict that is *not* handled correctly. If two processes are running at the same time, and one tries to create "refs/foo" at the same time that the other tries to create "refs/foo/bar", then they can race with each other. Both processes can obtain their respective locks ("refs/foo.lock" and "refs/foo/bar.lock"), proceed to the "commit" phase of ref_transaction_commit(), and then the slower process will discover that it cannot rename its lockfile into place (after possibly having committed changes to other references). There appears to be no way to fix this race without changing the locking policy, which in turn would require a change to *all* Git clients. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | ref_transaction_commit(): use a string_list for detecting duplicatesMichael Haggerty2015-05-111-14/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Detect duplicates by storing the reference names in a string_list and sorting that, instead of sorting the ref_updates directly. * In a moment the string_list will be used for another purpose, too. * This removes the need for the custom comparison function ref_update_compare(). * This means that we can carry out the updates in the order that the user specified them instead of reordering them. This might be handy someday if, we want to permit multiple updates to a single reference as long as they are compatible with each other. Note: we can't use string_list_remove_duplicates() to check for duplicates, because we need to know the name of the reference that appeared multiple times, to be used in the error message. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | is_refname_available(): use dirname in first loopMichael Haggerty2015-05-111-14/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the first loop (over prefixes of refname), use dirname to keep track of the current prefix. This is not an improvement in itself, but in a moment we will start using dirname for a role where a NUL-terminated string is needed. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | struct nonmatching_ref_data: store a refname instead of a ref_entryMichael Haggerty2015-05-111-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that we don't need a ref_entry to pass to report_refname_conflict(), it is sufficient to store the refname of the conflicting reference. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | report_refname_conflict(): inline functionMichael Haggerty2015-05-111-8/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It wasn't pulling its weight. And we are about to need code similar to this where no ref_entry is available and with more diverse error messages. Rather than try to generalize the function, just inline it. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | entry_matches(): inline functionMichael Haggerty2015-05-111-7/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It wasn't pulling its weight. And in a moment we will need similar tests that take a refname rather than a ref_entry as parameter, which would have made entry_matches() even less useful. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | is_refname_available(): convert local variable "dirname" to strbufMichael Haggerty2015-05-111-7/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This change wouldn't be worth it by itself, but in a moment we will use the strbuf for more string juggling. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | is_refname_available(): avoid shadowing "dir" variableMichael Haggerty2015-05-111-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function had a "dir" parameter that was shadowed by a local "dir" variable within a code block. Use the former in place of the latter. (This is consistent with "dir"'s use elsewhere in the function.) Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
| * | | is_refname_available(): revamp the commentsMichael Haggerty2015-05-111-22/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the comments to a running example of running the function with refname set to "refs/foo/bar". Add some more explanation of the logic. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* | | | Merge branch 'mh/write-refs-sooner-2.4'Junio C Hamano2015-05-221-31/+82
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | ref_transaction_commit(): fix atomicity and avoid fd exhaustionMichael Haggerty2015-05-121-21/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ref_transaction_commit(): remove the local flags variableMichael Haggerty2015-05-121-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ref_transaction_commit(): inline call to write_ref_sha1()Michael Haggerty2015-05-121-18/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | That was the last caller, so delete function write_ref_sha1(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | rename_ref(): inline calls to write_ref_sha1() from this functionMichael Haggerty2015-05-121-4/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most of what it does is unneeded from these call sites. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | commit_ref_update(): new function, extracted from write_ref_sha1()Michael Haggerty2015-05-121-7/+20
| | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | write_ref_to_lockfile(): new function, extracted from write_ref_sha1()Michael Haggerty2015-05-121-4/+17
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sb/ref-lock-lose-lock-fd'Junio C Hamano2015-05-191-10/+5
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | The refs API uses ref_lock struct which had its own "int fd", even though the same file descriptor was in the lock struct it contains. Clean-up the code to lose this redundant field. * sb/ref-lock-lose-lock-fd: refs.c: remove lock_fd from struct ref_lock
| * | | refs.c: remove lock_fd from struct ref_locksb/ref-lock-lose-lock-fdStefan Beller2015-05-101-10/+5
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 'lock_fd' is the same as 'lk->fd'. No need to store it twice so remove it. No functional changes intended. Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
View Lorry Controller Status