On Thu, Jan 22, 2015 at 8:33 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote: > On 01/22/2015 02:10 PM, Jeff King wrote: >> On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote: >> >>> I can't figure out where to apply this series or where to fetch it from, >>> so I can't see these changes in context, so maybe I'm misunderstanding >>> something. It looks like this code is doing >>> >>> open(), close(), open(), fdopen(), write(), fclose(), rename() >>> >>> on each lockfile. But don't we have enough information to write the >>> SHA-1 into the lockfile the first time we touch it? I.e., couldn't we >>> reduce this to >>> >>> open(), fdopen(), write(), fclose(), rename() >>> >>> , where the first four calls all happen in the initial loop? If a >>> problem is discovered when writing a later reference, we would roll back >>> the transaction anyway. >>> >>> I understand that this would require a bigger rewrite, so maybe it is >>> not worth it. >> >> I had a nagging feeling on the multiple-open thing, too, and would much >> prefer to just write out the contents early (since we know what they >> are). It looks like we would just need to split write_ref_sha1() into >> its two halves: >> >> 1. Write out the lockfile >> >> 2. Commit the change >> >> And then call them at the appropriate spots from ref_transaction_commit(). >> >> I guess that is maybe a step backwards for abstracted ref backends, >> though. > > Nah, the implementation of ref_transaction_commit() will have to differ > between backends anyway. I don't think this would be a step backwards. > > Michael > I also dislike the double open/close thing, but I just wanted to come up with a quick and unobtrusive fix which doesn't rewrite the whole refs backend as we have some code churn in the refs lately. Michael, I forgot your short term intentions on the refs backend, so I tried to be shy with that bug fix. What huge changes are you planning in the next few weeks w.r.t. the refs handling? I would look more into that if there are no code conflicts likely to arise. Thanks, Stefan -- 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