Re: [PATCH 0/2] Avoid rewriting "packed-refs" unnecessarily
On 10/25/2017 10:10 PM, Eric Sunshine wrote: > On Wed, Oct 25, 2017 at 5:53 AM, Michael Haggerty > wrote: >> Since commit dc39e09942 (files_ref_store: use a transaction to update >> packed refs, 2017-09-08), we've been rewriting the `packed-refs` file >> unnecessarily when deleting a loose reference that has no packed >> counterpart. Add some tests demonstrating this problem, then fix it by >> teaching `files_backend` to see whether any references being deleted >> have packed versions, and if not, skipping the packed_refs >> transaction. >> >> This is a mild regression since 2.14, which avoided rewriting the >> `packed-refs` file in these cases [...] > > Should the above information (that this fixes a regression) be > mentioned in the commit message of at least one of the patches > (probably 2/2)? Without such context, it may not be clear to someone > later reading those commit message -- someone who wasn't following > this email discussion -- that this behavior had been lost and is now > being restored. Yes, that's a good idea. I'll send a re-roll with that change. Michael
Re: [PATCH 0/2] Avoid rewriting "packed-refs" unnecessarily
On Wed, Oct 25, 2017 at 5:53 AM, Michael Haggerty wrote: > Since commit dc39e09942 (files_ref_store: use a transaction to update > packed refs, 2017-09-08), we've been rewriting the `packed-refs` file > unnecessarily when deleting a loose reference that has no packed > counterpart. Add some tests demonstrating this problem, then fix it by > teaching `files_backend` to see whether any references being deleted > have packed versions, and if not, skipping the packed_refs > transaction. > > This is a mild regression since 2.14, which avoided rewriting the > `packed-refs` file in these cases [...] Should the above information (that this fixes a regression) be mentioned in the commit message of at least one of the patches (probably 2/2)? Without such context, it may not be clear to someone later reading those commit message -- someone who wasn't following this email discussion -- that this behavior had been lost and is now being restored.
[PATCH 0/2] Avoid rewriting "packed-refs" unnecessarily
Since commit dc39e09942 (files_ref_store: use a transaction to update packed refs, 2017-09-08), we've been rewriting the `packed-refs` file unnecessarily when deleting a loose reference that has no packed counterpart. Add some tests demonstrating this problem, then fix it by teaching `files_backend` to see whether any references being deleted have packed versions, and if not, skipping the packed_refs transaction. This is a mild regression since 2.14, which avoided rewriting the `packed-refs` file in these cases (though it still had to *read* the whole file to determine whether the rewrite could be skipped). Therefore, it might be considered for 2.15. On the other hand, it is late in the release cycle, so the risk of accepting this change might be considered too risky. These patches apply on top of master and commute with mh/ref-locking-fix. They are also available from my GitHub fork [1] as branch `avoid-rewriting-packed-refs`. Michael [1] https://github.com/mhagger/git Michael Haggerty (2): t1409: check that `packed-refs` is not rewritten unnecessarily files-backend: don't rewrite the `packed-refs` file unnecessarily refs/files-backend.c | 18 ++- refs/packed-backend.c | 68 refs/packed-backend.h | 8 +++ t/t1409-avoid-packing-refs.sh | 118 ++ 4 files changed, 211 insertions(+), 1 deletion(-) create mode 100755 t/t1409-avoid-packing-refs.sh -- 2.14.1
