Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)

From: Max Horn <max@quendi.de>
Date: Feb 21, 2014, 09:55 UTC
Message-ID: <FB0B19C8-65BF-49CF-8EE4-5B9D55BBCE7C@quendi.de>
In-Reply-To: <xmqqsird6137.fsf@gitster.dls.corp.google.com>

On 20.02.2014, at 20:22, Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

> Max Horn <max@quendi.de> writes:
> 
>> On 19.02.2014, at 22:41, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>>> * fc/transport-helper-fixes (2013-12-09) 6 commits
>>> - remote-bzr: support the new 'force' option
>>> - test-hg.sh: tests are now expected to pass
>>> - transport-helper: check for 'forced update' message
>>> - transport-helper: add 'force' to 'export' helpers
>>> - transport-helper: don't update refs in dry-run
>>> - transport-helper: mismerge fix
>>> 
>>> Reported to break t5541, and has been stalled for a while without
>>> fixes.
>> ...
>> Since I somewhat care about transport-helpers, I had a closer look
>> at this failure.
> 
> Thanks.  Let's keep it a bit longer and see how your new
> investigation (and possibly help from others) develops to a
> solution.

So I had a closer look, and I now believe to now understand what the right fix is. Simply dropping 
  transport-helper: check for 'forced update' message
would be wrong, because it would cause the contrib/remote-helpers test cases to fail -- when I tested last night, I forgot that I had to run those separately. Silly me!

Indeed, these tests include a test with a force push, and trigger the code path added in that commit. The remaining problem then is that the new code should be changed to only tell git when a remote-helper signals a forced update; but it should not incorrectly reset the forced_update flag to 0 if git already considers the update to be forced.

In other words, the following patch should be the correct solution, as far as I can tell. I verified that it fixes t5541 and causes no regressions in the contrib/remote-helpers test suite.

-- 8< --

diff --git a/transport-helper.c b/transport-helper.c
index fa7c608..86e1679 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -734,7 +734,7 @@ static int push_update_ref_status(struct strbuf *buf,
        }

        (*ref)->status = status;
-       (*ref)->forced_update = forced;
+       (*ref)->forced_update |= forced;
        (*ref)->remote_status = msg;
        return !(status == REF_STATUS_OK);
 }
-- 8< --