openstack/deb-python-pygit2
Code Issues Proposed changes

Do not use hasattr, not robust

Browse Source 
In Python versions older than 3.2 hasattr is not robust as it masks real
errors. And usually is not efficient as a call to hasattr is often
followed by another call to getattr. It is best to avoid using it
completely.

See https://docs.python.org/3/whatsnew/3.2.html#other-language-changes
This commit is contained in:
J. David Ibáñez
2015-10-11 11:58:07 +02:00
parent 9db8737364
commit f5aa1829ac
3 changed files with 45 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
pygit2/index.py
View File

@@ -48,6 +48,7 @@ class Index(object):
err = C.git_index_open(cindex, to_bytes(path)) err = C.git_index_open(cindex, to_bytes(path))
check_error(err) check_error(err)
self._repo = None
self._index = cindex[0] self._index = cindex[0]
self._cindex = cindex self._cindex = cindex
@@ -125,14 +126,15 @@ class Index(object):
The tree will be read recursively and all its children will also be The tree will be read recursively and all its children will also be
inserted into the Index. inserted into the Index.
""" """
repo = self._repo
if is_string(tree): if is_string(tree):
tree = self._repo[tree] tree = repo[tree]
if isinstance(tree, Oid): if isinstance(tree, Oid):
if not hasattr(self, '_repo'): if repo is None:
raise TypeError("id given but no associated repository") raise TypeError("id given but no associated repository")
tree = self._repo[tree] tree = repo[tree]
elif not isinstance(tree, Tree): elif not isinstance(tree, Tree):
raise TypeError("argument must be Oid or Tree") raise TypeError("argument must be Oid or Tree")
@@ -214,7 +216,8 @@ class Index(object):
interhunk_lines: the maximum number of unchanged lines between hunk interhunk_lines: the maximum number of unchanged lines between hunk
boundaries before the hunks will be merged into a one boundaries before the hunks will be merged into a one
""" """
if not hasattr(self, '_repo'): repo = self._repo
if repo is None:
raise ValueError('diff needs an associated repository') raise ValueError('diff needs an associated repository')
copts = ffi.new('git_diff_options *') copts = ffi.new('git_diff_options *')
@@ -226,11 +229,11 @@ class Index(object):
copts.interhunk_lines = interhunk_lines copts.interhunk_lines = interhunk_lines
cdiff = ffi.new('git_diff **') cdiff = ffi.new('git_diff **')
err = C.git_diff_index_to_workdir(cdiff, self._repo._repo, err = C.git_diff_index_to_workdir(cdiff, repo._repo, self._index,
self._index, copts) copts)
check_error(err) check_error(err)
return Diff.from_c(bytes(ffi.buffer(cdiff)[:]), self._repo) return Diff.from_c(bytes(ffi.buffer(cdiff)[:]), repo)
def diff_to_tree(self, tree, flags=0, context_lines=3, interhunk_lines=0): def diff_to_tree(self, tree, flags=0, context_lines=3, interhunk_lines=0):
"""Diff the index against a tree. Return a <Diff> object with the """Diff the index against a tree. Return a <Diff> object with the
@@ -248,8 +251,8 @@ class Index(object):
interhunk_lines: the maximum number of unchanged lines between hunk interhunk_lines: the maximum number of unchanged lines between hunk
boundaries before the hunks will be merged into a one. boundaries before the hunks will be merged into a one.
""" """
repo = self._repo
if not hasattr(self, '_repo'): if repo is None:
raise ValueError('diff needs an associated repository') raise ValueError('diff needs an associated repository')
if not isinstance(tree, Tree): if not isinstance(tree, Tree):
@@ -267,11 +270,11 @@ class Index(object):
ffi.buffer(ctree)[:] = tree._pointer[:] ffi.buffer(ctree)[:] = tree._pointer[:]
cdiff = ffi.new('git_diff **') cdiff = ffi.new('git_diff **')
err = C.git_diff_tree_to_index(cdiff, self._repo._repo, ctree[0], err = C.git_diff_tree_to_index(cdiff, repo._repo, ctree[0],
self._index, copts) self._index, copts)
check_error(err) check_error(err)
return Diff.from_c(bytes(ffi.buffer(cdiff)[:]), self._repo) return Diff.from_c(bytes(ffi.buffer(cdiff)[:]), repo)
# #

12
pygit2/libgit2_build.py
View File

