Patchwork [V2] subrepo: make it possible to update when a subrepo revision is missing

login
register
mail settings
Submitter Angel Ezquerra
Date May 4, 2014, 3:39 p.m.
Message ID <ed7ad31bca4852206268.1399217959@Angel-PC.localdomain>
Download mbox | patch
Permalink /patch/4591/
State Changes Requested
Headers show

Comments

Angel Ezquerra - May 4, 2014, 3:39 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1391950775 -3600
#      Sun Feb 09 13:59:35 2014 +0100
# Node ID ed7ad31bca48522062689a457559d25e802ef59e
# Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
subrepo: make it possible to update when a subrepo revision is missing

Up until now, updating to a revision that referred to a missing subrepo revision
would fail (after trying to pull it from the remote subrepo source).

This made it very easy to get into a situation in which it was no longer
possible to update to a revision in the parent repo (for example if the user
used MQ or rebase on the subrepo).

With this change we now show a prompt which informs the user of the problem and
lets it choose what to do. By default the update will be aborted (as it did
before this patch). However the user will be able to update anyway if he wants
to. If the user decides to update anyway the repository is placed in a new
'missingsubrev' unfinished state which makes it impossible to commit until the
user updates to another revision.

We could improve this by allowing the user to amend the broken revision. However
the unfinished state machinery is not currently able to distinguish between
commit and amend.
Matt Mackall - May 29, 2014, 10:35 p.m.
On Sun, 2014-05-04 at 17:39 +0200, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1391950775 -3600
> #      Sun Feb 09 13:59:35 2014 +0100
> # Node ID ed7ad31bca48522062689a457559d25e802ef59e
> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
> subrepo: make it possible to update when a subrepo revision is missing
> 
> Up until now, updating to a revision that referred to a missing subrepo revision
> would fail (after trying to pull it from the remote subrepo source).

http://mercurial.markmail.org/thread/ujnahcp47by4gokf

Broken checkouts are not a subrepo-specific problem. So if you want to
add a "broken checkout" state, it must not be subrepo-specific. This is
why I pointed you to the closest thing we currently have to a broken
checkout state.

And again: prompts = unloved. I'd rather have a (non-subrepo-specific)
command line flag to update to allow checking out a broken state.
Angel Ezquerra - June 7, 2014, 11:53 p.m.
On Fri, May 30, 2014 at 12:35 AM, Matt Mackall <mpm@selenic.com> wrote:
> On Sun, 2014-05-04 at 17:39 +0200, Angel Ezquerra wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>> # Date 1391950775 -3600
>> #      Sun Feb 09 13:59:35 2014 +0100
>> # Node ID ed7ad31bca48522062689a457559d25e802ef59e
>> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
>> subrepo: make it possible to update when a subrepo revision is missing
>>
>> Up until now, updating to a revision that referred to a missing subrepo revision
>> would fail (after trying to pull it from the remote subrepo source).
>
> http://mercurial.markmail.org/thread/ujnahcp47by4gokf
>
> Broken checkouts are not a subrepo-specific problem. So if you want to
> add a "broken checkout" state, it must not be subrepo-specific. This is
> why I pointed you to the closest thing we currently have to a broken
> checkout state.
>
> And again: prompts = unloved. I'd rather have a (non-subrepo-specific)
> command line flag to update to allow checking out a broken state.

OK, I've re-read your original reply and it seems that I missed that
you very clearly suggested introducing a non subrepo specific state.
Sorry about that.

I've been thinking about this a little. I have a couple of questions:

- Would you prefer that I introduced a new state (e.g.
brokencheckoutstate or incompleteupdatestate) or that I reuse the
existing "updatestate"? They are not exactly the same, but they are
quite similar...

- Would you like for the new update flag to be called --force? It
would basically mean "go ahead, try to update and do as many of the
update actions as you can, ignoring errors". For now the flag would
limit its effect to subrepo update failures (and possibly not even
cover all subrepo types at first), but it could later be expanded to
allow for other types of errors.

Cheers,

