D9999: sshpeer: enable+fix warning about sshpeers not being closed explicitly

[valentin.gatienbaron (Valentin Gatien-Baron)]

valentin.gatienbaron created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: martinvonz.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  I recommend looking at this with a diff that ignores indentation.
  
  The test changes are because localrepo.close() updates some cache,
  which appears happens earlier now on rollbacks or strips or something.
  
  The http changes are because httppeer.close() prints stats with
  --verbose.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/infinitepush/__init__.py
  hgext/narrow/narrowcommands.py
  mercurial/commands.py
  mercurial/debugcommands.py
  mercurial/hg.py
  mercurial/revset.py
  mercurial/sshpeer.py
  mercurial/subrepo.py
  tests/remotefilelog-getflogheads.py
  tests/test-acl.t
  tests/test-http.t
  tests/test-lfs-serve.t

CHANGE DETAILS

diff --git a/tests/test-lfs-serve.t b/tests/test-lfs-serve.t
--- a/tests/test-lfs-serve.t
+++ b/tests/test-lfs-serve.t
@@ -462,6 +462,7 @@
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
+  (sent 8 HTTP requests and 3526 bytes; received 961 bytes in responses) (?)
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
   $TESTTMP/server/.hg/requires:lfs
diff --git a/tests/test-http.t b/tests/test-http.t
--- a/tests/test-http.t
+++ b/tests/test-http.t
@@ -382,6 +382,7 @@
   devel-peer-request:   16 bytes of commands arguments in headers
   devel-peer-request:   finished in *. seconds (200) (glob)
   received listkey for "phases": 15 bytes
+  (sent 9 HTTP requests and 3898 bytes; received 920 bytes in responses)
   $ hg rollback -q
 
   $ sed 's/.*] "/"/' < ../access.log
diff --git a/tests/test-acl.t b/tests/test-acl.t
--- a/tests/test-acl.t
+++ b/tests/test-acl.t
@@ -361,6 +361,7 @@
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" not allowed on "foo/file.txt" (changeset 
"ef1ea85a6374")
   no rollback information available
   0:6675d58eff77
@@ -808,7 +809,6 @@
   acl: acl.deny.bookmarks not enabled
   acl: bookmark access granted: "ef1ea85a6374b77d6da9dcda9541f498f2d17df7" on 
bookmark "moving-bookmark"
   bundle2-input-bundle: 7 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   invalid branch cache (served.hidden): tip differs
   added 1 changesets with 1 changes to 1 files
@@ -900,6 +900,7 @@
   bundle2-input-bundle: 7 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on bookmark "moving-bookmark" (changeset 
"ef1ea85a6374b77d6da9dcda9541f498f2d17df7")
   no rollback information available
   0:6675d58eff77
@@ -985,7 +986,6 @@
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1073,6 +1073,7 @@
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "wilma" not allowed on "quux/file.py" (changeset 
"911600dab2ae")
   no rollback information available
   0:6675d58eff77
@@ -1322,7 +1323,6 @@
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1499,6 +1499,7 @@
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on "foo/Bar/file.txt" (changeset 
"f9cafe1212c8")
   no rollback information available
   0:6675d58eff77
@@ -1583,7 +1584,6 @@
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1671,6 +1671,7 @@
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on "foo/Bar/file.txt" (changeset 
"f9cafe1212c8")
   no rollback information available
   0:6675d58eff77
diff --git a/tests/remotefilelog-getflogheads.py 
b/tests/remotefilelog-getflogheads.py
--- a/tests/remotefilelog-getflogheads.py
+++ b/tests/remotefilelog-getflogheads.py
@@ -21,7 +21,10 @@
 dest = repo.ui.expandpath(b'default')
 peer = hg.peer(repo, {}, dest)
 
-flogheads = peer.x_rfl_getflogheads(path)
+try:
+  

D9998: sshpeer: add a develwarning if an sshpeer is not closed explicitly

[valentin.gatienbaron (Valentin Gatien-Baron)]

valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The warning is disabled until the next commit, because fixing it
  results in a noisy diff due to indentation changes.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/remotefilelog/connectionpool.py
  mercurial/sshpeer.py

CHANGE DETAILS

diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -148,14 +148,18 @@
 return self._main.flush()
 
 
