Patchwork D10608: revlog: open files in 'r+' instead of 'a+'

login
register
mail settings
Submitter phabricator
Date May 3, 2021, 12:07 p.m.
Message ID <differential-rev-PHID-DREV-2jd2lgv3rujoules2ba3-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48926/
State New
Headers show

Comments

phabricator - May 3, 2021, 12:07 p.m.
marmoute created this revision.
Herald added a reviewer: indygreg.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The code doing actual writing is already doing the necessary seeking, so we
  could safely use 'r+'. This make the file objecs usable in more situation, like
  updating the sidedata information during pull.
  
  revlog: forcibly move the file cursor at the right location before writing
  
  This is a paranoid change in case the changelog computation moved the cursors
  under our feets.
  
  This is not known to happens right now.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/revlog.py
  mercurial/store.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/store.py b/mercurial/store.py
--- a/mercurial/store.py
+++ b/mercurial/store.py
@@ -706,7 +706,7 @@ 
             # do not trigger a fncache load when adding a file that already is
             # known to exist.
             notload = self.fncache.entries is None and self.vfs.exists(encoded)
-            if notload and b'a' in mode and not self.vfs.stat(encoded).st_size:
+            if notload and b'r+' in mode and not self.vfs.stat(encoded).st_size:
                 # when appending to an existing file, if the file has size zero,
                 # it should be considered as missing. Such zero-size files are
                 # the result of truncation when a transaction is aborted.
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -2016,7 +2016,7 @@ 
 
             if existing_handles:
                 # switched from inline to conventional reopen the index
-                ifh = self._indexfp(b"a+")
+                ifh = self._indexfp(b"r+")
                 self._writinghandles = (ifh, new_dfh)
                 new_dfh = None
         finally:
@@ -2037,11 +2037,23 @@ 
                 dsize = self.end(r - 1)
             dfh = None
             if not self._inline:
-                dfh = self._datafp(b"a+")
+                try:
+                    dfh = self._datafp(b"r+")
+                    dfh.seek(0, os.SEEK_END)
+                except IOError as inst:
+                    if inst.errno != errno.ENOENT:
+                        raise
+                    dfh = self._datafp(b"w+")
                 transaction.add(self._datafile, dsize)
             try:
                 isize = r * self.index.entry_size
-                ifh = self._indexfp(b"a+")
+                try:
+                    ifh = self._indexfp(b"r+")
+                    ifh.seek(0, os.SEEK_END)
+                except IOError as inst:
+                    if inst.errno != errno.ENOENT:
+                        raise
+                    ifh = self._indexfp(b"w+")
                 if self._inline:
                     transaction.add(self._indexfile, dsize + isize)
                 else:
@@ -3174,6 +3186,8 @@ 
                 entry = (new_offset_flags,) + entry[1:8]
                 entry += (current_offset, len(serialized_sidedata))
 
+                # the sidedata computation might have move the file cursors around
+                dfh.seek(current_offset, os.SEEK_SET)
                 dfh.write(serialized_sidedata)
                 new_entries.append(entry)
                 current_offset += len(serialized_sidedata)