From: CaselIT <cfederico87@gmail.com>
Date: Tue, 15 Jun 2021 20:32:05 +0000 (+0200)
Subject: Revendor editor and make dateutil optional
X-Git-Tag: rel_1_7_0~27^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7756b35e23ec11b64a1ac37afcb41dda3ca29913;p=thirdparty%2Fsqlalchemy%2Falembic.git

Revendor editor and make dateutil optional

Re-implemented the ``python-editor`` dependency as a small internal
function to avoid the need for external dependencies.
The implementation is based on the original
version in 7b91b325ff43a0e9235e0f15b57391fa92352626.

Make the ``python-dateutil`` library an optional dependency.
This library is only required if the ``timezone`` option
is used in the Alembic configuration.
An extra require named ``tz`` is available with
``pip install alembic[tz]`` to install it.

Fixes: #674
Fixes: #856
Change-Id: I07f17b2fea01e3a3d677ce95333fe3e8d8d438fd
---

diff --git a/alembic/command.py b/alembic/command.py
index 6a4a3d0e..ada458d3 100644
--- a/alembic/command.py
+++ b/alembic/command.py
@@ -587,7 +587,7 @@ def edit(config, rev):
             if not rev:
                 raise util.CommandError("No current revisions")
             for sc in script.get_revisions(rev):
-                util.edit(sc.path)
+                util.open_in_editor(sc.path)
             return []
 
         with EnvironmentContext(config, script, fn=edit_current):
@@ -599,4 +599,4 @@ def edit(config, rev):
                 "No revision files indicated by symbol '%s'" % rev
             )
         for sc in revs:
-            util.edit(sc.path)
+            util.open_in_editor(sc.path)
diff --git a/alembic/script/base.py b/alembic/script/base.py
index 3f618643..723c0116 100644
--- a/alembic/script/base.py
+++ b/alembic/script/base.py
@@ -5,14 +5,17 @@ import re
 import shutil
 import sys
 
-from dateutil import tz
-
 from . import revision
 from . import write_hooks
 from .. import util
 from ..runtime import migration
 from ..util import compat
 
+try:
+    from dateutil import tz
+except ImportError:
+    tz = None  # noqa
+
 _sourceless_rev_file = re.compile(r"(?!\.\#|__init__)(.*\.py)(c|o)?$")
 _only_source_rev_file = re.compile(r"(?!\.\#|__init__)(.*\.py)$")
 _legacy_rev = re.compile(r"([a-f0-9]+)\.py$")
@@ -515,6 +518,11 @@ class ScriptDirectory(object):
 
     def _generate_create_date(self):
         if self.timezone is not None:
+            if tz is None:
+                raise util.CommandError(
+                    "The library 'python-dateutil' is required "
+                    "for timezone support"
+                )
             # First, assume correct capitalization
             tzinfo = tz.gettz(self.timezone)
             if tzinfo is None:
diff --git a/alembic/templates/async/alembic.ini.mako b/alembic/templates/async/alembic.ini.mako
index 31de75a4..64af3018 100644
--- a/alembic/templates/async/alembic.ini.mako
+++ b/alembic/templates/async/alembic.ini.mako
@@ -11,8 +11,10 @@ script_location = ${script_location}
 # defaults to the current working directory.
 prepend_sys_path = .
 
-# timezone to use when rendering the date
-# within the migration file as well as the filename.
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the python-dateutil library that can be
+# installed by adding `alembic[tz]` to the pip requirements
 # string value is passed to dateutil.tz.gettz()
 # leave blank for localtime
 # timezone =
diff --git a/alembic/templates/generic/alembic.ini.mako b/alembic/templates/generic/alembic.ini.mako
index 31de75a4..64af3018 100644
--- a/alembic/templates/generic/alembic.ini.mako
+++ b/alembic/templates/generic/alembic.ini.mako
@@ -11,8 +11,10 @@ script_location = ${script_location}
 # defaults to the current working directory.
 prepend_sys_path = .
 
-# timezone to use when rendering the date
-# within the migration file as well as the filename.
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the python-dateutil library that can be
+# installed by adding `alembic[tz]` to the pip requirements
 # string value is passed to dateutil.tz.gettz()
 # leave blank for localtime
 # timezone =
