Repository: pypa/setuptools (python)

Issue:
package_dir and setup.cfg version attr: <foo> mutually exclusive?
I was messing around with the new ``setup.cfg`` support for metadata, and it looks like there is a poor interaction with ``package_dir`` and the version field support, specifically if you have a ``setup.cfg`` like:

```
[metadata]
version = attr: foobar.__version__

[options]
package_dir =
    = src
```

You'll get an error trying to import foobar. This is because it simply appends the current directory to ``sys.path`` and then attempts to import the above.

Pull request:
Fix attr: with package_dirs
## Summary of changes

Takes the `package_dir` option into account when loading `version` from a module attribute.  This should handle cases where all modules are located under a common parent and where that particular module is in a subdirectory and/or has a different name.  It doesn't attempt to handle cases where `package_dir` contains keys like "foo.bar", but I'm not sure such entries are even valid.

This required adding a dependency on the `options` section parsing in the `metadata` section parsing, in order to get the value of `package_dir`.  This seems pretty harmless since they happen right after each other, but let me know if it poses any problems.

Closes #1333 

### Pull Request Checklist
- [x] Changes have tests
- [x] News fragment added in changelog.d. See [documentation](http://setuptools.readthedocs.io/en/latest/developer-guide.html#making-a-pull-request) for details


Patch:
diff --git a/changelog.d/1365.change.rst b/changelog.d/1365.change.rst
new file mode 100644
index 000000000..63eb008b8
--- /dev/null
+++ b/changelog.d/1365.change.rst
@@ -0,0 +1,2 @@
+Take the package_dir option into account when loading the version from a
+module attribute.
diff --git a/setuptools/config.py b/setuptools/config.py
index 6343840e2..d3f0b123d 100644
--- a/setuptools/config.py
+++ b/setuptools/config.py
@@ -102,14 +102,14 @@ def parse_configuration(
         If False exceptions are propagated as expected.
     :rtype: list
     """
-    meta = ConfigMetadataHandler(
-        distribution.metadata, command_options, ignore_option_errors)
-    meta.parse()
-
     options = ConfigOptionsHandler(
         distribution, command_options, ignore_option_errors)
     options.parse()
 
+    meta = ConfigMetadataHandler(
+        distribution.metadata, command_options, ignore_option_errors, distribution.package_dir)
+    meta.parse()
+
     return meta, options
 
 
@@ -281,7 +281,7 @@ class ConfigHandler(object):
             return f.read()
 
     @classmethod
-    def _parse_attr(cls, value):
+    def _parse_attr(cls, value, package_dir=None):
         """Represents value as a module attribute.
 
         Examples:
@@ -301,7 +301,21 @@ class ConfigHandler(object):
         module_name = '.'.join(attrs_path)
         module_name = module_name or '__init__'
 
-        sys.path.insert(0, os.getcwd())
+        parent_path = os.getcwd()
+        if package_dir:
+            if attrs_path[0] in package_dir:
+                # A custom path was specified for the module we want to import
+                custom_path = package_dir[attrs_path[0]]
+                parts = custom_path.rsplit('/', 1)
+                if len(parts) > 1:
+                    parent_path = os.path.join(os.getcwd(), parts[0])
+                    module_name = parts[1]
+                else:
+                    module_name = custom_path
+            elif '' in package_dir:
+                # A custom parent directory was specified for all root modules
+                parent_path = os.path.join(os.getcwd(), package_dir[''])
+        sys.path.insert(0, parent_path)
         try:
             module = import_module(module_name)
             value = getattr(module, attr_name)
@@ -400,6 +414,12 @@ class ConfigMetadataHandler(ConfigHandler):
 
     """
 
+    def __init__(self, target_obj, options, ignore_option_errors=False,
+                 package_dir=None):
+        super(ConfigMetadataHandler, self).__init__(target_obj, options,
+                                                    ignore_option_errors)
+        self.package_dir = package_dir
+
     @property
     def parsers(self):
         """Metadata item name to parser function mapping."""
@@ -440,7 +460,7 @@ class ConfigMetadataHandler(ConfigHandler):
                 ))
             return version
 
-        version = self._parse_attr(value)
+        version = self._parse_attr(value, self.package_dir)
 
         if callable(version):
             version = version()


Test patch:
diff --git a/setuptools/tests/test_config.py b/setuptools/tests/test_config.py
index 17ac09c8f..de7c8b4da 100644
--- a/setuptools/tests/test_config.py
+++ b/setuptools/tests/test_config.py
@@ -10,13 +10,15 @@ class ErrConfigHandler(ConfigHandler):
 
 
 def make_package_dir(name, base_dir):
-    dir_package = base_dir.mkdir(name)
+    dir_package = base_dir
+    for dir_name in name.split('/'):
+        dir_package = dir_package.mkdir(dir_name)
     init_file = dir_package.join('__init__.py')
     init_file.write('')
     return dir_package, init_file
 
 
-def fake_env(tmpdir, setup_cfg, setup_py=None):
+def fake_env(tmpdir, setup_cfg, setup_py=None, package_path='fake_package'):
 
     if setup_py is None:
         setup_py = (
@@ -28,7 +30,7 @@ def fake_env(tmpdir, setup_cfg, setup_py=None):
     config = tmpdir.join('setup.cfg')
     config.write(setup_cfg)
 
-    package_dir, init_file = make_package_dir('fake_package', tmpdir)
+    package_dir, init_file = make_package_dir(package_path, tmpdir)
 
     init_file.write(
         'VERSION = (1, 2, 3)\n'
@@ -285,6 +287,51 @@ class TestMetadata:
             with get_dist(tmpdir) as dist:
                 _ = dist.metadata.version
 
+    def test_version_with_package_dir_simple(self, tmpdir):
+
+        _, config = fake_env(
+            tmpdir,
+            '[metadata]\n'
+            'version = attr: fake_package_simple.VERSION\n'
+            '[options]\n'
+            'package_dir =\n'
+            '    = src\n',
+            package_path='src/fake_package_simple'
+        )
+
+        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(
+            tmpdir,
+            '[metadata]\n'
+            'version = attr: fake_package_rename.VERSION\n'
+            '[options]\n'
+            'package_dir =\n'
+            '    fake_package_rename = fake_dir\n',
+            package_path='fake_dir'
+        )
+
+        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(
+            tmpdir,
+            '[metadata]\n'
+            'version = attr: fake_package_complex.VERSION\n'
+            '[options]\n'
+            'package_dir =\n'
+            '    fake_package_complex = src/fake_dir\n',
+            package_path='src/fake_dir'
+        )
+
+        with get_dist(tmpdir) as dist:
+            assert dist.metadata.version == '1.2.3'
+
     def test_unknown_meta_item(self, tmpdir):
 
         fake_env(