Angel
Matt Mackall - June 9, 2014, 10:40 p.m.
On Sun, 2014-06-08 at 01:53 +0200, Angel Ezquerra wrote:
> On Fri, May 30, 2014 at 12:35 AM, Matt Mackall <mpm@selenic.com> wrote:
> > On Sun, 2014-05-04 at 17:39 +0200, Angel Ezquerra wrote:
> >> # HG changeset patch
> >> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> >> # Date 1391950775 -3600
> >> #      Sun Feb 09 13:59:35 2014 +0100
> >> # Node ID ed7ad31bca48522062689a457559d25e802ef59e
> >> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
> >> subrepo: make it possible to update when a subrepo revision is missing
> >>
> >> Up until now, updating to a revision that referred to a missing subrepo revision
> >> would fail (after trying to pull it from the remote subrepo source).
> >
> > http://mercurial.markmail.org/thread/ujnahcp47by4gokf
> >
> > Broken checkouts are not a subrepo-specific problem. So if you want to
> > add a "broken checkout" state, it must not be subrepo-specific. This is
> > why I pointed you to the closest thing we currently have to a broken
> > checkout state.
> >
> > And again: prompts = unloved. I'd rather have a (non-subrepo-specific)
> > command line flag to update to allow checking out a broken state.
> 
> OK, I've re-read your original reply and it seems that I missed that
> you very clearly suggested introducing a non subrepo specific state.
> Sorry about that.
> 
> I've been thinking about this a little. I have a couple of questions:
> 
> - Would you prefer that I introduced a new state (e.g.
> brokencheckoutstate or incompleteupdatestate) or that I reuse the
> existing "updatestate"? They are not exactly the same, but they are
> quite similar...

I don't have a strong opinion here. An interrupted checkout is just a
subset of broken checkouts.

> - Would you like for the new update flag to be called --force? It
> would basically mean "go ahead, try to update and do as many of the
> update actions as you can, ignoring errors". For now the flag would
> limit its effect to subrepo update failures (and possibly not even
> cover all subrepo types at first), but it could later be expanded to
> allow for other types of errors.

Hmm. If we call it --force, users will try to use --force to bypass the
other non-integrity errors and complain that doesn't work. If we make it
work, they'll use --force habitually and then be surprised when they
have a sad. Such people deserve to have a sad, but that's perhaps not
how we should design it.

We might want something like:

    --broken      ignore integrity errors (and disable commit)

And then if people complain that --broken made them sad, we can make
them stand in the corner.

Patch

# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1391950775 -3600
#      Sun Feb 09 13:59:35 2014 +0100
# Node ID ed7ad31bca48522062689a457559d25e802ef59e
# Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
subrepo: make it possible to update when a subrepo revision is missing

Up until now, updating to a revision that referred to a missing subrepo revision
would fail (after trying to pull it from the remote subrepo source).

This made it very easy to get into a situation in which it was no longer
possible to update to a revision in the parent repo (for example if the user
used MQ or rebase on the subrepo).

With this change we now show a prompt which informs the user of the problem and
lets it choose what to do. By default the update will be aborted (as it did
before this patch). However the user will be able to update anyway if he wants
to. If the user decides to update anyway the repository is placed in a new
'missingsubrev' unfinished state which makes it impossible to commit until the
user updates to another revision.

We could improve this by allowing the user to amend the broken revision. However
the unfinished state machinery is not currently able to distinguish between
commit and amend.

diff -r cadad384c97c -r ed7ad31bca48 mercurial/cmdutil.py
--- a/mercurial/cmdutil.py	Thu May 01 17:48:02 2014 -0500
+++ b/mercurial/cmdutil.py	Sun Feb 09 13:59:35 2014 +0100
@@ -2453,7 +2453,9 @@ 
     ('graftstate', True, False, _('graft in progress'),
      _("use 'hg graft --continue' or 'hg update' to abort")),
     ('updatestate', True, False, _('last update was interrupted'),
-     _("use 'hg update' to get a consistent checkout"))
+     _("use 'hg update' to get a consistent checkout")),
+    ('missingsubrevstate', True, False, _('missing subrepo revision'),
+     _("use 'hg update' to update to another revision")),
     ]
 
 def checkunfinished(repo, commit=False):
diff -r cadad384c97c -r ed7ad31bca48 mercurial/commands.py
--- a/mercurial/commands.py	Thu May 01 17:48:02 2014 -0500
+++ b/mercurial/commands.py	Sun Feb 09 13:59:35 2014 +0100
@@ -5449,6 +5449,8 @@ 
     t = ', '.join(t)
     cleanworkdir = False
 
+    if repo.vfs.exists('missingsubrevstate'):
+        t += _(' (missing subrepo revisions)')
     if repo.vfs.exists('updatestate'):
         t += _(' (interrupted update)')
     elif len(parents) > 1:
