Drop Python2 support.

In Python3, if there are filenames encoded with 8-bit encodings, there
might be an issues with converting them into unicode objects. This is a
workaround on this subject. Python2 is not affected.

Other than that, there was tests added to cover this case, appropriate
Makefile which automate creating venvs for both: Python 2 and 3, and
also there is a check against pep8 rules using flake8.

.gitignore

@@ -0,0 +1,3 @@
+.test
+__pycache__
+adbfsc

Makefile

@@ -0,0 +1,55 @@
+# simple makefile for running tests for the adbfs plugin
+
+all: test_dir py2 py3 flake8
+
+TEST_DIR='.test'
+PY2_VENV=$(TEST_DIR)/py2
+PY3_VENV=$(TEST_DIR)/py3
+FL8_VENV=$(TEST_DIR)/flake8
+TST_EXISTS=$(shell [ -e $(TEST_DIR) ] && echo 1 || echo 0)
+PY2_EXISTS=$(shell [ -e $(PY2_VENV) ] && echo 1 || echo 0)
+PY3_EXISTS=$(shell [ -e $(PY3_VENV) ] && echo 1 || echo 0)
+FL8_EXISTS=$(shell [ -e $(FL8_VENV) ] && echo 1 || echo 0)
+
+py3: test_dir virtualenv3
+	.test/py3/bin/python test_adbfs.py
+
+py2: test_dir virtualenv2
+	.test/py2/bin/python test_adbfs.py
+
+flake8: test_dir virtualenv_flake8
+	.test/flake8/bin/flake8 adbfs test_adbfs.py
+
+ifeq ($(TST_EXISTS), 0)
+test_dir:
+	mkdir -p .test
+else
+test_dir:
+endif
+
+ifeq ($(PY3_EXISTS), 0)
+virtualenv3:
+	virtualenv -p python3 $(PY3_VENV)
+	$(PY3_VENV)/bin/pip install six
+else
+virtualenv3:
+endif
+
+ifeq ($(PY2_EXISTS), 0)
+virtualenv2:
+	virtualenv -p python2 $(PY2_VENV)
+	$(PY2_VENV)/bin/pip install mock
+else
+virtualenv2:
+endif
+
+ifeq ($(FL8_EXISTS), 0)
+virtualenv_flake8:
+	virtualenv -p python2 $(FL8_VENV)
+	$(FL8_VENV)/bin/pip install flake8
+else
+virtualenv_flake8:
+endif
+
+clean:
+	rm -fr $(TEST_DIR) __pycache__ adbfsc

README.rst

@@ -5,10 +5,11 @@ Midnight Commander adbfs external fs plugin
 This is Midnight Commander extfs plugin for browsing Android device through
 ``adb`` interface written in Python.
 
+
 Rquirements
 ===========
 
-* Python 2.7 or 3.x (tested on 3.5.4)
+* Python 3.x (tested on 3.5.4, 3.6 and 3.7)
 * ``adb`` installed and in ``$PATH`` or provided via the config file
 * An Android device or emulator preferably rooted
 * ``busybox`` (``toolbox``, ``toybox``) installed and available in the path on
@@ -24,6 +25,7 @@ Make sure, that issuing from command line:
 
 it should display files from root directory on the device.
 
+
 Features
 ========
 
@@ -35,12 +37,14 @@ Features
 * Symbolic links in lists are corrected to be relative to the file system
 * Symbolic links also point to the right target, skipping intermediate links
 
+
 Installation
 ============
 
 Copy adbfs into ``~/.local/share/mc/extfs.d/`` directory and make it executable
 if needed.
 
+
 Usage
 =====
 
@@ -48,13 +52,14 @@ To use it, just issue:
 
 .. code:: shell-session
 
-   cd adbfs://
+   $ cd adbfs://
 
 under MC - after some time you should see the files and directories on your
-device. For convenience you can add a bookmark (accessible under CTRL+\) for
+device. For convenience you can add a bookmark (accessible under CTRL+\\) for
 fast access. The time is depended on how many files and directories you have on
 your device and how fast it is :)
 
+
 Configuration
 =============
 
@@ -66,14 +71,12 @@ You can configure behaviour of this plugin using ``.ini`` file located under
 
    [adbfs]
    debug = false
