On Thu, 2010-09-02 at 16:33 +0200, Frank Becker wrote:
> Hi,
>
> I need to checkout kernel revisions by branch, commit id, or tag before
> building and testing them.
>
> Here is my patch to extend server/git.py and server/git_kernel.py
>
> I'm very open for any kind of suggestions.
Copying Divya - Divya, I will try to speed up all the changes so you can
have an API to use git repos in your client side tests.
Hi Frank, I reviewed your patchset and it looks mostly good to me. I
have some small comments with regards to coding style (I know it's
boring, but we have to enforce the coding style as much as possible).
I've been thinking about this and here is a braindump about the
situation:
1 - We need a good API to manipulate and interact with git trees
2 - This API has to be common between client and tests, so in another
patchset server.git will be moved to common_lib.git
3 - We should be careful to maintain existing API and fix any changes in
the whole source tree
4 - In KVM autotest, we are still using our own, simple function to get
git trees. I think some of the code of that function could be used on
the improved Git() class.
5 - My plan is to do some minimal changes to your patchset, send an
updated version, apply it and then execute 2).
Thanks for your contribution, see comments below:
> The following new methods exist in GitKernel()
> * checkout - Check out a certain branch, tag or revision. Optional
> a local branch name can be given for -b
> * show_branch - print the current local branch
> * show_branches - print the local and/or remote branches
> * show_revision - print the current tip commit id (checked out revision)
> * extended install - optional a commit id / revision that is checked out
> before
> patching, configure, and build
>
> Therefore the following methods were added to Git()
> * get_revision() - To print the current tip commit id (aka revision).
> * checkout() - Check out a certain branch, tag or revision.
> * get_branch() - return the branches depending on options
>
> Risk: Low (Just extends functionality, doesn't change existing one.)
> Visibility: Users who build Linux kernels may profit.
>
>
> Bye,
>
> Frank
> ---
>
> diff --git a/server/git.py b/server/git.py
> --- a/server/git.py
> +++ b/server/git.py
> @@ -183,3 +183,69 @@
> return True
>
> return False
> +
> + def get_revision(self):
> + """
> + Return current HEAD commit id
> + """
> +
> + if not self.is_repo_initialized():
> + self.get()
> +
> + cmd = 'rev-parse --verify HEAD'
> + gitlog = self.gitcmd(cmd, True)
> + if gitlog.exit_status != 0:
> + print gitlog.stderr
> + raise error.CmdError('Failed to find git sha1 revision', gitlog)
> + else:
> + return gitlog.stdout.strip('\n')
^ We enforce 2 lines of separation between functions even if they belong
to a class. Need to fix them both.
> + def checkout(self, remote, local=None):
> + """
> + Check out the git commit id, branch, or tag given by remote.
> +
> + Optional give the local branch name as local.
> +
> + Note, for git checkout tag git version >= 1.5.0 is required
^ Need to document the parameters:
@param remote: Remote commit id, branch or tag
@local local: Local branch name
> + """
> + if not self.is_repo_initialized():
> + self.get()
> +
> + assert(isinstance(remote, basestring))
> + if local:
> + cmd = 'checkout -b %s %s' % (local, remote)
> + else:
> + cmd = 'checkout %s' % (remote)
> + gitlog = self.gitcmd(cmd, True)
> + if gitlog.exit_status != 0:
> + print gitlog.stderr
> + raise error.CmdError('Failed to checkout git branch', gitlog)
> + else:
> + print gitlog.stdout
> +
> + def get_branch(self, all=False, remote_tracking=False):
> + """
> + Show the branches.
> +
> + all - list both remote-tracking branches and local branches.
> + remote_tracking - lists the remote-tracking branches.
^ Have to use @param to document the parameters
> + """
> + if not self.is_repo_initialized():
> + self.get()
> +
> + cmd = 'branch --no-color'
> + if all:
> + cmd = " ".join([cmd, "-a"])
> + if remote_tracking:
> + cmd = " ".join([cmd, "-r"])
> +
> + gitlog = self.gitcmd(cmd, True)
> + if gitlog.exit_status != 0:
> + print gitlog.stderr
> + raise error.CmdError('Failed to get git branch', gitlog)
> + elif all or remote_tracking:
> + return gitlog.stdout.strip('\n')
> + else:
> + branch = [b.lstrip('* ') for b in gitlog.stdout.split('\n') \
^ Here you can use implicit line continuation instead of the backslash
> + if b.startswith('*')][0]
> + return branch
> diff --git a/server/git_kernel.py b/server/git_kernel.py
> --- a/server/git_kernel.py
> +++ b/server/git_kernel.py
> @@ -29,21 +29,61 @@
> self.__patches = []
> self.__config = None
> self.__build = None
> -
> + self.__branch = None
> + self.__revision = None
^ Even though the original code is using double underscores, the ideal
situation is to use a single underscore, unless we really need it (ie,
we need the name mangling aspect of using double underscore). This will
have to be fixed on a later patch though.
> def configure(self, config):
> self.__config = config
>
> -
> def patch(self, patch):
> self.__patches.append(patch)
>
> + def checkout(self, revision, local=None):
> + """
> + Checkout the commit id, branch, or tag
^ The checkout method could support more parameters, such as branch,
source dir, commit, lbranch... so whether each one of them are present
(we can initialize , we build an appropriate git fetch command). For kvm
autotest, in order to checkout a git repo we use the rudimentary utility
function
def get_git_branch(repository, branch, srcdir, commit=None, lbranch=None):
Present on client/tests/kvm/kvm_utils.py. Of course, when Divya started
the git thread, and you came with this patchset, it occurred to me that
we can come up with a decent common API to do this job.
> - def install(self, host, build=True, builddir=None):
> + revision:str - name of the git remote branch, revision or tag
> + local:str - name of the local branch, implies -b
> +
> + """
> + print '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)
> +
> + def show_branch(self):
> + """
> + Print the current local branch name
> + """
> + self.__branch = super(GitKernel, self).get_branch()
> + print self.__branch
> +
> + def show_branches(self, all=True):
> + """
> + Print the local and remote branches
^ Document the parameters
> + """
> + self.__branch = super(GitKernel, self).get_branch()
> + print super(GitKernel, self).get_branch(all=all)
> +
> + def show_revision(self):
> + """
> + Show the current git revision
> + """
> + self.__revision = super(GitKernel, self).get_revision()
> + print self.__revision
> +
> + def install(self, host, build=True, builddir=None, revision=None):
> # use tmpdir if no builddir specified
> # NB: pass a builddir to install() method if you
> # need to ensure the build remains after the completion
> # of a job
> + # If revision is given, that revision is checked out before
> + if revision:
> + self.checkout(revision)
> + self.__revision = super(GitKernel, self).get_revision()
> + print '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)
>
>
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
