| Submitter | phabricator |
|---|---|
| Date | June 26, 2019, 4:46 p.m. |
| Message ID | <differential-rev-PHID-DREV-xokjqpothdrbcfa3ifvf-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/40682/ |
| State | Superseded |
| Headers | show |
Comments
pulkit added a comment. This patch needs to be rebased on tip of hg-committed as shelve is in core now. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: pulkit, mercurial-devel
taapas1128 added a comment. I have rebased it. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: pulkit, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > shelve.py:630 > + if not state: > + try: > + state = shelvedstate.load(repo) Let's take this code out into a new function and reuse that function at both the places instead of duplicating code. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: pulkit, mercurial-devel
mharbison72 added inline comments. INLINE COMMENTS > test-shelve2.t:734 > + abort: merge does not support 'hg abort' (stripbased !) > + (use 'hg commit' or 'hg merge --abort') (stripbased !) > [255] I think that you can eliminate the duplication with this: abort: no unshelve in progress (abortflag !) abort: merge does not support 'hg abort' (no-abortflag !) (use 'hg commit' or 'hg merge --abort') (no-abortflag !) REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
mharbison72 added inline comments. INLINE COMMENTS > test-shelve2.t:1 > -#testcases stripbased phasebased > +#testcases stripbased phasebased abortflag abortcommand > Shouldn't these be 2 lines, to maximize the test coverage? Presumably `--abort` and `hg abort` can work with either phase or strip based shelving. See test-narrow.t, or some of the lfs tests. #testcases stripbased phasebased #testcases abortflag abortcommand REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
taapas1128 added a comment. @mharbison72 Thanks for the suggestions. I have amended the patch accordingly. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > shelve.py:628 > > -def unshelveabort(ui, repo, state, opts): > +def _loadshelvedstate(ui, repo, continuef=False, abortf=False, **opts): > + try: continuef and abortf can be read from opts, no need to pass them separately. Also, `opts` should be pass as a dictionary here, no need to pass that as kwargs. > shelve.py:648 > + 'commit\n') > + ui.warn(msg) > + shelvedstate.clear(repo) I think we can convert this `ui.warn()` to `error.Abort()` and hence get rid of `sys.exit()`. Send a standalone patch before this one which make this an error instead of warn. > shelve.py:656 > + if not state: > + statetuple = _loadshelvedstate(ui, repo, abortf=True) > + state, opts = statetuple nit: `state, opts = _loadshelvedstate(ui, repo, {'abort'=True})` > shelve.py:658 > + state, opts = statetuple > + with repo.wlock(), repo.lock(): > try: let's take `wlock()` only again when if we are processing `hg abort`. We can define this function as `unshelveabort(ui, repo, state, abortcommand=False)` and pass state as None, and abortcommand as True. When abortcommand is True, reassign state to new one and take wlock. Also make sure to release the wlock later in case of abortcommand. > shelve.py:896 > > - try: > - state = shelvedstate.load(repo) > - if opts.get('keep') is None: > - opts['keep'] = state.keep > - except IOError as err: > - if err.errno != errno.ENOENT: > - raise > - cmdutil.wrongtooltocontinue(repo, _('unshelve')) > - except error.CorruptedState as err: > - ui.debug(pycompat.bytestr(err) + '\n') > - if continuef: > - msg = _('corrupted shelved state file') > - hint = _('please run hg unshelve --abort to abort unshelve ' > - 'operation') > - raise error.Abort(msg, hint=hint) > - elif abortf: > - msg = _('could not read shelved state file, your working copy ' > - 'may be in an unexpected state\nplease update to some ' > - 'commit\n') > - ui.warn(msg) > - shelvedstate.clear(repo) > - return > + statetuple = _loadshelvedstate(ui, repo, continuef=continuef, > + abortf=abortf, **opts) same as a comment above related to directly assigning it to `state, opts` REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
pulkit added a comment. The commit message is outdated. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
taapas1128 added a comment. updated that. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > shelve.py:648 > + 'please update to some commit\n')) > + return (state, opts) > + we can prevent returning opts here. One user does not need it and other one is passing by reference. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel
Patch
diff --git a/tests/test-abort.t b/tests/test-abort.t --- a/tests/test-abort.t +++ b/tests/test-abort.t @@ -1,9 +1,8 @@ -####TEST `hg abort` operation rebase - $ cat >> $HGRCPATH <<EOF > [extensions] > rebase= - > + > shelve= + > mq = > [phases] > publish=False > @@ -11,7 +10,7 @@ > tglog = log -G --template "{rev}:{phase} '{desc}' {branches}\n" > EOF - +####TEST `hg abort` operation rebase $ hg init a $ cd a @@ -597,8 +596,78 @@ cannot clean up public changesets 6ec71c037d94 graft aborted working directory is now at 6ec71c037d94 - - - - - + $ cd .. + +###TEST `hg abort` operation unshelve + +Prepare unshelve with a corrupted shelvedstate + $ hg init r1 && cd r1 + $ echo text1 > file && hg add file + $ hg shelve + shelved as default + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo text2 > file && hg ci -Am text1 + adding file + $ hg unshelve + unshelving change 'default' + rebasing shelved changes + merging file + warning: conflicts while merging file! (edit, then use 'hg resolve --mark') + unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue') + [1] + $ echo somethingsomething > .hg/shelvedstate + +Unshelve abort works with a corrupted shelvedstate + $ hg abort + could not read shelved state file, your working copy may be in an unexpected state + please update to some commit + +Unshelve abort fails with appropriate message if there's no unshelve in +progress + $ hg abort + abort: merge does not support 'hg abort' + (use 'hg commit' or 'hg merge --abort') + [255] + $ cd .. + +Abort due to pending changes + $ hg init onlypendingchanges + $ cd onlypendingchanges + $ touch a + $ hg ci -Aqm a + $ echo f > f + $ hg add f + $ hg shelve + shelved as default + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo g > f + $ echo 1 > a + $ hg unshelve + unshelving change 'default' + temporarily committing pending changes (restore with 'hg unshelve --abort') + rebasing shelved changes + merging f + warning: conflicts while merging f! (edit, then use 'hg resolve --mark') + unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue') + [1] + $ hg st + M f + ? f.orig + $ cat f + <<<<<<< shelve: f0be9a229575 - shelve: pending changes temporary commit + g + ======= + f + >>>>>>> working-copy: 7d4133053e12 - shelve: changes to: a + $ cat f.orig + g + $ hg ci a -m 'intermediate other change' + abort: unshelve already in progress + (use 'hg unshelve --continue' or 'hg unshelve --abort') + [255] + $ hg abort + unshelve of 'default' aborted + $ cd .. + + + diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -645,9 +645,28 @@ raise error.Abort(_('working directory parents do not match unshelve ' 'state')) -def unshelveabort(ui, repo, state, opts): - """subcommand that abort an in-progress unshelve""" - with repo.lock(): +def unshelveabort(ui, repo, state=None): + """subcommand that abort an in-progress unshelve + flag abort determines abort is called as a flag + or 'hg abort' + """ + if not state: + try: + state = shelvedstate.load(repo) + except IOError as err: + if err.errno != errno.ENOENT: + raise + cmdutil.wrongtooltocontinue(repo, _('unshelve')) + except error.CorruptedState as err: + ui.debug(pycompat.bytestr(err) + '\n') + msg = _('could not read shelved state file, your working copy ' + 'may be in an unexpected state\nplease update to some ' + 'commit\n') + ui.warn(msg) + shelvedstate.clear(repo) + return + + with repo.wlock(), repo.lock(): try: checkparents(repo, state) @@ -972,7 +991,7 @@ return if abortf: - return unshelveabort(ui, repo, state, opts) + return unshelveabort(ui, repo, state) elif continuef: return unshelvecontinue(ui, repo, state, opts) elif len(shelved) > 1: @@ -1140,6 +1159,6 @@ def extsetup(ui): statemod.addunfinished( 'unshelve', fname=shelvedstate._filename, continueflag=True, - cmdmsg=_('unshelve already in progress') + cmdmsg=_('unshelve already in progress'), abortfunc=unshelveabort )