-   skip_dirs = true
    dirs_to_skip = ["acct", "charger", "d", "dev", "proc", "sys"]
    suppress_colors = false
    root =
    adb_command = adb
    adb_connect =
 
-
 where:
 
 * ``debug`` will provide a little bit more verbose information, useful for
@@ -87,12 +90,54 @@ where:
 * ``root`` root directory to read. Everything outside of that directory will be
   omitted. That would be the fastest way to access certain location on the
   device. Note, that ``dirs_to_skip`` still apply inside this directory.
-* ``adb_command`` absolute or relative path to ``adb`` command. `~/` or
+* ``adb_command`` absolute or relative path to ``adb`` command. ``~/`` or
   environment variables are allowed.
 * ``adb_connect`` specifies if connection to specific device needs to be
   performed before accessing shell. It is useful for *adb over network*
   feature. Typical value here is a device IP address with optional port, which
   defaults to 5555.
+* ``try_su`` specifies whether or not to try to detect if ``su`` command is
+  available and usable.
+
+
+Contribution
+============
+
+There is a ``Makefile`` in the top directory, which is basic helper for running
+the tests. Please use it, and adapt/add tests for provided fixes/functionality.
+The reason why `tox`_ wasn't used is, that there is no ``setup.py`` file, and
+it's difficult to install simple script, which isn't a python module (python
+interpreter will refuse to import module without ``.py`` extension).
+
+It requires GNU ``make`` program, and also ``virtualenv`` besides Python in
+version 2 and 3. Using it is simple as running following command:
+
+.. code:: shell-session
+
+   $ make
+
+it will run `py2`, `py3` and `flake8` jobs to check it against the code. For
+running tests against Python 3:
+
+.. code:: shell-session
+
+   $ make py3
+
+or Python 2:
+
+.. code:: shell-session
+
+   $ make py2
+
+or flake 8:
+
+.. code:: shell-session
+
+   $ make flake8
+
+Exit status on any of those means that test fail. Appropriate message/traceback
+will also be visible.
+
 
 Limitations
 ===========
@@ -104,8 +149,11 @@ Limitations
 * The implementation is experimental and it's by now working with mine device;
   while it might not work with yours
 
+
 License
 =======
 
 This software is licensed under 3-clause BSD license. See LICENSE file for
 details.
+
+.. _tox: https://tox.readthedocs.io

adbfs

@@ -5,9 +5,6 @@ adbfs Virtual filesystem for Midnight Commander
 * Copyright (c) 2016, Roman Dobosz,
 * Published under 3-clause BSD-style license (see LICENSE file)
 """
-try:
-    import ConfigParser as configparser
-except ImportError:
 import configparser
 import argparse
 from datetime import datetime
@@ -16,8 +13,9 @@ import os
 import re
 import subprocess
 import sys
+import shlex
 
-__version__ = 0.11
+__version__ = 0.13
 
 XDG_CONFIG_HOME = os.getenv('XDG_CONFIG_HOME', os.path.expanduser('~/.config'))
 
@@ -30,18 +28,18 @@ def check_output(command_list, stderr=None):
     """
     result = subprocess.check_output(command_list, stderr=stderr)
     if not isinstance(result, str):
-        result = result.decode('utf-8')
+        _result = []
+        for t in result.split(b'\n'):
+            if not t:
+                continue
+            try:
+                _result.append(t.decode('utf-8'))
+            except UnicodeDecodeError:
+                _result.append(t.decode('iso-8859-1'))
+        result = '\n'.join(_result) + '\n'
     return result
 
 
-class NoBoxFoundException(OSError):
-    """
-    Exception raised in case of not found either toolbox or busybox on remote
-    filesystem accessed via adb
-    """
-    pass
-
-
 class Conf(object):
     """Simple config parser"""
     boxes = {'busybox': {'ls': 'busybox ls -anel',
@@ -195,7 +193,8 @@ class Conf(object):
                    'suppress_colors': (cfg.get, 'suppress_colors'),
                    'root': (cfg.get, 'root'),
                    'adb_command': (cfg.get, 'adb_command'),
-                   'adb_connect': (cfg.get, 'adb_connect')}
+                   'adb_connect': (cfg.get, 'adb_connect'),
+                   'try_su': (cfg.getboolean, 'try_su')}
         cfg.read(conf_fname)
 
         for key, (function, attr) in cfg_map.items():
