durin42 created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers.
REVISION SUMMARY Consider the program: #include <stdio.h> int main() { FILE *f = fopen("narf", "w"); fprintf(f, "narf\n"); fclose(f); f = fopen("narf", "a"); printf("%ld\n", ftell(f)); fprintf(f, "troz\n"); printf("%ld\n", ftell(f)); return 0; } on macOS, FreeBSD, and Linux with glibc, this program prints 5 10 but on musl libc (Alpine Linux and probably others) this prints 0 10 By my reading of https://pubs.opengroup.org/onlinepubs/009695399/functions/fopen.html this is technically correct, specifically: > Opening a file with append mode (a as the first character in the > mode argument) shall cause all subsequent writes to the file to be > forced to the then current end-of-file, regardless of intervening > calls to fseek(). in other words, the file position doesn't really matter in append-mode files, and we can't depend on it being at all meaningful unless we perform a seek() before tell() after open(..., 'a'). Experimentally after a .write() we can do a .tell() and it'll always be reasonable, but I'm unclear from reading the specification if that's a smart thing to rely on. These callsites were identified by applying the next changeset and then fixing new crashes. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6554 AFFECTED FILES mercurial/branchmap.py mercurial/obsolete.py mercurial/tags.py CHANGE DETAILS diff --git a/mercurial/tags.py b/mercurial/tags.py --- a/mercurial/tags.py +++ b/mercurial/tags.py @@ -13,6 +13,7 @@ from __future__ import absolute_import import errno +import io from .node import ( bin, @@ -584,6 +585,7 @@ fp = repo.vfs('localtags', 'r+') except IOError: fp = repo.vfs('localtags', 'a') + fp.seek(0, io.SEEK_END) else: prevtags = fp.read() @@ -599,6 +601,7 @@ if e.errno != errno.ENOENT: raise fp = repo.wvfs('.hgtags', 'ab') + fp.seek(0, io.SEEK_END) else: prevtags = fp.read() @@ -793,6 +796,7 @@ try: f = repo.cachevfs.open(_fnodescachefile, 'ab') + f.seek(0, io.SEEK_END) try: # if the file has been truncated actualoffset = f.tell() diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py --- a/mercurial/obsolete.py +++ b/mercurial/obsolete.py @@ -71,6 +71,7 @@ import errno import hashlib +import io import struct from .i18n import _ @@ -625,6 +626,7 @@ new.append(m) if new: f = self.svfs('obsstore', 'ab') + f.seek(0, io.SEEK_END) try: offset = f.tell() transaction.add('obsstore', offset) diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -7,6 +7,7 @@ from __future__ import absolute_import +import io import struct from .node import ( @@ -640,6 +641,7 @@ """ write the new branch names to revbranchcache """ if self._rbcnamescount != 0: f = repo.cachevfs.open(_rbcnames, 'ab') + f.seek(0, io.SEEK_END) if f.tell() == self._rbcsnameslen: f.write('\0') else: @@ -661,6 +663,7 @@ """ write the new revs to revbranchcache """ revs = min(len(repo.changelog), len(self._rbcrevs) // _rbcrecsize) with repo.cachevfs.open(_rbcrevs, 'ab') as f: + f.seek(0, io.SEEK_END) if f.tell() != start: repo.ui.debug("truncating cache/%s to %d\n" % (_rbcrevs, start)) f.seek(start) To: durin42, #hg-reviewers Cc: mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel