Patchwork [2,of,5] transaction: use "locations" instead of "vfs" objects for file generation

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 18, 2014, 11:44 a.m.
Message ID <580364627689487b0c13.1413632694@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6399/
State Deferred
Headers show

Comments

Pierre-Yves David - Oct. 18, 2014, 11:44 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1413604422 25200
#      Fri Oct 17 20:53:42 2014 -0700
# Node ID 580364627689487b0c13185a28d925f676fa0526
# Parent  c5f48e3801a9393a0a318b1b01effba1dc110ea1
transaction: use "locations" instead of "vfs" objects for file generation

The argument is now a location name. The location must be present in the
`vfsmap` provided to the transaction at creation time. We need to use a textual
value for storage in the on-disk transaction state file. Such persistence is
implemented in a later changeset.

I'm not very happy to have the default value ("store") explicitly as the default
argument since it feel like a layer violation. However, it seems to be a lesser
evil compared to other solutions I could think of.

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -50,11 +50,11 @@  class bmstore(dict):
     def recordchange(self, tr):
         """record that bookmarks have been changed in a transaction
 
         The transaction is then responsible for updating the file content."""
         tr.addfilegenerator('bookmarks', ('bookmarks',), self._write,
-                            vfs=self._repo.vfs)
+                            location='plain')
         tr.hookargs['bookmark_moved'] = '1'
 
     def write(self):
         '''Write bookmarks
 
diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -179,11 +179,12 @@  class transaction(object):
         self.backupmap[file] = len(self.backupentries) - 1
         self.backupsfile.write("%s\0%s\0" % (file, backupfile))
         self.backupsfile.flush()
 
     @active
-    def addfilegenerator(self, genid, filenames, genfunc, order=0, vfs=None):
+    def addfilegenerator(self, genid, filenames, genfunc, order=0,
+                         location='store'):
         """add a function to generates some files at transaction commit
 
         The `genfunc` argument is a function capable of generating proper
         content of each entry in the `filename` tuple.
 
@@ -200,12 +201,12 @@  class transaction(object):
         The `order` argument may be used to control the order in which multiple
         generator will be executed.
         """
         # For now, we are unable to do proper backup and restore of custom vfs
         # but for bookmarks that are handled outside this mechanism.
-        assert vfs is None or filenames == ('bookmarks',)
-        self._filegenerators[genid] = (order, filenames, genfunc, vfs)
+        assert location == 'store' or filenames == ('bookmarks',)
+        self._filegenerators[genid] = (order, filenames, genfunc, location)
 
     @active
     def find(self, file):
         if file in self.map:
             return self.entries[self.map[file]]
@@ -246,13 +247,12 @@  class transaction(object):
     @active
     def close(self):
         '''commit the transaction'''
         # write files registered for generation
         for entry in sorted(self._filegenerators.values()):
-            order, filenames, genfunc, vfs = entry
-            if vfs is None:
-                vfs = self.opener
+            order, filenames, genfunc, location = entry
+            vfs = self._vfsmap[location]
             files = []
             try:
                 for name in filenames:
                     # Some files are already backed up when creating the
                     # localrepo. Until this is properly fixed we disable the