On Mon, May 22, 2017 at 7:17 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> This is the second iteration of a patch series that prepares the
> ground for separating out a `packed_ref_store` and then for changing
> `packed-refs` to be read using `mmap()`. Thanks to Peff, Junio,
> Stefan, Brandon, and Johannes for their feedback about v1 [1]. I think
> I have addressed all of your comments.
>
> Changes since v1:
>
> * Since v1, branch `bc/object-id` has been merged to `next`, and it
> has lots of conflicts with these changes. So I rebased this branch
> onto a merge of `master` and `bc/object-id`. (I hope this makes
> Junio's job easier.) This unfortunately causes a bit of tbdiff noise
> between v1 and v2.
>
> * Patch [01/25]: in t3600, register the `test_when_finished` command
> before executing `chmod a-w`.
>
> * Patch [04/25] (new patch): convert a few `die("internal error: ...")`
> to `die("BUG: ...")`.
>
> * Patch [05/25]: Use `strlen()` rather than `memchr()` to check the
> trim length, and `die()` rather than skipping if it is longer than
> the reference name.
>
> * Patch [08/25]: Name the log message arguments `msg` for consistency
> with existing code.
>
> * Patch [10/25]: Rename the new member from `packlock` to
> `packed_refs_lock`.
>
> * Patch [13/25] (new patch): Move the check for valid
> `transaction->state` from `files_transaction_commit()` to
> `ref_transaction_commit()`.
>
> * Patch [14/25]:
>
> * Add more sanity checks of `transaction->state`.
>
> * Don't add `ref_transaction_finish()` to the public API. Instead,
> teach `ref_transaction_commit()` to do the right thing whether or
> not `ref_transaction_prepare()` has been called.
>
> * Add and improve docstrings.
>
> * Allow `ref_transaction_abort()` to be called even before
> `ref_transaction_prepare()` (in which case it just calls
> `ref_transaction_free()`).
>
> * Lots of improvements to commit messages and comments, mostly to
> clarify points that reviewers asked about.
>
> These changes (along with the merge commit that they are based on) are
> also available as branch `packed-ref-store-prep` in my GitHub fork
> [2]. If you'd like to see a preview of the rest of the changes (which
> works but is not yet polished), checkout the `mmap-packed-refs` branch
> from the same place.
I have read this version and quite like it. I did not have any nits to remark.
Thanks,
Stefan