@@ -31,7 +31,6 @@ pygit2 at run-time.
""" """
# Import from the Standard Library # Import from the Standard Library
from binascii import crc32
import codecs import codecs
import os import os
from os import getenv from os import getenv
@@ -75,9 +74,8 @@ ffi = cffi.FFI()
# Load C definitions # Load C definitions
if getattr(sys, 'frozen', False): if getattr(sys, 'frozen', False):
if hasattr(sys, '_MEIPASS'): dir_path = getattr(sys, '_MEIPASS', None)
dir_path = sys._MEIPASS if dir_path is None:
else:
dir_path = dirname(abspath(sys.executable)) dir_path = dirname(abspath(sys.executable))
else: else:
dir_path = dirname(abspath(__file__)) dir_path = dirname(abspath(__file__))
@@ -94,10 +92,10 @@ C_KEYWORDS = dict(libraries=['git2'],
# The modulename # The modulename
# Simplified version of what cffi does: remove kwargs and vengine # Simplified version of what cffi does: remove kwargs and vengine
set_source = getattr(ffi, 'set_source', None)
if set_source is not None:
preamble = "#include <git2.h>" preamble = "#include <git2.h>"
set_source("pygit2._libgit2", preamble, **C_KEYWORDS)
if hasattr(ffi, 'set_source'):
ffi.set_source("pygit2._libgit2", preamble, **C_KEYWORDS)
ffi.cdef(C_HEADER_SRC) ffi.cdef(C_HEADER_SRC)

47
pygit2/remote.py
View File

@@ -30,9 +30,8 @@ from __future__ import absolute_import
# Import from pygit2 # Import from pygit2
from _pygit2 import Oid from _pygit2 import Oid
from .errors import check_error, GitError, Passthrough from .errors import check_error, Passthrough
from .ffi import ffi, C from .ffi import ffi, C
from .credentials import KeypairFromAgent
from .refspec import Refspec from .refspec import Refspec
from .utils import to_bytes, strarray_to_strings, StrArray from .utils import to_bytes, strarray_to_strings, StrArray
@@ -100,7 +99,6 @@ class RemoteCallbacks(object):
:param str string: Progress output from the remote :param str string: Progress output from the remote
""" """
pass
def credentials(self, url, username_from_url, allowed_types): def credentials(self, url, username_from_url, allowed_types):
"""Credentials callback """Credentials callback
@@ -148,7 +146,6 @@ class RemoteCallbacks(object):
:param TransferProgress stats: The progress up to now :param TransferProgress stats: The progress up to now
""" """
pass
def update_tips(self, refname, old, new): def update_tips(self, refname, old, new):
"""Update tips callabck """Update tips callabck
@@ -200,12 +197,12 @@ class RemoteCallbacks(object):
def _transfer_progress_cb(stats_ptr, data): def _transfer_progress_cb(stats_ptr, data):
self = ffi.from_handle(data) self = ffi.from_handle(data)
if not hasattr(self, 'transfer_progress') \ transfer_progress = getattr(self, 'transfer_progress', None)
or not self.transfer_progress: if not transfer_progress:
return 0 return 0
try: try:
self.transfer_progress(TransferProgress(stats_ptr)) transfer_progress(TransferProgress(stats_ptr))
except Exception as e: except Exception as e:
self._stored_exception = e self._stored_exception = e
return C.GIT_EUSER return C.GIT_EUSER
@@ -216,12 +213,13 @@ class RemoteCallbacks(object):
def _sideband_progress_cb(string, length, data): def _sideband_progress_cb(string, length, data):
self = ffi.from_handle(data) self = ffi.from_handle(data)
if not hasattr(self, 'progress') or not self.progress: progress = getattr(self, 'progress', None)
if not progress:
return 0 return 0
try: try:
s = ffi.string(string, length).decode() s = ffi.string(string, length).decode()
self.progress(s) progress(s)
except Exception as e: except Exception as e:
self._stored_exception = e self._stored_exception = e
return C.GIT_EUSER return C.GIT_EUSER
@@ -233,7 +231,8 @@ class RemoteCallbacks(object):
def _update_tips_cb(refname, a, b, data): def _update_tips_cb(refname, a, b, data):
self = ffi.from_handle(data) self = ffi.from_handle(data)
if not hasattr(self, 'update_tips') or not self.update_tips: update_tips = getattr(self, 'update_tips', None)
if not update_tips:
return 0 return 0
try: try:
@@ -241,7 +240,7 @@ class RemoteCallbacks(object):
a = Oid(raw=bytes(ffi.buffer(a)[:])) a = Oid(raw=bytes(ffi.buffer(a)[:]))
b = Oid(raw=bytes(ffi.buffer(b)[:])) b = Oid(raw=bytes(ffi.buffer(b)[:]))
self.update_tips(s, a, b) update_tips(s, a, b)
except Exception as e: except Exception as e:
self._stored_exception = e self._stored_exception = e
return C.GIT_EUSER return C.GIT_EUSER
@@ -252,13 +251,14 @@ class RemoteCallbacks(object):
def _push_update_reference_cb(ref, msg, data): def _push_update_reference_cb(ref, msg, data):
self = ffi.from_handle(data) self = ffi.from_handle(data)
if not hasattr(self, 'push_update_reference') or not self.push_update_reference: push_update_reference = getattr(self, 'push_update_reference', None)
if not push_update_reference:
return 0 return 0
try: try:
refname = ffi.string(ref) refname = ffi.string(ref)
message = maybe_string(msg) message = maybe_string(msg)
self.push_update_reference(refname, message) push_update_reference(refname, message)
except Exception as e: except Exception as e:
self._stored_exception = e self._stored_exception = e
return C.GIT_EUSER return C.GIT_EUSER
@@ -271,11 +271,12 @@ class RemoteCallbacks(object):
def _credentials_cb(cred_out, url, username, allowed, data): def _credentials_cb(cred_out, url, username, allowed, data):
self = ffi.from_handle(data) self = ffi.from_handle(data)
if not hasattr(self, 'credentials') or not self.credentials: credentials = getattr(self, 'credentials', None)
if not credentials:
return 0 return 0
try: try:
ccred = get_credentials(self.credentials, url, username, allowed) ccred = get_credentials(credentials, url, username, allowed)
cred_out[0] = ccred[0] cred_out[0] = ccred[0]
except Exception as e: except Exception as e:
@@ -299,11 +300,12 @@ class RemoteCallbacks(object):
try: try:
is_ssh = cert_i.cert_type == C.GIT_CERT_HOSTKEY_LIBSSH2 is_ssh = cert_i.cert_type == C.GIT_CERT_HOSTKEY_LIBSSH2
if not hasattr(self, 'certificate_check') or not self.certificate_check: certificate_check = getattr(self, 'certificate_check', None)
if not certificate_check:
raise Passthrough raise Passthrough
# python's parsing is deep in the libraries and assumes an OpenSSL-owned cert # python's parsing is deep in the libraries and assumes an OpenSSL-owned cert
val = self.certificate_check(None, bool(valid), ffi.string(host)) val = certificate_check(None, bool(valid), ffi.string(host))
if not val: if not val:
return C.GIT_ECERTIFICATE return C.GIT_ECERTIFICATE
except Exception as e: except Exception as e:
@@ -440,23 +442,24 @@ def get_credentials(fn, url, username, allowed):
creds = fn(url_str, username_str, allowed) creds = fn(url_str, username_str, allowed)
if not hasattr(creds, 'credential_type') \ credential_type = getattr(creds, 'credential_type', None)
or not hasattr(creds, 'credential_tuple'): credential_tuple = getattr(creds, 'credential_tuple', None)
if not credential_type or not credential_tuple:
raise TypeError("credential does not implement interface") raise TypeError("credential does not implement interface")
cred_type = creds.credential_type cred_type = credential_type
if not (allowed & cred_type): if not (allowed & cred_type):
raise TypeError("invalid credential type") raise TypeError("invalid credential type")
ccred = ffi.new('git_cred **') ccred = ffi.new('git_cred **')
if cred_type == C.GIT_CREDTYPE_USERPASS_PLAINTEXT: if cred_type == C.GIT_CREDTYPE_USERPASS_PLAINTEXT:
name, passwd = creds.credential_tuple name, passwd = credential_tuple
err = C.git_cred_userpass_plaintext_new(ccred, to_bytes(name), err = C.git_cred_userpass_plaintext_new(ccred, to_bytes(name),
to_bytes(passwd)) to_bytes(passwd))
elif cred_type == C.GIT_CREDTYPE_SSH_KEY: elif cred_type == C.GIT_CREDTYPE_SSH_KEY:
name, pubkey, privkey, passphrase = creds.credential_tuple name, pubkey, privkey, passphrase = credential_tuple
if pubkey is None and privkey is None: if pubkey is None and privkey is None:
err = C.git_cred_ssh_key_from_agent(ccred, to_bytes(name)) err = C.git_cred_ssh_key_from_agent(ccred, to_bytes(name))
else: else: