mq: update .hgsubstate if subrepos are clean (issue2499)

This patch prevents MQ from creating an inconsistent subrepo state. If
the .hgsub file has been changed, and none of the subrepos have
uncommitted changes, creating or updating a patch (using qnew, qrefresh,
or qrecord) will update .hgsubstate accordingly.

If any subrepos _do_ have uncommitted changes, qnew/qrefresh/qrecord
will abort.

Thanks to pmezard for proposing this solution.

tests/test-mq-subrepo.t

@@ -0,0 +1,342 b''
+  $ echo "[extensions]" >> $HGRCPATH
+  $ echo "mq=" >> $HGRCPATH
+  $ echo "record=" >> $HGRCPATH
+  $ echo "[diff]" >> $HGRCPATH
+  $ echo "nodates=1" >> $HGRCPATH
+
+fn to create new repository w/dirty subrepo, and cd into it
+  $ mkrepo() {
+  >     hg init $1
+  >     cd $1
+  >     hg qinit
+  > }
+
+fn to create dirty subrepo
+  $ mksubrepo() {
+  >     hg init $1
+  >     cd $1
+  >     echo a > a
+  >     hg add
+  >     cd ..
+  > }
+
+  $ testadd() {
+  >     local stdin=`cat`
+  >     mksubrepo sub
+  >     echo sub = sub >> .hgsub
+  >     hg add .hgsub
+  >     echo % abort when adding .hgsub w/dirty subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     echo [$?]
+  >     hg -R sub ci -m0sub
+  >     echo % update substate when adding .hgsub w/clean updated subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     hg debugsub
+  > }
+
+  $ testmod() {
+  >     local stdin=`cat`
+  >     mksubrepo sub2
+  >     echo sub2 = sub2 >> .hgsub
+  >     echo % abort when modifying .hgsub w/dirty subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     echo [$?]
+  >     hg -R sub2 ci -m0sub2
+  >     echo % update substate when modifying .hgsub w/clean updated subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     hg debugsub
+  > }
+
+  $ testrm1() {
+  >     mksubrepo sub3
+  >     echo sub3 = sub3 >> .hgsub
+  >     hg ci -Aqmsub3
+  >     $EXTRA
+  >     echo b >> sub3/a
+  >     hg rm .hgsub
+  >     echo % update substate when removing .hgsub w/dirty subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     echo % debugsub should be empty
+  >     hg debugsub
+  > }
+  $ testrm2() {
+  >     mksubrepo sub4
+  >     echo sub4 = sub4 >> .hgsub
+  >     hg ci -Aqmsub4
+  >     $EXTRA
+  >     hg rm .hgsub
+  >     echo % update substate when removing .hgsub w/clean updated subrepo
+  >     hg status -S
+  >     echo '%' $*
+  >     echo "$stdin" | hg $*
+  >     echo % debugsub should be empty
+  >     hg debugsub
+  > }
+
+
+handle subrepos safely on qnew
+
+  $ mkrepo repo-2499-qnew
+  $ testadd qnew -m0 0.diff
+  adding a
+  % abort when adding .hgsub w/dirty subrepo
+  A .hgsub
+  A sub/a
+  % qnew -m0 0.diff
+  abort: uncommitted changes in subrepository sub
+  [255]
+  % update substate when adding .hgsub w/clean updated subrepo
+  A .hgsub
+  % qnew -m0 0.diff
+  committing subrepository sub
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+
+  $ testmod qnew -m1 1.diff
+  adding a
+  % abort when modifying .hgsub w/dirty subrepo
+  M .hgsub
+  A sub2/a
+  % qnew -m1 1.diff
+  abort: uncommitted changes in subrepository sub2
+  [255]
+  % update substate when modifying .hgsub w/clean updated subrepo
+  M .hgsub
+  % qnew -m1 1.diff
+  committing subrepository sub2
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+  path sub2
+   source   sub2
+   revision 1f94c7611cc6b74f5a17b16121a1170d44776845
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ testrm1 qnew -m2 2.diff
+  adding a
+  % update substate when removing .hgsub w/dirty subrepo
+  M sub3/a
+  R .hgsub
+  % qnew -m2 2.diff
+  % debugsub should be empty
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ testrm2 qnew -m3 3.diff
+  adding a
+  % update substate when removing .hgsub w/clean updated subrepo
+  R .hgsub
+  % qnew -m3 3.diff
+  % debugsub should be empty
+
+  $ cd ..
+
+
+handle subrepos safely on qrefresh
+
+  $ mkrepo repo-2499-qrefresh
+  $ hg qnew -m0 0.diff
+  $ testadd qrefresh
+  adding a
+  % abort when adding .hgsub w/dirty subrepo
+  A .hgsub
+  A sub/a
+  % qrefresh
+  abort: uncommitted changes in subrepository sub
+  [255]
+  % update substate when adding .hgsub w/clean updated subrepo
+  A .hgsub
+  % qrefresh
+  committing subrepository sub
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+
+  $ hg qnew -m1 1.diff
+  $ testmod qrefresh
+  adding a
+  % abort when modifying .hgsub w/dirty subrepo
+  M .hgsub
+  A sub2/a
+  % qrefresh
+  abort: uncommitted changes in subrepository sub2
+  [255]
+  % update substate when modifying .hgsub w/clean updated subrepo
+  M .hgsub
+  % qrefresh
+  committing subrepository sub2
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+  path sub2
+   source   sub2
+   revision 1f94c7611cc6b74f5a17b16121a1170d44776845
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ EXTRA='hg qnew -m2 2.diff' testrm1 qrefresh
+  adding a
+  % update substate when removing .hgsub w/dirty subrepo
+  M sub3/a
+  R .hgsub
+  % qrefresh
+  % debugsub should be empty
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ EXTRA='hg qnew -m3 3.diff' testrm2 qrefresh
+  adding a
+  % update substate when removing .hgsub w/clean updated subrepo
+  R .hgsub
+  % qrefresh
+  % debugsub should be empty
+
+  $ cd ..
+
+
+handle subrepos safely on qpush/qpop
+
+  $ mkrepo repo-2499-qpush
+  $ mksubrepo sub
+  adding a
+  $ hg -R sub ci -m0sub
+  $ echo sub = sub > .hgsub
+  $ hg add .hgsub
+  $ hg qnew -m0 0.diff
+  committing subrepository sub
+  $ hg debugsub
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+
+qpop
+  $ hg qpop
+  popping 0.diff
+  patch queue now empty
+  $ hg status -AS
+  $ hg debugsub
+
+qpush
+  $ hg qpush
+  applying 0.diff
+  now at: 0.diff
+  $ hg status -AS
+  C .hgsub
+  C .hgsubstate
+  C sub/a
+  $ hg debugsub
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+
+  $ cd ..
+
+
+handle subrepos safely on qrecord
+
+  $ mkrepo repo-2499-qrecord
+  $ testadd qrecord --config ui.interactive=1 -m0 0.diff <<EOF
+  > y
+  > y
+  > EOF
+  adding a
+  % abort when adding .hgsub w/dirty subrepo
+  A .hgsub
+  A sub/a
+  % qrecord --config ui.interactive=1 -m0 0.diff
+  diff --git a/.hgsub b/.hgsub
+  new file mode 100644
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  abort: uncommitted changes in subrepository sub
+  [255]
+  % update substate when adding .hgsub w/clean updated subrepo
+  A .hgsub
+  % qrecord --config ui.interactive=1 -m0 0.diff
+  diff --git a/.hgsub b/.hgsub
+  new file mode 100644
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  committing subrepository sub
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+
+  $ testmod qrecord --config ui.interactive=1 -m1 1.diff <<EOF
+  > y
+  > y
+  > EOF
+  adding a
+  % abort when modifying .hgsub w/dirty subrepo
+  M .hgsub
+  A sub2/a
+  % qrecord --config ui.interactive=1 -m1 1.diff
+  diff --git a/.hgsub b/.hgsub
+  1 hunks, 1 lines changed
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  @@ -1,1 +1,2 @@
+   sub = sub
+  +sub2 = sub2
+  record this change to '.hgsub'? [Ynsfdaq?] 
+  abort: uncommitted changes in subrepository sub2
+  [255]
+  % update substate when modifying .hgsub w/clean updated subrepo
+  M .hgsub
+  % qrecord --config ui.interactive=1 -m1 1.diff
+  diff --git a/.hgsub b/.hgsub
+  1 hunks, 1 lines changed
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  @@ -1,1 +1,2 @@
+   sub = sub
+  +sub2 = sub2
+  record this change to '.hgsub'? [Ynsfdaq?] 
+  committing subrepository sub2
+  path sub
+   source   sub
+   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+  path sub2
+   source   sub2
+   revision 1f94c7611cc6b74f5a17b16121a1170d44776845
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ EXTRA= testrm1 qrecord --config ui.interactive=1 -m2 2.diff <<EOF
+  > y
+  > y
+  > EOF
+  adding a
+  % update substate when removing .hgsub w/dirty subrepo
+  M sub3/a
+  R .hgsub
+  % qrecord --config ui.interactive=1 -m2 2.diff
+  diff --git a/.hgsub b/.hgsub
+  deleted file mode 100644
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  % debugsub should be empty
+
+  $ hg qpop -qa
+  patch queue now empty
+  $ EXTRA= testrm2 qrecord --config ui.interactive=1 -m3 3.diff <<EOF
+  > y
+  > y
+  > EOF
+  adding a
+  % update substate when removing .hgsub w/clean updated subrepo
+  R .hgsub
+  % qrecord --config ui.interactive=1 -m3 3.diff
+  diff --git a/.hgsub b/.hgsub
+  deleted file mode 100644
+  examine changes to '.hgsub'? [Ynsfdaq?] 
+  % debugsub should be empty
+
+  $ cd ..

