Page MenuHomePhabricator

shelve: first prototype of storing unresolved changes
AbandonedPublic

Authored by navaneeth.suresh on Wed, Jun 5, 2:36 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit added a subscriber: pulkit.Thu, Jun 6, 1:02 PM

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.

pulkit added inline comments.Mon, Jun 10, 1:37 PM
hgext/shelve.py
511

vfs.exists can be used

511

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

512

vfs.makedir or vfs.mkdir can be used

513

vfs.rename should be used.

616

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.

pulkit added inline comments.Mon, Jun 10, 1:41 PM
hgext/shelve.py
616

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.

navaneeth.suresh marked 4 inline comments as done.Mon, Jun 10, 3:41 PM
navaneeth.suresh updated this revision to Diff 15432.
hgext/shelve.py
616

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.

pulkit added inline comments.Mon, Jun 10, 6:45 PM
hgext/shelve.py
435

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

616

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

1107

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

navaneeth.suresh marked 6 inline comments as done.Tue, Jun 11, 8:08 AM
navaneeth.suresh updated this revision to Diff 15436.
pulkit added inline comments.Tue, Jun 11, 2:02 PM
hgext/shelve.py
618

let's do namewidth += len('unresolved)'

hgext/shelve.py
618

ideally, that should be len(' (unresolved)'), right? i haven't seen such use cases in the codebase though.

navaneeth.suresh marked an inline comment as done.Tue, Jun 11, 3:31 PM

Adding changes to D6479 and abandoning.