Repository: Python-Markdown/markdown
Pull request #948 (2020-04-26 10:13:41)

Avoid importing packaging or pkg_resources for version validation
Importing the `pkg_resources` module has high memory and startup time cost. A recent change 102e01c2462f20fdd29073bc0b1fe2252b517add already avoided it for loading extensions, but it's still used for validating `__version__` has correct format.

It is possible to avoid it by installing the `packaging` package, but that adds a dependency for something quite trivial.

Instead, remove the validation and add tests which check the output is as expected (they pass before and after).

waylan: The concern I have, and the reason for running the validation, is that someone might provide bad data in `__version_info__`. With the validation, this would be caught, even if `get_version` returns something that looks right at a glance. With your change we don't get that safety net. I'm not sure I'm comfortable with that.

I wonder if perhaps we should run the validation in `setup.py`. That way any such issues would be addressed on build, etc.
bluetech: Thanks for taking a look!

> With your change we don't get that safety net. I'm not sure I'm comfortable with that.

Understood.

> I wonder if perhaps we should run the validation in setup.py. That way any such issues would be addressed on build, etc.

Is it viable to do it in the test suite? If all modifications to `__version_info__` go through CI (or a test run in any other way), then any bad change will be caught this way.

Doing it in the tests seems preferable to me, but I can also do it in setup.py if the tests don't work.
mitya57: In case you move this check to tests or setup.py, please also remove setuptools from `install_requires`, as it will be no longer used on runtime.
waylan: Yeah, I suppose we could test that the value set in `__version_info__` is valid in a test. Actually, now that I think about it, that probably makes more sense. It also is completely inline with our release process.
bluetech: I pushed an updated version:

- Added a test which verifies `__version__` is valid and normalized. I verified that if I modify `__version_info__` in some bad way, e.g. replaced `1` with `-1`, it fails.

- Removed `setuptools` from `install_required` as suggested by @mitya57.
waylan: Looks good. Thank you.

I was going to suggest that perhaps we should list `packaging` under `extra_requires` | `testing`, but I see that the commit message to f5b6579e1 notes:

> [I]f `pkg_resources` is installed at all, then we can be fairly certain one of `packaging` or `pkg_resources._vendor.packaging` will be available. That being the case, there is no need to list `packaging` as a dependency.

Just recording that here to document it for future reference.

Patch:
diff --git a/docs/change_log/index.md b/docs/change_log/index.md
index 121dab0..8e601bf 100644
--- a/docs/change_log/index.md
+++ b/docs/change_log/index.md
@@ -10,6 +10,8 @@ Under development: version 3.2.2 (a bug-fix release).
 * Correctly report if an extension raises a `TypeError` (#939).
 * Raise a `KeyError` when attempting to delete a nonexistent key from the
   extension registry (#939). 
+* Remove import of `packaging` (or `pkg_resources` fallback) entirely.
+* Remove `setuptools` as a run-time dependency (`install_required`).
 
 Feb 12, 2020: Released version 3.2.1 (a bug-fix release).
 
diff --git a/markdown/__meta__.py b/markdown/__meta__.py
index ead7c31..4d88aa4 100644
--- a/markdown/__meta__.py
+++ b/markdown/__meta__.py
@@ -19,11 +19,6 @@ Copyright 2004 Manfred Stienstra (the original version)
 License: BSD (see LICENSE.md for details).
 """
 
-try:
-    import packaging.version
-except ImportError:
-    from pkg_resources.extern import packaging
-
 # __version_info__ format:
 # (major, minor, patch, dev/alpha/beta/rc/final, #)
 # (1, 1, 2, 'dev', 0) => "1.1.2.dev0"
@@ -34,22 +29,21 @@ except ImportError:
 __version_info__ = (3, 2, 1, 'final', 0)
 
 
-def _get_version():  # pragma: no cover
+def _get_version(version_info):
     " Returns a PEP 440-compliant version number from version_info. "
-    assert len(__version_info__) == 5
-    assert __version_info__[3] in ('dev', 'alpha', 'beta', 'rc', 'final')
+    assert len(version_info) == 5
+    assert version_info[3] in ('dev', 'alpha', 'beta', 'rc', 'final')
 
-    parts = 2 if __version_info__[2] == 0 else 3
-    v = '.'.join(map(str, __version_info__[:parts]))
+    parts = 2 if version_info[2] == 0 else 3
+    v = '.'.join(map(str, version_info[:parts]))
 
-    if __version_info__[3] == 'dev':
-        v += '.dev' + str(__version_info__[4])
-    elif __version_info__[3] != 'final':
+    if version_info[3] == 'dev':
+        v += '.dev' + str(version_info[4])
+    elif version_info[3] != 'final':
         mapping = {'alpha': 'a', 'beta': 'b', 'rc': 'rc'}
-        v += mapping[__version_info__[3]] + str(__version_info__[4])
+        v += mapping[version_info[3]] + str(version_info[4])
 
-    # Ensure version is valid and normalized
-    return str(packaging.version.Version(v))
+    return v
 
 
-__version__ = _get_version()
+__version__ = _get_version(__version_info__)
diff --git a/setup.py b/setup.py
index b8bb80b..62f870d 100755
--- a/setup.py
+++ b/setup.py
@@ -88,7 +88,7 @@ setup(
     license='BSD License',
     packages=['markdown', 'markdown.extensions'],
     python_requires='>=3.5',
-    install_requires=['setuptools >= 36', "importlib_metadata;python_version<'3.8'"],
+    install_requires=["importlib_metadata;python_version<'3.8'"],
     extras_require={
         'testing': [
             'coverage',


Test patch:
diff --git a/tests/test_meta.py b/tests/test_meta.py
new file mode 100644
index 0000000..10a2d33
--- /dev/null
+++ b/tests/test_meta.py
@@ -0,0 +1,24 @@
+import unittest
+from markdown.__meta__ import _get_version, __version__
+
+
+class TestVersion(unittest.TestCase):
+
+    def test_get_version(self):
+        """Test that _get_version formats __version_info__ as required by PEP 440."""
+
+        self.assertEqual(_get_version((1, 1, 2, 'dev', 0)), "1.1.2.dev0")
+        self.assertEqual(_get_version((1, 1, 2, 'alpha', 1)), "1.1.2a1")
+        self.assertEqual(_get_version((1, 2, 0, 'beta', 2)), "1.2b2")
+        self.assertEqual(_get_version((1, 2, 0, 'rc', 4)), "1.2rc4")
+        self.assertEqual(_get_version((1, 2, 0, 'final', 0)), "1.2")
+
+    def test__version__IsValid(self):
+        """Test that __version__ is valid and normalized."""
+
+        try:
+            import packaging.version
+        except ImportError:
+            from pkg_resources.extern import packaging
+
+        self.assertEqual(__version__, str(packaging.version.Version(__version__)))