hgext/mq.py

@@ -793,6 +793,19 b' class queue(object):'
             return top, patch
         return None, None
 
+    def check_substate(self, repo):
+        '''return list of subrepos at a different revision than substate.
+        Abort if any subrepos have uncommitted changes.'''
+        inclsubs = []
+        wctx = repo[None]
+        for s in wctx.substate:
+            if wctx.sub(s).dirty(True):
+                raise util.Abort(
+                    _("uncommitted changes in subrepository %s") % s)
+            elif wctx.sub(s).dirty():
+                inclsubs.append(s)
+        return inclsubs
+
     def check_localchanges(self, repo, force=False, refresh=True):
         m, a, r, d = repo.status()[:4]
         if (m or a or r or d) and not force:
@@ -826,16 +839,23 b' class queue(object):'
                                  % patchfn)
             else:
                 raise util.Abort(_('patch "%s" already exists') % patchfn)
+
+        inclsubs = self.check_substate(repo)
+        if inclsubs:
+            inclsubs.append('.hgsubstate')
         if opts.get('include') or opts.get('exclude') or pats:
+            if inclsubs:
+                pats = list(pats or []) + inclsubs
             match = cmdutil.match(repo, pats, opts)
             # detect missing files in pats
             def badfn(f, msg):
-                raise util.Abort('%s: %s' % (f, msg))
+                if f != '.hgsubstate': # .hgsubstate is auto-created
+                    raise util.Abort('%s: %s' % (f, msg))
             match.bad = badfn
             m, a, r, d = repo.status(match=match)[:4]
         else:
             m, a, r, d = self.check_localchanges(repo, force=True)
