Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Without any configuration the current branch is pushed out, which
loosens the safety we implemented in the current 'safer upstream'.

I am not convinced this is a good change.  I am not convinced this is
a bad change, either, yet, but this loosening smells bad.

Provided that we would want to keep the "Push the current one to the
same name but you have to have it set up as your integration source"
safety for central workflow (which I am starting to think we
should), we would want something like this on top of your entire
series, I think.  The behaviour change can be seen in the revert of
one test you made to the test that expects "simple" to fail due to
the safety.

And with the small refactoring of setup_default_push_refspecs (the
important part being that we grab branch in this function, not in
its helper functions per push.default mode), branch.*.push can fall
out rather naturally, like this patch on top.


A footnote unrelated to this patch.

The function is_workflow_triangular() in the "how about this" patch
needs to be tweaked from the version I am responding to, in order to
take the case where fetch-remote is not defined into account, i.e.

    static int is_workflow_triagular(struct remote *remote)
    {
            struct remote *fetch_remote = remote_get(NULL);
            return (fetch_remote && fetch_remote != remote);
    }




 builtin/push.c | 18 +++++++++++++++++-
 remote.c       |  5 +++++
 remote.h       |  2 ++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin/push.c b/builtin/push.c
index f6c8047..a140b8e 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -185,6 +185,15 @@ static void warn_unspecified_push_default_configuration(void)
 	warning("%s\n", _(warn_unspecified_push_default_msg));
 }
 
+static void setup_per_branch_push(struct branch *branch)
+{
+	struct strbuf refspec = STRBUF_INIT;
+
+	strbuf_addf(&refspec, "%s:%s",
+		    branch->name, branch->push_name);
+	add_refspec(refspec.buf);
+}
+
 static int is_workflow_triagular(struct remote *remote)
 {
 	struct remote *fetch_remote = remote_get(NULL);

@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	struct branch *branch = branch_get(NULL);
-	int triangular = is_workflow_triagular(remote);
+	int triangular;
+
+	if (branch->push_name) {
+		setup_per_branch_push(branch);
+		return;
+	}
+
+	triangular = is_workflow_triagular(remote);
 
 	switch (push_default) {
 	default:

diff --git a/remote.c b/remote.c
index e71f66d..e033fef 100644
--- a/remote.c
+++ b/remote.c

@@ -372,6 +372,11 @@ static int handle_config(const char *key, const char *value, void *cb)
 			if (!value)
 				return config_error_nonbool(key);
 			add_merge(branch, xstrdup(value));
+		} else if (!strcmp(subkey, ".push")) {
+			if (!value)
+				return config_error_nonbool(key);
+			free(branch->push_name);
+			branch->push_name = xstrdup(value);
 		}
 		return 0;
 	}

diff --git a/remote.h b/remote.h
index cf56724..84e0f72 100644
--- a/remote.h
+++ b/remote.h

@@ -138,6 +138,8 @@ struct branch {
 	struct refspec **merge;
 	int merge_nr;
 	int merge_alloc;
+
+	char *push_name;
 };
 
 struct branch *branch_get(const char *name);