[PATCH 4 of 6] resolve: abort when not applicable

[Pierre-Yves David]

On 05/09/2014 02:54 AM, Mads Kiilerich wrote:
> On 05/09/2014 03:46 AM, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1397873312 25200
>> #      Fri Apr 18 19:08:32 2014 -0700
>> # Node ID a5e623de6557ea8854cefc108d5b069adb0f737a
>> # Parent  6d5b402aae2d697b93b19ccf8f4d0245c4fc29d3
>> resolve: abort when not applicable
>>
>> The resolve command is only relevant when mergestate is present.
>> This patch will make resolve abort when no mergestate is present.
>>
>> This change will let people know when they are using resolve when they
>> shouldn't be. This change will let people know when their use of resolve
>> doesn't do anything.
>>
>> Previously, |hg resolve -m| would allow mergestate to be created. This
>> patch now forbids that. Strictly speaking, this is backwards
>> incompatible. The author of this patch believes creating mergestate via
>> resolve doesn't make much sense and this side-effect was unintended.
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -4926,8 +4926,12 @@ def resolve(ui, repo, *pats, **opts):
>>           raise util.Abort(_('no files or directories specified; '
>>                              'use --all to remerge all files'))
>>       ms = mergemod.mergestate(repo)
>> +
>> +    if not ms.active():
>> +        raise util.Abort(_('resolve command not applicable when not
>> merging'))
>> +
>>       m = scmutil.match(repo[None], pats, opts)
>>       ret = 0
>>       for f in ms:
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>> --- a/mercurial/merge.py
>> +++ b/mercurial/merge.py
>> @@ -175,8 +175,20 @@ class mergestate(object):
>>               if err.errno != errno.ENOENT:
>>                   raise
>>           return records
>> +    def active(self):
>> +        """Whether mergestate is active.
>> +
>> +        Returns True if there appears to be mergestate. This is a
>> rough proxy
>> +        for "is a merge in progress."
>> +        """
>> +        # Check local variables before looking at filesystem for
>> performance
>> +        # reasons.
>> +        return bool(self._local) or bool(self._state) or \
>> +               self._repo.opener.exists(self.statepathv1) or \
>> +               self._repo.opener.exists(self.statepathv2)
>
> Did you have a response to my question on
> http://selenic.com/pipermail/mercurial-devel/2014-May/058585.html :
>
> Will there ever be a (relevant) case where _local doesn't give the full
> answer? A mergestate without  a _local (and _other) is no mergestate.
>
> It seems to me like this code is more complex than necessary.

Woops, I missed the fact your started reviewing this series.

This change is already crewed. so please choose one of

1. request unqueuing
2. request followup comit

-- 
Pierre-Yves David