-def _cleanuppipes(ui, pipei, pipeo, pipee):
+def _cleanuppipes(ui, pipei, pipeo, pipee, warn):
 """Clean up pipes used by an SSH connection."""
-if pipeo:
+didsomething = False
+if pipeo and not pipeo.closed:
+didsomething = True
 pipeo.close()
-if pipei:
+if pipei and not pipei.closed:
+didsomething = True
 pipei.close()
 
-if pipee:
+if pipee and not pipee.closed:
+didsomething = True
 # Try to read from the err descriptor until EOF.
 try:
 for l in pipee:
@@ -165,6 +169,17 @@
 
 pipee.close()
 
+if didsomething and warn is not None:
+# Encourage explicit close of sshpeers. Closing via __del__ is
+# not very predictable when exceptions are thrown, which has led
+# to deadlocks due to a peer get gc'ed in a fork
+# We add our own stack trace, because the stacktrace when called
+# from __del__ is useless.
+if False:  # enabled in next commit
+ui.develwarn(
+b'missing close on SSH connection created at:\n%s' % warn
+)
+
 
 def _makeconnection(ui, sshcmd, args, remotecmd, path, sshenv=None):
 """Create an SSH connection to a server.
@@ -416,6 +431,7 @@
 self._pipee = stderr
 self._caps = caps
 self._autoreadstderr = autoreadstderr
+self._initstack = b''.join(util.getstackframes(1))
 
 # Commands that have a "framed" response where the first line of the
 # response contains the length of that response.
@@ -456,10 +472,11 @@
 self._cleanup()
 raise exception
 
-def _cleanup(self):
-_cleanuppipes(self.ui, self._pipei, self._pipeo, self._pipee)
-
-__del__ = _cleanup
+def _cleanup(self, warn=None):
+_cleanuppipes(self.ui, self._pipei, self._pipeo, self._pipee, 
warn=warn)
+
+def __del__(self):
+self._cleanup(warn=self._initstack)
 
 def _sendrequest(self, cmd, args, framed=False):
 if self.ui.debugflag and self.ui.configbool(
@@ -611,7 +628,7 @@
 try:
 protoname, caps = _performhandshake(ui, stdin, stdout, stderr)
 except Exception:
-_cleanuppipes(ui, stdout, stdin, stderr)
+_cleanuppipes(ui, stdout, stdin, stderr, warn=None)
 raise
 
 if protoname == wireprototypes.SSHV1:
@@ -637,7 +654,7 @@
 autoreadstderr=autoreadstderr,
 )
 else:
-_cleanuppipes(ui, stdout, stdin, stderr)
+_cleanuppipes(ui, stdout, stdin, stderr, warn=None)
 raise error.RepoError(
 _(b'unknown version of SSH protocol: %s') % protoname
 )
diff --git a/hgext/remotefilelog/connectionpool.py 
b/hgext/remotefilelog/connectionpool.py
--- a/hgext/remotefilelog/connectionpool.py
+++ b/hgext/remotefilelog/connectionpool.py
@@ -47,7 +47,7 @@
 if util.safehasattr(peer, '_cleanup'):
 
 class mypeer(peer.__class__):
-def _cleanup(self):
+def _cleanup(self, warn=None):
 # close pipee first so peer.cleanup reading it won't
 # deadlock, if there are other processes with pipeo
 # open (i.e. us).



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9996: sshpeer: add a method to check if a doublepipe is closed

[valentin.gatienbaron (Valentin Gatien-Baron)]

valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  So we can tell in a next commit if we're trying to close an already
  closed connection or not (in which case, we may warn).

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/sshpeer.py
  tests/test-check-interfaces.py

CHANGE DETAILS

diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py
--- a/tests/test-check-interfaces.py
+++ b/tests/test-check-interfaces.py
@@ -113,6 +113,10 @@
 def close(self):
 pass
 
+@property
+def closed(self):
+pass
+
 
 def main():
 ui = uimod.ui()
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -140,6 +140,10 @@
 def close(self):
 return self._main.close()
 
+@property
+def closed(self):
+return self._main.closed
+
 def flush(self):
 return self._main.flush()
 



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9997: remotefilelog: rework workaround for sshpeer deadlocks

[valentin.gatienbaron (Valentin Gatien-Baron)]

valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The wrapping of `sshpeer.cleanup` silently broke when `cleanup` was
  renamed to `_cleanup`, a couple of years ago.
  
  I don't know what `orig.im_self` is, but regardless, the intention of
  the wrapping seems pretty clear: close stderr before
  sshpeer._cleanuppipes blocks on it. So do that.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/remotefilelog/connectionpool.py

CHANGE DETAILS

diff --git a/hgext/remotefilelog/connectionpool.py 
b/hgext/remotefilelog/connectionpool.py
--- a/hgext/remotefilelog/connectionpool.py
+++ b/hgext/remotefilelog/connectionpool.py
@@ -43,17 +43,19 @@
 
 if conn is None:
 
-def _cleanup(orig):
-# close pipee first so peer.cleanup reading it won't deadlock,
-# if there are other processes with pipeo open (i.e. us).
-peer = orig.im_self
-if util.safehasattr(peer, 'pipee'):
-peer.pipee.close()
-return orig()
+peer = hg.peer(self._repo.ui, {}, path)
+if util.safehasattr(peer, '_cleanup'):
 
-peer = hg.peer(self._repo.ui, {}, path)
-if util.safehasattr(peer, 'cleanup'):
-extensions.wrapfunction(peer, b'cleanup', _cleanup)
+class mypeer(peer.__class__):
+def _cleanup(self):
+# close pipee first so peer.cleanup reading it won't
+# deadlock, if there are other processes with pipeo
+# open (i.e. us).
+if util.safehasattr(self, 'pipee'):
+self.pipee.close()
+return super(mypeer, self)._cleanup()
+
+peer.__class__ = mypeer
 
 conn = connection(pathpool, peer)
 



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9995: sshpeer: make sshpeer.close() close the underlying connection

[valentin.gatienbaron (Valentin Gatien-Baron)]

valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  So the connection can be closed eagerly in future commits, instead of
  relying on __del__.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/sshpeer.py

CHANGE DETAILS

diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -434,7 +434,7 @@
 return True
 
 def close(self):
-pass
+self._cleanup()
 
 # End of ipeerconnection interface.
 



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

SHA1 replacement steps

[Joerg Sonnenberger]

Hello all,
to help the review process along, here are the rough steps I see in
preparation for supporting 256bit hashes:

(1) Move the current 160bit constants from mercurial.node into a
subclass. Instead of a global constant, derive the correct constant from
the repo/revlog/... instance and pass it down as necessary. The API
change itself is in D9750. The expectation for this step is that a
repository has one hash size and one set of magic values, but it doesn't
change anything regarding the hash function itself. A follow-up change
is necessary to replace the global constants (approximately D9465 minus
D9750).

(2) Adjust various on-disk formats to switch between the current 160bit
and 256bit encoding based on the node constants in use. This would be a
non-functional change for existing repositories.

(3) Introduce the tagged 256(*) hash function. My plan here is to use
Blake2b configured for 248bit output and a suffix of b'\x01'. It is a
bit wasteful to reserve 8bit for the tag, but simplifies code. Biggest
downside is that the full Blake2b support is not available in Python 2.

The tag would allow different hash functions to co-exist and embed
existing SHA1 hashes by zero padding.

(4) Adjust hash verification logic to derive the hash function from the
tag of a node, not just hard-coding it.

At the end of step 4, most repositories can be converted in a mostly
transparent way. Some additional changes might be necessary for allowing
"short" node ids for things like .hgtags, but overall, existing hashes
should just continue to work as before.

Joerg
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9994: error: remove shortening of node in error message

[pulkit (Pulkit Goyal)]

pulkit created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This makes it difficult to figure what exactly was node on which LookupError 
was
  called if we only have the error message.
  
  I was debugging an error message which contained a short node and I tried to
  find out which callers raise LookupError with a short node. This turned out to
  be wrong because we short-ed the node before printing.
  
  If a short node is to be displayed, that should be done by callers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/error.py
  tests/test-bundle-r.t
  tests/test-bundle.t
  tests/test-convert-filemap.t
  tests/test-convert-hg-source.t
  tests/test-hgweb-filelog.t
  tests/test-hgweb.t
  tests/test-hgwebdir.t
  tests/test-narrow-exchange.t
  tests/test-pull-bundle.t

CHANGE DETAILS

diff --git a/tests/test-pull-bundle.t b/tests/test-pull-bundle.t
--- a/tests/test-pull-bundle.t
+++ b/tests/test-pull-bundle.t
@@ -185,7 +185,7 @@
   adding changesets
   adding manifests
   adding file changes
-  abort: 00changelog.i@66f7d451a68b: no node
+  abort: 00changelog.i@66f7d451a68b85ed82ff5fcc254daf50c74144bd: no node
   [50]
   $ cd ..
   $ killdaemons.py
diff --git a/tests/test-narrow-exchange.t b/tests/test-narrow-exchange.t
--- a/tests/test-narrow-exchange.t
+++ b/tests/test-narrow-exchange.t
@@ -105,7 +105,7 @@
   remote: adding file changes
   remote: transaction abort!
   remote: rollback completed
-  remote: abort: data/inside2/f.i@4a1aa07735e6: unknown parent 
(reporevlogstore !)
+  remote: abort: data/inside2/f.i@4a1aa07735e673e20c00fae80f40dc301ee30616: 
unknown parent (reporevlogstore !)
   remote: abort: data/inside2/f/index@4a1aa07735e6: no node (reposimplestore !)
   abort: stream ended unexpectedly (got 0 bytes, expected 4)
   [255]
@@ -218,8 +218,8 @@
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 0 changes to 0 files (no-lfs-on !)
-  remote: error: pretxnchangegroup.lfs hook raised an exception: 
data/inside2/f.i@f59b4e021835: no match found (lfs-on !)
+  remote: error: pretxnchangegroup.lfs hook raised an exception: 
data/inside2/f.i@f59b4e0218355383d2789196f1092abcf2262b0c: no match found 
(lfs-on !)
   remote: transaction abort! (lfs-on !)
   remote: rollback completed (lfs-on !)
-  remote: abort: data/inside2/f.i@f59b4e021835: no match found (lfs-on !)
+  remote: abort: data/inside2/f.i@f59b4e0218355383d2789196f1092abcf2262b0c: no 
match found (lfs-on !)
   abort: stream ended unexpectedly (got 0 bytes, expected 4) (lfs-on !)
diff --git a/tests/test-hgwebdir.t b/tests/test-hgwebdir.t
--- a/tests/test-hgwebdir.t
+++ b/tests/test-hgwebdir.t
@@ -103,7 +103,7 @@
   404 Not Found
   
   
-  error: bork@8580ff50825a: not found in manifest
+  error: bork@8580ff50825a50c8f716709acdf8de0deddcd6ab: not found in manifest
   [1]
 
 should succeed
diff --git a/tests/test-hgweb.t b/tests/test-hgweb.t
--- a/tests/test-hgweb.t
+++ b/tests/test-hgweb.t
@@ -149,7 +149,7 @@
   404 Not Found
   
   
-  error: bork@2ef0ac749a14: not found in manifest
+  error: bork@2ef0ac749a14e4f57a5a822464a0902c6f7f448f: not found in manifest
   [1]
   $ get-with-headers.py localhost:$HGPORT 'file/tip/bork'
   404 Not Found
@@ -202,7 +202,7 @@
   An error occurred while processing your request:
   
   
-  bork@2ef0ac749a14: not found in manifest
+  bork@2ef0ac749a14e4f57a5a822464a0902c6f7f448f: not found in manifest
   
   
   
@@ -218,7 +218,7 @@
   404 Not Found
   
   
-  error: bork@2ef0ac749a14: not found in manifest
+  error: bork@2ef0ac749a14e4f57a5a822464a0902c6f7f448f: not found in manifest
   [1]
 
 try bad style
diff --git a/tests/test-hgweb-filelog.t b/tests/test-hgweb-filelog.t
--- a/tests/test-hgweb-filelog.t
+++ b/tests/test-hgweb-filelog.t
@@ -656,7 +656,7 @@
   An error occurred while processing your request:
   
   
-  a@6563da9dcf87: not found in manifest
+  a@6563da9dcf87b1949716e38ff3e3dfaa3198eb06: not found in manifest
   
   
   
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -182,7 +182,7 @@
   sorting...
   converting...
   4 init
-  ignoring: data/b.i@1e88685f5dde: no match found (reporevlogstore !)
+  ignoring: data/b.i@1e88685f5ddec574a34c70af492f95b6debc8741: no match found 
(reporevlogstore !)
   ignoring: data/b/index@1e88685f5dde: no node (reposimplestore !)
   3 changeall
   2 changebagain
diff --git a/tests/test-convert-filemap.t b/tests/test-convert-filemap.t
--- a/tests/test-convert-filemap.t
+++ b/tests/test-convert-filemap.t
@@ -292,12 +292,12 @@
   $ rm -rf source/.hg/store/data/dir/file4
 #endif
   $ hg -q convert --filemap renames.fmap --datesort source dummydest
-  abort: data/dir/file3.i@e96dce0bc6a2: no match found (reporevlogstore !)
+  abort: 

mercurial-devel | Pipeline #17937 has failed for branch/default | a3d0182b

[Heptapod]

Your pipeline has failed.

Project: mercurial-devel ( https://foss.heptapod.net/octobus/mercurial-devel )
Branch: branch/default ( 
https://foss.heptapod.net/octobus/mercurial-devel/-/commits/branch/default )

Commit: a3d0182b ( 
https://foss.heptapod.net/octobus/mercurial-devel/-/commit/a3d0182b7dec9b3060f828019f306346773cd380
 )
Commit Message: packaging: add Provides: python3-mercurial and ...
Commit Author: Kyle Lippincott ( https://foss.heptapod.net/spectral )

Pipeline #17937 ( 
https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/17937 ) triggered 
by Pulkit Goyal ( https://foss.heptapod.net/pulkit.goyal )
had 1 failed build.

Job #166591 ( 
https://foss.heptapod.net/octobus/mercurial-devel/-/jobs/166591/raw )

Stage: tests
Name: test-py3

-- 
You're receiving this email because of your account on foss.heptapod.net.



___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9993: sidedata: move to new sidedata storage in revlogv2

[Raphaël Gomès]

Alphare created this revision.
Herald added a reviewer: indygreg.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The current (experimental) sidedata system uses flagprocessors to signify the
  presence and store/retrieve sidedata from the raw revlog data. This proved to 
be
  quite fragile from an exchange perspective and a lot more complex than simply
  having a dedicated space in the new revlog format.
  
  This change does not handle exchange (ironically), so the test for amend - 
that
  uses a bundle - is broken. This functionality is split into the next patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/lfs/wrapper.py
  hgext/remotefilelog/remotefilelog.py
  mercurial/revlog.py
  mercurial/revlogutils/flagutil.py
  mercurial/revlogutils/sidedata.py
  tests/flagprocessorext.py
  tests/simplestorerepo.py
  tests/test-copies-in-changeset.t
  tests/test-revlog-raw.py
  tests/testlib/ext-sidedata.py

CHANGE DETAILS

diff --git a/tests/testlib/ext-sidedata.py b/tests/testlib/ext-sidedata.py
--- a/tests/testlib/ext-sidedata.py
+++ b/tests/testlib/ext-sidedata.py
@@ -40,19 +40,20 @@
 return orig(self, text, transaction, link, p1, p2, *args, **kwargs)
 
 
-def wraprevision(orig, self, nodeorrev, *args, **kwargs):
-text = orig(self, nodeorrev, *args, **kwargs)
+def wrap_revisiondata(orig, self, nodeorrev, *args, **kwargs):
+text, sd = orig(self, nodeorrev, *args, **kwargs)
 if getattr(self, 'sidedatanocheck', False):
-return text
+return text, sd
+if self.version & 0x != 2:
+return text, sd
 if nodeorrev != nullrev and nodeorrev != nullid:
-sd = self.sidedata(nodeorrev)
 if len(text) != struct.unpack('>I', sd[sidedata.SD_TEST1])[0]:
 raise RuntimeError('text size mismatch')
 expected = sd[sidedata.SD_TEST2]
 got = hashlib.sha256(text).digest()
 if got != expected:
 raise RuntimeError('sha256 mismatch')
-return text
+return text, sd
 
 
 def wrapgetsidedatacompanion(orig, srcrepo, dstrepo):
@@ -81,7 +82,7 @@
 
 def extsetup(ui):
 extensions.wrapfunction(revlog.revlog, 'addrevision', wrapaddrevision)
-extensions.wrapfunction(revlog.revlog, 'revision', wraprevision)
+extensions.wrapfunction(revlog.revlog, '_revisiondata', wrap_revisiondata)
 extensions.wrapfunction(
 upgrade_engine, 'getsidedatacompanion', wrapgetsidedatacompanion
 )
diff --git a/tests/test-revlog-raw.py b/tests/test-revlog-raw.py
--- a/tests/test-revlog-raw.py
+++ b/tests/test-revlog-raw.py
@@ -51,10 +51,10 @@
 def readprocessor(self, rawtext):
 # True: the returned text could be used to verify hash
 text = rawtext[len(_extheader) :].replace(b'i', b'1')
-return text, True, {}
+return text, True
 
 
-def writeprocessor(self, text, sidedata):
+def writeprocessor(self, text):
 # False: the returned rawtext shouldn't be used to verify hash
 rawtext = _extheader + text.replace(b'1', b'i')
 return rawtext, False
@@ -293,7 +293,7 @@
 
 # Verify text, rawtext, and rawsize
 if isext:
-rawtext = writeprocessor(None, text, {})[0]
+rawtext = writeprocessor(None, text)[0]
 else:
 rawtext = text
 if rlog.rawsize(rev) != len(rawtext):
diff --git a/tests/test-copies-in-changeset.t b/tests/test-copies-in-changeset.t
--- a/tests/test-copies-in-changeset.t
+++ b/tests/test-copies-in-changeset.t
@@ -273,12 +273,13 @@
   $ hg ci --amend -m 'copy a to j, v2'
   saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-*-amend.hg (glob)
   $ hg debugsidedata -c -v -- -1
-  1 sidedata entries
-   entry-0014 size 24
-
'\x00\x00\x00\x02\x00\x00\x00\x00\x01\x00\x00\x00\x00\x06\x00\x00\x00\x02\x00\x00\x00\x00aj'
+  1 sidedata entries (missing-correct-output !)
+   entry-0014 size 24 (missing-correct-output !)
+
'\x00\x00\x00\x02\x00\x00\x00\x00\x01\x00\x00\x00\x00\x06\x00\x00\x00\x02\x00\x00\x00\x00aj'
 (missing-correct-output !)
 #endif
   $ hg showcopies --config experimental.copies.read-from=filelog-only
-  a -> j
+  a -> j (sidedata missing-correct-output !)
+  a -> j (no-sidedata !)
 The entries should be written to extras even if they're empty (so the client
 won't have to fall back to reading from filelogs)
   $ echo x >> j
@@ -356,7 +357,8 @@
   saved backup bundle to $TESTTMP/rebase-rename/.hg/strip-backup/*-*-rebase.hg 
(glob)
   $ hg st --change . --copies
   A b
-a
+a (sidedata missing-correct-output !)
+a (no-sidedata !)
   R a
   $ cd ..
 
diff --git a/tests/simplestorerepo.py b/tests/simplestorerepo.py
--- a/tests/simplestorerepo.py
+++ b/tests/simplestorerepo.py
@@ -300,7 +300,7 @@
 text = rawtext
 else:
 r = flagutil.processflagsread(self, rawtext, flags)
-text, validatehash, sidedata = r
+text, 

D9992: upgrade: speed up when we have only nodemap to downgrade

[pulkit (Pulkit Goyal)]

pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Similar to what we do on upgrade, if we have only persistent-nodemap to
  downgrade we will just delete the nodemap files and update repository
  requirements instead of processing all the revlogs.
  
  After downgrade, we are left with unrequired docket and transaction files 
which
  seems fine but can work on deleting them if someone feels we should.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/revlogutils/nodemap.py
  mercurial/upgrade_utils/engine.py
  tests/test-persistent-nodemap.t

CHANGE DETAILS

diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -581,7 +581,7 @@
   plain-cl-delta: yesyes yes
   compression:zlib   zlibzlib
   compression-level:  default default default
-  $ hg debugupgraderepo --run --no-backup --quiet
+  $ hg debugupgraderepo --run --no-backup
   upgrade will perform the following actions:
   
   requirements
@@ -593,8 +593,17 @@
 - changelog
 - manifest
   
+  beginning upgrade...
+  repository locked and read-only
+  creating temporary repository to stage upgraded data: 
$TESTTMP/test-repo/.hg/upgrade.* (glob)
+  (it is safe to interrupt this process any time before data migration 
completes)
+  downgrading repository to not use persistent nodemap feature
+  removing temporary repository $TESTTMP/test-repo/.hg/upgrade.* (glob)
   $ ls -1 .hg/store/ | egrep '00(changelog|manifest)(\.n|-.*\.nd)'
-  [1]
+  00changelog-*.nd (glob)
+  00manifest-*.nd (glob)
+  undo.backup.00changelog.n
+  undo.backup.00manifest.n
   $ hg debugnodemap --metadata
 
 
@@ -643,6 +652,8 @@
   00changelog.n
   00manifest-*.nd (glob)
   00manifest.n
+  undo.backup.00changelog.n
+  undo.backup.00manifest.n
 
   $ hg debugnodemap --metadata
   uid: * (glob)
diff --git a/mercurial/upgrade_utils/engine.py 
b/mercurial/upgrade_utils/engine.py
--- a/mercurial/upgrade_utils/engine.py
+++ b/mercurial/upgrade_utils/engine.py
@@ -476,6 +476,27 @@
 tr, unfi.manifestlog._rootstore._revlog, force=True
 )
 scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements)
+elif (
+len(upgrade_op.removed_actions) == 1
+and [
+x
+for x in upgrade_op.removed_actions
+if x.name == b'persistent-nodemap'
+]
+and not upgrade_op.upgrade_actions
+):
+ui.status(
+_(b'downgrading repository to not use persistent nodemap 
feature\n')
+)
+with srcrepo.transaction(b'upgrade') as tr:
+unfi = srcrepo.unfiltered()
+cl = unfi.changelog
+nodemap.delete_nodemap(tr, srcrepo, cl)
+# check comment 20 lines above for accessing private attributes
+nodemap.delete_nodemap(
+tr, srcrepo, unfi.manifestlog._rootstore._revlog
+)
+scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements)
 else:
 with dstrepo.transaction(b'upgrade') as tr:
 _clonerevlogs(
diff --git a/mercurial/revlogutils/nodemap.py b/mercurial/revlogutils/nodemap.py
--- a/mercurial/revlogutils/nodemap.py
+++ b/mercurial/revlogutils/nodemap.py
@@ -128,6 +128,14 @@
 notr._postclose[k](None)
 
 
+def delete_nodemap(tr, repo, revlog):
+""" Delete nodemap data on disk for a given revlog"""
+if revlog.nodemap_file is None:
+msg = "calling persist nodemap on a revlog without the feature enabled"
+raise error.ProgrammingError(msg)
+repo.svfs.unlink(revlog.nodemap_file)
+
+
 def persist_nodemap(tr, revlog, pending=False, force=False):
 """Write nodemap data on disk for a given revlog"""
 if getattr(revlog, 'filteredrevs', ()):



To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D9991: upgrade: write nodemap for manifests too

[pulkit (Pulkit Goyal)]

pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  In 98e39f04d60e 
 I 
assumed that writing nodemap for manifests was not desirable
  and stopped writing it during upgrade. However in recent discussion with
  Pierre-Yves, I learnt that that's not true.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/upgrade_utils/engine.py
  tests/test-persistent-nodemap.t

CHANGE DETAILS

diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -641,6 +641,8 @@
   $ ls -1 .hg/store/ | egrep '00(changelog|manifest)(\.n|-.*\.nd)'
   00changelog-*.nd (glob)
   00changelog.n
+  00manifest-*.nd (glob)
+  00manifest.n
 
   $ hg debugnodemap --metadata
   uid: * (glob)
diff --git a/mercurial/upgrade_utils/engine.py 
b/mercurial/upgrade_utils/engine.py
--- a/mercurial/upgrade_utils/engine.py
+++ b/mercurial/upgrade_utils/engine.py
@@ -468,6 +468,13 @@
 unfi = srcrepo.unfiltered()
 cl = unfi.changelog
 nodemap.persist_nodemap(tr, cl, force=True)
+# we want to directly operate on the underlying revlog to force
+# create a nodemap file. This is fine since this is upgrade code
+# and it heavily relies on repository being revlog based
+# hence accessing private attributes can be justified
+nodemap.persist_nodemap(
+tr, unfi.manifestlog._rootstore._revlog, force=True
+)
 scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements)
 else:
 with dstrepo.transaction(b'upgrade') as tr:



To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel