{"thread":{"id":"38171","subject":"[PATCH v2 03/24] refs.c: add a function to append a reflog entry to a fd","startedAt":"2014-12-12T08:56:39Z","lastAt":"2014-12-12T20:58:30Z","messageCount":31,"participants":["Michael Haggerty","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":24},"messages":[{"id":"253626","messageId":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":null,"subject":"[PATCH v2 00/24] Add reflog_expire() to the references API","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:39Z","receivedAt":"2014-12-12T08:56:39Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is v2 of the series. Thanks to Jonathan, Stefan, Junio, and\nRonnie for their feedback on v1 [1]. I think I have addressed all of\nthe issues that were raised.\n\nChanges since v1:\n\n* Several improvements to commit messages and comments; added some\n Reviewed-by comments from the mailing list.\n\n* Change the signature of expire_reflog() early in the series to cast\n off its heritage as an each_ref_fn.\n\n* Move the static lock_file object into expire_reflog(), and explain\n the locking policy better.\n\n* Report errors if hold_lock_file_for_update() or fdopen_lock_file()\n fails.\n\n* Fix the capitalization in some error messages.\n\n* When \"enum expire_reflog_flags\" is first introduced, put its\n definition earlier in the file so that a later patch in the series\n doesn't have to move it.\n\n* Rename reflog_expiry_select_fn to reflog_expiry_should_prune_fn.\n\n* Append Stefan's patch 24/24 \"refs.c: let fprintf handle the\n formatting\"\n\nThis branch is also available on GitHub:\n\n https://github.com/mhagger/git.git, branch reflog-expire-api-v2\n\nMichael\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/260812\n\nMichael Haggerty (18):\n expire_reflog(): it's not an each_ref_fn anymore\n expire_reflog(): rename \"ref\" parameter to \"refname\"\n expire_reflog(): return early if the reference has no reflog\n expire_reflog(): use a lock_file for rewriting the reflog file\n Extract function should_expire_reflog_ent()\n expire_reflog(): extract two policy-related functions\n expire_reflog(): add a \"flags\" argument\n expire_reflog(): move dry_run to flags argument\n expire_reflog(): move updateref to flags argument\n Rename expire_reflog_cb to expire_reflog_policy_cb\n struct expire_reflog_cb: a new callback data type\n expire_reflog(): pass flags through to expire_reflog_ent()\n expire_reflog(): move verbose to flags argument\n expire_reflog(): move rewrite to flags argument\n Move newlog and last_kept_sha1 to \"struct expire_reflog_cb\"\n expire_reflog(): treat the policy callback data as opaque\n reflog_expire(): new function in the reference API\n lock_any_ref_for_update(): inline function\n\nRonnie Sahlberg (4):\n refs.c: make ref_transaction_create a wrapper for\n ref_transaction_update\n refs.c: make ref_transaction_delete a wrapper for\n ref_transaction_update\n refs.c: add a function to append a reflog entry to a fd\n refs.c: remove unlock_ref/close_ref/commit_ref from the refs api\n\nStefan Beller (2):\n refs.c: don't expose the internal struct ref_lock in the header file\n refs.c: let fprintf handle the formatting\n\n builtin/reflog.c | 259 ++++++++++++++++++++++--------------------------------\n refs.c | 263 ++++++++++++++++++++++++++++++++++++++-----------------\n refs.h | 75 ++++++++++------\n 3 files changed, 332 insertions(+), 265 deletions(-)\n\n-- \n2.1.3\n"},{"id":"253609","messageId":"1418374623-5566-2-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 01/24] refs.c: make ref_transaction_create a wrapper for ref_transaction_update","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:40Z","receivedAt":"2014-12-12T08:56:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nThe ref_transaction_update function can already be used to create refs by\npassing null_sha1 as the old_sha1 parameter. Simplify by replacing\ntransaction_create with a thin wrapper.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Stefan Beller \nReviewed-by: Michael Haggerty \nReviewed-by: Jonathan Nieder \nSigned-off-by: Michael Haggerty \n---\n refs.c | 27 ++-------------------------\n 1 file changed, 2 insertions(+), 25 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5ff457e..005eb18 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3623,31 +3623,8 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t int flags, const char *msg,\n \t\t\t struct strbuf *err)\n {\n-\tstruct ref_update *update;\n-\n-\tassert(err);\n-\n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n-\t\tdie(\"BUG: create called for transaction that is not open\");\n-\n-\tif (!new_sha1 || is_null_sha1(new_sha1))\n-\t\tdie(\"BUG: create ref with null new_sha1\");\n-\n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\tstrbuf_addf(err, \"refusing to create ref with bad name %s\",\n-\t\t\t refname);\n-\t\treturn -1;\n-\t}\n-\n-\tupdate = add_update(transaction, refname);\n-\n-\thashcpy(update->new_sha1, new_sha1);\n-\thashclr(update->old_sha1);\n-\tupdate->flags = flags;\n-\tupdate->have_old = 1;\n-\tif (msg)\n-\t\tupdate->msg = xstrdup(msg);\n-\treturn 0;\n+\treturn ref_transaction_update(transaction, refname, new_sha1,\n+\t\t\t\t null_sha1, flags, 1, msg, err);\n }\n \n int ref_transaction_delete(struct ref_transaction *transaction,\n-- \n2.1.3\n"},{"id":"253630","messageId":"1418374623-5566-3-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 02/24] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:41Z","receivedAt":"2014-12-12T08:56:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Stefan Beller \nReviewed-by: Michael Haggerty \nReviewed-by: Jonathan Nieder \nSigned-off-by: Michael Haggerty \n---\n refs.c | 22 ++--------------------\n refs.h | 2 +-\n 2 files changed, 3 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 005eb18..05cb299 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3633,26 +3633,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t int flags, int have_old, const char *msg,\n \t\t\t struct strbuf *err)\n {\n-\tstruct ref_update *update;\n-\n-\tassert(err);\n-\n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n-\t\tdie(\"BUG: delete called for transaction that is not open\");\n-\n-\tif (have_old && !old_sha1)\n-\t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n-\n-\tupdate = add_update(transaction, refname);\n-\tupdate->flags = flags;\n-\tupdate->have_old = have_old;\n-\tif (have_old) {\n-\t\tassert(!is_null_sha1(old_sha1));\n-\t\thashcpy(update->old_sha1, old_sha1);\n-\t}\n-\tif (msg)\n-\t\tupdate->msg = xstrdup(msg);\n-\treturn 0;\n+\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\t\t\t\t old_sha1, flags, have_old, msg, err);\n }\n \n int update_ref(const char *action, const char *refname,\ndiff --git a/refs.h b/refs.h\nindex 2bc3556..7d675b7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -283,7 +283,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\n \n /*\n * Add a reference update to transaction. new_sha1 is the value that\n- * the reference should have after the update, or zeros if it should\n+ * the reference should have after the update, or null_sha1 if it should\n * be deleted. If have_old is true, then old_sha1 holds the value\n * that the reference should have had before the update, or zeros if\n * it must not have existed beforehand.\n-- \n2.1.3\n"},{"id":"253607","messageId":"1418374623-5566-4-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 03/24] refs.c: add a function to append a reflog entry to a fd","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:42Z","receivedAt":"2014-12-12T08:56:42Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nBreak out the code to create the string and writing it to the file\ndescriptor from log_ref_write and add it into a dedicated function\nlog_ref_write_fd. It is a nice unit of work.\n\nFor now this is only used from log_ref_write, but in the future it\nmight have other callers.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Stefan Beller \nReviewed-by: Jonathan Nieder \nSigned-off-by: Michael Haggerty \n---\n refs.c | 48 ++++++++++++++++++++++++++++++------------------\n 1 file changed, 30 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 05cb299..150c980 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2990,15 +2990,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)\n \treturn 0;\n }\n \n+static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n+\t\t\t const unsigned char *new_sha1,\n+\t\t\t const char *committer, const char *msg)\n+{\n+\tint msglen, written;\n+\tunsigned maxlen, len;\n+\tchar *logrec;\n+\n+\tmsglen = msg ? strlen(msg) : 0;\n+\tmaxlen = strlen(committer) + msglen + 100;\n+\tlogrec = xmalloc(maxlen);\n+\tlen = sprintf(logrec, \"%s %s %s\\n\",\n+\t\t sha1_to_hex(old_sha1),\n+\t\t sha1_to_hex(new_sha1),\n+\t\t committer);\n+\tif (msglen)\n+\t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n+\n+\twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\n+\tfree(logrec);\n+\tif (written != len)\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\t\t const unsigned char *new_sha1, const char *msg)\n {\n-\tint logfd, result, written, oflags = O_APPEND | O_WRONLY;\n-\tunsigned maxlen, len;\n-\tint msglen;\n+\tint logfd, result, oflags = O_APPEND | O_WRONLY;\n \tchar log_file[PATH_MAX];\n-\tchar *logrec;\n-\tconst char *committer;\n \n \tif (log_all_ref_updates < 0)\n \t\tlog_all_ref_updates = !is_bare_repository();\n@@ -3010,19 +3032,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \tlogfd = open(log_file, oflags);\n \tif (logfd < 0)\n \t\treturn 0;\n-\tmsglen = msg ? strlen(msg) : 0;\n-\tcommitter = git_committer_info(0);\n-\tmaxlen = strlen(committer) + msglen + 100;\n-\tlogrec = xmalloc(maxlen);\n-\tlen = sprintf(logrec, \"%s %s %s\\n\",\n-\t\t sha1_to_hex(old_sha1),\n-\t\t sha1_to_hex(new_sha1),\n-\t\t committer);\n-\tif (msglen)\n-\t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n-\twritten = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;\n-\tfree(logrec);\n-\tif (written != len) {\n+\tresult = log_ref_write_fd(logfd, old_sha1, new_sha1,\n+\t\t\t\t git_committer_info(0), msg);\n+\tif (result) {\n \t\tint save_errno = errno;\n \t\tclose(logfd);\n \t\terror(\"Unable to append to %s\", log_file);\n-- \n2.1.3\n"},{"id":"253608","messageId":"1418374623-5566-5-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 04/24] expire_reflog(): it's not an each_ref_fn anymore","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:43Z","receivedAt":"2014-12-12T08:56:43Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Prior to v1.5.4~14, expire_reflog() had to be an each_ref_fn because\nit was passed to for_each_reflog(). Since then, there has been no\nreason for it to implement the each_ref_fn interface. So...\n\n* Remove the \"unused\" parameter (which took the place of \"flags\", but\n was really unused).\n\n* Declare the last parameter to be (struct cmd_reflog_expire_cb *)\n rather than (void *).\n\nHelped-by: Jonathan Nieder \nSigned-off-by: Michael Haggerty \n---\n builtin/reflog.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 2d85d26..160541a 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -349,9 +349,9 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n \treturn 0;\n }\n \n-static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)\n+static int expire_reflog(const char *ref, const unsigned char *sha1,\n+\t\t\t struct cmd_reflog_expire_cb *cmd)\n {\n-\tstruct cmd_reflog_expire_cb *cmd = cb_data;\n \tstruct expire_reflog_cb cb;\n \tstruct ref_lock *lock;\n \tchar *log_file, *newlog_path = NULL;\n@@ -663,7 +663,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n \t\t\tset_reflog_expiry_param(&cb, explicit_expiry, e->reflog);\n-\t\t\tstatus |= expire_reflog(e->reflog, e->sha1, 0, &cb);\n+\t\t\tstatus |= expire_reflog(e->reflog, e->sha1, &cb);\n \t\t\tfree(e);\n \t\t}\n \t\tfree(collected.e);\n@@ -677,7 +677,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tset_reflog_expiry_param(&cb, explicit_expiry, ref);\n-\t\tstatus |= expire_reflog(ref, sha1, 0, &cb);\n+\t\tstatus |= expire_reflog(ref, sha1, &cb);\n \t}\n \treturn status;\n }\n@@ -748,7 +748,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\t\tcb.expire_total = 0;\n \t\t}\n \n-\t\tstatus |= expire_reflog(ref, sha1, 0, &cb);\n+\t\tstatus |= expire_reflog(ref, sha1, &cb);\n \t\tfree(ref);\n \t}\n \treturn status;\n-- \n2.1.3\n"},{"id":"253631","messageId":"1418374623-5566-6-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 05/24] expire_reflog(): rename \"ref\" parameter to \"refname\"","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:44Z","receivedAt":"2014-12-12T08:56:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is our usual convention.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Jonathan Nieder \n---\n builtin/reflog.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 160541a..ff51dbf 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -349,7 +349,7 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n \treturn 0;\n }\n \n-static int expire_reflog(const char *ref, const unsigned char *sha1,\n+static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t struct cmd_reflog_expire_cb *cmd)\n {\n \tstruct expire_reflog_cb cb;\n@@ -365,20 +365,20 @@ static int expire_reflog(const char *ref, const unsigned char *sha1,\n \t * we take the lock for the ref itself to prevent it from\n \t * getting updated.\n \t */\n-\tlock = lock_any_ref_for_update(ref, sha1, 0, NULL);\n+\tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n \tif (!lock)\n-\t\treturn error(\"cannot lock ref '%s'\", ref);\n-\tlog_file = git_pathdup(\"logs/%s\", ref);\n-\tif (!reflog_exists(ref))\n+\t\treturn error(\"cannot lock ref '%s'\", refname);\n+\tlog_file = git_pathdup(\"logs/%s\", refname);\n+\tif (!reflog_exists(refname))\n \t\tgoto finish;\n \tif (!cmd->dry_run) {\n-\t\tnewlog_path = git_pathdup(\"logs/%s.lock\", ref);\n+\t\tnewlog_path = git_pathdup(\"logs/%s.lock\", refname);\n \t\tcb.newlog = fopen(newlog_path, \"w\");\n \t}\n \n \tcb.cmd = cmd;\n \n-\tif (!cmd->expire_unreachable || !strcmp(ref, \"HEAD\")) {\n+\tif (!cmd->expire_unreachable || !strcmp(refname, \"HEAD\")) {\n \t\ttip_commit = NULL;\n \t\tcb.unreachable_expire_kind = UE_HEAD;\n \t} else {\n@@ -407,7 +407,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1,\n \t\tmark_reachable(&cb);\n \t}\n \n-\tfor_each_reflog_ent(ref, expire_reflog_ent, &cb);\n+\tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n \n \tif (cb.unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb.unreachable_expire_kind == UE_HEAD) {\n-- \n2.1.3\n"},{"id":"253628","messageId":"1418374623-5566-7-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 06/24] expire_reflog(): return early if the reference has no reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:45Z","receivedAt":"2014-12-12T08:56:45Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There is very little cleanup needed if the reference has no reflog. If\nwe move the initialization of log_file down a bit, there's even less.\nSo instead of jumping to the cleanup code at the end of the function,\njust do the cleanup and return inline.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Jonathan Nieder \n---\n builtin/reflog.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex ff51dbf..37b33c9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -368,9 +368,11 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n \tif (!lock)\n \t\treturn error(\"cannot lock ref '%s'\", refname);\n+\tif (!reflog_exists(refname)) {\n+\t\tunlock_ref(lock);\n+\t\treturn 0;\n+\t}\n \tlog_file = git_pathdup(\"logs/%s\", refname);\n-\tif (!reflog_exists(refname))\n-\t\tgoto finish;\n \tif (!cmd->dry_run) {\n \t\tnewlog_path = git_pathdup(\"logs/%s.lock\", refname);\n \t\tcb.newlog = fopen(newlog_path, \"w\");\n@@ -419,7 +421,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\tclear_commit_marks(tip_commit, REACHABLE);\n \t\t}\n \t}\n- finish:\n+\n \tif (cb.newlog) {\n \t\tif (fclose(cb.newlog)) {\n \t\t\tstatus |= error(\"%s: %s\", strerror(errno),\n-- \n2.1.3\n"},{"id":"253627","messageId":"1418374623-5566-8-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 07/24] expire_reflog(): use a lock_file for rewriting the reflog file","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:46Z","receivedAt":"2014-12-12T08:56:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"We don't actually need the locking functionality, because we already\nhold the lock on the reference itself, which is how the reflog file is\nlocked. But the lock_file code can do some of the bookkeeping for us,\nand it is more careful than the old code here was. For example:\n\n* It correctly handles the case that the reflog lock file already\n exists for some reason or cannot be opened.\n\n* It correctly cleans up the lockfile if the program dies.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/reflog.c | 60 ++++++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 41 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 37b33c9..ba5b3d3 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -352,9 +352,10 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t struct cmd_reflog_expire_cb *cmd)\n {\n+\tstatic struct lock_file reflog_lock;\n \tstruct expire_reflog_cb cb;\n \tstruct ref_lock *lock;\n-\tchar *log_file, *newlog_path = NULL;\n+\tchar *log_file;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n \tint status = 0;\n@@ -362,8 +363,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \tmemset(&cb, 0, sizeof(cb));\n \n \t/*\n-\t * we take the lock for the ref itself to prevent it from\n-\t * getting updated.\n+\t * The reflog file is locked by holding the lock on the\n+\t * reference itself, plus we might need to update the\n+\t * reference if --updateref was specified:\n \t */\n \tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n \tif (!lock)\n@@ -372,10 +374,29 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\tunlock_ref(lock);\n \t\treturn 0;\n \t}\n+\n \tlog_file = git_pathdup(\"logs/%s\", refname);\n \tif (!cmd->dry_run) {\n-\t\tnewlog_path = git_pathdup(\"logs/%s.lock\", refname);\n-\t\tcb.newlog = fopen(newlog_path, \"w\");\n+\t\t/*\n+\t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n+\t\t * no locking implications, we use the lock_file\n+\t\t * machinery here anyway because it does a lot of the\n+\t\t * work we need, including cleaning up if the program\n+\t\t * exits unexpectedly.\n+\t\t */\n+\t\tif (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {\n+\t\t\tstruct strbuf err = STRBUF_INIT;\n+\t\t\tunable_to_lock_message(log_file, errno, &err);\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t\tgoto failure;\n+\t\t}\n+\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n+\t\tif (!cb.newlog) {\n+\t\t\terror(\"cannot fdopen %s (%s)\",\n+\t\t\t reflog_lock.filename.buf, strerror(errno));\n+\t\t\tgoto failure;\n+\t\t}\n \t}\n \n \tcb.cmd = cmd;\n@@ -423,32 +444,33 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t}\n \n \tif (cb.newlog) {\n-\t\tif (fclose(cb.newlog)) {\n-\t\t\tstatus |= error(\"%s: %s\", strerror(errno),\n-\t\t\t\t\tnewlog_path);\n-\t\t\tunlink(newlog_path);\n+\t\tif (close_lock_file(&reflog_lock)) {\n+\t\t\tstatus |= error(\"couldn't write %s: %s\", log_file,\n+\t\t\t\t\tstrerror(errno));\n \t\t} else if (cmd->updateref &&\n \t\t\t(write_in_full(lock->lock_fd,\n \t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n \t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n \t\t\t close_ref(lock) < 0)) {\n-\t\t\tstatus |= error(\"Couldn't write %s\",\n+\t\t\tstatus |= error(\"couldn't write %s\",\n \t\t\t\t\tlock->lk->filename.buf);\n-\t\t\tunlink(newlog_path);\n-\t\t} else if (rename(newlog_path, log_file)) {\n-\t\t\tstatus |= error(\"cannot rename %s to %s\",\n-\t\t\t\t\tnewlog_path, log_file);\n-\t\t\tunlink(newlog_path);\n+\t\t\trollback_lock_file(&reflog_lock);\n+\t\t} else if (commit_lock_file(&reflog_lock)) {\n+\t\t\tstatus |= error(\"unable to commit reflog '%s' (%s)\",\n+\t\t\t\t\tlog_file, strerror(errno));\n \t\t} else if (cmd->updateref && commit_ref(lock)) {\n-\t\t\tstatus |= error(\"Couldn't set %s\", lock->ref_name);\n-\t\t} else {\n-\t\t\tadjust_shared_perm(log_file);\n+\t\t\tstatus |= error(\"couldn't set %s\", lock->ref_name);\n \t\t}\n \t}\n-\tfree(newlog_path);\n \tfree(log_file);\n \tunlock_ref(lock);\n \treturn status;\n+\n+ failure:\n+\trollback_lock_file(&reflog_lock);\n+\tfree(log_file);\n+\tunlock_ref(lock);\n+\treturn -1;\n }\n \n static int collect_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)\n-- \n2.1.3\n"},{"id":"253629","messageId":"1418374623-5566-9-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 08/24] Extract function should_expire_reflog_ent()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:47Z","receivedAt":"2014-12-12T08:56:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract from expire_reflog_ent() a function that is solely responsible\nfor deciding whether a reflog entry should be expired. By separating\nthis \"business logic\" from the mechanics of actually expiring entries,\nwe are working towards the goal of encapsulating reflog expiry within\nthe refs API, with policy decided by a callback function passed to it\nby its caller.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/reflog.c | 70 +++++++++++++++++++++++++++++++++-----------------------\n 1 file changed, 42 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex ba5b3d3..06ce8b1 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -288,51 +288,65 @@ static int unreachable(struct expire_reflog_cb *cb, struct commit *commit, unsig\n \treturn !(commit->object.flags & REACHABLE);\n }\n \n-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n-\t\tconst char *email, unsigned long timestamp, int tz,\n-\t\tconst char *message, void *cb_data)\n+/*\n+ * Return true iff the specified reflog entry should be expired.\n+ */\n+static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t\t const char *email, unsigned long timestamp, int tz,\n+\t\t\t\t const char *message, void *cb_data)\n {\n \tstruct expire_reflog_cb *cb = cb_data;\n \tstruct commit *old, *new;\n \n \tif (timestamp < cb->cmd->expire_total)\n-\t\tgoto prune;\n-\n-\tif (cb->cmd->rewrite)\n-\t\tosha1 = cb->last_kept_sha1;\n+\t\treturn 1;\n \n \told = new = NULL;\n \tif (cb->cmd->stalefix &&\n \t (!keep_entry(&old, osha1) || !keep_entry(&new, nsha1)))\n-\t\tgoto prune;\n+\t\treturn 1;\n \n \tif (timestamp < cb->cmd->expire_unreachable) {\n \t\tif (cb->unreachable_expire_kind == UE_ALWAYS)\n-\t\t\tgoto prune;\n+\t\t\treturn 1;\n \t\tif (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))\n-\t\t\tgoto prune;\n+\t\t\treturn 1;\n \t}\n \n \tif (cb->cmd->recno && --(cb->cmd->recno) == 0)\n-\t\tgoto prune;\n-\n-\tif (cb->newlog) {\n-\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n-\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\temail, timestamp, sign, zone,\n-\t\t\tmessage);\n-\t\thashcpy(cb->last_kept_sha1, nsha1);\n-\t}\n-\tif (cb->cmd->verbose)\n-\t\tprintf(\"keep %s\", message);\n+\t\treturn 1;\n+\n \treturn 0;\n- prune:\n-\tif (!cb->newlog)\n-\t\tprintf(\"would prune %s\", message);\n-\telse if (cb->cmd->verbose)\n-\t\tprintf(\"prune %s\", message);\n+}\n+\n+static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\tconst char *email, unsigned long timestamp, int tz,\n+\t\tconst char *message, void *cb_data)\n+{\n+\tstruct expire_reflog_cb *cb = cb_data;\n+\n+\tif (cb->cmd->rewrite)\n+\t\tosha1 = cb->last_kept_sha1;\n+\n+\tif (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n+\t\t\t\t message, cb_data)) {\n+\t\tif (!cb->newlog)\n+\t\t\tprintf(\"would prune %s\", message);\n+\t\telse if (cb->cmd->verbose)\n+\t\t\tprintf(\"prune %s\", message);\n+\t} else {\n+\t\tif (cb->newlog) {\n+\t\t\tchar sign = (tz < 0) ? '-' : '+';\n+\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n+\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n+\t\t\t\temail, timestamp, sign, zone,\n+\t\t\t\tmessage);\n+\t\t\thashcpy(cb->last_kept_sha1, nsha1);\n+\t\t}\n+\t\tif (cb->cmd->verbose)\n+\t\t\tprintf(\"keep %s\", message);\n+\t}\n \treturn 0;\n }\n \n-- \n2.1.3\n"},{"id":"253610","messageId":"1418374623-5566-10-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 09/24] expire_reflog(): extract two policy-related functions","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:48Z","receivedAt":"2014-12-12T08:56:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Extract two functions, reflog_expiry_prepare() and\nreflog_expiry_cleanup(), from expire_reflog(). This is a further step\ntowards separating the code for deciding on expiration policy from the\ncode that manages the physical deletion of reflog entries.\n\nThis change requires a couple of local variables from expire_reflog()\nto be turned into fields of \"struct expire_reflog_cb\". More\nreorganization of the callback data will follow in later commits.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 94 +++++++++++++++++++++++++++++++-------------------------\n 1 file changed, 52 insertions(+), 42 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 06ce8b1..8db52d7 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -43,6 +43,8 @@ struct expire_reflog_cb {\n \tunsigned long mark_limit;\n \tstruct cmd_reflog_expire_cb *cmd;\n \tunsigned char last_kept_sha1[20];\n+\tstruct commit *tip_commit;\n+\tstruct commit_list *tips;\n };\n \n struct collected_reflog {\n@@ -363,6 +365,54 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n \treturn 0;\n }\n \n+static void reflog_expiry_prepare(const char *refname,\n+\t\t\t\t const unsigned char *sha1,\n+\t\t\t\t struct expire_reflog_cb *cb)\n+{\n+\tif (!cb->cmd->expire_unreachable || !strcmp(refname, \"HEAD\")) {\n+\t\tcb->tip_commit = NULL;\n+\t\tcb->unreachable_expire_kind = UE_HEAD;\n+\t} else {\n+\t\tcb->tip_commit = lookup_commit_reference_gently(sha1, 1);\n+\t\tif (!cb->tip_commit)\n+\t\t\tcb->unreachable_expire_kind = UE_ALWAYS;\n+\t\telse\n+\t\t\tcb->unreachable_expire_kind = UE_NORMAL;\n+\t}\n+\n+\tif (cb->cmd->expire_unreachable <= cb->cmd->expire_total)\n+\t\tcb->unreachable_expire_kind = UE_ALWAYS;\n+\n+\tcb->mark_list = NULL;\n+\tcb->tips = NULL;\n+\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n+\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n+\t\t\tstruct commit_list *elem;\n+\t\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n+\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n+\t\t} else {\n+\t\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n+\t\t}\n+\t\tcb->mark_limit = cb->cmd->expire_total;\n+\t\tmark_reachable(cb);\n+\t}\n+}\n+\n+static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)\n+{\n+\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n+\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n+\t\t\tstruct commit_list *elem;\n+\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\t\tclear_commit_marks(elem->item, REACHABLE);\n+\t\t\tfree_commit_list(cb->tips);\n+\t\t} else {\n+\t\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n+\t\t}\n+\t}\n+}\n+\n static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t struct cmd_reflog_expire_cb *cmd)\n {\n@@ -370,8 +420,6 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \tstruct expire_reflog_cb cb;\n \tstruct ref_lock *lock;\n \tchar *log_file;\n-\tstruct commit *tip_commit;\n-\tstruct commit_list *tips;\n \tint status = 0;\n \n \tmemset(&cb, 0, sizeof(cb));\n@@ -415,47 +463,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \n \tcb.cmd = cmd;\n \n-\tif (!cmd->expire_unreachable || !strcmp(refname, \"HEAD\")) {\n-\t\ttip_commit = NULL;\n-\t\tcb.unreachable_expire_kind = UE_HEAD;\n-\t} else {\n-\t\ttip_commit = lookup_commit_reference_gently(sha1, 1);\n-\t\tif (!tip_commit)\n-\t\t\tcb.unreachable_expire_kind = UE_ALWAYS;\n-\t\telse\n-\t\t\tcb.unreachable_expire_kind = UE_NORMAL;\n-\t}\n-\n-\tif (cmd->expire_unreachable <= cmd->expire_total)\n-\t\tcb.unreachable_expire_kind = UE_ALWAYS;\n-\n-\tcb.mark_list = NULL;\n-\ttips = NULL;\n-\tif (cb.unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb.unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\t\t\tfor_each_ref(push_tip_to_list, &tips);\n-\t\t\tfor (elem = tips; elem; elem = elem->next)\n-\t\t\t\tcommit_list_insert(elem->item, &cb.mark_list);\n-\t\t} else {\n-\t\t\tcommit_list_insert(tip_commit, &cb.mark_list);\n-\t\t}\n-\t\tcb.mark_limit = cmd->expire_total;\n-\t\tmark_reachable(&cb);\n-\t}\n-\n+\treflog_expiry_prepare(refname, sha1, &cb);\n \tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n-\n-\tif (cb.unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb.unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\t\t\tfor (elem = tips; elem; elem = elem->next)\n-\t\t\t\tclear_commit_marks(elem->item, REACHABLE);\n-\t\t\tfree_commit_list(tips);\n-\t\t} else {\n-\t\t\tclear_commit_marks(tip_commit, REACHABLE);\n-\t\t}\n-\t}\n+\treflog_expiry_cleanup(&cb);\n \n \tif (cb.newlog) {\n \t\tif (close_lock_file(&reflog_lock)) {\n-- \n2.1.3\n"},{"id":"253622","messageId":"1418374623-5566-11-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 10/24] expire_reflog(): add a \"flags\" argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:49Z","receivedAt":"2014-12-12T08:56:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"We want to separate the options relevant to the expiry machinery from\nthe options affecting the expiration policy. So add a \"flags\" argument\nto expire_reflog() to hold the former.\n\nThe argument doesn't yet do anything.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 8db52d7..dfff5f2 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -414,7 +414,7 @@ static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)\n }\n \n static int expire_reflog(const char *refname, const unsigned char *sha1,\n-\t\t\t struct cmd_reflog_expire_cb *cmd)\n+\t\t\t unsigned int flags, struct cmd_reflog_expire_cb *cmd)\n {\n \tstatic struct lock_file reflog_lock;\n \tstruct expire_reflog_cb cb;\n@@ -642,6 +642,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tunsigned long now = time(NULL);\n \tint i, status, do_all;\n \tint explicit_expiry = 0;\n+\tunsigned int flags = 0;\n \n \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n \tdefault_reflog_expire = now - 90 * 24 * 3600;\n@@ -711,7 +712,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n \t\t\tset_reflog_expiry_param(&cb, explicit_expiry, e->reflog);\n-\t\t\tstatus |= expire_reflog(e->reflog, e->sha1, &cb);\n+\t\t\tstatus |= expire_reflog(e->reflog, e->sha1, flags, &cb);\n \t\t\tfree(e);\n \t\t}\n \t\tfree(collected.e);\n@@ -725,7 +726,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tset_reflog_expiry_param(&cb, explicit_expiry, ref);\n-\t\tstatus |= expire_reflog(ref, sha1, &cb);\n+\t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n \t}\n \treturn status;\n }\n@@ -744,6 +745,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n {\n \tstruct cmd_reflog_expire_cb cb;\n \tint i, status = 0;\n+\tunsigned int flags = 0;\n \n \tmemset(&cb, 0, sizeof(cb));\n \n@@ -796,7 +798,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\t\tcb.expire_total = 0;\n \t\t}\n \n-\t\tstatus |= expire_reflog(ref, sha1, &cb);\n+\t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n \t\tfree(ref);\n \t}\n \treturn status;\n-- \n2.1.3\n"},{"id":"253625","messageId":"1418374623-5566-12-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 11/24] expire_reflog(): move dry_run to flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:50Z","receivedAt":"2014-12-12T08:56:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The policy objects don't care about \"--dry-run\". So move it to\nexpire_reflog()'s flags parameter.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex dfff5f2..2f21735 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -20,9 +20,12 @@ static const char reflog_delete_usage[] =\n static unsigned long default_reflog_expire;\n static unsigned long default_reflog_expire_unreachable;\n \n+enum expire_reflog_flags {\n+\tEXPIRE_REFLOGS_DRY_RUN = 1 << 0\n+};\n+\n struct cmd_reflog_expire_cb {\n \tstruct rev_info revs;\n-\tint dry_run;\n \tint stalefix;\n \tint rewrite;\n \tint updateref;\n@@ -438,7 +441,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t}\n \n \tlog_file = git_pathdup(\"logs/%s\", refname);\n-\tif (!cmd->dry_run) {\n+\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\t/*\n \t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n \t\t * no locking implications, we use the lock_file\n@@ -467,7 +470,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n \treflog_expiry_cleanup(&cb);\n \n-\tif (cb.newlog) {\n+\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\tif (close_lock_file(&reflog_lock)) {\n \t\t\tstatus |= error(\"couldn't write %s: %s\", log_file,\n \t\t\t\t\tstrerror(errno));\n@@ -658,7 +661,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n-\t\t\tcb.dry_run = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (starts_with(arg, \"--expire=\")) {\n \t\t\tif (parse_expiry_date(arg + 9, &cb.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n@@ -752,7 +755,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n-\t\t\tcb.dry_run = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tcb.rewrite = 1;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n-- \n2.1.3\n"},{"id":"253613","messageId":"1418374623-5566-13-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 12/24] expire_reflog(): move updateref to flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:51Z","receivedAt":"2014-12-12T08:56:51Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The policy objects don't care about \"--updateref\". So move it to\nexpire_reflog()'s flags parameter.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 2f21735..e238fe0 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -21,14 +21,14 @@ static unsigned long default_reflog_expire;\n static unsigned long default_reflog_expire_unreachable;\n \n enum expire_reflog_flags {\n-\tEXPIRE_REFLOGS_DRY_RUN = 1 << 0\n+\tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n+\tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1\n };\n \n struct cmd_reflog_expire_cb {\n \tstruct rev_info revs;\n \tint stalefix;\n \tint rewrite;\n-\tint updateref;\n \tint verbose;\n \tunsigned long expire_total;\n \tunsigned long expire_unreachable;\n@@ -474,7 +474,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\tif (close_lock_file(&reflog_lock)) {\n \t\t\tstatus |= error(\"couldn't write %s: %s\", log_file,\n \t\t\t\t\tstrerror(errno));\n-\t\t} else if (cmd->updateref &&\n+\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t\t(write_in_full(lock->lock_fd,\n \t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n \t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n@@ -485,7 +485,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t} else if (commit_lock_file(&reflog_lock)) {\n \t\t\tstatus |= error(\"unable to commit reflog '%s' (%s)\",\n \t\t\t\t\tlog_file, strerror(errno));\n-\t\t} else if (cmd->updateref && commit_ref(lock)) {\n+\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {\n \t\t\tstatus |= error(\"couldn't set %s\", lock->ref_name);\n \t\t}\n \t}\n@@ -677,7 +677,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tcb.rewrite = 1;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n-\t\t\tcb.updateref = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--all\"))\n \t\t\tdo_all = 1;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n@@ -759,7 +759,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tcb.rewrite = 1;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n-\t\t\tcb.updateref = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n \t\t\tcb.verbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n-- \n2.1.3\n"},{"id":"253621","messageId":"1418374623-5566-14-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 13/24] Rename expire_reflog_cb to expire_reflog_policy_cb","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:52Z","receivedAt":"2014-12-12T08:56:52Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is the first step towards separating the data needed by the\npolicy code from the data needed by the reflog expiration machinery.\n\n(In a moment we will add a *new* \"struct expire_reflog_cb\" for the use\nof expire_reflog() itself, then move fields selectively from\nexpire_reflog_policy_cb to expire_reflog_cb.)\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 19 ++++++++++---------\n 1 file changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex e238fe0..94c34fc 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -35,7 +35,7 @@ struct cmd_reflog_expire_cb {\n \tint recno;\n };\n \n-struct expire_reflog_cb {\n+struct expire_reflog_policy_cb {\n \tFILE *newlog;\n \tenum {\n \t\tUE_NORMAL,\n@@ -225,7 +225,7 @@ static int keep_entry(struct commit **it, unsigned char *sha1)\n * the expire_limit and queue them back, so that the caller can call\n * us again to restart the traversal with longer expire_limit.\n */\n-static void mark_reachable(struct expire_reflog_cb *cb)\n+static void mark_reachable(struct expire_reflog_policy_cb *cb)\n {\n \tstruct commit *commit;\n \tstruct commit_list *pending;\n@@ -264,7 +264,7 @@ static void mark_reachable(struct expire_reflog_cb *cb)\n \tcb->mark_list = leftover;\n }\n \n-static int unreachable(struct expire_reflog_cb *cb, struct commit *commit, unsigned char *sha1)\n+static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, unsigned char *sha1)\n {\n \t/*\n \t * We may or may not have the commit yet - if not, look it\n@@ -300,7 +300,7 @@ static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\t\t const char *email, unsigned long timestamp, int tz,\n \t\t\t\t const char *message, void *cb_data)\n {\n-\tstruct expire_reflog_cb *cb = cb_data;\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n \tstruct commit *old, *new;\n \n \tif (timestamp < cb->cmd->expire_total)\n@@ -328,7 +328,7 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\tconst char *email, unsigned long timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct expire_reflog_cb *cb = cb_data;\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n \n \tif (cb->cmd->rewrite)\n \t\tosha1 = cb->last_kept_sha1;\n@@ -355,7 +355,8 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \treturn 0;\n }\n \n-static int push_tip_to_list(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n+static int push_tip_to_list(const char *refname, const unsigned char *sha1,\n+\t\t\t int flags, void *cb_data)\n {\n \tstruct commit_list **list = cb_data;\n \tstruct commit *tip_commit;\n@@ -370,7 +371,7 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n \n static void reflog_expiry_prepare(const char *refname,\n \t\t\t\t const unsigned char *sha1,\n-\t\t\t\t struct expire_reflog_cb *cb)\n+\t\t\t\t struct expire_reflog_policy_cb *cb)\n {\n \tif (!cb->cmd->expire_unreachable || !strcmp(refname, \"HEAD\")) {\n \t\tcb->tip_commit = NULL;\n@@ -402,7 +403,7 @@ static void reflog_expiry_prepare(const char *refname,\n \t}\n }\n \n-static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)\n+static void reflog_expiry_cleanup(struct expire_reflog_policy_cb *cb)\n {\n \tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n@@ -420,7 +421,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t unsigned int flags, struct cmd_reflog_expire_cb *cmd)\n {\n \tstatic struct lock_file reflog_lock;\n-\tstruct expire_reflog_cb cb;\n+\tstruct expire_reflog_policy_cb cb;\n \tstruct ref_lock *lock;\n \tchar *log_file;\n \tint status = 0;\n-- \n2.1.3\n"},{"id":"253612","messageId":"1418374623-5566-15-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 14/24] struct expire_reflog_cb: a new callback data type","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:53Z","receivedAt":"2014-12-12T08:56:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a new data type, \"struct expire_reflog_cb\", for holding the data\nthat expire_reflog() passes to expire_reflog_ent() via\nfor_each_reflog_ent(). For now it only holds a pointer to a \"struct\nexpire_reflog_policy_cb\", which still contains all of the actual data.\nIn future commits we will move some fields from the latter to the\nformer.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 43 ++++++++++++++++++++++++++-----------------\n 1 file changed, 26 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 94c34fc..9ee66d4 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -50,10 +50,15 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *tips;\n };\n \n+struct expire_reflog_cb {\n+\tvoid *policy_cb;\n+};\n+\n struct collected_reflog {\n \tunsigned char sha1[20];\n \tchar reflog[FLEX_ARRAY];\n };\n+\n struct collect_reflog_cb {\n \tstruct collected_reflog **e;\n \tint alloc;\n@@ -328,28 +333,29 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\tconst char *email, unsigned long timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct expire_reflog_cb *cb = cb_data;\n+\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n \n-\tif (cb->cmd->rewrite)\n-\t\tosha1 = cb->last_kept_sha1;\n+\tif (policy_cb->cmd->rewrite)\n+\t\tosha1 = policy_cb->last_kept_sha1;\n \n \tif (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n-\t\t\t\t message, cb_data)) {\n-\t\tif (!cb->newlog)\n+\t\t\t\t message, policy_cb)) {\n+\t\tif (!policy_cb->newlog)\n \t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (cb->cmd->verbose)\n+\t\telse if (policy_cb->cmd->verbose)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (cb->newlog) {\n+\t\tif (policy_cb->newlog) {\n \t\t\tchar sign = (tz < 0) ? '-' : '+';\n \t\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\tfprintf(policy_cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n \t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n \t\t\t\temail, timestamp, sign, zone,\n \t\t\t\tmessage);\n-\t\t\thashcpy(cb->last_kept_sha1, nsha1);\n+\t\t\thashcpy(policy_cb->last_kept_sha1, nsha1);\n \t\t}\n-\t\tif (cb->cmd->verbose)\n+\t\tif (policy_cb->cmd->verbose)\n \t\t\tprintf(\"keep %s\", message);\n \t}\n \treturn 0;\n@@ -421,12 +427,15 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t unsigned int flags, struct cmd_reflog_expire_cb *cmd)\n {\n \tstatic struct lock_file reflog_lock;\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct expire_reflog_cb cb;\n+\tstruct expire_reflog_policy_cb policy_cb;\n \tstruct ref_lock *lock;\n \tchar *log_file;\n \tint status = 0;\n \n \tmemset(&cb, 0, sizeof(cb));\n+\tmemset(&policy_cb, 0, sizeof(policy_cb));\n+\tcb.policy_cb = &policy_cb;\n \n \t/*\n \t * The reflog file is locked by holding the lock on the\n@@ -457,19 +466,19 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\tstrbuf_release(&err);\n \t\t\tgoto failure;\n \t\t}\n-\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n-\t\tif (!cb.newlog) {\n+\t\tpolicy_cb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n+\t\tif (!policy_cb.newlog) {\n \t\t\terror(\"cannot fdopen %s (%s)\",\n \t\t\t reflog_lock.filename.buf, strerror(errno));\n \t\t\tgoto failure;\n \t\t}\n \t}\n \n-\tcb.cmd = cmd;\n+\tpolicy_cb.cmd = cmd;\n \n-\treflog_expiry_prepare(refname, sha1, &cb);\n+\treflog_expiry_prepare(refname, sha1, &policy_cb);\n \tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n-\treflog_expiry_cleanup(&cb);\n+\treflog_expiry_cleanup(&policy_cb);\n \n \tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\tif (close_lock_file(&reflog_lock)) {\n@@ -477,7 +486,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t\t\tstrerror(errno));\n \t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t\t(write_in_full(lock->lock_fd,\n-\t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n+\t\t\t\tsha1_to_hex(policy_cb.last_kept_sha1), 40) != 40 ||\n \t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n \t\t\t close_ref(lock) < 0)) {\n \t\t\tstatus |= error(\"couldn't write %s\",\n-- \n2.1.3\n"},{"id":"253623","messageId":"1418374623-5566-16-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 15/24] expire_reflog(): pass flags through to expire_reflog_ent()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:54Z","receivedAt":"2014-12-12T08:56:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a flags field to \"struct expire_reflog_cb\", and pass the flags\nargument through to expire_reflog_ent(). In a moment we will start\nusing it to pass through flags that expire_reflog_ent() needs.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 9ee66d4..08867a2 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -51,6 +51,7 @@ struct expire_reflog_policy_cb {\n };\n \n struct expire_reflog_cb {\n+\tunsigned int flags;\n \tvoid *policy_cb;\n };\n \n@@ -435,6 +436,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \n \tmemset(&cb, 0, sizeof(cb));\n \tmemset(&policy_cb, 0, sizeof(policy_cb));\n+\tcb.flags = flags;\n \tcb.policy_cb = &policy_cb;\n \n \t/*\n-- \n2.1.3\n"},{"id":"253611","messageId":"1418374623-5566-17-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 16/24] expire_reflog(): move verbose to flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:55Z","receivedAt":"2014-12-12T08:56:55Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The policy objects don't care about \"--verbose\". So move it to\nexpire_reflog()'s flags parameter.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 08867a2..15d3cd5 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -22,14 +22,14 @@ static unsigned long default_reflog_expire_unreachable;\n \n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n-\tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1\n+\tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n+\tEXPIRE_REFLOGS_VERBOSE = 1 << 2\n };\n \n struct cmd_reflog_expire_cb {\n \tstruct rev_info revs;\n \tint stalefix;\n \tint rewrite;\n-\tint verbose;\n \tunsigned long expire_total;\n \tunsigned long expire_unreachable;\n \tint recno;\n@@ -344,7 +344,7 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\t\t message, policy_cb)) {\n \t\tif (!policy_cb->newlog)\n \t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (policy_cb->cmd->verbose)\n+\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n \t\tif (policy_cb->newlog) {\n@@ -356,7 +356,7 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\t\tmessage);\n \t\t\thashcpy(policy_cb->last_kept_sha1, nsha1);\n \t\t}\n-\t\tif (policy_cb->cmd->verbose)\n+\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"keep %s\", message);\n \t}\n \treturn 0;\n@@ -693,7 +693,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--all\"))\n \t\t\tdo_all = 1;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tcb.verbose = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -711,10 +711,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t */\n \tif (cb.stalefix) {\n \t\tinit_revisions(&cb.revs, prefix);\n-\t\tif (cb.verbose)\n+\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"Marking reachable objects...\");\n \t\tmark_reachable_objects(&cb.revs, 0, 0, NULL);\n-\t\tif (cb.verbose)\n+\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -773,7 +773,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tcb.verbose = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n-- \n2.1.3\n"},{"id":"253616","messageId":"1418374623-5566-18-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 17/24] expire_reflog(): move rewrite to flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:56Z","receivedAt":"2014-12-12T08:56:56Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The policy objects don't care about \"--rewrite\". So move it to\nexpire_reflog()'s flags parameter.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 15d3cd5..f86e7de 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -23,13 +23,13 @@ static unsigned long default_reflog_expire_unreachable;\n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n \tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-\tEXPIRE_REFLOGS_VERBOSE = 1 << 2\n+\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n+\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n };\n \n struct cmd_reflog_expire_cb {\n \tstruct rev_info revs;\n \tint stalefix;\n-\tint rewrite;\n \tunsigned long expire_total;\n \tunsigned long expire_unreachable;\n \tint recno;\n@@ -337,7 +337,7 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \tstruct expire_reflog_cb *cb = cb_data;\n \tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n \n-\tif (policy_cb->cmd->rewrite)\n+\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n \t\tosha1 = policy_cb->last_kept_sha1;\n \n \tif (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n@@ -687,7 +687,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n \t\t\tcb.stalefix = 1;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n-\t\t\tcb.rewrite = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--all\"))\n@@ -769,7 +769,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n-\t\t\tcb.rewrite = 1;\n+\t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-- \n2.1.3\n"},{"id":"253624","messageId":"1418374623-5566-19-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 18/24] Move newlog and last_kept_sha1 to \"struct expire_reflog_cb\"","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:57Z","receivedAt":"2014-12-12T08:56:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"These members are not needed by the policy functions.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex f86e7de..d36c2c6 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -36,7 +36,6 @@ struct cmd_reflog_expire_cb {\n };\n \n struct expire_reflog_policy_cb {\n-\tFILE *newlog;\n \tenum {\n \t\tUE_NORMAL,\n \t\tUE_ALWAYS,\n@@ -45,7 +44,6 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *mark_list;\n \tunsigned long mark_limit;\n \tstruct cmd_reflog_expire_cb *cmd;\n-\tunsigned char last_kept_sha1[20];\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n };\n@@ -53,6 +51,8 @@ struct expire_reflog_policy_cb {\n struct expire_reflog_cb {\n \tunsigned int flags;\n \tvoid *policy_cb;\n+\tFILE *newlog;\n+\tunsigned char last_kept_sha1[20];\n };\n \n struct collected_reflog {\n@@ -338,23 +338,23 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n \n \tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n-\t\tosha1 = policy_cb->last_kept_sha1;\n+\t\tosha1 = cb->last_kept_sha1;\n \n \tif (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n \t\t\t\t message, policy_cb)) {\n-\t\tif (!policy_cb->newlog)\n+\t\tif (!cb->newlog)\n \t\t\tprintf(\"would prune %s\", message);\n \t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (policy_cb->newlog) {\n+\t\tif (cb->newlog) {\n \t\t\tchar sign = (tz < 0) ? '-' : '+';\n \t\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\t\tfprintf(policy_cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n \t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n \t\t\t\temail, timestamp, sign, zone,\n \t\t\t\tmessage);\n-\t\t\thashcpy(policy_cb->last_kept_sha1, nsha1);\n+\t\t\thashcpy(cb->last_kept_sha1, nsha1);\n \t\t}\n \t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"keep %s\", message);\n@@ -468,8 +468,8 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\tstrbuf_release(&err);\n \t\t\tgoto failure;\n \t\t}\n-\t\tpolicy_cb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n-\t\tif (!policy_cb.newlog) {\n+\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n+\t\tif (!cb.newlog) {\n \t\t\terror(\"cannot fdopen %s (%s)\",\n \t\t\t reflog_lock.filename.buf, strerror(errno));\n \t\t\tgoto failure;\n@@ -488,7 +488,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t\t\t\tstrerror(errno));\n \t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t\t(write_in_full(lock->lock_fd,\n-\t\t\t\tsha1_to_hex(policy_cb.last_kept_sha1), 40) != 40 ||\n+\t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n \t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n \t\t\t close_ref(lock) < 0)) {\n \t\t\tstatus |= error(\"couldn't write %s\",\n-- \n2.1.3\n"},{"id":"253615","messageId":"1418374623-5566-20-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 19/24] expire_reflog(): treat the policy callback data as opaque","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:58Z","receivedAt":"2014-12-12T08:56:58Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Now that expire_reflog() doesn't actually look in the\nexpire_reflog_policy_cb data structure, we can make it opaque:\n\n* Change the callers of expire_reflog() to pass it a pointer to an\n entire \"struct expire_reflog_policy_cb\" rather than a pointer to a\n \"struct cmd_reflog_expire_cb\".\n\n* Change expire_reflog() to accept the argument as a \"void *\" and\n simply pass it through to the policy functions.\n\n* Change the policy functions, reflog_expiry_prepare(),\n reflog_expiry_cleanup(), and should_expire_reflog_ent(), to accept\n \"void *cb_data\" arguments and cast them back to \"struct\n expire_reflog_policy_cb\" internally.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 72 ++++++++++++++++++++++++++++----------------------------\n 1 file changed, 36 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex d36c2c6..0df5721 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -43,7 +43,7 @@ struct expire_reflog_policy_cb {\n \t} unreachable_expire_kind;\n \tstruct commit_list *mark_list;\n \tunsigned long mark_limit;\n-\tstruct cmd_reflog_expire_cb *cmd;\n+\tstruct cmd_reflog_expire_cb cmd;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n };\n@@ -309,22 +309,22 @@ static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \tstruct commit *old, *new;\n \n-\tif (timestamp < cb->cmd->expire_total)\n+\tif (timestamp < cb->cmd.expire_total)\n \t\treturn 1;\n \n \told = new = NULL;\n-\tif (cb->cmd->stalefix &&\n+\tif (cb->cmd.stalefix &&\n \t (!keep_entry(&old, osha1) || !keep_entry(&new, nsha1)))\n \t\treturn 1;\n \n-\tif (timestamp < cb->cmd->expire_unreachable) {\n+\tif (timestamp < cb->cmd.expire_unreachable) {\n \t\tif (cb->unreachable_expire_kind == UE_ALWAYS)\n \t\t\treturn 1;\n \t\tif (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))\n \t\t\treturn 1;\n \t}\n \n-\tif (cb->cmd->recno && --(cb->cmd->recno) == 0)\n+\tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n \t\treturn 1;\n \n \treturn 0;\n@@ -378,9 +378,11 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1,\n \n static void reflog_expiry_prepare(const char *refname,\n \t\t\t\t const unsigned char *sha1,\n-\t\t\t\t struct expire_reflog_policy_cb *cb)\n+\t\t\t\t void *cb_data)\n {\n-\tif (!cb->cmd->expire_unreachable || !strcmp(refname, \"HEAD\")) {\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n+\n+\tif (!cb->cmd.expire_unreachable || !strcmp(refname, \"HEAD\")) {\n \t\tcb->tip_commit = NULL;\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n@@ -391,7 +393,7 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\tcb->unreachable_expire_kind = UE_NORMAL;\n \t}\n \n-\tif (cb->cmd->expire_unreachable <= cb->cmd->expire_total)\n+\tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n \tcb->mark_list = NULL;\n@@ -405,13 +407,15 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t} else {\n \t\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n \t\t}\n-\t\tcb->mark_limit = cb->cmd->expire_total;\n+\t\tcb->mark_limit = cb->cmd.expire_total;\n \t\tmark_reachable(cb);\n \t}\n }\n \n-static void reflog_expiry_cleanup(struct expire_reflog_policy_cb *cb)\n+static void reflog_expiry_cleanup(void *cb_data)\n {\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n+\n \tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n \t\t\tstruct commit_list *elem;\n@@ -425,19 +429,17 @@ static void reflog_expiry_cleanup(struct expire_reflog_policy_cb *cb)\n }\n \n static int expire_reflog(const char *refname, const unsigned char *sha1,\n-\t\t\t unsigned int flags, struct cmd_reflog_expire_cb *cmd)\n+\t\t\t unsigned int flags, void *policy_cb_data)\n {\n \tstatic struct lock_file reflog_lock;\n \tstruct expire_reflog_cb cb;\n-\tstruct expire_reflog_policy_cb policy_cb;\n \tstruct ref_lock *lock;\n \tchar *log_file;\n \tint status = 0;\n \n \tmemset(&cb, 0, sizeof(cb));\n-\tmemset(&policy_cb, 0, sizeof(policy_cb));\n \tcb.flags = flags;\n-\tcb.policy_cb = &policy_cb;\n+\tcb.policy_cb = policy_cb_data;\n \n \t/*\n \t * The reflog file is locked by holding the lock on the\n@@ -476,11 +478,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n \t\t}\n \t}\n \n-\tpolicy_cb.cmd = cmd;\n-\n-\treflog_expiry_prepare(refname, sha1, &policy_cb);\n+\treflog_expiry_prepare(refname, sha1, cb.policy_cb);\n \tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n-\treflog_expiry_cleanup(&policy_cb);\n+\treflog_expiry_cleanup(cb.policy_cb);\n \n \tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\tif (close_lock_file(&reflog_lock)) {\n@@ -653,7 +653,7 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c\n \n static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n {\n-\tstruct cmd_reflog_expire_cb cb;\n+\tstruct expire_reflog_policy_cb cb;\n \tunsigned long now = time(NULL);\n \tint i, status, do_all;\n \tint explicit_expiry = 0;\n@@ -667,25 +667,25 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tdo_all = status = 0;\n \tmemset(&cb, 0, sizeof(cb));\n \n-\tcb.expire_total = default_reflog_expire;\n-\tcb.expire_unreachable = default_reflog_expire_unreachable;\n+\tcb.cmd.expire_total = default_reflog_expire;\n+\tcb.cmd.expire_unreachable = default_reflog_expire_unreachable;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (starts_with(arg, \"--expire=\")) {\n-\t\t\tif (parse_expiry_date(arg + 9, &cb.expire_total))\n+\t\t\tif (parse_expiry_date(arg + 9, &cb.cmd.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_TOTAL;\n \t\t}\n \t\telse if (starts_with(arg, \"--expire-unreachable=\")) {\n-\t\t\tif (parse_expiry_date(arg + 21, &cb.expire_unreachable))\n+\t\t\tif (parse_expiry_date(arg + 21, &cb.cmd.expire_unreachable))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_UNREACH;\n \t\t}\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n-\t\t\tcb.stalefix = 1;\n+\t\t\tcb.cmd.stalefix = 1;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n@@ -709,11 +709,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * even in older repository. We cannot trust what's reachable\n \t * from reflog if the repository was pruned with older git.\n \t */\n-\tif (cb.stalefix) {\n-\t\tinit_revisions(&cb.revs, prefix);\n+\tif (cb.cmd.stalefix) {\n+\t\tinit_revisions(&cb.cmd.revs, prefix);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"Marking reachable objects...\");\n-\t\tmark_reachable_objects(&cb.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&cb.cmd.revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n@@ -726,7 +726,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfor_each_reflog(collect_reflog, &collected);\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n-\t\t\tset_reflog_expiry_param(&cb, explicit_expiry, e->reflog);\n+\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n \t\t\tstatus |= expire_reflog(e->reflog, e->sha1, flags, &cb);\n \t\t\tfree(e);\n \t\t}\n@@ -740,7 +740,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tstatus |= error(\"%s points nowhere!\", argv[i]);\n \t\t\tcontinue;\n \t\t}\n-\t\tset_reflog_expiry_param(&cb, explicit_expiry, ref);\n+\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);\n \t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n \t}\n \treturn status;\n@@ -750,15 +750,15 @@ static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\tconst char *email, unsigned long timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct cmd_reflog_expire_cb *cb = cb_data;\n-\tif (!cb->expire_total || timestamp < cb->expire_total)\n-\t\tcb->recno++;\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tif (!cb->cmd.expire_total || timestamp < cb->cmd.expire_total)\n+\t\tcb->cmd.recno++;\n \treturn 0;\n }\n \n static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n {\n-\tstruct cmd_reflog_expire_cb cb;\n+\tstruct expire_reflog_policy_cb cb;\n \tint i, status = 0;\n \tunsigned int flags = 0;\n \n@@ -805,12 +805,12 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \n \t\trecno = strtoul(spec + 2, &ep, 10);\n \t\tif (*ep == '}') {\n-\t\t\tcb.recno = -recno;\n+\t\t\tcb.cmd.recno = -recno;\n \t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n \t\t} else {\n-\t\t\tcb.expire_total = approxidate(spec + 2);\n+\t\t\tcb.cmd.expire_total = approxidate(spec + 2);\n \t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n-\t\t\tcb.expire_total = 0;\n+\t\t\tcb.cmd.expire_total = 0;\n \t\t}\n \n \t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n-- \n2.1.3\n"},{"id":"253614","messageId":"1418374623-5566-21-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 20/24] reflog_expire(): new function in the reference API","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:56:59Z","receivedAt":"2014-12-12T08:56:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Move expire_reflog() into refs.c and rename it to reflog_expire().\nTurn the three policy functions into function pointers that are passed\ninto reflog_expire(). Add function prototypes and documentation to\nrefs.h.\n\nSigned-off-by: Michael Haggerty \nReviewed-by: Stefan Beller \n---\n builtin/reflog.c | 148 ++++++-------------------------------------------------\n refs.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++\n refs.h | 46 +++++++++++++++++\n 3 files changed, 190 insertions(+), 133 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 0df5721..49c64f9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -20,13 +20,6 @@ static const char reflog_delete_usage[] =\n static unsigned long default_reflog_expire;\n static unsigned long default_reflog_expire_unreachable;\n \n-enum expire_reflog_flags {\n-\tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n-\tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n-\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n-};\n-\n struct cmd_reflog_expire_cb {\n \tstruct rev_info revs;\n \tint stalefix;\n@@ -48,13 +41,6 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *tips;\n };\n \n-struct expire_reflog_cb {\n-\tunsigned int flags;\n-\tvoid *policy_cb;\n-\tFILE *newlog;\n-\tunsigned char last_kept_sha1[20];\n-};\n-\n struct collected_reflog {\n \tunsigned char sha1[20];\n \tchar reflog[FLEX_ARRAY];\n@@ -330,38 +316,6 @@ static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \treturn 0;\n }\n \n-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n-\t\tconst char *email, unsigned long timestamp, int tz,\n-\t\tconst char *message, void *cb_data)\n-{\n-\tstruct expire_reflog_cb *cb = cb_data;\n-\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n-\n-\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n-\t\tosha1 = cb->last_kept_sha1;\n-\n-\tif (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n-\t\t\t\t message, policy_cb)) {\n-\t\tif (!cb->newlog)\n-\t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"prune %s\", message);\n-\t} else {\n-\t\tif (cb->newlog) {\n-\t\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n-\t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\t\temail, timestamp, sign, zone,\n-\t\t\t\tmessage);\n-\t\t\thashcpy(cb->last_kept_sha1, nsha1);\n-\t\t}\n-\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"keep %s\", message);\n-\t}\n-\treturn 0;\n-}\n-\n static int push_tip_to_list(const char *refname, const unsigned char *sha1,\n \t\t\t int flags, void *cb_data)\n {\n@@ -428,90 +382,6 @@ static void reflog_expiry_cleanup(void *cb_data)\n \t}\n }\n \n-static int expire_reflog(const char *refname, const unsigned char *sha1,\n-\t\t\t unsigned int flags, void *policy_cb_data)\n-{\n-\tstatic struct lock_file reflog_lock;\n-\tstruct expire_reflog_cb cb;\n-\tstruct ref_lock *lock;\n-\tchar *log_file;\n-\tint status = 0;\n-\n-\tmemset(&cb, 0, sizeof(cb));\n-\tcb.flags = flags;\n-\tcb.policy_cb = policy_cb_data;\n-\n-\t/*\n-\t * The reflog file is locked by holding the lock on the\n-\t * reference itself, plus we might need to update the\n-\t * reference if --updateref was specified:\n-\t */\n-\tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n-\tif (!lock)\n-\t\treturn error(\"cannot lock ref '%s'\", refname);\n-\tif (!reflog_exists(refname)) {\n-\t\tunlock_ref(lock);\n-\t\treturn 0;\n-\t}\n-\n-\tlog_file = git_pathdup(\"logs/%s\", refname);\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\t/*\n-\t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n-\t\t * no locking implications, we use the lock_file\n-\t\t * machinery here anyway because it does a lot of the\n-\t\t * work we need, including cleaning up if the program\n-\t\t * exits unexpectedly.\n-\t\t */\n-\t\tif (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {\n-\t\t\tstruct strbuf err = STRBUF_INIT;\n-\t\t\tunable_to_lock_message(log_file, errno, &err);\n-\t\t\terror(\"%s\", err.buf);\n-\t\t\tstrbuf_release(&err);\n-\t\t\tgoto failure;\n-\t\t}\n-\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n-\t\tif (!cb.newlog) {\n-\t\t\terror(\"cannot fdopen %s (%s)\",\n-\t\t\t reflog_lock.filename.buf, strerror(errno));\n-\t\t\tgoto failure;\n-\t\t}\n-\t}\n-\n-\treflog_expiry_prepare(refname, sha1, cb.policy_cb);\n-\tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n-\treflog_expiry_cleanup(cb.policy_cb);\n-\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\tif (close_lock_file(&reflog_lock)) {\n-\t\t\tstatus |= error(\"couldn't write %s: %s\", log_file,\n-\t\t\t\t\tstrerror(errno));\n-\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n-\t\t\t(write_in_full(lock->lock_fd,\n-\t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n-\t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n-\t\t\t close_ref(lock) < 0)) {\n-\t\t\tstatus |= error(\"couldn't write %s\",\n-\t\t\t\t\tlock->lk->filename.buf);\n-\t\t\trollback_lock_file(&reflog_lock);\n-\t\t} else if (commit_lock_file(&reflog_lock)) {\n-\t\t\tstatus |= error(\"unable to commit reflog '%s' (%s)\",\n-\t\t\t\t\tlog_file, strerror(errno));\n-\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {\n-\t\t\tstatus |= error(\"couldn't set %s\", lock->ref_name);\n-\t\t}\n-\t}\n-\tfree(log_file);\n-\tunlock_ref(lock);\n-\treturn status;\n-\n- failure:\n-\trollback_lock_file(&reflog_lock);\n-\tfree(log_file);\n-\tunlock_ref(lock);\n-\treturn -1;\n-}\n-\n static int collect_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)\n {\n \tstruct collected_reflog *e;\n@@ -727,7 +597,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n-\t\t\tstatus |= expire_reflog(e->reflog, e->sha1, flags, &cb);\n+\t\t\tstatus |= reflog_expire(e->reflog, e->sha1, flags,\n+\t\t\t\t\t\treflog_expiry_prepare,\n+\t\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\t\treflog_expiry_cleanup,\n+\t\t\t\t\t\t&cb);\n \t\t\tfree(e);\n \t\t}\n \t\tfree(collected.e);\n@@ -741,7 +615,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);\n-\t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n+\t\tstatus |= reflog_expire(ref, sha1, flags,\n+\t\t\t\t\treflog_expiry_prepare,\n+\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\treflog_expiry_cleanup,\n+\t\t\t\t\t&cb);\n \t}\n \treturn status;\n }\n@@ -813,7 +691,11 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\t\tcb.cmd.expire_total = 0;\n \t\t}\n \n-\t\tstatus |= expire_reflog(ref, sha1, flags, &cb);\n+\t\tstatus |= reflog_expire(ref, sha1, flags,\n+\t\t\t\t\treflog_expiry_prepare,\n+\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\treflog_expiry_cleanup,\n+\t\t\t\t\t&cb);\n \t\tfree(ref);\n \t}\n \treturn status;\ndiff --git a/refs.c b/refs.c\nindex 150c980..0252fcc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3943,3 +3943,132 @@ int ref_is_hidden(const char *refname)\n \t}\n \treturn 0;\n }\n+\n+struct expire_reflog_cb {\n+\tunsigned int flags;\n+\treflog_expiry_should_prune_fn *should_prune_fn;\n+\tvoid *policy_cb;\n+\tFILE *newlog;\n+\tunsigned char last_kept_sha1[20];\n+};\n+\n+static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t const char *email, unsigned long timestamp, int tz,\n+\t\t\t const char *message, void *cb_data)\n+{\n+\tstruct expire_reflog_cb *cb = cb_data;\n+\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n+\n+\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n+\t\tosha1 = cb->last_kept_sha1;\n+\n+\tif ((*cb->should_prune_fn)(osha1, nsha1, email, timestamp, tz,\n+\t\t\t\t message, policy_cb)) {\n+\t\tif (!cb->newlog)\n+\t\t\tprintf(\"would prune %s\", message);\n+\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\t\tprintf(\"prune %s\", message);\n+\t} else {\n+\t\tif (cb->newlog) {\n+\t\t\tchar sign = (tz < 0) ? '-' : '+';\n+\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n+\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n+\t\t\t\temail, timestamp, sign, zone,\n+\t\t\t\tmessage);\n+\t\t\thashcpy(cb->last_kept_sha1, nsha1);\n+\t\t}\n+\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\t\tprintf(\"keep %s\", message);\n+\t}\n+\treturn 0;\n+}\n+\n+extern int reflog_expire(const char *refname, const unsigned char *sha1,\n+\t\t\t unsigned int flags,\n+\t\t\t reflog_expiry_prepare_fn prepare_fn,\n+\t\t\t reflog_expiry_should_prune_fn should_prune_fn,\n+\t\t\t reflog_expiry_cleanup_fn cleanup_fn,\n+\t\t\t void *policy_cb_data)\n+{\n+\tstatic struct lock_file reflog_lock;\n+\tstruct expire_reflog_cb cb;\n+\tstruct ref_lock *lock;\n+\tchar *log_file;\n+\tint status = 0;\n+\n+\tmemset(&cb, 0, sizeof(cb));\n+\tcb.flags = flags;\n+\tcb.policy_cb = policy_cb_data;\n+\tcb.should_prune_fn = should_prune_fn;\n+\n+\t/*\n+\t * The reflog file is locked by holding the lock on the\n+\t * reference itself, plus we might need to update the\n+\t * reference if --updateref was specified:\n+\t */\n+\tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n+\tif (!lock)\n+\t\treturn error(\"cannot lock ref '%s'\", refname);\n+\tif (!reflog_exists(refname)) {\n+\t\tunlock_ref(lock);\n+\t\treturn 0;\n+\t}\n+\n+\tlog_file = git_pathdup(\"logs/%s\", refname);\n+\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\t\t/*\n+\t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n+\t\t * no locking implications, we use the lock_file\n+\t\t * machinery here anyway because it does a lot of the\n+\t\t * work we need, including cleaning up if the program\n+\t\t * exits unexpectedly.\n+\t\t */\n+\t\tif (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {\n+\t\t\tstruct strbuf err = STRBUF_INIT;\n+\t\t\tunable_to_lock_message(log_file, errno, &err);\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t\tgoto failure;\n+\t\t}\n+\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n+\t\tif (!cb.newlog) {\n+\t\t\terror(\"cannot fdopen %s (%s)\",\n+\t\t\t reflog_lock.filename.buf, strerror(errno));\n+\t\t\tgoto failure;\n+\t\t}\n+\t}\n+\n+\t(*prepare_fn)(refname, sha1, cb.policy_cb);\n+\tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n+\t(*cleanup_fn)(cb.policy_cb);\n+\n+\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\t\tif (close_lock_file(&reflog_lock)) {\n+\t\t\tstatus |= error(\"couldn't write %s: %s\", log_file,\n+\t\t\t\t\tstrerror(errno));\n+\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n+\t\t\t(write_in_full(lock->lock_fd,\n+\t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n+\t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n+\t\t\t close_ref(lock) < 0)) {\n+\t\t\tstatus |= error(\"couldn't write %s\",\n+\t\t\t\t\tlock->lk->filename.buf);\n+\t\t\trollback_lock_file(&reflog_lock);\n+\t\t} else if (commit_lock_file(&reflog_lock)) {\n+\t\t\tstatus |= error(\"unable to commit reflog '%s' (%s)\",\n+\t\t\t\t\tlog_file, strerror(errno));\n+\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {\n+\t\t\tstatus |= error(\"couldn't set %s\", lock->ref_name);\n+\t\t}\n+\t}\n+\tfree(log_file);\n+\tunlock_ref(lock);\n+\treturn status;\n+\n+ failure:\n+\trollback_lock_file(&reflog_lock);\n+\tfree(log_file);\n+\tunlock_ref(lock);\n+\treturn -1;\n+}\ndiff --git a/refs.h b/refs.h\nindex 7d675b7..99e707b 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -353,4 +353,50 @@ int update_ref(const char *action, const char *refname,\n extern int parse_hide_refs_config(const char *var, const char *value, const char *);\n extern int ref_is_hidden(const char *);\n \n+enum expire_reflog_flags {\n+\tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n+\tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n+\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n+\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n+};\n+\n+/*\n+ * The following interface is used for reflog expiration. The caller\n+ * calls reflog_expire(), supplying it with three callback functions,\n+ * of the following types. The callback functions define the\n+ * expiration policy that is desired.\n+ *\n+ * reflog_expiry_prepare_fn -- Called once after the reference is\n+ * locked.\n+ *\n+ * reflog_expiry_should_prune_fn -- Called once for each entry in the\n+ * existing reflog. It should return true iff that entry should be\n+ * pruned.\n+ *\n+ * reflog_expiry_cleanup_fn -- Called once before the reference is\n+ * unlocked again.\n+ */\n+typedef void reflog_expiry_prepare_fn(const char *refname,\n+\t\t\t\t const unsigned char *sha1,\n+\t\t\t\t void *cb_data);\n+typedef int reflog_expiry_should_prune_fn(unsigned char *osha1,\n+\t\t\t\t\t unsigned char *nsha1,\n+\t\t\t\t\t const char *email,\n+\t\t\t\t\t unsigned long timestamp, int tz,\n+\t\t\t\t\t const char *message, void *cb_data);\n+typedef void reflog_expiry_cleanup_fn(void *cb_data);\n+\n+/*\n+ * Expire reflog entries for the specified reference. sha1 is the old\n+ * value of the reference. flags is a combination of the constants in\n+ * enum expire_reflog_flags. The three function pointers are described\n+ * above. On success, return zero.\n+ */\n+extern int reflog_expire(const char *refname, const unsigned char *sha1,\n+\t\t\t unsigned int flags,\n+\t\t\t reflog_expiry_prepare_fn prepare_fn,\n+\t\t\t reflog_expiry_should_prune_fn should_prune_fn,\n+\t\t\t reflog_expiry_cleanup_fn cleanup_fn,\n+\t\t\t void *policy_cb_data);\n+\n #endif /* REFS_H */\n-- \n2.1.3\n"},{"id":"253620","messageId":"1418374623-5566-22-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 21/24] refs.c: remove unlock_ref/close_ref/commit_ref from the refs api","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:57:00Z","receivedAt":"2014-12-12T08:57:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nunlock|close|commit_ref can be made static since there are no more external\ncallers.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Stefan Beller \nSigned-off-by: Michael Haggerty \n---\n refs.c | 24 ++++++++++++------------\n refs.h | 9 ---------\n 2 files changed, 12 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0252fcc..618ef9c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2090,6 +2090,16 @@ int refname_match(const char *abbrev_name, const char *full_name)\n \treturn 0;\n }\n \n+static void unlock_ref(struct ref_lock *lock)\n+{\n+\t/* Do not free lock->lk -- atexit() still looks at them */\n+\tif (lock->lk)\n+\t\trollback_lock_file(lock->lk);\n+\tfree(lock->ref_name);\n+\tfree(lock->orig_ref_name);\n+\tfree(lock);\n+}\n+\n /* This function should make sure errno is meaningful on error */\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n@@ -2888,7 +2898,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \treturn 1;\n }\n \n-int close_ref(struct ref_lock *lock)\n+static int close_ref(struct ref_lock *lock)\n {\n \tif (close_lock_file(lock->lk))\n \t\treturn -1;\n@@ -2896,7 +2906,7 @@ int close_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n-int commit_ref(struct ref_lock *lock)\n+static int commit_ref(struct ref_lock *lock)\n {\n \tif (commit_lock_file(lock->lk))\n \t\treturn -1;\n@@ -2904,16 +2914,6 @@ int commit_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n-void unlock_ref(struct ref_lock *lock)\n-{\n-\t/* Do not free lock->lk -- atexit() still looks at them */\n-\tif (lock->lk)\n-\t\trollback_lock_file(lock->lk);\n-\tfree(lock->ref_name);\n-\tfree(lock->orig_ref_name);\n-\tfree(lock);\n-}\n-\n /*\n * copy the reflog message msg to buf, which has been allocated sufficiently\n * large, while cleaning up the whitespaces. Especially, convert LF to space,\ndiff --git a/refs.h b/refs.h\nindex 99e707b..4bb58b9 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -198,15 +198,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n \n-/** Close the file descriptor owned by a lock and return the status */\n-extern int close_ref(struct ref_lock *lock);\n-\n-/** Close and commit the ref locked by the lock */\n-extern int commit_ref(struct ref_lock *lock);\n-\n-/** Release any lock taken but not written. **/\n-extern void unlock_ref(struct ref_lock *lock);\n-\n /*\n * Setup reflog before using. Set errno to something meaningful on failure.\n */\n-- \n2.1.3\n"},{"id":"253619","messageId":"1418374623-5566-23-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 22/24] lock_any_ref_for_update(): inline function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:57:01Z","receivedAt":"2014-12-12T08:57:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Inline the function at its one remaining caller (which is within\nrefs.c) and remove it.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 9 +--------\n refs.h | 9 +--------\n 2 files changed, 2 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 618ef9c..166c0f6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2346,13 +2346,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \treturn NULL;\n }\n \n-struct ref_lock *lock_any_ref_for_update(const char *refname,\n-\t\t\t\t\t const unsigned char *old_sha1,\n-\t\t\t\t\t int flags, int *type_p)\n-{\n-\treturn lock_ref_sha1_basic(refname, old_sha1, NULL, flags, type_p);\n-}\n-\n /*\n * Write an entry to the packed-refs file for the specified refname.\n * If peeled is non-NULL, write it as the entry's peeled value.\n@@ -4007,7 +4000,7 @@ extern int reflog_expire(const char *refname, const unsigned char *sha1,\n \t * reference itself, plus we might need to update the\n \t * reference if --updateref was specified:\n \t */\n-\tlock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n+\tlock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);\n \tif (!lock)\n \t\treturn error(\"cannot lock ref '%s'\", refname);\n \tif (!reflog_exists(refname)) {\ndiff --git a/refs.h b/refs.h\nindex 4bb58b9..28e7834 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -181,8 +181,7 @@ extern int is_branch(const char *refname);\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /*\n- * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),\n- * ref_transaction_create(), etc.\n+ * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n * REF_NODEREF: act on the ref directly, instead of dereferencing\n * symbolic references.\n * REF_DELETING: tolerate broken refs\n@@ -191,12 +190,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n */\n #define REF_NODEREF\t0x01\n #define REF_DELETING\t0x02\n-/*\n- * This function sets errno to something meaningful on failure.\n- */\n-extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n-\t\t\t\t\t\tconst unsigned char *old_sha1,\n-\t\t\t\t\t\tint flags, int *type_p);\n \n /*\n * Setup reflog before using. Set errno to something meaningful on failure.\n-- \n2.1.3\n"},{"id":"253618","messageId":"1418374623-5566-24-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 23/24] refs.c: don't expose the internal struct ref_lock in the header file","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:57:02Z","receivedAt":"2014-12-12T08:57:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Stefan Beller \n\nNow the struct ref_lock is used completely internally, so let's\nremove it from the header file.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Michael Haggerty \n---\n refs.c | 9 +++++++++\n refs.h | 9 ---------\n 2 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 166c0f6..582a051 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -6,6 +6,15 @@\n #include \"dir.h\"\n #include \"string-list.h\"\n \n+struct ref_lock {\n+\tchar *ref_name;\n+\tchar *orig_ref_name;\n+\tstruct lock_file *lk;\n+\tunsigned char old_sha1[20];\n+\tint lock_fd;\n+\tint force_write;\n+};\n+\n /*\n * How to handle various characters in refnames:\n * 0: An acceptable character for refs\ndiff --git a/refs.h b/refs.h\nindex 28e7834..fc88ba6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -1,15 +1,6 @@\n #ifndef REFS_H\n #define REFS_H\n \n-struct ref_lock {\n-\tchar *ref_name;\n-\tchar *orig_ref_name;\n-\tstruct lock_file *lk;\n-\tunsigned char old_sha1[20];\n-\tint lock_fd;\n-\tint force_write;\n-};\n-\n /*\n * A ref_transaction represents a collection of ref updates\n * that should succeed or fail together.\n-- \n2.1.3\n"},{"id":"253617","messageId":"1418374623-5566-25-git-send-email-mhagger@alum.mit.edu","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 24/24] refs.c: let fprintf handle the formatting","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T08:57:03Z","receivedAt":"2014-12-12T08:57:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Stefan Beller \n\nInstead of calculating whether to put a plus or minus sign, offload\nthe responsibilty to the fprintf function.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Michael Haggerty \n---\n refs.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 582a051..9e6d21c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3972,12 +3972,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n \t\tif (cb->newlog) {\n-\t\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %+05d\\t%s\",\n \t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\t\temail, timestamp, sign, zone,\n-\t\t\t\tmessage);\n+\t\t\t\temail, timestamp, tz, message);\n \t\t\thashcpy(cb->last_kept_sha1, nsha1);\n \t\t}\n \t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-- \n2.1.3\n"},{"id":"253638","messageId":"CAGZ79kZOa77DKLnvinhgAVwCeRRXWMD5Krk8QK6N170+oqA-xA@mail.gmail.com","threadId":"38171","inReplyTo":"1418374623-5566-5-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 04/24] expire_reflog(): it's not an each_ref_fn anymore","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T18:09:55Z","receivedAt":"2014-12-12T18:09:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 12:56 AM, Michael Haggerty wrote:\n> Prior to v1.5.4~14, expire_reflog() had to be an each_ref_fn because\n> it was passed to for_each_reflog(). Since then, there has been no\n> reason for it to implement the each_ref_fn interface. So...\n>\n> * Remove the \"unused\" parameter (which took the place of \"flags\", but\n> was really unused).\n>\n> * Declare the last parameter to be (struct cmd_reflog_expire_cb *)\n> rather than (void *).\n>\n> Helped-by: Jonathan Nieder \n> Signed-off-by: Michael Haggerty \n\nReviewed-by: Stefan Beller \n\n> ---\n> builtin/reflog.c | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index 2d85d26..160541a 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -349,9 +349,9 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n> return 0;\n> }\n>\n> -static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)\n> +static int expire_reflog(const char *ref, const unsigned char *sha1,\n> + struct cmd_reflog_expire_cb *cmd)\n> {\n> - struct cmd_reflog_expire_cb *cmd = cb_data;\n> struct expire_reflog_cb cb;\n> struct ref_lock *lock;\n> char *log_file, *newlog_path = NULL;\n> @@ -663,7 +663,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n> for (i = 0; i < collected.nr; i++) {\n> struct collected_reflog *e = collected.e[i];\n> set_reflog_expiry_param(&cb, explicit_expiry, e->reflog);\n> - status |= expire_reflog(e->reflog, e->sha1, 0, &cb);\n> + status |= expire_reflog(e->reflog, e->sha1, &cb);\n> free(e);\n> }\n> free(collected.e);\n> @@ -677,7 +677,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n> continue;\n> }\n> set_reflog_expiry_param(&cb, explicit_expiry, ref);\n> - status |= expire_reflog(ref, sha1, 0, &cb);\n> + status |= expire_reflog(ref, sha1, &cb);\n> }\n> return status;\n> }\n> @@ -748,7 +748,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n> cb.expire_total = 0;\n> }\n>\n> - status |= expire_reflog(ref, sha1, 0, &cb);\n> + status |= expire_reflog(ref, sha1, &cb);\n> free(ref);\n> }\n> return status;\n> --\n> 2.1.3\n>\n"},{"id":"253639","messageId":"CAGZ79kYHWZW0Q15nx7xWdzw8-2iL-rT034T-vQ4u9nz3Gi7JNQ@mail.gmail.com","threadId":"38171","inReplyTo":"1418374623-5566-7-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 06/24] expire_reflog(): return early if the reference has no reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T18:15:14Z","receivedAt":"2014-12-12T18:15:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 12:56 AM, Michael Haggerty wrote:\n> There is very little cleanup needed if the reference has no reflog. If\n> we move the initialization of log_file down a bit, there's even less.\n> So instead of jumping to the cleanup code at the end of the function,\n> just do the cleanup and return inline.\n>\n> Signed-off-by: Michael Haggerty \n> Reviewed-by: Jonathan Nieder \n\nReviewed-by: Stefan Beller \n\n> ---\n> builtin/reflog.c | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index ff51dbf..37b33c9 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -368,9 +368,11 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n> lock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n> if (!lock)\n> return error(\"cannot lock ref '%s'\", refname);\n> + if (!reflog_exists(refname)) {\n> + unlock_ref(lock);\n> + return 0;\n> + }\n> log_file = git_pathdup(\"logs/%s\", refname);\n> - if (!reflog_exists(refname))\n> - goto finish;\n> if (!cmd->dry_run) {\n> newlog_path = git_pathdup(\"logs/%s.lock\", refname);\n> cb.newlog = fopen(newlog_path, \"w\");\n> @@ -419,7 +421,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n> clear_commit_marks(tip_commit, REACHABLE);\n> }\n> }\n> - finish:\n> +\n> if (cb.newlog) {\n> if (fclose(cb.newlog)) {\n> status |= error(\"%s: %s\", strerror(errno),\n> --\n> 2.1.3\n>\n"},{"id":"253644","messageId":"CAGZ79kaPzMPcxMqsTeW5LjYNc7LbpjHE5eBPHWKvUkBU3NvGJw@mail.gmail.com","threadId":"38171","inReplyTo":"1418374623-5566-8-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 07/24] expire_reflog(): use a lock_file for rewriting the reflog file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T18:57:27Z","receivedAt":"2014-12-12T18:57:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 12:56 AM, Michael Haggerty wrote:\n> We don't actually need the locking functionality, because we already\n> hold the lock on the reference itself, which is how the reflog file is\n> locked. But the lock_file code can do some of the bookkeeping for us,\n> and it is more careful than the old code here was. For example:\n>\n> * It correctly handles the case that the reflog lock file already\n> exists for some reason or cannot be opened.\n>\n> * It correctly cleans up the lockfile if the program dies.\n>\n> Signed-off-by: Michael Haggerty \n> ---\n> builtin/reflog.c | 60 ++++++++++++++++++++++++++++++++++++++------------------\n> 1 file changed, 41 insertions(+), 19 deletions(-)\n>\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index 37b33c9..ba5b3d3 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -352,9 +352,10 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int\n> static int expire_reflog(const char *refname, const unsigned char *sha1,\n> struct cmd_reflog_expire_cb *cmd)\n> {\n> + static struct lock_file reflog_lock;\n> struct expire_reflog_cb cb;\n> struct ref_lock *lock;\n> - char *log_file, *newlog_path = NULL;\n> + char *log_file;\n> struct commit *tip_commit;\n> struct commit_list *tips;\n> int status = 0;\n> @@ -362,8 +363,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n> memset(&cb, 0, sizeof(cb));\n>\n> /*\n> - * we take the lock for the ref itself to prevent it from\n> - * getting updated.\n> + * The reflog file is locked by holding the lock on the\n> + * reference itself, plus we might need to update the\n> + * reference if --updateref was specified:\n> */\n> lock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n> if (!lock)\n> @@ -372,10 +374,29 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n> unlock_ref(lock);\n> return 0;\n> }\n> +\n> log_file = git_pathdup(\"logs/%s\", refname);\n> if (!cmd->dry_run) {\n> - newlog_path = git_pathdup(\"logs/%s.lock\", refname);\n> - cb.newlog = fopen(newlog_path, \"w\");\n> + /*\n> + * Even though holding $GIT_DIR/logs/$reflog.lock has\n> + * no locking implications, we use the lock_file\n> + * machinery here anyway because it does a lot of the\n> + * work we need, including cleaning up if the program\n> + * exits unexpectedly.\n> + */\n> + if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {\n> + struct strbuf err = STRBUF_INIT;\n> + unable_to_lock_message(log_file, errno, &err);\n> + error(\"%s\", err.buf);\n> + strbuf_release(&err);\n> + goto failure;\n> + }\n> + cb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n> + if (!cb.newlog) {\n> + error(\"cannot fdopen %s (%s)\",\n> + reflog_lock.filename.buf, strerror(errno));\n> + goto failure;\n> + }\n> }\n>\n> cb.cmd = cmd;\n> @@ -423,32 +444,33 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,\n> }\n>\n> if (cb.newlog) {\n> - if (fclose(cb.newlog)) {\n> - status |= error(\"%s: %s\", strerror(errno),\n> - newlog_path);\n> - unlink(newlog_path);\n> + if (close_lock_file(&reflog_lock)) {\n> + status |= error(\"couldn't write %s: %s\", log_file,\n> + strerror(errno));\n> } else if (cmd->updateref &&\n> (write_in_full(lock->lock_fd,\n> sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n\noptional nit:\nWhile being here, you may fix the indentation of the sha1_to_hex to align to the\nopening parentheses of write_in_full the line before.\n\n> write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n> close_ref(lock) < 0)) {\n> - status |= error(\"Couldn't write %s\",\n> + status |= error(\"couldn't write %s\",\n> lock->lk->filename.buf);\n> - unlink(newlog_path);\n> - } else if (rename(newlog_path, log_file)) {\n> - status |= error(\"cannot rename %s to %s\",\n> - newlog_path, log_file);\n> - unlink(newlog_path);\n> + rollback_lock_file(&reflog_lock);\n> + } else if (commit_lock_file(&reflog_lock)) {\n> + status |= error(\"unable to commit reflog '%s' (%s)\",\n> + log_file, strerror(errno));\n> } else if (cmd->updateref && commit_ref(lock)) {\n> - status |= error(\"Couldn't set %s\", lock->ref_name);\n> - } else {\n> - adjust_shared_perm(log_file);\n\nSo we don't need adjust_shared_perm anymore?\n\nI have been looking at this excerpt for a while now and I am confused by the\nfirst and third statement in the origin/master state as well as after\nyour changes.\nSo it's essentially:\n * write out the reflog lock file\n * update ref in the lock file\n * rename the reflog lock into place\n * rename the ref lock into place\n * (in origin/master) if everything went fine, call adjust_shared_perm.\n * In your patch we actually don't need it as on locking the lock\nfile there is a call\n to adjust_shared_perm already.\n\nSo I think it's clear to me now and looks good. It took me sometime to\nunderstand\nit. Maybe that's because I just got up or it's in fact hard to\nunderstand. Would comments\nhelp in laying out the sequence of actions beforehand?\n\nI think the most confusing thing was the close_lock_file as a first\nstatement in the if chain.\nclose_lock_file is primarily used here to have the file written out to\ndisk, but the function\nname suggests that the primary functionality is the closing of the fd.\n\n\n> + status |= error(\"couldn't set %s\", lock->ref_name);\n> }\n> }\n> - free(newlog_path);\n> free(log_file);\n> unlock_ref(lock);\n> return status;\n> +\n> + failure:\n> + rollback_lock_file(&reflog_lock);\n> + free(log_file);\n> + unlock_ref(lock);\n> + return -1;\n> }\n>\n> static int collect_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)\n> --\n> 2.1.3\n>\n"},{"id":"253652","messageId":"CAGZ79kYYoJL4oXzTq0w7rV-qVfdn0HnDGCua1ONd5MKD91PJ-Q@mail.gmail.com","threadId":"38171","inReplyTo":"1418374623-5566-9-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 08/24] Extract function should_expire_reflog_ent()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T20:02:09Z","receivedAt":"2014-12-12T20:02:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 12:56 AM, Michael Haggerty wrote:\n> Extract from expire_reflog_ent() a function that is solely responsible\n> for deciding whether a reflog entry should be expired. By separating\n> this \"business logic\" from the mechanics of actually expiring entries,\n> we are working towards the goal of encapsulating reflog expiry within\n> the refs API, with policy decided by a callback function passed to it\n> by its caller.\n>\n> Signed-off-by: Michael Haggerty \n\nReviewed-by: Stefan Beller \n\n> ---\n> builtin/reflog.c | 70 +++++++++++++++++++++++++++++++++-----------------------\n> 1 file changed, 42 insertions(+), 28 deletions(-)\n>\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index ba5b3d3..06ce8b1 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -288,51 +288,65 @@ static int unreachable(struct expire_reflog_cb *cb, struct commit *commit, unsig\n> return !(commit->object.flags & REACHABLE);\n> }\n>\n> -static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n> - const char *email, unsigned long timestamp, int tz,\n> - const char *message, void *cb_data)\n> +/*\n> + * Return true iff the specified reflog entry should be expired.\n> + */\n> +static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n> + const char *email, unsigned long timestamp, int tz,\n> + const char *message, void *cb_data)\n> {\n> struct expire_reflog_cb *cb = cb_data;\n> struct commit *old, *new;\n>\n> if (timestamp < cb->cmd->expire_total)\n> - goto prune;\n> -\n> - if (cb->cmd->rewrite)\n> - osha1 = cb->last_kept_sha1;\n> + return 1;\n>\n> old = new = NULL;\n> if (cb->cmd->stalefix &&\n> (!keep_entry(&old, osha1) || !keep_entry(&new, nsha1)))\n> - goto prune;\n> + return 1;\n>\n> if (timestamp < cb->cmd->expire_unreachable) {\n> if (cb->unreachable_expire_kind == UE_ALWAYS)\n> - goto prune;\n> + return 1;\n> if (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))\n> - goto prune;\n> + return 1;\n> }\n>\n> if (cb->cmd->recno && --(cb->cmd->recno) == 0)\n> - goto prune;\n> -\n> - if (cb->newlog) {\n> - char sign = (tz < 0) ? '-' : '+';\n> - int zone = (tz < 0) ? (-tz) : tz;\n> - fprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n> - sha1_to_hex(osha1), sha1_to_hex(nsha1),\n> - email, timestamp, sign, zone,\n> - message);\n> - hashcpy(cb->last_kept_sha1, nsha1);\n> - }\n> - if (cb->cmd->verbose)\n> - printf(\"keep %s\", message);\n> + return 1;\n> +\n> return 0;\n> - prune:\n> - if (!cb->newlog)\n> - printf(\"would prune %s\", message);\n> - else if (cb->cmd->verbose)\n> - printf(\"prune %s\", message);\n> +}\n> +\n> +static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n> + const char *email, unsigned long timestamp, int tz,\n> + const char *message, void *cb_data)\n> +{\n> + struct expire_reflog_cb *cb = cb_data;\n> +\n> + if (cb->cmd->rewrite)\n> + osha1 = cb->last_kept_sha1;\n> +\n> + if (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,\n> + message, cb_data)) {\n> + if (!cb->newlog)\n> + printf(\"would prune %s\", message);\n> + else if (cb->cmd->verbose)\n> + printf(\"prune %s\", message);\n> + } else {\n> + if (cb->newlog) {\n> + char sign = (tz < 0) ? '-' : '+';\n> + int zone = (tz < 0) ? (-tz) : tz;\n> + fprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n> + sha1_to_hex(osha1), sha1_to_hex(nsha1),\n> + email, timestamp, sign, zone,\n> + message);\n> + hashcpy(cb->last_kept_sha1, nsha1);\n> + }\n> + if (cb->cmd->verbose)\n> + printf(\"keep %s\", message);\n> + }\n> return 0;\n> }\n>\n> --\n> 2.1.3\n>\n"},{"id":"253653","messageId":"CAGZ79kaw=BnZzAuP5+Krw_agY=vtocjAiVEFuuwYoNmMnQ6_qw@mail.gmail.com","threadId":"38171","inReplyTo":"1418374623-5566-23-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 22/24] lock_any_ref_for_update(): inline function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T20:04:27Z","receivedAt":"2014-12-12T20:04:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 12:57 AM, Michael Haggerty wrote:\n> Inline the function at its one remaining caller (which is within\n> refs.c) and remove it.\n>\n> Signed-off-by: Michael Haggerty \n\nReviewed-by: Stefan Beller \n> ---\n> refs.c | 9 +--------\n> refs.h | 9 +--------\n> 2 files changed, 2 insertions(+), 16 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 618ef9c..166c0f6 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2346,13 +2346,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n> return NULL;\n> }\n>\n> -struct ref_lock *lock_any_ref_for_update(const char *refname,\n> - const unsigned char *old_sha1,\n> - int flags, int *type_p)\n> -{\n> - return lock_ref_sha1_basic(refname, old_sha1, NULL, flags, type_p);\n> -}\n> -\n> /*\n> * Write an entry to the packed-refs file for the specified refname.\n> * If peeled is non-NULL, write it as the entry's peeled value.\n> @@ -4007,7 +4000,7 @@ extern int reflog_expire(const char *refname, const unsigned char *sha1,\n> * reference itself, plus we might need to update the\n> * reference if --updateref was specified:\n> */\n> - lock = lock_any_ref_for_update(refname, sha1, 0, NULL);\n> + lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);\n> if (!lock)\n> return error(\"cannot lock ref '%s'\", refname);\n> if (!reflog_exists(refname)) {\n> diff --git a/refs.h b/refs.h\n> index 4bb58b9..28e7834 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -181,8 +181,7 @@ extern int is_branch(const char *refname);\n> extern int peel_ref(const char *refname, unsigned char *sha1);\n>\n> /*\n> - * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),\n> - * ref_transaction_create(), etc.\n> + * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n> * REF_NODEREF: act on the ref directly, instead of dereferencing\n> * symbolic references.\n> * REF_DELETING: tolerate broken refs\n> @@ -191,12 +190,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n> */\n> #define REF_NODEREF 0x01\n> #define REF_DELETING 0x02\n> -/*\n> - * This function sets errno to something meaningful on failure.\n> - */\n> -extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n> - const unsigned char *old_sha1,\n> - int flags, int *type_p);\n>\n> /*\n> * Setup reflog before using. Set errno to something meaningful on failure.\n> --\n> 2.1.3\n>\n"},{"id":"253659","messageId":"xmqqegs4bn61.fsf@gitster.dls.corp.google.com","threadId":"38171","inReplyTo":"1418374623-5566-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 00/24] Add reflog_expire() to the references API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-12T20:58:30Z","receivedAt":"2014-12-12T20:58:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> This is v2 of the series. Thanks to Jonathan, Stefan, Junio, and\n> Ronnie for their feedback on v1 [1]. I think I have addressed all of\n> the issues that were raised.\n>\n> Changes since v1:\n>\n> * Several improvements to commit messages and comments; added some\n> Reviewed-by comments from the mailing list.\n>\n> * Change the signature of expire_reflog() early in the series to cast\n> off its heritage as an each_ref_fn.\n>\n> * Move the static lock_file object into expire_reflog(), and explain\n> the locking policy better.\n>\n> * Report errors if hold_lock_file_for_update() or fdopen_lock_file()\n> fails.\n>\n> * Fix the capitalization in some error messages.\n>\n> * When \"enum expire_reflog_flags\" is first introduced, put its\n> definition earlier in the file so that a later patch in the series\n> doesn't have to move it.\n>\n> * Rename reflog_expiry_select_fn to reflog_expiry_should_prune_fn.\n>\n> * Append Stefan's patch 24/24 \"refs.c: let fprintf handle the\n> formatting\"\n\nAll looked reasonable (three patches in the series somehow stood out\nin a funny way in shortlog output, though).\n\nWill queue. I think a few topics from Stefan I have in 'pu' can\nsafely be dropped as the early part of this series has the same.\n\nThanks.\n\n\n>\n> This branch is also available on GitHub:\n>\n> https://github.com/mhagger/git.git, branch reflog-expire-api-v2\n>\n> Michael\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/260812\n>\n> Michael Haggerty (18):\n> expire_reflog(): it's not an each_ref_fn anymore\n> expire_reflog(): rename \"ref\" parameter to \"refname\"\n> expire_reflog(): return early if the reference has no reflog\n> expire_reflog(): use a lock_file for rewriting the reflog file\n> Extract function should_expire_reflog_ent()\n> expire_reflog(): extract two policy-related functions\n> expire_reflog(): add a \"flags\" argument\n> expire_reflog(): move dry_run to flags argument\n> expire_reflog(): move updateref to flags argument\n> Rename expire_reflog_cb to expire_reflog_policy_cb\n> struct expire_reflog_cb: a new callback data type\n> expire_reflog(): pass flags through to expire_reflog_ent()\n> expire_reflog(): move verbose to flags argument\n> expire_reflog(): move rewrite to flags argument\n> Move newlog and last_kept_sha1 to \"struct expire_reflog_cb\"\n> expire_reflog(): treat the policy callback data as opaque\n> reflog_expire(): new function in the reference API\n> lock_any_ref_for_update(): inline function\n>\n> Ronnie Sahlberg (4):\n> refs.c: make ref_transaction_create a wrapper for\n> ref_transaction_update\n> refs.c: make ref_transaction_delete a wrapper for\n> ref_transaction_update\n> refs.c: add a function to append a reflog entry to a fd\n> refs.c: remove unlock_ref/close_ref/commit_ref from the refs api\n>\n> Stefan Beller (2):\n> refs.c: don't expose the internal struct ref_lock in the header file\n> refs.c: let fprintf handle the formatting\n>\n> builtin/reflog.c | 259 ++++++++++++++++++++++--------------------------------\n> refs.c | 263 ++++++++++++++++++++++++++++++++++++++-----------------\n> refs.h | 75 ++++++++++------\n> 3 files changed, 332 insertions(+), 265 deletions(-)\n"}]}