Merge lp:~bac/tarmac/make_treedir into lp:tarmac

[Brad Crittenden]

If the dirname() portion of tree_dir does not exist, then create it in create_tree.

If the dirname() portion of tree_dir does not exist, then create it in create_tree.

Also made some changes to the documentation that I think helps clarify things. One change that may be controversial is to *not* use 'tarmac' as the example when explaining the configuration as it is too subtle for new users to see the difference between the [Tarmac] stanzas and the subsequent [lp:tarmac] parts. Perhaps just an introductory statement would've sufficed but it seemed reasonable to avoid the confusion all together.

Some line changes are due to deleting trailing whitespace, which my editor does automatically. Sorry for polluting the diff with those. Similarly, I removed some apostrophes (single-quotes) contained within docstrings as they appear as unbalanced quotes.

[Paul Hummer (rockstar)]

review: Approve

[Brad Crittenden (bac)]

Thanks for the review Paul. I assume you'll merge it into trunk shortly?

=== modified file 'HACKING'
--- HACKING	2010-11-04 16:34:30 +0000
+++ HACKING	2012-06-29 20:15:23 +0000
@@ -24,6 +24,17 @@
 
 
 =============
+Running Tests
+=============
+
+Tests are run using 'trial' from Twisted and rely on bzrlib.tests.  So
+you must install the `python-twisted-core` and `python-bzrlib.tests`
+packages in order to run the tests.  Once done, from the top of the
+repository you can run:
+
+% trial tarmac
+
+=============
 Writing Tests
 =============
 
 
=== modified file 'docs/introduction.txt'
--- docs/introduction.txt	2011-06-22 09:50:42 +0000
+++ docs/introduction.txt	2012-06-29 20:15:23 +0000
@@ -25,13 +25,13 @@
 =============
 
 Tarmac gets it's configuration data from ``~/.config/tarmac/tarmac.conf`` --
