public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Johannes Schindelin <Johannes.Schindelin@gmx•de>
Cc: git@vger•kernel.org, Jens Lehmann <Jens.Lehmann@web•de>,
	Heiko Voigt <hvoigt@hvoigt•net>
Subject: Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules
Date: Mon, 01 Dec 2014 19:24:44 -0800	[thread overview]
Message-ID: <xmqqa936ohs3.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqq1tp643yb.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 13 Nov 2014 23:49:16 -0800")

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

  reply	other threads:[~2014-12-02  3:24 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-07 13:58 [PATCH 0/2] Support updating working trees when pushing into non-bare repos Johannes Schindelin
2014-11-07 13:58 ` [PATCH 1/2] Add a few more values for receive.denyCurrentBranch Johannes Schindelin
2014-11-07 18:49   ` Junio C Hamano
2014-11-07 18:58     ` Johannes Schindelin
2014-11-10 12:54     ` Johannes Schindelin
2014-11-10 16:00       ` Junio C Hamano
2014-11-12 11:13         ` Johannes Schindelin
2014-11-12 18:00           ` Junio C Hamano
2014-11-08 11:18   ` Jeff King
2014-11-08 18:48     ` brian m. carlson
2014-11-10 13:03     ` Johannes Schindelin
2014-11-07 13:58 ` [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules Johannes Schindelin
2014-11-07 19:20   ` Junio C Hamano
2014-11-09 16:42     ` Jens Lehmann
2014-11-10 13:04       ` Johannes Schindelin
2014-11-10 13:01     ` Johannes Schindelin
2014-11-10 15:42       ` Junio C Hamano
2014-11-10 19:32         ` Junio C Hamano
2014-11-12 11:09           ` Johannes Schindelin
2014-11-12 17:59             ` Junio C Hamano
2014-11-13 10:29               ` Johannes Schindelin
2014-11-13 10:38                 ` Johannes Schindelin
2014-11-13 17:41                   ` Junio C Hamano
2014-11-13 18:55                     ` Johannes Schindelin
2014-11-13 19:48                       ` Junio C Hamano
2014-11-13 21:06                         ` Junio C Hamano
2014-11-14  7:49                           ` Junio C Hamano
2014-12-02  3:24                             ` Junio C Hamano [this message]
2014-12-02  3:25                               ` [PATCH 2/2] receive-pack: support push-to-checkout hook Junio C Hamano
2014-12-02  8:47                                 ` Johannes Schindelin
2014-12-02 13:03                                   ` Michael J Gruber
2014-12-02 13:25                                     ` Johannes Schindelin
2014-12-02 16:39                                   ` Junio C Hamano
2014-12-02 16:45                                     ` Johannes Schindelin
2014-12-02 17:00                                       ` Junio C Hamano
2014-12-02 17:12                                         ` Johannes Schindelin
2014-12-02 17:19                                           ` Junio C Hamano
2014-11-13 17:41                   ` [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules Junio C Hamano
2014-11-12 11:06         ` Johannes Schindelin
2014-11-10 14:38 ` [PATCH v2 0/2] Support updating working trees when pushing into non-bare repos Johannes Schindelin
2014-11-13 11:03   ` [PATCH v3 0/1] " Johannes Schindelin
2014-11-13 11:03     ` [PATCH v3 1/1] Add another option for receive.denyCurrentBranch Johannes Schindelin
2014-11-13 17:51       ` Junio C Hamano
2014-11-13 19:21         ` Johannes Schindelin
2014-11-13 17:47     ` [PATCH v3 0/1] Support updating working trees when pushing into non-bare repos Junio C Hamano
2014-11-13 19:11       ` Junio C Hamano
2014-11-13 19:18       ` Johannes Schindelin
2014-11-26 20:21     ` [PATCH v4] " Johannes Schindelin
2014-11-26 20:21       ` [PATCH v4] Add another option for receive.denyCurrentBranch Johannes Schindelin
2014-11-26 21:02         ` Junio C Hamano
2014-11-26 22:44       ` [PATCH v5] Support updating working trees when pushing into non-bare repos Johannes Schindelin
2014-11-26 22:44         ` [PATCH v5] Add another option for receive.denyCurrentBranch Johannes Schindelin
2014-12-01  3:18           ` Junio C Hamano
2014-12-01  7:44             ` Johannes Schindelin
2014-12-01 23:49           ` Junio C Hamano
2014-12-02  0:51             ` Junio C Hamano
2014-12-02  8:21               ` Johannes Schindelin
2014-12-02 16:20                 ` Junio C Hamano
2014-12-02 16:51                   ` Johannes Schindelin
2014-12-02 17:23                   ` Junio C Hamano
     [not found] ` <cover.1415630072.git.johannes.schindelin@gmx.de>
2014-11-10 14:38   ` [PATCH v2 1/2] Clean stale environment pointer in finish_command() Johannes Schindelin
2014-11-10 14:41     ` Johannes Schindelin
2014-11-11  3:16       ` Jeff King
2014-11-11 15:55         ` Junio C Hamano
2014-11-12 10:45           ` Johannes Schindelin
2014-11-12 10:52             ` Jeff King
2014-11-12 10:59               ` Jeff King
2014-11-12 16:17                 ` Junio C Hamano
2014-11-10 21:44     ` Junio C Hamano
2014-11-11  3:11       ` Jeff King
2014-11-10 14:38   ` [PATCH v2 2/2] Add a few more options for receive.denyCurrentBranch Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa936ohs3.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=Jens.Lehmann@web$(echo .)de \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=hvoigt@hvoigt$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox