Patchwork D11993: sparse: take lock before writing requirements

login
register
mail settings
Submitter phabricator
Date Jan. 12, 2022, 2:11 p.m.
Message ID <differential-rev-PHID-DREV-itr4dmxx2evclu26sydn-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50318/
State New
Headers show

Comments

phabricator - Jan. 12, 2022, 2:11 p.m.
marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  with `share-safe`, we will also write file in the store. So we take the `lock`
  before altering the requirements.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/sparse.py

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -600,38 +600,41 @@ 
     repo, includes, excludes, profiles, force=False, removing=False
 ):
     """Update the sparse config and working directory state."""
-    raw = repo.vfs.tryread(b'sparse')
-    oldincludes, oldexcludes, oldprofiles = parseconfig(repo.ui, raw, b'sparse')
-
-    oldstatus = repo.status()
-    oldmatch = matcher(repo)
-    oldrequires = set(repo.requirements)
+    with repo.lock():
+        raw = repo.vfs.tryread(b'sparse')
+        oldincludes, oldexcludes, oldprofiles = parseconfig(
+            repo.ui, raw, b'sparse'
+        )
 
-    # TODO remove this try..except once the matcher integrates better
-    # with dirstate. We currently have to write the updated config
-    # because that will invalidate the matcher cache and force a
-    # re-read. We ideally want to update the cached matcher on the
-    # repo instance then flush the new config to disk once wdir is
-    # updated. But this requires massive rework to matcher() and its
-    # consumers.
+        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
+        # because that will invalidate the matcher cache and force a
+        # re-read. We ideally want to update the cached matcher on the
+        # repo instance then flush the new config to disk once wdir is
+        # updated. But this requires massive rework to matcher() and its
+        # consumers.
 
-    if requirements.SPARSE_REQUIREMENT in oldrequires and removing:
-        repo.requirements.discard(requirements.SPARSE_REQUIREMENT)
-        scmutil.writereporequirements(repo)
-    elif requirements.SPARSE_REQUIREMENT not in oldrequires:
-        repo.requirements.add(requirements.SPARSE_REQUIREMENT)
-        scmutil.writereporequirements(repo)
+        if requirements.SPARSE_REQUIREMENT in oldrequires and removing:
+            repo.requirements.discard(requirements.SPARSE_REQUIREMENT)
+            scmutil.writereporequirements(repo)
+        elif requirements.SPARSE_REQUIREMENT not in oldrequires:
+            repo.requirements.add(requirements.SPARSE_REQUIREMENT)
+            scmutil.writereporequirements(repo)
 
-    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.writereporequirements(repo)
-        writeconfig(repo, oldincludes, oldexcludes, oldprofiles)
-        raise
+        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.writereporequirements(repo)
+            writeconfig(repo, oldincludes, oldexcludes, oldprofiles)
+            raise
 
 
 def clearrules(repo, force=False):