Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

>> And thinking about the names again, I have a feeling that
>> updateInstead and mergeInstead are both probably misnomer.
>
> Let me take this part back.  After all, I do not think I would
> design the mechanism to implement an alternative logic that decides
> when it is safe to allow the update of the ref and to reflect the
> changes to the working tree, and that actually does the checkout to
> the working tree by using a new value like mergeInstead.  So we
> would only need a single name, and updateInstead is not too bad.
> ...
> The mechanism I would employ when doing an alternative logic,
> possibly looser one but does not necessarily so, would be to have a
> hook script "push-to-checkout".  When denyCurrentBranch is set to
> updateInstead, if there is no such hook, the "working tree has to be
> absolutely clean and we would do a 'read-tree -m -u $old $new'
> (which is the same as 'reset --hard $new' under the precondition)"
> you implemented will be used as the "logic that decides when it is
> safe, and that does the checkout to the working tree".  When the
> "push-to-checkout" hook exists, however, we just invoke that hook
> with $new as argument, and it can decide when it is safe in whatever
> way it chooses to, and it can checkout the $new to the working tree
> in whatever way it wants.

So here comes a two-patch series on top of your series (with the
test update I sent earlier).  As I never do "push to deploy" that
requires no changes to the working tree or to the index, while I
have seem myself in a situation where I have to emulate a "git pull"
with a "git push" in the opposite direction (and work it around if
the target of the 'git pull' I wanted to do were the current branch,
by first pushing into a throw-away branch, because of denyCurrent),
I could imagine myself using this variant.  Having said that, this
is primarily so that I do not want to forget and discard the brain
cycles we spent discussing this to the waste, more than that I
cannot wait to use this feature myself ;-)

The first one here is a pure refactoring.  The second one is the
real fun.

-- >8 --
Subject: [PATCH 1/2] receive-pack: refactor updateInstead codepath

Keep the "there is nothing to update in a bare repository", "when
the check and update process runs, here are the GIT_DIR and
GIT_WORK_TREE" logic, which will be common regardless of how the
decision to update and the actual update are done, in the original
update_worktree() function, and split out the "working tree and
the index must match the original HEAD exactly" and "use two-way
read-tree to update the working tree" into a new push_to_deploy()
helper function.  This will allow customizing the logic more cleanly
and easily.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c | 53 ++++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c047418..11800cd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -733,7 +733,9 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 	return 0;
 }
 
-static const char *update_worktree(unsigned char *sha1)
+static const char *push_to_deploy(unsigned char *sha1,
+				  struct argv_array *env,
+				  const char *work_tree)
 {
 	const char *update_refresh[] = {
 		"update-index", "-q", "--ignore-submodules", "--refresh", NULL
@@ -748,69 +750,70 @@ static const char *update_worktree(unsigned char *sha1)
 	const char *read_tree[] = {
 		"read-tree", "-u", "-m", NULL, NULL
 	};
-	const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
-	struct argv_array env = ARGV_ARRAY_INIT;
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	if (is_bare_repository())
-		return "denyCurrentBranch = updateInstead needs a worktree";
-
-	argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(get_git_dir()));
-
 	child.argv = update_refresh;
-	child.env = env.argv;
+	child.env = env->argv;
 	child.dir = work_tree;
 	child.no_stdin = 1;
 	child.stdout_to_stderr = 1;
 	child.git_cmd = 1;
-	if (run_command(&child)) {
-		argv_array_clear(&env);
+	if (run_command(&child))
 		return "Up-to-date check failed";
-	}
 
 	/* run_command() does not clean up completely; reinitialize */
 	child_process_init(&child);
 	child.argv = diff_files;
-	child.env = env.argv;
+	child.env = env->argv;
 	child.dir = work_tree;
 	child.no_stdin = 1;
 	child.stdout_to_stderr = 1;
 	child.git_cmd = 1;
-	if (run_command(&child)) {
-		argv_array_clear(&env);
+	if (run_command(&child))
 		return "Working directory has unstaged changes";
-	}
 
 	child_process_init(&child);
 	child.argv = diff_index;
-	child.env = env.argv;
+	child.env = env->argv;
 	child.no_stdin = 1;
 	child.no_stdout = 1;
 	child.stdout_to_stderr = 0;
 	child.git_cmd = 1;
-	if (run_command(&child)) {
-		argv_array_clear(&env);
+	if (run_command(&child))
 		return "Working directory has staged changes";
-	}
 
 	read_tree[3] = sha1_to_hex(sha1);
 	child_process_init(&child);
 	child.argv = read_tree;
-	child.env = env.argv;
+	child.env = env->argv;
 	child.dir = work_tree;
 	child.no_stdin = 1;
 	child.no_stdout = 1;
 	child.stdout_to_stderr = 0;
 	child.git_cmd = 1;
-	if (run_command(&child)) {
-		argv_array_clear(&env);
+	if (run_command(&child))
 		return "Could not update working tree to new HEAD";
-	}
 
-	argv_array_clear(&env);
 	return NULL;
 }
 
+static const char *update_worktree(unsigned char *sha1)
+{
+	const char *retval;
+	const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
+	struct argv_array env = ARGV_ARRAY_INIT;
+
+	if (is_bare_repository())
+		return "denyCurrentBranch = updateInstead needs a worktree";
+
+	argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(get_git_dir()));
+
+	retval = push_to_deploy(sha1, &env, work_tree);
+
+	argv_array_clear(&env);
+	return retval;
+}
+
 static const char *update(struct command *cmd, struct shallow_info *si)
 {
 	const char *name = cmd->ref_name;
-- 
2.2.0-141-gd3f4719