I will look at this once i finish the next respin. On Tue, Apr 22, 2014 at 12:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Ronnie Sahlberg <sahlberg@xxxxxxxxxx> writes: > >> This patch series changes most of the places where the ref functions for >> locking and writing refs to instead use the new ref transaction API. There >> are still three more places where write_ref_sha1() is called from outside >> of refs.c but those all will require more complex work and review so those >> changes belong in a different patch series. >> >> Version 2: >> - Add a patch to ref_transaction_commit to make it honor onerr even if the >> error triggered in ref_Transaction_commit itself rather than in a call >> to other functions (that already honor onerr). >> - Add a patch to make the update_ref() helper function use transactions >> internally. >> - Change ref_transaction_update to die() instead of error() if we pass >> if a NULL old_sha1 but have have_old == true. >> - Change ref_transaction_create to die() instead of error() if new_sha1 >> is false but we pass it a null_sha1. >> - Change ref_transaction_delete die() instead of error() if we pass >> if a NULL old_sha1 but have have_old == true. >> - Change several places to do if(!transaction || ref_transaction_update() >> || ref_Transaction_commit()) die(generic-message) instead of checking each >> step separately and having a different message for each failure. >> Most users are likely not interested in what step of the transaction >> failed and only whether it failed or not. >> - Change commit.c to only pass a pointer to ref_transaction_update >> iff current_head is non-NULL. >> The previous patch used to compute a garbage pointer for >> current_head->object.sha1 and relied on the fact that ref_transaction_update >> would not try to dereference this pointer if !!current_head was 0. >> - Updated commit message for the walker_fetch change to try to justify why >> the change in locking semantics should not be harmful. > > Will queue, but when applied on top of mh/ref-transaction and tested > standalone, it appears to fail test #14 of t5550-http-fetch-dumb.sh > for me. > > It seems that the culprit is that this step: > > git http-fetch -a -w heads/master-new $HEAD $(git config remote.origin.url) && > > creates ".git/heads/master-new" when what it was asked to create was > ".git/refs/heads/master-new". Perhaps there is something fishy in > the conversion in walker.c? We used to do lock_ref_sha1() on > "heads/master-new", which prepended "refs/" prefix before calling > lock_ref_sha1_basic() that expects the full path from $GIT_DIR/, but > ref_transaction_update(), which wants to see the full path, is still > fed "heads/master-new" after this series. > -- 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