Merge `literal_attr:` functionality into `attr:`

changelog.d/1753.change.rst

-Added a ``literal_attr:`` config directive to support reading versions from attributes of modules that import third-party modules
+``attr:`` now extracts variables through rudimentary examination of the AST,
+thereby supporting modules with third-party imports.  If examining the AST
+fails to find the variable, ``attr:`` falls back to the old behavior of
+importing the module.

docs/setuptools.txt

@@ -2193,7 +2193,7 @@ Metadata and options are set in the config sections of the same name.
 * In some cases, complex values can be provided in dedicated subsections for
   clarity.
 
-* Some keys allow ``file:``, ``attr:``, ``literal_attr:``, ``find:``, and ``find_namespace:`` directives in
+* Some keys allow ``file:``, ``attr:``, ``find:``, and ``find_namespace:`` directives in
   order to cover common usecases.
 
 * Unknown keys are ignored.
@@ -2291,13 +2291,10 @@ Special directives:
 * ``attr:`` - Value is read from a module attribute.  ``attr:`` supports
   callables and iterables; unsupported types are cast using ``str()``.
 
-* ``literal_attr:`` — Like ``attr:``, except that the value is parsed using
-  ``ast.literal_eval()`` instead of by importing the module.  This allows one
-  to specify an attribute of a module that imports one or more third-party
-  modules without having to install those modules first; as a downside,
-  ``literal_attr:`` only supports variables that are assigned constant
-  expressions, not more complex assignments like ``__version__ =
-  '.'.join(map(str, (VERSION_MAJOR, VERSION_MINOR, VERSION_PATCH)))``.
+  In order to support the common case of a literal value assigned to a variable
+  in a module containing (directly or indirectly) third-party imports,
+  ``attr:`` first tries to read the value from the module by examining the
+  module's AST.  If that fails, ``attr:`` falls back to importing the module.
 
 * ``file:`` - Value is read from a list of files and then concatenated
 
@@ -2314,11 +2311,11 @@ Metadata
     The aliases given below are supported for compatibility reasons,
     but their use is not advised.
 
-==============================  =================  ================================  =============== =====
+==============================  =================  =================  =============== =====
 Key                             Aliases            Type               Minimum Version Notes
-==============================  =================  ================================  =============== =====
+==============================  =================  =================  =============== =====
 name                                               str
-version                                            attr:, literal_attr:, file:, str  39.2.0          (1)
+version                                            attr:, file:, str  39.2.0          (1)
 url                             home-page          str
 download_url                    download-url       str
 project_urls                                       dict               38.3.0
@@ -2338,7 +2335,7 @@ platforms                       platform           list-comma
 provides                                           list-comma
 requires                                           list-comma
 obsoletes                                          list-comma
-==============================  =================  ================================  =============== =====
+==============================  =================  =================  =============== =====
 
 .. note::
     A version loaded using the ``file:`` directive must comply with PEP 440.

setuptools/config.py

@@ -317,22 +317,15 @@ class ConfigHandler:
         Examples:
             attr: package.attr
             attr: package.module.attr