-The configuration is split up by projects.  As an example, Tarmac will
-demonstrate its own config throughout this project.  To start with, the Tarmac
-project should be added to the config (with the file being created if it
-doesn't exist already).::
+The configuration is split up by projects.  As an example, a fictional
+`phoo` project will be used.  The config file starts with items that
+apply to the Tarmac command.  Add to the following to the config file,
+creating it if it doesn't exist already).::
 
   [Tarmac]
-  # Configuration for the Tarmac project
+  # Configuration for the Tarmac command.
 
 This doesn't mean much by itself.  Optionally, we can also specify where to
 write the log (by default, it logs to ~/tarmac.log).  A basic log file setting
@@ -40,19 +40,22 @@
   [Tarmac]
   log_file = /var/log/tarmac.log
 
-Now let's provide some branches for Tarmac to manage.  How about Tarmac's
+If placing the file in ``/var/log`` be sure the user running
+``tarmac`` has write permission to that file.
+
+Now let's provide some branches for Tarmac to manage.  How about phoo's
 development focus first?  Specifying this is REALLY easy.  Now my config file
 will look like this::
 
   [Tarmac]
   log_file = /var/log/tarmac.log
 
-  [lp:tarmac]
+  [lp:phoo]
 
 That's it!  I specify the branch by it's lp: name, and Tarmac knows that when
-it's instructed to land branches, it should check up on lp:tarmac for approved
+it's instructed to land branches, it should check up on lp:phoo for approved
 merge proposals.  However, this means that Tarmac has to make a new tree of
-lp:tarmac every time it makes a landing run.  This is rather inefficient.  We
+lp:phoo every time it makes a landing run.  This is rather inefficient.  We
 can actually have Tarmac cache a tree, and just update it on every run.  This
 path to the cache tree can be specified by changing the config file to look
 like this::
@@ -60,8 +63,8 @@
   [Tarmac]
   log_file = /var/log/tarmac.log
 
-  [lp:tarmac]
-  tree_dir = /var/cache/tarmac
+  [lp:phoo]
+  tree_dir = /var/cache/tarmac/phoo/trunk
 
 If this directory or tree doesn't exist, Tarmac will go ahead and create it.
 
@@ -71,14 +74,15 @@
 
 Once Tarmac is all configured, simply run ``tarmac merge``.  Voila!  If there
 are issues at all, you can run ``tarmac merge --debug`` to get more debug
-imformation.
+information.
 
 ==============
 Tarmac on Cron
 ==============
 
-Tarmac runs best on a cron, because it's doing stuff you shouldn't even need to
-worry about.  To make Tarmac run on a cron, add this to your crontab::
+Tarmac runs best on a cron job, because it's doing stuff you shouldn't
+even need to worry about.  To make Tarmac run on a cron, add this to
+your crontab::
 
 0 * * * * tarmac merge
 
@@ -190,10 +194,11 @@
 CIA.vc Notifier
 ===============
 
-The CIA.vc Notifier can be configured to notify the CIA.vc service of
-successful merges.  Tarmac has already been configured to work with the CIA.vc
-service and is set up to notify those in the #tarmac IRC channel on Freenode.
-This configuration looks like this::
+The CIA.vc Notifier (see ``http://cia.vc``) can be configured to
+notify the CIA.vc service of successful merges.  Tarmac has already
+been configured to work with the CIA.vc service and is set up to
+notify those in the #tarmac IRC channel on Freenode.  This
+configuration looks like this::
 
   [lp:tarmac]
   cia_project = tarmac
 
=== modified file 'docs/writingplugins.txt'
--- docs/writingplugins.txt	2010-09-17 21:57:28 +0000
+++ docs/writingplugins.txt	2012-06-29 20:15:23 +0000
@@ -53,7 +53,7 @@
 arguments to the plugin are explained below.
 
 **command**
-  The tarmac command.  For instance, a ``tarmac land`` call.
+  The tarmac command.  For instance, a ``tarmac merge`` call.
 
 **target**
   An instance of ``tarmac.branch.Branch`` containing details about the target
 
=== modified file 'tarmac/bin/commands.py'
--- tarmac/bin/commands.py	2012-06-28 13:50:56 +0000
+++ tarmac/bin/commands.py	2012-06-29 20:15:23 +0000
@@ -40,7 +40,7 @@
             self.config.set('Tarmac', name, False)
 
     def _usage(self):
-        """Custom _usage for referencing "tarmac" instead of "bzr."""
+        """Custom _usage for referencing 'tarmac' instead of 'bzr'."""
         s = 'tarmac ' + self.name() + ' '
         for aname in self.takes_args:
             aname = aname.upper()
@@ -306,7 +306,7 @@
                     "'Approved'".format(entry.queue_status))
                 continue
 
-            if (not self.config.imply_commit_message and 
+            if (not self.config.imply_commit_message and
                 not entry.commit_message):
                 self.logger.debug(
                     "  Skipping proposal: proposal has no commit message")
 
=== modified file 'tarmac/branch.py'
--- tarmac/branch.py	2010-10-22 17:59:26 +0000
+++ tarmac/branch.py	2012-06-29 20:15:23 +0000
@@ -72,6 +72,11 @@
                 self.tree = WorkingTree.open(self.config.tree_dir)
             else:
                 self.logger.debug('Tree does not exist.  Creating dir')
+                # Create the path up to but not including tree_dir if it does
+                # not exist.
+                parent_dir = os.path.dirname(self.config.tree_dir)
+                if not os.path.exists(parent_dir):
+                    os.makedirs(parent_dir)
                 self.tree = self.bzr_branch.create_checkout(
                     self.config.tree_dir, lightweight=True)
         except AttributeError:
 
=== modified file 'tarmac/config.py'
--- tarmac/config.py	2011-01-05 17:25:58 +0000
+++ tarmac/config.py	2012-06-29 20:15:23 +0000
@@ -104,7 +104,7 @@
                 section.startswith('lp:')]
 
     def _check_config_dirs(self):
-        '''Create the configuration directory if it doesn't exist.'''
+        '''Create the configuration directory if it does not exist.'''
         if not os.path.exists(self.CONFIG_HOME):
             os.makedirs(self.CONFIG_HOME)
         if not os.path.exists(self.CACHE_HOME):
@@ -117,7 +117,7 @@
 class BranchConfig:
     '''A Branch specific config.
 
-    Instead of providing the whole config for branches, it's better to provide
+    Instead of providing the whole config for branches, it is better to provide
     it with only its specific config vars.
     '''
 
 
=== modified file 'tarmac/tests/test_branch.py'
--- tarmac/tests/test_branch.py	2010-10-22 13:51:39 +0000
+++ tarmac/tests/test_branch.py	2012-06-29 20:15:23 +0000
@@ -41,6 +41,25 @@
         self.assertFalse(hasattr(a_branch, 'tree'))
         self.remove_branch_config(tree_dir)
 
+    def test_create_missing_parent_dir(self):
+        '''Test the creation of a TarmacBranch instance in a path that does
+        not fully exist, with a tree'''
+        branch_name = 'test_branch'
+        parent_dir = os.path.join(self.TEST_ROOT, 'missing')
+        tree_dir = os.path.join(parent_dir, branch_name)
+        self.add_branch_config(tree_dir)
+        # Create the mock somewhere other than where the tarmac branch will be
+        # located.  Keep it right under TEST_ROOT so the
+        # TarmacDirectoryFactory mocking will work.
+        mock = MockLPBranch(os.path.join(self.TEST_ROOT, branch_name))
+        self.assertFalse(os.path.exists(parent_dir))
+        a_branch = branch.Branch.create(mock, self.config, create_tree=True)
+        self.assertTrue(os.path.exists(parent_dir))
+        self.assertTrue(isinstance(a_branch, branch.Branch))
+        self.assertTrue(a_branch.lp_branch.bzr_identity is not None)
+        self.assertTrue(hasattr(a_branch, 'tree'))
+        self.remove_branch_config(tree_dir)
+
     def test_create_with_tree(self):
         '''Test the creation of a TarmacBranch with a created tree.'''
         self.assertTrue(isinstance(self.branch1, branch.Branch))
 
=== modified file 'tarmac/tests/test_commands.py'
--- tarmac/tests/test_commands.py	2010-10-12 19:58:22 +0000
+++ tarmac/tests/test_commands.py	2012-06-29 20:15:23 +0000
@@ -60,7 +60,7 @@
     #    sys.stdout = old_stdout
 
     def test_run_already_authenticated(self):
-        '''If the user has already been authenticated, don't try again.'''
+        '''If the user has already been authenticated, do not try again.'''
         registry = CommandRegistry(config=self.config)
         registry.register_command('authenticate', commands.cmd_authenticate)
         command = registry._get_command(commands.cmd_authenticate,