Skip to content

Move checkout to CFFI and add a target directory option #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 11, 2014
+83 −83
Merged

Move checkout to CFFI and add a target directory option #390

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af38211
checkout: move the code to cffi
carlosmn Jul 11, 2014
763b571
checkout: add support for overriding the target directory
carlosmn Jul 11, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
checkout: move the code to cffi 
As part of this, make the strategy part of **kwargs, in preparation for
supporting more options.
  • Loading branch information
@carlosmn
carlosmn committed Jul 11, 2014
commit af38211d0cb0123db35dd85ffb976c082ae33520
6 changes: 6 additions & 0 deletions pygit2/decl.h
View file
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ typedef ... git_remote;
typedef ... git_refspec;
typedef ... git_push;
typedef ... git_cred;
typedef ... git_object;
typedef ... git_tree;
typedef ... git_signature;
typedef ... git_index;
@@ -309,6 +310,11 @@ typedef enum {
GIT_CLONE_LOCAL_NO_LINKS,
} git_clone_local_t;

int git_checkout_init_options(git_checkout_options *opts, unsigned int version);
int git_checkout_tree(git_repository *repo, const git_object *treeish, const git_checkout_options *opts);
int git_checkout_head(git_repository *repo, const git_checkout_options *opts);
int git_checkout_index(git_repository *repo, git_index *index, const git_checkout_options *opts);

/*
* git_clone
*/
61 changes: 56 additions & 5 deletions pygit2/repository.py
View file
Original file line number Diff line number Diff line change
@@ -176,11 +176,52 @@ def create_reference(self, name, target, force=False):

return self.create_reference_symbolic(name, target, force)


#
# Checkout
#
def checkout(self, refname=None, strategy=GIT_CHECKOUT_SAFE_CREATE):
@staticmethod
def _checkout_args_to_options(**kwargs):
# Create the options struct to pass
copts = ffi.new('git_checkout_options *')
check_error(C.git_checkout_init_options(copts, 1))

# pygit2's default is SAFE_CREATE
copts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE
# and go through the arguments to see what the user wanted
for k, v in kwargs.iteritems():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to iterate? You can do it with 2 if statements:

if 'strategy' in kwargs:
  copts.checkout_strategy = kwargs['strategy']

if 'directory' in kwargs:
  target_dir = ffi.new('char[]', to_str(kwargs['directory']))
  refs.append(target_dir)
  copts.target_directory = target_dir

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a leftover from working with less abstract constructions; but it's a guess as to how many parameters will actually be passed versus how many we will be looking at. I'm not a fan of looking up the same key twice, but it can be reduced.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember now why I wanted to iterate over the inputs instead of checking for the known keys: if the programmer using this method makes a typo, we will ignore that option instead of complaining, which can cause subtle bugs, as there is no indication that we're ignoring an argument that was passed. I didn't end up doing it as we just have the two things, but when this gets extended, we should raise an exception if we get something we don't recognise as an option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just changed this static method so it is shorter and fails on unexpected arguments.

if k == 'strategy':
copts.checkout_strategy = v

return copts

def checkout_head(self, **kwargs):
"""Checkout HEAD

For arguments, see Repository.checkout().
"""
copts = Repository._checkout_args_to_options(**kwargs)
check_error(C.git_checkout_head(self._repo, copts))

def checkout_index(self, **kwargs):
"""Checkout the repository's index

For arguments, see Repository.checkout().
"""
copts = Repository._checkout_args_to_options(**kwargs)
check_error(C.git_checkout_index(self._repo, ffi.NULL, copts))

def checkout_tree(self, treeish, **kwargs):
"""Checkout the given treeish

For arguments, see Repository.checkout().
"""
copts = Repository._checkout_args_to_options(**kwargs)
cptr = ffi.new('git_object **')
ffi.buffer(cptr)[:] = treeish._pointer[:]

check_error(C.git_checkout_tree(self._repo, cptr[0], copts))

def checkout(self, refname=None, **kwargs):
"""
Checkout the given reference using the given strategy, and update
the HEAD.
@@ -193,14 +234,24 @@ def checkout(self, refname=None, strategy=GIT_CHECKOUT_SAFE_CREATE):

If no reference is given, checkout from the index.

Arguments:

:param str refname: The reference to checkout. After checkout,
the current branch will be switched to this one.

:param int strategy: A ``GIT_CHECKOUT_`` value. The default is
``GIT_CHECKOUT_SAFE_CREATE``.

"""