@@ -335,11 +334,23 @@ class Adb(object):
         self._links = {}
         self._got_root = False
 
+        if self.conf.try_su:
             self.__su_check()
 
+    def _shell_cmd(self, with_root, *args):
+        cmd = [self.conf.adb_command, 'shell']
+
+        if with_root and self._got_root:
+            _args = [shlex.quote(x) for x in args]
+            cmd += ['su', '-c', shlex.quote(' '.join(_args))]
+        else:
+            cmd += args
+
+        return cmd
+
     def __su_check(self):
         """Check if we are able to get elevated privileges"""
-        cmd = [self.conf.adb_command] + 'shell su -c whoami'.split()
+        cmd = self._shell_cmd(False, 'su -c whoami')
         try:
             with open(os.devnull, 'w') as fnull:
                 result = check_output(cmd, stderr=fnull)
@@ -389,10 +400,8 @@ class Adb(object):
 
     def _retrieve_single_dir_list(self, dir_):
         """Retrieve file list using adb"""
-        lscmd = self.conf.box['rls'].format(dir_)
+        lscmd = self.conf.box['rls'].format(shlex.quote(dir_))
-        if self._got_root:
+        command = self._shell_cmd(True, *shlex.split(lscmd))
-            lscmd = 'su -c "{}"'.format(lscmd)
-        command = [self.conf.adb_command, 'shell', lscmd]
 
         try:
             if self.conf.debug:
@@ -449,12 +458,9 @@ class Adb(object):
         if not root:
             lscmd = self.conf.box['ls']
         else:
-            lscmd = self.conf.box['rls'].format(root.filepath)
+            lscmd = self.conf.box['rls'].format(shlex.quite(root.filepath))
 
-        if self._got_root:
+        command = self._shell_cmd(True, *shlex.split(lscmd))
-            lscmd = 'su -c "{}"'.format(lscmd)
-
-        command = [self.conf.adb_command, 'shell', lscmd]
 
         try:
             if self.conf.debug:
@@ -463,7 +469,7 @@ class Adb(object):
             lines = check_output(command)
         except subprocess.CalledProcessError:
             sys.stderr.write('Cannot read directory. Is device connected?\n')
-            return 1
+            return 2
 
         current_dir = root.dirname if root else '/'
         for line in lines.split('\n'):
@@ -499,13 +505,13 @@ class Adb(object):
         """Not supported"""
         sys.stderr.write('Not supported - or maybe you are on compatible '
                          'architecture?\n')
-        return 1
+        return 3
 
     def list(self):
         """Output list contents directory"""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 4
 
         if self.conf.root:
             self._retrieve_single_dir_list(self.conf.root)
@@ -520,7 +526,7 @@ class Adb(object):
         """Copy file form the device using adb."""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 5
 
         cmd = [self.conf.adb_command, 'pull', src, dst]
         if self.conf.debug:
@@ -531,7 +537,7 @@ class Adb(object):
                 err = subprocess.call(cmd, stdout=fnull, stderr=fnull)
             except subprocess.CalledProcessError:
                 sys.stderr.write('Error executing adb shell')
-                return 1
+                return 6
 
         return err
 
@@ -539,7 +545,7 @@ class Adb(object):
         """Copy file to the device through adb."""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 7
         if not dst.startswith('/'):
             dst = '/' + dst
 
@@ -552,69 +558,69 @@ class Adb(object):
                 err = subprocess.call(cmd, stdout=fnull, stderr=fnull)
             except subprocess.CalledProcessError:
                 sys.stderr.write('Error executing adb shell')
-                return 1
+                return 8
 
         if err != 0:
             sys.stderr.write('Cannot push the file, '
                              '%s, error %d' % (dst, err))
-            return 1
+            return 9
         return 0
 
     def rm(self, dst):
         """Remove file from device."""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 10
 
-        cmd = [self.conf.adb_command, 'shell', 'rm', dst]
+        cmd = self._shell_cmd(False, 'rm', dst)
         try:
