[PATCH v3 2/4] refs: support replacing reflogs in a transaction

@@ -1413,7 +1413,6 @@ enum ref_transaction_error ref_transaction_update(struct ref_transaction *transa
 		strbuf_addstr(err, _("refusing to force and skip creation of reflog"));
 		return REF_TRANSACTION_ERROR_GENERIC;
 	}
-
 	if (!transaction_refname_valid(refname, new_oid, flags, err))
 		return REF_TRANSACTION_ERROR_GENERIC;
 

@@ -1496,6 +1495,39 @@ int ref_transaction_update_reflog(struct ref_transaction *transaction,
 	return 0;
 }
 
+int ref_transaction_replace_reflog(struct ref_transaction *transaction,
+				    const char *refname,
+				    struct strbuf *err)
+{
+	size_t i;
+	unsigned int flags = REF_LOG_ONLY | REF_NO_DEREF |
+		REF_SKIP_CREATE_REFLOG | REF_REPLACE_REFLOG;
+
+	assert(err);
+	if (transaction->flags &
+	    (REF_TRANSACTION_FLAG_INITIAL | REF_TRANSACTION_ALLOW_FAILURE)) {
+		strbuf_addstr(err, _("reflog replacement requires an "
+				     "all-or-nothing, non-initial transaction"));
+		return REF_TRANSACTION_ERROR_GENERIC;
+	}
+	if (!transaction_refname_valid(refname, NULL, flags, err))
+		return REF_TRANSACTION_ERROR_GENERIC;
+	for (i = 0; i < transaction->nr; i++) {
+		struct ref_update *update = transaction->updates[i];
+
+		if ((update->flags & REF_REPLACE_REFLOG) &&
+		    !strcmp(update->refname, refname)) {
+			strbuf_addf(err, _("reflog replacement for '%s' already queued"),
+				    refname);
+			return REF_TRANSACTION_ERROR_NAME_CONFLICT;
+		}
+	}
+
+	ref_transaction_add_update(transaction, refname, flags,
+				   NULL, NULL, NULL, NULL, NULL, NULL, NULL);
+	return 0;
+}
+
 int ref_transaction_create(struct ref_transaction *transaction,
 			   const char *refname,
 			   const struct object_id *new_oid,

@@ -939,6 +939,15 @@ int ref_transaction_update_reflog(struct ref_transaction *transaction,
 				  uint64_t index,
 				  struct strbuf *err);
 
+/*
+ * Replace a reference's reflog with the explicit entries added to the same
+ * transaction via ref_transaction_update_reflog(). Without such entries,
+ * remove the reflog. This operation does not change the reference itself.
+ */
+int ref_transaction_replace_reflog(struct ref_transaction *transaction,
+				    const char *refname,
+				    struct strbuf *err);
+
 /*
  * Add a reference creation to transaction. new_oid is the value that
  * the reference should have after the update; it must not be

@@ -1983,6 +1983,22 @@ static int files_create_reflog(struct ref_store *ref_store, const char *refname,
 	return 0;
 }
 
+static void format_reflog_entry(struct strbuf *sb,
+				const struct object_id *old_oid,
+				const struct object_id *new_oid,
+				const char *committer, const char *msg)
+{
+	if (!committer)
+		committer = git_committer_info(0);
+
+	strbuf_addf(sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer);
+	if (msg && *msg) {
+		strbuf_addch(sb, '\t');
+		strbuf_addstr(sb, msg);
+	}
+	strbuf_addch(sb, '\n');
+}
+
 static int log_ref_write_fd(int fd, const struct object_id *old_oid,
 			    const struct object_id *new_oid,
 			    const char *committer, const char *msg)

@@ -1990,15 +2006,7 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid,
 	struct strbuf sb = STRBUF_INIT;
 	int ret = 0;
 
-	if (!committer)
-		committer = git_committer_info(0);
-
-	strbuf_addf(&sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer);
-	if (msg && *msg) {
-		strbuf_addch(&sb, '\t');
-		strbuf_addstr(&sb, msg);
-	}
-	strbuf_addch(&sb, '\n');
+	format_reflog_entry(&sb, old_oid, new_oid, committer, msg);
 	if (write_in_full(fd, sb.buf, sb.len) < 0)
 		ret = -1;
 	strbuf_release(&sb);

@@ -2397,6 +2405,8 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 
 	while (!ret && !strbuf_getwholeline(&sb, logfp, '\n'))
 		ret = show_one_reflog_ent(refs, refname, &sb, fn, cb_data);
+	if (ferror(logfp))
+		ret = -1;
 	fclose(logfp);
 	strbuf_release(&sb);
 	return ret;

@@ -2665,12 +2675,120 @@ static enum ref_transaction_error check_old_oid(struct ref_update *update,
 	return REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE;
 }
 
+struct staged_reflog {
+	struct tempfile *file;
+	struct ref_update **updates;
+	size_t nr, alloc;
+};
+
 struct files_transaction_backend_data {
 	struct ref_transaction *packed_transaction;
 	int packed_refs_locked;
 	struct strmap ref_locks;
+	struct strmap reflog_files;
 };
 
+static int reflog_update_cmp(const void *a, const void *b)
+{
+	const struct ref_update *one = *(struct ref_update * const *)a;
+	const struct ref_update *two = *(struct ref_update * const *)b;
+
+	return (one->index > two->index) - (one->index < two->index);
+}
+
+static int prepare_reflog_replacements(struct files_ref_store *refs,
+				      struct ref_transaction *transaction,
+				      struct strbuf *err)
+{
+	struct files_transaction_backend_data *data = transaction->backend_data;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf contents = STRBUF_INIT;
+	struct hashmap_iter iter;
+	struct strmap_entry *entry;
+	size_t i;
+	int ret = -1;
+
+	for (i = 0; i < transaction->nr; i++) {
+		struct ref_update *update = transaction->updates[i];
+		struct staged_reflog *log;
+
+		if (!(update->flags & REF_REPLACE_REFLOG))
+			continue;
+		CALLOC_ARRAY(log, 1);
+		strmap_put(&data->reflog_files, update->refname, log);
+	}
+	for (i = 0; i < transaction->nr; i++) {
+		struct ref_update *update = transaction->updates[i];
+		struct staged_reflog *log = strmap_get(&data->reflog_files, update->refname);
+
+		if (!log || !(update->flags & REF_LOG_ONLY) ||
+		    !(update->flags & REF_HAVE_NEW))
+			continue;
+		if (!(update->flags & REF_LOG_USE_PROVIDED_OIDS)) {
+			strbuf_addf(err, "replacing reflog '%s' requires explicit OIDs",
+				    update->refname);
+			goto out;
+		}
+		ALLOC_GROW(log->updates, log->nr + 1, log->alloc);
+		log->updates[log->nr++] = update;
+	}
+	strmap_for_each_entry(&data->reflog_files, &iter, entry) {
+		struct staged_reflog *log = entry->value;
+		int fd;
+
+		if (!log->nr)
+			continue;
+		QSORT(log->updates, log->nr, reflog_update_cmp);
+		/* Keep the staging file beside the logs, which may be on another device. */
+		strbuf_reset(&path);
+		files_reflog_path(refs, &path, is_root_ref(entry->key) ?
+				 ".tmp-reflog-XXXXXX" : "refs/.tmp-reflog-XXXXXX");
+		if (safe_create_leading_directories(refs->base.repo, path.buf))
+			goto write_error;
+		log->file = mks_tempfile_m(path.buf, 0666);
+		if (!log->file)
+			goto write_error;
+		fd = get_tempfile_fd(log->file);
+		for (i = 0; i < log->nr; i++) {
+			struct ref_update *update = log->updates[i];
+
+			format_reflog_entry(&contents, &update->old_oid, &update->new_oid,
+					    update->committer_info, update->msg);
+			if (contents.len >= 65536) {
+				if (write_in_full(fd, contents.buf, contents.len) < 0)
+					goto write_error;
+				strbuf_reset(&contents);
+			}
+		}
+		if (write_in_full(fd, contents.buf, contents.len) < 0)
+			goto write_error;
+		strbuf_reset(&contents);
+		if (adjust_shared_perm(refs->base.repo, get_tempfile_path(log->file)) ||
+		    close_tempfile_gently(log->file))
+			goto write_error;
+	}
+	ret = 0;
+	goto out;
+
+write_error:
+	strbuf_addf(err, "cannot write staged reflog: %s", strerror(errno));
+out:
+	strbuf_release(&path);
+	strbuf_release(&contents);
+	return ret;
+}
+
+static int install_reflog(const char *path, void *data)
+{
+	struct staged_reflog *log = data;
+	int ret = rename(get_tempfile_path(log->file), path);
+
+	/* Some systems report ENOTDIR when the destination is a directory. */
+	if (ret && errno == ENOTDIR)
+		errno = EISDIR;
+	return ret;
+}
+
 /*
  * Prepare for carrying out update:
  * - Lock the reference referred to by update.

@@ -2929,6 +3047,17 @@ static void files_transaction_cleanup(struct files_ref_store *refs,
 	}
 
 	if (backend_data) {
+		struct hashmap_iter iter;
+		struct strmap_entry *entry;
+
+		strmap_for_each_entry(&backend_data->reflog_files, &iter, entry) {
+			struct staged_reflog *log = entry->value;
+
+			delete_tempfile(&log->file);
+			free(log->updates);
+			free(log);
+		}
+		strmap_clear(&backend_data->reflog_files, 0);
 		if (backend_data->packed_transaction &&
 		    ref_transaction_abort(backend_data->packed_transaction, &err)) {
 			error("error aborting transaction: %s", err.buf);

@@ -2970,6 +3099,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 
 	CALLOC_ARRAY(backend_data, 1);
 	strmap_init(&backend_data->ref_locks);
+	strmap_init(&backend_data->reflog_files);
 	transaction->backend_data = backend_data;
 
 	/*

@@ -3120,6 +3250,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 		}
 	}
 
+	ret = prepare_reflog_replacements(refs, transaction, err);
+
 cleanup:
 	free(head_ref);
 	string_list_clear(&refnames_to_check, 1);

@@ -3344,6 +3476,31 @@ static int files_transaction_finish(struct ref_store *ref_store,
 	backend_data = transaction->backend_data;
 	packed_transaction = backend_data->packed_transaction;
 
+	for (i = 0; i < transaction->nr; i++) {
+		struct ref_update *update = transaction->updates[i];
+		struct staged_reflog *log;
+
+		if (!(update->flags & REF_REPLACE_REFLOG))
+			continue;
+		log = strmap_get(&backend_data->reflog_files, update->refname);
+		strbuf_reset(&sb);
+		files_reflog_path(refs, &sb, update->refname);
+		if (!log->file) {
+			if (unlink(sb.buf) && errno != ENOENT && errno != EISDIR)
+				ret = -1;
+		} else if (raceproof_create_file(refs, sb.buf, install_reflog, log)) {
+			ret = -1;
+		} else {
+			delete_tempfile(&log->file);
+		}
+		if (ret) {
+			strbuf_addf(err, "cannot replace reflog '%s': %s",
+				    update->refname, strerror(errno));
+		}
+		if (ret)
+			goto cleanup;
+	}
+
 	/* Perform updates first so live commits remain referenced */
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];

