* [PATCH 00/31] Finish implementing ref and reflog transactions
@ 2014-05-14 22:12 Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update Ronnie Sahlberg
` (30 more replies)
0 siblings, 31 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:12 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
This patch series is available also from
https://github.com/rsahlberg/git/tree/ref-transactions-next
and is based on
https://github.com/rsahlberg/git/tree/ref-transactions
This is a preview of the direction for the transaction work and finished
converting also reflog handling to transactions. This greatly simplifies
both the implementation of rename_ref in refs.c as well as reflog.c
This patch series is not fully finished and can not be applied until
the previous series is merged. Once this series is finished we will be
in pretty good shape to start experimenting with alternative refs backends
such as a TDB database.
Ronnie Sahlberg (31):
refs.c make ref_transaction_create a wrapper to ref_transaction_update
refs.c: make ref_transaction_delete a wrapper for
ref_transaction_update
refs.c: rename the transaction functions
refs.c: add a new update_type field to ref_update
refs.c: add a function to append a reflog entry to a fd
refs.c: add a transaction function to append a reflog entry
refs.c: add a flag to allow reflog updates to truncate the log
refs.c: only write reflog update if msg is non-NULL
refs.c: allow multiple reflog updates during a single transaction
reflog.c: use a reflog transaction when writing during expire
refs.c: null terminate the string in copy_msg
refs.c: track the refnames we are deleting in the transaction
structure
refs.c: update the list of deleted refs during _update instead of
_commit
refs.c: return error instead of dying when locking fails during
transaction
refs.c: lock the ref during _update instead of during _commit
refs.c: add an error argument to create/delete/update just like commit
refs.c: make _update_reflog take an error argument
refs.c: return immediately from _commit if the transaction has an
error
tests: move tests for -z update/delete/verify to after the ref is
created
refs.c: check for lock conflicts already in _update
refs.c allow multiple updates of the same ref in a transaction
refs.c: release all remaining locks during transaction_free
reflog.c: use the existing transaction to also lock and update the ref
refs.c: make unlock_ref static
refs.c: make close_ref static
refs.c: make commit_ref static
refs.c: remove the function lock_any_ref_for_update
refs.c: make struct ref_lock private to refs.c
refs.c: allow passing raw git_committer_info as email to
_update_reflog
refs.c: move ref_update and other definitions to earlier in the file
refs.c: use the transaction to manage the reflog in rename_refs
branch.c | 11 +-
builtin/commit.c | 14 +-
builtin/fetch.c | 10 +-
builtin/receive-pack.c | 11 +-
builtin/reflog.c | 76 +++---
builtin/replace.c | 10 +-
builtin/tag.c | 10 +-
builtin/update-ref.c | 34 +--
fast-import.c | 26 ++-
refs.c | 611 +++++++++++++++++++++++++++++++++----------------
refs.h | 86 +++----
sequencer.c | 12 +-
t/t1400-update-ref.sh | 59 +++--
walker.c | 15 +-
14 files changed, 610 insertions(+), 375 deletions(-)
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 02/31] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update Ronnie Sahlberg
` (29 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 13 +++----------
refs.h | 7 ++++---
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index aed700b..fe195dd 100644
--- a/refs.c
+++ b/refs.c
@@ -3354,17 +3354,10 @@ int ref_transaction_create(struct ref_transaction *transaction,
die("BUG: create on transaction that is not open");
if (flags & REF_ISPACKONLY)
- die("BUG: REF_ISPACKONLY can not be used with creates");
+ die("BUG: REF_ISPACKONLY can not be used with create");
- 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);
}
int ref_transaction_delete(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index 2d2362c..d615871 100644
--- a/refs.h
+++ b/refs.h
@@ -233,9 +233,10 @@ void ref_transaction_rollback(struct ref_transaction *transaction);
/*
* Add a reference update to transaction. new_sha1 is the value that
* the reference should have after the update, or zeros 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.
+ * be deleted. If have_old is true and old_sha is not the null_sha1
+ * then the previous value of the ref must match or the update will fail.
+ * If have_old is true and old_sha1 is the null_sha1 then the ref must not
+ * already exist and a new ref will be created with new_sha1.
* Function returns 0 on success and non-zero on failure.
*/
int ref_transaction_update(struct ref_transaction *transaction,
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 02/31] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 03/31] refs.c: rename the transaction functions Ronnie Sahlberg
` (28 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We want to allow to delete a ref even if it only exists as a packed ref
so we must tweak the REF_ISPACKONLY test in _update so it only applies
to the non-delete case.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 17 ++++++-----------
refs.h | 2 +-
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index fe195dd..c180fa2 100644
--- a/refs.c
+++ b/refs.c
@@ -3326,7 +3326,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
if (transaction->status != REF_TRANSACTION_OPEN)
die("BUG: update on transaction that is not open");
- if (flags & REF_ISPACKONLY)
+ if (flags & REF_ISPACKONLY && !is_null_sha1(new_sha1))
die("BUG: REF_ISPACKONLY can not be used with updates");
update = add_update(transaction, refname);
@@ -3370,19 +3370,14 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (have_old && !old_sha1)
die("BUG: have_old is true but old_sha1 is NULL");
+ if (have_old && is_null_sha1(old_sha1))
+ die("BUG: have_old is true but old_sha1 is null_sha1");
+
if (transaction->status != REF_TRANSACTION_OPEN)
die("BUG: delete on transaction that is not open");
- 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);
}
int update_ref(const char *action, const char *refname,
diff --git a/refs.h b/refs.h
index d615871..8d56edf 100644
--- a/refs.h
+++ b/refs.h
@@ -232,7 +232,7 @@ void ref_transaction_rollback(struct ref_transaction *transaction);
/*
* 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 and old_sha is not the null_sha1
* then the previous value of the ref must match or the update will fail.
* If have_old is true and old_sha1 is the null_sha1 then the ref must not
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 03/31] refs.c: rename the transaction functions
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 02/31] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-16 21:15 ` Junio C Hamano
2014-05-14 22:13 ` [PATCH 04/31] refs.c: add a new update_type field to ref_update Ronnie Sahlberg
` (27 subsequent siblings)
30 siblings, 1 reply; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Rename the transaction functions. Remove the leading ref_ from the names
and append _sha1 to the names for functions that create/delete/update sha1
refs.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
branch.c | 10 +++---
builtin/commit.c | 14 ++++----
builtin/fetch.c | 10 +++---
builtin/receive-pack.c | 10 +++---
builtin/replace.c | 10 +++---
builtin/tag.c | 10 +++---
builtin/update-ref.c | 22 ++++++------
fast-import.c | 22 ++++++------
refs.c | 92 +++++++++++++++++++++++++-------------------------
refs.h | 36 ++++++++++----------
sequencer.c | 12 +++----
walker.c | 14 ++++----
12 files changed, 131 insertions(+), 131 deletions(-)
diff --git a/branch.c b/branch.c
index 74d55e7..0a4d4f3 100644
--- a/branch.c
+++ b/branch.c
@@ -298,14 +298,14 @@ void create_branch(const char *head,
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, msg) ||
- ref_transaction_commit(transaction, &err))
+ transaction_update_sha1(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing, msg) ||
+ transaction_commit(transaction, &err))
die_errno(_("%s: failed to write ref: %s"),
ref.buf, err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
}
if (real_ref && track)
diff --git a/builtin/commit.c b/builtin/commit.c
index e6bb7b3..386dfb1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1716,17 +1716,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();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, "HEAD", sha1,
- current_head ?
- current_head->object.sha1 : NULL,
- 0, !!current_head, sb.buf) ||
- ref_transaction_commit(transaction, &err)) {
+ transaction_update_sha1(transaction, "HEAD", sha1,
+ current_head ?
+ current_head->object.sha1 : NULL,
+ 0, !!current_head, sb.buf) ||
+ transaction_commit(transaction, &err)) {
rollback_index_files();
die(_("HEAD: cannot update ref: %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 3a849b0..f74a267 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -383,8 +383,8 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), "%s: %s", rla, action);
- if (ref_transaction_update(transaction, ref->name, ref->new_sha1,
- ref->old_sha1, 0, check_old, msg))
+ if (transaction_update_sha1(transaction, ref->name, ref->new_sha1,
+ ref->old_sha1, 0, check_old, msg))
return STORE_REF_ERROR_OTHER;
return 0;
@@ -559,7 +559,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
}
errno = 0;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction) {
rc = error(_("cannot start ref transaction\n"));
goto abort;
@@ -676,10 +676,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
}
}
}
- if (ref_transaction_commit(transaction, NULL))
+ if (transaction_commit(transaction, NULL))
rc |= errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
- ref_transaction_free(transaction);
+ transaction_free(transaction);
if (rc & STORE_REF_ERROR_DF_CONFLICT)
error(_("some local refs could not be updated; try running\n"
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 991c659..0461f93 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -581,8 +581,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
update_shallow_ref(cmd, si))
return "shallow error";
- if (ref_transaction_update(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, "push"))
+ if (transaction_update_sha1(transaction, namespaced_name,
+ new_sha1, old_sha1, 0, 1, "push"))
return "failed to update";
return NULL; /* good */
}
@@ -807,7 +807,7 @@ static void execute_commands(struct command *commands,
head_name = head_name_to_free = resolve_refdup("HEAD", sha1, 0, NULL);
checked_connectivity = 1;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
for (cmd = commands; cmd; cmd = cmd->next) {
if (cmd->error_string)
continue;
@@ -823,9 +823,9 @@ static void execute_commands(struct command *commands,
checked_connectivity = 0;
}
}
- if (ref_transaction_commit(transaction, &err))
+ if (transaction_commit(transaction, &err))
error("%s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
if (shallow_update && !checked_connectivity)
diff --git a/builtin/replace.c b/builtin/replace.c
index 820d703..2587a06 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -157,14 +157,14 @@ static int replace_object_sha1(const char *object_ref,
else if (!force)
die("replace ref '%s' already exists", ref);
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, ref, repl, prev,
- 0, !is_null_sha1(prev), NULL) ||
- ref_transaction_commit(transaction, &err))
+ transaction_update_sha1(transaction, ref, repl, prev,
+ 0, !is_null_sha1(prev), NULL) ||
+ transaction_commit(transaction, &err))
die(_("%s: failed to replace ref: %s"), ref, err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 2cc260f..3739e23 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -702,13 +702,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();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, object, prev,
- 0, !is_null_sha1(prev), NULL) ||
- ref_transaction_commit(transaction, &err))
+ transaction_update_sha1(transaction, ref.buf, object, prev,
+ 0, !is_null_sha1(prev), NULL) ||
+ transaction_commit(transaction, &err))
die(_("%s: cannot update the ref: %s"), ref.buf, 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 c5aff92..f7e33bd 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -198,8 +198,8 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)
if (*next != line_termination)
die("update %s: extra input: %s", refname, next);
- if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old, msg))
+ if (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old, msg))
die("update %s: failed", refname);
update_flags = 0;
@@ -226,8 +226,8 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
if (*next != line_termination)
die("create %s: extra input: %s", refname, next);
- if (ref_transaction_create(transaction, refname, new_sha1,
- update_flags, msg))
+ if (transaction_create_sha1(transaction, refname, new_sha1,
+ update_flags, msg))
die("failed transaction create for %s", refname);
update_flags = 0;
@@ -258,8 +258,8 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
if (*next != line_termination)
die("delete %s: extra input: %s", refname, next);
- if (ref_transaction_delete(transaction, refname, old_sha1,
- update_flags, have_old, msg))
+ if (transaction_delete_sha1(transaction, refname, old_sha1,
+ update_flags, have_old, msg))
die("failed transaction delete for %s", refname);
update_flags = 0;
@@ -291,8 +291,8 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)
if (*next != line_termination)
die("verify %s: extra input: %s", refname, next);
- if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old, msg))
+ if (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old, msg))
die("failed transaction update for %s", refname);
update_flags = 0;
@@ -366,15 +366,15 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
die("Refusing to perform update with empty message.");
if (read_stdin) {
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (delete || no_deref || argc > 0)
usage_with_options(git_update_ref_usage, options);
if (end_null)
line_termination = '\0';
update_refs_stdin();
- if (ref_transaction_commit(transaction, &err))
+ if (transaction_commit(transaction, &err))
die("%s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}
diff --git a/fast-import.c b/fast-import.c
index 099e71b..2fa1d29 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1707,17 +1707,17 @@ static int update_branch(struct branch *b)
return -1;
}
}
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
- 0, 1, msg) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_rollback(transaction);
+ transaction_update_sha1(transaction, b->name, b->sha1, old_sha1,
+ 0, 1, msg) ||
+ transaction_commit(transaction, &err)) {
+ transaction_rollback(transaction);
error("Unable to update branch %s: %s", b->name, err.buf);
strbuf_release(&err);
return -1;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}
@@ -1740,17 +1740,17 @@ static void dump_tags(void)
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
for (t = first_tag; t; t = t->next_tag) {
sprintf(ref_name, "refs/tags/%s", t->name);
- if (ref_transaction_update(transaction, ref_name, t->sha1,
- NULL, 0, 0, msg))
+ if (transaction_update_sha1(transaction, ref_name, t->sha1,
+ NULL, 0, 0, msg))
failure |= error("Unable to update %s", err.buf);
}
- if (failure || ref_transaction_commit(transaction, &err))
+ if (failure || transaction_commit(transaction, &err))
failure |= error("Unable to update %s", err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&err);
}
diff --git a/refs.c b/refs.c
index c180fa2..6785fa6 100644
--- a/refs.c
+++ b/refs.c
@@ -2357,17 +2357,17 @@ static void prune_ref(struct ref_to_prune *r)
if (check_refname_format(r->name + 5, 0))
return;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_delete(transaction, r->name, r->sha1,
- REF_ISPRUNING, 1, NULL) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_rollback(transaction);
+ transaction_delete_sha1(transaction, r->name, r->sha1,
+ REF_ISPRUNING, 1, NULL) ||
+ transaction_commit(transaction, &err)) {
+ transaction_rollback(transaction);
warning("prune_ref: %s", err.buf);
strbuf_release(&err);
return;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
try_remove_empty_parents(r->name);
}
@@ -2521,15 +2521,15 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
struct ref_transaction *transaction;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_delete(transaction, refname, sha1, delopt,
- sha1 && !is_null_sha1(sha1), NULL) ||
- ref_transaction_commit(transaction, NULL)) {
- ref_transaction_rollback(transaction);
+ transaction_delete_sha1(transaction, refname, sha1, delopt,
+ sha1 && !is_null_sha1(sha1), NULL) ||
+ transaction_commit(transaction, NULL)) {
+ transaction_rollback(transaction);
return 1;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return 0;
}
@@ -2625,20 +2625,20 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
return error("unable to pack refs");
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_delete(transaction, oldrefname, sha1,
- REF_NODEREF | REF_ISPACKONLY,
- 1, NULL) ||
- ref_transaction_update(transaction, newrefname, sha1,
- NULL, 0, 0, logmsg) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_rollback(transaction);
+ transaction_delete_sha1(transaction, oldrefname, sha1,
+ REF_NODEREF | REF_ISPACKONLY,
+ 1, NULL) ||
+ transaction_update_sha1(transaction, newrefname, sha1,
+ NULL, 0, 0, logmsg) ||
+ transaction_commit(transaction, &err)) {
+ transaction_rollback(transaction);
error("rename_ref failed: %s", err.buf);
strbuf_release(&err);
goto rollbacklog;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
if (log && rename_tmp_log(newrefname))
goto rollbacklog;
@@ -3270,12 +3270,12 @@ struct ref_transaction {
enum ref_transaction_status status;
};
-struct ref_transaction *ref_transaction_begin(void)
+struct ref_transaction *transaction_begin(void)
{
return xcalloc(1, sizeof(struct ref_transaction));
}
-void ref_transaction_free(struct ref_transaction *transaction)
+void transaction_free(struct ref_transaction *transaction)
{
int i;
@@ -3290,14 +3290,14 @@ void ref_transaction_free(struct ref_transaction *transaction)
free(transaction);
}
-void ref_transaction_rollback(struct ref_transaction *transaction)
+void transaction_rollback(struct ref_transaction *transaction)
{
if (!transaction)
return;
transaction->status = REF_TRANSACTION_ERROR;
- ref_transaction_free(transaction);
+ transaction_free(transaction);
}
static struct ref_update *add_update(struct ref_transaction *transaction,
@@ -3312,11 +3312,11 @@ static struct ref_update *add_update(struct ref_transaction *transaction,
return update;
}
-int ref_transaction_update(struct ref_transaction *transaction,
+int transaction_update_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
- int flags, int have_old, const char *msg)
+ int flags, int have_old, const char *msg)
{
struct ref_update *update;
@@ -3340,10 +3340,10 @@ int ref_transaction_update(struct ref_transaction *transaction,
return 0;
}
-int ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags, const char *msg)
+int transaction_create_sha1(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ int flags, const char *msg)
{
struct ref_update *update;
@@ -3356,14 +3356,14 @@ int ref_transaction_create(struct ref_transaction *transaction,
if (flags & REF_ISPACKONLY)
die("BUG: REF_ISPACKONLY can not be used with create");
- return ref_transaction_update(transaction, refname, new_sha1,
- null_sha1, flags, 1, msg);
+ return transaction_update_sha1(transaction, refname, new_sha1,
+ null_sha1, flags, 1, msg);
}
-int ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old, const char *msg)
+int transaction_delete_sha1(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old, const char *msg)
{
struct ref_update *update;
@@ -3376,8 +3376,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (transaction->status != REF_TRANSACTION_OPEN)
die("BUG: delete on transaction that is not open");
- return ref_transaction_update(transaction, refname, null_sha1,
- old_sha1, flags, have_old, msg);
+ return transaction_update_sha1(transaction, refname, null_sha1,
+ old_sha1, flags, have_old, msg);
}
int update_ref(const char *action, const char *refname,
@@ -3387,14 +3387,14 @@ int update_ref(const char *action, const char *refname,
struct ref_transaction *t;
struct strbuf err = STRBUF_INIT;
- t = ref_transaction_begin();
+ t = transaction_begin();
if (!t ||
- ref_transaction_update(t, refname, sha1, oldval, flags,
- !!oldval, action) ||
- ref_transaction_commit(t, &err)) {
+ transaction_update_sha1(t, refname, sha1, oldval, flags,
+ !!oldval, action) ||
+ transaction_commit(t, &err)) {
const char *str = "update_ref failed for ref '%s': %s";
- ref_transaction_rollback(t);
+ transaction_rollback(t);
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR:
error(str, refname, err.buf); break;
@@ -3405,7 +3405,7 @@ int update_ref(const char *action, const char *refname,
strbuf_release(&err);
return 1;
}
- ref_transaction_free(t);
+ transaction_free(t);
return 0;
}
@@ -3432,7 +3432,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
return 0;
}
-int ref_transaction_commit(struct ref_transaction *transaction,
+int transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
int ret = 0, delnum = 0, i;
diff --git a/refs.h b/refs.h
index 8d56edf..127c12f 100644
--- a/refs.h
+++ b/refs.h
@@ -213,12 +213,12 @@ enum action_on_err {
* eventually be freed by either calling ref_transaction_rollback()
* or ref_transaction_free().
*/
-struct ref_transaction *ref_transaction_begin(void);
+struct ref_transaction *transaction_begin(void);
/*
* Roll back a ref_transaction and free all associated data.
*/
-void ref_transaction_rollback(struct ref_transaction *transaction);
+void transaction_rollback(struct ref_transaction *transaction);
/*
@@ -239,11 +239,11 @@ void ref_transaction_rollback(struct ref_transaction *transaction);
* already exist and a new ref will be created with new_sha1.
* Function returns 0 on success and non-zero on failure.
*/
-int ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- int flags, int have_old, const char *msg);
+int transaction_update_sha1(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ int flags, int have_old, const char *msg);
/*
* Add a reference creation to transaction. new_sha1 is the value
@@ -252,10 +252,10 @@ int ref_transaction_update(struct ref_transaction *transaction,
* already.
* Function returns 0 on success and non-zero on failure.
*/
-int ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags, const char *msg);
+int transaction_create_sha1(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ int flags, const char *msg);
/*
* Add a reference deletion to transaction. If have_old is true, then
@@ -263,10 +263,10 @@ int ref_transaction_create(struct ref_transaction *transaction,
* the update (which must not be the null SHA-1).
* Function returns 0 on success and non-zero on failure.
*/
-int ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old, const char *msg);
+int transaction_delete_sha1(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old, const char *msg);
/*
* Commit all of the changes that have been queued in transaction, as
@@ -274,13 +274,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,
* problem. If err is non-NULL we will add an error string to it to explain
* why the transaction failed. The string does not end in newline.
*/
-int ref_transaction_commit(struct ref_transaction *transaction,
- struct strbuf *err);
+int transaction_commit(struct ref_transaction *transaction,
+ struct strbuf *err);
/*
* Free an existing transaction.
*/
-void ref_transaction_free(struct ref_transaction *transaction);
+void transaction_free(struct ref_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 3a0ee09..3f7996f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -282,18 +282,18 @@ 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();
+ transaction = transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, "HEAD", to, from,
- 0, !unborn, sb.buf) ||
- ref_transaction_commit(transaction, &err)) {
- ref_transaction_rollback(transaction);
+ transaction_update_sha1(transaction, "HEAD", to, from,
+ 0, !unborn, sb.buf) ||
+ transaction_commit(transaction, &err)) {
+ transaction_rollback(transaction);
error(_("HEAD: Could not fast-forward: %s\n"), err.buf);
strbuf_release(&sb);
strbuf_release(&err);
return -1;
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(&sb);
return 0;
diff --git a/walker.c b/walker.c
index c2a1266..3701c78 100644
--- a/walker.c
+++ b/walker.c
@@ -260,7 +260,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
save_commit_buffer = 0;
- transaction = ref_transaction_begin();
+ transaction = transaction_begin();
if (!transaction)
return -1;
@@ -289,14 +289,14 @@ int walker_fetch(struct walker *walker, int targets, char **target,
if (!write_ref || !write_ref[i])
continue;
sprintf(ref_name, "refs/%s", write_ref[i]);
- if (ref_transaction_update(transaction, ref_name,
- &sha1[20 * i], NULL,
- 0, 0,
- msg ? msg : "fetch (unknown)"))
+ if (transaction_update_sha1(transaction, ref_name,
+ &sha1[20 * i], NULL,
+ 0, 0,
+ msg ? msg : "fetch (unknown)"))
goto rollback_and_fail;
}
- if (ref_transaction_commit(transaction, &err)) {
+ if (transaction_commit(transaction, &err)) {
error("%s", err.buf);
goto rollback_and_fail;
}
@@ -307,7 +307,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
rollback_and_fail:
free(msg);
strbuf_release(&err);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
return -1;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 04/31] refs.c: add a new update_type field to ref_update
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (2 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 03/31] refs.c: rename the transaction functions Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 05/31] refs.c: add a function to append a reflog entry to a fd Ronnie Sahlberg
` (26 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
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.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 6785fa6..018062d 100644
--- a/refs.c
+++ b/refs.c
@@ -3235,6 +3235,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
@@ -3242,6 +3246,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? */
@@ -3301,12 +3306,14 @@ void transaction_rollback(struct ref_transaction *transaction)
}
static struct ref_update *add_update(struct ref_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;
@@ -3329,7 +3336,7 @@ int transaction_update_sha1(struct ref_transaction *transaction,
if (flags & REF_ISPACKONLY && !is_null_sha1(new_sha1))
die("BUG: REF_ISPACKONLY can not be used with updates");
- 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;
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 05/31] refs.c: add a function to append a reflog entry to a fd
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (3 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 04/31] refs.c: add a new update_type field to ref_update Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 06/31] refs.c: add a transaction function to append a reflog entry Ronnie Sahlberg
` (25 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
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).
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 48 ++++++++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 018062d..98c6c00 100644
--- a/refs.c
+++ b/refs.c
@@ -2745,15 +2745,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();
@@ -2765,19 +2787,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 (close(logfd) != 0 || written != len)
+ result = log_ref_write_fd(logfd, old_sha1, new_sha1,
+ git_committer_info(0), msg);
+ if (close(logfd) != 0 || result)
return error("Unable to append to %s", log_file);
return 0;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 06/31] refs.c: add a transaction function to append a reflog entry
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (4 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 05/31] refs.c: add a function to append a reflog entry to a fd Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log Ronnie Sahlberg
` (24 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
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.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Conflicts:
refs.h
---
refs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
refs.h | 13 ++++++++
2 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 98c6c00..d8a1568 100644
--- a/refs.c
+++ b/refs.c
@@ -3249,6 +3249,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
enum transaction_update_type {
UPDATE_SHA1 = 0,
+ UPDATE_LOG = 1,
};
/**
@@ -3266,6 +3267,12 @@ struct ref_update {
struct ref_lock *lock;
int type;
const char *msg;
+
+ /* used by reflog updates */
+ int reflog_fd;
+ struct lock_file reflog_lock;
+ const char *committer;
+
const char refname[FLEX_ARRAY];
};
@@ -3300,7 +3307,8 @@ void transaction_free(struct ref_transaction *transaction)
return;
for (i = 0; i < transaction->nr; i++) {
- free((char *)transaction->updates[i]->msg);
+ free((char *)transaction->updates[i]->committer);
+ free((char *)transaction->updates[i]->msg);
free(transaction->updates[i]);
}
free(transaction->updates);
@@ -3331,6 +3339,38 @@ static struct ref_update *add_update(struct ref_transaction *transaction,
return update;
}
+int transaction_update_reflog(struct ref_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 ref_update *update;
+
+ 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_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
@@ -3440,6 +3480,8 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
{
int i;
for (i = 1; i < n; i++)
+ if (updates[i]->update_type != UPDATE_SHA1)
+ continue;
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
const char *str =
"Multiple updates for ref '%s' not allowed.";
@@ -3479,14 +3521,18 @@ int transaction_commit(struct ref_transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (update->flags & REF_ISPACKONLY)
delnames[delnum++] = update->refname;
}
- /* 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];
+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (update->flags & REF_ISPACKONLY)
continue;
@@ -3506,10 +3552,32 @@ int transaction_commit(struct ref_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'.";
+
+ ret = -1;
+ if (err)
+ strbuf_addf(err, str, update->refname);
+ goto cleanup;
+ }
+ }
+
+ /* Perform ref updates first so live commits remain referenced */
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)) {
ret = write_ref_sha1(update->lock, update->new_sha1,
update->msg);
@@ -3528,6 +3596,8 @@ int transaction_commit(struct ref_transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
+ if (update->update_type != UPDATE_SHA1)
+ continue;
if (update->flags & REF_ISPACKONLY)
continue;
@@ -3538,6 +3608,36 @@ int transaction_commit(struct ref_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)) {
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ }
+ }
+
+ /* Unock 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)) {
+ update->reflog_fd = -1;
+ }
+ }
+
ret |= repack_without_refs(delnames, delnum);
for (i = 0; i < delnum; i++)
unlink_or_warn(git_path("logs/%s", delnames[i]));
@@ -3688,4 +3788,5 @@ int ref_is_hidden(const char *refname)
return 1;
}
return 0;
+
}
diff --git a/refs.h b/refs.h
index 127c12f..2e22a26 100644
--- a/refs.h
+++ b/refs.h
@@ -134,6 +134,7 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
/** Locks any ref (for 'HEAD' type refs). */
#define REF_NODEREF 0x01
+
extern struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p);
@@ -269,6 +270,18 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
int flags, int have_old, const char *msg);
/*
+ * Append a reflog entry for refname.
+ */
+int transaction_update_reflog(struct ref_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);
+
+/*
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem. If err is non-NULL we will add an error string to it to explain
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (5 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 06/31] refs.c: add a transaction function to append a reflog entry Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-16 21:20 ` Junio C Hamano
2014-05-14 22:13 ` [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL Ronnie Sahlberg
` (23 subsequent siblings)
30 siblings, 1 reply; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Add a flag that allows us to truncate the reflog before we write the update.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 12 ++++++++++--
refs.h | 4 +++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index d8a1568..a8b583a 100644
--- a/refs.c
+++ b/refs.c
@@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_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];
@@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_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");
+ }
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 2e22a26..b1b97e7 100644
--- a/refs.h
+++ b/refs.h
@@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg);
+#define REFLOG_TRUNCATE 0x01
/*
- * Append a reflog entry for refname.
+ * 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 ref_transaction *transaction,
const char *refname,
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (6 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-16 21:24 ` Junio C Hamano
2014-05-14 22:13 ` [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction Ronnie Sahlberg
` (22 subsequent siblings)
30 siblings, 1 reply; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
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.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 8 +++++---
refs.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index a8b583a..a3f60ad 100644
--- a/refs.c
+++ b/refs.c
@@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,
ftruncate(update->reflog_fd, 0)) {
error("Could not truncate reflog");
}
- if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
- update->new_sha1,
- update->committer, update->msg)) {
+ if (update->msg && log_ref_write_fd(update->reflog_fd,
+ update->old_sha1,
+ update->new_sha1,
+ update->committer,
+ update->msg)) {
rollback_lock_file(&update->reflog_lock);
update->reflog_fd = -1;
}
diff --git a/refs.h b/refs.h
index b1b97e7..c552f04 100644
--- a/refs.h
+++ b/refs.h
@@ -273,6 +273,7 @@ int transaction_delete_sha1(struct ref_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 ref_transaction *transaction,
const char *refname,
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (7 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-16 21:35 ` Junio C Hamano
2014-05-14 22:13 ` [PATCH 10/31] reflog.c: use a reflog transaction when writing during expire Ronnie Sahlberg
` (21 subsequent siblings)
30 siblings, 1 reply; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
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-somehting...
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.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 49 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index a3f60ad..e7ede03 100644
--- a/refs.c
+++ b/refs.c
@@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
* need to lock the loose ref during the transaction.
*/
#define REF_ISPACKONLY 0x0200
+/** 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 0x0300
/*
* Try to read one refname component from the front of refname. Return
@@ -3252,6 +3256,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
@@ -3262,7 +3267,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;
@@ -3270,8 +3275,9 @@ struct ref_update {
/* used by reflog updates */
int reflog_fd;
- struct lock_file reflog_lock;
+ struct lock_file *reflog_lock;
const char *committer;
+ struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */
const char refname[FLEX_ARRAY];
};
@@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,
int flags)
{
struct ref_update *update;
+ int i;
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;
@@ -3366,7 +3387,6 @@ int transaction_update_reflog(struct ref_transaction *transaction,
}
if (msg)
update->msg = xstrdup(msg);
- update->flags = flags;
return 0;
}
@@ -3479,7 +3499,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
struct strbuf *err)
{
int i;
- for (i = 1; i < n; i++)
+ for (i = 1; i < n; i++) {
if (updates[i]->update_type != UPDATE_SHA1)
continue;
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
@@ -3490,6 +3510,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
return 1;
}
+ }
return 0;
}
@@ -3558,10 +3579,15 @@ int transaction_commit(struct ref_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_NODEREF);
if (update->reflog_fd < 0) {
const char *str = "Cannot lock reflog for '%s'.";
@@ -3630,20 +3656,22 @@ int transaction_commit(struct ref_transaction *transaction,
update->new_sha1,
update->committer,
update->msg)) {
- rollback_lock_file(&update->reflog_lock);
+ rollback_lock_file(update->reflog_lock);
update->reflog_fd = -1;
}
}
- /* Unock all reflog files */
+ /* 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->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)) {
update->reflog_fd = -1;
}
}
@@ -3654,6 +3682,18 @@ int transaction_commit(struct ref_transaction *transaction,
clear_loose_ref_cache(&ref_cache);
cleanup:
+ /* Rollback any reflog files that are still open */
+ for (i = 0; i < n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update->update_type != UPDATE_LOG)
+ continue;
+ if (update->flags & UPDATE_REFLOG_NOLOCK)
+ continue;
+ if (update->reflog_fd == -1)
+ continue;
+ rollback_lock_file(update->reflog_lock);
+ }
transaction->status = ret ? REF_TRANSACTION_ERROR
: REF_TRANSACTION_CLOSED;
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 10/31] reflog.c: use a reflog transaction when writing during expire
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (8 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 11/31] refs.c: null terminate the string in copy_msg Ronnie Sahlberg
` (20 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Use a transaction when doing the updates of the reflog during expire_reflog.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/reflog.c | 54 ++++++++++++++++++++++--------------------------------
refs.c | 4 ++--
2 files changed, 24 insertions(+), 34 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index e8a8fb1..5f07647 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -33,7 +33,8 @@ struct cmd_reflog_expire_cb {
};
struct expire_reflog_cb {
- FILE *newlog;
+ struct ref_transaction *t;
+ const char *refname;
enum {
UE_NORMAL,
UE_ALWAYS,
@@ -316,20 +317,16 @@ 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) {
+ transaction_update_reflog(cb->t, cb->refname, nsha1, osha1,
+ email, timestamp, tz, message, 0);
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);
@@ -354,12 +351,12 @@ 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
@@ -368,12 +365,14 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
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();
+ transaction_update_reflog(cb.t, cb.refname,
+ null_sha1, null_sha1, NULL,
+ 0, 0, NULL,
+ REFLOG_TRUNCATE);
}
cb.cmd = cmd;
@@ -420,31 +419,22 @@ 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)) {
+ if (cb.t) {
+ 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);
- unlink(newlog_path);
- } else if (rename(newlog_path, log_file)) {
- status |= error("cannot rename %s to %s",
- newlog_path, log_file);
- unlink(newlog_path);
+ transaction_rollback(cb.t);
+ } else if (transaction_commit(cb.t, NULL)) {
+ status |= error("cannot commit reflog for %s", ref);
} else if (cmd->updateref && commit_ref(lock)) {
status |= error("Couldn't set %s", lock->ref_name);
- } else {
- adjust_shared_perm(log_file);
}
+ transaction_free(cb.t);
}
- free(newlog_path);
- free(log_file);
unlock_ref(lock);
return status;
}
diff --git a/refs.c b/refs.c
index e7ede03..5a5f9df 100644
--- a/refs.c
+++ b/refs.c
@@ -2766,8 +2766,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
committer);
if (msglen)
len += copy_msg(logrec + len - 1, msg) - 1;
- written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
+ written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
free(logrec);
if (written != len)
return -1;
@@ -3634,7 +3634,7 @@ int transaction_commit(struct ref_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.
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 11/31] refs.c: null terminate the string in copy_msg
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (9 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 10/31] reflog.c: use a reflog transaction when writing during expire Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 12/31] refs.c: track the refnames we are deleting in the transaction structure Ronnie Sahlberg
` (19 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/refs.c b/refs.c
index 5a5f9df..58bbf1b 100644
--- a/refs.c
+++ b/refs.c
@@ -2707,6 +2707,7 @@ static int copy_msg(char *buf, const char *msg)
while (buf < cp && isspace(cp[-1]))
cp--;
*cp++ = '\n';
+ *cp = 0;
return cp - buf;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 12/31] refs.c: track the refnames we are deleting in the transaction structure
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (10 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 11/31] refs.c: null terminate the string in copy_msg Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 13/31] refs.c: update the list of deleted refs during _update instead of _commit Ronnie Sahlberg
` (18 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Track the names of the refs we are deleting in the ref_transaction structure
instead of building an array of names during _commit.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 58bbf1b..cc31efe 100644
--- a/refs.c
+++ b/refs.c
@@ -3299,6 +3299,8 @@ struct ref_transaction {
size_t alloc;
size_t nr;
enum ref_transaction_status status;
+ const char **delnames;
+ int delnum;
};
struct ref_transaction *transaction_begin(void)
@@ -3306,6 +3308,15 @@ struct ref_transaction *transaction_begin(void)
return xcalloc(1, sizeof(struct ref_transaction));
}
+static void add_delname(struct ref_transaction *transaction,
+ const char *refname)
+{
+ transaction->delnames = xrealloc(transaction->delnames,
+ ++transaction->delnum *
+ sizeof(const char *));
+ transaction->delnames[transaction->delnum - 1] = xstrdup(refname);
+}
+
void transaction_free(struct ref_transaction *transaction)
{
int i;
@@ -3319,6 +3330,9 @@ void transaction_free(struct ref_transaction *transaction)
free(transaction->updates[i]);
}
free(transaction->updates);
+ for (i = 0; i < transaction->delnum; i++)
+ free((char *)transaction->delnames[i]);
+ free((char *)transaction->delnames);
free(transaction);
}
@@ -3518,8 +3532,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
int transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- int ret = 0, delnum = 0, i;
- const char **delnames;
+ int ret = 0, i;
int n = transaction->nr;
struct ref_update **updates = transaction->updates;
@@ -3531,9 +3544,6 @@ int transaction_commit(struct ref_transaction *transaction,
return 0;
}
- /* Allocate work space */
- delnames = xmalloc(sizeof(*delnames) * n);
-
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
ret = ref_update_reject_duplicates(updates, n, err);
@@ -3546,7 +3556,7 @@ int transaction_commit(struct ref_transaction *transaction,
if (update->update_type != UPDATE_SHA1)
continue;
if (update->flags & REF_ISPACKONLY)
- delnames[delnum++] = update->refname;
+ add_delname(transaction, update->refname);
}
/* Acquire all ref locks while verifying old values */
@@ -3564,7 +3574,8 @@ int transaction_commit(struct ref_transaction *transaction,
NULL),
update->flags,
&update->type,
- delnames, delnum);
+ transaction->delnames,
+ transaction->delnum);
if (!update->lock) {
if (err)
strbuf_addf(err, "Cannot lock the ref '%s'.",
@@ -3631,7 +3642,8 @@ int transaction_commit(struct ref_transaction *transaction,
if (update->lock) {
ret |= delete_ref_loose(update->lock, update->type);
if (!(update->flags & REF_ISPRUNING))
- delnames[delnum++] = update->lock->ref_name;
+ add_delname(transaction,
+ update->lock->ref_name);
}
}
@@ -3677,9 +3689,9 @@ int transaction_commit(struct ref_transaction *transaction,
}
}
- ret |= repack_without_refs(delnames, delnum);
- for (i = 0; i < delnum; i++)
- unlink_or_warn(git_path("logs/%s", delnames[i]));
+ ret |= repack_without_refs(transaction->delnames, transaction->delnum);
+ for (i = 0; i < transaction->delnum; i++)
+ unlink_or_warn(git_path("logs/%s", transaction->delnames[i]));
clear_loose_ref_cache(&ref_cache);
cleanup:
@@ -3701,7 +3713,6 @@ cleanup:
for (i = 0; i < n; i++)
if (updates[i]->lock)
unlock_ref(updates[i]->lock);
- free(delnames);
return ret;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 13/31] refs.c: update the list of deleted refs during _update instead of _commit
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (11 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 12/31] refs.c: track the refnames we are deleting in the transaction structure Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 14/31] refs.c: return error instead of dying when locking fails during transaction Ronnie Sahlberg
` (17 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
When deleting refs during a transaction, update the list of the refs to be
deleted already during _update instead of waiting until the _commit stage.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index cc31efe..3249576 100644
--- a/refs.c
+++ b/refs.c
@@ -3431,6 +3431,13 @@ int transaction_update_sha1(struct ref_transaction *transaction,
hashcpy(update->old_sha1, old_sha1);
if (msg)
update->msg = xstrdup(msg);
+
+ /* This is a deletion of a ref that exists as a packed ref which means
+ * we do not need to check against this ref for name collissions
+ * during locking.
+ */
+ if (update->flags & REF_ISPACKONLY)
+ add_delname(transaction, update->refname);
return 0;
}
@@ -3550,15 +3557,6 @@ int transaction_commit(struct ref_transaction *transaction,
if (ret)
goto cleanup;
- for (i = 0; i < n; i++) {
- struct ref_update *update = updates[i];
-
- if (update->update_type != UPDATE_SHA1)
- continue;
- if (update->flags & REF_ISPACKONLY)
- add_delname(transaction, update->refname);
- }
-
/* Acquire all ref locks while verifying old values */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 14/31] refs.c: return error instead of dying when locking fails during transaction
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (12 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 13/31] refs.c: update the list of deleted refs during _update instead of _commit Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 15/31] refs.c: lock the ref during _update instead of during _commit Ronnie Sahlberg
` (16 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Change lock_ref_sha1_basic to return an error instead of dying when
we fail to lock a file during a transaction.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 3249576..8212d77 100644
--- a/refs.c
+++ b/refs.c
@@ -2147,7 +2147,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
*/
goto retry;
else
- unable_to_lock_index_die(ref_file, errno);
+ goto error_return;
}
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 15/31] refs.c: lock the ref during _update instead of during _commit
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (13 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 14/31] refs.c: return error instead of dying when locking fails during transaction Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 16/31] refs.c: add an error argument to create/delete/update just like commit Ronnie Sahlberg
` (15 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Try to lock the ref during the transaction update instead of later during the
commit phase. This allows us to do things such as :
t = transaction_begin()
transaction_update_sha1(t, ref, ...) /* locking ref */
...
do stuff while we know that ref is locked
...
transaction_commit(t)
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 8212d77..b006238 100644
--- a/refs.c
+++ b/refs.c
@@ -3438,6 +3438,16 @@ int transaction_update_sha1(struct ref_transaction *transaction,
*/
if (update->flags & REF_ISPACKONLY)
add_delname(transaction, update->refname);
+ else {
+ update->lock = lock_ref_sha1_basic(update->refname,
+ (update->have_old ?
+ update->old_sha1 :
+ NULL),
+ update->flags,
+ &update->type,
+ transaction->delnames,
+ transaction->delnum);
+ }
return 0;
}
@@ -3566,14 +3576,6 @@ int transaction_commit(struct ref_transaction *transaction,
if (update->flags & REF_ISPACKONLY)
continue;
- update->lock = lock_ref_sha1_basic(update->refname,
- (update->have_old ?
- update->old_sha1 :
- NULL),
- update->flags,
- &update->type,
- transaction->delnames,
- transaction->delnum);
if (!update->lock) {
if (err)
strbuf_addf(err, "Cannot lock the ref '%s'.",
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 16/31] refs.c: add an error argument to create/delete/update just like commit
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (14 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 15/31] refs.c: lock the ref during _update instead of during _commit Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 17/31] refs.c: make _update_reflog take an error argument Ronnie Sahlberg
` (14 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Update the signatures for transaction create/delete/update and add
a strbuf err argument we can use to pass an error back to the caller.
We will need this later once we start moving checks from _commit to _update.
Change all callers that would die() on _commit failures to handle _update
failures similarly.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
branch.c | 3 ++-
builtin/commit.c | 2 +-
builtin/fetch.c | 2 +-
builtin/receive-pack.c | 3 ++-
builtin/replace.c | 2 +-
builtin/tag.c | 2 +-
builtin/update-ref.c | 20 ++++++++++++--------
fast-import.c | 8 +++++---
refs.c | 24 ++++++++++++++----------
refs.h | 9 ++++++---
sequencer.c | 2 +-
walker.c | 3 ++-
12 files changed, 48 insertions(+), 32 deletions(-)
diff --git a/branch.c b/branch.c
index 0a4d4f3..0b9fd61 100644
--- a/branch.c
+++ b/branch.c
@@ -301,7 +301,8 @@ void create_branch(const char *head,
transaction = transaction_begin();
if (!transaction ||
transaction_update_sha1(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, msg) ||
+ null_sha1, 0, !forcing, msg,
+ &err) ||
transaction_commit(transaction, &err))
die_errno(_("%s: failed to write ref: %s"),
ref.buf, err.buf);
diff --git a/builtin/commit.c b/builtin/commit.c
index 386dfb1..fd61a8e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1721,7 +1721,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
transaction_update_sha1(transaction, "HEAD", sha1,
current_head ?
current_head->object.sha1 : NULL,
- 0, !!current_head, sb.buf) ||
+ 0, !!current_head, sb.buf, &err) ||
transaction_commit(transaction, &err)) {
rollback_index_files();
die(_("HEAD: cannot update ref: %s"), err.buf);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f74a267..e5e891f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -384,7 +384,7 @@ static int s_update_ref(const char *action,
snprintf(msg, sizeof(msg), "%s: %s", rla, action);
if (transaction_update_sha1(transaction, ref->name, ref->new_sha1,
- ref->old_sha1, 0, check_old, msg))
+ ref->old_sha1, 0, check_old, msg, NULL))
return STORE_REF_ERROR_OTHER;
return 0;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0461f93..777ecf8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -582,7 +582,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
return "shallow error";
if (transaction_update_sha1(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, "push"))
+ new_sha1, old_sha1, 0, 1, "push",
+ &err))
return "failed to update";
return NULL; /* good */
}
diff --git a/builtin/replace.c b/builtin/replace.c
index 2587a06..1598d69 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -160,7 +160,7 @@ static int replace_object_sha1(const char *object_ref,
transaction = transaction_begin();
if (!transaction ||
transaction_update_sha1(transaction, ref, repl, prev,
- 0, !is_null_sha1(prev), NULL) ||
+ 0, !is_null_sha1(prev), NULL, &err) ||
transaction_commit(transaction, &err))
die(_("%s: failed to replace ref: %s"), ref, err.buf);
diff --git a/builtin/tag.c b/builtin/tag.c
index 3739e23..9b6da6c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -705,7 +705,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = transaction_begin();
if (!transaction ||
transaction_update_sha1(transaction, ref.buf, object, prev,
- 0, !is_null_sha1(prev), NULL) ||
+ 0, !is_null_sha1(prev), NULL, &err) ||
transaction_commit(transaction, &err))
die(_("%s: cannot update the ref: %s"), ref.buf, err.buf);
transaction_free(transaction);
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index f7e33bd..1b0c889 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -183,6 +183,7 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int have_old;
+ struct strbuf err = STRBUF_INIT;
refname = parse_refname(input, &next);
if (!refname)
@@ -199,8 +200,8 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)
die("update %s: extra input: %s", refname, next);
if (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old, msg))
- die("update %s: failed", refname);
+ update_flags, have_old, msg, &err))
+ die("%s", err.buf);
update_flags = 0;
free(refname);
@@ -212,6 +213,7 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
{
char *refname;
unsigned char new_sha1[20];
+ struct strbuf err = STRBUF_INIT;
refname = parse_refname(input, &next);
if (!refname)
@@ -227,8 +229,8 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
die("create %s: extra input: %s", refname, next);
if (transaction_create_sha1(transaction, refname, new_sha1,
- update_flags, msg))
- die("failed transaction create for %s", refname);
+ update_flags, msg, &err))
+ die("%s", err.buf);
update_flags = 0;
free(refname);
@@ -241,6 +243,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
char *refname;
unsigned char old_sha1[20];
int have_old;
+ struct strbuf err = STRBUF_INIT;
refname = parse_refname(input, &next);
if (!refname)
@@ -259,8 +262,8 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
die("delete %s: extra input: %s", refname, next);
if (transaction_delete_sha1(transaction, refname, old_sha1,
- update_flags, have_old, msg))
- die("failed transaction delete for %s", refname);
+ update_flags, have_old, msg, &err))
+ die("%s", err.buf);
update_flags = 0;
free(refname);
@@ -274,6 +277,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int have_old;
+ struct strbuf err = STRBUF_INIT;
refname = parse_refname(input, &next);
if (!refname)
@@ -292,8 +296,8 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)
die("verify %s: extra input: %s", refname, next);
if (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old, msg))
- die("failed transaction update for %s", refname);
+ update_flags, have_old, msg, &err))
+ die("%s", err.buf);
update_flags = 0;
free(refname);
diff --git a/fast-import.c b/fast-import.c
index 2fa1d29..24378d1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1710,7 +1710,7 @@ static int update_branch(struct branch *b)
transaction = transaction_begin();
if (!transaction ||
transaction_update_sha1(transaction, b->name, b->sha1, old_sha1,
- 0, 1, msg) ||
+ 0, 1, msg, &err) ||
transaction_commit(transaction, &err)) {
transaction_rollback(transaction);
error("Unable to update branch %s: %s", b->name, err.buf);
@@ -1745,8 +1745,10 @@ static void dump_tags(void)
sprintf(ref_name, "refs/tags/%s", t->name);
if (transaction_update_sha1(transaction, ref_name, t->sha1,
- NULL, 0, 0, msg))
- failure |= error("Unable to update %s", err.buf);
+ NULL, 0, 0, msg, &err)) {
+ failure |= 1;
+ break;
+ }
}
if (failure || transaction_commit(transaction, &err))
failure |= error("Unable to update %s", err.buf);
diff --git a/refs.c b/refs.c
index b006238..59f1ca1 100644
--- a/refs.c
+++ b/refs.c
@@ -2364,7 +2364,7 @@ static void prune_ref(struct ref_to_prune *r)
transaction = transaction_begin();
if (!transaction ||
transaction_delete_sha1(transaction, r->name, r->sha1,
- REF_ISPRUNING, 1, NULL) ||
+ REF_ISPRUNING, 1, NULL, &err) ||
transaction_commit(transaction, &err)) {
transaction_rollback(transaction);
warning("prune_ref: %s", err.buf);
@@ -2528,7 +2528,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
transaction = transaction_begin();
if (!transaction ||
transaction_delete_sha1(transaction, refname, sha1, delopt,
- sha1 && !is_null_sha1(sha1), NULL) ||
+ sha1 && !is_null_sha1(sha1), NULL, NULL) ||
transaction_commit(transaction, NULL)) {
transaction_rollback(transaction);
return 1;
@@ -2633,9 +2633,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
if (!transaction ||
transaction_delete_sha1(transaction, oldrefname, sha1,
REF_NODEREF | REF_ISPACKONLY,
- 1, NULL) ||
+ 1, NULL, &err) ||
transaction_update_sha1(transaction, newrefname, sha1,
- NULL, 0, 0, logmsg) ||
+ NULL, 0, 0, logmsg, &err) ||
transaction_commit(transaction, &err)) {
transaction_rollback(transaction);
error("rename_ref failed: %s", err.buf);
@@ -3410,7 +3410,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
- int flags, int have_old, const char *msg)
+ int flags, int have_old, const char *msg,
+ struct strbuf *err)
{
struct ref_update *update;
@@ -3454,7 +3455,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,
int transaction_create_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
- int flags, const char *msg)
+ int flags, const char *msg,
+ struct strbuf *err)
{
struct ref_update *update;
@@ -3468,13 +3470,14 @@ int transaction_create_sha1(struct ref_transaction *transaction,
die("BUG: REF_ISPACKONLY can not be used with create");
return transaction_update_sha1(transaction, refname, new_sha1,
- null_sha1, flags, 1, msg);
+ null_sha1, flags, 1, msg, err);
}
int transaction_delete_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
- int flags, int have_old, const char *msg)
+ int flags, int have_old, const char *msg,
+ struct strbuf *err)
{
struct ref_update *update;
@@ -3488,7 +3491,8 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
die("BUG: delete on transaction that is not open");
return transaction_update_sha1(transaction, refname, null_sha1,
- old_sha1, flags, have_old, msg);
+ old_sha1, flags, have_old, msg,
+ err);
}
int update_ref(const char *action, const char *refname,
@@ -3501,7 +3505,7 @@ int update_ref(const char *action, const char *refname,
t = transaction_begin();
if (!t ||
transaction_update_sha1(t, refname, sha1, oldval, flags,
- !!oldval, action) ||
+ !!oldval, action, &err) ||
transaction_commit(t, &err)) {
const char *str = "update_ref failed for ref '%s': %s";
diff --git a/refs.h b/refs.h
index c552f04..ebe7368 100644
--- a/refs.h
+++ b/refs.h
@@ -244,7 +244,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
- int flags, int have_old, const char *msg);
+ int flags, int have_old, const char *msg,
+ struct strbuf *err);
/*
* Add a reference creation to transaction. new_sha1 is the value
@@ -256,7 +257,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,
int transaction_create_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
- int flags, const char *msg);
+ int flags, const char *msg,
+ struct strbuf *err);
/*
* Add a reference deletion to transaction. If have_old is true, then
@@ -267,7 +269,8 @@ int transaction_create_sha1(struct ref_transaction *transaction,
int transaction_delete_sha1(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
- int flags, int have_old, const char *msg);
+ int flags, int have_old, const char *msg,
+ struct strbuf *err);
#define REFLOG_TRUNCATE 0x01
/*
diff --git a/sequencer.c b/sequencer.c
index 3f7996f..604df39 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -285,7 +285,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
transaction = transaction_begin();
if (!transaction ||
transaction_update_sha1(transaction, "HEAD", to, from,
- 0, !unborn, sb.buf) ||
+ 0, !unborn, sb.buf, &err) ||
transaction_commit(transaction, &err)) {
transaction_rollback(transaction);
error(_("HEAD: Could not fast-forward: %s\n"), err.buf);
diff --git a/walker.c b/walker.c
index 3701c78..8630fcb 100644
--- a/walker.c
+++ b/walker.c
@@ -292,7 +292,8 @@ int walker_fetch(struct walker *walker, int targets, char **target,
if (transaction_update_sha1(transaction, ref_name,
&sha1[20 * i], NULL,
0, 0,
- msg ? msg : "fetch (unknown)"))
+ msg ? msg : "fetch (unknown)",
+ &err))
goto rollback_and_fail;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 17/31] refs.c: make _update_reflog take an error argument
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (15 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 16/31] refs.c: add an error argument to create/delete/update just like commit Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 18/31] refs.c: return immediately from _commit if the transaction has an error Ronnie Sahlberg
` (13 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Add a strbug argument to the update reflog transaction call so we can return
an error string back to the caller on failure.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/reflog.c | 6 ++++--
refs.c | 3 ++-
refs.h | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 5f07647..2239249 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -319,7 +319,8 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
if (cb->t) {
transaction_update_reflog(cb->t, cb->refname, nsha1, osha1,
- email, timestamp, tz, message, 0);
+ email, timestamp, tz, message, 0,
+ NULL);
hashcpy(cb->last_kept_sha1, nsha1);
}
if (cb->cmd->verbose)
@@ -372,7 +373,8 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
transaction_update_reflog(cb.t, cb.refname,
null_sha1, null_sha1, NULL,
0, 0, NULL,
- REFLOG_TRUNCATE);
+ REFLOG_TRUNCATE,
+ NULL);
}
cb.cmd = cmd;
diff --git a/refs.c b/refs.c
index 59f1ca1..8b09258 100644
--- a/refs.c
+++ b/refs.c
@@ -3367,7 +3367,8 @@ int transaction_update_reflog(struct ref_transaction *transaction,
const unsigned char *email,
unsigned long timestamp, int tz,
const char *msg,
- int flags)
+ int flags,
+ struct strbuf *err)
{
struct ref_update *update;
int i;
diff --git a/refs.h b/refs.h
index ebe7368..6c628bb 100644
--- a/refs.h
+++ b/refs.h
@@ -285,7 +285,8 @@ int transaction_update_reflog(struct ref_transaction *transaction,
const unsigned char *email,
unsigned long timestamp, int tz,
const char *msg,
- int flags);
+ int flags,
+ struct strbuf *err);
/*
* Commit all of the changes that have been queued in transaction, as
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 18/31] refs.c: return immediately from _commit if the transaction has an error
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (16 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 17/31] refs.c: make _update_reflog take an error argument Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 19/31] tests: move tests for -z update/delete/verify to after the ref is created Ronnie Sahlberg
` (12 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
If a previous transaction operation has failed and flagged a transaction
as having an error, then return immediately from commit and indicate failure.
Once we move checks from _commit to _update, _update will already have updated
the error string so the caller will know what failed. Thus we don't need to do
anything at all in _commit than return failure.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/refs.c b/refs.c
index 8b09258..93f01e8 100644
--- a/refs.c
+++ b/refs.c
@@ -3558,6 +3558,9 @@ int transaction_commit(struct ref_transaction *transaction,
int n = transaction->nr;
struct ref_update **updates = transaction->updates;
+ if (transaction->status == REF_TRANSACTION_ERROR)
+ return 1;
+
if (transaction->status != REF_TRANSACTION_OPEN)
die("BUG: commit on transaction that is not open");
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 19/31] tests: move tests for -z update/delete/verify to after the ref is created
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (17 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 18/31] refs.c: return immediately from _commit if the transaction has an error Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 20/31] refs.c: check for lock conflicts already in _update Ronnie Sahlberg
` (11 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
The tests for stdin -z fails for update/delete/verify are of the form
update refs/heads/a $m $m $m
and rely on the fact that when parsed we will first parse everything except
the trailing $m. And once that command has completed it will start parsing
the next command, find the trailing $m from before and fail because this
would not look like a proper command. I.e. we do not detect that there are
too many arguments so much as detect that the next command looks like
garbage.
But since the ref refs/heads/a does not exist at this point in the test we
actually have two failures that trigger. We have one error that is due to
update refs/heads/a $m $m would be invalid because the ref does not exist
and we have a second error that is due to the trailing $m causing a "too
many arguments". The current test depends on the order of check for these two
which makes the test fragile (and would break after the next patch).
Move these tests further down in the test to occur after we have created
refs/heads/a so that there is only one error condition in the test and that
we no longer depend on in which order the checks for error occur.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
t/t1400-update-ref.sh | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 4e2459a..f9b7bef 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -756,12 +756,6 @@ test_expect_success 'stdin -z fails update with no old value' '
grep "fatal: update $a: unexpected end of input when reading <oldvalue>" err
'
-test_expect_success 'stdin -z fails update with too many arguments' '
- printf $F "update $a" "$m" "$m" "$m" >stdin &&
- test_must_fail git update-ref -z --stdin <stdin 2>err &&
- grep "fatal: unknown command: $m" err
-'
-
test_expect_success 'stdin -z fails delete with no ref' '
printf $F "delete " >stdin &&
test_must_fail git update-ref -z --stdin <stdin 2>err &&
@@ -780,18 +774,6 @@ test_expect_success 'stdin -z fails delete with no old value' '
grep "fatal: delete $a: unexpected end of input when reading <oldvalue>" err
'
-test_expect_success 'stdin -z fails delete with too many arguments' '
- printf $F "delete $a" "$m" "$m" >stdin &&
- test_must_fail git update-ref -z --stdin <stdin 2>err &&
- grep "fatal: unknown command: $m" err
-'
-
-test_expect_success 'stdin -z fails verify with too many arguments' '
- printf $F "verify $a" "$m" "$m" >stdin &&
- test_must_fail git update-ref -z --stdin <stdin 2>err &&
- grep "fatal: unknown command: $m" err
-'
-
test_expect_success 'stdin -z fails verify with no old value' '
printf $F "verify $a" >stdin &&
test_must_fail git update-ref -z --stdin <stdin 2>err &&
@@ -818,6 +800,24 @@ test_expect_success 'stdin -z create ref works' '
test_cmp expect actual
'
+test_expect_success 'stdin -z fails update with too many arguments' '
+ printf $F "update $a" "$m" "$m" "$m" >stdin &&
+ test_must_fail git update-ref -z --stdin <stdin 2>err &&
+ grep "fatal: unknown command: $m" err
+'
+
+test_expect_success 'stdin -z fails delete with too many arguments' '
+ printf $F "delete $a" "$m" "$m" >stdin &&
+ test_must_fail git update-ref -z --stdin <stdin 2>err &&
+ grep "fatal: unknown command: $m" err
+'
+
+test_expect_success 'stdin -z fails verify with too many arguments' '
+ printf $F "verify $a" "$m" "$m" >stdin &&
+ test_must_fail git update-ref -z --stdin <stdin 2>err &&
+ grep "fatal: unknown command: $m" err
+'
+
test_expect_success 'stdin -z update ref creates with zero old value' '
printf $F "update $b" "$m" "$Z" >stdin &&
git update-ref -z --stdin <stdin &&
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 20/31] refs.c: check for lock conflicts already in _update
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (18 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 19/31] tests: move tests for -z update/delete/verify to after the ref is created Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 21/31] refs.c allow multiple updates of the same ref in a transaction Ronnie Sahlberg
` (10 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Check for lock conflicts in _update and flag the transaction as errored
instead of waiting until _commit for doing these checks. If there was a lock
failure we check if this was due to a previous update in the same transaction
or not. This so that we can preserve the current error messages on lock failure
and log "Multiple updates for ref '%s' not allowed." when we have multiple
updates in the same transaction and "Cannot lock the ref '%s'." for any other
reason that the lock failed.
This also means that we no longer need to sort all the refs and check for
duplicates during _commit so all that code can be removed too.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 85 +++++++++++++++++++++++++++++++++---------------------------------
1 file changed, 42 insertions(+), 43 deletions(-)
diff --git a/refs.c b/refs.c
index 93f01e8..76cab6e 100644
--- a/refs.c
+++ b/refs.c
@@ -3415,6 +3415,7 @@ int transaction_update_sha1(struct ref_transaction *transaction,
struct strbuf *err)
{
struct ref_update *update;
+ int i;
if (have_old && !old_sha1)
die("BUG: have_old is true but old_sha1 is NULL");
@@ -3438,19 +3439,49 @@ int transaction_update_sha1(struct ref_transaction *transaction,
* we do not need to check against this ref for name collissions
* during locking.
*/
- if (update->flags & REF_ISPACKONLY)
+ if (update->flags & REF_ISPACKONLY) {
add_delname(transaction, update->refname);
- else {
- update->lock = lock_ref_sha1_basic(update->refname,
- (update->have_old ?
- update->old_sha1 :
- NULL),
- update->flags,
- &update->type,
- transaction->delnames,
- transaction->delnum);
+ return 0;
}
- return 0;
+
+ update->lock = lock_ref_sha1_basic(update->refname,
+ (update->have_old ?
+ update->old_sha1 :
+ NULL),
+ update->flags,
+ &update->type,
+ transaction->delnames,
+ transaction->delnum);
+ if (update->lock)
+ return 0;
+
+ /* If we could not lock the ref it means we either collided with a
+ different command or that we tried to perform a second update to
+ the same ref from within the same transaction.
+ */
+ transaction->status = REF_TRANSACTION_ERROR;
+
+ /* -1 is the update we just added. Start at -2 and find the most recent
+ previous update for this ref.
+ */
+ for (i = transaction->nr - 2; i >= 0; i--) {
+ if (transaction->updates[i]->update_type != UPDATE_SHA1) {
+ continue;
+ }
+ if (!strcmp(transaction->updates[i]->refname,
+ update->refname))
+ break;
+ }
+ if (err)
+ if (i >= 0) {
+ const char *str =
+ "Multiple updates for ref '%s' not allowed.";
+ strbuf_addf(err, str, update->refname);
+ } else {
+ const char *str = "Cannot lock the ref '%s'.";
+ strbuf_addf(err, str, update->refname);
+ }
+ return 1;
}
int transaction_create_sha1(struct ref_transaction *transaction,
@@ -3525,32 +3556,6 @@ int update_ref(const char *action, const char *refname,
return 0;
}
-static int ref_update_compare(const void *r1, const void *r2)
-{
- const struct ref_update * const *u1 = r1;
- const struct ref_update * const *u2 = r2;
- return strcmp((*u1)->refname, (*u2)->refname);
-}
-
-static int ref_update_reject_duplicates(struct ref_update **updates, int n,
- struct strbuf *err)
-{
- int i;
- for (i = 1; i < n; i++) {
- if (updates[i]->update_type != UPDATE_SHA1)
- continue;
- if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
- const char *str =
- "Multiple updates for ref '%s' not allowed.";
- if (err)
- strbuf_addf(err, str, updates[i]->refname);
-
- return 1;
- }
- }
- return 0;
-}
-
int transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
@@ -3569,12 +3574,6 @@ int transaction_commit(struct ref_transaction *transaction,
return 0;
}
- /* Copy, sort, and reject duplicate refs */
- qsort(updates, n, sizeof(*updates), ref_update_compare);
- ret = ref_update_reject_duplicates(updates, n, err);
- if (ret)
- goto cleanup;
-
/* Acquire all ref locks while verifying old values */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 21/31] refs.c allow multiple updates of the same ref in a transaction
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (19 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 20/31] refs.c: check for lock conflicts already in _update Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 22/31] refs.c: release all remaining locks during transaction_free Ronnie Sahlberg
` (9 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Allow multiple updates of a ref in the same transaction as long as
each update has have_old and old_sha1 matches the new_sha1 of the
previous update.
Add a test that verifies that a valid sequence such as
create ref a
update ref b a
update ref c b
works and a test that an invalid sequence such as this still fails:
update ref a c
update ref b b
update ref c c
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 23 +++++++++++++++++------
t/t1400-update-ref.sh | 23 +++++++++++++++++++++--
2 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 76cab6e..87cdd91 100644
--- a/refs.c
+++ b/refs.c
@@ -3455,12 +3455,6 @@ int transaction_update_sha1(struct ref_transaction *transaction,
if (update->lock)
return 0;
- /* If we could not lock the ref it means we either collided with a
- different command or that we tried to perform a second update to
- the same ref from within the same transaction.
- */
- transaction->status = REF_TRANSACTION_ERROR;
-
/* -1 is the update we just added. Start at -2 and find the most recent
previous update for this ref.
*/
@@ -3472,6 +3466,23 @@ int transaction_update_sha1(struct ref_transaction *transaction,
update->refname))
break;
}
+ /* If the current update has_old==1 and old_sha1 matches the new_sha1
+ * of the previous update then merge the two updates into one.
+ */
+ if (i >= 0 && update->have_old && !hashcmp(update->old_sha1,
+ transaction->updates[i]->new_sha1)) {
+ hashcpy(transaction->updates[i]->new_sha1, update->new_sha1);
+ transaction->nr--;
+ free((char *)transaction->updates[transaction->nr]->msg);
+ free(transaction->updates[transaction->nr]);
+ return 0;
+ }
+ /* If we could not lock the ref it means we either collided with a
+ different command or that we tried to perform a second update to
+ the same ref from within the same transaction.
+ */
+ transaction->status = REF_TRANSACTION_ERROR;
+
if (err)
if (i >= 0) {
const char *str =
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f9b7bef..078cd4b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -446,7 +446,7 @@ test_expect_success 'stdin fails option with unknown name' '
grep "fatal: option unknown: unknown" err
'
-test_expect_success 'stdin fails with duplicate refs' '
+test_expect_success 'stdin fails with duplicate create refs' '
cat >stdin <<-EOF &&
create $a $m
create $b $m
@@ -464,6 +464,25 @@ test_expect_success 'stdin create ref works' '
test_cmp expect actual
'
+test_expect_success 'stdin succeeds with correctly chained update refs' '
+ cat >stdin <<-EOF &&
+ update $a $A $m
+ update $a $B $A
+ update $a $C $B
+ EOF
+ git update-ref --stdin <stdin
+'
+
+test_expect_success 'stdin fails with incorrectly chained update refs' '
+ cat >stdin <<-EOF &&
+ update $a $A $C
+ update $a $B $B
+ update $a $B $B
+ EOF
+ test_must_fail git update-ref --stdin <stdin &&
+ grep "fatal: Multiple updates for ref '"'"'$a'"'"' not allowed." err
+'
+
test_expect_success 'stdin succeeds with quoted argument' '
git update-ref -d $a &&
echo "create $a \"$m\"" >stdin &&
@@ -786,7 +805,7 @@ test_expect_success 'stdin -z fails option with unknown name' '
grep "fatal: option unknown: unknown" err
'
-test_expect_success 'stdin -z fails with duplicate refs' '
+test_expect_success 'stdin -z fails with duplicate create refs' '
printf $F "create $a" "$m" "create $b" "$m" "create $a" "$m" >stdin &&
test_must_fail git update-ref -z --stdin <stdin 2>err &&
grep "fatal: Multiple updates for ref '"'"'$a'"'"' not allowed." err
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 22/31] refs.c: release all remaining locks during transaction_free
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (20 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 21/31] refs.c allow multiple updates of the same ref in a transaction Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 23/31] reflog.c: use the existing transaction to also lock and update the ref Ronnie Sahlberg
` (8 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
If there are remaining locks held when we free a transaction, make sure that
we release them instead of relying on the atexit magic to release them for
us. Otherwise, if we perform two transactions on the same ref and the first
transaction is aborted then the second transaction will fail to lock the ref.
This can currently not trigger since there are no commands that use multiple
transactions.
This sequence illustrates the bug:
t = transaction_begin() /* first transaction */
transaction_update_sha1(t, ref, ...) /* ref is locked */
transaction_free(t)
t = transaction_begin() /* second transaction */
transaction_update_sha1(t, ref, ...) /* will fail since ref was locked above */
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 87cdd91..17feec5 100644
--- a/refs.c
+++ b/refs.c
@@ -3325,6 +3325,8 @@ void transaction_free(struct ref_transaction *transaction)
return;
for (i = 0; i < transaction->nr; i++) {
+ if (transaction->updates[i]->lock)
+ unlock_ref(transaction->updates[i]->lock);
free((char *)transaction->updates[i]->committer);
free((char *)transaction->updates[i]->msg);
free(transaction->updates[i]);
@@ -3729,8 +3731,10 @@ cleanup:
: REF_TRANSACTION_CLOSED;
for (i = 0; i < n; i++)
- if (updates[i]->lock)
+ if (updates[i]->lock) {
unlock_ref(updates[i]->lock);
+ updates[i]->lock = NULL;
+ }
return ret;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 23/31] reflog.c: use the existing transaction to also lock and update the ref
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (21 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 22/31] refs.c: release all remaining locks during transaction_free Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 24/31] refs.c: make unlock_ref static Ronnie Sahlberg
` (7 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
reflog.c has a transaction for updating the reflog. Use this transaction
also for locking the ref itself and updating the ref instead of
locking the file directly and writing to it. This now completely eliminates
the refs and reflog implementation from reflog.c.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/reflog.c | 44 ++++++++++++++++++++------------------------
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 2239249..78fd442 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -351,7 +351,6 @@ 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;
struct commit *tip_commit;
struct commit_list *tips;
int status = 0;
@@ -359,23 +358,24 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
memset(&cb, 0, sizeof(cb));
cb.refname = ref;
+ if (!reflog_exists(ref))
+ goto finish;
+
+ cb.t = transaction_begin();
/*
- * we take the lock for the ref itself to prevent it from
- * getting updated.
+ * This locks the ref itself, preventing it from getting
+ * updated.
*/
- lock = lock_any_ref_for_update(ref, sha1, 0, NULL);
- if (!lock)
+ if (transaction_update_sha1(cb.t, cb.refname,
+ sha1, sha1, 0, 1, NULL, NULL)) {
+ transaction_rollback(cb.t);
return error("cannot lock ref '%s'", ref);
- if (!reflog_exists(ref))
- goto finish;
- if (!cmd->dry_run) {
- cb.t = transaction_begin();
- transaction_update_reflog(cb.t, cb.refname,
- null_sha1, null_sha1, NULL,
- 0, 0, NULL,
- REFLOG_TRUNCATE,
- NULL);
}
+ transaction_update_reflog(cb.t, cb.refname,
+ null_sha1, null_sha1, NULL,
+ 0, 0, NULL,
+ REFLOG_TRUNCATE,
+ NULL);
cb.cmd = cmd;
@@ -421,23 +421,19 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
}
}
finish:
- if (cb.t) {
+ if (!cmd->dry_run) {
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)) {
+ transaction_update_sha1(cb.t, cb.refname,
+ cb.last_kept_sha1, sha1,
+ 0, 1, NULL, NULL)) {
status |= error("Couldn't write %s",
- lock->lk->filename);
+ cb.refname);
transaction_rollback(cb.t);
} else if (transaction_commit(cb.t, NULL)) {
status |= error("cannot commit reflog for %s", ref);
- } else if (cmd->updateref && commit_ref(lock)) {
- status |= error("Couldn't set %s", lock->ref_name);
}
- transaction_free(cb.t);
}
- unlock_ref(lock);
+ transaction_free(cb.t);
return status;
}
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 24/31] refs.c: make unlock_ref static
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (22 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 23/31] reflog.c: use the existing transaction to also lock and update the ref Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 25/31] refs.c: make close_ref static Ronnie Sahlberg
` (6 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We no longer need to export unlock_ref. Make the function static.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 20 ++++++++++----------
refs.h | 3 ---
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 17feec5..6933036 100644
--- a/refs.c
+++ b/refs.c
@@ -1931,6 +1931,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);
+}
+
static struct ref_lock *verify_lock(struct ref_lock *lock,
const unsigned char *old_sha1, int mustexist)
{
@@ -2674,16 +2684,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 6c628bb..47982ca 100644
--- a/refs.h
+++ b/refs.h
@@ -145,9 +145,6 @@ 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);
-
/** Setup reflog before using. **/
int log_ref_setup(const char *refname, char *logfile, int bufsize);
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 25/31] refs.c: make close_ref static
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (23 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 24/31] refs.c: make unlock_ref static Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 26/31] refs.c: make commit_ref static Ronnie Sahlberg
` (5 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We no longer need to export close_ref. Make the function static.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 2 +-
refs.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 6933036..a430b22 100644
--- a/refs.c
+++ b/refs.c
@@ -2668,7 +2668,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;
diff --git a/refs.h b/refs.h
index 47982ca..f211e13 100644
--- a/refs.h
+++ b/refs.h
@@ -139,9 +139,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);
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 26/31] refs.c: make commit_ref static
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (24 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 25/31] refs.c: make close_ref static Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 27/31] refs.c: remove the function lock_any_ref_for_update Ronnie Sahlberg
` (4 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We no longer need to export commit_ref. Make the function static.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 2 +-
refs.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index a430b22..0cb4ae8 100644
--- a/refs.c
+++ b/refs.c
@@ -2676,7 +2676,7 @@ static 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;
diff --git a/refs.h b/refs.h
index f211e13..289558c 100644
--- a/refs.h
+++ b/refs.h
@@ -139,9 +139,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p);
-/** Close and commit the ref locked by the lock */
-extern int commit_ref(struct ref_lock *lock);
-
/** Setup reflog before using. **/
int log_ref_setup(const char *refname, char *logfile, int bufsize);
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 27/31] refs.c: remove the function lock_any_ref_for_update
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (25 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 26/31] refs.c: make commit_ref static Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 28/31] refs.c: make struct ref_lock private to refs.c Ronnie Sahlberg
` (3 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We no longer need the function lock_any_ref_for_update so lets remove it.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 7 -------
refs.h | 4 ----
2 files changed, 11 deletions(-)
diff --git a/refs.c b/refs.c
index 0cb4ae8..918cda3 100644
--- a/refs.c
+++ b/refs.c
@@ -2167,13 +2167,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, flags, type_p, NULL, 0);
-}
-
/*
* 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 289558c..1e89187 100644
--- a/refs.h
+++ b/refs.h
@@ -135,10 +135,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
/** Locks any ref (for 'HEAD' type refs). */
#define REF_NODEREF 0x01
-extern struct ref_lock *lock_any_ref_for_update(const char *refname,
- const unsigned char *old_sha1,
- int flags, int *type_p);
-
/** Setup reflog before using. **/
int log_ref_setup(const char *refname, char *logfile, int bufsize);
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 28/31] refs.c: make struct ref_lock private to refs.c
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (26 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 27/31] refs.c: remove the function lock_any_ref_for_update Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 29/31] refs.c: allow passing raw git_committer_info as email to _update_reflog Ronnie Sahlberg
` (2 subsequent siblings)
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We no longer need to expose the ref_lock structure to any external callers.
Make it private to refs.c.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 9 +++++++++
refs.h | 9 ---------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 918cda3..8129b20 100644
--- a/refs.c
+++ b/refs.c
@@ -5,6 +5,15 @@
#include "dir.h"
#include "string-list.h"
+struct ref_lock {
+ char *ref_name;
+ char *orig_ref_name;
+ struct lock_file *lk;
+ unsigned char old_sha1[20];
+ int lock_fd;
+ int force_write;
+};
+
/*
* Make sure "ref" is something reasonable to have under ".git/refs/";
* We do not like it if:
diff --git a/refs.h b/refs.h
index 1e89187..558a711 100644
--- a/refs.h
+++ b/refs.h
@@ -1,15 +1,6 @@
#ifndef REFS_H
#define REFS_H
-struct ref_lock {
- char *ref_name;
- char *orig_ref_name;
- struct lock_file *lk;
- unsigned char old_sha1[20];
- int lock_fd;
- int force_write;
-};
-
struct ref_transaction;
/*
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 29/31] refs.c: allow passing raw git_committer_info as email to _update_reflog
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (27 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 28/31] refs.c: make struct ref_lock private to refs.c Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 30/31] refs.c: move ref_update and other definitions to earlier in the file Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 31/31] refs.c: use the transaction to manage the reflog in rename_refs Ronnie Sahlberg
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
In many places in the code we do not have access to the individual fields
in the committer data. Instead we might only have access to prebaked data
such as what is returned by git_committer_info() containing a string
that consists of email, timestamp, zone etc.
This makes it inconvenient to use transaction_update_reflog since it means you
would have to first parse git_committer_info before you can call update_reflog.
Add a new flag REFLOG_EMAIL_IS_COMMITTER to _update_reflog to tell it
that what we pass in as email is already the fully baked committer string
we can use as-is.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Conflicts:
refs.c
---
refs.c | 20 ++++++++++++--------
refs.h | 1 +
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 8129b20..46a31c1 100644
--- a/refs.c
+++ b/refs.c
@@ -3396,14 +3396,18 @@ int transaction_update_reflog(struct ref_transaction *transaction,
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 (flags & REFLOG_EMAIL_IS_COMMITTER)
+ update->committer = xstrdup(email);
+ else {
+ 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);
diff --git a/refs.h b/refs.h
index 558a711..933cae7 100644
--- a/refs.h
+++ b/refs.h
@@ -251,6 +251,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
struct strbuf *err);
#define REFLOG_TRUNCATE 0x01
+#define REFLOG_EMAIL_IS_COMMITTER 0x02
/*
* Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
* this update will first truncate the reflog before writing the entry.
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 30/31] refs.c: move ref_update and other definitions to earlier in the file
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (28 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 29/31] refs.c: allow passing raw git_committer_info as email to _update_reflog Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 31/31] refs.c: use the transaction to manage the reflog in rename_refs Ronnie Sahlberg
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
Move the struct ref_update definition and some enums to earlier in the file.
This does not change any actual code or definitions but just moves them
to a different place in the code since we will soon reference some of these
from the rename_ref function.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Conflicts:
refs.c
---
refs.c | 61 ++++++++++++++++++++++++++++++-------------------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/refs.c b/refs.c
index 46a31c1..bcce8fe 100644
--- a/refs.c
+++ b/refs.c
@@ -2603,6 +2603,36 @@ static int rename_tmp_log(const char *newrefname)
return 0;
}
+enum transaction_update_type {
+ UPDATE_SHA1 = 0,
+ 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
+ * 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? */
+ int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
+ struct ref_lock *lock;
+ int type;
+ const char *msg;
+
+ /* used by reflog updates */
+ int reflog_fd;
+ struct lock_file *reflog_lock;
+ const char *committer;
+ struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */
+
+ const char refname[FLEX_ARRAY];
+};
+
int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
{
unsigned char sha1[20];
@@ -3254,37 +3284,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-enum transaction_update_type {
- UPDATE_SHA1 = 0,
- 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
- * 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? or private flags */
- int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
- struct ref_lock *lock;
- int type;
- const char *msg;
-
- /* used by reflog updates */
- int reflog_fd;
- struct lock_file *reflog_lock;
- const char *committer;
- struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */
-
- const char refname[FLEX_ARRAY];
-};
-
enum ref_transaction_status {
REF_TRANSACTION_OPEN = 0,
REF_TRANSACTION_CLOSED = 1,
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 31/31] refs.c: use the transaction to manage the reflog in rename_refs
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
` (29 preceding siblings ...)
2014-05-14 22:13 ` [PATCH 30/31] refs.c: move ref_update and other definitions to earlier in the file Ronnie Sahlberg
@ 2014-05-14 22:13 ` Ronnie Sahlberg
30 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-14 22:13 UTC (permalink / raw)
To: git; +Cc: mhagger, Ronnie Sahlberg
We have a transaction in rename_ref to managing the ref during the rename.
Use this transaction to manage the changes to the reflog too.
This now means that rename_ref is almost completely agnostic about how refs
storage is implemented and should be easier to migrate if/when we allow a
different store for refs, such as a database.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
refs.c | 62 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index bcce8fe..829c89a 100644
--- a/refs.c
+++ b/refs.c
@@ -2633,6 +2633,24 @@ struct ref_update {
const char refname[FLEX_ARRAY];
};
+struct rename_reflog_cb {
+ struct ref_transaction *t;
+ const char *refname;
+};
+
+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct rename_reflog_cb *cb = cb_data;
+
+ transaction_update_reflog(cb->t, cb->refname, nsha1, osha1,
+ email, timestamp, tz, message, 0,
+ NULL);
+
+ return 0;
+}
+
int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
{
unsigned char sha1[20];
@@ -2664,40 +2682,44 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
get_loose_refs(&ref_cache), NULL, 0))
return 1;
- if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG)))
- return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
- oldrefname, strerror(errno));
-
if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
return error("unable to pack refs");
transaction = transaction_begin();
- if (!transaction ||
- transaction_delete_sha1(transaction, oldrefname, sha1,
+ if (log) {
+ struct rename_reflog_cb cb;
+
+ transaction_update_reflog(transaction, newrefname,
+ null_sha1, null_sha1, NULL,
+ 0, 0, NULL,
+ REFLOG_TRUNCATE,
+ NULL);
+
+ cb.t = transaction;
+ cb.refname = newrefname;
+ for_each_reflog_ent(oldrefname, rename_reflog_ent, &cb);
+
+ transaction_update_reflog(transaction, newrefname,
+ sha1, sha1,
+ git_committer_info(0),
+ 0, 0, logmsg,
+ REFLOG_EMAIL_IS_COMMITTER,
+ &err);
+ }
+
+ if (transaction_delete_sha1(transaction, oldrefname, sha1,
REF_NODEREF | REF_ISPACKONLY,
1, NULL, &err) ||
transaction_update_sha1(transaction, newrefname, sha1,
- NULL, 0, 0, logmsg, &err) ||
+ NULL, 0, 0, NULL, &err) ||
transaction_commit(transaction, &err)) {
transaction_rollback(transaction);
error("rename_ref failed: %s", err.buf);
strbuf_release(&err);
- goto rollbacklog;
+ return 1;
}
transaction_free(transaction);
-
- if (log && rename_tmp_log(newrefname))
- goto rollbacklog;
-
return 0;
-
- rollbacklog:
- if (log &&
- rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname)))
- error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
- oldrefname, strerror(errno));
-
- return 1;
}
static int close_ref(struct ref_lock *lock)
--
2.0.0.rc3.506.g3739a35
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 03/31] refs.c: rename the transaction functions
2014-05-14 22:13 ` [PATCH 03/31] refs.c: rename the transaction functions Ronnie Sahlberg
@ 2014-05-16 21:15 ` Junio C Hamano
2014-05-19 23:11 ` Ronnie Sahlberg
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2014-05-16 21:15 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git, mhagger
Ronnie Sahlberg <sahlberg@google.com> writes:
> Rename the transaction functions. Remove the leading ref_ from the names
> and append _sha1 to the names for functions that create/delete/update sha1
> refs.
> ...
> - transaction = ref_transaction_begin();
> + transaction = transaction_begin();
Why? Do we forsee that there will never be other kinds of
transaction, and whenever we say a "transaction" that will be always
about updating the refs and nothing else?
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log
2014-05-14 22:13 ` [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log Ronnie Sahlberg
@ 2014-05-16 21:20 ` Junio C Hamano
2014-05-19 23:27 ` Ronnie Sahlberg
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2014-05-16 21:20 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git, mhagger
Ronnie Sahlberg <sahlberg@google.com> writes:
> Add a flag that allows us to truncate the reflog before we write the update.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> ---
Until we read the callers it is hard to see how such a feature is
useful, though.
(style) The two multi-line comments are formatted differently.
> refs.c | 12 ++++++++++--
> refs.h | 4 +++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index d8a1568..a8b583a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_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];
>
> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_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");
> + }
The {} looks somewhat curious here. If you seeked to the beginning,
and failed to truncate, do you still want to go on without "return"
in front of the error()? Would that overwrite existing entries?
> 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 2e22a26..b1b97e7 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
> const unsigned char *old_sha1,
> int flags, int have_old, const char *msg);
>
> +#define REFLOG_TRUNCATE 0x01
> /*
> - * Append a reflog entry for refname.
> + * 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 ref_transaction *transaction,
> const char *refname,
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL
2014-05-14 22:13 ` [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL Ronnie Sahlberg
@ 2014-05-16 21:24 ` Junio C Hamano
2014-05-19 22:55 ` Ronnie Sahlberg
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2014-05-16 21:24 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git, mhagger
Ronnie Sahlberg <sahlberg@google.com> writes:
> 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.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> ---
> refs.c | 8 +++++---
> refs.h | 1 +
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index a8b583a..a3f60ad 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,
> ftruncate(update->reflog_fd, 0)) {
> error("Could not truncate reflog");
> }
> - if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
> - update->new_sha1,
> - update->committer, update->msg)) {
> + if (update->msg && log_ref_write_fd(update->reflog_fd,
> + update->old_sha1,
> + update->new_sha1,
> + update->committer,
> + update->msg)) {
Wouldn't it make it easier to read if you chopped immediately after
the &&, i.e. chopping at a gap at a higher-level in the parse tree?
if (update->msg &&
log_ref_write_fd(update->reflog_fd,
update->old_sha1, update->old_sha1,
update->committer, update->msg)) {
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction
2014-05-14 22:13 ` [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction Ronnie Sahlberg
@ 2014-05-16 21:35 ` Junio C Hamano
2014-05-16 22:01 ` Eric Sunshine
2014-05-19 23:06 ` Ronnie Sahlberg
0 siblings, 2 replies; 43+ messages in thread
From: Junio C Hamano @ 2014-05-16 21:35 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git, mhagger
Ronnie Sahlberg <sahlberg@google.com> writes:
> 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-somehting...
> transaction_reflog_update(t, "foo", 0, <message>);
> transaction_commit(t)
OK, so you are now doing not just "refs" but also "reflogs", you
felt that "ref_transaction()" does not cover the latter. Is that
the reason for the rename in the earlier step?
I am sort-of on the fence.
Calling the begin "ref_transaction_begin" and then calling the new
function "ref_transaction_log_update" would still allow us to
differentiate transactions on refs and reflogs, while allowing other
kinds of transactions that are not related to refs at all to employ
a mechanism that is different from the one that is used to implement
the transactions on refs and reflogs you are building here.
But I think I am OK with the generic "transaction-begin" now.
Having one mechanism for refs and reflogs, and then having another
completely different mechanism for other things, will not let us
coordinate between the two easily, so "allow transactions that are
not related to refs at all to be built on a different mechanism" may
not be a worthwhile goal to pursue in the first place. Please
consider the question on the naming in the earlier one dropped.
>
> where we first truncate the reflog and then build the new content one line at a
> time.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> ---
> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 49 insertions(+), 9 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index a3f60ad..e7ede03 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
> * need to lock the loose ref during the transaction.
> */
> #define REF_ISPACKONLY 0x0200
> +/** Only the first reflog update needs to lock the reflog file. Further updates
> + * just use the lock taken by the first update.
> + */
Style.
> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,
> int flags)
> {
> struct ref_update *update;
> + int i;
>
> 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));
So with two calls to transaction-update-reflog, we make two calls to
add-update, and each holds a separate lock? If we write two entries
to record two updates of the same ref, would we still want to do so?
> + /* Rollback any reflog files that are still open */
> + for (i = 0; i < n; i++) {
> + struct ref_update *update = updates[i];
> +
> + if (update->update_type != UPDATE_LOG)
> + continue;
> + if (update->flags & UPDATE_REFLOG_NOLOCK)
> + continue;
> + if (update->reflog_fd == -1)
> + continue;
> + rollback_lock_file(update->reflog_lock);
> + }
> transaction->status = ret ? REF_TRANSACTION_ERROR
> : REF_TRANSACTION_CLOSED;
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction
2014-05-16 21:35 ` Junio C Hamano
@ 2014-05-16 22:01 ` Eric Sunshine
2014-05-19 22:58 ` Ronnie Sahlberg
2014-05-19 23:06 ` Ronnie Sahlberg
1 sibling, 1 reply; 43+ messages in thread
From: Eric Sunshine @ 2014-05-16 22:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ronnie Sahlberg, Git List, Michael Haggerty
On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> 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-somehting...
>> transaction_reflog_update(t, "foo", 0, <message>);
>> transaction_commit(t)
>
> OK, so you are now doing not just "refs" but also "reflogs", you
> felt that "ref_transaction()" does not cover the latter. Is that
> the reason for the rename in the earlier step?
>
> I am sort-of on the fence.
>
> Calling the begin "ref_transaction_begin" and then calling the new
> function "ref_transaction_log_update" would still allow us to
> differentiate transactions on refs and reflogs, while allowing other
> kinds of transactions that are not related to refs at all to employ
> a mechanism that is different from the one that is used to implement
> the transactions on refs and reflogs you are building here.
>
> But I think I am OK with the generic "transaction-begin" now.
> Having one mechanism for refs and reflogs, and then having another
> completely different mechanism for other things, will not let us
> coordinate between the two easily, so "allow transactions that are
> not related to refs at all to be built on a different mechanism" may
> not be a worthwhile goal to pursue in the first place. Please
> consider the question on the naming in the earlier one dropped.
>
>>
>> where we first truncate the reflog and then build the new content one line at a
>> time.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index a3f60ad..e7ede03 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
>> * need to lock the loose ref during the transaction.
>> */
>> #define REF_ISPACKONLY 0x0200
>> +/** Only the first reflog update needs to lock the reflog file. Further updates
>> + * just use the lock taken by the first update.
>> + */
>
> Style.
>
>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,
>> int flags)
>> {
>> struct ref_update *update;
>> + int i;
>>
>> 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));
>
> So with two calls to transaction-update-reflog, we make two calls to
> add-update, and each holds a separate lock? If we write two entries
> to record two updates of the same ref, would we still want to do so?
Also, indent with tabs rather than spaces (the line following the 'if').
>> + /* Rollback any reflog files that are still open */
>> + for (i = 0; i < n; i++) {
>> + struct ref_update *update = updates[i];
>> +
>> + if (update->update_type != UPDATE_LOG)
>> + continue;
>> + if (update->flags & UPDATE_REFLOG_NOLOCK)
>> + continue;
>> + if (update->reflog_fd == -1)
>> + continue;
>> + rollback_lock_file(update->reflog_lock);
>> + }
>> transaction->status = ret ? REF_TRANSACTION_ERROR
>> : REF_TRANSACTION_CLOSED;
Indent with tabs.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL
2014-05-16 21:24 ` Junio C Hamano
@ 2014-05-19 22:55 ` Ronnie Sahlberg
0 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-19 22:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
On Fri, May 16, 2014 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> 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.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>> refs.c | 8 +++++---
>> refs.h | 1 +
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index a8b583a..a3f60ad 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,
>> ftruncate(update->reflog_fd, 0)) {
>> error("Could not truncate reflog");
>> }
>> - if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
>> - update->new_sha1,
>> - update->committer, update->msg)) {
>> + if (update->msg && log_ref_write_fd(update->reflog_fd,
>> + update->old_sha1,
>> + update->new_sha1,
>> + update->committer,
>> + update->msg)) {
>
> Wouldn't it make it easier to read if you chopped immediately after
> the &&, i.e. chopping at a gap at a higher-level in the parse tree?
Yepp it does.
Changed. Thanks.
>
> if (update->msg &&
> log_ref_write_fd(update->reflog_fd,
> update->old_sha1, update->old_sha1,
> update->committer, update->msg)) {
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction
2014-05-16 22:01 ` Eric Sunshine
@ 2014-05-19 22:58 ` Ronnie Sahlberg
0 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-19 22:58 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, Git List, Michael Haggerty
On Fri, May 16, 2014 at 3:01 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ronnie Sahlberg <sahlberg@google.com> writes:
>>
>>> 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-somehting...
>>> transaction_reflog_update(t, "foo", 0, <message>);
>>> transaction_commit(t)
>>
>> OK, so you are now doing not just "refs" but also "reflogs", you
>> felt that "ref_transaction()" does not cover the latter. Is that
>> the reason for the rename in the earlier step?
>>
>> I am sort-of on the fence.
>>
>> Calling the begin "ref_transaction_begin" and then calling the new
>> function "ref_transaction_log_update" would still allow us to
>> differentiate transactions on refs and reflogs, while allowing other
>> kinds of transactions that are not related to refs at all to employ
>> a mechanism that is different from the one that is used to implement
>> the transactions on refs and reflogs you are building here.
>>
>> But I think I am OK with the generic "transaction-begin" now.
>> Having one mechanism for refs and reflogs, and then having another
>> completely different mechanism for other things, will not let us
>> coordinate between the two easily, so "allow transactions that are
>> not related to refs at all to be built on a different mechanism" may
>> not be a worthwhile goal to pursue in the first place. Please
>> consider the question on the naming in the earlier one dropped.
>>
>>>
>>> where we first truncate the reflog and then build the new content one line at a
>>> time.
>>>
>>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>>> ---
>>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>>> 1 file changed, 49 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/refs.c b/refs.c
>>> index a3f60ad..e7ede03 100644
>>> --- a/refs.c
>>> +++ b/refs.c
>>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
>>> * need to lock the loose ref during the transaction.
>>> */
>>> #define REF_ISPACKONLY 0x0200
>>> +/** Only the first reflog update needs to lock the reflog file. Further updates
>>> + * just use the lock taken by the first update.
>>> + */
>>
>> Style.
>>
>>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,
>>> int flags)
>>> {
>>> struct ref_update *update;
>>> + int i;
>>>
>>> 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));
>>
>> So with two calls to transaction-update-reflog, we make two calls to
>> add-update, and each holds a separate lock? If we write two entries
>> to record two updates of the same ref, would we still want to do so?
>
> Also, indent with tabs rather than spaces (the line following the 'if').
Done.
>
>>> + /* Rollback any reflog files that are still open */
>>> + for (i = 0; i < n; i++) {
>>> + struct ref_update *update = updates[i];
>>> +
>>> + if (update->update_type != UPDATE_LOG)
>>> + continue;
>>> + if (update->flags & UPDATE_REFLOG_NOLOCK)
>>> + continue;
>>> + if (update->reflog_fd == -1)
>>> + continue;
>>> + rollback_lock_file(update->reflog_lock);
>>> + }
>>> transaction->status = ret ? REF_TRANSACTION_ERROR
>>> : REF_TRANSACTION_CLOSED;
>
> Indent with tabs.
Done. Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction
2014-05-16 21:35 ` Junio C Hamano
2014-05-16 22:01 ` Eric Sunshine
@ 2014-05-19 23:06 ` Ronnie Sahlberg
1 sibling, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-19 23:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
On Fri, May 16, 2014 at 2:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> 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-somehting...
>> transaction_reflog_update(t, "foo", 0, <message>);
>> transaction_commit(t)
>
> OK, so you are now doing not just "refs" but also "reflogs", you
> felt that "ref_transaction()" does not cover the latter. Is that
> the reason for the rename in the earlier step?
>
> I am sort-of on the fence.
>
> Calling the begin "ref_transaction_begin" and then calling the new
> function "ref_transaction_log_update" would still allow us to
> differentiate transactions on refs and reflogs, while allowing other
> kinds of transactions that are not related to refs at all to employ
> a mechanism that is different from the one that is used to implement
> the transactions on refs and reflogs you are building here.
>
> But I think I am OK with the generic "transaction-begin" now.
> Having one mechanism for refs and reflogs, and then having another
> completely different mechanism for other things, will not let us
> coordinate between the two easily, so "allow transactions that are
> not related to refs at all to be built on a different mechanism" may
> not be a worthwhile goal to pursue in the first place. Please
> consider the question on the naming in the earlier one dropped.
>
>>
>> where we first truncate the reflog and then build the new content one line at a
>> time.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index a3f60ad..e7ede03 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
>> * need to lock the loose ref during the transaction.
>> */
>> #define REF_ISPACKONLY 0x0200
>> +/** Only the first reflog update needs to lock the reflog file. Further updates
>> + * just use the lock taken by the first update.
>> + */
>
> Style.
Done.
>
>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,
>> int flags)
>> {
>> struct ref_update *update;
>> + int i;
>>
>> 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));
>
> So with two calls to transaction-update-reflog, we make two calls to
> add-update, and each holds a separate lock? If we write two entries
> to record two updates of the same ref, would we still want to do so?
If we call transaction_update_reflog twice we will call add_update
twice since we are doing two updates.
But if the two calls are for the same ref then we only lock the reflog
for the first update and then in the second update we re-use the lock
structure from the first.
Later updates to reflog.c will show how to loop over the old reflog,
using the existing iterator, and then create a new expired reflog one
entry at a time.
>
>> + /* Rollback any reflog files that are still open */
>> + for (i = 0; i < n; i++) {
>> + struct ref_update *update = updates[i];
>> +
>> + if (update->update_type != UPDATE_LOG)
>> + continue;
>> + if (update->flags & UPDATE_REFLOG_NOLOCK)
>> + continue;
>> + if (update->reflog_fd == -1)
>> + continue;
>> + rollback_lock_file(update->reflog_lock);
>> + }
>> transaction->status = ret ? REF_TRANSACTION_ERROR
>> : REF_TRANSACTION_CLOSED;
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/31] refs.c: rename the transaction functions
2014-05-16 21:15 ` Junio C Hamano
@ 2014-05-19 23:11 ` Ronnie Sahlberg
2014-05-19 23:25 ` Junio C Hamano
0 siblings, 1 reply; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-19 23:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
On Fri, May 16, 2014 at 2:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> Rename the transaction functions. Remove the leading ref_ from the names
>> and append _sha1 to the names for functions that create/delete/update sha1
>> refs.
>> ...
>> - transaction = ref_transaction_begin();
>> + transaction = transaction_begin();
>
> Why? Do we forsee that there will never be other kinds of
> transaction, and whenever we say a "transaction" that will be always
> about updating the refs and nothing else?
ref_transaction_... is a bit of a mouthful.
I think initially we will only need a transaction framework for normal
sha1 refs, for symrefs and for reflog updates and these should all be
handled by the same transaction system so a single
_begin/_update*/_commit should be able to atomically commit changes
that involve updates to all three types.
I am not sure if we need transactions for other types of data, such as
sha1 objects, but if it turns out we do in the future we can rename
these functions again.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/31] refs.c: rename the transaction functions
2014-05-19 23:11 ` Ronnie Sahlberg
@ 2014-05-19 23:25 ` Junio C Hamano
0 siblings, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2014-05-19 23:25 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git, Michael Haggerty
Ronnie Sahlberg <sahlberg@google.com> writes:
> I am not sure if we need transactions for other types of data, such as
> sha1 objects, but if it turns out we do in the future we can rename
> these functions again.
I was wrong (and I think you read it in the later patch review).
If we need transaction for other types of data, and we will
eventually need to coordinate the transaction semantics over refs
and those other types of data. It would be far cleaner to express
that coordination within the same transaction framework.
In other words, we do not want to be in a situation like this:
other_transaction_begin();
ref_transaction_begin();
ref_transaction_update();
other_transaction_update();
ref_transaction_commit();
if (other_transaction_commit() != SUCCESS) {
... oops it is too late to roll back the ref_transaction ...
other_transaction_rollback();
}
and force us doing 3-phase commit inside ourselves to work it
around.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log
2014-05-16 21:20 ` Junio C Hamano
@ 2014-05-19 23:27 ` Ronnie Sahlberg
0 siblings, 0 replies; 43+ messages in thread
From: Ronnie Sahlberg @ 2014-05-19 23:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
On Fri, May 16, 2014 at 2:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> Add a flag that allows us to truncate the reflog before we write the update.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>
> Until we read the callers it is hard to see how such a feature is
> useful, though.
Shortly later in the series comes the update to builtin/reflog.c which
uses this feature.
Even later in the series I also use this in builtin/reflog.c to both
do the ref update as well as the corresponding reflog updates all in
one single transaction.
>
> (style) The two multi-line comments are formatted differently.
Fixed. Thanks.
>
>> refs.c | 12 ++++++++++--
>> refs.h | 4 +++-
>> 2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index d8a1568..a8b583a 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_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];
>>
>> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_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");
>> + }
>
> The {} looks somewhat curious here. If you seeked to the beginning,
> and failed to truncate, do you still want to go on without "return"
> in front of the error()? Would that overwrite existing entries?
I have changed it to do this :
+ 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",
+ update->refname);
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ }
At this point we have already committed all the changes to refs and
also performed all unlinks for refs we are deleting. So if things
start to go wrong here during the reflog updates it is too late to do
much more than complain.
>
>> 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 2e22a26..b1b97e7 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
>> const unsigned char *old_sha1,
>> int flags, int have_old, const char *msg);
>>
>> +#define REFLOG_TRUNCATE 0x01
>> /*
>> - * Append a reflog entry for refname.
>> + * 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 ref_transaction *transaction,
>> const char *refname,
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2014-05-19 23:27 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-14 22:12 [PATCH 00/31] Finish implementing ref and reflog transactions Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 02/31] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 03/31] refs.c: rename the transaction functions Ronnie Sahlberg
2014-05-16 21:15 ` Junio C Hamano
2014-05-19 23:11 ` Ronnie Sahlberg
2014-05-19 23:25 ` Junio C Hamano
2014-05-14 22:13 ` [PATCH 04/31] refs.c: add a new update_type field to ref_update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 05/31] refs.c: add a function to append a reflog entry to a fd Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 06/31] refs.c: add a transaction function to append a reflog entry Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log Ronnie Sahlberg
2014-05-16 21:20 ` Junio C Hamano
2014-05-19 23:27 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL Ronnie Sahlberg
2014-05-16 21:24 ` Junio C Hamano
2014-05-19 22:55 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction Ronnie Sahlberg
2014-05-16 21:35 ` Junio C Hamano
2014-05-16 22:01 ` Eric Sunshine
2014-05-19 22:58 ` Ronnie Sahlberg
2014-05-19 23:06 ` Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 10/31] reflog.c: use a reflog transaction when writing during expire Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 11/31] refs.c: null terminate the string in copy_msg Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 12/31] refs.c: track the refnames we are deleting in the transaction structure Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 13/31] refs.c: update the list of deleted refs during _update instead of _commit Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 14/31] refs.c: return error instead of dying when locking fails during transaction Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 15/31] refs.c: lock the ref during _update instead of during _commit Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 16/31] refs.c: add an error argument to create/delete/update just like commit Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 17/31] refs.c: make _update_reflog take an error argument Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 18/31] refs.c: return immediately from _commit if the transaction has an error Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 19/31] tests: move tests for -z update/delete/verify to after the ref is created Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 20/31] refs.c: check for lock conflicts already in _update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 21/31] refs.c allow multiple updates of the same ref in a transaction Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 22/31] refs.c: release all remaining locks during transaction_free Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 23/31] reflog.c: use the existing transaction to also lock and update the ref Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 24/31] refs.c: make unlock_ref static Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 25/31] refs.c: make close_ref static Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 26/31] refs.c: make commit_ref static Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 27/31] refs.c: remove the function lock_any_ref_for_update Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 28/31] refs.c: make struct ref_lock private to refs.c Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 29/31] refs.c: allow passing raw git_committer_info as email to _update_reflog Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 30/31] refs.c: move ref_update and other definitions to earlier in the file Ronnie Sahlberg
2014-05-14 22:13 ` [PATCH 31/31] refs.c: use the transaction to manage the reflog in rename_refs Ronnie Sahlberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).