This patch does some cleaning up on the git checkout class:

1 - Used the logging module
2 - Made docstrings comply with coding standards

Next steps on the work of getting the git class suitable
for common autotest usage are:

* Decouple interaction with git repo from the concept of
host (which is only available on the autotest server)
* Move the libraries to the common lib

That will be made on a separate patchset.

Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
---
 server/git.py        |  102 +++++++++++++++++++++++++++++++++----------------
 server/git_kernel.py |   82 +++++++++++++++++++--------------------
 2 files changed, 109 insertions(+), 75 deletions(-)

diff --git a/server/git.py b/server/git.py
index 5f6068b..2f1bb80 100644
--- a/server/git.py
+++ b/server/git.py
@@ -1,18 +1,11 @@
-#
-# Copyright 2007 IBM Corp. Released under the GPL v2
-# Authors: Ryan Harper <[email protected]>
-#
-
 """
 This module defines a class for handling building from git repos
-"""
 
-__author__ = """
[email protected] (Ryan Harper)
+...@author: Ryan Harper ([email protected])
+...@copyright: IBM 2007
 """
 
-
-import os, warnings
+import os, warnings, logging
 from autotest_lib.client.common_lib import error
 from autotest_lib.client.bin import os_dep
 from autotest_lib.server import utils, installable_object
@@ -25,17 +18,16 @@ class GitRepo(installable_object.InstallableObject):
     It is used to pull down a local copy of a git repo, check if the local
     repo is up-to-date, if not update.  It delegates the install to
     implementation classes.
-
     """
 
     def __init__(self, repodir, giturl, weburl=None):
         super(installable_object.InstallableObject, self).__init__()
         if repodir is None:
-            e_msg = 'You must provide a directory to hold the git repository'
-            raise ValueError(e_msg)
+            raise ValueError('You must provide a path that will hold the'
+                             'git repository')
         self.repodir = utils.sh_escape(repodir)
         if giturl is None:
-            raise ValueError('You must provide a git URL to the repository')
+            raise ValueError('You must provide a git URL repository')
         self.giturl = giturl
         if weburl is not None:
             warnings.warn("Param weburl: You are no longer required to provide 
"
@@ -51,28 +43,53 @@ class GitRepo(installable_object.InstallableObject):
         self.gitcmdbase = '%s --git-dir=%s' % (git_base_cmd, self.gitpath)
 
         # default to same remote path as local
-        self.__build = os.path.dirname(self.repodir)
+        self._build = os.path.dirname(self.repodir)
 
 
-    def run(self, command, timeout=None, ignore_status=False):
+    def _run(self, command, timeout=None, ignore_status=False):
+        """
+        Auxiliary function to run a command, with proper shell escaping.
+
+        @param timeout: Timeout to run the command.
+        @param ignore_status: Whether we should supress error.CmdError
+                exceptions if the command did return exit code !=0 (True), or
+                not supress them (False).
+        """
         return utils.run(r'%s' % (utils.sh_escape(command)),
-                          timeout, ignore_status)
+                         timeout, ignore_status)
 
 
     # base install method
     def install(self, host, builddir=None):
+        """
+        Install a git repo on a host. It works by pushing the downloaded source
+        code to the host.
+
+        @param host: Host object.
+        @param builddir: Directory on the host filesystem that will host the
+                source code.
+        """
         # allow override of target remote dir
         if builddir:
-            self.__build = builddir
+            self._build = builddir
 
         # push source to host for install
-        print 'pushing %s to host:%s' %(self.source_material, self.__build)
-        host.send_file(self.source_material, self.__build)
+        logging.info('Pushing code dir %s to host %s', self.source_material,
+                     self._build)
+        host.send_file(self.source_material, self._build)
 
 
     def gitcmd(self, cmd, ignore_status=False):
+        """
+        Wrapper for a git command.
+
+        @param cmd: Git subcommand (ex 'clone').
+        @param ignore_status: Whether we should supress error.CmdError
+                exceptions if the command did return exit code !=0 (True), or
+                not supress them (False).
+        """
         cmd = '%s %s' % (self.gitcmdbase, cmd)
-        return self.run(cmd, ignore_status=ignore_status)
+        return self._run(cmd, ignore_status=ignore_status)
 
 
     def get(self, **kwargs):
@@ -82,12 +99,13 @@ class GitRepo(installable_object.InstallableObject):
         this method will leave an up-to-date version of git repo at
         'giturl' in 'repodir' directory to be used by build/install
         methods.
-        """
 