diff --git a/alembic/templates/multidb/alembic.ini.mako b/alembic/templates/multidb/alembic.ini.mako
index 363be679..c3b0ec0f 100644
--- a/alembic/templates/multidb/alembic.ini.mako
+++ b/alembic/templates/multidb/alembic.ini.mako
@@ -11,8 +11,10 @@ script_location = ${script_location}
 # defaults to the current working directory.
 prepend_sys_path = .
 
-# timezone to use when rendering the date
-# within the migration file as well as the filename.
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the python-dateutil library that can be
+# installed by adding `alembic[tz]` to the pip requirements
 # string value is passed to dateutil.tz.gettz()
 # leave blank for localtime
 # timezone =
diff --git a/alembic/templates/pylons/alembic.ini.mako b/alembic/templates/pylons/alembic.ini.mako
index b0b8fe1d..aa745a8a 100644
--- a/alembic/templates/pylons/alembic.ini.mako
+++ b/alembic/templates/pylons/alembic.ini.mako
@@ -11,8 +11,10 @@ script_location = ${script_location}
 # defaults to the current working directory.
 prepend_sys_path = .
 
-# timezone to use when rendering the date
-# within the migration file as well as the filename.
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the python-dateutil library that can be
+# installed by adding `alembic[tz]` to the pip requirements
 # string value is passed to dateutil.tz.gettz()
 # leave blank for localtime
 # timezone =
diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py
index 3a5426b6..5a110688 100644
--- a/alembic/testing/requirements.py
+++ b/alembic/testing/requirements.py
@@ -67,18 +67,6 @@ class SuiteRequirements(Requirements):
     def reflects_fk_options(self):
         return exclusions.closed()
 
