{"thread":{"id":"49283","subject":"[PATCH 1/2] update-ref: fix type of update_flags variable to match its usage","startedAt":"2018-09-05T17:25:55Z","lastAt":"2018-09-05T17:25:57Z","messageCount":3,"participants":["Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"357453","messageId":"20180905172550.11621-2-newren@gmail.com","threadId":"49283","inReplyTo":"20180905172550.11621-1-newren@gmail.com","subject":"[PATCH 1/2] update-ref: fix type of update_flags variable to match its usage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-09-05T17:25:49Z","receivedAt":"2018-09-05T17:25:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The ref_transaction_*() family of functions expect a flags parameter\nwhich is of type unsigned int. Make the update_flags variable, which\nis passed as that parameter, be of the same type.\n\nSigned-off-by: Elijah Newren \n---\n builtin/update-ref.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 4fa3c0a86f..54fac01f21 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = {\n };\n \n static char line_termination = '\\n';\n-static int update_flags;\n+static unsigned int update_flags;\n static unsigned create_reflog_flag;\n static const char *msg;\n \n-- \n2.19.0.rc2.2.g1aedc61e22\n\n"},{"id":"357454","messageId":"20180905172550.11621-1-newren@gmail.com","threadId":"49283","inReplyTo":null,"subject":"[PATCH 0/2] --no-deref and --stdin compatibility for update-ref","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-09-05T17:25:48Z","receivedAt":"2018-09-05T17:25:56Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Currently, the --no-deref and --stdin options of update-ref cannot be\nused together (the code aborts immediately with a usage message), though\nit makes sense to do so and is easier than repeatedly specifying on\nstdin that each ref should not be dereferenced. Also, the documentation\nfor the --no-deref option has a few problems, making it unclear what it\nis and isn't compatible with.\n\nThe first patch is just a minor code fixup that the second lightly\ndepends on. The second patch has the changes to fix the issues stated\nabove.\n\n\nElijah Newren (2):\n update-ref: fix type of update_flags variable to match its usage\n update-ref: allow --no-deref with --stdin\n\n Documentation/git-update-ref.txt | 2 +-\n builtin/update-ref.c | 25 ++++++++++++++-----------\n t/t1400-update-ref.sh | 31 +++++++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+), 12 deletions(-)\n\n-- \n2.19.0.rc2.2.g1aedc61e22\n\n"},{"id":"357455","messageId":"20180905172550.11621-3-newren@gmail.com","threadId":"49283","inReplyTo":"20180905172550.11621-1-newren@gmail.com","subject":"[PATCH 2/2] update-ref: allow --no-deref with --stdin","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-09-05T17:25:50Z","receivedAt":"2018-09-05T17:25:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"If passed both --no-deref and --stdin, update-ref would error out with a\ngeneral usage message that did not at all suggest these options were\nincompatible. The manpage for update-ref did suggest through its\nsynopsis line that --no-deref and --stdin were incompatible, but it sadly\nalso incorrectly suggested that -d and --no-deref were incompatible. So\nthe help around the --no-deref option is buggy in a few ways.\n\nThe --stdin option did provide a different mechanism for avoiding\ndereferencing symbolic-refs: adding a line reading\n option no-deref\nbefore every other directive in the input. (Technically, if the user\nwants to do the extra work of first determining which refs they want to\nupdate or delete are symbolic, then they only need to put the extra\n\"option no-deref\" lines before the updates of those refs. But in some\ncases, that's more work than just adding the \"option no-deref\" before\nevery other directive.)\n\nIt's easier to allow the user to just pass --no-deref along with --stdin\nin order to tell update-ref that the user doesn't want any symbolic ref\nto be dereferenced. It also makes the update-ref documentation simpler.\nImplement that, and update the documentation to match.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-update-ref.txt | 2 +-\n builtin/update-ref.c | 23 +++++++++++++----------\n t/t1400-update-ref.sh | 31 +++++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex bc8fdfd469..fda8516677 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely\n SYNOPSIS\n --------\n [verse]\n-'git update-ref' [-m ] (-d [] | [--no-deref] [--create-reflog] [] | --stdin [-z])\n+'git update-ref' [-m ] [--no-deref] (-d [] | [--create-reflog] [] | --stdin [-z])\n \n DESCRIPTION\n -----------\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 54fac01f21..2d8f7f0578 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -15,6 +15,7 @@ static const char * const git_update_ref_usage[] = {\n \n static char line_termination = '\\n';\n static unsigned int update_flags;\n+static unsigned int default_flags;\n static unsigned create_reflog_flag;\n static const char *msg;\n \n@@ -205,7 +206,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\n \t\t\t\t msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n-\tupdate_flags = 0;\n+\tupdate_flags = default_flags;\n \tfree(refname);\n \tstrbuf_release(&err);\n \n@@ -237,7 +238,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\n \t\t\t\t msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n-\tupdate_flags = 0;\n+\tupdate_flags = default_flags;\n \tfree(refname);\n \tstrbuf_release(&err);\n \n@@ -273,7 +274,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\n \t\t\t\t update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n-\tupdate_flags = 0;\n+\tupdate_flags = default_flags;\n \tfree(refname);\n \tstrbuf_release(&err);\n \n@@ -302,7 +303,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,\n \t\t\t\t update_flags, &err))\n \t\tdie(\"%s\", err.buf);\n \n-\tupdate_flags = 0;\n+\tupdate_flags = default_flags;\n \tfree(refname);\n \tstrbuf_release(&err);\n \n@@ -357,7 +358,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval;\n \tstruct object_id oid, oldoid;\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;\n-\tunsigned int flags = 0;\n \tint create_reflog = 0;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n@@ -378,6 +378,11 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \n \tcreate_reflog_flag = create_reflog ? REF_FORCE_CREATE_REFLOG : 0;\n \n+\tif (no_deref) {\n+\t\tdefault_flags = REF_NO_DEREF;\n+\t\tupdate_flags = default_flags;\n+\t}\n+\n \tif (read_stdin) {\n \t\tstruct strbuf err = STRBUF_INIT;\n \t\tstruct ref_transaction *transaction;\n@@ -385,7 +390,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\ttransaction = ref_transaction_begin(&err);\n \t\tif (!transaction)\n \t\t\tdie(\"%s\", err.buf);\n-\t\tif (delete || no_deref || argc > 0)\n+\t\tif (delete || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n@@ -427,8 +432,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"%s: not a valid old SHA1\", oldval);\n \t}\n \n-\tif (no_deref)\n-\t\tflags = REF_NO_DEREF;\n \tif (delete)\n \t\t/*\n \t\t * For purposes of backwards compatibility, we treat\n@@ -436,9 +439,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\treturn delete_ref(msg, refname,\n \t\t\t\t (oldval && !is_null_oid(&oldoid)) ? &oldoid : NULL,\n-\t\t\t\t flags);\n+\t\t\t\t default_flags);\n \telse\n \t\treturn update_ref(msg, refname, &oid, oldval ? &oldoid : NULL,\n-\t\t\t\t flags | create_reflog_flag,\n+\t\t\t\t default_flags | create_reflog_flag,\n \t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n }\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7c8df20955..02493f14ba 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -807,6 +807,37 @@ test_expect_success 'stdin delete symref works option no-deref' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stdin update symref works flag --no-deref' '\n+\tgit symbolic-ref TESTSYMREFONE $b &&\n+\tgit symbolic-ref TESTSYMREFTWO $b &&\n+\tcat >stdin <<-EOF &&\n+\tupdate TESTSYMREFONE $a $b\n+\tupdate TESTSYMREFTWO $a $b\n+\tEOF\n+\tgit update-ref --no-deref --stdin expect &&\n+\tgit rev-parse $a $a >actual &&\n+\ttest_cmp expect actual &&\n+\tgit rev-parse $m~1 >expect &&\n+\tgit rev-parse $b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin delete symref works flag --no-deref' '\n+\tgit symbolic-ref TESTSYMREFONE $b &&\n+\tgit symbolic-ref TESTSYMREFTWO $b &&\n+\tcat >stdin <<-EOF &&\n+\tdelete TESTSYMREFONE $b\n+\tdelete TESTSYMREFTWO $b\n+\tEOF\n+\tgit update-ref --no-deref --stdin expect &&\n+\tgit rev-parse $b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'stdin delete ref works with right old value' '\n \techo \"delete $b $m~1\" >stdin &&\n \tgit update-ref --stdin