+        @param **kwargs: Dictionary of parameters to the method get.
+        """
         if not self.is_repo_initialized():
             # this is your first time ...
-            print 'cloning repo...'
-            cmd = 'clone %s %s ' %(self.giturl, self.repodir)
+            logging.info('Cloning git repo %s', self.giturl)
+            cmd = 'clone %s %s ' % (self.giturl, self.repodir)
             rv = self.gitcmd(cmd, True)
             if rv.exit_status != 0:
                 print rv.stderr
@@ -98,7 +116,7 @@ class GitRepo(installable_object.InstallableObject):
         else:
             # exiting repo, check if we're up-to-date
             if self.is_out_of_date():
-                print 'updating repo...'
+                logging.info('Updating git repo %s', self.giturl)
                 rv = self.gitcmd('pull', True)
                 if rv.exit_status != 0:
                     print rv.stderr
@@ -107,18 +125,27 @@ class GitRepo(installable_object.InstallableObject):
             else:
                 print 'repo up-to-date'
 
-
         # remember where the source is
         self.source_material = self.repodir
 
 
     def get_local_head(self):
+        """
+        Get the top commit hash of the current local git branch.
+
+        @return: Top commit hash of local git branch
+        """
         cmd = 'log --pretty=format:"%H" -1'
         l_head_cmd = self.gitcmd(cmd)
         return l_head_cmd.stdout
 
 
     def get_remote_head(self):
+        """
+        Get the top commit hash of the current remote git branch.
+
+        @return: Top commit hash of remote git branch
+        """
         cmd1 = 'remote show'
         origin_name_cmd = self.gitcmd(cmd1)
         cmd2 = 'log --pretty=format:"%H" -1 ' + origin_name_cmd.stdout
@@ -127,6 +154,11 @@ class GitRepo(installable_object.InstallableObject):
 
 
     def is_out_of_date(self):
+        """
+        Return whether this branch is out of date with regards to remote 
branch.
+
+        @return: False, if the branch is outdated, True if it is current.
+        """
         local_head = self.get_local_head()
         remote_head = self.get_remote_head()
 
@@ -138,9 +170,11 @@ class GitRepo(installable_object.InstallableObject):
 
 
     def is_repo_initialized(self):
-        # if we fail to get a rv of 0 out of the git log command
-        # then the repo is bogus
+        """
+        Return whether the git repo was already initialized (has a top commit).
 
+        @return: False, if the repo was initialized, True if it was not.
+        """
         cmd = 'log --max-count=1'
         rv = self.gitcmd(cmd, True)
         if rv.exit_status == 0:
@@ -153,7 +187,6 @@ class GitRepo(installable_object.InstallableObject):
         """
         Return current HEAD commit id
         """
-
         if not self.is_repo_initialized():
             self.get()
 
@@ -172,7 +205,9 @@ class GitRepo(installable_object.InstallableObject):
 
         Optional give the local branch name as local.
 
-        Note, for git checkout tag git version >= 1.5.0 is required
+        @param remote: Remote commit hash
+        @param local: Local commit hash
+        @note: For git checkout tag git version >= 1.5.0 is required
         """
         if not self.is_repo_initialized():
             self.get()
@@ -194,8 +229,9 @@ class GitRepo(installable_object.InstallableObject):
         """
         Show the branches.
 
-        all - list both remote-tracking branches and local branches.
-        remote_tracking - lists the remote-tracking branches.
+        @param all: List both remote-tracking branches and local branches 
(True)
+                or only the local ones (False).
+        @param remote_tracking: Lists the remote-tracking branches.
         """
         if not self.is_repo_initialized():
             self.get()
@@ -213,6 +249,6 @@ class GitRepo(installable_object.InstallableObject):
         elif all or remote_tracking:
             return gitlog.stdout.strip('\n')
         else:
