On 2014-02-21 10.55, Max Horn wrote:
>
> On 20.02.2014, at 20:22, Junio C Hamano <[email protected]> wrote:
>
>> Max Horn <[email protected]> writes:
>>
>>> On 19.02.2014, at 22:41, Junio C Hamano <[email protected]> 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< --
>
>
Ack from my side:
There are 2 fields:
ref->force and ref->forced_update.
forced_update is set in set_ref_status_for_push() in remote.c:
if (!force_ref_update)
ref->status = reject_reason;
else if (reject_reason)
ref->forced_update = 1;
}
And transport-helper.c sets it to 0 even if it had been 1, which is wrong.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
