Git development
 help / color / mirror / Atom feed
From: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
	Junio C Hamano <gitster@pobox.com>,
	Karthik Nayak <karthik.188@gmail.com>
Subject: [PATCH v4 0/4] refs: run copy and rename through transactions
Date: Thu, 08 Oct 2026 11:44:15 +0200	[thread overview]
Message-ID: <cover.1791452597.git.maciej.ciemborowicz@gmail.com> (raw)
In-Reply-To: <20260920165037.88524-1-maciej.ciemborowicz@gmail.com>

Reference copy and rename bypass the transaction API. With files, the
reference-transaction hook sees only the source deletion; with reftable,
it sees neither endpoint. This series represents the logical ref updates
and reflog history through ordinary transactions, allowing hooks to
observe and reject the operation.

Junio, thank you for reporting the t0600 and t5510 failures in v3. I
reproduced both on macOS with the original base as well. I had not run
those suites before submitting v3; they were outside the test selection
reported in that cover letter. That was a gap in my validation, and I
am sorry for sending the regression.

Patch 2 added a call to prepare_reflog_replacements() after preparing the
packed transaction. If packed preparation failed, its error was saved in
ret, but the new call overwrote it with success. The files transaction
could then proceed despite the failure to prepare packed-refs. The fix
jumps to cleanup after freeing the failed packed transaction, preserving
the error and releasing the remaining resources. The existing t0600.16
and t5510.35 cover this path; their expectations have not been changed.

Changes since v3:

* Rebase onto 6de20f6092 (The 4th batch, 2026-10-06), the master commit
  used in Junio's report.
* Preserve the packed preparation error in patch 2 as described above.
* Register t1425 and t1424 in t/meson.build in the commits adding them.

The four-patch structure and implementation otherwise remain unchanged.
This iteration does not address the concern about the size of the main
patch; the range-diff below isolates the correction and registrations.

I ran the complete standard core test collection, including the unit-test
executable, on macOS/arm64 with each default ref backend:

  files:    Files=1064, Tests=34710, Result: PASS
  reftable: Files=1064, Tests=34712, Result: PASS

Both full runs include successful t0600, t5510, t1424 and t1425 runs.
The build enables Perl, Python, cURL and gettext. The files run used:

  LC_ALL=C make -j8 PYTHON_PATH=/opt/homebrew/bin/python3 \
    GNU_GETTEXT_PATH=/opt/homebrew/opt/gettext \
    DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS='--jobs 8' test

The reftable run used GIT_TEST_DEFAULT_REF_FORMAT=reftable and ran every
t[0-9][0-9][0-9][0-9]-*.sh plus unit-tests/bin/unit-tests through prove,
with four jobs and a separate TEST_OUTPUT_DIRECTORY. Backend-specific
suites can override the default. The Meson registration check and
git diff --check also pass. The contrib test target completed with no
unexpected failures; diff-highlight retains two known TODO failures.

The harness totals include skips and expected failures. The files run
skipped 126 whole scripts, mostly for unavailable SVN, Perforce and CVS
tools. Other skips include GPG, JGit, Windows-specific tests, tests
requiring sudo or writable /, case-sensitive filesystem tests, the
opt-in 2GB clone test, and t5564 because its web-server setup failed.
There are also individual prerequisite-based skips within suites.
I have not validated this iteration on Linux or Windows.

The performance trade-off described in v3 remains: replaying history
uses O(N) time and memory, replacing the files backend's constant-time
reflog rename. The measurements in patch 3 are from the v3 base, not a
new benchmark on this base. Files transactions also remain non-atomic
with respect to process crashes and failures late in finish.

Previous iteration:
https://lore.kernel.org/git/cover.1791395643.git.maciej.ciemborowicz@gmail.com/

Maciej Ciemborowicz (4):
  refs: distinguish internal transactions from logical updates
  refs: support replacing reflogs in a transaction
  refs: run copy and rename through ordinary transactions
  refs: remove backend-specific copy and rename callbacks

 Documentation/githooks.adoc     |  10 +
 refs.c                          | 200 ++++++++-
 refs.h                          |  25 ++
 refs/debug.c                    |  24 --
 refs/files-backend.c            | 740 +++++++++++++++++---------------
 refs/packed-backend.c           |   2 -
 refs/refs-internal.h            |  28 +-
 refs/reftable-backend.c         | 334 ++------------
 t/helper/test-ref-store.c       |  76 ++++
 t/meson.build                   |   2 +
 t/perf/p1424-ref-copy-rename.sh |  48 +++
 t/t1424-ref-copy-transaction.sh | 352 +++++++++++++++
 t/t1425-reflog-transaction.sh   |  59 +++
 13 files changed, 1209 insertions(+), 691 deletions(-)
 create mode 100755 t/perf/p1424-ref-copy-rename.sh
 create mode 100755 t/t1424-ref-copy-transaction.sh
 create mode 100755 t/t1425-reflog-transaction.sh