-            branch = [b.lstrip('* ') for b in gitlog.stdout.split('\n') \
+            branch = [b.lstrip('* ') for b in gitlog.stdout.split('\n')
                       if b.startswith('*')][0]
             return branch
diff --git a/server/git_kernel.py b/server/git_kernel.py
index a23881e..8a3ea8f 100644
--- a/server/git_kernel.py
+++ b/server/git_kernel.py
@@ -1,18 +1,11 @@
-#
-# Copyright 2007 IBM Corp. Released under the GPL v2
-# Authors: Ryan Harper <[email protected]>
-#
-
 """
 This module defines the GitKernel class
-"""
 
-__author__ = """
[email protected] (Ryan Harper)
+...@author: Ryan Harper ([email protected])
+...@copyright: IBM 2007
 """
 
-
-import os
+import os, logging
 import git, source_kernel
 
 
@@ -25,56 +18,60 @@ class GitKernel(git.GitRepo):
     """
     def __init__(self, repodir, giturl, weburl):
         git.GitRepo.__init__(self, repodir, giturl, weburl)
-        self.__patches = []
-        self.__config = None
-        self.__build = None
-        self.__branch = None
-        self.__revision = None
+        self._patches = []
+        self._config = None
+        self._build = None
+        self._branch = None
+        self._revision = None
 
 
     def configure(self, config):
-        self.__config = config
+        self._config = config
 
 
     def patch(self, patch):
-        self.__patches.append(patch)
+        self._patches.append(patch)
 
 
     def checkout(self, revision, local=None):
         """
-        Checkout the commit id, branch, or tag
+        Checkout the commit id, branch, or tag.
 
-        revision:str - name of the git remote branch, revision or tag
-        local:str - name of the local branch, implies -b
+        @param revision: Name of the git remote branch, revision or tag
+        @param local: Name of the local branch, implies -b
         """
-        print 'checking out %s' % revision
+        logging.info('Checking out %s', revision)
         super(GitKernel, self).checkout(revision)
-        self.__revision = super(GitKernel, self).get_revision()
-        self.__branch = super(GitKernel, self).get_branch()
-        print 'checked out %s on branch %s' % (self.__revision, self.__branch)
+        self._revision = super(GitKernel, self).get_revision()
+        self._branch = super(GitKernel, self).get_branch()
+        logging.info('Checked out %s on branch %s', self._revision,
+                     self._branch)
 
     def show_branch(self):
         """
-        Print the current local branch name
+        Print the current local branch name.
         """
-        self.__branch = super(GitKernel, self).get_branch()
-        print self.__branch
+        self._branch = super(GitKernel, self).get_branch()
+        logging.info(self._branch)
 
 
     def show_branches(self, all=True):
         """
-        Print the local and remote branches
+        Print the local and remote branches.
+
+        @param all: Whether to show all branches (True) or only local branches
+                (False).
         """
-        self.__branch = super(GitKernel, self).get_branch()
-        print super(GitKernel, self).get_branch(all=all)
+        self._branch = super(GitKernel, self).get_branch()
+        logging.info(super(GitKernel, self).get_branch(all=all))
 
 
     def show_revision(self):
         """
-        Show the current git revision
+        Show the current git revision.
         """
-        self.__revision = super(GitKernel, self).get_revision()
-        print self.__revision
+        self._revision = super(GitKernel, self).get_revision()
+        logging.info(self._revision)
 
 
     def install(self, host, build=True, builddir=None, revision=None):
@@ -91,17 +88,18 @@ class GitKernel(git.GitRepo):
         """
         if revision:
             self.checkout(revision)
-            self.__revision = super(GitKernel, self).get_revision()
-            print 'checked out revision: %s' %(self.__revision)
+            self._revision = super(GitKernel, self).get_revision()
+            logging.info('Checked out revision: %s', self._revision)
 
         if not builddir:
-            self.__build = os.path.join(host.get_tmp_dir(),"build")
-            print 'warning: builddir %s is not persistent' %(self.__build)
+            self._build = os.path.join(host.get_tmp_dir(),"build")
+            logging.warning('Builddir %s is not persistent (it will be erased '
+                            'in future jobs)', self._build)
 
         # push source to host for install
-        print 'pushing %s to host' % self.source_material
-        host.send_file(self.source_material, self.__build)
-        remote_source_material= os.path.join(self.__build,
+        logging.info('Pushing %s to host', self.source_material)
+        host.send_file(self.source_material, self._build)
+        remote_source_material= os.path.join(self._build,
                                         os.path.basename(self.source_material))
 
         # use a source_kernel to configure, patch, build and install.
@@ -109,11 +107,11 @@ class GitKernel(git.GitRepo):
 
         if build:
             # apply patches
-            for p in self.__patches:
+            for p in self._patches:
                 sk.patch(p)
 
             # configure
-            sk.configure(self.__config)
+            sk.configure(self._config)
 
             # build
             sk.build(host)
-- 
1.7.2.2

_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to