Re: [PATCH 2/3] cmd_push: pass "flags" pointer to config callback

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/3] cmd_push: pass "flags" pointer to config callback

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:03:49

Jeff King [off-list ref] writes:

This will let us manipulate any transport flags which have matching
config options (there are none yet, but we will add one in
the next patch).

Nice---this will later lets us do push.atomic if we really wanted
to, right?

To be honest, the whole do_push is confusing to me. It seems like that
should just be part of cmd_push.

Yeah, that part of the push callchain always confuses me every time
I look at it.  I think it was a consequence of how transport layer
was wedged into the existing codepath that only handled push that
called send-pack to unify the codepaths that push calls into
different transport backends, and we may have done it differently
and more cleanly if we were designing the push to transport to
backends from scratch.

quoted hunk

 builtin/push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/push.c b/builtin/push.c
index f558c2e..c25108f 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -555,7 +555,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	};
 
 	packet_trace_identity("push");
-	git_config(git_push_config, NULL);
+	git_config(git_push_config, &flags);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))

Re: [PATCH 2/3] cmd_push: pass "flags" pointer to config callback

From: Jeff King <hidden>
Date: 2016-06-15 23:03:49

On Sun, Feb 15, 2015 at 11:05:57PM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

This will let us manipulate any transport flags which have matching
config options (there are none yet, but we will add one in
the next patch).

Nice---this will later lets us do push.atomic if we really wanted
to, right?

Yes, exactly. Or push.signed, or whatever.

quoted

To be honest, the whole do_push is confusing to me. It seems like that
should just be part of cmd_push.

Yeah, that part of the push callchain always confuses me every time
I look at it.  I think it was a consequence of how transport layer
was wedged into the existing codepath that only handled push that
called send-pack to unify the codepaths that push calls into
different transport backends, and we may have done it differently
and more cleanly if we were designing the push to transport to
backends from scratch.

I took a very cursory look at folding do_push into cmd_push. It's not
_too_ bad. You wouldn't want to fold push_with_options in, as that gets
called from a loop (you could make it the loop body, but I think it is
more clear as-is).

However, it is really do_push which continues to manipulate the flags
and set up the push, so that is the bit that should be folded in. And
then it would be fine to make transport_flags a global, and
push_with_options could just use it directly, I think.

-Peff

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help