# HG changeset patch # User Gregory Szorc # Date 1537226952 25200 # Node ID 3d22aef3ecd5fe02c9411503b04dfbdcb6240cc1 # Parent 4024c363cd33a473e27482ff91ab763787ca4258 transaction: make entries a private attribute (API) This attribute is tracking changes to append-only files. It is an implementation detail and should not be exposed as part of the public interface. But code in repair was accessing it, so it seemingly does belong as part of the public API. But that code in repair is making assumptions about how storage works and is grossly wrong when alternate storage backends are in play. We'll need some kind of "strip" API at the storage layer that knows how to handle things in a storage-agnostic manner. I don't think accessing a private attribute on the transaction is any worse than what this code is already doing. So I'm fine with violating the abstraction for transactions. And with this change, all per-instance attributes on transaction have been made private except for "changes" and "hookargs." Both are used by multiple consumers and look like they need to be part of the public interface. .. api:: Various attributes of ``transaction.transaction`` are now ``_`` prefixed to indicate they shouldn't be used by external consumers. Differential Revision: https://phab.mercurial-scm.org/D4634 diff -r 4024c363cd33 -r 3d22aef3ecd5 mercurial/repair.py --- a/mercurial/repair.py Mon Sep 17 16:19:55 2018 -0700 +++ b/mercurial/repair.py Mon Sep 17 16:29:12 2018 -0700 @@ -190,7 +190,11 @@ with ui.uninterruptable(): try: with repo.transaction("strip") as tr: - offset = len(tr.entries) + # TODO this code violates the interface abstraction of the + # transaction and makes assumptions that file storage is + # using append-only files. We'll need some kind of storage + # API to handle stripping for us. + offset = len(tr._entries) tr.startgroup() cl.strip(striprev, tr) @@ -200,8 +204,8 @@ repo.file(fn).strip(striprev, tr) tr.endgroup() - for i in pycompat.xrange(offset, len(tr.entries)): - file, troffset, ignore = tr.entries[i] + for i in pycompat.xrange(offset, len(tr._entries)): + file, troffset, ignore = tr._entries[i] with repo.svfs(file, 'a', checkambig=True) as fp: fp.truncate(troffset) if troffset == 0: diff -r 4024c363cd33 -r 3d22aef3ecd5 mercurial/transaction.py --- a/mercurial/transaction.py Mon Sep 17 16:19:55 2018 -0700 +++ b/mercurial/transaction.py Mon Sep 17 16:29:12 2018 -0700 @@ -129,7 +129,7 @@ vfsmap[''] = opener # set default value self._vfsmap = vfsmap self._after = after - self.entries = [] + self._entries = [] self._map = {} self._journal = journalname self._undoname = undoname @@ -230,8 +230,8 @@ """add a append-only entry to memory and on-disk state""" if file in self._map or file in self._backupmap: return - self.entries.append((file, offset, data)) - self._map[file] = len(self.entries) - 1 + self._entries.append((file, offset, data)) + self._map[file] = len(self._entries) - 1 # add enough data to the journal to do the truncate self._file.write("%s\0%d\n" % (file, offset)) self._file.flush() @@ -352,7 +352,7 @@ @active def find(self, file): if file in self._map: - return self.entries[self._map[file]] + return self._entries[self._map[file]] if file in self._backupmap: return self._backupentries[self._backupmap[file]] return None @@ -367,7 +367,7 @@ if file not in self._map: raise KeyError(file) index = self._map[file] - self.entries[index] = (file, offset, data) + self._entries[index] = (file, offset, data) self._file.write("%s\0%d\n" % (file, offset)) self._file.flush() @@ -486,7 +486,7 @@ # Abort may be raise by read only opener self._report("couldn't remove %s: %s\n" % (vfs.join(b), inst)) - self.entries = [] + self._entries = [] self._writeundo() if self._after: self._after() @@ -564,7 +564,7 @@ self._backupsfile.close() try: - if not self.entries and not self._backupentries: + if not self._entries and not self._backupentries: if self._backupjournal: self._opener.unlink(self._backupjournal) if self._journal: @@ -579,7 +579,7 @@ # Prevent double usage and help clear cycles. self._abortcallback = None _playback(self._journal, self._report, self._opener, - self._vfsmap, self.entries, self._backupentries, + self._vfsmap, self._entries, self._backupentries, False, checkambigfiles=self._checkambigfiles) self._report(_("rollback completed\n")) except BaseException: