Cleaned up saving/sync_with_disk.
authorW. Trevor King <wking@drexel.edu>
Tue, 21 Jul 2009 11:28:26 +0000 (07:28 -0400)
committerW. Trevor King <wking@drexel.edu>
Tue, 21 Jul 2009 11:28:26 +0000 (07:28 -0400)
Got rid of a whole bunch of redundant .save() calls when
sync_with_disk==True.

Fixed up the "File-system access" portion of the BugDir docstring so
we can all remember how things are supposed to work ;).

Note that some .save() calls are still required.  For example in
becommands/merge.py, the copied comments have their .bug changed, but
that is not a versioned property, so it doesn't trigger an automatic
save, and we have to force the .save() by hand.

libbe.rcs.RCS.mkdir() is now recursive by default, but you can set
check_parents==False if you want it to fail in the case of missing
parents.  Because of the recursion, we removed the .update() call
on preexisting directories, since there will be at least one of
these occurrences for every .mkdir(check_parents=True) call, and
I don't know of any VCS that actually needs them...

Also stripped trailing whitespace from some files...

16 files changed:
becommands/comment.py
becommands/depend.py
becommands/diff.py
becommands/merge.py
becommands/new.py
becommands/open.py
becommands/remove.py
becommands/set.py
becommands/severity.py
becommands/status.py
becommands/tag.py
becommands/target.py
libbe/bug.py
libbe/bugdir.py
libbe/comment.py
libbe/rcs.py

index 66f8da17e52c56eaf241eec5330545f588cc8676..eba640e8f6626a34419bf8915e26c75b36067de5 100644 (file)
@@ -153,7 +153,6 @@ def execute(args, test=False):
         kids = [c.uuid for c in parent.traverse()]
         for nc in new_comments:
             assert nc.uuid in kids, "%s wasn't added to %s" % (nc.uuid, parent.uuid)
-    bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be comment ID [COMMENT]")
index 48e152724665915b92395f408a3d0f1b5bae3f64..4a23b0f8a2c9ee94896d6b3490903c863a07aa71 100644 (file)
@@ -58,7 +58,6 @@ def execute(args, test=False):
         else: # add the dependency
             estrs.append(depend_string)
         bugA.extra_strings = estrs # reassign to notice change
-        bugA.save()
 
     depends = []
     for estr in bugA.extra_strings:
index f3474b39a6771a2eed0d8af55f7b9dbe5bbb5515..0acfcb2f1b2b5e12751db18b05ac702a98027170 100644 (file)
@@ -24,10 +24,10 @@ def execute(args, test=False):
     """
     >>> import os
     >>> bd = bugdir.simple_bug_dir()
+    >>> bd.set_sync_with_disk(True)
     >>> original = bd.rcs.commit("Original status")
     >>> bug = bd.bug_from_uuid("a")
     >>> bug.status = "closed"
-    >>> bd.save()
     >>> changed = bd.rcs.commit("Closed bug a")
     >>> os.chdir(bd.root)
     >>> if bd.rcs.versioned == True:
index c030dd0cd0300a187d3c805a738a20201340ddd0..4aaefa85a7fd16ef83e47ec6a022eb2c45cc0252 100644 (file)
@@ -22,6 +22,7 @@ def execute(args, test=False):
     """
     >>> from libbe import utility
     >>> bd = bugdir.simple_bug_dir()
+    >>> bd.set_sync_with_disk(True)
     >>> a = bd.bug_from_shortname("a")
     >>> a.comment_root.time = 0
     >>> dummy = a.new_comment("Testing")
@@ -35,7 +36,6 @@ def execute(args, test=False):
     >>> dummy.time = 1
     >>> dummy = dummy.new_reply("1 2 3 4")
     >>> dummy.time = 2
-    >>> bd.save()
     >>> os.chdir(bd.root)
     >>> execute(["a", "b"], test=True)
     Merging bugs a and b
@@ -140,13 +140,13 @@ def execute(args, test=False):
     bugB.load_comments()
     mergeA = bugA.new_comment("Merged from bug %s" % bugB.uuid)
     newCommTree = copy.deepcopy(bugB.comment_root)
-    for comment in newCommTree.traverse():
+    for comment in newCommTree.traverse(): # all descendant comments
         comment.bug = bugA
-    for comment in newCommTree:
+        comment.save() # force onto disk under bugA
+    for comment in newCommTree: # just the child comments
         mergeA.add_reply(comment, allow_time_inversion=True)
     bugB.new_comment("Merged into bug %s" % bugA.uuid)
     bugB.status = "closed"