-            literal_attr: package.attr
-            literal_attr: package.module.attr
 
         :param str value:
         :rtype: str
         """
         attr_directive = 'attr:'
-        literal_attr_directive = 'literal_attr:'
-        if value.startswith(attr_directive):
-            directive = attr_directive
-        elif value.startswith(literal_attr_directive):
-            directive = literal_attr_directive
-        else:
+        if not value.startswith(attr_directive):
             return value
 
-        attrs_path = value.replace(directive, '').strip().split('.')
+        attrs_path = value.replace(attr_directive, '').strip().split('.')
         attr_name = attrs_path.pop()
 
         module_name = '.'.join(attrs_path)
@@ -352,24 +345,14 @@ class ConfigHandler:
             elif '' in package_dir:
                 # A custom parent directory was specified for all root modules
                 parent_path = os.path.join(os.getcwd(), package_dir[''])
-        if directive == attr_directive:
-            sys.path.insert(0, parent_path)
-            try:
-                module = import_module(module_name)
-                value = getattr(module, attr_name)
-            finally:
-                sys.path = sys.path[1:]
 
-        elif directive == literal_attr_directive:
         fpath = os.path.join(parent_path, *module_name.split('.'))
         if os.path.exists(fpath + '.py'):
             fpath += '.py'
         elif os.path.isdir(fpath):
             fpath = os.path.join(fpath, '__init__.py')
         else:
-                raise DistutilsOptionError(
-                    'Could not find module ' + module_name
-                )
+            raise DistutilsOptionError('Could not find module ' + module_name)
         with open(fpath, 'rb') as fp:
             src = fp.read()
         found = False
@@ -379,23 +362,33 @@ class ConfigHandler:
                 for target in statement.targets:
                     if isinstance(target, ast.Name) \
                             and target.id == attr_name:
+                        try:
                             value = ast.literal_eval(statement.value)
+                        except ValueError:
+                            found = False
+                        else:
                             found = True
                     elif isinstance(target, ast.Tuple) \
-                            and any(isinstance(t, ast.Name) and t.id==attr_name
+                        and any(isinstance(t, ast.Name) and t.id == attr_name
                                 for t in target.elts):
+                        try:
                             stmnt_value = ast.literal_eval(statement.value)
-                                for t,v in zip(target.elts, stmnt_value):
+                        except ValueError:
+                            found = False
+                        else:
+                            for t, v in zip(target.elts, stmnt_value):
                                 if isinstance(t, ast.Name) \
                                         and t.id == attr_name:
                                     value = v
                                     found = True
         if not found:
-                raise DistutilsOptionError(
-                    'No literal assignment to {!r} found in file'
-                    .format(attr_name)
-                )
-
+            # Fall back to extracting attribute via importing
+            sys.path.insert(0, parent_path)
+            try:
+                module = import_module(module_name)
+                value = getattr(module, attr_name)
+            finally:
+                sys.path = sys.path[1:]
         return value
 
     @classmethod

setuptools/tests/test_config.py

@@ -103,7 +103,7 @@ class TestConfigurationReader:
             'version = attr: none.VERSION\n'
             'keywords = one, two\n'
         )
-        with pytest.raises(ImportError):
+        with pytest.raises(DistutilsOptionError):
             read_configuration('%s' % config)
 
         config_dict = read_configuration(
@@ -300,25 +300,6 @@ class TestMetadata:
         with get_dist(tmpdir) as dist:
             assert dist.metadata.version == '2016.11.26'
 
-    def test_literal_version(self, tmpdir):
-
-        _, config = fake_env(
-            tmpdir,
-            '[metadata]\n'
-            'version = literal_attr: fake_package.VERSION\n'
-        )
-        with get_dist(tmpdir) as dist:
-            assert dist.metadata.version == '1.2.3'
-
-        config.write(
-            '[metadata]\n'
-            'version = literal_attr: fake_package.VERSION_MAJOR\n'
-        )
-        with get_dist(tmpdir) as dist:
-            assert dist.metadata.version == '1'
-
-        subpack = tmpdir.join('fake_package').mkdir('subpackage')
-        subpack.join('__init__.py').write('')
         subpack.join('submodule.py').write(
             'import third_party_module\n'
             'VERSION = (2016, 11, 26)'
@@ -363,17 +344,6 @@ class TestMetadata:
         with get_dist(tmpdir) as dist:
             assert dist.metadata.version == '1.2.3'
 
-        config.write(
-            '[metadata]\n'
-            'version = literal_attr: fake_package_simple.VERSION\n'
-            '[options]\n'
-            'package_dir =\n'
-            '    = src\n'
-        )
-
-        with get_dist(tmpdir) as dist:
-            assert dist.metadata.version == '1.2.3'
-
     def test_version_with_package_dir_rename(self, tmpdir):
 
         _, config = fake_env(
@@ -389,17 +359,6 @@ class TestMetadata:
         with get_dist(tmpdir) as dist:
             assert dist.metadata.version == '1.2.3'
 
-        config.write(
-            '[metadata]\n'
-            'version = literal_attr: fake_package_rename.VERSION\n'
-            '[options]\n'
-            'package_dir =\n'
-            '    fake_package_rename = fake_dir\n'
-        )
-
-        with get_dist(tmpdir) as dist:
-            assert dist.metadata.version == '1.2.3'
-
     def test_version_with_package_dir_complex(self, tmpdir):
 
         _, config = fake_env(
@@ -415,17 +374,6 @@ class TestMetadata:
         with get_dist(tmpdir) as dist:
             assert dist.metadata.version == '1.2.3'
 
-        config.write(
-            '[metadata]\n'
-            'version = literal_attr: fake_package_complex.VERSION\n'
-            '[options]\n'
-            'package_dir =\n'
-            '    fake_package_complex = src/fake_dir\n'
-        )
-
-        with get_dist(tmpdir) as dist:
-            assert dist.metadata.version == '1.2.3'
-
     def test_unknown_meta_item(self, tmpdir):
 
         fake_env(