Range-diff against v3:
1:  6d7c146e57 = 1:  948927d8fb refs: distinguish internal transactions from logical updates
2:  5c3ec4eb49 ! 2:  d023da3c09 refs: support replacing reflogs in a transaction
    @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref
      	transaction->backend_data = backend_data;
      
      	/*
    +@@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref_store,
    + 			if (ret) {
    + 				ref_transaction_free(packed_transaction);
    + 				backend_data->packed_transaction = NULL;
    ++				goto cleanup;
    + 			}
    + 		} else {
    + 			/*
     @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref_store,
      		}
      	}
    @@ t/helper/test-ref-store.c: static struct command commands[] = {
      	{ "for-each-ref--exclude", cmd_for_each_ref__exclude },
      	{ "resolve-ref", cmd_resolve_ref },
     
    + ## t/meson.build ##
    +@@ t/meson.build: integration_tests = [
    +   't1421-reflog-write.sh',
    +   't1422-show-ref-exists.sh',
    +   't1423-ref-backend.sh',
    ++  't1425-reflog-transaction.sh',
    +   't1430-bad-ref-name.sh',
    +   't1450-fsck.sh',
    +   't1451-fsck-buffer.sh',
    +
      ## t/t1425-reflog-transaction.sh (new) ##
     @@
     +#!/bin/sh
3:  77af4e809c ! 3:  150349f9d0 refs: run copy and rename through ordinary transactions
    @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref
     -		if (update->flags & REF_DELETING &&
     +		if (update->flags & (REF_DELETING | REF_NEEDS_PACK) &&
      		    !(update->flags & REF_LOG_ONLY) &&
    - 		    !(update->flags & REF_IS_PRUNING)) {
    - 			/*
    + 		    !(update->flags & REF_IS_PRUNING) &&
    + 		    !is_root_ref(update->refname)) {
     @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref_store,
      					REF_HAVE_NEW | REF_NO_DEREF,
      					&update->new_oid, NULL, NULL,
    @@ t/helper/test-ref-store.c: static struct command commands[] = {
      	{ "for-each-ref--exclude", cmd_for_each_ref__exclude },
      	{ "resolve-ref", cmd_resolve_ref },
     
    + ## t/meson.build ##
    +@@ t/meson.build: integration_tests = [
    +   't1421-reflog-write.sh',
    +   't1422-show-ref-exists.sh',
    +   't1423-ref-backend.sh',
    ++  't1424-ref-copy-transaction.sh',
    +   't1425-reflog-transaction.sh',
    +   't1430-bad-ref-name.sh',
    +   't1450-fsck.sh',
    +
      ## t/perf/p1424-ref-copy-rename.sh (new) ##
     @@
     +#!/bin/sh
4:  83fa644fb3 = 4:  f3f2c7ee11 refs: remove backend-specific copy and rename callbacks

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
-- 
2.39.3 (Apple Git-146)

  parent reply	other threads:[~2026-10-08  9:44 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 13:33 [BUG] reference-transaction hook misses destination of git branch -m Maciej Ciemborowicz
2026-09-19 20:52 ` Karthik Nayak
2026-09-20 16:50   ` [PATCH] refs: run copy and rename through transactions Maciej Ciemborowicz
2026-09-21 17:54     ` Junio C Hamano
2026-09-21 23:28       ` Junio C Hamano
2026-09-22 13:08         ` Maciej Ciemborowicz
2026-09-23 13:36     ` [PATCH v2] " Maciej Ciemborowicz
2026-09-30  3:32       ` Maciej Ciemborowicz
2026-10-02 10:56       ` Patrick Steinhardt
2026-10-02 14:16         ` Maciej Ciemborowicz
2026-10-05  6:03           ` Patrick Steinhardt
2026-10-07 18:05     ` [PATCH v3 0/4] " Maciej Ciemborowicz
2026-10-07 18:05       ` [PATCH v3 1/4] refs: distinguish internal transactions from logical updates Maciej Ciemborowicz
2026-10-07 18:05       ` [PATCH v3 2/4] refs: support replacing reflogs in a transaction Maciej Ciemborowicz
2026-10-07 18:05       ` [PATCH v3 3/4] refs: run copy and rename through ordinary transactions Maciej Ciemborowicz
2026-10-07 18:05       ` [PATCH v3 4/4] refs: remove backend-specific copy and rename callbacks Maciej Ciemborowicz
2026-10-07 19:55       ` [PATCH v3 0/4] refs: run copy and rename through transactions Junio C Hamano
2026-10-08  9:44     ` Maciej Ciemborowicz [this message]
2026-10-08  9:44       ` [PATCH v4 1/4] refs: distinguish internal transactions from logical updates Maciej Ciemborowicz
2026-10-08  9:44       ` [PATCH v4 2/4] refs: support replacing reflogs in a transaction Maciej Ciemborowicz
2026-10-08  9:44       ` [PATCH v4 3/4] refs: run copy and rename through ordinary transactions Maciej Ciemborowicz
2026-10-08  9:44       ` [PATCH v4 4/4] refs: remove backend-specific copy and rename callbacks Maciej Ciemborowicz
2026-10-08 10:10       ` [PATCH v4 0/4] refs: run copy and rename through transactions Patrick Steinhardt
2026-10-08 10:43         ` Maciej Ciemborowicz
2026-10-08 11:01           ` Maciej Ciemborowicz
2026-10-08 15:45             ` Junio C Hamano
2026-10-08 19:06               ` Maciej Ciemborowicz
2026-10-08 19:19                 ` Kristoffer Haugsbakk
2026-10-08 21:11                   ` Maciej Ciemborowicz
2026-10-09  5:41                 ` Patrick Steinhardt
2026-10-08 15:54         ` Junio C Hamano
2026-09-23 12:49   ` [PATCH v4 0/3] refs: report old OIDs for batched deletions Maciej Ciemborowicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1791452597.git.maciej.ciemborowicz@gmail.com \
    --to=maciej.ciemborowicz@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox