[PATCH 08/15] refs.c: add a flag to allow reflog updates to truncate the log

[Ronnie Sahlberg]

commit 8d0a342375fbd926ae6ae93f9be42a436a787fb6 upstream.

Add a flag that allows us to truncate the reflog before we write the
update.

Change-Id: I187b22e0910499b1976689b46866698b6fd46eb3
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 17 +++++++++++++++--
refs.h | 10 +++++++++-
2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 100b3a3..d54c3b9 100644
--- a/refs.c
+++ b/refs.c
@@ -3873,7 +3873,12 @@ int transaction_commit(struct transaction *transaction,
}
}

- /* Update all reflog files */
+ /*
+ * Update all reflog files
+ * We have already done all ref updates and deletes.
+ * There is not much we can do here if there are any reflog
+ * update errors other than complain.
+ */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];

@@ -3881,7 +3886,15 @@ int transaction_commit(struct transaction *transaction,
continue;
if (update->reflog_fd == -1)
continue;
-
+ if (update->flags & REFLOG_TRUNCATE)
+ if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
+ ftruncate(update->reflog_fd, 0)) {
+ error("Could not truncate reflog: %s. %s",
+ update->refname, strerror(errno));
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ continue;
+ }
if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
update->new_sha1,
update->committer, update->msg)) {
diff --git a/refs.h b/refs.h
index 8220d18..5075073 100644
--- a/refs.h
+++ b/refs.h
@@ -328,7 +328,15 @@ int transaction_delete_ref(struct transaction *transaction,
struct strbuf *err);

/*
- * Append a reflog entry for refname.
+ * Flags controlling transaction_update_reflog().
+ * REFLOG_TRUNCATE: Truncate the reflog.
+ *
+ * Flags >= 0x100 are reserved for internal use.
+ */
+#define REFLOG_TRUNCATE 0x01
+/*
+ * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
+ * this update will first truncate the reflog before writing the entry.
*/
int transaction_update_reflog(struct transaction *transaction,
const char *refname,

[Ronnie Sahlberg]

commit de045215a52a6a9591b0e786488de2293d79d245 upstream.

Define a new transaction update type, UPDATE_LOG, and a new function
transaction_update_reflog. This function will lock the reflog and append
an entry to it during transaction commit.

Change-Id: I8cc935ef311688d561d447fa51b44ac98492693b
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
refs.h | 12 ++++++++
2 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 0e11b1c..100b3a3 100644
--- a/refs.c
+++ b/refs.c
@@ -3517,7 +3517,8 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
}

enum transaction_update_type {
- UPDATE_SHA1 = 0
+ UPDATE_SHA1 = 0,
+ UPDATE_LOG = 1
};

/**
@@ -3535,6 +3536,12 @@ struct ref_update {
struct ref_lock *lock;
int type;
char *msg;
+
+ /* used by reflog updates */
+ int reflog_fd;
+ struct lock_file reflog_lock;
+ char *committer;
+
const char refname[FLEX_ARRAY];
};

@@ -3581,6 +3588,7 @@ void transaction_free(struct transaction *transaction)

for (i = 0; i < transaction->nr; i++) {
free(transaction->updates[i]->msg);
+ free(transaction->updates[i]->committer);
free(transaction->updates[i]);
}
free(transaction->updates);
@@ -3601,6 +3609,41 @@ static struct ref_update *add_update(struct transaction *transaction,
return update;
}

+int transaction_update_reflog(struct transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ const unsigned char *email,
+ unsigned long timestamp, int tz,
+ const char *msg, int flags,
+ struct strbuf *err)
+{
+ struct ref_update *update;
+
+ if (transaction->state != TRANSACTION_OPEN)
+ die("BUG: update_reflog called for transaction that is not open");
+
+ update = add_update(transaction, refname, UPDATE_LOG);
+ hashcpy(update->new_sha1, new_sha1);
+ hashcpy(update->old_sha1, old_sha1);
+ update->reflog_fd = -1;
+ if (email) {
+ struct strbuf buf = STRBUF_INIT;
+ char sign = (tz < 0) ? '-' : '+';
+ int zone = (tz < 0) ? (-tz) : tz;
+
+ strbuf_addf(&buf, "%s %lu %c%04d", email, timestamp, sign,
+ zone);
+ update->committer = xstrdup(buf.buf);
+ strbuf_release(&buf);
+ }
+ if (msg)
+ update->msg = xstrdup(msg);
+ update->flags = flags;
+
+ return 0;
+}
+
int transaction_update_ref(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
@@ -3773,7 +3816,28 @@ int transaction_commit(struct transaction *transaction,
}
}

- /* Perform updates first so live commits remain referenced */
+ /* Lock all reflog files */
+ for (i = 0; i < n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update->update_type != UPDATE_LOG)
+ continue;
+ update->reflog_fd = hold_lock_file_for_append(
+ &update->reflog_lock,
+ git_path("logs/%s", update->refname),
+ 0);
+ if (update->reflog_fd < 0) {
+ const char *str = "Cannot lock reflog for '%s'. %s";
+
+ ret = -1;
+ if (err)
+ strbuf_addf(err, str, update->refname,
+ strerror(errno));
+ goto cleanup;
+ }
+ }
+
+ /* Perform ref updates first so live commits remain referenced */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];

@@ -3809,6 +3873,40 @@ int transaction_commit(struct transaction *transaction,
}
}

+ /* Update all reflog files */
+ for (i = 0; i < n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update->update_type != UPDATE_LOG)
+ continue;
+ if (update->reflog_fd == -1)
+ continue;
+
+ if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
+ update->new_sha1,
+ update->committer, update->msg)) {
+ error("Could write to reflog: %s. %s",
+ update->refname, strerror(errno));
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ }
+ }
+
+ /* Commit all reflog files */
+ for (i = 0; i < n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update->update_type != UPDATE_LOG)
+ continue;
+ if (update->reflog_fd == -1)
+ continue;
+ if (commit_lock_file(&update->reflog_lock)) {
+ error("Could not commit reflog: %s. %s",
+ update->refname, strerror(errno));
+ update->reflog_fd = -1;
+ }
+ }
+
if (repack_without_refs(delnames, delnum, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/refs.h b/refs.h
index 556adfd..8220d18 100644
--- a/refs.h
+++ b/refs.h
@@ -328,6 +328,18 @@ int transaction_delete_ref(struct transaction *transaction,
struct strbuf *err);

/*
+ * Append a reflog entry for refname.
+ */
+int transaction_update_reflog(struct transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ const unsigned char *email,
+ unsigned long timestamp, int tz,
+ const char *msg, int flags,
+ struct strbuf *err);
+
+/*
* Commit all of the changes that have been queued in transaction, as
* atomically as possible.
*

[Ronnie Sahlberg]

commit 03001144a015f81db5252005841bb78f57d62774 upstream.

The ref_transaction_update function can already be used to create refs by
passing null_sha1 as the old_sha1 parameter. Simplify by replacing
transaction_create with a thin wrapper.

Change-Id: I687dd47cc4f4e06766e8313b4fd1b07cd4a56c1a
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/refs.c b/refs.c
index 0368ed4..ed0485e 100644
--- a/refs.c
+++ b/refs.c
@@ -3623,31 +3623,8 @@ int ref_transaction_create(struct ref_transaction *transaction,
int flags, const char *msg,
struct strbuf *err)
{
- struct ref_update *update;
-
- assert(err);
-
- if (transaction->state != REF_TRANSACTION_OPEN)
- die("BUG: create called for transaction that is not open");
-
- if (!new_sha1 || is_null_sha1(new_sha1))
- die("BUG: create ref with null new_sha1");
-
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- strbuf_addf(err, "refusing to create ref with bad name %s",
- refname);
- return -1;
- }
-
- update = add_update(transaction, refname);
-
- hashcpy(update->new_sha1, new_sha1);
- hashclr(update->old_sha1);
- update->flags = flags;
- update->have_old = 1;
- if (msg)
- update->msg = xstrdup(msg);
- return 0;
+ return ref_transaction_update(transaction, refname, new_sha1,
+ null_sha1, flags, 1, msg, err);
}

int ref_transaction_delete(struct ref_transaction *transaction,

[Junio C Hamano]

Subject: Re: [PATCH 01/15] refs.c make ref_transaction_create a wrapper to ref_transaction_update

Missing colon after "refs.c"

commit 03001144a015f81db5252005841bb78f57d62774 upstream.

Huh?

The ref_transaction_update function can already be used to create refs by
passing null_sha1 as the old_sha1 parameter. Simplify by replacing
transaction_create with a thin wrapper.

Nice code reduction.

Change-Id: I687dd47cc4f4e06766e8313b4fd1b07cd4a56c1a

Please don't leak local housekeeping details into the official
history.

---
refs.c | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)
diff --git a/refs.c b/refs.c
index 0368ed4..ed0485e 100644
--- a/refs.c
+++ b/refs.c
@@ -3623,31 +3623,8 @@ int ref_transaction_create(struct ref_transaction *transaction,
int flags, const char *msg,
struct strbuf *err)
{
- struct ref_update *update;
-
- assert(err);
-
- if (transaction->state != REF_TRANSACTION_OPEN)
- die("BUG: create called for transaction that is not open");
-
- if (!new_sha1 || is_null_sha1(new_sha1))
- die("BUG: create ref with null new_sha1");
-
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- strbuf_addf(err, "refusing to create ref with bad name %s",
- refname);
- return -1;
- }
-
- update = add_update(transaction, refname);
-
- hashcpy(update->new_sha1, new_sha1);
- hashclr(update->old_sha1);
- update->flags = flags;
- update->have_old = 1;
- if (msg)
- update->msg = xstrdup(msg);
- return 0;
+ return ref_transaction_update(transaction, refname, new_sha1,
+ null_sha1, flags, 1, msg, err);
}
int ref_transaction_delete(struct ref_transaction *transaction,

[Ronnie Sahlberg]

Post by Junio C Hamano

Subject: Re: [PATCH 01/15] refs.c make ref_transaction_create a wrapper to ref_transaction_update

Missing colon after "refs.c"

commit 03001144a015f81db5252005841bb78f57d62774 upstream.

Huh?

The ref_transaction_update function can already be used to create refs by
passing null_sha1 as the old_sha1 parameter. Simplify by replacing
transaction_create with a thin wrapper.

Nice code reduction.

Change-Id: I687dd47cc4f4e06766e8313b4fd1b07cd4a56c1a

Please don't leak local housekeeping details into the official
history.

Ah, Ok.

Do you want me to re-send the series with these lines deleted ?

Post by Junio C Hamano

---
refs.c | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)
diff --git a/refs.c b/refs.c
index 0368ed4..ed0485e 100644
--- a/refs.c
+++ b/refs.c
@@ -3623,31 +3623,8 @@ int ref_transaction_create(struct ref_transaction *transaction,
int flags, const char *msg,
struct strbuf *err)
{
- struct ref_update *update;
-
- assert(err);
-
- if (transaction->state != REF_TRANSACTION_OPEN)
- die("BUG: create called for transaction that is not open");
-
- if (!new_sha1 || is_null_sha1(new_sha1))
- die("BUG: create ref with null new_sha1");
-
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
- strbuf_addf(err, "refusing to create ref with bad name %s",
- refname);
- return -1;
- }
-
- update = add_update(transaction, refname);
-
- hashcpy(update->new_sha1, new_sha1);
- hashclr(update->old_sha1);
- update->flags = flags;
- update->have_old = 1;
- if (msg)
- update->msg = xstrdup(msg);
- return 0;
+ return ref_transaction_update(transaction, refname, new_sha1,
+ null_sha1, flags, 1, msg, err);
}
int ref_transaction_delete(struct ref_transaction *transaction,

[Junio C Hamano]

Post by Ronnie Sahlberg

Post by Junio C Hamano

Subject: Re: [PATCH 01/15] refs.c make ref_transaction_create a
wrapper to ref_transaction_update

Missing colon after "refs.c"
...

Change-Id: I687dd47cc4f4e06766e8313b4fd1b07cd4a56c1a

Please don't leak local housekeeping details into the official
history.

Ah, Ok.
Do you want me to re-send the series with these lines deleted ?

When the series is rerolled, I'd like to see these crufts gone.

But please do not re-send the series without waiting for further
reviews and making necessary updates, if any. Otherwise it will
only make extra busywork for you and me.

[Ronnie Sahlberg]

commit a4369f77d1975566bcd29bfa46720d48372c241d upstream.

unlock|close|commit_ref can be made static since there are no more external
callers.

Change-Id: I743f9daaa1c101fdc781a76283c2e8b462591d07
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 24 ++++++++++++------------
refs.h | 9 ---------
2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/refs.c b/refs.c
index 952cc66..5e5f066 100644
--- a/refs.c
+++ b/refs.c
@@ -2096,6 +2096,16 @@ int refname_match(const char *abbrev_name, const char *full_name)
return 0;
}

+static void unlock_ref(struct ref_lock *lock)
+{
+ /* Do not free lock->lk -- atexit() still looks at them */
+ if (lock->lk)
+ rollback_lock_file(lock->lk);
+ free(lock->ref_name);
+ free(lock->orig_ref_name);
+ free(lock);
+}
+
/* This function should make sure errno is meaningful on error */
static struct ref_lock *verify_lock(struct ref_lock *lock,
const unsigned char *old_sha1, int mustexist)
@@ -2894,7 +2904,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
return 1;
}

-int close_ref(struct ref_lock *lock)
+static int close_ref(struct ref_lock *lock)
{
if (close_lock_file(lock->lk))
return -1;
@@ -2902,7 +2912,7 @@ int close_ref(struct ref_lock *lock)
return 0;
}

-int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock)
{
if (commit_lock_file(lock->lk))
return -1;
@@ -2910,16 +2920,6 @@ int commit_ref(struct ref_lock *lock)
return 0;
}

-void unlock_ref(struct ref_lock *lock)
-{
- /* Do not free lock->lk -- atexit() still looks at them */
- if (lock->lk)
- rollback_lock_file(lock->lk);
- free(lock->ref_name);
- free(lock->orig_ref_name);
- free(lock);
-}
-
/*
* copy the reflog message msg to buf, which has been allocated sufficiently
* large, while cleaning up the whitespaces. Especially, convert LF to space,
diff --git a/refs.h b/refs.h
index 17e3a3c..025e2cb 100644
--- a/refs.h
+++ b/refs.h
@@ -198,15 +198,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p);

-/** Close the file descriptor owned by a lock and return the status */
-extern int close_ref(struct ref_lock *lock);
-
-/** Close and commit the ref locked by the lock */
-extern int commit_ref(struct ref_lock *lock);
-
-/** Release any lock taken but not written. **/
-extern void unlock_ref(struct ref_lock *lock);
-
/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned int flags,
unsigned long at_time, int cnt,

[Ronnie Sahlberg]

commit 8f78d5d7a1e62f04c75524bb3e87aaad24372250 upstream.

log_ref_setup is used to do several semi-related things :
* sometimes it will create a new reflog including missing parent directories
and cleaning up any conflicting stale directories in the path.
* fill in a filename buffer for the full path to the reflog.
* unconditionally re-adjust the permissions for the file.

This function is only called from two places: checkout.c where it is always
used to create a reflog and refs.c:log_ref_write where it sometimes are
used to create a reflog and sometimes just used to fill in the filename.

Rename log_ref_setup to create_reflog and change it to only take the
refname as argument to make its signature similar to delete_reflog and
reflog_exists. Change create_reflog to ignore log_all_ref_updates and
"unconditionally" create the reflog when called. Since checkout.c always
wants to create a reflog we can call create_reflog directly and avoid the
temp-and-log_all_ref_update dance.

In log_ref_write, only call create_reflog iff we want to create a reflog
and if the reflog does not yet exist. This means that for the common case
where the log already exists we now only need to perform a single lstat()
instead of a open(O_CREAT)+lstat()+close().

Change-Id: Ib9493b3b81a97a0e154cc44303a5ed7cdceaaca7
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
builtin/checkout.c | 8 +-------
refs.c | 22 +++++++++++++---------
refs.h | 8 +++-----
3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..8550b6d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,19 +587,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
if (opts->new_branch) {
if (opts->new_orphan_branch) {
if (opts->new_branch_log && !log_all_ref_updates) {
- int temp;
- char log_file[PATH_MAX];
char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);

- temp = log_all_ref_updates;
- log_all_ref_updates = 1;
- if (log_ref_setup(ref_name, log_file, sizeof(log_file))) {
+ if (create_reflog(ref_name)) {
fprintf(stderr, _("Can not do reflog for '%s'\n"),
opts->new_orphan_branch);
- log_all_ref_updates = temp;
return;
}
- log_all_ref_updates = temp;
}
}
else
diff --git a/refs.c b/refs.c
index 3189f11..952cc66 100644
--- a/refs.c
+++ b/refs.c
@@ -2947,16 +2947,16 @@ static int copy_msg(char *buf, const char *msg)
}

/* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, char *logfile, int bufsize)
+int create_reflog(const char *refname)
{
int logfd, oflags = O_APPEND | O_WRONLY;
+ char logfile[PATH_MAX];

- git_snpath(logfile, bufsize, "logs/%s", refname);
- if (log_all_ref_updates &&
- (starts_with(refname, "refs/heads/") ||
- starts_with(refname, "refs/remotes/") ||
- starts_with(refname, "refs/notes/") ||
- !strcmp(refname, "HEAD"))) {
+ git_snpath(logfile, sizeof(logfile), "logs/%s", refname);
+ if (starts_with(refname, "refs/heads/") ||
+ starts_with(refname, "refs/remotes/") ||
+ starts_with(refname, "refs/notes/") ||
+ !strcmp(refname, "HEAD")) {
if (safe_create_leading_directories(logfile) < 0) {
int save_errno = errno;
error("unable to create directory for %s", logfile);
@@ -3025,16 +3025,20 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
static int log_ref_write(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg)
{
- int logfd, result, oflags = O_APPEND | O_WRONLY;
+ int logfd, result = 0, oflags = O_APPEND | O_WRONLY;
char log_file[PATH_MAX];

if (log_all_ref_updates < 0)
log_all_ref_updates = !is_bare_repository();

- result = log_ref_setup(refname, log_file, sizeof(log_file));
+ if (log_all_ref_updates && !reflog_exists(refname))
+ result = create_reflog(refname);
+
if (result)
return result;

+ git_snpath(log_file, sizeof(log_file), "logs/%s", refname);
+
logfd = open(log_file, oflags);
if (logfd < 0)
return 0;
diff --git a/refs.h b/refs.h
index 9f70b89..17e3a3c 100644
--- a/refs.h
+++ b/refs.h
@@ -207,11 +207,6 @@ extern int commit_ref(struct ref_lock *lock);
/** Release any lock taken but not written. **/
extern void unlock_ref(struct ref_lock *lock);

-/*
- * Setup reflog before using. Set errno to something meaningful on failure.
- */
-int log_ref_setup(const char *refname, char *logfile, int bufsize);
-
/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned int flags,
unsigned long at_time, int cnt,
@@ -221,6 +216,9 @@ extern int read_ref_at(const char *refname, unsigned int flags,
/** Check if a particular reflog exists */
extern int reflog_exists(const char *refname);

+/** Create reflog. Set errno to something meaningful on failure. */
+extern int create_reflog(const char *refname);
+
/** Delete a reflog */
extern int delete_reflog(const char *refname);

[Ronnie Sahlberg]

commit fd304abafb351f509b105da2d648298efd78df18 upstream.

Rename the transaction functions. Remove the leading ref_ from the
names and append _ref to the names for functions that create/delete/
update sha1 refs.

This also makes the names more appropriate for future changes that have been
discussed when the transactions could also operate on non ref objects. Such as
..git/config and .gitmodule.

Change-Id: Iffdc56536be71c5061caa23040ce0d89efd52196
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
branch.c | 14 ++++-----
builtin/commit.c | 10 +++----
builtin/fetch.c | 12 ++++----
builtin/receive-pack.c | 13 ++++-----
builtin/replace.c | 10 +++----
builtin/tag.c | 10 +++----
builtin/update-ref.c | 26 ++++++++---------
fast-import.c | 22 +++++++-------
refs.c | 78 +++++++++++++++++++++++++-------------------------
refs.h | 36 +++++++++++------------
sequencer.c | 12 ++++----
walker.c | 10 +++----
12 files changed, 126 insertions(+), 127 deletions(-)

diff --git a/branch.c b/branch.c
index 4bab55a..d9b1174 100644
--- a/branch.c
+++ b/branch.c
@@ -279,17 +279,17 @@ void create_branch(const char *head,
log_all_ref_updates = 1;

if (!dont_change_ref) {
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, msg, &err) ||
- ref_transaction_commit(transaction, &err))
+ transaction_update_ref(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing, msg,
+ &err) ||
+ transaction_commit(transaction, &err))
die("%s", err.buf);
- ref_transaction_free(transaction);
- strbuf_release(&err);
+ transaction_free(transaction);
}

if (real_ref && track)
diff --git a/builtin/commit.c b/builtin/commit.c
index e108c53..f50b7df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;

if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, "HEAD", sha1,
+ transaction_update_ref(transaction, "HEAD", sha1,
current_head
? current_head->object.sha1 : NULL,
0, !!current_head, sb.buf, &err) ||
- ref_transaction_commit(transaction, &err)) {
+ transaction_commit(transaction, &err)) {
rollback_index_files();
die("%s", err.buf);
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);

unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("REVERT_HEAD"));
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6ffd023..323ffd9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv("GIT_REFLOG_ACTION");
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
int ret, df_conflict = 0;

@@ -414,23 +414,23 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), "%s: %s", rla, action);

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, ref->name, ref->new_sha1,
+ transaction_update_ref(transaction, ref->name, ref->new_sha1,
ref->old_sha1, 0, check_old, msg, &err))
goto fail;

- ret = ref_transaction_commit(transaction, &err);
+ ret = transaction_commit(transaction, &err);
if (ret) {
df_conflict = (ret == TRANSACTION_NAME_CONFLICT);
goto fail;
}

- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return 0;
fail:
- ref_transaction_free(transaction);
+ transaction_free(transaction);
error("%s", err.buf);
strbuf_release(&err);
return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 916315f..7ddd756 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -833,26 +833,25 @@ static const char *update(struct command *cmd, struct shallow_info *si)
}
else {
struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
+ struct transaction *transaction;

if (shallow_update && si->shallow_ref[cmd->index] &&
update_shallow_ref(cmd, si))
return "shallow error";

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, namespaced_name,
+ transaction_update_ref(transaction, namespaced_name,
new_sha1, old_sha1, 0, 1, "push",
&err) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_free(transaction);
-
+ transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
rp_error("%s", err.buf);
strbuf_release(&err);
return "failed to update ref";
}

- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return NULL; /* good */
}
diff --git a/builtin/replace.c b/builtin/replace.c
index 85d39b5..5a7ab1f 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -155,7 +155,7 @@ static int replace_object_sha1(const char *object_ref,
unsigned char prev[20];
enum object_type obj_type, repl_type;
char ref[PATH_MAX];
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;

obj_type = sha1_object_info(object, NULL);
@@ -169,14 +169,14 @@ static int replace_object_sha1(const char *object_ref,

check_ref_valid(object, prev, ref, sizeof(ref), force);

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, ref, repl, prev,
+ transaction_update_ref(transaction, ref, repl, prev,
0, 1, NULL, &err) ||
- ref_transaction_commit(transaction, &err))
+ transaction_commit(transaction, &err))
die("%s", err.buf);

- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}

diff --git a/builtin/tag.c b/builtin/tag.c
index e633f4e..5f3554b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -583,7 +583,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
@@ -730,13 +730,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, &opt, prev, object);

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, object, prev,
+ transaction_update_ref(transaction, ref.buf, object, prev,
0, 1, NULL, &err) ||
- ref_transaction_commit(transaction, &err))
+ transaction_commit(transaction, &err))
die("%s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
if (force && !is_null_sha1(prev) && hashcmp(prev, object))
printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6c9be05..af08dd9 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -175,7 +175,7 @@ static int parse_next_sha1(struct strbuf *input, const char **next,
* depending on how line_termination is set.
*/

-static const char *parse_cmd_update(struct ref_transaction *transaction,
+static const char *parse_cmd_update(struct transaction *transaction,
struct strbuf *input, const char *next)
{
struct strbuf err = STRBUF_INIT;
@@ -198,7 +198,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,
if (*next != line_termination)
die("update %s: extra input: %s", refname, next);

- if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ if (transaction_update_ref(transaction, refname, new_sha1, old_sha1,
update_flags, have_old, msg, &err))
die("%s", err.buf);

@@ -209,7 +209,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,
return next;
}

-static const char *parse_cmd_create(struct ref_transaction *transaction,
+static const char *parse_cmd_create(struct transaction *transaction,
struct strbuf *input, const char *next)
{
struct strbuf err = STRBUF_INIT;
@@ -229,7 +229,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,
if (*next != line_termination)
die("create %s: extra input: %s", refname, next);

- if (ref_transaction_create(transaction, refname, new_sha1,
+ if (transaction_create_ref(transaction, refname, new_sha1,
update_flags, msg, &err))
die("%s", err.buf);

@@ -240,7 +240,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,
return next;
}

-static const char *parse_cmd_delete(struct ref_transaction *transaction,
+static const char *parse_cmd_delete(struct transaction *transaction,
struct strbuf *input, const char *next)
{
struct strbuf err = STRBUF_INIT;
@@ -264,7 +264,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,
if (*next != line_termination)
die("delete %s: extra input: %s", refname, next);

- if (ref_transaction_delete(transaction, refname, old_sha1,
+ if (transaction_delete_ref(transaction, refname, old_sha1,
update_flags, have_old, msg, &err))
die("%s", err.buf);

@@ -275,7 +275,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,
return next;
}

-static const char *parse_cmd_verify(struct ref_transaction *transaction,
+static const char *parse_cmd_verify(struct transaction *transaction,
struct strbuf *input, const char *next)
{
struct strbuf err = STRBUF_INIT;
@@ -300,7 +300,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
if (*next != line_termination)
die("verify %s: extra input: %s", refname, next);

- if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ if (transaction_update_ref(transaction, refname, new_sha1, old_sha1,
update_flags, have_old, msg, &err))
die("%s", err.buf);

@@ -320,7 +320,7 @@ static const char *parse_cmd_option(struct strbuf *input, const char *next)
return next + 8;
}

-static void update_refs_stdin(struct ref_transaction *transaction)
+static void update_refs_stdin(struct transaction *transaction)
{
struct strbuf input = STRBUF_INIT;
const char *next;
@@ -376,9 +376,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)

if (read_stdin) {
struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
+ struct transaction *transaction;

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction)
die("%s", err.buf);
if (delete || no_deref || argc > 0)
@@ -386,9 +386,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
if (end_null)
line_termination = '\0';
update_refs_stdin(transaction);
- if (ref_transaction_commit(transaction, &err))
+ if (transaction_commit(transaction, &err))
die("%s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return 0;
}
diff --git a/fast-import.c b/fast-import.c
index d0bd285..152d944 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1687,7 +1687,7 @@ found_entry:
static int update_branch(struct branch *b)
{
static const char *msg = "fast-import";
- struct ref_transaction *transaction;
+ struct transaction *transaction;
unsigned char old_sha1[20];
struct strbuf err = STRBUF_INIT;

@@ -1713,17 +1713,17 @@ static int update_branch(struct branch *b)
return -1;
}
}
- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
+ transaction_update_ref(transaction, b->name, b->sha1, old_sha1,
0, 1, msg, &err) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_free(transaction);
+ transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
error("%s", err.buf);
strbuf_release(&err);
return -1;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return 0;
}
@@ -1745,9 +1745,9 @@ static void dump_tags(void)
struct tag *t;
struct strbuf ref_name = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
+ struct transaction *transaction;

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction) {
failure |= error("%s", err.buf);
goto cleanup;
@@ -1756,17 +1756,17 @@ static void dump_tags(void)
strbuf_reset(&ref_name);
strbuf_addf(&ref_name, "refs/tags/%s", t->name);

- if (ref_transaction_update(transaction, ref_name.buf, t->sha1,
+ if (transaction_update_ref(transaction, ref_name.buf, t->sha1,
NULL, 0, 0, msg, &err)) {
failure |= error("%s", err.buf);
goto cleanup;
}
}
- if (ref_transaction_commit(transaction, &err))
+ if (transaction_commit(transaction, &err))
failure |= error("%s", err.buf);

cleanup:
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&ref_name);
strbuf_release(&err);
}
diff --git a/refs.c b/refs.c
index c607ab7..a1bfaa2 100644
--- a/refs.c
+++ b/refs.c
@@ -26,7 +26,7 @@ static unsigned char refname_disposition[256] = {
};

/*
- * Used as a flag to ref_transaction_delete when a loose ref is being
+ * Used as a flag to transaction_delete_ref when a loose ref is being
* pruned.
*/
#define REF_ISPRUNING 0x0100
@@ -2533,23 +2533,23 @@ static void try_remove_empty_parents(char *name)
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;

if (check_refname_format(r->name, 0))
return;

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_delete(transaction, r->name, r->sha1,
+ transaction_delete_ref(transaction, r->name, r->sha1,
REF_ISPRUNING, 1, NULL, &err) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_free(transaction);
+ transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
error("%s", err.buf);
strbuf_release(&err);
return;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
try_remove_empty_parents(r->name);
}
@@ -2711,20 +2711,20 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)

int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_delete(transaction, refname, sha1, delopt,
+ transaction_delete_ref(transaction, refname, sha1, delopt,
sha1 && !is_null_sha1(sha1), NULL, &err) ||
- ref_transaction_commit(transaction, &err)) {
+ transaction_commit(transaction, &err)) {
error("%s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return 1;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
return 0;
}
@@ -3531,9 +3531,9 @@ struct ref_update {
* an active transaction or if there is a failure while building
* the transaction thus rendering it failed/inactive.
*/
-enum ref_transaction_state {
- REF_TRANSACTION_OPEN = 0,
- REF_TRANSACTION_CLOSED = 1
+enum transaction_state {
+ TRANSACTION_OPEN = 0,
+ TRANSACTION_CLOSED = 1
};

/*
@@ -3541,21 +3541,21 @@ enum ref_transaction_state {
* consist of checks and updates to multiple references, carried out
* as atomically as possible. This structure is opaque to callers.
*/
-struct ref_transaction {
+struct transaction {
struct ref_update **updates;
size_t alloc;
size_t nr;
- enum ref_transaction_state state;
+ enum transaction_state state;
};

-struct ref_transaction *ref_transaction_begin(struct strbuf *err)
+struct transaction *transaction_begin(struct strbuf *err)
{
assert(err);

- return xcalloc(1, sizeof(struct ref_transaction));
+ return xcalloc(1, sizeof(struct transaction));
}

-void ref_transaction_free(struct ref_transaction *transaction)
+void transaction_free(struct transaction *transaction)
{
int i;

@@ -3570,7 +3570,7 @@ void ref_transaction_free(struct ref_transaction *transaction)
free(transaction);
}

-static struct ref_update *add_update(struct ref_transaction *transaction,
+static struct ref_update *add_update(struct transaction *transaction,
const char *refname)
{
size_t len = strlen(refname);
@@ -3582,7 +3582,7 @@ static struct ref_update *add_update(struct ref_transaction *transaction,
return update;
}

-int ref_transaction_update(struct ref_transaction *transaction,
+int transaction_update_ref(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
@@ -3593,7 +3593,7 @@ int ref_transaction_update(struct ref_transaction *transaction,

assert(err);

- if (transaction->state != REF_TRANSACTION_OPEN)
+ if (transaction->state != TRANSACTION_OPEN)
die("BUG: update called for transaction that is not open");

if (have_old && !old_sha1)
@@ -3617,23 +3617,23 @@ int ref_transaction_update(struct ref_transaction *transaction,
return 0;
}

-int ref_transaction_create(struct ref_transaction *transaction,
+int transaction_create_ref(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
int flags, const char *msg,
struct strbuf *err)
{
- return ref_transaction_update(transaction, refname, new_sha1,
+ return transaction_update_ref(transaction, refname, new_sha1,
null_sha1, flags, 1, msg, err);
}

-int ref_transaction_delete(struct ref_transaction *transaction,
+int transaction_delete_ref(struct transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
struct strbuf *err)
{
- return ref_transaction_update(transaction, refname, null_sha1,
+ return transaction_update_ref(transaction, refname, null_sha1,
old_sha1, flags, have_old, msg, err);
}

@@ -3641,17 +3641,17 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
{
- struct ref_transaction *t;
+ struct transaction *t;
struct strbuf err = STRBUF_INIT;

- t = ref_transaction_begin(&err);
+ t = transaction_begin(&err);
if (!t ||
- ref_transaction_update(t, refname, sha1, oldval, flags,
+ transaction_update_ref(t, refname, sha1, oldval, flags,
!!oldval, action, &err) ||
- ref_transaction_commit(t, &err)) {
+ transaction_commit(t, &err)) {
const char *str = "update_ref failed for ref '%s': %s";

- ref_transaction_free(t);
+ transaction_free(t);
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR:
error(str, refname, err.buf);
@@ -3666,7 +3666,7 @@ int update_ref(const char *action, const char *refname,
return 1;
}
strbuf_release(&err);
- ref_transaction_free(t);
+ transaction_free(t);
return 0;
}

@@ -3694,8 +3694,8 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
return 0;
}

-int ref_transaction_commit(struct ref_transaction *transaction,
- struct strbuf *err)
+int transaction_commit(struct transaction *transaction,
+ struct strbuf *err)
{
int ret = 0, delnum = 0, i;
const char **delnames;
@@ -3704,11 +3704,11 @@ int ref_transaction_commit(struct ref_transaction *transaction,

assert(err);

- if (transaction->state != REF_TRANSACTION_OPEN)
+ if (transaction->state != TRANSACTION_OPEN)
die("BUG: commit called for transaction that is not open");

if (!n) {
- transaction->state = REF_TRANSACTION_CLOSED;
+ transaction->state = TRANSACTION_CLOSED;
return 0;
}

@@ -3787,7 +3787,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
clear_loose_ref_cache(&ref_cache);

cleanup:
- transaction->state = REF_TRANSACTION_CLOSED;
+ transaction->state = TRANSACTION_CLOSED;

for (i = 0; i < n; i++)
if (updates[i]->lock)
diff --git a/refs.h b/refs.h
index 7d675b7..556adfd 100644
--- a/refs.h
+++ b/refs.h
@@ -11,22 +11,22 @@ struct ref_lock {
};

/*
- * A ref_transaction represents a collection of ref updates
+ * A transaction represents a collection of ref updates
* that should succeed or fail together.
*
* Calling sequence
* ----------------
- * - Allocate and initialize a `struct ref_transaction` by calling
- * `ref_transaction_begin()`.
+ * - Allocate and initialize a `struct transaction` by calling
+ * `transaction_begin()`.
*
* - List intended ref updates by calling functions like
- * `ref_transaction_update()` and `ref_transaction_create()`.
+ * `transaction_update_ref()` and `transaction_create_ref()`.
*
- * - Call `ref_transaction_commit()` to execute the transaction.
+ * - Call `transaction_commit()` to execute the transaction.
* If this succeeds, the ref updates will have taken place and
* the transaction cannot be rolled back.
*
- * - At any time call `ref_transaction_free()` to discard the
+ * - At any time call `transaction_free()` to discard the
* transaction and free associated resources. In particular,
* this rolls back the transaction if it has not been
* successfully committed.
@@ -42,7 +42,7 @@ struct ref_lock {
* The message is appended to err without first clearing err.
* err will not be '\n' terminated.
*/
-struct ref_transaction;
+struct transaction;

/*
* Bit values set in the flags argument passed to each_ref_fn():
@@ -181,8 +181,8 @@ extern int is_branch(const char *refname);
extern int peel_ref(const char *refname, unsigned char *sha1);

/*
- * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),
- * ref_transaction_create(), etc.
+ * Flags controlling lock_any_ref_for_update(), transaction_update_ref(),
+ * transaction_create_ref(), etc.
* REF_NODEREF: act on the ref directly, instead of dereferencing
* symbolic references.
* REF_DELETING: tolerate broken refs
@@ -269,13 +269,13 @@ enum action_on_err {

/*
* Begin a reference transaction. The reference transaction must
- * be freed by calling ref_transaction_free().
+ * be freed by calling transaction_free().
*/
-struct ref_transaction *ref_transaction_begin(struct strbuf *err);
+struct transaction *transaction_begin(struct strbuf *err);

/*
* The following functions add a reference check or update to a
- * ref_transaction. In all of them, refname is the name of the
+ * transaction. In all of them, refname is the name of the
* reference to be affected. The functions make internal copies of
* refname and msg, so the caller retains ownership of these parameters.
* flags can be REF_NODEREF; it is passed to update_ref_lock().
@@ -291,7 +291,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
* means that the transaction as a whole has failed and will need to be
* rolled back.
*/
-int ref_transaction_update(struct ref_transaction *transaction,
+int transaction_update_ref(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
@@ -307,7 +307,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
* means that the transaction as a whole has failed and will need to be
* rolled back.
*/
-int ref_transaction_create(struct ref_transaction *transaction,
+int transaction_create_ref(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
int flags, const char *msg,
@@ -321,7 +321,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
* means that the transaction as a whole has failed and will need to be
* rolled back.
*/
-int ref_transaction_delete(struct ref_transaction *transaction,
+int transaction_delete_ref(struct transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
@@ -337,13 +337,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,
#define TRANSACTION_NAME_CONFLICT -1
/* All other errors. */
#define TRANSACTION_GENERIC_ERROR -2
-int ref_transaction_commit(struct ref_transaction *transaction,
- struct strbuf *err);
+int transaction_commit(struct transaction *transaction,
+ struct strbuf *err);

/*
* Free an existing transaction and all associated data.
*/
-void ref_transaction_free(struct ref_transaction *transaction);
+void transaction_free(struct transaction *transaction);

/** Lock a ref and then write its file */
int update_ref(const char *action, const char *refname,
diff --git a/sequencer.c b/sequencer.c
index a03d4fa..f888005 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -238,7 +238,7 @@ static int error_dirty_index(struct replay_opts *opts)
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
int unborn, struct replay_opts *opts)
{
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;

@@ -248,13 +248,13 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,

strbuf_addf(&sb, "%s: fast-forward", action_name(opts));

- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, "HEAD",
+ transaction_update_ref(transaction, "HEAD",
to, unborn ? null_sha1 : from,
0, 1, sb.buf, &err) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_free(transaction);
+ transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
error("%s", err.buf);
strbuf_release(&sb);
strbuf_release(&err);
@@ -263,7 +263,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,

strbuf_release(&sb);
strbuf_release(&err);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}

diff --git a/walker.c b/walker.c
index f149371..f1d5e9b 100644
--- a/walker.c
+++ b/walker.c
@@ -253,7 +253,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
{
struct strbuf refname = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction = NULL;
+ struct transaction *transaction = NULL;
unsigned char *sha1 = xmalloc(targets * 20);
char *msg = NULL;
int i, ret = -1;
@@ -261,7 +261,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
save_commit_buffer = 0;

if (write_ref) {
- transaction = ref_transaction_begin(&err);
+ transaction = transaction_begin(&err);
if (!transaction) {
error("%s", err.buf);
goto done;
@@ -298,7 +298,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
continue;
strbuf_reset(&refname);
strbuf_addf(&refname, "refs/%s", write_ref[i]);
- if (ref_transaction_update(transaction, refname.buf,
+ if (transaction_update_ref(transaction, refname.buf,
&sha1[20 * i], NULL, 0, 0,
msg ? msg : "fetch (unknown)",
&err)) {
@@ -306,7 +306,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
goto done;
}
}
- if (ref_transaction_commit(transaction, &err)) {
+ if (transaction_commit(transaction, &err)) {
error("%s", err.buf);
goto done;
}
@@ -314,7 +314,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
ret = 0;

done:
- ref_transaction_free(transaction);
+ transaction_free(transaction);
free(msg);
free(sha1);
strbuf_release(&err);

[Ronnie Sahlberg]

commit 1bfd3091a3d95a6268894182117eed823217dd9d upstream.

Add a field that describes what type of update this refers to. For now
the only type is UPDATE_SHA1 but we will soon add more types.

Change-Id: I9bf76454d1c789877a6aeb360cbb309971c9b5c4
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index a1bfaa2..8803b95 100644
--- a/refs.c
+++ b/refs.c
@@ -3504,6 +3504,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}

+enum transaction_update_type {
+ UPDATE_SHA1 = 0
+};
+
/**
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
@@ -3511,6 +3515,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
* value or to zero to ensure the ref does not exist before update.
*/
struct ref_update {
+ enum transaction_update_type update_type;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
@@ -3571,12 +3576,14 @@ void transaction_free(struct transaction *transaction)
}

static struct ref_update *add_update(struct transaction *transaction,
- const char *refname)
+ const char *refname,
+ enum transaction_update_type update_type)
{
size_t len = strlen(refname);
struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);

strcpy((char *)update->refname, refname);
+ update->update_type = update_type;
ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
transaction->updates[transaction->nr++] = update;
return update;
@@ -3606,7 +3613,7 @@ int transaction_update_ref(struct transaction *transaction,
return -1;
}

- update = add_update(transaction, refname);
+ update = add_update(transaction, refname, UPDATE_SHA1);
hashcpy(update->new_sha1, new_sha1);
update->flags = flags;
update->have_old = have_old;
@@ -3684,13 +3691,17 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,

assert(err);

- for (i = 1; i < n; i++)
+ for (i = 1; i < n; i++) {
+ if (updates[i - 1]->update_type != UPDATE_SHA1 ||
+ updates[i]->update_type != UPDATE_SHA1)
+ continue;
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
strbuf_addf(err,
"Multiple updates for ref '%s' not allowed.",
updates[i]->refname);
return 1;
}
+ }
return 0;
}

@@ -3722,13 +3733,17 @@ int transaction_commit(struct transaction *transaction,
goto cleanup;
}

- /* Acquire all locks while verifying old values */
+ /* Acquire all ref locks while verifying old values */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
int flags = update->flags;

+ if (update->update_type != UPDATE_SHA1)
+ continue;
+
if (is_null_sha1(update->new_sha1))
flags |= REF_DELETING;
+
update->lock = lock_ref_sha1_basic(update->refname,
(update->have_old ?
update->old_sha1 :
@@ -3750,6 +3765,8 @@ int transaction_commit(struct transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];

+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (!is_null_sha1(update->new_sha1)) {
if (write_ref_sha1(update->lock, update->new_sha1,
update->msg)) {
@@ -3767,6 +3784,8 @@ int transaction_commit(struct transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];

+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (update->lock) {
if (delete_ref_loose(update->lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;

[Junio C Hamano]

Post by Ronnie Sahlberg
commit 1bfd3091a3d95a6268894182117eed823217dd9d upstream.
Add a field that describes what type of update this refers to. For now
the only type is UPDATE_SHA1 but we will soon add more types.

OK, so the idea is that 03/15 and 07/15 that let callers to say
transaction_update_REF or transaction_update_REFLOG are nicety
wrappers and the underlying machinery will use UPDATE_SHA1 (added
here) and UPDATE_LOG (added in 07/15), which I find is a nice
layering.

Going in a good direction.

Post by Ronnie Sahlberg
Change-Id: I9bf76454d1c789877a6aeb360cbb309971c9b5c4
---
refs.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index a1bfaa2..8803b95 100644
--- a/refs.c
+++ b/refs.c
@@ -3504,6 +3504,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
+enum transaction_update_type {
+ UPDATE_SHA1 = 0
+};
+
/**
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
@@ -3511,6 +3515,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
* value or to zero to ensure the ref does not exist before update.
*/
struct ref_update {
+ enum transaction_update_type update_type;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
@@ -3571,12 +3576,14 @@ void transaction_free(struct transaction *transaction)
}
static struct ref_update *add_update(struct transaction *transaction,
- const char *refname)
+ const char *refname,
+ enum transaction_update_type update_type)
{
size_t len = strlen(refname);
struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
strcpy((char *)update->refname, refname);
+ update->update_type = update_type;
ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
transaction->updates[transaction->nr++] = update;
return update;
@@ -3606,7 +3613,7 @@ int transaction_update_ref(struct transaction *transaction,
return -1;
}
- update = add_update(transaction, refname);
+ update = add_update(transaction, refname, UPDATE_SHA1);
hashcpy(update->new_sha1, new_sha1);
update->flags = flags;
update->have_old = have_old;
@@ -3684,13 +3691,17 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
assert(err);
- for (i = 1; i < n; i++)
+ for (i = 1; i < n; i++) {
+ if (updates[i - 1]->update_type != UPDATE_SHA1 ||
+ updates[i]->update_type != UPDATE_SHA1)
+ continue;
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
strbuf_addf(err,
"Multiple updates for ref '%s' not allowed.",
updates[i]->refname);
return 1;
}
+ }
return 0;
}
@@ -3722,13 +3733,17 @@ int transaction_commit(struct transaction *transaction,
goto cleanup;
}
- /* Acquire all locks while verifying old values */
+ /* Acquire all ref locks while verifying old values */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
int flags = update->flags;
+ if (update->update_type != UPDATE_SHA1)
+ continue;
+
if (is_null_sha1(update->new_sha1))
flags |= REF_DELETING;
+
update->lock = lock_ref_sha1_basic(update->refname,
(update->have_old ?
@@ -3750,6 +3765,8 @@ int transaction_commit(struct transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (!is_null_sha1(update->new_sha1)) {
if (write_ref_sha1(update->lock, update->new_sha1,
update->msg)) {
@@ -3767,6 +3784,8 @@ int transaction_commit(struct transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (update->lock) {
if (delete_ref_loose(update->lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;

[Ronnie Sahlberg]

commit 0beeda259297c92d411ecc92fa508ec7cfd87cc5 upstream.

Change-Id: I685291986e544a8dc14f94c73b6a7c6400acd9d2
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 22 ++--------------------
refs.h | 2 +-
2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/refs.c b/refs.c
index ed0485e..c607ab7 100644
--- a/refs.c
+++ b/refs.c
@@ -3633,26 +3633,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,
int flags, int have_old, const char *msg,
struct strbuf *err)
{
- struct ref_update *update;
-
- assert(err);
-
- if (transaction->state != REF_TRANSACTION_OPEN)
- die("BUG: delete called for transaction that is not open");
-
- if (have_old && !old_sha1)
- die("BUG: have_old is true but old_sha1 is NULL");
-
- update = add_update(transaction, refname);
- update->flags = flags;
- update->have_old = have_old;
- if (have_old) {
- assert(!is_null_sha1(old_sha1));
- hashcpy(update->old_sha1, old_sha1);
- }
- if (msg)
- update->msg = xstrdup(msg);
- return 0;
+ return ref_transaction_update(transaction, refname, null_sha1,
+ old_sha1, flags, have_old, msg, err);
}

int update_ref(const char *action, const char *refname,
diff --git a/refs.h b/refs.h
index 2bc3556..7d675b7 100644
--- a/refs.h
+++ b/refs.h
@@ -283,7 +283,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);

/*
* Add a reference update to transaction. new_sha1 is the value that
- * the reference should have after the update, or zeros if it should
+ * the reference should have after the update, or null_sha1 if it should
* be deleted. If have_old is true, then old_sha1 holds the value
* that the reference should have had before the update, or zeros if
* it must not have existed beforehand.

[Junio C Hamano]

Post by Ronnie Sahlberg
commit 0beeda259297c92d411ecc92fa508ec7cfd87cc5 upstream.
Change-Id: I685291986e544a8dc14f94c73b6a7c6400acd9d2
---
refs.c | 22 ++--------------------
refs.h | 2 +-
2 files changed, 3 insertions(+), 21 deletions(-)

Nice code reduction.

Post by Ronnie Sahlberg
diff --git a/refs.c b/refs.c
index ed0485e..c607ab7 100644
--- a/refs.c
+++ b/refs.c
@@ -3633,26 +3633,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,
int flags, int have_old, const char *msg,
struct strbuf *err)
{
- struct ref_update *update;
-
- assert(err);
-
- if (transaction->state != REF_TRANSACTION_OPEN)
- die("BUG: delete called for transaction that is not open");
-
- if (have_old && !old_sha1)
- die("BUG: have_old is true but old_sha1 is NULL");
-
- update = add_update(transaction, refname);
- update->flags = flags;
- update->have_old = have_old;
- if (have_old) {
- assert(!is_null_sha1(old_sha1));
- hashcpy(update->old_sha1, old_sha1);
- }
- if (msg)
- update->msg = xstrdup(msg);
- return 0;
+ return ref_transaction_update(transaction, refname, null_sha1,
+ old_sha1, flags, have_old, msg, err);
}
int update_ref(const char *action, const char *refname,
diff --git a/refs.h b/refs.h
index 2bc3556..7d675b7 100644
--- a/refs.h
+++ b/refs.h
@@ -283,7 +283,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
/*
* Add a reference update to transaction. new_sha1 is the value that
- * the reference should have after the update, or zeros if it should
+ * the reference should have after the update, or null_sha1 if it should
* be deleted. If have_old is true, then old_sha1 holds the value
* that the reference should have had before the update, or zeros if
* it must not have existed beforehand.

[Ronnie Sahlberg]

commit 5fd1fa7d8520cf03e94fa7d0e9aa8685de0ef63f upstream.

Allow to make multiple reflog updates to the same ref during a transaction.
This means we only need to lock the reflog once, during the first update
that touches the reflog, and that all further updates can just write the
reflog entry since the reflog is already locked.

This allows us to write code such as:

t = transaction_begin()
transaction_reflog_update(t, "foo", REFLOG_TRUNCATE, NULL);
loop-over-something...
transaction_reflog_update(t, "foo", 0, <message>);
transaction_commit(t)

where we first truncate the reflog and then build the new content one line
at a time.

While this technically looks like O(n2) behavior it not that bad.
We only do this loop for transactions that cover a single ref during
reflog expire. This means that the linear search inside
transaction_update_reflog() will find the match on the very first entry
thus making it O(1) and not O(n) or our usecases. Thus the whole expire
becomes O(n) instead of O(n2). If in the future we start doing this for many
refs in one single transaction we might want to optimize this.
But there is no need to complexify the code and optimize for future usecases
that might never materialize at this stage.

Change-Id: Ibe703310e33a4db07e5bcc704310211268b788fc
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/refs.c b/refs.c
index f14b76e..f7e947f 100644
--- a/refs.c
+++ b/refs.c
@@ -31,6 +31,12 @@ static unsigned char refname_disposition[256] = {
*/
#define REF_ISPRUNING 0x0100
/*
+ * Only the first reflog update needs to lock the reflog file. Further updates
+ * just use the lock taken by the first update.
+ */
+#define UPDATE_REFLOG_NOLOCK 0x0200
+
+/*
* Try to read one refname component from the front of refname.
* Return the length of the component found, or -1 if the component is
* not legal. It is legal if it is something reasonable to have under
@@ -3521,7 +3527,7 @@ enum transaction_update_type {
UPDATE_LOG = 1
};

-/**
+/*
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
* while locking the ref, set have_old to 1 and set old_sha1 to the
@@ -3531,7 +3537,7 @@ struct ref_update {
enum transaction_update_type update_type;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
- int flags; /* REF_NODEREF? */
+ int flags; /* REF_NODEREF? or private flags */
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
struct ref_lock *lock;
int type;
@@ -3539,8 +3545,9 @@ struct ref_update {

/* used by reflog updates */
int reflog_fd;
- struct lock_file reflog_lock;
+ struct lock_file *reflog_lock;
char *committer;
+ struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */

const char refname[FLEX_ARRAY];
};
@@ -3619,11 +3626,26 @@ int transaction_update_reflog(struct transaction *transaction,
struct strbuf *err)
{
struct ref_update *update;
+ int i;

if (transaction->state != TRANSACTION_OPEN)
die("BUG: update_reflog called for transaction that is not open");

update = add_update(transaction, refname, UPDATE_LOG);
+ update->flags = flags;
+ for (i = 0; i < transaction->nr - 1; i++) {
+ if (transaction->updates[i]->update_type != UPDATE_LOG)
+ continue;
+ if (!strcmp(transaction->updates[i]->refname,
+ update->refname)) {
+ update->flags |= UPDATE_REFLOG_NOLOCK;
+ update->orig_update = transaction->updates[i];
+ break;
+ }
+ }
+ if (!(update->flags & UPDATE_REFLOG_NOLOCK))
+ update->reflog_lock = xcalloc(1, sizeof(struct lock_file));
+
hashcpy(update->new_sha1, new_sha1);
hashcpy(update->old_sha1, old_sha1);
update->reflog_fd = -1;
@@ -3639,7 +3661,6 @@ int transaction_update_reflog(struct transaction *transaction,
}
if (msg)
update->msg = xstrdup(msg);
- update->flags = flags;

return 0;
}
@@ -3822,10 +3843,15 @@ int transaction_commit(struct transaction *transaction,

if (update->update_type != UPDATE_LOG)
continue;
+ if (update->flags & UPDATE_REFLOG_NOLOCK) {
+ update->reflog_fd = update->orig_update->reflog_fd;
+ update->reflog_lock = update->orig_update->reflog_lock;
+ continue;
+ }
update->reflog_fd = hold_lock_file_for_append(
- &update->reflog_lock,
+ update->reflog_lock,
git_path("logs/%s", update->refname),
- 0);
+ LOCK_NO_DEREF);
if (update->reflog_fd < 0) {
const char *str = "Cannot lock reflog for '%s'. %s";

@@ -3891,7 +3917,7 @@ int transaction_commit(struct transaction *transaction,
ftruncate(update->reflog_fd, 0)) {
error("Could not truncate reflog: %s. %s",
update->refname, strerror(errno));
- rollback_lock_file(&update->reflog_lock);
+ rollback_lock_file(update->reflog_lock);
update->reflog_fd = -1;
continue;
}
@@ -3901,7 +3927,7 @@ int transaction_commit(struct transaction *transaction,
update->committer, update->msg)) {
error("Could write to reflog: %s. %s",
update->refname, strerror(errno));
- rollback_lock_file(&update->reflog_lock);
+ rollback_lock_file(update->reflog_lock);
update->reflog_fd = -1;
}
}
@@ -3912,9 +3938,11 @@ int transaction_commit(struct transaction *transaction,

if (update->update_type != UPDATE_LOG)
continue;
+ if (update->flags & UPDATE_REFLOG_NOLOCK)
+ continue;
if (update->reflog_fd == -1)
continue;
- if (commit_lock_file(&update->reflog_lock)) {
+ if (commit_lock_file(update->reflog_lock)) {
error("Could not commit reflog: %s. %s",
update->refname, strerror(errno));
update->reflog_fd = -1;

[Junio C Hamano]

Post by Ronnie Sahlberg
@@ -3531,7 +3537,7 @@ struct ref_update {
enum transaction_update_type update_type;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
- int flags; /* REF_NODEREF? */
+ int flags; /* REF_NODEREF? or private flags */

Not a very informative comment, I'd have to say. How are users of
this API expected to avoid stepping on each others' and API
implementation's toes?

Post by Ronnie Sahlberg
@@ -3539,8 +3545,9 @@ struct ref_update {
/* used by reflog updates */
int reflog_fd;
- struct lock_file reflog_lock;
+ struct lock_file *reflog_lock;

What is this change about?

Does the lifetime rule for "struct lock_file" described in
Documentation/technical/api-lockfile.txt, namely, "once you call
hold_lock_file_* family on it, you cannot free it yourself", have
any implication on this?

Post by Ronnie Sahlberg
+ if (!(update->flags & UPDATE_REFLOG_NOLOCK))
+ update->reflog_lock = xcalloc(1, sizeof(struct lock_file));
+

Hmph, does this mean that the caller needs to keep track of the refs
it ever touched inside a single transaction, call this without nolock
on the first invocation on a particular ref and with nolock on the
subsequent invocation?

Or is the "caller" just implementation detail of the API and higher level
callers do not have to care?

[Ronnie Sahlberg]

commit 07baa71eda8d1047f003acc7ea32ad6b6600b179 upstream.

Break out the code to create the string and writing it to the file
descriptor from log_ref_write and into a dedicated function log_ref_write_fd.
For now this is only used from log_ref_write but later on we will call
this function from reflog transactions too which means that we will end
up with only a single place where we write a reflog entry to a file instead
of the current two places (log_ref_write and builtin/reflog.c).

Change-Id: I178e84a52af18c800844596af33dbc21bc076c1f
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 48 ++++++++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/refs.c b/refs.c
index 8803b95..0e11b1c 100644
--- a/refs.c
+++ b/refs.c
@@ -2990,15 +2990,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)
return 0;
}

+static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const char *committer, const char *msg)
+{
+ int msglen, written;
+ unsigned maxlen, len;
+ char *logrec;
+
+ msglen = msg ? strlen(msg) : 0;
+ maxlen = strlen(committer) + msglen + 100;
+ logrec = xmalloc(maxlen);
+ len = sprintf(logrec, "%s %s %s\n",
+ sha1_to_hex(old_sha1),
+ sha1_to_hex(new_sha1),
+ committer);
+ if (msglen)
+ len += copy_msg(logrec + len - 1, msg) - 1;
+
+ written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
+ free(logrec);
+ if (written != len)
+ return -1;
+
+ return 0;
+}
+
static int log_ref_write(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg)
{
- int logfd, result, written, oflags = O_APPEND | O_WRONLY;
- unsigned maxlen, len;
- int msglen;
+ int logfd, result, oflags = O_APPEND | O_WRONLY;
char log_file[PATH_MAX];
- char *logrec;
- const char *committer;

if (log_all_ref_updates < 0)
log_all_ref_updates = !is_bare_repository();
@@ -3010,19 +3032,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
logfd = open(log_file, oflags);
if (logfd < 0)
return 0;
- msglen = msg ? strlen(msg) : 0;
- committer = git_committer_info(0);
- maxlen = strlen(committer) + msglen + 100;
- logrec = xmalloc(maxlen);
- len = sprintf(logrec, "%s %s %s\n",
- sha1_to_hex(old_sha1),
- sha1_to_hex(new_sha1),
- committer);
- if (msglen)
- len += copy_msg(logrec + len - 1, msg) - 1;
- written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
- free(logrec);
- if (written != len) {
+ result = log_ref_write_fd(logfd, old_sha1, new_sha1,
+ git_committer_info(0), msg);
+ if (result) {
int save_errno = errno;
close(logfd);
error("Unable to append to %s", log_file);

[Ronnie Sahlberg]

commit 020ed65a12838bdead64bc3c5de249d3c8f5cfd8 upstream.

When performing a reflog transaction update, only write to the reflog iff
msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform
an update that only truncates but does not write.

Change-Id: I44c89caa7e7c4960777b79cfb5d339a5aa3ddf7a
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 5 +++--
refs.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index d54c3b9..f14b76e 100644
--- a/refs.c
+++ b/refs.c
@@ -3895,8 +3895,9 @@ int transaction_commit(struct transaction *transaction,
update->reflog_fd = -1;
continue;
}
- if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
- update->new_sha1,
+ if (update->msg &&
+ log_ref_write_fd(update->reflog_fd,
+ update->old_sha1, update->new_sha1,
update->committer, update->msg)) {
error("Could write to reflog: %s. %s",
update->refname, strerror(errno));
diff --git a/refs.h b/refs.h
index 5075073..bf96b36 100644
--- a/refs.h
+++ b/refs.h
@@ -337,6 +337,7 @@ int transaction_delete_ref(struct transaction *transaction,
/*
* Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
* this update will first truncate the reflog before writing the entry.
+ * If msg is NULL no update will be written to the log.
*/
int transaction_update_reflog(struct transaction *transaction,
const char *refname,

[Junio C Hamano]

Post by Ronnie Sahlberg
commit 020ed65a12838bdead64bc3c5de249d3c8f5cfd8 upstream.
When performing a reflog transaction update, only write to the reflog iff
msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform
an update that only truncates but does not write.

Does any existing caller call this codepath with update->msg == NULL?

Will "please truncate" stay to be the only plausible special cause
to call into this codepath without having any meaningful message?

I am trying to make sure that this patch is not painting us into a
corner where we will find out another reason for doing something
esoteric in this codepath but need to find a way other than setting
msg to NULL for the caller to trigger that new codepath. Put it in
another way, please convince me that a new boolean field in update,
e.g. update->truncate_reflog, is way overkill for this.

Post by Ronnie Sahlberg
Change-Id: I44c89caa7e7c4960777b79cfb5d339a5aa3ddf7a
---
refs.c | 5 +++--
refs.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index d54c3b9..f14b76e 100644
--- a/refs.c
+++ b/refs.c
@@ -3895,8 +3895,9 @@ int transaction_commit(struct transaction *transaction,
update->reflog_fd = -1;
continue;
}
- if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
- update->new_sha1,
+ if (update->msg &&
+ log_ref_write_fd(update->reflog_fd,
+ update->old_sha1, update->new_sha1,
update->committer, update->msg)) {
error("Could write to reflog: %s. %s",
update->refname, strerror(errno));
diff --git a/refs.h b/refs.h
index 5075073..bf96b36 100644
--- a/refs.h
+++ b/refs.h
@@ -337,6 +337,7 @@ int transaction_delete_ref(struct transaction *transaction,
/*
* Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
* this update will first truncate the reflog before writing the entry.
+ * If msg is NULL no update will be written to the log.
*/
int transaction_update_reflog(struct transaction *transaction,
const char *refname,

[Ronnie Sahlberg]

commit 8d75baa250c0f84cc438568cb53ef1a9dd3dfec9 upstream.

Add back support to make it possible to delete refs that have a broken
sha1.

Add new internal flags REF_ALLOW_BROKEN and RESOLVE_REF_ALLOW_BAD_SHA1
to pass intent from branch.c that we are willing to allow
resolve_ref_unsafe and lock_ref_sha1_basic to allow broken refs.
Since these refs can not actually be resolved to a sha1, they instead resolve
to null_sha1 when these flags are used.

For example, the ref:

echo "Broken ref" > .git/refs/heads/foo-broken-1

can now be deleted using git branch -d foo-broken-1

Change-Id: I4e744d9e7d8b7e81dde5479965819117d03c98db
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
builtin/branch.c | 5 +++--
cache.h | 7 +++++++
refs.c | 6 ++++++
refs.h | 6 ++++--
t/t1402-check-ref-format.sh | 8 ++++++++
5 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 3b79c50..04f57d4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
target = resolve_ref_unsafe(name,
RESOLVE_REF_READING
| RESOLVE_REF_NO_RECURSE
- | RESOLVE_REF_ALLOW_BAD_NAME,
+ | RESOLVE_REF_ALLOW_BAD_NAME
+ | RESOLVE_REF_ALLOW_BAD_SHA1,
sha1, &flags);
if (!target) {
error(remote_branch
@@ -255,7 +256,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
continue;
}

- if (delete_ref(name, sha1, REF_NODEREF)) {
+ if (delete_ref(name, sha1, REF_NODEREF|REF_ALLOW_BROKEN)) {
error(remote_branch
? _("Error deleting remote branch '%s'")
: _("Error deleting branch '%s'"),
diff --git a/cache.h b/cache.h
index 99ed096..61e61af 100644
--- a/cache.h
+++ b/cache.h
@@ -1000,10 +1000,17 @@ extern int read_ref(const char *refname, unsigned char *sha1);
* resolved. The function returns NULL for such ref names.
* Caps and underscores refers to the special refs, such as HEAD,
* FETCH_HEAD and friends, that all live outside of the refs/ directory.
+ *
+ * RESOLVE_REF_ALLOW_BAD_SHA1 when this flag is set and the ref contains
+ * an invalid sha1, resolve_ref_unsafe will clear the sha1 argument,
+ * set the REF_ISBROKEN flag and return the refname.
+ * This allows using resolve_ref_unsafe to check for existence of such
+ * broken refs.
*/
#define RESOLVE_REF_READING 0x01
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+#define RESOLVE_REF_ALLOW_BAD_SHA1 0x08
extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);

diff --git a/refs.c b/refs.c
index 5bd6d62..ae29d11 100644
--- a/refs.c
+++ b/refs.c
@@ -1584,6 +1584,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
(buffer[40] != '\0' && !isspace(buffer[40]))) {
if (flags)
*flags |= REF_ISBROKEN;
+ if (resolve_flags & RESOLVE_REF_ALLOW_BAD_SHA1) {
+ hashclr(sha1);
+ return refname;
+ }
errno = EINVAL;
return NULL;
}
@@ -2265,6 +2269,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
if (flags & REF_NODEREF)
resolve_flags |= RESOLVE_REF_NO_RECURSE;
}
+ if (flags & REF_ALLOW_BROKEN)
+ resolve_flags |= RESOLVE_REF_ALLOW_BAD_SHA1;

refname = resolve_ref_unsafe(refname, resolve_flags,
lock->old_sha1, &type);
diff --git a/refs.h b/refs.h
index 721e21f..2e97f4f 100644
--- a/refs.h
+++ b/refs.h
@@ -185,11 +185,13 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
* REF_NODEREF: act on the ref directly, instead of dereferencing
* symbolic references.
* REF_DELETING: tolerate broken refs
+ * REF_ALLOW_BROKEN: allow locking refs that are broken.
*
* Flags >= 0x100 are reserved for internal use.
*/
-#define REF_NODEREF 0x01
-#define REF_DELETING 0x02
+#define REF_NODEREF 0x01
+#define REF_DELETING 0x02
+#define REF_ALLOW_BROKEN 0x04

/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned int flags,
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index e5dc62e..a0aef69 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -197,4 +197,12 @@ invalid_ref_normalized 'heads///foo.lock'
invalid_ref_normalized 'foo.lock/bar'
invalid_ref_normalized 'foo.lock///bar'

+test_expect_success 'git branch -d can delete ref with broken sha1' '
+ echo "012brokensha1" > .git/refs/heads/brokensha1 &&
+ test_when_finished "rm -f .git/refs/heads/brokensha1" &&
+ git branch -d brokensha1 &&
+ git branch >output &&
+ ! grep -e "brokensha1" output
+'
+
test_done

[Ronnie Sahlberg]

commit 3a2f55c247ff290943fd552674e226062c13fd00 upstream.

No one is using this function so we can delete it.

Change-Id: I524dac239da500e5d4432328b997ed8ab34f6803
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
refs.c | 7 -------
refs.h | 9 +--------
2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/refs.c b/refs.c
index 5e5f066..5bd6d62 100644
--- a/refs.c
+++ b/refs.c
@@ -2352,13 +2352,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
return NULL;
}

-struct ref_lock *lock_any_ref_for_update(const char *refname,
- const unsigned char *old_sha1,
- int flags, int *type_p)
-{
- return lock_ref_sha1_basic(refname, old_sha1, NULL, flags, type_p);
-}
-
/*
* Write an entry to the packed-refs file for the specified refname.
* If peeled is non-NULL, write it as the entry's peeled value.
diff --git a/refs.h b/refs.h
index 025e2cb..721e21f 100644
--- a/refs.h
+++ b/refs.h
@@ -181,8 +181,7 @@ extern int is_branch(const char *refname);
extern int peel_ref(const char *refname, unsigned char *sha1);

/*
- * Flags controlling lock_any_ref_for_update(), transaction_update_ref(),
- * transaction_create_ref(), etc.
+ * Flags controlling transaction_update_ref(), transaction_create_ref(), etc.
* REF_NODEREF: act on the ref directly, instead of dereferencing
* symbolic references.
* REF_DELETING: tolerate broken refs
@@ -191,12 +190,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
*/
#define REF_NODEREF 0x01
#define REF_DELETING 0x02
-/*
- * This function sets errno to something meaningful on failure.
- */
-extern struct ref_lock *lock_any_ref_for_update(const char *refname,
- const unsigned char *old_sha1,
- int flags, int *type_p);

/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned int flags,

[Ronnie Sahlberg]

commit 306805ccd147bfdf160b288a8d51fdf9b77ae0fa upstream.

Update copy_fd to return a meaningful errno on failure.

Change-Id: I771f9703acc816902affcf35ff2a56d9426315ac
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
copy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/copy.c b/copy.c
index f2970ec..a8d366e 100644
--- a/copy.c
+++ b/copy.c
@@ -8,12 +8,17 @@ int copy_fd(int ifd, int ofd)
if (!len)
break;
if (len < 0) {
- return error("copy-fd: read returned %s",
- strerror(errno));
+ int save_errno = errno;
+ error("copy-fd: read returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
+ }
+ if (write_in_full(ofd, buffer, len) < 0) {
+ int save_errno = errno;
+ error("copy-fd: write returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
}
- if (write_in_full(ofd, buffer, len) < 0)
- return error("copy-fd: write returned %s",
- strerror(errno));
}
return 0;
}

[Junio C Hamano]

Post by Ronnie Sahlberg
commit 306805ccd147bfdf160b288a8d51fdf9b77ae0fa upstream.
Update copy_fd to return a meaningful errno on failure.

These two are good changes, but makes me wonder if more places
benefit from having error_errno() that does the "save away errno,
use strerror(errno) to give an error message and restore errno"
for us.

Not meant as a suggestion for further changes in this series, but as
a future discussion item.

Post by Ronnie Sahlberg
Change-Id: I771f9703acc816902affcf35ff2a56d9426315ac
---
copy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/copy.c b/copy.c
index f2970ec..a8d366e 100644
--- a/copy.c
+++ b/copy.c
@@ -8,12 +8,17 @@ int copy_fd(int ifd, int ofd)
if (!len)
break;
if (len < 0) {
- return error("copy-fd: read returned %s",
- strerror(errno));
+ int save_errno = errno;
+ error("copy-fd: read returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
+ }
+ if (write_in_full(ofd, buffer, len) < 0) {
+ int save_errno = errno;
+ error("copy-fd: write returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
}
- if (write_in_full(ofd, buffer, len) < 0)
- return error("copy-fd: write returned %s",
- strerror(errno));
}
return 0;
}

[Ronnie Sahlberg]

Post by Junio C Hamano

Post by Ronnie Sahlberg
commit 306805ccd147bfdf160b288a8d51fdf9b77ae0fa upstream.
Update copy_fd to return a meaningful errno on failure.

These two are good changes, but makes me wonder if more places
benefit from having error_errno() that does the "save away errno,
use strerror(errno) to give an error message and restore errno"
for us.
Not meant as a suggestion for further changes in this series, but as
a future discussion item.

That sounds like a good idea.

As a separate series once these are done, I can volunteer to go
through all the errno handling in git
and look into that.

Post by Junio C Hamano

Post by Ronnie Sahlberg
Change-Id: I771f9703acc816902affcf35ff2a56d9426315ac
---
copy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/copy.c b/copy.c
index f2970ec..a8d366e 100644
--- a/copy.c
+++ b/copy.c
@@ -8,12 +8,17 @@ int copy_fd(int ifd, int ofd)
if (!len)
break;
if (len < 0) {
- return error("copy-fd: read returned %s",
- strerror(errno));
+ int save_errno = errno;
+ error("copy-fd: read returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
+ }
+ if (write_in_full(ofd, buffer, len) < 0) {
+ int save_errno = errno;
+ error("copy-fd: write returned %s", strerror(errno));
+ errno = save_errno;
+ return -1;
}
- if (write_in_full(ofd, buffer, len) < 0)
- return error("copy-fd: write returned %s",
- strerror(errno));
}
return 0;
}

[Ronnie Sahlberg]

commit 5ac378cd8fad09a836d17fec379780854838bde5 upstream.

Use a transaction for all updates during expire_reflog.

Change-Id: Ieb81b2660cefeeecf0bcb3cdbc1ef3cbb86e7eb8
Signed-off-by: Ronnie Sahlberg <***@google.com>
Signed-off-by: Jonathan Nieder <***@gmail.com>
---
builtin/reflog.c | 85 ++++++++++++++++++++++++--------------------------------
refs.c | 4 +--
refs.h | 2 +-
3 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 2d85d26..6bb7454 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb {
int recno;
};

+static struct strbuf err = STRBUF_INIT;
+
struct expire_reflog_cb {
- FILE *newlog;
+ struct transaction *t;
+ const char *refname;
enum {
UE_NORMAL,
UE_ALWAYS,
@@ -316,20 +319,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
if (cb->cmd->recno && --(cb->cmd->recno) == 0)
goto prune;

- if (cb->newlog) {
- char sign = (tz < 0) ? '-' : '+';
- int zone = (tz < 0) ? (-tz) : tz;
- fprintf(cb->newlog, "%s %s %s %lu %c%04d\t%s",
- sha1_to_hex(osha1), sha1_to_hex(nsha1),
- email, timestamp, sign, zone,
- message);
+ if (cb->t) {
+ if (transaction_update_reflog(cb->t, cb->refname, nsha1, osha1,
+ email, timestamp, tz, message, 0,
+ &err))
+ return -1;
hashcpy(cb->last_kept_sha1, nsha1);
}
if (cb->cmd->verbose)
printf("keep %s", message);
return 0;
prune:
- if (!cb->newlog)
+ if (!cb->t)
printf("would prune %s", message);
else if (cb->cmd->verbose)
printf("prune %s", message);
@@ -353,29 +354,26 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
{
struct cmd_reflog_expire_cb *cmd = cb_data;
struct expire_reflog_cb cb;
- struct ref_lock *lock;
- char *log_file, *newlog_path = NULL;
struct commit *tip_commit;
struct commit_list *tips;
int status = 0;

memset(&cb, 0, sizeof(cb));
+ cb.refname = ref;

- /*
- * we take the lock for the ref itself to prevent it from
- * getting updated.
- */
- lock = lock_any_ref_for_update(ref, sha1, 0, NULL);
- if (!lock)
- return error("cannot lock ref '%s'", ref);
- log_file = git_pathdup("logs/%s", ref);
if (!reflog_exists(ref))
goto finish;
- if (!cmd->dry_run) {
- newlog_path = git_pathdup("logs/%s.lock", ref);
- cb.newlog = fopen(newlog_path, "w");
+ cb.t = transaction_begin(&err);
+ if (!cb.t) {
+ status |= error("%s", err.buf);
+ goto cleanup;
+ }
+ if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1,
+ NULL, 0, 0, NULL, REFLOG_TRUNCATE,
+ &err)) {
+ status |= error("%s", err.buf);
+ goto cleanup;
}
-
cb.cmd = cmd;

if (!cmd->expire_unreachable || !strcmp(ref, "HEAD")) {
@@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
mark_reachable(&cb);
}

- for_each_reflog_ent(ref, expire_reflog_ent, &cb);
+ if (for_each_reflog_ent(ref, expire_reflog_ent, &cb)) {
+ status |= error("%s", err.buf);
+ goto cleanup;
+ }

if (cb.unreachable_expire_kind != UE_ALWAYS) {
if (cb.unreachable_expire_kind == UE_HEAD) {
@@ -420,32 +421,20 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
}
}
finish:
- if (cb.newlog) {
- if (fclose(cb.newlog)) {
- status |= error("%s: %s", strerror(errno),
- newlog_path);
- unlink(newlog_path);
- } else if (cmd->updateref &&
- (write_in_full(lock->lock_fd,
- sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
- write_str_in_full(lock->lock_fd, "\n") != 1 ||
- close_ref(lock) < 0)) {
- status |= error("Couldn't write %s",
- lock->lk->filename.buf);
- unlink(newlog_path);
- } else if (rename(newlog_path, log_file)) {
- status |= error("cannot rename %s to %s",
- newlog_path, log_file);
- unlink(newlog_path);
- } else if (cmd->updateref && commit_ref(lock)) {
- status |= error("Couldn't set %s", lock->ref_name);
- } else {
- adjust_shared_perm(log_file);
+ if (!cmd->dry_run) {
+ if (cmd->updateref &&
+ transaction_update_ref(cb.t, cb.refname,
+ cb.last_kept_sha1, sha1,
+ 0, 1, NULL, &err)) {
+ status |= error("%s", err.buf);
+ goto cleanup;
}
+ if (transaction_commit(cb.t, &err))
+ status |= error("%s", err.buf);
}
- free(newlog_path);
- free(log_file);
- unlock_ref(lock);
+ cleanup:
+ transaction_free(cb.t);
+ strbuf_release(&err);
return status;
}

diff --git a/refs.c b/refs.c
index f7e947f..3189f11 100644
--- a/refs.c
+++ b/refs.c
@@ -3620,7 +3620,7 @@ int transaction_update_reflog(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
- const unsigned char *email,
+ const char *email,
unsigned long timestamp, int tz,
const char *msg, int flags,
struct strbuf *err)
@@ -3901,7 +3901,7 @@ int transaction_commit(struct transaction *transaction,

/*
* Update all reflog files
- * We have already done all ref updates and deletes.
+ * We have already committed all ref updates and deletes.
* There is not much we can do here if there are any reflog
* update errors other than complain.
*/
diff --git a/refs.h b/refs.h
index bf96b36..9f70b89 100644
--- a/refs.h
+++ b/refs.h
@@ -343,7 +343,7 @@ int transaction_update_reflog(struct transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
- const unsigned char *email,
+ const char *email,
unsigned long timestamp, int tz,
const char *msg, int flags,
struct strbuf *err);