Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 2, 2014, 03:24 UTC
- Message-ID
- <xmqqa936ohs3.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <xmqq1tp643yb.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
>> 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