Page MenuHomePhabricator

upgrade: don't perform anything if nothing to do
ClosedPublic

Authored by pulkit on Jan 8 2021, 1:45 PM.

Details

Summary

Before this patch, upgrade will process everything, re-clone all revlogs, write
requirements file again even there is no change to be made.

This patch makes it exit earlier.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Jan 8 2021, 1:45 PM
mharbison72 requested changes to this revision.Jan 12 2021, 8:48 PM
mharbison72 added a subscriber: mharbison72.

I get test failures with this:

$ ./run-tests.py --local test-upgrade-repo.t
running 1 tests using 1 parallel processes

--- /mnt/c/Users/Matt/hg/tests/test-upgrade-repo.t
+++ /mnt/c/Users/Matt/hg/tests/test-upgrade-repo.t.err
@@ -1317,17 +1317,6 @@
 downgrade

   $ hg debugupgraderepo --run --no-backup --quiet
-  upgrade will perform the following actions:
-
-  requirements
-     preserved: dotencode, fncache, generaldelta, revlogv1, sparserevlog, store
-     removed: revlog-compression-zstd
-
-  processed revlogs:
-    - all-filelogs
-    - changelog
-    - manifest
-
   $ hg debugformat -v
   format-variant     repo config default
   fncache:            yes    yes     yes
@@ -1339,12 +1328,13 @@
   persistent-nodemap:  no     no      no
   copies-sdc:          no     no      no
   plain-cl-delta:     yes    yes     yes
-  compression:        zlib   zlib    zlib
+  compression:        zstd   zlib    zlib
   compression-level:  default default default
   $ cat .hg/requires
   dotencode
   fncache
   generaldelta
+  revlog-compression-zstd
   revlogv1
   sparserevlog
   store
@@ -1356,17 +1346,6 @@
   > revlog-compression=zstd
   > EOF
   $ hg debugupgraderepo --run --no-backup --quiet
-  upgrade will perform the following actions:
-
-  requirements
-     preserved: dotencode, fncache, generaldelta, revlogv1, sparserevlog, store
-     added: revlog-compression-zstd
-
-  processed revlogs:
-    - all-filelogs
-    - changelog
-    - manifest
-
   $ hg debugformat -v
   format-variant     repo config default
   fncache:            yes    yes     yes

ERROR: test-upgrade-repo.t output changed
!
Failed test-upgrade-repo.t: output changed
# Ran 1 tests, 0 skipped, 1 failed.
python hash seed: 3820465322

I have no idea why the compression difference here. Something else to be aware of (not related to this commit) is that test-upgrade-repo.t is extremely flaky on Windows. If I run it by itself, it usually fails in a rename and then has cascading errors. If I run it with test-check-* in the list, it may or may not fail the same way. For example:

--- c:/Users/Matt/hg/tests/test-upgrade-repo.t
+++ c:/Users/Matt/hg/tests/test-upgrade-repo.t.err
@@ -944,75 +944,69 @@
   starting in-place swap of repository data
   replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
-  store replacement complete; repository was inconsistent for *s (glob)
-  finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
-  removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
+  removing temporary repository $TESTTMP\upgradegd\.hg\upgrade.qzp1er
+  Traceback (most recent call last):
+    File "c:\Users\Matt\hg\mercurial\scmutil.py", line 155, in callcatch
+      return func()
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 455, in _runcatchfunc
+      return _dispatch(req)
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1252, in _dispatch
+      lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 906, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1263, in _runcommand
+      return cmdfunc()
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1249, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "c:\Users\Matt\hg\mercurial\util.py", line 1867, in check
+      return func(*args, **kwargs)
+    File "c:\Users\Matt\hg\mercurial\debugcommands.py", line 3936, in debugupgraderepo
+      ui, repo, run=run, optimize=set(optimize), backup=backup, **opts
+    File "c:\Users\Matt\hg\mercurial\upgrade.py", line 206, in upgraderepo
+      ui, repo, dstrepo, upgrade_op
+    File "c:\Users\Matt\hg\mercurial\upgrade_utils\engine.py", line 512, in upgrade
+      _replacestores(srcrepo, dstrepo, backupvfs, upgrade_op)
+    File "c:\Users\Matt\hg\mercurial\upgrade_utils\engine.py", line 427, in _replacestores
+      util.rename(upgradedrepo.spath, currentrepo.spath)
+    File "c:\Users\Matt\hg\mercurial\windows.py", line 603, in rename
+      os.rename(src, dst)
+  WindowsError: [Error 5] Access is denied
+  abort: Access is denied
+  [255]
   $ hg verify
-  checking changesets
-  checking manifests
-  crosschecking files in changesets and manifests
-  checking files
-  checked 3 changesets with 3 changes to 3 files
+  abort: repository requires features unknown to this Mercurial: upgradeinprogress
+  (see https://mercurial-scm.org/wiki/MissingRequirement for more information)
+  [255]

 Check you can't skip revlog clone during important format upgrade
...
This revision now requires changes to proceed.Jan 12 2021, 8:48 PM

I get test failures with this:

$ ./run-tests.py --local test-upgrade-repo.t
running 1 tests using 1 parallel processes
--- /mnt/c/Users/Matt/hg/tests/test-upgrade-repo.t
+++ /mnt/c/Users/Matt/hg/tests/test-upgrade-repo.t.err
@@ -1317,17 +1317,6 @@
 downgrade
   $ hg debugupgraderepo --run --no-backup --quiet
-  upgrade will perform the following actions:
-
-  requirements
-     preserved: dotencode, fncache, generaldelta, revlogv1, sparserevlog, store
-     removed: revlog-compression-zstd
-
-  processed revlogs:
-    - all-filelogs
-    - changelog
-    - manifest
-
   $ hg debugformat -v
   format-variant     repo config default
   fncache:            yes    yes     yes
@@ -1339,12 +1328,13 @@
   persistent-nodemap:  no     no      no
   copies-sdc:          no     no      no
   plain-cl-delta:     yes    yes     yes
-  compression:        zlib   zlib    zlib
+  compression:        zstd   zlib    zlib
   compression-level:  default default default
   $ cat .hg/requires
   dotencode
   fncache
   generaldelta
+  revlog-compression-zstd
   revlogv1
   sparserevlog
   store
@@ -1356,17 +1346,6 @@
   > revlog-compression=zstd
   > EOF
   $ hg debugupgraderepo --run --no-backup --quiet
-  upgrade will perform the following actions:
-
-  requirements
-     preserved: dotencode, fncache, generaldelta, revlogv1, sparserevlog, store
-     added: revlog-compression-zstd
-
-  processed revlogs:
-    - all-filelogs
-    - changelog
-    - manifest
-
   $ hg debugformat -v
   format-variant     repo config default
   fncache:            yes    yes     yes
ERROR: test-upgrade-repo.t output changed
!
Failed test-upgrade-repo.t: output changed
# Ran 1 tests, 0 skipped, 1 failed.
python hash seed: 3820465322

Yeah, one of parent patch which needs change fixes this issue. Once D9693 is pushed, the failure will go away.

I have no idea why the compression difference here. Something else to be aware of (not related to this commit) is that test-upgrade-repo.t is extremely flaky on Windows. If I run it by itself, it usually fails in a rename and then has cascading errors. If I run it with test-check-* in the list, it may or may not fail the same way. For example:

--- c:/Users/Matt/hg/tests/test-upgrade-repo.t
+++ c:/Users/Matt/hg/tests/test-upgrade-repo.t.err
@@ -944,75 +944,69 @@
   starting in-place swap of repository data
   replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
-  store replacement complete; repository was inconsistent for *s (glob)
-  finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
-  removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
+  removing temporary repository $TESTTMP\upgradegd\.hg\upgrade.qzp1er
+  Traceback (most recent call last):
+    File "c:\Users\Matt\hg\mercurial\scmutil.py", line 155, in callcatch
+      return func()
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 455, in _runcatchfunc
+      return _dispatch(req)
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1252, in _dispatch
+      lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 906, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1263, in _runcommand
+      return cmdfunc()
+    File "c:\Users\Matt\hg\mercurial\dispatch.py", line 1249, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "c:\Users\Matt\hg\mercurial\util.py", line 1867, in check
+      return func(*args, **kwargs)
+    File "c:\Users\Matt\hg\mercurial\debugcommands.py", line 3936, in debugupgraderepo
+      ui, repo, run=run, optimize=set(optimize), backup=backup, **opts
+    File "c:\Users\Matt\hg\mercurial\upgrade.py", line 206, in upgraderepo
+      ui, repo, dstrepo, upgrade_op
+    File "c:\Users\Matt\hg\mercurial\upgrade_utils\engine.py", line 512, in upgrade
+      _replacestores(srcrepo, dstrepo, backupvfs, upgrade_op)
+    File "c:\Users\Matt\hg\mercurial\upgrade_utils\engine.py", line 427, in _replacestores
+      util.rename(upgradedrepo.spath, currentrepo.spath)
+    File "c:\Users\Matt\hg\mercurial\windows.py", line 603, in rename
+      os.rename(src, dst)
+  WindowsError: [Error 5] Access is denied
+  abort: Access is denied
+  [255]

Oh, this is error-ing out in final stage of upgrade where we swap data. Since I have a windows machine now, I will try to check what's going on.

$ hg verify
  • checking changesets
  • checking manifests
  • crosschecking files in changesets and manifests
  • checking files
  • checked 3 changesets with 3 changes to 3 files

+ abort: repository requires features unknown to this Mercurial: upgradeinprogress
+ (see https://mercurial-scm.org/wiki/MissingRequirement for more information)
+ [255]
Check you can't skip revlog clone during important format upgrade
...

mharbison72 accepted this revision.Jan 13 2021, 12:11 PM
This revision is now accepted and ready to land.Jan 13 2021, 12:11 PM
This revision was automatically updated to reflect the committed changes.

I mostly support this change. However, a supposed no-op upgrade may still result in differences! For example, if we change the algorithm for computing how revlog deltas are computed without actually changing the storage semantics, performing a no-op upgrade would migrate the store to use the new code.

For this reason, we probably want to have a --force option or something to override the default no-op behavior.