Patchwork D110: sparse: add a requirement when a repository uses sparse (BC)

login
register
mail settings
Submitter phabricator
Date July 17, 2017, 7:15 p.m.
Message ID <differential-rev-PHID-DREV-6mdfsqe2idwnsk45eri4-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/22453/
State Not Applicable
Headers show

Comments

phabricator - July 17, 2017, 7:15 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The presence of a sparse checkout can confuse legacy clients or
  clients without sparse enabled for reasons that should be obvious.
  
  This commit introduces a new repository requirement that tracks
  whether sparse is enabled. The requirement is added when a sparse
  config is activated and removed when the sparse config is reset.
  
  The localrepository constructor has been taught to not open repos
  with this requirement unless the sparse feature is enabled. It yields
  a more actionable error message than what you would get if the
  lockout were handled strictly at the requirements verification phase.
  Old clients that aren't sparse aware will see the generic
  "repository requires features unknown to this Mercurial" error,
  however.
  
  The new requirement has "exp" in its name to reflect the
  experimental nature of sparse. There's a chance that the eventual
  non-experimental feature won't change significantly and we could
  have squatted on the "sparse" requirement without ill effect. If
  that happens, we can teach new clients to still recognize the old
  name. But I suspect we'll sneak in some BC and we'll want a new
  requirement to convey new meaning.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D110

AFFECTED FILES
  mercurial/help/internals/requirements.txt
  mercurial/localrepo.py
  mercurial/sparse.py
  tests/test-sparse-requirement.t

CHANGE DETAILS




EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - July 17, 2017, 9:37 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> sparse.py:526
>  def _updateconfigandrefreshwdir(repo, includes, excludes, profiles,
> -                                force=False):
> +                                force=False, removing=False):
>      """Update the sparse config and working directory state."""

It feels like this would be clearer as two method: the old one and a new one that just turns sparse off. That would ideally unlink .hg/sparse, no? The new method would not accept includes, excludes, or profiles, and force also doesn't seem relevant as far as I can tell.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D110

EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - July 17, 2017, 10:26 p.m.
indygreg added inline comments.

INLINE COMMENTS

> martinvonz wrote in sparse.py:526
> It feels like this would be clearer as two method: the old one and a new one that just turns sparse off. That would ideally unlink .hg/sparse, no? The new method would not accept includes, excludes, or profiles, and force also doesn't seem relevant as far as I can tell.

Yes, it would be cleaner to have a dedicated function to turn off sparse. Yes, it would unlink .hg/sparse. I was being a bit lazy when I wrote this patch because I wanted to get something in before the freeze :)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D110

EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers, durin42
Cc: martinvonz, mercurial-devel
phabricator - July 17, 2017, 11:03 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> indygreg wrote in sparse.py:526
> Yes, it would be cleaner to have a dedicated function to turn off sparse. Yes, it would unlink .hg/sparse. I was being a bit lazy when I wrote this patch because I wanted to get something in before the freeze :)

I've accepted this version, but please send a follow-up (probably after the freeze), because this got pretty hard to read (relative to what it could be).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D110

EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers, durin42
Cc: martinvonz, mercurial-devel

Patch

diff --git a/tests/test-sparse-requirement.t b/tests/test-sparse-requirement.t
new file mode 100644
--- /dev/null
+++ b/tests/test-sparse-requirement.t
@@ -0,0 +1,65 @@ 
+  $ hg init repo
+  $ cd repo
+
+  $ touch a.html b.html c.py d.py
+
+  $ cat > frontend.sparse << EOF
+  > [include]
+  > *.html
+  > EOF
+
+  $ hg -q commit -A -m initial
+
+  $ echo 1 > a.html
+  $ echo 1 > c.py
+  $ hg commit -m 'commit 1'
+
+Enable sparse profile
+
+  $ cat .hg/requires
+  dotencode
+  fncache
+  generaldelta
+  revlogv1
+  store
+
+  $ hg debugsparse --config extensions.sparse= --enable-profile frontend.sparse
+  $ ls
+  a.html
+  b.html
+
+Requirement for sparse added when sparse is enabled
+
+  $ cat .hg/requires
+  dotencode
+  exp-sparse
+  fncache
+  generaldelta
+  revlogv1
+  store
+
+Client without sparse enabled reacts properly
+
+  $ hg files
+  abort: repository is using sparse feature but sparse is not enabled; enable the "sparse" extensions to access!
+  [255]
+
+Requirement for sparse is removed when sparse is disabled
+
+  $ hg debugsparse --reset --config extensions.sparse=
+
+  $ cat .hg/requires
+  dotencode
+  fncache
+  generaldelta
+  revlogv1
+  store
+
+And client without sparse can access
+
+  $ hg files
+  a.html
+  b.html
+  c.py
+  d.py
+  frontend.sparse
diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -18,6 +18,7 @@ 
     match as matchmod,
     merge as mergemod,
     pycompat,
