On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg <sahlberg@xxxxxxxxxx> wrote:
> 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 code.
> 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, assuming we have a fetch for two refs that both would fail.
s/assuming/assume/ perhaps?
> 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 sicne we would update the refs one ref at a time we would
s/sicne/since/
> 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 whole
> fetch with STORE_REF_ERROR_DF_CONFLICT
>
> In the new code, since we defer committing the transaction until all refs
> has been processed, we would now detect that the second ref was bad and
s/has/have/
> rollback the transaction before we would even try start writing the update to
> 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@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html