Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setuptools 69.5 breaks mpi4py installation #484

Closed
paulromano opened this issue Apr 12, 2024 · 13 comments
Closed

setuptools 69.5 breaks mpi4py installation #484

paulromano opened this issue Apr 12, 2024 · 13 comments
Assignees
Labels

Comments

@paulromano
Copy link
Contributor

I noticed a CI error on a project of mine when building mpi4py and after some digging determined that it is caused by a newly released version of setuptools (69.4.0) that breaks an assumption in within mpi4py. The error during installation is:

        File "/tmp/pip-build-env-qxqtzla9/overlay/lib/python3.10/site-packages/setuptools/_distutils/ccompiler.py", line 1232, in gen_lib_options
          lib_opts.extend(always_iterable(compiler.runtime_library_dir_option(dir)))
        File "/tmp/pip-install-u2viu3lx/mpi4py_70e5ecd7d41d4c9fb50ac4e92d5eb59f/conf/mpidistutils.py", line 68, in rpath_option
          if option.startswith('-R'):
      AttributeError: 'list' object has no attribute 'startswith'
      [end of output]

It looks like the file in question, conf/mpidistutils.py, makes an implicit assumption that the UnixCCompiler.runtime_library_dir_option function from setuptools/distutils returns a string:

from distutils.unixccompiler import UnixCCompiler
rpath_option_orig = UnixCCompiler.runtime_library_dir_option
def rpath_option(compiler, dir):
option = rpath_option_orig(compiler, dir)
if sys.platform == 'linux':
if option.startswith('-R'):

In setuptools 69.4.0, a change was made in that function whereby it may return a list:
pypa/setuptools@a131f83

@niyiyu
Copy link

niyiyu commented Apr 12, 2024

Having the same issue with my package. Surprised to see this issue TBH. Trying to find a solution.

@paulromano
Copy link
Contributor Author

@niyiyu I ended up doing the following in my GHA:

pip install --upgrade "setuptools<69.4.0" build wheel
pip install --no-build-isolation --no-binary=mpi4py mpi4py

To the mpi4py maintainers, I'd recommend putting a version limit on setuptools in pyproject.toml and putting out a new version until the underlying issue can be fixed. Otherwise downstream dependencies are stuck with ugly hacks like what I have above.

@niyiyu
Copy link

niyiyu commented Apr 12, 2024

@paulromano thanks! We did try to set a limit on the setuptools in our PR but noticed that the setuptools we are using is actually even lower version at 65.5.0. Still digging deeper into this. Thanks for the advice!

@dalcinl
Copy link
Member

dalcinl commented Apr 13, 2024

setuptools broke the API of public methods of public class in public modules. Even if I publish a new version, all previously published versions are uninstallable with the new setuptools. This is the second time setuptools break mpi4py installation. You should report the issue upstream to setuptools maintainers.

To the mpi4py maintainers, I'd recommend putting a version limit on setuptools in pyproject.toml

I'm not going to do that, more below...

and putting out a new version until the underlying issue can be fixed.

I'm almost about to release a new 4.0.0 release. I'll look into what's the best course of action tomorrow. In the meantime, please report the issue to setuptools. Upstream issue report already done.

Otherwise downstream dependencies are stuck with ugly hacks like what I have above.

And I'll not add an ugly upper bound pin that I'll have to maintain forever just because users find it inconvenient to add temporary ugly workaround. Again, this is the second time this happens in 20 years, so occurrences are rare, and it is ultimately not mpi4py's fault, because setuptools breaked API on everyone's back.

@dalcinl
Copy link
Member

dalcinl commented Apr 13, 2024

Reported upstream at pypa/distutils#246.

@paulromano
Copy link
Contributor Author

Completely understand @dalcinl. Regarding the change in setuptools, it looks like the possibility has existed for a while that the CCompiler.runtime_library_dir_option method returns either a string or a list of strings, so the assumption in mpi4py that it is always a string may not be safe. Unfortunately the API documentation doesn't say what type a user should expect.

@dalcinl
Copy link
Member

dalcinl commented Apr 13, 2024

For the time being, I pushed fixes to the maint branch 998e295 and the master branch 484e365 (this last want getting rid of workaround for good).

dalleyg added a commit to dalleyg/pyrseus that referenced this issue Apr 13, 2024
setuptools just changed their API without doing a good job advertising
what the full API already was, breaking mpi4py's installer. This patch
is an attempt to workaround the issue in our unit tests.
@dalcinl dalcinl changed the title setuptools 69.4.0 breaks mpi4py installation setuptools 69.5 breaks mpi4py installation Apr 13, 2024
@dalcinl
Copy link
Member

dalcinl commented Apr 13, 2024

@paulromano @danielk333 My usual local setup and CI testing infrastructure seems to not trigger the issue, most likely because I'm not in need of passing rpath explicitly. I could really use your help to verify my fixes. Any chance you can try building from branch mpi4py@release? The following should be enough for testing on your side:

python -m venv /tmp/venv
source /tmp/venv/bin/activate
python -m pip install https://github.com/mpi4py/mpi4py/tarball/release
python -m mpi4py --version
python -m mpi4py --mpi-lib-version

@dalcinl
Copy link
Member

dalcinl commented Apr 13, 2024

setuptools 69.5.1 was just released with a fix for this issue. Everything should be back to normal. Anyway, I'm planning to make a new mpi4py 3.1.6 release.

@danielk333
Copy link

@dalcinl Oh fantastic! That went at rocket speed while I was asleep. Since they released a fix i guess i wont test building anything, but feel free to poke me if me running some testing on my setup would be of help in the future.

@dalcinl
Copy link
Member

dalcinl commented Apr 14, 2024

@danielk333 I'm waiting for Windows wheels to finish building and I'll publish a new mpi4py release. Together with the new setuptools patch release, I'm hopeful this issue will be solved for good.

@mrmundt
Copy link

mrmundt commented Apr 16, 2024

I'm not sure if this is related, but we are also having issues in our CI specifically when installing from conda. That run has verbosity turned up. I traced the failure to the mpi4py installation but the logs are relatively unhelpful.

EDIT: I pinned to 3.1.5 and our CI runs normally, so it's something in 3.1.6.

@dalcinl
Copy link
Member

dalcinl commented Apr 17, 2024

@mrmundt That seems totally unrelated. You should submit an issue to the mpi4py-feedstock repository https://github.com/conda-forge/mpi4py-feedstock and provide a minimal reproducer (e.g. the exact conda create .. environment line that leads to failure).

@dalcinl dalcinl closed this as completed Apr 18, 2024
@dalcinl dalcinl unpinned this issue May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants