Patchwork D6478: shelve: first prototype of storing unresolved changes

login
register
mail settings
Submitter phabricator
Date June 5, 2019, 7:05 p.m.
Message ID <differential-rev-PHID-DREV-u2ear6mw7ouqxc6omipc-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/40326/
State Superseded
Headers show

Comments

phabricator - June 5, 2019, 7:05 p.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Before storing unresolved mergestate information in changeset level, I tried
  to make the user store the partially resolved merge to restore for later
  after fixing an urgent bug using the usual mergestate information in
  `$HGRCPATH/merge/` only.
  
  This patch adds an `--unresolved` flag to `shelve`. Until now, a `shelve`
  during a merge was aborted. This patch makes `shelve` to handle merge with
  unresolved files. The usual mergestate information in `$HGRCPATH/merge/` is
  moved to `$HGRCPATH/merge-unresolved/<basename>/` after shelving
  and the working directory is updated to `p1()`. In the next patch, I'll make
  `unshelve` to restore this information.
  
  Unresolved shelve changesets are stored with the extra mapping
  `{'unresolved-merge': True}`. Also, `hg shelve --patch` and
  `hg shelve --list` will show unresolved files separately as:
  
    $ hg shelve --list
    default (unresolved)         (1s ago)    changes to: C
  
  `shelve` will abort when:
  
  1. No merge is active on calling with `--unresolved`.
  2. No unresolved files found in active merge on calling with `--unresolved`.
  3. Merge is active on calling without `--unresolved`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

AFFECTED FILES
  hgext/shelve.py
  tests/test-shelve-unresolved.t
  tests/test-shelve.t

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
phabricator - June 6, 2019, 5:02 p.m.
pulkit added a comment.


  Over past few days, @navaneeth.suresh was working on building the initial prototype on bitbucket and I was reviewing his initial work there. Now we have a nice initial prototype ready, and we decided to move things to core.
  
  This is a prototype, which helps us (especially Navaneeth) understand more about the things required to store etc. Right now, it just moves files from merge/ to merge-unresolved/<shelve-name>/ and storing an extra in the commit to denote that it contains an unresolved merge.
  
  Reviews and ideas on how to move forward are greatly appreciated.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - June 10, 2019, 5:38 p.m.
pulkit added inline comments.

INLINE COMMENTS

> shelve.py:497
> +            extra['unresolved-merge'] = True
> +            if not os.path.exists(repo.vfs.join('merge-unresolved')):
> +                util.makedir(repo.vfs.join('merge-unresolved'), False)

`vfs.exists` can be used

> shelve.py:497
> +            extra['unresolved-merge'] = True
> +            if not os.path.exists(repo.vfs.join('merge-unresolved')):
> +                util.makedir(repo.vfs.join('merge-unresolved'), False)

also, this storing a mergestate code should go in a separate function.

> shelve.py:498
> +            if not os.path.exists(repo.vfs.join('merge-unresolved')):
> +                util.makedir(repo.vfs.join('merge-unresolved'), False)
> +            util.rename(repo.vfs.join('merge'),

`vfs.makedir` or `vfs.mkdir` can be used

> shelve.py:499
> +                util.makedir(repo.vfs.join('merge-unresolved'), False)
> +            util.rename(repo.vfs.join('merge'),
> +                        repo.vfs.join('merge-unresolved/%s/' % name))

`vfs.rename` should be used.

> shelve.py:605
>              continue
> +        if os.path.exists(repo.vfs.join('merge-unresolved/%s' % sname)):
> +            sname += ' (unresolved)'

can we look into changeset extras here to decide whether this is an unresolved shelve or not here?

It will be nice to remove any details of our storage layer and only keep them to functions which stores and restores merge states.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - June 10, 2019, 5:41 p.m.
pulkit added inline comments.

INLINE COMMENTS

> pulkit wrote in shelve.py:605
> can we look into changeset extras here to decide whether this is an unresolved shelve or not here?
> 
> It will be nice to remove any details of our storage layer and only keep them to functions which stores and restores merge states.

if looking into changeset extras can be expensive right now, let's have a function which tells whether a shelve is a unresolved shelve or not and move this code there.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - June 10, 2019, 7:50 p.m.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> pulkit wrote in shelve.py:605
> can we look into changeset extras here to decide whether this is an unresolved shelve or not here?
> 
> It will be nice to remove any details of our storage layer and only keep them to functions which stores and restores merge states.

i don't think we can get `shelvectx` here. i tried the following code but, that failed to work(got the error: node not found). the shelve changesets can't be used during other than unshelve i guess (reference from `_unshelverestorecommit()`).

  snode = shelvedfile(repo, sname, 'shelve').readinfo()['node']
  shelvectx = repo[snode]

also, i can't find any global variable declarations to make the extra parsing one time only. one workaround should be adding a `unresolved-merge: True` to the `shelvedfile`. but, it uses `scmutil.simplekeyvaluefile()` to write only a key-value pair which is its node id.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - June 10, 2019, 10:45 p.m.
pulkit added inline comments.

INLINE COMMENTS

> shelve.py:435
>  
> +def _storeunresolvedmerge(repo, name):
> +    if not repo.vfs.exists('merge-unresolved'):

the function needs documentation on what this does and why it does so.

> shelve.py:606
>              continue
> +        if repo.vfs.exists('merge-unresolved/%s' % sname):
> +            sname += ' (unresolved)'

okay, then let's move this one liner to a separate function. Something like `isunresolvedshelve(..)` which does the repo.vfs.exists() thing.

> shelve.py:1094
> +          ('', 'unresolved', None,
> +           _('unshelve mergestate with unresolved files'))] + cmdutil.walkopts,
>           _('hg shelve [OPTION]... [FILE]...'),

