Re: [PATCH] Give better 'pull' advice when pushing non-ff updates to current branch
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:39
Christopher Tiwald [off-list ref] writes:
quoted hunk
@@ -177,6 +192,16 @@ static int push_with_options(struct transport *transport, int flags) { int err; int nonfastforward; + struct branch *branch; + struct strbuf buf = STRBUF_INIT; + + branch = branch_get(NULL); + + if (branch) { + strbuf_addstr(&buf, transport->remote->name); + strbuf_addstr(&buf, "/"); + strbuf_addstr(&buf, branch->name); + }
The "buf" is a horrible name for a variable that is used to hold states in a long haul that has to span multiple hunks in a patch. Please name it after what the value means.
quoted hunk
@@ -201,7 +226,18 @@ static int push_with_options(struct transport *transport, int flags) default: break; case NON_FF_HEAD: - advise_pull_before_push(); + /* Branches configured for octopus merges should advise + * just 'git pull' */ + if (branch->remote_name && + branch->merge && + branch->merge_nr == 1 && + !strcmp(transport->remote->name, branch->remote_name) && + !strcmp(strbuf_detach(&buf, NULL), + prettify_refname(branch->merge[0]->dst))) {
Why detach? buf_to_be_renamed_more_sanely.buf, perhaps? Is comparison between whatever buf has and the result of prettify safe and sane? After all, prettify is a random abbreviation that is meant for human consumption, assuming that the reader is intelligent enough to guess "Ah, it must be a tag" when she sees "v1.0" which is the result of stripping the leading "refs/tags/" out. This part should be using straight strcmp against branch->merge[0]->dst, which means buf_to_be_renamed_more_sanely in the earlier hunk needs to be computing what's tracked and merged correctly using the configured refspec mapping, perhaps?