-    @property
-    def editor_installed(self):
-        def go():
-            try:
-                import editor  # noqa
-            except ImportError:
-                return False
-            else:
-                return True
-
-        return exclusions.only_if(go, "editor package not installed")
-
     @property
     def sqlalchemy_13(self):
         return exclusions.skip_if(
diff --git a/alembic/util/__init__.py b/alembic/util/__init__.py
index cfdab498..2a3d8a88 100644
--- a/alembic/util/__init__.py
+++ b/alembic/util/__init__.py
@@ -1,4 +1,5 @@
 from .compat import raise_  # noqa
+from .editor import open_in_editor  # noqa
 from .exc import CommandError
 from .langhelpers import _with_legacy_names  # noqa
 from .langhelpers import asbool  # noqa
@@ -19,7 +20,6 @@ from .messaging import status  # noqa
 from .messaging import warn  # noqa
 from .messaging import write_outstream  # noqa
 from .pyfiles import coerce_resource_to_filename  # noqa
-from .pyfiles import edit  # noqa
 from .pyfiles import load_python_file  # noqa
 from .pyfiles import pyc_file_from_path  # noqa
 from .pyfiles import template_to_file  # noqa
diff --git a/alembic/util/editor.py b/alembic/util/editor.py
new file mode 100644
index 00000000..c27f0f36
--- /dev/null
+++ b/alembic/util/editor.py
@@ -0,0 +1,71 @@
+import os
+from os.path import exists
+from os.path import join
+from os.path import splitext
+from subprocess import check_call
+
+from .compat import is_posix
+from .exc import CommandError
+
+
+def open_in_editor(filename, environ=None):
+    """
+    Opens the given file in a text editor. If the environment variable
+    ``EDITOR`` is set, this is taken as preference.
+
+    Otherwise, a list of commonly installed editors is tried.
+
+    If no editor matches, an :py:exc:`OSError` is raised.
+
+    :param filename: The filename to open. Will be passed  verbatim to the
+        editor command.
+    :param environ: An optional drop-in replacement for ``os.environ``. Used
+        mainly for testing.
+    """
+
+    try:
+        editor = _find_editor(environ)
+        check_call([editor, filename])
+    except Exception as exc:
+        raise CommandError("Error executing editor (%s)" % (exc,)) from exc
+
+
+def _find_editor(environ=None):
+    candidates = _default_editors()
+    for i, var in enumerate(("EDITOR", "VISUAL")):
+        if var in environ:
+            user_choice = environ[var]
+            if exists(user_choice):
+                return user_choice
+            if os.sep not in user_choice:
+                candidates.insert(i, user_choice)
+
+    for candidate in candidates:
+        path = _find_executable(candidate, environ)
+        if path is not None:
+            return path
+    raise OSError(
+        "No suitable editor found. Please set the "
+        '"EDITOR" or "VISUAL" environment variables'
+    )
+
+
+def _find_executable(candidate, environ):
+    # Assuming this is on the PATH, we need to determine it's absolute
+    # location. Otherwise, ``check_call`` will fail
+    if not is_posix and splitext(candidate)[1] != ".exe":
+        candidate += ".exe"
+    for path in environ.get("PATH", "").split(os.pathsep):
+        value = join(path, candidate)
+        if exists(value):
+            return value
+    return None
+
+
+def _default_editors():
+    # Look for an editor. Prefer the user's choice by env-var, fall back to
+    # most commonly installed editor (nano/vim)
+    if is_posix:
+        return ["sensible-editor", "editor", "nano", "vim", "code"]
+    else:
+        return ["code.exe", "notepad++.exe", "notepad.exe"]
diff --git a/alembic/util/pyfiles.py b/alembic/util/pyfiles.py
index 8dfb9db0..1bc5be36 100644
--- a/alembic/util/pyfiles.py
+++ b/alembic/util/pyfiles.py
@@ -10,7 +10,6 @@ from .compat import has_pep3147
 from .compat import load_module_py
 from .compat import load_module_pyc
 from .compat import py3k
-from .compat import raise_
 from .exc import CommandError
 
 
@@ -75,17 +74,6 @@ def pyc_file_from_path(path):
         return None
 
 
-def edit(path):
-    """Given a source path, run the EDITOR for it"""
-
-    import editor
-
-    try:
-        editor.edit(path)
-    except Exception as exc:
-        raise_(CommandError("Error executing editor (%s)" % (exc,)), from_=exc)
-
-
 def load_python_file(dir_, filename):
     """Load a file from the given path as a Python module."""
 
diff --git a/docs/build/tutorial.rst b/docs/build/tutorial.rst
index 2ef84193..df5d1158 100644
--- a/docs/build/tutorial.rst
+++ b/docs/build/tutorial.rst
@@ -137,8 +137,10 @@ The file generated with the "generic" configuration looks like::
     # (new in 1.5.5)
     prepend_sys_path = .
 
-    # timezone to use when rendering the date
-    # within the migration file as well as the filename.
+    # timezone to use when rendering the date within the migration file
+    # as well as the filename.
+    # If specified, requires the python-dateutil library that can be
+    # installed by adding `alembic[tz]` to the pip requirements
     # string value is passed to dateutil.tz.gettz()
     # leave blank for localtime
     # timezone =
@@ -253,7 +255,8 @@ This file contains the following features:
 
 * ``timezone`` - an optional timezone name (e.g. ``UTC``, ``EST5EDT``, etc.)
   that will be applied to the timestamp which renders inside the migration
-  file's comment as well as within the filename.  If ``timezone`` is specified,
+  file's comment as well as within the filename. This option requires installing
+  the ``python-dateutil`` library. If ``timezone`` is specified,
   the create date object is no longer derived from ``datetime.datetime.now()``
   and is instead generated as::
 
diff --git a/docs/build/unreleased/674.rst b/docs/build/unreleased/674.rst
new file mode 100644
index 00000000..8ed09dbc
--- /dev/null
+++ b/docs/build/unreleased/674.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: changed
+    :tickets: 674
+
+    Make the ``python-dateutil`` library an optional dependency.
+    This library is only required if the ``timezone`` option
+    is used in the Alembic configuration.
+    An extra require named ``tz`` is available with
+    ``pip install alembic[tz]`` to install it.
diff --git a/docs/build/unreleased/856.rst b/docs/build/unreleased/856.rst
new file mode 100644
index 00000000..b756fc32
--- /dev/null
+++ b/docs/build/unreleased/856.rst
@@ -0,0 +1,6 @@
+.. change::
+    :tags: bug, commands
+    :tickets: 856
+
+    Re-implemented the ``python-editor`` dependency as a small internal
+    function to avoid the need for external dependencies.
diff --git a/setup.cfg b/setup.cfg
index 59be5360..b168de5b 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -44,7 +44,9 @@ python_requires = >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*
 install_requires =
     SQLAlchemy>=1.3.0
     Mako
-    python-editor>=0.3
+
+[options.extras_require]
+tz =
     python-dateutil
 
 [options.packages.find]
diff --git a/tests/test_command.py b/tests/test_command.py
index 4114be07..a477a1d3 100644
--- a/tests/test_command.py
+++ b/tests/test_command.py
@@ -11,12 +11,10 @@ from sqlalchemy import text
 from alembic import __version__
 from alembic import command
 from alembic import config
-from alembic import testing
 from alembic import util
 from alembic.script import ScriptDirectory
 from alembic.testing import assert_raises
 from alembic.testing import assert_raises_message
-from alembic.testing import config as testing_config
 from alembic.testing import eq_
 from alembic.testing import is_false
 from alembic.testing import is_true
@@ -924,7 +922,7 @@ class EditTest(TestBase):
             % (EditTest.cfg.config_args["here"], EditTest.c)
         )
 