-    bd.save()
     print "Merging bugs %s and %s" % (bugA.uuid, bugB.uuid)
 
 def get_parser():
index 5325ccc91febc4fc851ba26b1ed89a9be6bb1297..af599d797d09c81f4ccfaba5d67c059cf1c92128 100644 (file)
@@ -58,7 +58,6 @@ def execute(args, test=False):
         bug.assigned = options.assigned
     elif bd.default_assignee != None:
         bug.assigned = bd.default_assignee
-    bd.save()
     print "Created bug with ID %s" % bd.bug_shortname(bug)
 
 def get_parser():
index b4b1025211149b21ec49572df0947b7e4d363e48..2ef5f4303aa628d58091bbc053b58e027194d6f8 100644 (file)
@@ -43,7 +43,6 @@ def execute(args, test=False):
     bd = bugdir.BugDir(from_disk=True, manipulate_encodings=not test)
     bug = bd.bug_from_shortname(args[0])
     bug.status = "open"
-    bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be open BUG-ID")
index d441bfeb4b8ecfe8b9314565abde2c5511d2dc14..d79a7bef125712497bf824370c65d85855ea88a9 100644 (file)
@@ -43,7 +43,6 @@ def execute(args, test=False):
     bd = bugdir.BugDir(from_disk=True, manipulate_encodings=not test)
     bug = bd.bug_from_shortname(args[0])
     bd.remove_bug(bug)
-    bd.save()
     print "Removed bug %s" % bug.uuid
 
 def get_parser():
index 510eca7d1239bb03358f0aca7401365952597d83..0c0862f3966bc53c3545a66ddb1a43ced49f5b5d 100644 (file)
@@ -61,7 +61,7 @@ def execute(args, test=False):
         print _value_string(bd, args[0])
     else:
         if args[1] == "none":
-            del bd.settings[args[0]]
+            setattr(bd, args[0], settings_object.EMPTY)
         else:
             if args[0] not in bd.settings_properties:
                 msg = "Invalid setting %s\n" % args[0]
@@ -70,7 +70,6 @@ def execute(args, test=False):
                 raise cmdutil.UserError(msg)
             old_setting = bd.settings.get(args[0])
             setattr(bd, args[0], args[1])
-        bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be set [NAME] [VALUE]")
index fde9fba01968a8ff48172ac31b416c087991dd3d..65467e3289bee5f0063df2744415043c797a14cd 100644 (file)
@@ -50,7 +50,6 @@ def execute(args, test=False):
             if e.name != "severity":
                 raise e
             raise cmdutil.UserError ("Invalid severity level: %s" % e.value)
-        bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be severity BUG-ID [SEVERITY]")
index 89ae49a2c75fdbb39d50af163d29e5c40cbce10f..edc948dc5820607710641cbbb6b2155195c36b2c 100644 (file)
@@ -47,7 +47,6 @@ def execute(args, test=False):
             if e.name != "status":
                 raise
             raise cmdutil.UserError ("Invalid status: %s" % e.value)