-            match = cmdutil.matchfiles(repo, m + a + r)
+            match = cmdutil.matchfiles(repo, m + a + r + inclsubs)
         if len(repo[None].parents()) > 1:
             raise util.Abort(_('cannot manage merge changesets'))
         commitfiles = m + a + r
@@ -1259,6 +1279,8 b' class queue(object):'
             if repo.changelog.heads(top) != [top]:
                 raise util.Abort(_("cannot refresh a revision with children"))
 
+            inclsubs = self.check_substate(repo)
+
             cparents = repo.changelog.parents(top)
             patchparent = self.qparents(repo, top)
             ph = patchheader(self.join(patchfn), self.plainmode)
@@ -1337,7 +1359,7 b' class queue(object):'
             r = list(dd)
             a = list(aa)
             c = [filter(matchfn, l) for l in (m, a, r)]
-            match = cmdutil.matchfiles(repo, set(c[0] + c[1] + c[2]))
+            match = cmdutil.matchfiles(repo, set(c[0] + c[1] + c[2] + inclsubs))
             chunks = patch.diff(repo, patchparent, match=match,
                                 changes=c, opts=diffopts)
             for chunk in chunks:

mercurial/subrepo.py

@@ -236,9 +236,10 b' def subrepo(ctx, path):'
 
 class abstractsubrepo(object):
 
-    def dirty(self):
-        """returns true if the dirstate of the subrepo does not match
-        current stored state
+    def dirty(self, ignoreupdate=False):
+        """returns true if the dirstate of the subrepo is dirty or does not
+        match current stored state. If ignoreupdate is true, only check
+        whether the subrepo has uncommitted changes in its dirstate.
         """
         raise NotImplementedError
 
@@ -390,12 +391,13 b' class hgsubrepo(abstractsubrepo):'
             s = subrepo(ctx, subpath)
             s.archive(ui, archiver, os.path.join(prefix, self._path))
 
-    def dirty(self):
+    def dirty(self, ignoreupdate=False):
         r = self._state[1]
-        if r == '':
+        if r == '' and not ignoreupdate: # no state recorded
             return True
         w = self._repo[None]
-        if w.p1() != self._repo[r]: # version checked out change
+        # version checked out changed?
+        if w.p1() != self._repo[r] and not ignoreupdate:
             return True
         return w.dirty() # working directory changed
 
@@ -538,9 +540,10 b' class svnsubrepo(abstractsubrepo):'
                     return True, True
         return bool(changes), False
 
-    def dirty(self):
-        if self._wcrev() == self._state[1] and not self._wcchanged()[0]:
-            return False
+    def dirty(self, ignoreupdate=False):
+        if not self._wcchanged()[0]:
+            if self._wcrev() == self._state[1] and not ignoreupdate:
+                return False
         return True
 
     def commit(self, text, user, date):