-        with mock.patch("alembic.util.edit") as edit:
+        with mock.patch("alembic.util.open_in_editor") as edit:
             command.edit(self.cfg, "head")
             edit.assert_called_with(expected_call_arg)
 
@@ -934,22 +932,10 @@ class EditTest(TestBase):
             % (EditTest.cfg.config_args["here"], EditTest.b)
         )
 
-        with mock.patch("alembic.util.edit") as edit:
+        with mock.patch("alembic.util.open_in_editor") as edit:
             command.edit(self.cfg, self.b[0:3])
             edit.assert_called_with(expected_call_arg)
 
-    @testing_config.requirements.editor_installed
-    @testing.emits_python_deprecation_warning("the imp module is deprecated")
-    def test_edit_with_missing_editor(self):
-        with mock.patch("editor.edit") as edit_mock:
-            edit_mock.side_effect = OSError("file not found")
-            assert_raises_message(
-                util.CommandError,
-                "file not found",
-                util.edit,
-                "/not/a/file.txt",
-            )
-
     def test_edit_no_revs(self):
         assert_raises_message(
             util.CommandError,
@@ -975,7 +961,7 @@ class EditTest(TestBase):
         )
 
         command.stamp(self.cfg, self.b)
-        with mock.patch("alembic.util.edit") as edit:
+        with mock.patch("alembic.util.open_in_editor") as edit:
             command.edit(self.cfg, "current")
             edit.assert_called_with(expected_call_arg)
 