-        bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be status BUG-ID [STATUS]")
index a1395284b21ad974557731c7c32471f64276eddc..216ffbcbe00e1b50fc1995c7cb6509a9f18cf33f 100644 (file)
@@ -22,6 +22,7 @@ def execute(args, test=False):
     """
     >>> from libbe import utility
     >>> bd = bugdir.simple_bug_dir()
+    >>> bd.set_sync_with_disk(True)
     >>> os.chdir(bd.root)
     >>> a = bd.bug_from_shortname("a")
     >>> print a.extra_strings
@@ -56,7 +57,6 @@ def execute(args, test=False):
     >>> a.extra_strings = []
     >>> print a.extra_strings
     []
-    >>> a.save()
     >>> execute(["a"], test=True)
     >>> bd._clear_bugs() # resync our copy of bug
     >>> a = bd.bug_from_shortname("a")
@@ -102,7 +102,6 @@ def execute(args, test=False):
         else: # add the tag
             estrs.append(tag_string)
         bug.extra_strings = estrs # reassign to notice change
-        bd.save()
 
     tags = []
     for estr in bug.extra_strings:
index 905c639b6df764fa27c09f0c982c40de0e0c9e4b..527b16a4984a2bc989ae406f2166da24321ff538 100644 (file)
@@ -65,7 +65,6 @@ def execute(args, test=False):
             bug.target = None
         else:
             bug.target = args[1]
-        bd.save()
 
 def get_parser():
     parser = cmdutil.CmdOptionParser("be target BUG-ID [TARGET]\nor:    be target --list")
index 8be6010b89f0d2baf5db1abd248130d8dd30756c..e3dc1e0f7e8e602d5c010b981bf3d67a0846bcf2 100644 (file)
@@ -252,6 +252,11 @@ class Bug(settings_object.SavedSettingsObject):
     def __repr__(self):
         return "Bug(uuid=%r)" % self.uuid
 
+    def set_sync_with_disk(self, value):
+        self.sync_with_disk = value
+        for comment in self.comments():
+            comment.set_sync_with_disk(value)
+
     def _setting_attr_string(self, setting):
         value = getattr(self, setting)
         if value == None:
@@ -371,10 +376,17 @@ class Bug(settings_object.SavedSettingsObject):
         mapfile.map_save(self.rcs, path, self._get_saved_settings())
         
     def save(self):
+        """
+        Save any loaded contents to disk.  Because of lazy loading of
+        comments, this is actually not too inefficient.
+        
+        However, if self.sync_with_disk = True, then any changes are
+        automatically written to disk as soon as they happen, so
+        calling this method will just waste time (unless something
+        else has been messing with your on-disk files).
+        """
         self.save_settings()
-
         if len(self.comment_root) > 0:
-            self.rcs.mkdir(self.get_path("comments"))
             comment.saveComments(self)
 
     def remove(self):
index e37db81e902f8836b2b9834902e1ba4f2ae90e3e..9802502d5bcb900dc5945b32221ef280315a9e9c 100644 (file)
@@ -51,7 +51,7 @@ class NoRootEntry(Exception):
 class AlreadyInitialized(Exception):
     def __init__(self, path):
         self.path = path
-        Exception.__init__(self, 
+        Exception.__init__(self,
                            "Specified root is already initialized: %s" % path)
 
 class MultipleBugMatches(ValueError):
@@ -70,7 +70,7 @@ class BugDir (list, settings_object.SavedSettingsObject):
     """
     Sink to existing root
     ======================
-    
+
     Consider the following usage case:
     You have a bug directory rooted in
       /path/to/source
@@ -85,23 +85,35 @@ class BugDir (list, settings_object.SavedSettingsObject):
       /path/to/source/GUI/.be             miss
       /path/to/source/.be                 hit!
     So it still roots itself appropriately without much work for you.
-        
+
     File-system access
     ==================
-    
-    When rooted in non-bugdir directory, BugDirs live completely in
-    memory until the first call to .save().  This creates a '.be'
-    sub-directory containing configurations options, bugs, comments,
-    etc.  Once this sub-directory has been created (possibly by
-    another BugDir instance) any changes to the BugDir in memory will
-    be flushed to the file system automatically.  However, the BugDir
-    will only load information from the file system when it loads new
-    bugs/comments that it doesn't already have in memory, or when it
-    explicitly asked to do so (e.g. .load() or __init__(from_disk=True)).
-    
+
+    BugDirs live completely in memory when .sync_with_disk is False.
+    This is the default configuration setup by BugDir(from_disk=False).
+    If .sync_with_disk == True (e.g. BugDir(from_disk=True)), then
+    any changes to the BugDir will be immediately written to disk.
+
+    If you want to change .sync_with_disk, we suggest you use
+    .set_sync_with_disk(), which propogates the new setting through to
+    all bugs/comments/etc. that have been loaded into memory.  If
+    you've been living in memory and want to move to
+    .sync_with_disk==True, but you're not sure if anything has been
+    changed in memoryy, a call to save() is a safe move.
+
+    Regardless of .sync_with_disk, a call to .save() will write out
+    all the contents that the BugDir instance has loaded into memory.
+    If sync_with_disk has been True over the course of all interesting
+    changes, this .save() call will be a waste of time.
+
+    The BugDir will only load information from the file system when it
+    loads new bugs/comments that it doesn't already have in memory, or
+    when it explicitly asked to do so (e.g. .load() or
+    __init__(from_disk=True)).
+
     Allow RCS initialization
     ========================
-    
+
     This one is for testing purposes.  Setting it to True allows the
     BugDir to search for an installed RCS backend and initialize it in
     the root directory.  This is a convenience option for supporting
@@ -109,7 +121,7 @@ class BugDir (list, settings_object.SavedSettingsObject):
 
     Disable encoding manipulation
     =============================
-    
+
     This one is for testing purposed.  You might have non-ASCII
     Unicode in your bugs, comments, files, etc.  BugDir instances try
     and support your preferred encoding scheme (e.g. "utf-8") when
@@ -160,7 +172,7 @@ class BugDir (list, settings_object.SavedSettingsObject):
     def encoding(): return {}
 
     def _setup_user_id(self, user_id):
-        self.rcs.user_id = user_id        
+        self.rcs.user_id = user_id
     def _guess_user_id(self):
         return self.rcs.get_user_id()
     def _set_user_id(self, old_user_id, new_user_id):
@@ -215,7 +227,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             if uuid not in map:
                 map[uuid] = None
         self._bug_map_value = map # ._bug_map_value used by @local_property
-    
+
     @Property
     @primed_property(primer=_bug_map_gen)
     @local_property("bug_map")
@@ -270,7 +282,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
         # get a temporary rcs until we've loaded settings
         self.sync_with_disk = False
         self.rcs = self._guess_rcs()
-        
+
         if from_disk == True:
             self.sync_with_disk = True
             self.load()
@@ -284,6 +296,11 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             self.rcs = rcs
             self._setup_user_id(self.user_id)
 
+    def set_sync_with_disk(self, value):
+        self.sync_with_disk = value
+        for bug in self:
+            bug.set_sync_with_disk(value)
+
     def _find_root(self, path):
         """
         Search for an existing bug database dir and it's ancestors and
@@ -302,7 +319,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             if beroot == None:
                 raise NoBugDir(path)
             return beroot
-        
+
     def get_version(self, path=None, use_none_rcs=False):
         if use_none_rcs == True:
             RCS = rcs.rcs_by_name("None")
@@ -317,6 +334,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
         return tree_version
 
     def set_version(self):
+        self.rcs.mkdir(self.get_path())
         self.rcs.set_file_contents(self.get_path("version"),
                                    TREE_VERSION_STRING)
 
@@ -348,7 +366,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             if not os.path.exists(self.get_path()):
                 raise NoBugDir(self.get_path())
             self.load_settings()
-            
+
             self.rcs = rcs.rcs_by_name(self.rcs_name)
             self._setup_user_id(self.user_id)
 
@@ -359,10 +377,17 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             self._load_bug(uuid)
 
     def save(self):
-        self.rcs.mkdir(self.get_path())
+        """
+        Save any loaded contents to disk.  Because of lazy loading of
+        bugs and comments, this is actually not too inefficient.
+
+        However, if self.sync_with_disk = True, then any changes are
+        automatically written to disk as soon as they happen, so
+        calling this method will just waste time (unless something
+        else has been messing with your on-disk files).
+        """
         self.set_version()
         self.save_settings()
-        self.rcs.mkdir(self.get_path("bugs"))
         for bug in self:
             bug.save()
 
@@ -378,7 +403,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
         allow_no_rcs = not self.rcs.path_in_root(settings_path)
         # allow_no_rcs=True should only be for the special case of
         # configuring duplicate bugdir settings
-        
+
         try:
             settings = mapfile.map_load(self.rcs, settings_path, allow_no_rcs)
         except rcs.NoSuchFile:
@@ -393,6 +418,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
         allow_no_rcs = not self.rcs.path_in_root(settings_path)
         # allow_no_rcs=True should only be for the special case of
         # configuring duplicate bugdir settings
+        self.rcs.mkdir(self.get_path(), allow_no_rcs)
         mapfile.map_save(self.rcs, settings_path, settings, allow_no_rcs)
 
     def duplicate_bugdir(self, revision):
@@ -443,6 +469,9 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
 
     def new_bug(self, uuid=None, summary=None):
         bg = bug.Bug(bugdir=self, uuid=uuid, summary=summary)
+        bg.set_sync_with_disk(self.sync_with_disk)
+        if bg.sync_with_disk == True:
+            bg.save()
         self.append(bg)
         self._bug_map_gen()
         return bg
@@ -456,7 +485,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
         Generate short names from uuids.  Picks the minimum number of
         characters (>=3) from the beginning of the uuid such that the
         short names are unique.
-        
+
         Obviously, as the number of bugs in the database grows, these
         short names will cease to be unique.  The complete uuid should be
         used for long term reference.
@@ -503,7 +532,7 @@ settings easy.  Don't set this attribute.  Set .rcs instead, and
             if bug_uuid not in self._bug_map:
                 return False
         return True
-        
+
 
 def simple_bug_dir():
     """
@@ -592,14 +621,17 @@ class BugDirTestCase(unittest.TestCase):
         self.failUnless(bugA == bugAprime, "%s != %s" % (bugA, bugAprime))
         self.bugdir.save()
         self.versionTest()
