DLS lightweight-venv + python -m setup bdist_wheel doesn't work

[thomascobb]

At the moment in setup.py we find _version_git.py by walking from the parent dir of setup.py and stopping when we find a _version_git.py:

# Place the directory containing _version_git on the path
for path, _, filenames in os.walk(os.path.dirname(os.path.abspath(__file__))):
    if "_version_git.py" in filenames:
        sys.path.append(path)
        break

If we make a lightweight-venv then do python -m setup bdist_wheel then the structure when setup.py is called looks something like this:

setup.py
my_package/
    _version_git.py
   ...
lightweight-venv/
    lib/
        python3.7/
            site-packages/
                other_package/
                    _version_git.py
                    ...
...

Sometimes os.walk will find other_package/_version_git.py instead of my_package/_version_git.py. A more correct approach would be to only look one directory deep:

# Place the directory containing _version_git on the path
TOP = os.path.dirname(os.path.abspath(__file__))
for d in os.listdir(TOP):
    if os.path.exists(os.path.join(TOP, d, "_version_git.py")):
        sys.path.append(os.path.join(TOP, d))
        break

[thomascobb]

As reported in DiamondLightSource/pythonSoftIOC#31

[GDYendell]

Could you use e.g. version = attr:<package-name>._version_git.__version__ in setup.cfg?

https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html?highlight=metadata#configuring-setup-using-setup-cfg-files

[thomascobb]

You could do, but you still need the import for get_cmdclass: https://github.com/dls-controls/versiongit/blob/c0a3be20f626194fb0ee1ae73dceb3d4d7db1ecb/setup.py#L17

[GDYendell]

I played around with this for a while

cmdclass =
    sdist = odinprocservcontrol._version_git.Sdist
    build_py = odinprocservcontrol._version_git.BuildPy

but, couldn't get it to work, I think at least partially for this reason. It is frustrating that there isn't a better way to do this...

[thomascobb]

The third party imports issue is the main problem here. That's why I put _version_git on the path and don't just import my_package._version_git.__version__ in setup.py. The AST parsing approach that attr: tries would get around the issue if we had a static string (like when we install from a pypi release) but the install from a git repo would have to run the code, which means all imports would have to work.

I did originally try an execfile approach, which I might have a go with again...

[GDYendell]

The thing is, this works fine (note that you do have to have the full path - I guess this allows it to be found without relying on odinprovservcontrol.__init__.py):

version = attr: odinprocservcontrol._version_git.__version__

▶ python setup.py --version
0.1+1.g3ea4a73.dirty

It is only when I add the cmdclass to setup.cfg that has the issue of trying to import everything in the module. Which makes no sense to me, it should be able to import the classes in the same way as the __version__. Adding the cmdclass also breaks python setup.py --version, but version = attr: ... on its own works. Seems like a bug to me, unless I am missing something that means these two imports are fundamentally different.

[thomascobb]

At the moment BuildPy and Sdist are embedded in the get_cmdclass function. You could extract them into the top level file, but that would stop you passing in your own BuildPy and Sdist baseclasses to inherit from

[GDYendell]

I have split them into classes, currently. See here if you're intersted: /dls_sw/work/python3/RHEL7-x86_64/odinprocservcontrol/odinprocservcontrol/_version_git.py

[thomascobb]

Ok, but even if we got that working, it would still try and import odinprocservcontrol, which runs __init__.py which import odinprocserv which imports softioc which is not necessarily installed when setup.py is run. This is the third party import issue we'll struggle to solve. The only way to get around it (and still use the cmdclasses) is to be able to import _version_git without importing odinprocserv. We can do that with a soft-link of _version_git.py -> odingprocservcontrol/_version_git.py, but that might not work on windows, which is why I went with the path hacking method.

[GDYendell]

Do you know if it is expected behaviour that importing __version__ does not try importing everything, but with the cmdclasses it does?

[thomascobb]

It looks like it tries to parse AST for attr: to see if its a static string, which is meant to avoid the imports. But I doubt it can create the classes from that AST, so it probably does the import there

[thomascobb]

The changes required for a working setup.cfg only example when you have no third party imports that run when you import your module: 89f4ef9
But when you add a third party import: 0d2006e
Then it fails like this:

$ python -m build --sdist --wheel
* Creating venv isolated environment...
* Installing packages in isolated environment... (setuptools==44.1.1, wheel==0.33.1)
* Getting dependencies for sdist...
Traceback (most recent call last):
  File "/scratch/tmc43/pipenv/versiongit-KaOsyyPZ/lib/python3.7/site-packages/pep517/in_process/_in_process.py", line 349, in <module>
    main()
  File "/scratch/tmc43/pipenv/versiongit-KaOsyyPZ/lib/python3.7/site-packages/pep517/in_process/_in_process.py", line 331, in main
    json_out['return_val'] = hook(**hook_input['kwargs'])
  File "/scratch/tmc43/pipenv/versiongit-KaOsyyPZ/lib/python3.7/site-packages/pep517/in_process/_in_process.py", line 284, in get_requires_for_build_sdist
    return hook(config_settings)
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/build_meta.py", line 150, in get_requires_for_build_sdist
    return self._get_build_requires(config_settings, requirements=[])
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/build_meta.py", line 127, in _get_build_requires
    self.run_setup()
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/build_meta.py", line 142, in run_setup
    exec(compile(code, __file__, 'exec'), locals())
  File "setup.py", line 1, in <module>
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/__init__.py", line 162, in setup
    return distutils.core.setup(**attrs)
  File "/dls_sw/prod/tools/RHEL7-x86_64/Python3/3-7-2dls1/prefix/lib/python3.7/distutils/core.py", line 121, in setup
    dist.parse_config_files()
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/dist.py", line 702, in parse_config_files
    ignore_option_errors=ignore_option_errors)
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 121, in parse_configuration
    meta.parse()
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 426, in parse
    section_parser_method(section_options)
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 399, in parse_section
    self[name] = value
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 184, in __setitem__
    value = parser(value)
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 515, in _parse_version
    version = self._parse_attr(value, self.package_dir)
  File "/tmp/build-env-e6dgkjnt/lib/python3.7/site-packages/setuptools/config.py", line 349, in _parse_attr
    module = import_module(module_name)
  File "/dls_sw/prod/tools/RHEL7-x86_64/Python3/3-7-2dls1/prefix/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 953, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/dls/science/users/tmc43/common/python/versiongit/versiongit/__init__.py", line 1, in <module>
    import pytest
ModuleNotFoundError: No module named 'pytest'

ERROR Backend subproccess exited when trying to invoke get_requires_for_build_sdist

[thomascobb]

I don't see any way around this, and I think the majority of modules will pull in some third party import when importing the top level __init__.py. I think we're stuck with hacking the path in setup.py but I'll try and find a safer way to do it, maybe with https://docs.python.org/3.7/library/imp.html?highlight=imp#imp.load_module