[PATCH 0/2] --no-deref and --stdin compatibility for update-ref

[PATCH 0/2] --no-deref and --stdin compatibility for update-ref

[Elijah Newren]

Currently, the --no-deref and --stdin options of update-ref cannot be
used together (the code aborts immediately with a usage message), though
it makes sense to do so and is easier than repeatedly specifying on
stdin that each ref should not be dereferenced.  Also, the documentation
for the --no-deref option has a few problems, making it unclear what it
is and isn't compatible with.

The first patch is just a minor code fixup that the second lightly
depends on.  The second patch has the changes to fix the issues stated
above.


Elijah Newren (2):
  update-ref: fix type of update_flags variable to match its usage
  update-ref: allow --no-deref with --stdin

 Documentation/git-update-ref.txt |  2 +-
 builtin/update-ref.c             | 25 ++++++++++++++-----------
 t/t1400-update-ref.sh            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 12 deletions(-)

-- 
2.19.0.rc2.2.g1aedc61e22

[PATCH 1/2] update-ref: fix type of update_flags variable to match its usage

[Elijah Newren]

The ref_transaction_*() family of functions expect a flags parameter
which is of type unsigned int.  Make the update_flags variable, which
is passed as that parameter, be of the same type.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/update-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 4fa3c0a86f..54fac01f21 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = {
 };
 
 static char line_termination = '\n';
-static int update_flags;
+static unsigned int update_flags;
 static unsigned create_reflog_flag;
 static const char *msg;
 
-- 
2.19.0.rc2.2.g1aedc61e22

[PATCH 2/2] update-ref: allow --no-deref with --stdin

[Elijah Newren]

If passed both --no-deref and --stdin, update-ref would error out with a
general usage message that did not at all suggest these options were
incompatible.  The manpage for update-ref did suggest through its
synopsis line that --no-deref and --stdin were incompatible, but it sadly
also incorrectly suggested that -d and --no-deref were incompatible.  So
the help around the --no-deref option is buggy in a few ways.

The --stdin option did provide a different mechanism for avoiding
dereferencing symbolic-refs: adding a line reading
  option no-deref
before every other directive in the input.  (Technically, if the user
wants to do the extra work of first determining which refs they want to
update or delete are symbolic, then they only need to put the extra
"option no-deref" lines before the updates of those refs.  But in some
cases, that's more work than just adding the "option no-deref" before
every other directive.)

It's easier to allow the user to just pass --no-deref along with --stdin
in order to tell update-ref that the user doesn't want any symbolic ref
to be dereferenced.  It also makes the update-ref documentation simpler.
Implement that, and update the documentation to match.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-update-ref.txt |  2 +-
 builtin/update-ref.c             | 23 +++++++++++++----------
 t/t1400-update-ref.sh            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index bc8fdfd469..fda8516677 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
 SYNOPSIS
 --------
 [verse]
-'git update-ref' [-m <reason>] (-d <ref> [<oldvalue>] | [--no-deref] [--create-reflog] <ref> <newvalue> [<oldvalue>] | --stdin [-z])
+'git update-ref' [-m <reason>] [--no-deref] (-d <ref> [<oldvalue>] | [--create-reflog] <ref> <newvalue> [<oldvalue>] | --stdin [-z])
 
 DESCRIPTION
 -----------
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 54fac01f21..2d8f7f0578 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -15,6 +15,7 @@ static const char * const git_update_ref_usage[] = {
 
 static char line_termination = '\n';
 static unsigned int update_flags;
+static unsigned int default_flags;
 static unsigned create_reflog_flag;
 static const char *msg;
 
@@ -205,7 +206,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,
 				   msg, &err))
 		die("%s", err.buf);
 
-	update_flags = 0;
+	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
 
@@ -237,7 +238,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,
 				   msg, &err))
 		die("%s", err.buf);
 
-	update_flags = 0;
+	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
 
@@ -273,7 +274,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,
 				   update_flags, msg, &err))
 		die("%s", err.buf);
 
-	update_flags = 0;
+	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
 
@@ -302,7 +303,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
 				   update_flags, &err))
 		die("%s", err.buf);
 
-	update_flags = 0;
+	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
 
@@ -357,7 +358,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 	const char *refname, *oldval;
 	struct object_id oid, oldoid;
 	int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
-	unsigned int flags = 0;
 	int create_reflog = 0;
 	struct option options[] = {
 		OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
@@ -378,6 +378,11 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 
 	create_reflog_flag = create_reflog ? REF_FORCE_CREATE_REFLOG : 0;
 
+	if (no_deref) {
+		default_flags = REF_NO_DEREF;
+		update_flags = default_flags;
+	}
+
 	if (read_stdin) {
 		struct strbuf err = STRBUF_INIT;
 		struct ref_transaction *transaction;
@@ -385,7 +390,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		transaction = ref_transaction_begin(&err);
 		if (!transaction)
 			die("%s", err.buf);
-		if (delete || no_deref || argc > 0)
+		if (delete || argc > 0)
 			usage_with_options(git_update_ref_usage, options);
 		if (end_null)
 			line_termination = '\0';
@@ -427,8 +432,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 			die("%s: not a valid old SHA1", oldval);
 	}
 
-	if (no_deref)
-		flags = REF_NO_DEREF;
 	if (delete)
 		/*
 		 * For purposes of backwards compatibility, we treat
@@ -436,9 +439,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		 */
 		return delete_ref(msg, refname,
 				  (oldval && !is_null_oid(&oldoid)) ? &oldoid : NULL,
-				  flags);
+				  default_flags);
 	else
 		return update_ref(msg, refname, &oid, oldval ? &oldoid : NULL,
-				  flags | create_reflog_flag,
+				  default_flags | create_reflog_flag,
 				  UPDATE_REFS_DIE_ON_ERR);
 }
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7c8df20955..02493f14ba 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -807,6 +807,37 @@ test_expect_success 'stdin delete symref works option no-deref' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stdin update symref works flag --no-deref' '
+	git symbolic-ref TESTSYMREFONE $b &&
+	git symbolic-ref TESTSYMREFTWO $b &&
+	cat >stdin <<-EOF &&
+	update TESTSYMREFONE $a $b
+	update TESTSYMREFTWO $a $b
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	git rev-parse TESTSYMREFONE TESTSYMREFTWO >expect &&
+	git rev-parse $a $a >actual &&
+	test_cmp expect actual &&
+	git rev-parse $m~1 >expect &&
+	git rev-parse $b >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stdin delete symref works flag --no-deref' '
+	git symbolic-ref TESTSYMREFONE $b &&
+	git symbolic-ref TESTSYMREFTWO $b &&
+	cat >stdin <<-EOF &&
+	delete TESTSYMREFONE $b
+	delete TESTSYMREFTWO $b
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	test_must_fail git rev-parse --verify -q TESTSYMREFONE &&
+	test_must_fail git rev-parse --verify -q TESTSYMREFTWO &&
+	git rev-parse $m~1 >expect &&
+	git rev-parse $b >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'stdin delete ref works with right old value' '
 	echo "delete $b $m~1" >stdin &&
 	git update-ref --stdin <stdin &&
-- 
2.19.0.rc2.2.g1aedc61e22