On Fri, Nov 1, 2013 at 8:49 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>>
>>> On Thu, Oct 31, 2013 at 12:21 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>>>>
>>>>> Otherwise they cannot know when to force the push or not (other than
>>>>> hacks).
>>>>> ...
>>>>> diff --git a/transport-helper.c b/transport-helper.c
>>>>> index d05fc7c..ed238e5 100644
>>>>> --- a/transport-helper.c
>>>>> +++ b/transport-helper.c
>>>>> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,
>>>>> die("helper %s does not support dry-run", data->name);
>>>>> }
>>>>>
>>>>> + if (flags & TRANSPORT_PUSH_FORCE) {
>>>>> + if (set_helper_option(transport, "force", "true") != 0)
>>>>> + die("helper %s does not support 'force'", data->name);
>>>>> + }
>>>>> +
>>>>
>>>> Does this cause a "git push --force $there A:B" to fail when $there
>>>> is a destination that goes via an existing helper does not suport
>>>> "force" option?
>>>
>>> Yes.
>>>
>>>> Should it fail even when the current value of B is an ancestor of A
>>>> (i.e. when an unforced push would succeed)?
>>>
>>> It might make sense to fail only when the push is non-fast-forward,
>>> but it's not so straight-forward to implement.
>>
>> OK; let's see if anybody screams by cooking the series in 'next'
>> (that is, when other issues people may discover in the series are
>> addressed).
>>
>> Thanks.
>
> Actually, thinking about it again, would it be a better alternative
> to issue an error message and continue, instead of dying here
> (i.e. replace the above die() with error() or even warning())?
>
> That way, if the helper has not been updated to support 'force', but
> the user has been happily accepting the result he gets from "git
> push $there +master" (perhaps because he has never pushed a non-ff
> history so far), he now gets a warning that essentially says that
> his push has been working by accident, even when the particular
> ff-push that showed the error message goes through successfully.
>
> If his push through the helper that has not been updated does need
> 'force', he gets the same old behaviour, possibly a broken one, out
> of the helper, and again he does get the warning.
>
> Because updated helpers know 'force' option, such a s/die/warning/
> change would not affect them at all.
Yeah, that makes sense.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html