+    scmutil,
     util,
 )
 
@@ -522,13 +523,14 @@ 
     prunetemporaryincludes(repo)
 
 def _updateconfigandrefreshwdir(repo, includes, excludes, profiles,
-                                force=False):
+                                force=False, removing=False):
     """Update the sparse config and working directory state."""
     raw = repo.vfs.tryread('sparse')
     oldincludes, oldexcludes, oldprofiles = parseconfig(repo.ui, raw)
 
     oldstatus = repo.status()
     oldmatch = matcher(repo)
+    oldrequires = set(repo.requirements)
 
     # TODO remove this try..except once the matcher integrates better
     # with dirstate. We currently have to write the updated config
@@ -538,11 +540,21 @@ 
     # updated. But this requires massive rework to matcher() and its
     # consumers.
 
-    writeconfig(repo, includes, excludes, profiles)
+    if 'exp-sparse' in oldrequires and removing:
+        repo.requirements.discard('exp-sparse')
+        scmutil.writerequires(repo.vfs, repo.requirements)
+    elif 'exp-sparse' not in oldrequires:
+        repo.requirements.add('exp-sparse')
+        scmutil.writerequires(repo.vfs, repo.requirements)
 
     try:
+        writeconfig(repo, includes, excludes, profiles)
         return refreshwdir(repo, oldstatus, oldmatch, force=force)
     except Exception:
+        if repo.requirements != oldrequires:
+            repo.requirements.clear()
+            repo.requirements |= oldrequires
+            scmutil.writerequires(repo.vfs, repo.requirements)
         writeconfig(repo, oldincludes, oldexcludes, oldprofiles)
         raise
 
@@ -647,7 +659,8 @@ 
                         len(oldexclude - newexclude))
 
         fcounts = map(len, _updateconfigandrefreshwdir(
-            repo, newinclude, newexclude, newprofiles, force=force))
+            repo, newinclude, newexclude, newprofiles, force=force,
+            removing=reset))
 
         printchanges(repo.ui, opts, profilecount, includecount,
                      excludecount, *fcounts)
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -288,6 +288,7 @@ 
         'shared',
         'relshared',
         'dotencode',
+        'exp-sparse',
     }
     openerreqs = {
         'revlogv1',
@@ -419,6 +420,11 @@ 
             if inst.errno != errno.ENOENT:
                 raise
 
+        if 'exp-sparse' in self.requirements and not sparse.enabled:
+            raise error.RepoError(_('repository is using sparse feature but '
+                                    'sparse is not enabled; enable the '
+                                    '"sparse" extensions to access'))
+
         self.store = store.store(
                 self.requirements, self.sharedpath, vfsmod.vfs)
         self.spath = self.store.path
diff --git a/mercurial/help/internals/requirements.txt b/mercurial/help/internals/requirements.txt
--- a/mercurial/help/internals/requirements.txt
+++ b/mercurial/help/internals/requirements.txt
@@ -117,3 +117,14 @@ 
 Support for this requirement was added in Mercurial 3.4 (released
 August 2015). The requirement is currently experimental and is
 disabled by default.
+
+exp-sparse
+==========
+
+The working directory is sparse (only contains a subset of files).
+
+Support for this requirement was added in Mercurial 4.3 (released
+August 2017). This requirement and feature are experimental and may
+disappear in a future Mercurial release. The requirement will only
+be present on repositories that have opted in to a sparse working
+directory.