summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* refs.c: remove forward declaration of write_ref_sha1rs/ref-transaction-2Ronnie Sahlberg2014-05-151-2/+0
| | | | | Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make rename_ref use a transactionRonnie Sahlberg2014-05-151-53/+20
| | | | | | | Change rename_ref to use a single transaction to perform the ref rename. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: pass a skip list to name_conflict_fnRonnie Sahlberg2014-05-151-10/+33
| | | | | | | | | | | | | | | | Allow passing a list of refs to skip checking to name_conflict_fn. There are some conditions where we want to allow a temporary conflict and skip checking those refs. For example if we have a transaction that 1, guarantees that m is a packed refs and there is no loose ref for m 2, the transaction will delete m from the packed ref 3, the transaction will create conflicting m/m For this case we want to be able to lock and create m/m since we know that the conflict is only transient. I.e. the conflict will be automatically resolved by the transaction when it deletes m. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add a new flag for transaction delete for refs we know are packed onlyRonnie Sahlberg2014-05-151-0/+23
| | | | | | | | | | | | | | | Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. This flag indicates that the ref does not exist as a loose ref andf only as a packed ref. If this is the case we then change the commit code so that we skip taking out a lock file and we skip calling delete_ref_loose. Check for this flag and die(BUG:...) if used with _update or _create. At the start of the transaction, before we even start locking any refs, we add all such REF_ISPACKONLY refs to delnames so that we have a list of all pack only refs that we will be deleting during this transaction. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: call lock_ref_sha1_basic directly from commitRonnie Sahlberg2014-05-151-6/+6
| | | | | | | | Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic directly from the commit function. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: move the check for valid refname to lock_ref_sha1_basicRonnie Sahlberg2014-05-151-2/+3
| | | | | | | | | | | | | | Move the check for check_refname_format from lock_any_ref_for_update to lock_ref_sha1_basic. At some later stage we will get rid of lock_any_ref_for_update completely. This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed. But this wrapper is also called from an external caller and we will soon make changes to the signature to lock_ref_sha1_basic that we do not want to expose to that caller. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: pack all refs before we start to rename a refRonnie Sahlberg2014-05-152-1/+4
| | | | | | | | | This means that most loose refs will no longer be present after the rename which triggered a test failure since it assumes the file for an unrelated ref would still be present after the rename. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: pass NULL as *flags to read_ref_fullRonnie Sahlberg2014-05-151-1/+1
| | | | | | | | We call read_ref_full with a pointer to flags from rename_ref but since we never actually use the returned flags we can just pass NULL here instead. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: pass the ref log message to _create/delete/update instead of _commitRonnie Sahlberg2014-05-1512-45/+60
| | | | | | | | | | Change the reference transactions so that we pass the reflog message through to the create/delete/update function instead of the commit message. This allows for individual messages for each change in a multi ref transaction. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make delete_ref use a transactionRonnie Sahlberg2014-05-151-21/+10
| | | | | | | | | Change delete_ref to use a ref transaction for the deletion. At the same time since we no longer have any callers of repack_without_ref we can now delete this function. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make prune_ref use a transaction to delete the refRonnie Sahlberg2014-05-151-7/+20
| | | | | | | | | | Change prune_ref to delete the ref using a ref transaction. To do this we also need to add a new flag REF_ISPRUNING that will tell the transaction that we do not want to delete this ref from the packed refs. This flag is private to refs.c and not exposed to external callers. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: remove lock_ref_sha1Ronnie Sahlberg2014-05-151-10/+5
| | | | | | | | | | lock_ref_sha1 was only called from one place in refc.c and only provided a check that the refname was sane before adding back the initial "refs/" part of the ref path name, the initial "refs/" that this caller had already stripped off before calling lock_ref_sha1. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: remove the update_ref_write functionRonnie Sahlberg2014-05-151-26/+9
| | | | | | | | | Since we only call update_ref_write from a single place and we only call it with onerr==QUIET_ON_ERR we can just as well get rid of it and just call write_ref_sha1 directly. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: remove the update_ref_lock functionRonnie Sahlberg2014-05-151-24/+6
| | | | | | | | | Since we now only call update_ref_lock with onerr==QUIET_ON_ERR we no longer need this function and can replace it with just calling lock_any_ref_for_update directly. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add transaction.status and track OPEN/CLOSED/ERRORRonnie Sahlberg2014-05-151-1/+30
| | | | | | | | | Track the status of a transaction in a new status field. Check the field for sanity, i.e. that status must be OPEN when _commit/_create/_delete or _update is called or else die(BUG:...) Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make lock_ref_sha1 staticRonnie Sahlberg2014-05-152-4/+1
| | | | | | | No external callers reference lock_ref_sha1 any more so lets declare it static. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make write_ref_sha1 staticRonnie Sahlberg2014-05-152-4/+3
| | | | | | | No external users call write_ref_sha1 any more so lets declare it static. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* walker.c: use ref transaction for ref updatesRonnie Sahlberg2014-05-151-25/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | Switch to using ref transactions in walker_fetch(). As part of the refactoring to use ref transactions we also fix a potential memory leak where in the original code if write_ref_sha1() would fail we would end up returning from the function without free()ing the msg string. This changes the locking slightly for walker_fetch. Previously the code would lock all refs before writing them but now we do not lock the refs until the commit stage. There is thus a very short window where changes could be done locally during the fetch which would be overwritten when the fetch completes and commits its transaction. But this window should be reasonably short. Even if this race does trigger, since both the old code and the new code just overwrites the refs to the new values without checking or comparing them with the previous value, this is not too dissimilar to a similar scenario where you first do a ref change locally and then later do a fetch that overwrites the local change. With this in mind I do not see the change in locking semantics to be critical. Note that this function is only called when fetching from a remote HTTP repository onto the local (most of the time single-user) repository which likely means that the type of collissions that the previous locking would protect against and cause the fetch to fail for to be even more rare. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fast-import.c: use a ref transaction when dumping tagsRonnie Sahlberg2014-05-151-5/+12
| | | | | Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* receive-pack.c: use a reference transaction for updating the refsRonnie Sahlberg2014-05-151-10/+10
| | | | | | | Wrap all the ref updates inside a transaction to make the update atomic. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fetch.c: use a single ref transaction for all ref updatesRonnie Sahlberg2014-05-151-18/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change store_updated_refs to use a single ref transaction for all refs that are updated during the fetch. This makes the fetch more atomic when update failures occur. Since ref update failures will now no longer occur in the code path for updating a single ref in s_update_ref, we no longer have as detailed error message logging the exact reference and the ref log action as in the old cod Instead since we fail the entire transaction we log a much more generic message. But since we commit the transaction using MSG_ON_ERR we will log an error containing the ref name if either locking of writing the ref would so the regression in the log message is minor. This will also change the order in which errors are checked for and logged which may alter which error will be logged if there are multiple errors occuring during a fetch. For example, assume we have a fetch for two refs that both would fail. Where the first ref would fail with ENOTDIR due to a directory in the ref path not existing, and the second ref in the fetch would fail due to the check in update_logical_ref(): if (current_branch && !strcmp(ref->name, current_branch->name) && !(update_head_ok || is_bare_repository()) && !is_null_sha1(ref->old_sha1)) { /* * If this is the head, and it's not okay to update * the head, and the old value of the head isn't empty... */ In the old code since we would update the refs one ref at a time we would first fail the ENOTDIR and then fail the second update of HEAD as well. But since the first ref failed with ENOTDIR we would eventually fail the who fetch with STORE_REF_ERROR_DF_CONFLICT In the new code, since we defer committing the transaction until all refs have been processed, we would now detect that the second ref was bad and rollback the transaction before we would even try start writing the update t disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case. I think this new behaviour is more correct, since if there was a problem we would not even try to commit the transaction but need to highlight this change in how/what errors are reported. This change in what error is returned only occurs if there are multiple refs that fail to update and only some, but not all, of them fail due to ENOTDIR. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fetch.c: change s_update_ref to use a ref transactionRonnie Sahlberg2014-05-151-8/+9
| | | | | | | Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fetch.c: clear errno before calling functions that might set itRonnie Sahlberg2014-05-151-1/+3
| | | | | | | | | | | | | | | In s_update_ref there are two calls that when they fail we return an error based on the errno value. In particular we want to return a specific error if ENOTDIR happened. Both these functions do have failure modes where they may return an error without updating errno, in which case a previous and unrelated ENOTDIR may cause us to return the wrong error. Clear errno before calling any functions if we check errno afterwards. Also skip initializing a static variable to 0. Statics live in .bss and are all automatically initialized to 0. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: ref_transaction_commit should not free the transactionRonnie Sahlberg2014-05-159-21/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change ref_transaction_commit so that it does not free the transaction. Instead require that a caller will end a transaction by either calling ref_transaction_rollback or ref_transaction_free. By having the transaction object remaining valid after _commit returns allows us to write much nicer code and still be able to call ref_transaction_rollback safely. Instead of this horribleness t = ref_transaction_begin(); if ((!t || ref_transaction_update(t, refname, sha1, oldval, flags, !!oldval)) || (ref_transaction_commit(t, action, &err) && !(t = NULL))) { ref_transaction_rollback(t); we can now just do the much nicer t = ref_transaction_begin(); if (!t || ref_transaction_update(t, refname, sha1, oldval, flags, !!oldval) || ref_transaction_commit(&t, action, &err)) { ref_transaction_rollback(t); ... die/return ... ref_transaction_free(transaction); Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: free the transaction before returning when number of updates is 0Ronnie Sahlberg2014-05-151-1/+3
| | | | | | | | | | | We have to free the transaction before returning in the early check for 'return early if number of updates == 0' or else the following code would create a memory leak with the transaction never being freed : t = ref_transaction_begin() ref_transaction_commit(t) Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: change update_ref to use a transactionRonnie Sahlberg2014-05-151-4/+21
| | | | | | | Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* branch.c: use ref transaction for all ref updatesRonnie Sahlberg2014-05-151-14/+16
| | | | | | | | | | | | | | | | | | | Change create_branch to use a ref transaction when creating the new branch. ref_transaction_create will check that the ref does not already exist and fail otherwise meaning that we no longer need to keep a lock on the ref during the setup_tracking. This simplifies the code since we can now do the transaction in one single step. If the forcing flag is false then use ref_transaction_create since this will fail if the ref already exist. Otherwise use ref_transaction_update. This also fixes a race condition in the old code where two concurrent create_branch could race since the lock_any_ref_for_update/write_ref_sha1 did not protect against the ref already existsing. I.e. one thread could end up overwriting a branch even if the forcing flag is false. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fast-import.c: change update_branch to use ref transactionsRonnie Sahlberg2014-05-151-8/+13
| | | | | | | Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* sequencer.c: use ref transactions for all ref updatesRonnie Sahlberg2014-05-151-8/+16
| | | | | | | Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit.c: use ref transactions for updatesRonnie Sahlberg2014-05-151-13/+10
| | | | | | | | | Change commit.c to use ref transactions for all ref updates. Make sure we pass a NULL pointer to ref_transaction_update if have_old is false. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* replace.c: use the ref transaction functions for updatesRonnie Sahlberg2014-05-151-6/+8
| | | | | | | Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* tag.c: use ref transactions when doing updatesRonnie Sahlberg2014-05-151-6/+8
| | | | | | | Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: ref_transaction_delete to check for error and return statusRonnie Sahlberg2014-05-153-11/+20
| | | | | | | | | | Change ref_transaction_delete() to do basic error checking and return non-zero of error. Update all callers to check the return for ref_transaction_delete(). There are currently no conditions in _delete that will return error but there will be in the future. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: change ref_transaction_create to do error checking and return statusRonnie Sahlberg2014-05-153-11/+21
| | | | | | | | | | Do basic error checking in ref_transaction_create() and make it return non-zero on error. Update all callers to check the result of ref_transaction_create(). There are currently no conditions in _create that will return error but there will be in the future. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: change ref_transaction_update() to do error checking and return statusRonnie Sahlberg2014-05-153-11/+21
| | | | | | | | | | | | | Update ref_transaction_update() do some basic error checking and return non-zero on error. Update all callers to check ref_transaction_update() for error. There are currently no conditions in _update that will return error but there will be in the future. Also check for BUGs during update and die(BUG:...) if we are calling _update with have_old but the old_sha1 pointer is NULL. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: remove the onerr argument to ref_transaction_commitRonnie Sahlberg2014-05-153-19/+9
| | | | | | | | | | Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr argument any more. Remove the onerr argument from the ref_transaction_commit signature. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* update-ref.c: log transaction error from the update_refRonnie Sahlberg2014-05-151-5/+5
| | | | | | | | Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is returned to print a log message if/after the transaction fails. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make update_ref_write update a strbuf on failureRonnie Sahlberg2014-05-151-3/+6
| | | | | | | | | | Change update_ref_write to also update an error strbuf on failure. This makes the error available to ref_transaction_commit callers if the transaction failed due to update_ref_sha1/write_ref_sha1 failures. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add an err argument ro delete_loose_refRonnie Sahlberg2014-05-151-6/+12
| | | | | | | | | | Add an err argument to delete_loose_ref so that we can pass a descriptive error string back to the caller. Pass the err argument from transaction commit to this function so that transaction users will have a nice error string if the transaction failed due to delete_loose_ref failing. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: make ref_update_reject_duplicates take a strbuf argument for errorsRonnie Sahlberg2014-05-151-1/+5
| | | | | | | | | | | Make ref_update_reject_duplicates return any error that occurs through a new strbuf argument. This means that when a transaction commit fails in this function we will now be able to pass a helpful error message back to the caller. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add an err argument to repack_without_refsRonnie Sahlberg2014-05-151-5/+11
| | | | | | | | | | Update repack_without_refs to take an err argument and update it if there is a failure. Pass the err variable from ref_transaction_commit to this function so that callers can print a meaningful error message if _commit fails due to a problem in repack_without_refs. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: add a strbuf argument to ref_transaction_commit for error loggingRonnie Sahlberg2014-05-153-3/+10
| | | | | | | | | | | | | | Add a strbuf argument to _commit so that we can pass an error string back to the caller. So that we can do error logging from the caller instead of from _commit. Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR and craft any log messages from the callers themselves and finally remove the onerr argument completely. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: allow passing NULL to ref_transaction_freeRonnie Sahlberg2014-05-151-0/+3
| | | | | | | | | | | | | | | | | | | | | | | Allow ref_transaction_free to be called with NULL and as a result allow ref_transaction_rollback to be called for a NULL transaction. This allows us to write code that will if ( (!transaction || ref_transaction_update(...)) || (ref_transaction_commit(...) && !(transaction = NULL)) { ref_transaction_rollback(transaction); ... } In this case transaction is reset to NULL IFF ref_transaction_commit() was invoked and thus the rollback becomes ref_transaction_rollback(NULL) which is safe. IF the conditional triggered prior to ref_transaction_commit() then transaction is untouched and then ref_transaction_rollback(transaction) will rollback the failed transaction. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.c: constify the sha arguments for ref_transaction_create|delete|updateRonnie Sahlberg2014-05-152-6/+8
| | | | | | | | | | | | | ref_transaction_create|delete|update has no need to modify the sha1 arguments passed to it so it should use const unsigned char* instead of unsigned char*. Some functions, such as fast_forward_to(), already have its old/new sha1 arguments as consts. This function will at some point need to use ref_transaction_update() in which case this change is required. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'rs/reflog-exists' into HEADJunio C Hamano2014-05-155-13/+32
|\ | | | | | | | | | | * rs/reflog-exists: checkout.c: use ref_exists instead of file_exist refs.c: add new functions reflog_exists and delete_reflog
| * checkout.c: use ref_exists instead of file_existrs/reflog-existsRonnie Sahlberg2014-05-082-4/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change checkout.c to check if a ref exists instead of checking if a loose ref file exists when deciding if to delete an orphaned log file. Otherwise, if a ref only exists as a packed ref without a corresponding loose ref for the currently checked out branch, we risk that the reflog will be deleted when we switch to a different branch. Update the reflog tests to check for this bug. The following reproduces the bug: $ git init-db $ git config core.logallrefupdates true $ git commit -m Initial --allow-empty [master (root-commit) bb11abe] Initial $ git reflog master [8561dcb master@{0}: commit (initial): Initial] $ find .git/{refs,logs} -type f | grep master [.git/refs/heads/master] [.git/logs/refs/heads/master] $ git branch foo $ git pack-refs --all $ find .git/{refs,logs} -type f | grep master [.git/logs/refs/heads/master] $ git checkout foo $ find .git/{refs,logs} -type f | grep master ... reflog file is missing ... $ git reflog master ... nothing ... Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs.c: add new functions reflog_exists and delete_reflogRonnie Sahlberg2014-05-084-11/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add two new functions, reflog_exists and delete_reflog, to hide the internal reflog implementation (that they are files under .git/logs/...) from callers. Update checkout.c to use these functions in update_refs_for_switch instead of building pathnames and calling out to file access functions. Update reflog.c to use these to check if the reflog exists. Now there are still many places in reflog.c where we are still leaking the reflog storage implementation but this at least reduces the number of such dependencies by one. Finally change two places in refs.c itself to use the new function to check if a ref exists or not isntead of build-path-and-stat(). Now, this is strictly not all that important since these are in parts of refs that are implementing the actual file storage backend but on the other hand it will not hurt either. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'rs/ref-update-check-errors-early' into HEADJunio C Hamano2014-05-152-4/+8
|\ \ | | | | | | | | | | | | | | | * rs/ref-update-check-errors-early: commit.c: check for lock error and return early sequencer.c: check for lock failure and bail early in fast_forward_to
| * | commit.c: check for lock error and return earlyrs/ref-update-check-errors-earlyRonnie Sahlberg2014-04-171-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the check for the lock failure to happen immediately after lock_any_ref_for_update(). Previously the lock and the check-if-lock-failed was separated by a handful of string manipulation statements. Moving the check to occur immediately after the failed lock makes the code slightly easier to read and makes it follow the pattern of try-to-take-a-lock(); if (check-if-lock-failed) { error(); } Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | sequencer.c: check for lock failure and bail early in fast_forward_toRonnie Sahlberg2014-04-171-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change fast_forward_to() to check if locking the ref failed, print a nice error message and bail out early. The old code did not check if ref_lock was NULL and relied on the fact that the write_ref_sha1() would safely detect this condition and set the return variable ret to indicate an error. While that is safe, it makes the code harder to read for two reasons: * Inconsistency. Almost all other places we do check the lock for NULL explicitly, so the naive reader is confused "why don't we check here?" * And relying on write_ref_sha1() to detect and return an error for when a previous lock_any_ref_for_update() failed feels obfuscated. This change should not change any functionality or logic aside from adding an extra error message when this condition is triggered (write_ref_sha1() returns an error silently for this condition). Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>