On Mon, Jan 26, 2015 at 7:22 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> I can redo the atomic-push-fix series with this cleanup merged
>> into the appropriate patches or you could just queue it on top
>> of said series.
>
> Yeah, I do not think we are expecting to fast track these two series
> through 'next' to 'master' before 2.3 final, so I think it would be
> better to use this patch _only_ to see if the final shape of the
> code this patch represents makes sense, so that we can expedite the
> final submission in the next development cycle, at which time we
> will have a chance to refresh 'next', hence a chance to clean-up
> atomic-push series in place.
I tried to rip this patch and its 3 previous patches apart to see if it could be
done another way. The outcome was to actually sqquash this patch
completely into b1c6da0a13 (refs.c: remove unlock_ref and commit_ref
from write_ref_sha1).
Looking at the end result the write_ref_sha1 function it has a good
design contract. Either you get 0 returned and all is good, or -1 is returned
and errno is set to a meaningful value which seems to adress your concerns
on that patch:
> I am not sure if it is sensible to call that "correct but hard to
> understand". I'd rather see us admit that its behaviour is screwey
> and needs fixing for better code health longer term.
>> @@ -2880,7 +2877,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
>> error("unable to lock %s for update", newrefname);
>> goto rollback;
>> }
>> - lock->force_write = 1;
>> hashcpy(lock->old_sha1, orig_sha1);
>
> Is this hashcpy() still necessary?
Thanks for catching that! It is not necessary any more and will be
removed in a reroll.
I think I'll wait for rerolling the atomic-push-fix series until 2.3
is out then?
--
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