openstack/deb-python-pygit2
Code Issues Proposed changes

submodule: reimplement with Python CFFI.

Browse Source 
The submodule type has been implemented as a C type. When opening
a submodule's repository this leads to the bug that instead of an
actual pygit2.Repository being instantiated we only create an
object of the C Repository type.

As this is not trivially fixed within the C code, reimplement the
submodule type as a Python interface with CFFI. As submodules
provide no functionality that is usually accessed repeatedly the
code paths should not prove performance critical. In addition,
maintainability is improved by this reimplementation.
This commit is contained in:
Patrick Steinhardt 2015-04-13 10:21:57 +02:00
parent cd7e2b21be
commit f923e20f2d
9 changed files with 104 additions and 271 deletions

4
docs/submodule.rst

@ -11,7 +11,9 @@ dedicated subdirectory of the repositories tree.
The Submodule type
====================
.. automethod:: pygit2.Submodule.open
.. autoattribute:: pygit2.Submodule.name
.. autoattribute:: pygit2.Submodule.path
.. autoattribute:: pygit2.Submodule.url
.. automethod:: pygit2.Submodule.open
.. autoattribute:: pygit2.Submodule.branch

13
pygit2/decl.h

@ -1,4 +1,5 @@
typedef ... git_repository;
typedef ... git_submodule;
typedef ... git_remote;
typedef ... git_refspec;
typedef ... git_cred;
@ -537,6 +538,18 @@ int git_repository_set_head(git_repository *repo, const char *refname, const git
int git_repository_set_head_detached(git_repository *repo, const git_oid *commitish, const git_signature *signature, const char *log_message);
int git_graph_ahead_behind(size_t *ahead, size_t *behind, git_repository *repo, const git_oid *local, const git_oid *upstream);
/*
* git_submodule
*/
int git_submodule_lookup(git_submodule **out, git_repository *repo, char *path);
void git_submodule_free(git_submodule *subm);
int git_submodule_open(git_repository **out, git_submodule *subm);
const char *git_submodule_name(git_submodule *subm);
const char *git_submodule_path(git_submodule *subm);
const char *git_submodule_url(git_submodule *subm);
const char *git_submodule_branch(git_submodule *subm);
/*
* git_index
*/

9
pygit2/repository.py

@ -51,6 +51,7 @@ from .index import Index
from .remote import RemoteCollection
from .blame import Blame
from .utils import to_bytes, is_string
from .submodule import Submodule
class Repository(_Repository):
@ -77,6 +78,14 @@ class Repository(_Repository):
ffi.buffer(repo_cptr)[:] = self._pointer[:]
self._repo = repo_cptr[0]
def lookup_submodule(self, path):
csub = ffi.new('git_submodule **')
cpath = ffi.new('char[]', to_bytes(path))
err = C.git_submodule_lookup(csub, self._repo, cpath)
check_error(err)
return Submodule._from_c(self, csub[0])
#
# Mapping interface
#

79
pygit2/submodule.py Normal file

@ -0,0 +1,79 @@
# -*- coding: utf-8 -*-
#
# Copyright 2010-2014 The pygit2 contributors
#
# This file is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License, version 2,
# as published by the Free Software Foundation.
#
# In addition to the permissions in the GNU General Public License,
# the authors give you unlimited permission to link the compiled
# version of this file into combinations with other programs,
# and to distribute those combinations without any restriction
# coming from the use of this file. (The General Public License
# restrictions do apply in other respects; for example, they cover
# modification of the file, and distribution when not linked into
# a combined executable.)
#
# This file is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; see the file COPYING. If not, write to
# the Free Software Foundation, 51 Franklin Street, Fifth Floor,
# Boston, MA 02110-1301, USA.
# Import from the future
from __future__ import absolute_import, unicode_literals
from .errors import check_error
from .ffi import ffi, C
class Submodule(object):
@classmethod
def _from_c(cls, repo, cptr):
subm = cls.__new__(cls)
subm._repo = repo
subm._subm = cptr
return subm
def __del__(self):
C.git_submodule_free(self._subm)
def open(self):
"""Open the repository for a submodule."""
crepo = ffi.new('git_repository **')
err = C.git_submodule_open(crepo, self._subm)
check_error(err)
return self._repo._from_c(crepo[0], True)
@property
def name(self):
"""Name of the submodule."""
name = C.git_submodule_name(self._subm)
return ffi.string(name).decode('utf-8')
@property
def path(self):
"""Path of the submodule."""
path = C.git_submodule_path(self._subm)
return ffi.string(path).decode('utf-8')
@property
def url(self):
"""URL of the submodule."""
url = C.git_submodule_url(self._subm)
return ffi.string(url).decode('utf-8')
@property
def branch(self):
"""Branch that is to be tracked by the submodule."""
branch = C.git_submodule_branch(self._subm)
return ffi.string(branch).decode('utf-8')

7
src/pygit2.c

@ -42,7 +42,6 @@ extern PyTypeObject RepositoryType;
extern PyTypeObject OidType;
extern PyTypeObject ObjectType;
extern PyTypeObject CommitType;
extern PyTypeObject SubmoduleType;
extern PyTypeObject DiffType;
extern PyTypeObject DiffIterType;
extern PyTypeObject DiffDeltaType;
@ -220,12 +219,6 @@ moduleinit(PyObject* m)
ADD_CONSTANT_INT(m, GIT_FILEMODE_LINK)
ADD_CONSTANT_INT(m, GIT_FILEMODE_COMMIT)
/*
* Submodules
*/
INIT_TYPE(SubmoduleType, NULL, NULL);
ADD_TYPE(m, Submodule);
/*
* Log
*/

29
src/repository.c

@ -37,7 +37,6 @@
#include "repository.h"
#include "branch.h"
#include "signature.h"
#include "submodule.h"
#include <git2/odb_backend.h>
extern PyObject *GitError;
@ -1100,33 +1099,6 @@ error:
return NULL;
}
PyDoc_STRVAR(Repository_lookup_submodule__doc__,
"lookup_submodule(path) -> Submodule\n"
"\n"
"Lookup a submodule by its path in a repository.");
PyObject *
Repository_lookup_submodule(Repository *self, PyObject *py_path)
{
git_submodule *c_submodule;
char *c_name;
int err;
c_name = py_path_to_c_str(py_path);
if (c_name == NULL)
return NULL;
err = git_submodule_lookup(&c_submodule, self->repo, c_name);
if (err < 0) {
PyObject *err_obj = Error_set_str(err, c_name);
free(c_name);
return err_obj;
}
free(c_name);
return wrap_submodule(self, c_submodule);
}
PyDoc_STRVAR(Repository_listall_submodules__doc__,
"listall_submodules() -> [str, ...]\n"
"\n"
@ -1583,7 +1555,6 @@ PyMethodDef Repository_methods[] = {
METHOD(Repository, create_reference_direct, METH_VARARGS),
METHOD(Repository, create_reference_symbolic, METH_VARARGS),
METHOD(Repository, listall_references, METH_NOARGS),
METHOD(Repository, lookup_submodule, METH_O),
METHOD(Repository, listall_submodules, METH_NOARGS),
METHOD(Repository, lookup_reference, METH_O),
METHOD(Repository, revparse_single, METH_O),

189
src/submodule.c

@ -1,189 +0,0 @@
/*
* Copyright 2010-2015 The pygit2 contributors
*
* This file is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License, version 2,
* as published by the Free Software Foundation.
*
* In addition to the permissions in the GNU General Public License,
* the authors give you unlimited permission to link the compiled
* version of this file into combinations with other programs,
* and to distribute those combinations without any restriction
* coming from the use of this file. (The General Public License
* restrictions do apply in other respects; for example, they cover
* modification of the file, and distribution when not linked into
* a combined executable.)
*
* This file is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; see the file COPYING. If not, write to
* the Free Software Foundation, 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/
#define PY_SSIZE_T_CLEAN
#include <Python.h>
#include <structmember.h>
#include "error.h"
#include "utils.h"
#include "types.h"
#include "submodule.h"
#include "repository.h"
PyTypeObject SubmoduleType;
PyDoc_STRVAR(Submodule_open__doc__,
"open() -> Repository\n"
"\n"
"Open the submodule as repository.");
PyObject *
Submodule_open(Submodule *self, PyObject *args)
{
int err;
git_repository *repo;
err = git_submodule_open(&repo, self->submodule);
if (err < 0)
return Error_set_str(err, giterr_last()->message);
return wrap_repository(repo);
}
PyDoc_STRVAR(Submodule_name__doc__,
"Gets name of the submodule\n");
PyObject *
Submodule_name__get__(Submodule *self)
{
return to_unicode(git_submodule_name(self->submodule), NULL, NULL);
}
PyDoc_STRVAR(Submodule_path__doc__,
"Gets path of the submodule\n");
PyObject *
Submodule_path__get__(Submodule *self)
{
const char *path = git_submodule_path(self->submodule);
assert(path);
return to_unicode(path, NULL, NULL);
}
PyDoc_STRVAR(Submodule_url__doc__,
"Gets URL of the submodule\n");
PyObject *
Submodule_url__get__(Submodule *self)
{
const char *url = git_submodule_url(self->submodule);
if (url == NULL)
Py_RETURN_NONE;
return to_unicode(url, NULL, NULL);
}
PyDoc_STRVAR(Submodule_branch__doc__,
"Gets branch of the submodule\n");
PyObject *
Submodule_branch__get__(Submodule *self)
{
const char *branch = git_submodule_branch(self->submodule);
if (branch == NULL)
Py_RETURN_NONE;
return to_unicode(branch, NULL, NULL);
}
PyObject *
Submodule_repr(PyObject *self)
{
Submodule *subm = (Submodule *)self;
return PyString_FromFormat("pygit2.Submodule(\"%s\")",
git_submodule_name(subm->submodule));
}
static void
Submodule_dealloc(Submodule *self)
{
Py_CLEAR(self->repo);
git_submodule_free(self->submodule);
PyObject_Del(self);
}
PyMethodDef Submodule_methods[] = {
METHOD(Submodule, open, METH_NOARGS),
{NULL}
};
PyGetSetDef Submodule_getseters[] = {
GETTER(Submodule, name),
GETTER(Submodule, path),
GETTER(Submodule, url),
GETTER(Submodule, branch),
{NULL}
};
PyDoc_STRVAR(Submodule__doc__, "Submodule object.");
PyTypeObject SubmoduleType = {
PyVarObject_HEAD_INIT(NULL, 0)
"_pygit2.Submodule", /* tp_name */
sizeof(Submodule), /* tp_basicsize */
0, /* tp_itemsize */
(destructor)Submodule_dealloc, /* tp_dealloc */
0, /* tp_print */
0, /* tp_getattr */
0, /* tp_setattr */
0, /* tp_compare */
Submodule_repr, /* tp_repr */
0, /* tp_as_number */
0, /* tp_as_sequence */
0, /* tp_as_mapping */
0, /* tp_hash */
0, /* tp_call */
0, /* tp_str */
0, /* tp_getattro */
0, /* tp_setattro */
0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT, /* tp_flags */
Submodule__doc__, /* tp_doc */
0, /* tp_traverse */
0, /* tp_clear */
0, /* tp_richcompare */
0, /* tp_weaklistoffset */
0, /* tp_iter */
0, /* tp_iternext */
Submodule_methods, /* tp_methods */
0, /* tp_members */
Submodule_getseters, /* tp_getset */
0, /* tp_base */
0, /* tp_dict */
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
0, /* tp_init */
0, /* tp_alloc */
0, /* tp_new */
};
PyObject *
wrap_submodule(Repository *repo, git_submodule *c_submodule)
{
Submodule *py_submodule = NULL;
py_submodule = PyObject_New(Submodule, &SubmoduleType);
if (py_submodule) {
py_submodule->submodule = c_submodule;
py_submodule->repo = repo;
if (repo) {
Py_INCREF(repo);
}
}
return (PyObject *)py_submodule;
}

38
src/submodule.h

@ -1,38 +0,0 @@
/*
* Copyright 2010-2015 The pygit2 contributors
*
* This file is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License, version 2,
* as published by the Free Software Foundation.
*
* In addition to the permissions in the GNU General Public License,
* the authors give you unlimited permission to link the compiled
* version of this file into combinations with other programs,
* and to distribute those combinations without any restriction
* coming from the use of this file. (The General Public License
* restrictions do apply in other respects; for example, they cover
* modification of the file, and distribution when not linked into
* a combined executable.)
*
* This file is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; see the file COPYING. If not, write to
* the Free Software Foundation, 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/
#ifndef INCLUDE_pygit2_submodule_h
#define INCLUDE_pygit2_submodule_h
#define PY_SSIZE_T_CLEAN
#include <Python.h>
#include <git2.h>
PyObject *wrap_submodule(Repository* repo, git_submodule *submodule);
#endif

7
src/types.h

@ -75,13 +75,6 @@ SIMPLE_TYPE(Tree, git_tree, tree)
SIMPLE_TYPE(Blob, git_blob, blob)
SIMPLE_TYPE(Tag, git_tag, tag)
/* git_submodule */
typedef struct {
PyObject_HEAD
Repository *repo;
git_submodule *submodule;
} Submodule;
/* git_note */
typedef struct {
PyObject_HEAD