diff -r cadad384c97c -r ed7ad31bca48 mercurial/subrepo.py
--- a/mercurial/subrepo.py	Thu May 01 17:48:02 2014 -0500
+++ b/mercurial/subrepo.py	Sun Feb 09 13:59:35 2014 +0100
@@ -169,6 +169,10 @@ 
 
     repo.ui.debug("subrepo merge %s %s %s\n" % (wctx, mctx, actx))
 
+    missingsubs = []
+    if repo.vfs.exists('missingsubrevstate'):
+        repo.vfs.unlink('missingsubrevstate')
+
     def debug(s, msg, r=""):
         if r:
             r = "%s:%s:%s" % r
@@ -189,7 +193,8 @@ 
                 continue
             elif ld == a: # other side changed
                 debug(s, "other changed, get", r)
-                wctx.sub(s).get(r, overwrite)
+                if wctx.sub(s).get(r, overwrite):
+                    missingsubs.append('%s %s\n' % (r[1], s))
                 sm[s] = r
             elif ld[0] != r[0]: # sources differ
                 if repo.ui.promptchoice(
@@ -197,11 +202,13 @@ 
                       'use (l)ocal source (%s) or (r)emote source (%s)?'
                       '$$ &Local $$ &Remote') % (s, l[0], r[0]), 0):
                     debug(s, "prompt changed, get", r)
-                    wctx.sub(s).get(r, overwrite)
+                    if wctx.sub(s).get(r, overwrite):
+                        missingsubs.append('%s %s\n' % (r[1], s))
                     sm[s] = r
             elif ld[1] == a[1]: # local side is unchanged
                 debug(s, "other side changed, get", r)
-                wctx.sub(s).get(r, overwrite)
+                if wctx.sub(s).get(r, overwrite):
+                    missingsubs.append('%s %s\n' % (r[1], s))
                 sm[s] = r
             else:
                 debug(s, "both sides changed")
@@ -219,7 +226,8 @@ 
                     sm[s] = l
                     debug(s, "keep local subrepo revision", l)
                 else:
-                    wctx.sub(s).get(r, overwrite)
+                    if wctx.sub(s).get(r, overwrite):
+                        missingsubs.append('%s %s\n' % (r[1], s))
                     sm[s] = r
                     debug(s, "get remote subrepo revision", r)
         elif ld == a: # remote removed, local unchanged
@@ -242,7 +250,8 @@ 
             continue
         elif s not in sa:
             debug(s, "remote added, get", r)
-            mctx.sub(s).get(r)
+            if mctx.sub(s).get(r):
+                missingsubs.append('%s %s\n' % (r[1], s))
             sm[s] = r
         elif r != sa[s]:
             if repo.ui.promptchoice(
@@ -250,11 +259,19 @@ 
                   'use (c)hanged version or (d)elete?'
                   '$$ &Changed $$ &Delete') % s, 0) == 0:
                 debug(s, "prompt recreate", r)
-                wctx.sub(s).get(r)
+                if wctx.sub(s).get(r):
+                    missingsubs.append('%s %s\n' % (r[1], s))
                 sm[s] = r
 
     # record merged .hgsubstate
     writestate(repo, sm)
+
+    if missingsubs:
+        repo.vfs.write('missingsubrevstate', '\n'.join(missingsubs))
+        repo.ui.warn(_("revisions are missing in %d subrepos\n"
+                     "you won't be able to commit until you update to "
+                     "another revision\n") % len(missingsubs))
+
     return sm
 
 def _updateprompt(ui, sub, dirty, local, remote):
@@ -743,7 +760,19 @@ 
                     _('revision %s in subrepo %s is hidden\n') \
                     % (revision[0:12], self._path))
                 repo = urepo
+        elif revision not in repo:
+            choice = repo.ui.promptchoice(
+                _('target revision (%s) in subrepo %s is missing\n'
+                  'do you want to continue with the update (yN)? '
+                  '$$ &Yes $$ &No') % (revision[:12], self._path), default=1)
+            if choice == 0:
+                # note that we're in the middle of an update
+                repo.ui.status(
+                    _('ignoring missing revision %s in subrepo %s\n')
+                        % (revision[:12], self._path))
+                return revision
         hg.updaterepo(repo, revision, overwrite)
+        return None
 
     @annotatesubrepoerror
     def merge(self, state):