-            err = check_output(cmd)
+            err = check_output(cmd).strip()
         except subprocess.CalledProcessError:
             sys.stderr.write('Error executing adb shell')
-            return 1
+            return 11
 
         if err != '':
             sys.stderr.write(err)
-            return 1
+            return 12
         return 0
 
     def rmdir(self, dst):
         """Remove directory from device."""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 13
 
-        cmd = [self.conf.adb_command, 'shell', 'rm', '-r', dst]
+        cmd = self._shell_cmd(False, 'rm -r %s' % shlex.quote(dst))
         try:
-            err = check_output(cmd)
+            err = check_output(cmd).strip()
         except subprocess.CalledProcessError:
             sys.stderr.write('Error executing adb shell')
-            return 1
+            return 14
 
         if err != '':
             sys.stderr.write(err)
-            return 1
+            return 15
         return 0
 
     def mkdir(self, dst):
         """Make directory on the device through adb."""
         if self.error:
             sys.stderr.write(self.error)
-            return 1
+            return 16
 
         if not dst.startswith('/'):
             dst = '/' + dst
 
-        cmd = [self.conf.adb_command, 'shell', 'mkdir', dst]
+        cmd = self._shell_cmd(False, 'mkdir %s' % shlex.quote(dst))
         try:
-            err = check_output(cmd)
+            err = check_output(cmd).strip()
         except subprocess.CalledProcessError:
             sys.stderr.write('Error executing adb shell')
-            return 1
+            return 17
 
         if err != '':
             sys.stderr.write(err)
-            return 1
+            return 18
         return 0
 
 
@@ -673,7 +679,11 @@ def main():
 
     args = parser.parse_args()
 
+    try:
         return args.func(args)
+    except AttributeError:
+        parser.print_help()
+        parser.exit()
 
 
 if __name__ == '__main__':

test_adbfs.py

@@ -0,0 +1,71 @@
+# -*- coding: utf-8 -*-
+import os
+import six
+import unittest
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+FILE = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'adbfs')
+
+try:
+    from importlib.util import spec_from_loader, module_from_spec
+    from importlib.machinery import SourceFileLoader
+    spec = spec_from_loader("adbfs", SourceFileLoader("adbfs", FILE))
+    adbfs = module_from_spec(spec)
+    spec.loader.exec_module(adbfs)
+except ImportError:
+    # py27
+    import imp
+    adbfs = imp.load_source('adbfs', FILE)
+
+LISTING = '''\
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/Grüß Gott
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/\x80
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/Γεια σας
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/Здравствуйте
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/שָׁלוֹם
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/السَّلامُ عَلَيْكُمْ
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/გამარჯობა
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/こんにちは。
+-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 /storage/emulated/0/안녕하십니까
+'''  # noqa
+
+
+class TestCheckOutput(unittest.TestCase):
+
+    @mock.patch('subprocess.check_output')
+    def test_check_output(self, out):
+        """
+        As for Python2 (and its last version: 2.7), subprocess.check_output
+        always return string like objects, contrary to bytes - no conversion
+        to string is needed.
+        Python3 treats string as unicode objects, but subprocess.check_output
+        returns bytes object, which is equvalend for py2 string… annoying.
+        """
+        if six.PY3:
+            out.return_value = bytes(LISTING, 'utf-8')
+        else:
+            out.return_value = LISTING
+        result = adbfs.check_output(None)
+        self.assertEqual(result, LISTING)
+
+    @mock.patch('subprocess.check_output')
+    def test_check_output_py3_invalid_char(self, out):
+        """
+        Special case for py3. We have bytes with some weird character - like
+        some system write something with codepage, instead of utf8.
+        """
+        if six.PY2:
+            # doesn't affect Python 2
+            return
+        line = (b'-rw-rw----  1 0  1015  0 01/01/2010 22:11:01 '
+                b'/storage/emulated/0/\xe2\n')  # Latin 1 char â
+        out.return_value = bytes(line)
+        result = adbfs.check_output(None)
+        self.assertEqual(result, line.decode('iso-8859-1'))
+
+
+if __name__ == "__main__":
+    unittest.main()