{"thread":{"id":"38020","subject":"[PATCH] refs.c: move reflog updates into its own function","startedAt":"2014-11-21T00:38:02Z","lastAt":"2014-11-24T21:50:39Z","messageCount":3,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"252315","messageId":"1416530282-13192-1-git-send-email-sbeller@google.com","threadId":"38020","inReplyTo":null,"subject":"[PATCH] refs.c: move reflog updates into its own function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-21T00:38:02Z","receivedAt":"2014-11-21T00:38:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg \n\nwrite_ref_sha1 tries to update the reflog while updating the ref.\nMove these reflog changes out into its own function so that we can do the\nsame thing if we write a sha1 ref differently, for example by writing a ref\nto the packed refs file instead.\n\nNo functional changes intended. We only move some code out into a separate\nfunction.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Stefan Beller \n---\n\n Examining the refs-transaction-reflog series a bit closer, this seems to be \n one of the last independant patches, which make sense to rip out on a per-patch \n basis.\n \n \n refs.c | 60 +++++++++++++++++++++++++++++++++++-------------------------\n 1 file changed, 35 insertions(+), 25 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 005eb18..6837367 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3043,6 +3043,40 @@ int is_branch(const char *refname)\n \treturn !strcmp(refname, \"HEAD\") || starts_with(refname, \"refs/heads/\");\n }\n \n+static int write_sha1_update_reflog(struct ref_lock *lock,\n+\tconst unsigned char *sha1, const char *logmsg)\n+{\n+\tif (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||\n+\t (strcmp(lock->ref_name, lock->orig_ref_name) &&\n+\t log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {\n+\t\tunlock_ref(lock);\n+\t\treturn -1;\n+\t}\n+\tif (strcmp(lock->orig_ref_name, \"HEAD\") != 0) {\n+\t\t/*\n+\t\t * Special hack: If a branch is updated directly and HEAD\n+\t\t * points to it (may happen on the remote side of a push\n+\t\t * for example) then logically the HEAD reflog should be\n+\t\t * updated too.\n+\t\t * A generic solution implies reverse symref information,\n+\t\t * but finding all symrefs pointing to the given branch\n+\t\t * would be rather costly for this rare event (the direct\n+\t\t * update of a branch) to be worth it. So let's cheat and\n+\t\t * check with HEAD only which should cover 99% of all usage\n+\t\t * scenarios (even 100% of the default ones).\n+\t\t */\n+\t\tunsigned char head_sha1[20];\n+\t\tint head_flag;\n+\t\tconst char *head_ref;\n+\t\thead_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t\t head_sha1, &head_flag);\n+\t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n+\t\t !strcmp(head_ref, lock->ref_name))\n+\t\t\tlog_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n+\t}\n+\treturn 0;\n+}\n+\n /*\n * Write sha1 into the ref specified by the lock. Make sure that errno\n * is sane on error.\n@@ -3086,34 +3120,10 @@ static int write_ref_sha1(struct ref_lock *lock,\n \t\treturn -1;\n \t}\n \tclear_loose_ref_cache(&ref_cache);\n-\tif (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||\n-\t (strcmp(lock->ref_name, lock->orig_ref_name) &&\n-\t log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {\n+\tif (write_sha1_update_reflog(lock, sha1, logmsg)) {\n \t\tunlock_ref(lock);\n \t\treturn -1;\n \t}\n-\tif (strcmp(lock->orig_ref_name, \"HEAD\") != 0) {\n-\t\t/*\n-\t\t * Special hack: If a branch is updated directly and HEAD\n-\t\t * points to it (may happen on the remote side of a push\n-\t\t * for example) then logically the HEAD reflog should be\n-\t\t * updated too.\n-\t\t * A generic solution implies reverse symref information,\n-\t\t * but finding all symrefs pointing to the given branch\n-\t\t * would be rather costly for this rare event (the direct\n-\t\t * update of a branch) to be worth it. So let's cheat and\n-\t\t * check with HEAD only which should cover 99% of all usage\n-\t\t * scenarios (even 100% of the default ones).\n-\t\t */\n-\t\tunsigned char head_sha1[20];\n-\t\tint head_flag;\n-\t\tconst char *head_ref;\n-\t\thead_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t\t head_sha1, &head_flag);\n-\t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n-\t\t !strcmp(head_ref, lock->ref_name))\n-\t\t\tlog_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n-\t}\n \tif (commit_ref(lock)) {\n \t\terror(\"Couldn't set %s\", lock->ref_name);\n \t\tunlock_ref(lock);\n-- \n2.2.0.rc2.23.gca0107e\n"},{"id":"252452","messageId":"CAGZ79kY8FagM_RPMZ9AchBmSK=BWR1A+4JQ7DiG2JmvyvtsPiw@mail.gmail.com","threadId":"38020","inReplyTo":"1416530282-13192-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] refs.c: move reflog updates into its own function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-24T21:24:50Z","receivedAt":"2014-11-24T21:24:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Anything holding this back?\n\nOn Thu, Nov 20, 2014 at 4:38 PM, Stefan Beller wrote:\n> From: Ronnie Sahlberg \n>\n> write_ref_sha1 tries to update the reflog while updating the ref.\n> Move these reflog changes out into its own function so that we can do the\n> same thing if we write a sha1 ref differently, for example by writing a ref\n> to the packed refs file instead.\n>\n> No functional changes intended. We only move some code out into a separate\n> function.\n>\n> Signed-off-by: Ronnie Sahlberg \n> Signed-off-by: Stefan Beller \n> ---\n>\n> Examining the refs-transaction-reflog series a bit closer, this seems to be\n> one of the last independant patches, which make sense to rip out on a per-patch\n> basis.\n>\n>\n> refs.c | 60 +++++++++++++++++++++++++++++++++++-------------------------\n> 1 file changed, 35 insertions(+), 25 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 005eb18..6837367 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3043,6 +3043,40 @@ int is_branch(const char *refname)\n> return !strcmp(refname, \"HEAD\") || starts_with(refname, \"refs/heads/\");\n> }\n>\n> +static int write_sha1_update_reflog(struct ref_lock *lock,\n> + const unsigned char *sha1, const char *logmsg)\n> +{\n> + if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||\n> + (strcmp(lock->ref_name, lock->orig_ref_name) &&\n> + log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {\n> + unlock_ref(lock);\n> + return -1;\n> + }\n> + if (strcmp(lock->orig_ref_name, \"HEAD\") != 0) {\n> + /*\n> + * Special hack: If a branch is updated directly and HEAD\n> + * points to it (may happen on the remote side of a push\n> + * for example) then logically the HEAD reflog should be\n> + * updated too.\n> + * A generic solution implies reverse symref information,\n> + * but finding all symrefs pointing to the given branch\n> + * would be rather costly for this rare event (the direct\n> + * update of a branch) to be worth it. So let's cheat and\n> + * check with HEAD only which should cover 99% of all usage\n> + * scenarios (even 100% of the default ones).\n> + */\n> + unsigned char head_sha1[20];\n> + int head_flag;\n> + const char *head_ref;\n> + head_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n> + head_sha1, &head_flag);\n> + if (head_ref && (head_flag & REF_ISSYMREF) &&\n> + !strcmp(head_ref, lock->ref_name))\n> + log_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n> + }\n> + return 0;\n> +}\n> +\n> /*\n> * Write sha1 into the ref specified by the lock. Make sure that errno\n> * is sane on error.\n> @@ -3086,34 +3120,10 @@ static int write_ref_sha1(struct ref_lock *lock,\n> return -1;\n> }\n> clear_loose_ref_cache(&ref_cache);\n> - if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||\n> - (strcmp(lock->ref_name, lock->orig_ref_name) &&\n> - log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {\n> + if (write_sha1_update_reflog(lock, sha1, logmsg)) {\n> unlock_ref(lock);\n> return -1;\n> }\n> - if (strcmp(lock->orig_ref_name, \"HEAD\") != 0) {\n> - /*\n> - * Special hack: If a branch is updated directly and HEAD\n> - * points to it (may happen on the remote side of a push\n> - * for example) then logically the HEAD reflog should be\n> - * updated too.\n> - * A generic solution implies reverse symref information,\n> - * but finding all symrefs pointing to the given branch\n> - * would be rather costly for this rare event (the direct\n> - * update of a branch) to be worth it. So let's cheat and\n> - * check with HEAD only which should cover 99% of all usage\n> - * scenarios (even 100% of the default ones).\n> - */\n> - unsigned char head_sha1[20];\n> - int head_flag;\n> - const char *head_ref;\n> - head_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n> - head_sha1, &head_flag);\n> - if (head_ref && (head_flag & REF_ISSYMREF) &&\n> - !strcmp(head_ref, lock->ref_name))\n> - log_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n> - }\n> if (commit_ref(lock)) {\n> error(\"Couldn't set %s\", lock->ref_name);\n> unlock_ref(lock);\n> --\n> 2.2.0.rc2.23.gca0107e\n>\n"},{"id":"252457","messageId":"xmqqy4r0uv2o.fsf@gitster.dls.corp.google.com","threadId":"38020","inReplyTo":"CAGZ79kY8FagM_RPMZ9AchBmSK=BWR1A+4JQ7DiG2JmvyvtsPiw@mail.gmail.com","subject":"Re: [PATCH] refs.c: move reflog updates into its own function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-24T21:50:39Z","receivedAt":"2014-11-24T21:50:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Anything holding this back?\n\nI do not recall anything objectionable offhand. Perhaps fell\nbetween the cracks during a patch storm or something.\n"}]}