mdavidsaver / setuptools_dso

setuptools extension for building non-Python Dynamic Shared Objects
Other
9 stars 6 forks source link

Editable install fails when an extension depending on a DSO is located in a namespace package #28

Closed Thf772 closed 9 months ago

Thf772 commented 10 months ago

I have a minimal working example here: https://gitlab.com/Thf772/setuptools-dso-multi-namespace-test

When a namespace package is installed in editable mode, trying to install an Extension depending on a DSO located in the same namespace package will fail, whether that install is editable or not, on both Linux and Windows.

Steps to reproduce:

My MWE above uses different names.

Expected result

The second install succeeds

Actual result

An error occurs while building the second project:

  × Building editable for test-dso-nsp-dso (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [159 lines of output]
      running editable_wheel
      creating /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info
      writing /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/PKG-INFO
      writing dependency_links to /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/dependency_links.txt
      writing requirements to /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/requires.txt
      writing top-level names to /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/top_level.txt
      writing manifest file '/tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/SOURCES.txt'
      reading manifest file '/tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/SOURCES.txt'
      reading manifest template 'MANIFEST.in'
      writing manifest file '/tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso.egg-info/SOURCES.txt'
      creating '/tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso-0.0.0.dist-info'
      Original Wheel Tag: cp311, cp311, linux_x86_64
      creating /tmp/pip-wheel-4259yequ/.tmp-9qjk9ar7/test_dso_nsp_dso-0.0.0.dist-info/WHEEL
      Original Wheel Tag: cp311, cp311, linux_x86_64
      running build_py
      running build_dso
      Building DSOs
      Patch _fix_compile_args() to avoid modification to compiler.include_dirs
      building 'testnsp.testdso.thedso' DSO as /tmp/tmptr6erjpr.build-lib/testnsp/testdso/libthedso.so
      effective NUM_JOBS=2
      _patch_fix_compile_args include_dirs=[]
      creating /tmp/tmpqx2ffds9.build-temp/testnsp
      creating /tmp/tmpqx2ffds9.build-temp/testnsp/testdso
      gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -c testnsp/testdso/libtest.c -o /tmp/tmpqx2ffds9.build-temp/testnsp/testdso/libtest.o
      _patch_fix_compile_args include_dirs=[]
      creating /tmp/tmptr6erjpr.build-lib/testnsp
      creating /tmp/tmptr6erjpr.build-lib/testnsp/testdso
      gcc -shared /tmp/tmpqx2ffds9.build-temp/testnsp/testdso/libtest.o -o /tmp/tmptr6erjpr.build-lib/testnsp/testdso/libthedso.so
      copying /tmp/tmptr6erjpr.build-lib/testnsp/testdso/libthedso.so -> testnsp/testdso
      creating info module for testnsp.testdso.thedso at /tmp/tmptr6erjpr.build-lib/testnsp/testdso/thedso_dsoinfo.py
      copying /tmp/tmptr6erjpr.build-lib/testnsp/testdso/thedso_dsoinfo.py -> testnsp/testdso
      running build_ext
      Traceback (most recent call last):
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/command/editable_wheel.py", line 156, in run
          self._create_wheel_file(bdist_wheel)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/command/editable_wheel.py", line 345, in _create_wheel_file
          files, mapping = self._run_build_commands(dist_name, unpacked, lib, tmp)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/command/editable_wheel.py", line 268, in _run_build_commands
          self._run_build_subcommands()
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/command/editable_wheel.py", line 295, in _run_build_subcommands
          self.run_command(name)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/cmd.py", line 318, in run_command
          self.distribution.run_command(command)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/dist.py", line 989, in run_command
          super().run_command(command)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
          cmd_obj.run()
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools_dso/dsocmd.py", line 577, in run
          _build_ext.run(self)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/command/build_ext.py", line 88, in run
          _build_ext.run(self)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/command/build_ext.py", line 345, in run
          self.build_extensions()
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/command/build_ext.py", line 467, in build_extensions
          self._build_extensions_serial()
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/command/build_ext.py", line 493, in _build_extensions_serial
          self.build_extension(ext)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools_dso/dsocmd.py", line 588, in build_extension
          self.dso2lib_pre(ext)
        File "/tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools_dso/dsocmd.py", line 222, in dso2lib_pre
          dsobase = os.path.dirname(import_module(parts[0]).__file__)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "<frozen posixpath>", line 152, in dirname
      TypeError: expected str, bytes or os.PathLike object, not NoneType
      /tmp/pip-build-env-wqunbmez/overlay/lib/python3.11/site-packages/setuptools/_distutils/dist.py:988: _DebuggingTips: Problem in editable installation.
      !!

What I think happens

Looking at the source, this exception is thrown while Setuptools-DSO is building the possible locations for an existing DSO. It looks for the init file of the root package (nsp in the reproduction instructions, testnsp in my MWE), but since this is a namespace package, this file does not exist. So while the import works (no ImportError so the except clause is not triggered), __file__ contains None and causes a TypeError when fed to os.path.dirname.

To solve it, I think it should be possible to set up a while (or even better, for) loop to look first at parts[0:1], see if it's a real package (__file__ is not None), and if it's not, try again with parts[0:2] and so on. Since I'm in this exact situation in a project of mine, I might make a PR for this, if no one does it before.

mdavidsaver commented 10 months ago

I have not had to work with namespace packages before. Reading through PEP420 I find:

...  The new namespace package:

    Has a __path__ attribute set to an iterable of the path strings that were found and recorded during the scan.
    Does not have a __file__ attribute.

I read the second point as saying that <ns_mod>.__file__ should not exist (so AttributeError). From #29 I take it that for the version(s) of python you use, instead __file__ exists, but is None. Am I misreading, or is the PEP incorrect?

I have a minimal working example here: https://gitlab.com/Thf772/setuptools-dso-multi-namespace-test

Could you translate this example into a test case? Like project2 under example/. With this, a GHA run should answer my question about portability/history.

$ git grep project2
.github/workflows/build.yml:          cd project2
.github/workflows/build.yml:          # install project2.
.github/workflows/build.yml:          cd project2
.github/workflows/build.yml:          (cd example/project2/src && python -m use_dsodemo.cli)
testme.sh:(cd project2/src && python -m use_dsodemo.cli 2>/dev/null) && die "error: worktree must be clean"
testme.sh:cd project2
testme.sh:(cd project2/src && PYTHONPATH=$X python -m use_dsodemo.cli)
testme.sh:cd project2
testme.sh:(cd project2/src && python -m use_dsodemo.cli 2>/dev/null) && die "error: worktree must be clean"
testme.sh:cd project2
Thf772 commented 9 months ago

From https://github.com/mdavidsaver/setuptools_dso/pull/29 I take it that for the version(s) of python you use, instead file exists, but is None. Am I misreading, or is the PEP incorrect?

I'm using Python 3.11, I don't know if that makes any difference. I don't think you've misread the PEP, it sounds like it should trigger an AttributeError but maybe it stays there as None for some API compatibility reasons, or some CPython-specific shenanigans...

I'll try setting up a test case for namespace packages.

Thf772 commented 9 months ago

Alright, I've added tests for both editable and non-editable installs of namespace packages. These tests pass on my computer with Python 3.11 and Windows 10. Let me know if there's anything you think I should change or improve.

Thf772 commented 9 months ago

@mdavidsaver any news on the PR workflow? This issue is still blocking for my project.

Thf772 commented 9 months ago

Thanks for the run, I think the tests failed because of a stupid mistake. I added Cython to the Deps step, but only to the download command and not the install command. So when trying to install my test package that has a Cython file, it looked for the generated C file which wasn't present. It should be fixed now.

Thf772 commented 9 months ago

Now apparently it's old version of Python not compatible with pyproject builds... I'll have to rewrite the setup files.

mdavidsaver commented 9 months ago

I have released 2.10a1, please let me know if this is sufficient.

Thf772 commented 9 months ago

Thanks for the help, I'm testing the new version right now on my project. I'll let you know how it works.

Thf772 commented 9 months ago

Alright, everything works for me now! Thanks again.

Also, I'm just noting this here for now, but for newer Python versions, it might be interesting to refactor the code to use setuptool's SubCommand class, which is the recommended way to add new commands supporting editable installs. I doubt old Setuptools and Python versions support it though, so it depends on whether supporting them is important for you.