# Case 1: Checkout index
if refname is None:
return self.checkout_index(strategy)
return self.checkout_index(**kwargs)

# Case 2: Checkout head
if refname == 'HEAD':
return self.checkout_head(strategy)
return self.checkout_head(**kwargs)

# Case 3: Reference
if type(refname) is Reference:
@@ -211,7 +262,7 @@ def checkout(self, refname=None, strategy=GIT_CHECKOUT_SAFE_CREATE):

oid = reference.resolve().target
treeish = self[oid]
self.checkout_tree(treeish, strategy)
self.checkout_tree(treeish, **kwargs)
self.head = refname


76 changes: 0 additions & 76 deletions src/repository.c
View file
Original file line number Diff line number Diff line change
@@ -1288,79 +1288,6 @@ Repository__pointer__get__(Repository *self)
return PyBytes_FromStringAndSize((char *) &self->repo, sizeof(git_repository *));
}

PyDoc_STRVAR(Repository_checkout_head__doc__,
"checkout_head(strategy)\n"
"\n"
"Checkout the head using the given strategy.");

PyObject *
Repository_checkout_head(Repository *self, PyObject *args)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
unsigned int strategy;
int err;

if (!PyArg_ParseTuple(args, "I", &strategy))
return NULL;

opts.checkout_strategy = strategy;
err = git_checkout_head(self->repo, &opts);
if (err < 0)
return Error_set(err);

Py_RETURN_NONE;
}


PyDoc_STRVAR(Repository_checkout_index__doc__,
"checkout_index(strategy)\n"
"\n"
"Checkout the index using the given strategy.");

PyObject *
Repository_checkout_index(Repository *self, PyObject *args)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
unsigned int strategy;
int err;

if (!PyArg_ParseTuple(args, "I", &strategy))
return NULL;

opts.checkout_strategy = strategy;
err = git_checkout_index(self->repo, NULL, &opts);
if (err < 0)
return Error_set(err);

Py_RETURN_NONE;
}


PyDoc_STRVAR(Repository_checkout_tree__doc__,
"checkout_tree(treeish, strategy)\n"
"\n"
"Checkout the given tree, commit or tag, using the given strategy.");

PyObject *
Repository_checkout_tree(Repository *self, PyObject *args)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
unsigned int strategy;
Object *py_object;
int err;

if (!PyArg_ParseTuple(args, "O!I", &ObjectType, &py_object, &strategy))
return NULL;

opts.checkout_strategy = strategy;
err = git_checkout_tree(self->repo, py_object->obj, &opts);
if (err < 0)
return Error_set(err);

Py_RETURN_NONE;
}


PyDoc_STRVAR(Repository_notes__doc__, "");

PyObject *
@@ -1570,9 +1497,6 @@ PyMethodDef Repository_methods[] = {
METHOD(Repository, revparse_single, METH_O),
METHOD(Repository, status, METH_NOARGS),
METHOD(Repository, status_file, METH_O),
METHOD(Repository, checkout_head, METH_VARARGS),
METHOD(Repository, checkout_index, METH_VARARGS),
METHOD(Repository, checkout_tree, METH_VARARGS),
METHOD(Repository, notes, METH_VARARGS),
METHOD(Repository, create_note, METH_VARARGS),
METHOD(Repository, lookup_note, METH_VARARGS),
4 changes: 2 additions & 2 deletions test/test_repository.py
View file
Original file line number Diff line number Diff line change
@@ -213,7 +213,7 @@ def test_checkout_ref(self):
head = self.repo.head
head = self.repo[head.target]
self.assertTrue('new' not in head.tree)
self.repo.checkout(ref_i18n, pygit2.GIT_CHECKOUT_FORCE)
self.repo.checkout(ref_i18n, strategy=pygit2.GIT_CHECKOUT_FORCE)

head = self.repo.head
head = self.repo[head.target]
@@ -243,7 +243,7 @@ def test_checkout_head(self):
self.assertTrue('bye.txt' in self.repo.status())

# checkout from head will reset index as well
self.repo.checkout('HEAD', pygit2.GIT_CHECKOUT_FORCE)
self.repo.checkout('HEAD', strategy=pygit2.GIT_CHECKOUT_FORCE)
self.assertTrue('bye.txt' not in self.repo.status())

def test_merge_base(self):