Patchwork D12195: obsolete: don't use os.stat in repo.obsstore.__nonzero__ if it's static HTTP

login
register
mail settings
Submitter phabricator
Date Feb. 16, 2022, 6:22 a.m.
Message ID <differential-rev-PHID-DREV-time5ukvzcwzo5w5kkek-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50532/
State New
Headers show

Comments

phabricator - Feb. 16, 2022, 6:22 a.m.
av6 created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  If a repo is accessed via static HTTP, then we obviously can't use os.stat() to
  just peek at the file size. Let's download the entire file to check its size.
  Yes, this feels wasteful, but:
  
  1. If we're cloning or pulling a repo from a static HTTP server, we need the contents of the obsstore anyway.
  
  2. Implementing statichttpvfs.stat() that uses HEAD will result in one more request to a static-only HTTP server, which is already slow. Also parsing a response to a HEAD request to construct os.stat_result is pretty hacky. There's also a question of the remote server properly supporting HEAD method and reporting at least file size.
  
  3. Implementing statichttpvfs.stat() that uses GET is pretty much the same thing as we do here, except we can't even cache the response easily, unlike simply accessing obsstore._data, which is @propertycache'd.
  
  Importing statichttprepo locally to avoid circular import.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/obsolete.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -575,11 +575,16 @@ 
         return len(self._all)
 
     def __nonzero__(self):
+        from . import statichttprepo
+        if isinstance(self.repo, statichttprepo.statichttprepository):
+            # If repo is accessed via static HTTP, then we can't use os.stat()
+            # to just peek at the file size.
+            return self._data > 1
         if not self._cached('_all'):
             try:
                 return self.svfs.stat(b'obsstore').st_size > 1
             except OSError as inst:
-                if inst.errno not in (errno.ENOENT, errno.EINVAL):
+                if inst.errno != errno.ENOENT:
                     raise
                 # just build an empty _all list if no obsstore exists, which
                 # avoids further stat() syscalls