Page MenuHomePhabricator

shelve: first prototype of storing/restoring unresolved changes
Needs ReviewPublic

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

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

This is an RFC patch. This does three things:

  1. Adds an --unresolved flag to hg shelve.
  2. Adds an --unresolved flag to hg unshelve.
  3. Adds an experimental config option store-in-extra to handle storage method.

When experimental.store-in-extra == True:
The mergestate records are stored in changeset extras. I have used the storage
format supported for hg versions 3.7 or later. For the time being, I have omitted
the storage of the content of the local version of unresolved files.

Otherwise:
The usual mergestate information in $HGRCPATH/merge/ is moved to
$HGRCPATH/merge-unresolved/<basename>/.

After shelving, and the working directory is updated to p1().

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

hg 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.

hg unshelve --unresolved will get the user back to the old unresolved merge by
the following steps:

Step 1: If the user has committed new changesets after shelving the changes,
they must update the working directory to one of the merge parents.

Step 2: Change parents of dirstate to p1 and p2.

Step 3: In order to restore our partially resolved state, we will move the
contents of $HGRCPATH/merge-unresolved/<basename>/
to $HGRCPATH/merge/ if experimental.store-in-extra not
set True. Otherwise, we'll write from data stored in changeset extras.

Step 4: We now have a state in which files marked as resolved might have
conflicts. But, we will apply the changes in shelve on the top of this so that we
can get our old unresolved merge again by the usual unshelve mechanism.

The usual rebase step is avoided on unresolved shelve changesets. Also, we
use the functions present in merge.py to read/write mergestate records
from/to changeset extras.

$ hg unshelve --unresolved will abort when:

  1. The working directory is dirty.
  2. If there is an ongoing merge.
  3. If the working directory is not at either p1 or p2.

(p1, p2 are the parents of the unresolved shelve changeset)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit added a subscriber: pulkit.Mon, Jun 10, 10:40 AM

step 2: Internally, merge p1 with p2 with the merge tool :fail.
This will not update the contents of the files with conflicting changes.
Instead, it will mark them as unresolved.

Can we prevent merging again? Since we are getting content from old unresolved commit, copying mergestate from previously stored one, what purpose does this merging serves?

step 3: This internal merge will also mark the files which are already
resolved by the user in the unresolved shelve changeset as unresolved. But,
we will move the contents of $HGRCPATH/merge-unresolved/<basename>/
to $HGRCPATH/merge/ so that we can restore the partially resolved states.
step 4: We now have a state in which files marked as resolved might have
conflicts. But, we will apply the changes in shelve on the top of this so that we
can get our old unresolved merge again by the usual unshelve mechanism.
The usual rebase step is avoided on unresolved shelve changesets.

Can we prevent merging again? Since we are getting content from old unresolved commit, copying mergestate from previously stored one, what purpose does this merging serves?

The usual unshelve mechanism restores the content of the files in the mergestate. However, we'll be still in a state with only one parent. This merge will help to recreate the exact same state with two parents.

Can we prevent merging again? Since we are getting content from old unresolved commit, copying mergestate from previously stored one, what purpose does this merging serves?

The usual unshelve mechanism restores the content of the files in the mergestate. However, we'll be still in a state with only one parent. This merge will help to recreate the exact same state with two parents.

There are other ways to set the second parent, like dirstate.setparents() I think. It is worth investigating to find a way to do it without merging again.

In D6479#94556, @pulkit wrote:

Can we prevent merging again? Since we are getting content from old unresolved commit, copying mergestate from previously stored one, what purpose does this merging serves?

The usual unshelve mechanism restores the content of the files in the mergestate. However, we'll be still in a state with only one parent. This merge will help to recreate the exact same state with two parents.

There are other ways to set the second parent, like dirstate.setparents() I think. It is worth investigating to find a way to do it without merging again.

Interesting. Then, I shall investigate it and update the diff.

navaneeth.suresh edited the summary of this revision. (Show Details)Mon, Jun 10, 12:29 PM
navaneeth.suresh updated this revision to Diff 15420.
pulkit added inline comments.Mon, Jun 10, 12:35 PM
hgext/shelve.py
768

why do we need to do this?

770

if merge-unresolved/ does not exists, it means there are no unresolved shelves, right? we should not be creating it then.

1092

we can do this check by looking into shelvectx extras below.

navaneeth.suresh marked 2 inline comments as done.Mon, Jun 10, 1:11 PM
navaneeth.suresh updated this revision to Diff 15423.
navaneeth.suresh marked an inline comment as done.Mon, Jun 10, 1:12 PM
pulkit added inline comments.Mon, Jun 10, 1:39 PM
hgext/shelve.py
771

vfs.rename should be used here.

navaneeth.suresh marked an inline comment as done.Mon, Jun 10, 2:41 PM
navaneeth.suresh updated this revision to Diff 15431.
pulkit added inline comments.Mon, Jun 10, 6:47 PM
hgext/shelve.py
764

the functions needs documentation.

769

this comment can be a part of function documentation.

navaneeth.suresh marked 2 inline comments as done.Tue, Jun 11, 7:30 AM
navaneeth.suresh updated this revision to Diff 15435.

Can you fold both the commits, it will be better to see code for both storing and restoring mergestates in one patch.

hgext/shelve.py
772

Let's move this parent changing of dirstate out of this function. This functions should only have logic related to storing and restoring unresolved states.

1114

let's have shelvectx an optional argument to _isunresolvedshelve(). If shelvectx is passed we use that, otherwise we fallback to checking the directory in merge-unresolved/.

And then use that function here.

navaneeth.suresh marked 2 inline comments as done.Tue, Jun 11, 3:21 PM
navaneeth.suresh updated this revision to Diff 15449.
navaneeth.suresh retitled this revision from unshelve: first prototype of restoring unresolved changes to shelve: first prototype of storing/restoring unresolved changes.Tue, Jun 11, 3:45 PM
navaneeth.suresh edited the summary of this revision. (Show Details)
navaneeth.suresh updated this revision to Diff 15451.
In D6479#94711, @pulkit wrote:

Can you fold both the commits, it will be better to see code for both storing and restoring mergestates in one patch.

@pulkit I've folded the changesets and updated the revision. Abandoned D6478. This looks like a huge change though. No worries as this work more or less like an RFC patch.

navaneeth.suresh edited the summary of this revision. (Show Details)Fri, Jun 14, 3:56 PM
navaneeth.suresh updated this revision to Diff 15517.

Now, we have mergestate information storage in changeset level too with the help of extra mapping.

pulkit added inline comments.Sun, Jun 16, 10:23 AM
tests/test-shelve-unresolved.t
139

this if and endif can be removed.

170

We can get rid of this #if and #endif too. the usualstorage one will have (unresolved) in the test output. We have functionality to have test output case wise which can be used.

For example, here we have two different outputs in py3 and non-py3 cases. https://www.mercurial-scm.org/repo/hg-committed/file/tip/tests/test-debugcommands.t#l213

254

we don't need this #if and #endif, it can be removed.

280

we don't need this #if and #endif, it can be removed.

345

we don't need this #if and #endif, it can be removed.

372

we don't need this #if and #endif, it can be removed.

443

we don't need this #if and #endif, it can be removed.

529

we don't need this #if and #endif, it can be removed.

navaneeth.suresh marked 7 inline comments as done.Sun, Jun 16, 11:13 AM
navaneeth.suresh updated this revision to Diff 15534.
tests/test-shelve-unresolved.t
170

fixed. i added (extrastorage !) manually to the corresponding test case. this will prevent it from changing on executing with usualstorage test case. thanks!