nit: will be nice to keep `)] + cmdutil.walkopts,` to a new line. as it was before.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6478

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel

Patch

diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -85,6 +85,7 @@ 
       --stat                output diffstat-style summary of changes (provide
                             the names of the shelved changes as positional
                             arguments)
+      --unresolved          unshelve mergestate with unresolved files
    -I --include PATTERN [+] include names matching the given patterns
    -X --exclude PATTERN [+] exclude names matching the given patterns
       --mq                  operate on patch repository
diff --git a/tests/test-shelve-unresolved.t b/tests/test-shelve-unresolved.t
new file mode 100644
--- /dev/null
+++ b/tests/test-shelve-unresolved.t
@@ -0,0 +1,158 @@ 
+Test shelve with unresolved mergestate
+
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > shelve =
+  > EOF
+
+  $ hg init shelve-unresolved
+  $ cd shelve-unresolved
+  $ echo A >> file1
+  $ echo A >> file2
+  $ hg ci -Am A
+  adding file1
+  adding file2
+  $ echo foo >> bar
+  $ hg add bar
+
+-- should abort on absence of mergestate
+  $ hg shelve --unresolved
+  abort: no active mergestate found
+  [255]
+
+  $ hg forget bar
+  $ echo B >> file1
+  $ echo B >> file2
+  $ hg ci -m B
+  $ hg up 0
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ echo C >> file1
+  $ echo C >> file2
+  $ hg ci -m C
+  created new head
+  $ hg merge
+  merging file1
+  merging file2
+  warning: conflicts while merging file1! (edit, then use 'hg resolve --mark')
+  warning: conflicts while merging file2! (edit, then use 'hg resolve --mark')
+  0 files updated, 0 files merged, 0 files removed, 2 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon
+  [1]
+
+-- let's partially solve the conflicts
+  $ cat > file1 <<EOF
+  > A
+  > B
+  > C
+  > EOF
+  $ hg resolve -m file1
+
+-- mark file2 as resolved to check abort
+  $ hg resolve -m file2
+  (no more unresolved files)
+  $ hg log -G
+  @  changeset:   2:69004294ad57
+  |  tag:         tip
+  |  parent:      0:c32ef6121744
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     C
+  |
+  | @  changeset:   1:fd9a4049234b
+  |/   user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     B
+  |
+  o  changeset:   0:c32ef6121744
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     A
+  
+  $ cat file1
+  A
+  B
+  C
+  $ hg diff
+  diff -r 69004294ad57 file1
+  --- a/file1	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/file1	Thu Jan 01 00:00:00 1970 +0000
+  @@ -1,2 +1,3 @@
+   A
+  +B
+   C
+  diff -r 69004294ad57 file2
+  --- a/file2	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/file2	Thu Jan 01 00:00:00 1970 +0000
+  @@ -1,2 +1,6 @@
+   A
+  +<<<<<<< working copy: 69004294ad57 - test: C
+   C
+  +=======
+  +B
+  +>>>>>>> merge rev:    fd9a4049234b - test: B
+
+-- should abort on absence of conflicts
+  $ hg shelve
+  abort: mergestate found
+  try with --unresolved to shelve conflicts
+  [255]
+  $ hg shelve --unresolved
+  abort: no unresolved files found in the mergestate
+  [255]
+  $ hg resolve -u file2
+  $ hg resolve -l
+  R file1
+  U file2
+
+-- should suggest --unresolved on shelving with mergestate
+  $ hg shelve
+  abort: mergestate found
+  try with --unresolved to shelve conflicts
+  [255]
+
+  $ hg shelve --unresolved
+  shelved as default
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cat file1
+  A
+  C
+  $ hg log -G
+  @  changeset:   2:69004294ad57
+  |  tag:         tip
+  |  parent:      0:c32ef6121744
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     C
+  |
+  | o  changeset:   1:fd9a4049234b
+  |/   user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     B
+  |
+  o  changeset:   0:c32ef6121744
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     A
+  
+  $ hg shelve --list
+  default (unresolved)         (1s ago)    changes to: C
+  $ hg shelve --patch
+  default (unresolved)         (1s ago)    changes to: C
+  
+  diff --git a/file1 b/file1
+  --- a/file1
+  +++ b/file1
+  @@ -1,2 +1,3 @@
+   A
+  +B
+   C
+  diff --git a/file2 b/file2
+  --- a/file2
+  +++ b/file2
+  @@ -1,2 +1,6 @@
+   A
+  +<<<<<<< working copy: 69004294ad57 - test: C
+   C
+  +=======
+  +B
+  +>>>>>>> merge rev:    fd9a4049234b - test: B
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -25,6 +25,7 @@ 
 import collections
 import errno
 import itertools
