Patchwork [6,of,6] update: add config for default way of handling dirty wdir

# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1487030585 28800
#      Mon Feb 13 16:03:05 2017 -0800
# Node ID 9df8dd6fd23bd52f4acaf87ea455749a97da4099
# Parent  75e5a492d7f69420a554fe498ae24060c755b09f
update: add config for default way of handling dirty wdir

This allows the user to set e.g. ui.updatecheck=abort to abort update
if the working directory is dirty, but still be able to override the
behavior with e.g. --merge when needed.

I considered adding a --mergelinear option to get back the old
behavior even when ui.updatecheck=abort is set, but I couldn't see why
anyone would prefer that over --merge.

On Tue, Feb 14, 2017 at 10:58 AM, Gábor STEFANIK <Gabor.STEFANIK@nng.com> wrote:
>>
>
>
> --------------------------------------------------------------------------
> This message, including its attachments, is confidential. For more information please read NNG's email policy here:
> http://www.nng.com/emailpolicy/
> By responding to this email you accept the email policy.
>
>
> -----Original Message-----
>> From: Mercurial-devel [mailto:mercurial-devel-bounces@mercurial-scm.org]
>> On Behalf Of Martin von Zweigbergk via Mercurial-devel
>> Sent: Tuesday, February 14, 2017 2:07 AM
>> To: mercurial-devel@mercurial-scm.org
>> Subject: [PATCH 6 of 6] update: add config for default way of handling dirty
>> wdir
>>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com> # Date
>> 1487030585 28800
>> #      Mon Feb 13 16:03:05 2017 -0800
>> # Node ID 9df8dd6fd23bd52f4acaf87ea455749a97da4099
>> # Parent  75e5a492d7f69420a554fe498ae24060c755b09f
>> update: add config for default way of handling dirty wdir
>>
>> This allows the user to set e.g. ui.updatecheck=abort to abort update if the
>> working directory is dirty, but still be able to override the behavior with e.g. --
>> merge when needed.
>>
>> I considered adding a --mergelinear option to get back the old behavior even
>> when ui.updatecheck=abort is set, but I couldn't see why anyone would
>> prefer that over --merge.
>>
>> diff -r 75e5a492d7f6 -r 9df8dd6fd23b mercurial/commands.py
>> --- a/mercurial/commands.py   Mon Feb 13 12:58:37 2017 -0800
>> +++ b/mercurial/commands.py   Mon Feb 13 16:03:05 2017 -0800
>> @@ -6544,7 +6544,14 @@
>>          raise error.Abort(_("can only specify one of -c/--check, -C/--clean, "
>>                              "and -m/merge"))
>>
>> -    updatecheck = 'linear'
>> +    # Mapping of config value to internal value
>> +    updatecheckconfigs = {
>> +        "abort": "abort",
>> +        "merge": None,
>> +        "mergelinear": "linear"
>> +    }
>> +    configvalue = ui.config('ui', 'updatecheck')
>> +    updatecheck = updatecheckconfigs.get(configvalue, "linear")
>
> Would be better to have this inside merge.update(), so extension authors
> who call that don't have to manually update extensions to follow
> this config option.

As I also said in patch 5/6, I don't think other callers should get a
behavior that depends on config.

>
> Also, "abort", "merge", "linear" is IMHO fine for terminology, and would
> do away with this mapping dict.

Will do.

>
>>      if check:
>>          updatecheck = 'abort'
>>      elif merge:
>> diff -r 75e5a492d7f6 -r 9df8dd6fd23b mercurial/help/config.txt
>> --- a/mercurial/help/config.txt       Mon Feb 13 12:58:37 2017 -0800
>> +++ b/mercurial/help/config.txt       Mon Feb 13 16:03:05 2017 -0800
>> @@ -1966,6 +1966,23 @@
>>      on all exceptions, even those recognized by Mercurial (such as
>>      IOError or MemoryError). (default: False)
>>
>> +``updatecheck``
>> +    The default strategy for ``hg update`` to handle a dirty working copy. Can
>> +    be ``abort``, ``merge`` or ``mergelinear``.
>> +
>> +    ``abort``
>> +      Abort if the working copy is dirty.
>> +
>> +      ``merge``
>> +      Allow a dirty working copy and carry changes over to destination.
>> +
>> +      ``mergelinear``
>> +      Allow a dirty working copy and carry changes over to destination, but
>> +      fail if the destination is not an ancestor or descendant of the working
>> +      copy parent.
>> +
>> +    (default: ``mergelinear``)
>
> Again, I would prefer plain "linear".

Will do.

>
> Plus, see my earlier comment about evolution; though maybe it would be better
> to solve that by reverting the behavior to really test for "linear" rather than
> "evolutionarily quasilinear" - IIRC evolution is still experimental, so no BC issue
> there (correct me if I'm wrong), and I don't expect anyone to intentionally rely on
> the default behavior while using evolution.

That's outside the scope of this series, and I don't want to include
it. I agree that
we should move away that mode over time. I'll have some further
patches after this
series that should help with that . They add a mode that allows merges as long
as they are don't involve file merges. Hopefully that mode is a good enough
compromise between safety (which would favor 'updatecheck=abort') and
BC (which I'd say prevents 'updatecheck=abort' from becoming the default).

>
>> +
>>  ``username``
>>      The committer of a changeset created when running "commit".
>>      Typically a person's name and email address, e.g. ``Fred Widget diff -r
>> 75e5a492d7f6 -r 9df8dd6fd23b tests/test-update-branches.t
>> --- a/tests/test-update-branches.t    Mon Feb 13 12:58:37 2017 -0800
>> +++ b/tests/test-update-branches.t    Mon Feb 13 16:03:05 2017 -0800
>> @@ -195,6 +195,58 @@
>>    parent=1
>>    M foo
>>
>> +  $ echo '[ui]' >> .hg/hgrc
>> +  $ echo 'updatecheck = abort' >> .hg/hgrc
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2
>> +  abort: uncommitted changes
>> +  parent=1
>> +  M foo
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2 -c
>> +  abort: uncommitted changes
>> +  parent=1
>> +  M foo
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2 -C
>> +  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  parent=2
>> +
>> +  $ echo 'updatecheck = merge' >> .hg/hgrc
>> +
>> +  $ revtest 'dirty cross'  dirty 3 4
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  parent=4
>> +  M foo
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  parent=2
>> +  M foo
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2 -c
>> +  abort: uncommitted changes
>> +  parent=1
>> +  M foo
>> +
>> +  $ revtest 'none dirty linear' dirty 1 2 -C
>> +  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  parent=2
>
> Please include a nonlinear test where a filemerge is performed, and
> actually check the resulting file content. This is to catch potential errors
> where the wrong ancestor is used for merging.

Will do.

>
>> +
>> +  $ echo 'updatecheck = mergelinear' >> .hg/hgrc
>> +
>> +  $ revtest 'dirty cross'  dirty 3 4
>> +  abort: uncommitted changes
>> +  (commit or update --clean to discard changes)
>> +  parent=3
>> +  M foo
>> +
>> +Setup for later tests
>> +  $ revtest 'none dirty linear' dirty 1 2 -c
>> +  abort: uncommitted changes
>> +  parent=1
>> +  M foo
>> +
>>    $ cd ..
>>
>>  Test updating to null revision
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel