Intermediate commit: test_config and test_actor works

Kind of tackling the tasks step by step, picking low-hanging fruit first,
or the ones that everyone depends on

7 files changed, 110 insertions, 63 deletions

diff --git a/git/cmd.py b/git/cmd.py
index aa5e3a25..c536b43c 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -19,7 +19,11 @@ from .util import (
     stream_copy
 )
 from .exc import GitCommandError
-from git.compat import text_type
+from git.compat import (
+    text_type,
+    string_types,
+    defenc
+)
 
 execute_kwargs = ('istream', 'with_keep_cwd', 'with_extended_output',
                   'with_exceptions', 'as_process',
@@ -373,9 +377,9 @@ class Git(LazyMixin):
             if output_stream is None:
                 stdout_value, stderr_value = proc.communicate()
                 # strip trailing "\n"
-                if stdout_value.endswith("\n"):
+                if stdout_value.endswith(b"\n"):
                     stdout_value = stdout_value[:-1]
-                if stderr_value.endswith("\n"):
+                if stderr_value.endswith(b"\n"):
                     stderr_value = stderr_value[:-1]
                 status = proc.returncode
             else:
@@ -394,9 +398,9 @@ class Git(LazyMixin):
         if self.GIT_PYTHON_TRACE == 'full':
             cmdstr = " ".join(command)
             if stderr_value:
-                log.info("%s -> %d; stdout: '%s'; stderr: '%s'", cmdstr, status, stdout_value, stderr_value)
+                log.info("%s -> %d; stdout: '%s'; stderr: '%s'", cmdstr, status, stdout_value.decode(defenc), stderr_value.decode(defenc))
             elif stdout_value:
-                log.info("%s -> %d; stdout: '%s'", cmdstr, status, stdout_value)
+                log.info("%s -> %d; stdout: '%s'", cmdstr, status, stdout_value.decode(defenc))
             else:
                 log.info("%s -> %d", cmdstr, status)
         # END handle debug printing
@@ -436,7 +440,7 @@ class Git(LazyMixin):
     def __unpack_args(cls, arg_list):
         if not isinstance(arg_list, (list, tuple)):
             if isinstance(arg_list, text_type):
-                return [arg_list.encode('utf-8')]
+                return [arg_list.encode(defenc)]
             return [str(arg_list)]
 
         outlist = list()
@@ -444,7 +448,7 @@ class Git(LazyMixin):
             if isinstance(arg_list, (list, tuple)):
                 outlist.extend(cls.__unpack_args(arg))
             elif isinstance(arg_list, text_type):
-                outlist.append(arg_list.encode('utf-8'))
+                outlist.append(arg_list.encode(defenc))
             # END recursion
             else:
                 outlist.append(str(arg))
@@ -569,14 +573,20 @@ class Git(LazyMixin):
             raise ValueError("Failed to parse header: %r" % header_line)
         return (tokens[0], tokens[1], int(tokens[2]))
 
-    def __prepare_ref(self, ref):
-        # required for command to separate refs on stdin
-        refstr = str(ref)               # could be ref-object
-        if refstr.endswith("\n"):
-            return refstr
-        return refstr + "\n"
+    def _prepare_ref(self, ref):
+        # required for command to separate refs on stdin, as bytes
+        refstr = ref
+        if isinstance(ref, bytes):
+            # Assume 40 bytes hexsha - bin-to-ascii for some reason returns bytes, not text
+            refstr = ref.decode('ascii')
+        elif not isinstance(ref, string_types):
+            refstr = str(ref)               # could be ref-object
+
+        if not refstr.endswith("\n"):
+            refstr += "\n"
+        return refstr.encode(defenc)
 
-    def __get_persistent_cmd(self, attr_name, cmd_name, *args, **kwargs):
+    def _get_persistent_cmd(self, attr_name, cmd_name, *args, **kwargs):
         cur_val = getattr(self, attr_name)
         if cur_val is not None:
             return cur_val
@@ -589,7 +599,7 @@ class Git(LazyMixin):
         return cmd
 
     def __get_object_header(self, cmd, ref):
-        cmd.stdin.write(self.__prepare_ref(ref))
+        cmd.stdin.write(self._prepare_ref(ref))
         cmd.stdin.flush()
         return self._parse_object_header(cmd.stdout.readline())
 
@@ -601,7 +611,7 @@ class Git(LazyMixin):
             once and reuses the command in subsequent calls.
 
         :return: (hexsha, type_string, size_as_int)"""
-        cmd = self.__get_persistent_cmd("cat_file_header", "cat_file", batch_check=True)
+        cmd = self._get_persistent_cmd("cat_file_header", "cat_file", batch_check=True)
         return self.__get_object_header(cmd, ref)
 
     def get_object_data(self, ref):
@@ -618,7 +628,7 @@ class Git(LazyMixin):
         :return: (hexsha, type_string, size_as_int, stream)
         :note: This method is not threadsafe, you need one independent  Command instance
             per thread to be safe !"""
-        cmd = self.__get_persistent_cmd("cat_file_all", "cat_file", batch=True)
+        cmd = self._get_persistent_cmd("cat_file_all", "cat_file", batch=True)
         hexsha, typename, size = self.__get_object_header(cmd, ref)
         return (hexsha, typename, size, self.CatFileContentStream(size, cmd.stdout))
 
diff --git a/git/compat.py b/git/compat.py
index a95c5667..4a892ad2 100644
--- a/git/compat.py
+++ b/git/compat.py
@@ -7,6 +7,8 @@
 """utilities to help provide compatibility with python 3"""
 # flake8: noqa
 
+import sys
+
 from gitdb.utils.compat import (
     PY3,
     xrange,
@@ -17,11 +19,39 @@ from gitdb.utils.compat import (
 from gitdb.utils.encoding import (
     string_types,
     text_type,
-    force_bytes
+    force_bytes,
+    force_text
 )
 
+defenc = sys.getdefaultencoding()
 if PY3:
     import io
     FileType = io.IOBase
 else:
     FileType = file
+    # usually, this is just ascii, which might not enough for our encoding needs
+    # Unless it's set specifically, we override it to be utf-8
+    if defenc == 'ascii':
+        defenc = 'utf-8'
+
+
+def with_metaclass(meta, *bases):
+    """copied from https://github.com/Byron/bcore/blob/master/src/python/butility/future.py#L15"""
+    class metaclass(meta):
+        __call__ = type.__call__
+        __init__ = type.__init__
+
+        def __new__(cls, name, nbases, d):
+            if nbases is None:
+                return type.__new__(cls, name, (), d)
+            # There may be clients who rely on this attribute to be set to a reasonable value, which is why
+            # we set the __metaclass__ attribute explicitly
+            if not PY3 and '___metaclass__' not in d:
+                d['__metaclass__'] = meta
+            # end
+            return meta(name, bases, d)
+        # end
+    # end metaclass
+    return metaclass(meta.__name__ + 'Helper', None, {})
+    # end handle py2
+
diff --git a/git/config.py b/git/config.py
index 34fe290b..988547a0 100644
--- a/git/config.py
+++ b/git/config.py
@@ -14,12 +14,15 @@ except ImportError:
     import configparser as cp
 import inspect
 import logging
+import abc
 
 from git.odict import OrderedDict
 from git.util import LockFile
 from git.compat import (
     string_types,
-    FileType
+    FileType,
+    defenc,
+    with_metaclass
 )
 
 __all__ = ('GitConfigParser', 'SectionConstraint')
@@ -28,7 +31,7 @@ __all__ = ('GitConfigParser', 'SectionConstraint')
 log = logging.getLogger('git.config')
 
 
-class MetaParserBuilder(type):
+class MetaParserBuilder(abc.ABCMeta):
 
     """Utlity class wrapping base-class methods into decorators that assure read-only properties"""
     def __new__(metacls, name, bases, clsdict):
@@ -39,7 +42,7 @@ class MetaParserBuilder(type):
         if kmm in clsdict:
             mutating_methods = clsdict[kmm]
             for base in bases:
-                methods = (t for t in inspect.getmembers(base, inspect.ismethod) if not t[0].startswith("_"))
+                methods = (t for t in inspect.getmembers(base, inspect.isroutine) if not t[0].startswith("_"))
                 for name, method in methods:
                     if name in clsdict:
                         continue
@@ -112,7 +115,7 @@ class SectionConstraint(object):
         return self._config
 
 
-class GitConfigParser(cp.RawConfigParser, object):
+class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)):
 
     """Implements specifics required to read git style configuration files.
 
@@ -128,7 +131,6 @@ class GitConfigParser(cp.RawConfigParser, object):
     :note:
         The config is case-sensitive even when queried, hence section and option names
         must match perfectly."""
-    __metaclass__ = MetaParserBuilder
 
     #{ Configuration
     # The lock type determines the type of lock to use in new configuration readers.
@@ -150,7 +152,6 @@ class GitConfigParser(cp.RawConfigParser, object):
 
     # list of RawConfigParser methods able to change the instance
     _mutating_methods_ = ("add_section", "remove_section", "remove_option", "set")
-    __slots__ = ("_sections", "_defaults", "_file_or_files", "_read_only", "_is_initialized", '_lock')
 
     def __init__(self, file_or_files, read_only=True):
         """Initialize a configuration reader to read the given file_or_files and to
@@ -162,12 +163,12 @@ class GitConfigParser(cp.RawConfigParser, object):
         :param read_only:
             If True, the ConfigParser may only read the data , but not change it.
             If False, only a single file path or file object may be given."""
-        super(GitConfigParser, self).__init__()
-        # initialize base with ordered dictionaries to be sure we write the same
-        # file back
-        self._sections = OrderedDict()
-        self._defaults = OrderedDict()
+        cp.RawConfigParser.__init__(self, dict_type=OrderedDict)
 
+        # Used in python 3, needs to stay in sync with sections for underlying implementation to work
+        if not hasattr(self, '_proxies'):
+            self._proxies = self._dict()
+        
         self._file_or_files = file_or_files
         self._read_only = read_only
         self._is_initialized = False
@@ -222,7 +223,8 @@ class GitConfigParser(cp.RawConfigParser, object):
         lineno = 0
         e = None                                  # None, or an exception
         while True:
-            line = fp.readline()
+            # we assume to read binary !
+            line = fp.readline().decode(defenc)
             if not line:
                 break
             lineno = lineno + 1
@@ -242,9 +244,9 @@ class GitConfigParser(cp.RawConfigParser, object):
                     elif sectname == cp.DEFAULTSECT:
                         cursect = self._defaults
                     else:
-                        # THE ONLY LINE WE CHANGED !
-                        cursect = OrderedDict((('__name__', sectname),))
+                        cursect = self._dict((('__name__', sectname),))
                         self._sections[sectname] = cursect
+                        self._proxies[sectname] = None
                     # So sections can't start with a continuation line
                     optname = None
                 # no section header in the file?
@@ -295,7 +297,7 @@ class GitConfigParser(cp.RawConfigParser, object):
             # assume a path if it is not a file-object
             if not hasattr(file_object, "seek"):
                 try:
-                    fp = open(file_object)
+                    fp = open(file_object, 'rb')
                     close_fp = True
                 except IOError:
                     continue
@@ -314,16 +316,17 @@ class GitConfigParser(cp.RawConfigParser, object):
         """Write an .ini-format representation of the configuration state in
         git compatible format"""
         def write_section(name, section_dict):
-            fp.write("[%s]\n" % name)
+            fp.write(("[%s]\n" % name).encode(defenc))
             for (key, value) in section_dict.items():
                 if key != "__name__":
-                    fp.write("\t%s = %s\n" % (key, str(value).replace('\n', '\n\t')))
+                    fp.write(("\t%s = %s\n" % (key, str(value).replace('\n', '\n\t'))).encode(defenc))
                 # END if key is not __name__
         # END section writing
 
         if self._defaults:
             write_section(cp.DEFAULTSECT, self._defaults)
-        map(lambda t: write_section(t[0], t[1]), self._sections.items())
+        for name, value in self._sections.items():
+            write_section(name, value)
 
     @needs_values
     def write(self):
@@ -371,8 +374,7 @@ class GitConfigParser(cp.RawConfigParser, object):
     @set_dirty_and_flush_changes
     def add_section(self, section):
         """Assures added options will stay in order"""
-        super(GitConfigParser, self).add_section(section)
-        self._sections[section] = OrderedDict()
+        return super(GitConfigParser, self).add_section(section)
 
     @property
     def read_only(self):
diff --git a/git/objects/util.py b/git/objects/util.py
index fdf9622b..cefef862 100644
--- a/git/objects/util.py
+++ b/git/objects/util.py
@@ -46,17 +46,17 @@ def get_object_type_by_name(object_type_name):
     :param object_type_name: Member of TYPES
 
     :raise ValueError: In case object_type_name is unknown"""
-    if object_type_name == "commit":
-        import commit
+    if object_type_name == b"commit":
+        from . import commit
         return commit.Commit
-    elif object_type_name == "tag":
-        import tag
+    elif object_type_name == b"tag":
+        from . import tag
         return tag.TagObject
-    elif object_type_name == "blob":
-        import blob
+    elif object_type_name == b"blob":
+        from . import blob
         return blob.Blob
-    elif object_type_name == "tree":
-        import tree
+    elif object_type_name == b"tree":
+        from . import tree
         return tree.Tree
     else:
         raise ValueError("Cannot handle unknown object type: %s" % object_type_name)
diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py
index 624b1a09..1ac9ac65 100644
--- a/git/refs/symbolic.py
+++ b/git/refs/symbolic.py
@@ -19,7 +19,9 @@ from gitdb.util import (
     hex_to_bin,
     LockedFD
 )
-from git.compat import string_types
+from git.compat import (
+    string_types,
+)
 
 from .log import RefLog
 
@@ -79,10 +81,10 @@ class SymbolicReference(object):
 
     @classmethod
     def _iter_packed_refs(cls, repo):
-        """Returns an iterator yielding pairs of sha1/path pairs for the corresponding refs.
+        """Returns an iterator yielding pairs of sha1/path pairs (as bytes) for the corresponding refs.
         :note: The packed refs file will be kept open as long as we iterate"""
         try:
-            fp = open(cls._get_packed_refs_path(repo), 'rb')
+            fp = open(cls._get_packed_refs_path(repo), 'rt')
             for line in fp:
                 line = line.strip()
                 if not line:
@@ -123,12 +125,12 @@ class SymbolicReference(object):
 
     @classmethod
     def _get_ref_info(cls, repo, ref_path):
-        """Return: (sha, target_ref_path) if available, the sha the file at
+        """Return: (str(sha), str(target_ref_path)) if available, the sha the file at
         rela_path points to, or None. target_ref_path is the reference we
         point to, or None"""
         tokens = None
         try:
-            fp = open(join(repo.git_dir, ref_path), 'r')
+            fp = open(join(repo.git_dir, ref_path), 'rt')
             value = fp.read().rstrip()
             fp.close()
             # Don't only split on spaces, but on whitespace, which allows to parse lines like
@@ -141,7 +143,8 @@ class SymbolicReference(object):
             for sha, path in cls._iter_packed_refs(repo):
                 if path != ref_path:
                     continue
-                tokens = (sha, path)
+                # sha will be used as 
+                tokens = sha, path
                 break
             # END for each packed ref
         # END handle packed refs
diff --git a/git/remote.py b/git/remote.py
index 9ebc52fe..63f21c4e 100644
--- a/git/remote.py
+++ b/git/remote.py
@@ -32,6 +32,7 @@ from git.util import (
     finalize_process
 )
 from gitdb.util import join
+from git.compat import defenc
 
 
 __all__ = ('RemoteProgress', 'PushInfo', 'FetchInfo', 'Remote')
@@ -46,16 +47,16 @@ def digest_process_messages(fh, progress):
     :param fh: File handle to read from
     :return: list(line, ...) list of lines without linebreaks that did
         not contain progress information"""
-    line_so_far = ''
+    line_so_far = b''
     dropped_lines = list()
     while True:
-        char = fh.read(1)
+        char = fh.read(1)       # reads individual single byte strings
         if not char:
             break
 
-        if char in ('\r', '\n') and line_so_far:
-            dropped_lines.extend(progress._parse_progress_line(line_so_far))
-            line_so_far = ''
+        if char in (b'\r', b'\n') and line_so_far:
+            dropped_lines.extend(progress._parse_progress_line(line_so_far.decode(defenc)))
+            line_so_far = b''
         else:
             line_so_far += char
         # END process parsed line
diff --git a/git/test/test_config.py b/git/test/test_config.py
index 0301c54f..f02754d5 100644
--- a/git/test/test_config.py
+++ b/git/test/test_config.py
@@ -11,16 +11,17 @@ from git.test.lib import (
 from git import (
     GitConfigParser
 )
-from git.compat import string_types
+from git.compat import (
+    string_types,
+)
 import io
 from copy import copy
-from ConfigParser import NoSectionError
-
+from git.config import cp
 
 class TestBase(TestCase):
 
     def _to_memcache(self, file_path):
-        fp = open(file_path, "r")
+        fp = open(file_path, "rb")
         sio = io.BytesIO(fp.read())
         sio.name = file_path
         return sio
@@ -39,7 +40,7 @@ class TestBase(TestCase):
             w_config.write()                # enforce writing
 
             # we stripped lines when reading, so the results differ
-            assert file_obj.getvalue() != file_obj_orig.getvalue()
+            assert file_obj.getvalue() and file_obj.getvalue() != file_obj_orig.getvalue()
 
             # creating an additional config writer must fail due to exclusive access
             self.failUnlessRaises(IOError, GitConfigParser, file_obj, read_only=False)
@@ -105,4 +106,4 @@ class TestBase(TestCase):
         assert r_config.get_value("doesnt", "exist", default) == default
 
         # it raises if there is no default though
-        self.failUnlessRaises(NoSectionError, r_config.get_value, "doesnt", "exist")
+        self.failUnlessRaises(cp.NoSectionError, r_config.get_value, "doesnt", "exist")