+import os
 import stat
 
 from mercurial.i18n import _
@@ -454,10 +455,19 @@ 
 def _docreatecmd(ui, repo, pats, opts):
     wctx = repo[None]
     parents = wctx.parents()
-    if len(parents) > 1:
-        raise error.Abort(_('cannot shelve while merging'))
+    unresolved = opts.get('unresolved')
     parent = parents[0]
     origbranch = wctx.branch()
+    ms = merge.mergestate.read(repo)
+    if unresolved:
+        if not ms.active():
+                raise error.Abort(_('no active mergestate found'))
+        elif not list(ms.unresolved()):
+            raise error.Abort(_('no unresolved '
+                                'files found in the mergestate'))
+    elif ms.active():
+        raise error.Abort(_('mergestate found\n'
+                            'try with --unresolved to shelve conflicts'))
 
     if parent.node() != nodemod.nullid:
         desc = "changes to: %s" % parent.description().split('\n', 1)[0]
@@ -482,6 +492,12 @@ 
         name = getshelvename(repo, parent, opts)
         activebookmark = _backupactivebookmark(repo)
         extra = {'internal': 'shelve'}
+        if unresolved:
+            extra['unresolved-merge'] = True
+            if not os.path.exists(repo.vfs.join('merge-unresolved')):
+                util.makedir(repo.vfs.join('merge-unresolved'), False)
+            util.rename(repo.vfs.join('merge'),
+                        repo.vfs.join('merge-unresolved/%s/' % name))
         if includeunknown:
             _includeunknownfiles(repo, pats, opts, extra)
 
@@ -577,21 +593,25 @@ 
     """subcommand that displays the list of shelves"""
     pats = set(pats)
     width = 80
+    namewidth = 16
     if not ui.plain():
         width = ui.termwidth()
     namelabel = 'shelve.newest'
     ui.pager('shelve')
     for mtime, name in listshelves(repo):
         sname = util.split(name)[1]
         if pats and sname not in pats:
             continue
+        if os.path.exists(repo.vfs.join('merge-unresolved/%s' % sname)):
+            sname += ' (unresolved)'
+            namewidth += 13
         ui.write(sname, label=namelabel)
         namelabel = 'shelve.name'
         if ui.quiet:
             ui.write('\n')
             continue
-        ui.write(' ' * (16 - len(sname)))
-        used = 16
+        ui.write(' ' * (namewidth - len(sname)))
+        used = namewidth
         date = dateutil.makedate(mtime)
         age = '(%s)' % templatefilters.age(date, abbrev=True)
         ui.write(age, label='shelve.age')
@@ -1068,8 +1088,9 @@ 
            _('interactive mode, only works while creating a shelve')),
           ('', 'stat', None,
            _('output diffstat-style summary of changes (provide the names of '
-             'the shelved changes as positional arguments)')
-           )] + cmdutil.walkopts,
+             'the shelved changes as positional arguments)')),
+          ('', 'unresolved', None,
+           _('unshelve mergestate with unresolved files'))] + cmdutil.walkopts,
          _('hg shelve [OPTION]... [FILE]...'),
          helpcategory=command.CATEGORY_WORKING_DIRECTORY)
 def shelvecmd(ui, repo, *pats, **opts):