Patchwork [6,of,8] subrepo: consider the parent repo dirty when a file is missing

login
register
mail settings
Submitter Matt Harbison
Date July 9, 2017, 11:34 p.m.
Message ID <97e2a1b90d6f74fbff1f.1499643294@Envy>
Download mbox | patch
Permalink /patch/22190/
State Accepted
Headers show

Comments

Matt Harbison - July 9, 2017, 11:34 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1499583346 14400
#      Sun Jul 09 02:55:46 2017 -0400
# Node ID 97e2a1b90d6f74fbff1f7c62dc97b16462b9a7d2
# Parent  81a3a3e44a191cfea66e25d472d65cce6cf2020d
subrepo: consider the parent repo dirty when a file is missing

This simply passes the 'missing' argument down from the context of the parent
repo, so the same rules apply.  subrepo.bailifchanged() is hardcoded to care
about missing files, because cmdutil.bailifchanged() is too.

In the end, it looks like this addresses inconsistencies with 'archive',
'identify', blackbox logs, 'merge', and 'update --check'.  I wasn't sure how to
implement this in git, so that's left for someone more familiar with it.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -289,10 +289,10 @@ 
     finally:
         repo.lfstatus = False
 
-def overridedirty(orig, repo, ignoreupdate=False):
+def overridedirty(orig, repo, ignoreupdate=False, missing=False):
     try:
         repo._repo.lfstatus = True
-        return orig(repo, ignoreupdate)
+        return orig(repo, ignoreupdate=ignoreupdate, missing=missing)
     finally:
         repo._repo.lfstatus = False
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1508,7 +1508,7 @@ 
         "check whether a working directory is modified"
         # check subrepos first
         for s in sorted(self.substate):
-            if self.sub(s).dirty():
+            if self.sub(s).dirty(missing=missing):
                 return True
         # check current working dir
         return ((merge and self.p2()) or
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -460,14 +460,15 @@ 
         """
         return False
 
-    def dirty(self, ignoreupdate=False):
+    def dirty(self, ignoreupdate=False, missing=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.
+        whether the subrepo has uncommitted changes in its dirstate.  If missing
+        is true, check for deleted files.
         """
         raise NotImplementedError
 
-    def dirtyreason(self, ignoreupdate=False):
+    def dirtyreason(self, ignoreupdate=False, missing=False):
         """return reason string if it is ``dirty()``
 
         Returned string should have enough information for the message
@@ -475,14 +476,15 @@ 
 
         This returns None, otherwise.
         """
-        if self.dirty(ignoreupdate=ignoreupdate):
+        if self.dirty(ignoreupdate=ignoreupdate, missing=missing):
             return _("uncommitted changes in subrepository '%s'"
                      ) % subrelpath(self)
 
     def bailifchanged(self, ignoreupdate=False, hint=None):
         """raise Abort if subrepository is ``dirty()``
         """
-        dirtyreason = self.dirtyreason(ignoreupdate=ignoreupdate)
+        dirtyreason = self.dirtyreason(ignoreupdate=ignoreupdate,
+                                       missing=True)
         if dirtyreason:
             raise error.Abort(dirtyreason, hint=hint)
 
@@ -815,7 +817,7 @@ 
         return total
 
     @annotatesubrepoerror
-    def dirty(self, ignoreupdate=False):
+    def dirty(self, ignoreupdate=False, missing=False):
         r = self._state[1]
         if r == '' and not ignoreupdate: # no state recorded
             return True
@@ -823,7 +825,7 @@ 
         if r != w.p1().hex() and not ignoreupdate:
             # different version checked out
             return True
-        return w.dirty() # working directory changed
+        return w.dirty(missing=missing) # working directory changed
 
     def basestate(self):
         return self._repo['.'].hex()
@@ -1202,8 +1204,10 @@ 
                     return True, True, bool(missing)
         return bool(changes), False, bool(missing)
 
-    def dirty(self, ignoreupdate=False):
-        if not self._wcchanged()[0]:
+    def dirty(self, ignoreupdate=False, missing=False):
+        wcchanged = self._wcchanged()
+        changed = wcchanged[0] or (missing and wcchanged[2])
+        if not changed:
             if self._state[1] in self._wcrevs() or ignoreupdate:
                 return False
         return True
@@ -1555,7 +1559,7 @@ 
                                (revision, self._relpath))
 
     @annotatesubrepoerror
-    def dirty(self, ignoreupdate=False):
+    def dirty(self, ignoreupdate=False, missing=False):
         if self._gitmissing():
             return self._state[1] != ''
         if self._gitisbare():
diff --git a/tests/test-merge-subrepos.t b/tests/test-merge-subrepos.t
--- a/tests/test-merge-subrepos.t
+++ b/tests/test-merge-subrepos.t
@@ -55,13 +55,13 @@ 
 
   $ rm subrepo/b
 
-TODO: a deleted subrepo file should be flagged as dirty, like the top level repo
+A deleted subrepo file is flagged as dirty, like the top level repo
 
   $ hg id --config extensions.blackbox= --config blackbox.dirty=True
-  9bfe45a197d7 tip
+  9bfe45a197d7+ tip
   $ cat .hg/blackbox.log
-  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5 (*)> id (glob)
-  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5 (*)> id --config "extensions.blackbox=" --config "blackbox.dirty=True" exited 0 * (glob)
+  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> id (glob)
+  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> id --config *extensions.blackbox=* --config *blackbox.dirty=True* exited 0 * (glob)
 
 TODO: a deleted file should be listed as such, like the top level repo
 
@@ -99,10 +99,14 @@ 
   $ hg st -S
   ! subrepo/b
 
-TODO: --check should notice a subrepo with a missing file.  It already notices
-a modified file.
+`hg update --check` notices a subrepo with a missing file, like it notices a
+missing file in the top level repo.
 
-  $ hg up -r '.^' --check --config ui.interactive=True << EOF
+  $ hg up -r '.^' --check
+  abort: uncommitted changes in subrepository 'subrepo'
+  [255]
+
+  $ hg up -r '.^' --config ui.interactive=True << EOF
   > c
   > EOF
   other [destination] changed b which local [working copy] deleted
@@ -122,8 +126,5 @@ 
 Merge sees deleted subrepo files as an uncommitted change
 
   $ hg merge @other
-   subrepository subrepo diverged (local revision: de222c2e1eac, remote revision: 7d3f8eba8116)
-  (M)erge, keep (l)ocal [working copy] or keep (r)emote [merge rev]? m
-  abort: uncommitted changes (in subrepo subrepo)
-  (use 'hg status' to list changes)
+  abort: uncommitted changes in subrepository 'subrepo'
   [255]
diff --git a/tests/test-subrepo-deep-nested-change.t b/tests/test-subrepo-deep-nested-change.t
--- a/tests/test-subrepo-deep-nested-change.t
+++ b/tests/test-subrepo-deep-nested-change.t
@@ -249,13 +249,13 @@ 
   changessincelatesttag: 4
   $ hg update -Cq .
 
-TODO: add the dirty flag for missing subrepo files
+A deleted subrepo file is flagged as dirty, like the top level repo
 
   $ rm -r ../wdir sub1/sub2/folder/test.txt
   $ hg archive -S -qr 'wdir()' ../wdir
   $ cat ../wdir/.hg_archival.txt
   repo: 7f491f53a367861f47ee64a80eb997d1f341b77a
-  node: 9bb10eebee29dc0f1201dcf5977b811a540255fd
+  node: 9bb10eebee29dc0f1201dcf5977b811a540255fd+
   branch: default
   latesttag: null
   latesttagdistance: 4