@@ -3352,8 +3509,9 @@ static int files_transaction_finish(struct ref_store *ref_store,
 		if (update->rejection_err)
 			continue;
 
-		if (update->flags & REF_NEEDS_COMMIT ||
-		    update->flags & REF_LOG_ONLY) {
+		if ((update->flags & REF_NEEDS_COMMIT ||
+		     update->flags & REF_LOG_ONLY) &&
+		    !strmap_contains(&backend_data->reflog_files, update->refname)) {
 			if (parse_and_write_reflog(refs, update, lock, err)) {
 				ret = REF_TRANSACTION_ERROR_GENERIC;
 				goto cleanup;

@@ -49,6 +49,9 @@ struct ref_transaction;
  */
 #define REF_HAVE_PEELED (1 << 15)
 
+/* Replace a reflog with the explicit log-only updates in the transaction. */
+#define REF_REPLACE_REFLOG (1 << 16)
+
 /*
  * Return the length of time to retry acquiring a loose reference lock
  * before giving up, in milliseconds:

@@ -1503,9 +1503,11 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 		 * - `core.logAllRefUpdates` tells us to create the reflog for
 		 *   the given ref.
 		 */
-		if ((u->flags & REF_HAVE_NEW) &&
+		if ((u->flags & REF_REPLACE_REFLOG) ||
+		    ((u->flags & REF_HAVE_NEW) &&
+		    !(u->flags & REF_LOG_ONLY) &&
 		    !(u->type & REF_ISSYMREF) &&
-		    ref_update_has_null_new_value(u)) {
+		    ref_update_has_null_new_value(u))) {
 			struct reftable_log_record log = {0};
 			struct reftable_iterator it = {0};
 

@@ -1548,8 +1550,10 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 
 			if (ret)
 				goto done;
-		} else if (!(u->flags & REF_SKIP_CREATE_REFLOG) &&
+		}
+		if (!(u->flags & REF_SKIP_CREATE_REFLOG) &&
 			   (u->flags & REF_HAVE_NEW) &&
+			   (!ref_update_has_null_new_value(u) || (u->flags & REF_LOG_ONLY)) &&
 			   (u->flags & REF_FORCE_CREATE_REFLOG ||
 			    should_write_log(arg->refs, u->refname))) {
 			struct reftable_log_record *log;

@@ -146,6 +146,45 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv)
 	return refs_rename_ref(refs, oldref, newref, logmsg);
 }
 
+static int cmd_reflog_transaction(struct ref_store *refs, const char **argv)
+{
+	const char *refname = notnull(*argv++, "refname");
+	const char *mode = notnull(*argv++, "mode");
+	struct strbuf err = STRBUF_INIT;
+	struct ref_transaction *transaction = ref_store_transaction_begin(refs, 0, &err);
+	int ret = !transaction;
+
+	if (!ret) {
+		if (!strcmp(mode, "replace")) {
+			ret = ref_transaction_replace_reflog(transaction, refname, &err);
+		} else if (!strcmp(mode, "replace-twice")) {
+			ret = ref_transaction_replace_reflog(transaction, refname, &err) ||
+				ref_transaction_replace_reflog(transaction, refname, &err);
+		} else if (strcmp(mode, "append")) {
+			die("unknown reflog transaction mode: %s", mode);
+		}
+	}
+	while (!ret && *argv) {
+		uint64_t index = strtoumax(notnull(*argv++, "index"), NULL, 10);
+		struct object_id old_oid, new_oid;
+		const char *message;
+
+		if (get_oid_hex(notnull(*argv++, "old-oid"), &old_oid) ||
+		    get_oid_hex(notnull(*argv++, "new-oid"), &new_oid))
+			die("invalid object ID");
+		message = notnull(*argv++, "message");
+		ret = ref_transaction_update_reflog(transaction, refname,
+				&new_oid, &old_oid, NULL, message, index, &err);
+	}
+	if (!ret)
+		ret = ref_transaction_commit(transaction, &err);
+	if (ret)
+		error("%s", err.buf);
+	ref_transaction_free(transaction);
+	strbuf_release(&err);
+	return ret;
+}
+
 static int each_ref(const struct reference *ref, void *cb_data UNUSED)
 {
 	printf("%s %s 0x%x\n", oid_to_hex(ref->oid), ref->name, ref->flags);

@@ -308,6 +347,7 @@ static struct command commands[] = {
 	{ "create-symref", cmd_create_symref },
 	{ "delete-refs", cmd_delete_refs },
 	{ "rename-ref", cmd_rename_ref },
+	{ "reflog-transaction", cmd_reflog_transaction },
 	{ "for-each-ref", cmd_for_each_ref },
 	{ "for-each-ref--exclude", cmd_for_each_ref__exclude },
 	{ "resolve-ref", cmd_resolve_ref },

@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='explicit reflog entries in reference transactions'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit A &&
+	test_commit B &&
+	A=$(git rev-parse A) &&
+	B=$(git rev-parse B)
+'
+
+test_expect_success 'log-only entry with null new OID does not delete existing history' '
+	git branch source "$A" &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/source >before &&
+	test-tool ref-store main reflog-transaction refs/heads/source append \
+		1 "$A" "$ZERO_OID" deletion &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/source >after &&
+	sed "$ d" after >history &&
+	test_cmp before history &&
+	test_grep "$A $ZERO_OID .*deletion" after &&
+	test_cmp_rev "$A" source
+'
+
+test_expect_success 'replace reflog with entries ordered by index' '
+	test-tool ref-store main reflog-transaction refs/heads/source replace \
+		2 "$A" "$B" second \
+		1 "$ZERO_OID" "$A" first &&
+	git reflog show --format=%gs source >actual &&
+	printf "%s\n" second first >expect &&
+	test_cmp expect actual &&
+	test_cmp_rev "$A" source
+'
+
+test_expect_success 'prepared hook can reject replacement without losing history' '
+	test_when_finished "rm -f .git/hooks/reference-transaction" &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/source >before &&
+	write_script .git/hooks/reference-transaction <<-\EOF &&
+		test "$1" != prepared
+	EOF
+	test_must_fail test-tool ref-store main reflog-transaction refs/heads/source replace \
+		1 "$ZERO_OID" "$B" replacement &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/source >after &&
+	test_cmp before after
+'
+
+test_expect_success 'replacement with no entries removes only the reflog' '
+	test-tool ref-store main reflog-transaction refs/heads/source replace &&
+	test_must_fail git reflog exists refs/heads/source &&
+	test_cmp_rev "$A" source
+'
+
+test_expect_success 'duplicate replacement is rejected' '
+	test_must_fail test-tool ref-store main reflog-transaction \
+		refs/heads/source replace-twice
+'
+
+test_done