From 4c7a9d837a268480e61124bba84b5fb01ec3728f Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 21 Jul 2009 07:28:26 -0400 Subject: [PATCH] Cleaned up saving/sync_with_disk. 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... --- becommands/comment.py | 1 - becommands/depend.py | 1 - becommands/diff.py | 2 +- becommands/merge.py | 8 ++-- becommands/new.py | 1 - becommands/open.py | 1 - becommands/remove.py | 1 - becommands/set.py | 3 +- becommands/severity.py | 1 - becommands/status.py | 1 - becommands/tag.py | 3 +- becommands/target.py | 1 - libbe/bug.py | 16 +++++++- libbe/bugdir.py | 92 +++++++++++++++++++++++++++++------------- libbe/comment.py | 20 +++++++-- libbe/rcs.py | 10 ++++- 16 files changed, 108 insertions(+), 54 deletions(-) diff --git a/becommands/comment.py b/becommands/comment.py index 66f8da1..eba640e 100644 --- a/becommands/comment.py +++ b/becommands/comment.py @@ -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]") diff --git a/becommands/depend.py b/becommands/depend.py index 48e1527..4a23b0f 100644 --- a/becommands/depend.py +++ b/becommands/depend.py @@ -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: diff --git a/becommands/diff.py b/becommands/diff.py index f3474b3..0acfcb2 100644 --- a/becommands/diff.py +++ b/becommands/diff.py @@ -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: diff --git a/becommands/merge.py b/becommands/merge.py index c030dd0..4aaefa8 100644 --- a/becommands/merge.py +++ b/becommands/merge.py @@ -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(): diff --git a/becommands/new.py b/becommands/new.py index 5325ccc..af599d7 100644 --- a/becommands/new.py +++ b/becommands/new.py @@ -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(): diff --git a/becommands/open.py b/becommands/open.py index b4b1025..2ef5f43 100644 --- a/becommands/open.py +++ b/becommands/open.py @@ -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") diff --git a/becommands/remove.py b/becommands/remove.py index d441bfe..d79a7be 100644 --- a/becommands/remove.py +++ b/becommands/remove.py @@ -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(): diff --git a/becommands/set.py b/becommands/set.py index 510eca7..0c0862f 100644 --- a/becommands/set.py +++ b/becommands/set.py @@ -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]") diff --git a/becommands/severity.py b/becommands/severity.py index fde9fba..65467e3 100644 --- a/becommands/severity.py +++ b/becommands/severity.py @@ -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]") diff --git a/becommands/status.py b/becommands/status.py index 89ae49a..edc948d 100644 --- a/becommands/status.py +++ b/becommands/status.py @@ -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]") diff --git a/becommands/tag.py b/becommands/tag.py index a139528..216ffbc 100644 --- a/becommands/tag.py +++ b/becommands/tag.py @@ -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: diff --git a/becommands/target.py b/becommands/target.py index 905c639..527b16a 100644 --- a/becommands/target.py +++ b/becommands/target.py @@ -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") diff --git a/libbe/bug.py b/libbe/bug.py index 8be6010..e3dc1e0 100644 --- a/libbe/bug.py +++ b/libbe/bug.py @@ -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): diff --git a/libbe/bugdir.py b/libbe/bugdir.py index e37db81..9802502 100644 --- a/libbe/bugdir.py +++ b/libbe/bugdir.py @@ -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()]) diff --git a/libbe/comment.py b/libbe/comment.py index 60cccff..d19953b 100644 --- a/libbe/comment.py +++ b/libbe/comment.py @@ -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 diff --git a/libbe/rcs.py b/libbe/rcs.py index 3bf8c9d..1e1cfa7 100644 --- a/libbe/rcs.py +++ b/libbe/rcs.py @@ -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. -- 2.26.2