diff --git a/tests/test_editor.py b/tests/test_editor.py
new file mode 100644
index 00000000..0ec6f5f4
--- /dev/null
+++ b/tests/test_editor.py
@@ -0,0 +1,106 @@
+import os
+from os.path import join
+from unittest.mock import patch
+
+from alembic import util
+from alembic.testing import combinations
+from alembic.testing import expect_raises_message
+from alembic.testing.fixtures import TestBase
+
+
+class TestHelpers(TestBase):
+    def common(self, cb, is_posix=True):
+        with patch("alembic.util.editor.check_call") as check_call, patch(
+            "alembic.util.editor.exists"
+        ) as exists, patch(
+            "alembic.util.editor.is_posix",
+            new=is_posix,
+        ), patch(
+            "os.pathsep", new=":" if is_posix else ";"
+        ):
+            cb(check_call, exists)
+
+    @combinations((True,), (False,))
+    def test_edit_with_user_editor(self, posix):
+        def go(check_call, exists):
+            test_environ = {"EDITOR": "myvim", "PATH": "/usr/bin"}
+            executable = join("/usr/bin", "myvim")
+            if not posix:
+                executable += ".exe"
+
+            exists.side_effect = lambda fname: fname == executable
+            util.open_in_editor("myfile", test_environ)
+            check_call.assert_called_with([executable, "myfile"])
+
+        self.common(go, posix)
+
+    @combinations(("EDITOR",), ("VISUAL",))
+    def test_edit_with_user_editor_exists(self, key):
+        def go(check_call, exists):
+            test_environ = {key: "myvim", "PATH": "/usr/bin"}
+            exists.side_effect = lambda fname: fname == "myvim"
+            util.open_in_editor("myfile", test_environ)
+            check_call.assert_called_with(["myvim", "myfile"])
+
+        self.common(go)
+
+    @combinations((True,), (False,))
+    def test_edit_with_user_editor_precedence(self, with_path):
+        def go(check_call, exists):
+            test_environ = {
+                "EDITOR": "myvim",
+                "VISUAL": "myvisual",
+                "PATH": "/usr/bin",
+            }
+            exes = ["myvim", "myvisual"]
+            if with_path:
+                exes = [join("/usr/bin", n) for n in exes]
+            exists.side_effect = lambda fname: fname in exes
+            util.open_in_editor("myfile", test_environ)
+            check_call.assert_called_with([exes[0], "myfile"])
+
+        self.common(go)
+
+    def test_edit_with_user_editor_abs(self):
+        def go(check_call, exists):
+            test_environ = {"EDITOR": "/foo/myvim", "PATH": "/usr/bin"}
+            exists.side_effect = lambda fname: fname == "/usr/bin/foo/myvim"
+            with expect_raises_message(util.CommandError, "EDITOR"):
+                util.open_in_editor("myfile", test_environ)
+
+        self.common(go)
+
+    def test_edit_with_default_editor(self):
+        def go(check_call, exists):
+            test_environ = {"PATH": os.pathsep.join(["/usr/bin", "/bin"])}
+            executable = join("/bin", "vim")
+
+            exists.side_effect = lambda fname: fname == executable
+            util.open_in_editor("myfile", test_environ)
+            check_call.assert_called_with([executable, "myfile"])
+
+        self.common(go)
+
+    def test_edit_with_default_editor_windows(self):
+        def go(check_call, exists):
+            test_environ = {
+                "PATH": os.pathsep.join(
+                    [r"C:\Windows\System32", r"C:\Users\user\bin"]
+                )
+            }
+            executable = join(r"C:\Users\user\bin", "notepad.exe")
+
+            exists.side_effect = lambda fname: fname == executable
+            util.open_in_editor("myfile", test_environ)
+            check_call.assert_called_with([executable, "myfile"])
+
+        self.common(go, False)
+
+    def test_edit_with_missing_editor(self):
+        def go(check_call, exists):
+            test_environ = {}
+            exists.return_value = False
+            with expect_raises_message(util.CommandError, "EDITOR"):
+                util.open_in_editor("myfile", test_environ)
+
+        self.common(go)
diff --git a/tests/test_script_production.py b/tests/test_script_production.py
index 15d9366a..2edae267 100644
--- a/tests/test_script_production.py
+++ b/tests/test_script_production.py
@@ -15,6 +15,7 @@ from alembic.script import ScriptDirectory
 from alembic.testing import assert_raises_message
 from alembic.testing import assertions
 from alembic.testing import eq_
+from alembic.testing import expect_raises_message
 from alembic.testing import is_
 from alembic.testing import mock
 from alembic.testing import ne_
@@ -34,6 +35,10 @@ from alembic.testing.env import write_script
 from alembic.testing.fixtures import TestBase
 from alembic.util import CommandError
 
+try:
+    from unittest.mock import patch
+except ImportError:
+    from mock import patch  # noqa
 env, abc, def_ = None, None, None
 
 
@@ -250,6 +255,17 @@ class ScriptNamingTest(TestBase):
             datetime.datetime(2012, 7, 25, 15, 8, 5),
         )
 
+    def test_no_dateutil_module(self):
+        with patch("alembic.script.base.tz", new=None):
+            with expect_raises_message(
+                CommandError, "The library 'python-dateutil' is required"
+            ):
+                self._test_tz(
+                    "utc",
+                    datetime.datetime(2012, 7, 25, 15, 8, 5),
+                    datetime.datetime(2012, 7, 25, 15, 8, 5),
+                )
+
 
 class RevisionCommandTest(TestBase):
     def setUp(self):
diff --git a/tox.ini b/tox.ini
index ec69f94e..596942a8 100644
--- a/tox.ini
+++ b/tox.ini
@@ -22,7 +22,6 @@ deps=pytest>4.6
      cov: pytest-cov
      sqlalchemy: sqlalchemy>=1.3.0
      mako
-     python-editor>=0.3
      python-dateutil