Patchwork D6697: cmdutil: add allowunfinished to prevent checkunfinished() on docommit()

login
register
mail settings
Submitter phabricator
Date July 26, 2019, 12:52 p.m.
Message ID <differential-rev-PHID-DREV-gvgxbwgmrlg4ypeputvx-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41078/
State New
Headers show

Comments

phabricator - July 26, 2019, 12:52 p.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  `cmdutil.dorecord()` has a `checkunfinished()` function call. We might not
  want to do that on certain occasions. For example, `unshelve --continue`
  on interactive mode is trying to create a commit during an existing
  unfinished state.
  
  We are now allowing that case using an option `interactive-unshelve`.
  However, there should be a general way to do that. This patch adds
  a `allowunfinished` optional argument to the `cmdutil.dorecord()`
  function. In the next patch, I will make unshelve to migrate to
  this argument by removing `interactive-unshelve` option.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
phabricator - July 26, 2019, 1 p.m.
pulkit added inline comments.

INLINE COMMENTS

> cmdutil.py:274
>          """
> -        if not opts.get('interactive-unshelve'):
> +        if not (allowunfinished or opts.get('interactive-unshelve')):
>              checkunfinished(repo, commit=True)

Why not get rid of `interactive-shelve` passed by unshelve and make that pass `allowunfinished=True` to dorecord?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 26, 2019, 1:02 p.m.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> pulkit wrote in cmdutil.py:274
> Why not get rid of `interactive-shelve` passed by unshelve and make that pass `allowunfinished=True` to dorecord?

I thought of splitting into two patches. Will be amending the changes soon in this patch itself.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 29, 2019, 9:41 a.m.
This revision is now accepted and ready to land.
pulkit added a comment.
pulkit accepted this revision.


  > cmdutil: add allowunfinished to prevent checkunfinished() on docommit()
  
  s/docommit/dorecord/ in flight

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - July 29, 2019, 10:04 a.m.
pulkit added a comment.


  `test-commit-interactive.t` and some other tests fail for me with this, can you have a look?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - July 29, 2019, 3:30 p.m.
navaneeth.suresh added a comment.


  In D6697#98061 <https://phab.mercurial-scm.org/D6697#98061>, @pulkit wrote:
  
  > `test-commit-interactive.t` and some other tests fail for me with this, can you have a look?
  
  Doing that right away!

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - Aug. 7, 2019, 10:20 p.m.
This revision now requires changes to proceed.
pulkit added a comment.
pulkit requested changes to this revision.


  The test still fails.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6697/new/

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

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

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -239,7 +239,7 @@ 
     return newchunks, newopts
 
 def dorecord(ui, repo, commitfunc, cmdsuggest, backupall,
-            filterfn, *pats, **opts):
+            filterfn, allowunfinished=False, *pats, **opts):
     opts = pycompat.byteskwargs(opts)
     if not ui.interactive():
         if cmdsuggest:
@@ -265,8 +265,13 @@ 
 
         In the end we'll record interesting changes, and everything else
         will be left in place, so the user can continue working.
+
+        We will prevent committing changesets on unfinished states if and only
+        if `allowunfinished` is not set `True`. For example, `unshelve` on
+        interactive mode does a commit during an unfinished state and we don't
+        want to prevent that.
         """
-        if not opts.get('interactive-unshelve'):
+        if not (allowunfinished or opts.get('interactive-unshelve')):
             checkunfinished(repo, commit=True)
         wctx = repo[None]
         merge = len(wctx.parents()) > 1