-    def testComments(self):
+    def testComments(self, sync_with_disk=False):
+        if sync_with_disk == True:
+            self.bugdir.set_sync_with_disk(True)
         self.bugdir.new_bug(uuid="a", summary="Ant")
         bug = self.bugdir.bug_from_uuid("a")
         comm = bug.comment_root
         rep = comm.new_reply("Ants are small.")
         rep.new_reply("And they have six legs.")
-        self.bugdir.save()
-        self.bugdir._clear_bugs()        
+        if sync_with_disk == False:
+            self.bugdir.save()
+        self.bugdir._clear_bugs()
         bug = self.bugdir.bug_from_uuid("a")
         bug.load_comments()
         self.failUnless(len(bug.comment_root)==1, len(bug.comment_root))
@@ -623,6 +655,8 @@ class BugDirTestCase(unittest.TestCase):
                                 comment.body)
             else:
                 self.failIf(True, "Invalid comment: %d\n%s" % (index, comment))
+    def testSyncedComments(self):
+        self.testComments(sync_with_disk=True)
 
 unitsuite = unittest.TestLoader().loadTestsFromTestCase(BugDirTestCase)
 suite = unittest.TestSuite([unitsuite])#, doctest.DocTestSuite()])
index 60cccff2bdec7abf82c5da60875fc42789413ba5..d19953bd8620ac1cb08e133f80550096e7f7fc03 100644 (file)
@@ -123,6 +123,7 @@ def loadComments(bug, load_full=False):
         if uuid.startswith('.'):
             continue
         comm = Comment(bug, uuid, from_disk=True)
+        comm.set_sync_with_disk(bug.sync_with_disk)
         if load_full == True:
             comm.load_settings()
             dummy = comm.body # force the body to load
@@ -130,8 +131,6 @@ def loadComments(bug, load_full=False):
     return list_to_root(comments, bug)
 
 def saveComments(bug):
-    path = bug.get_path("comments")
-    bug.rcs.mkdir(path)
     for comment in bug.comment_root.traverse():
         comment.save()
 
@@ -249,6 +248,9 @@ class Comment(Tree, settings_object.SavedSettingsObject):
             self.in_reply_to = in_reply_to
             self.body = body
 
+    def set_sync_with_disk(self, value):
+        self.sync_with_disk = True
+
     def traverse(self, *args, **kwargs):
         """Avoid working with the possible dummy root comment"""
         for comment in Tree.traverse(self, *args, **kwargs):
@@ -429,13 +431,19 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         self._setup_saved_settings()
 
     def save_settings(self):
-        parent_dir = os.path.dirname(self.get_path())
-        self.rcs.mkdir(parent_dir)
         self.rcs.mkdir(self.get_path())
         path = self.get_path("values")
         mapfile.map_save(self.rcs, path, self._get_saved_settings())
 
     def save(self):
+        """
+        Save any loaded contents to disk.
+        
+        However, if self.sync_with_disk = True, then any changes are
+        automatically written to disk as soon as they happen, so
+        calling this method will just waste time (unless something
+        else has been messing with your on-disk files).
+        """
         assert self.body != None, "Can't save blank comment"
         self.save_settings()
         self._set_comment_body(new=self.body, force=True)
@@ -460,6 +468,10 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         True
         """
         reply = Comment(self.bug, body=body)
+        if self.bug != None:
+            reply.set_sync_with_disk(self.bug.sync_with_disk)
+        if reply.sync_with_disk == True:
+            reply.save()
         self.add_reply(reply)
         return reply
 
index 3bf8c9d78ca0f3074a896b3cfb95f47a41910b7f..1e1cfa7b7f73e3c16133e3db8d91f7541d194a3e 100644 (file)
@@ -339,11 +339,15 @@ class RCS(object):
                 self.add(path)
             else:
                 self.update(path)
-    def mkdir(self, path, allow_no_rcs=False):
+    def mkdir(self, path, allow_no_rcs=False, check_parents=True):
         """
         Create (if neccessary) a directory at path under version
         control.
         """
+        if check_parents == True:
+            parent = os.path.dirname(path)
+            if not os.path.exists(parent): # recurse through parents
+                self.mkdir(parent, allow_no_rcs, check_parents)
         if not os.path.exists(path):
             os.mkdir(path)
             if self._use_rcs(path, allow_no_rcs):
@@ -351,7 +355,9 @@ class RCS(object):
         else:
             assert os.path.isdir(path)
             if self._use_rcs(path, allow_no_rcs):
-                self.update(path)
+                #self.update(path)# Don't update directories.  Changing files
+                pass              # underneath them should be sufficient.
+                
     def duplicate_repo(self, revision=None):
         """
         Get the repository as it was in a given revision.