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

maturin fails if sys.stdout is an io.StringIO object #1814

Closed
1 of 2 tasks
zlind0 opened this issue Oct 24, 2023 · 5 comments
Closed
1 of 2 tasks

maturin fails if sys.stdout is an io.StringIO object #1814

zlind0 opened this issue Oct 24, 2023 · 5 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@zlind0
Copy link

zlind0 commented Oct 24, 2023

Bug Description

I am a package maintainer of a fedora-like linux distribution. In our case of using rpmbuild to build a python package by maturin, it throws an exception. This happens in the stage that some rpmbuild-related python scripts are called to generate the dependency of package.

Handling maturin>=1.0,<2.0 from build-system.requires
Requirement satisfied: maturin>=1.0,<2.0
   (installed: maturin 1.3.0)
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.10 at /usr/bin/python3
📡 Using build options features, bindings from pyproject.toml
Traceback (most recent call last):
  File "/usr/lib/rpm/anolis/pyproject_buildrequires.py", line 428, in main
    generate_requires(
  File "/usr/lib/rpm/anolis/pyproject_buildrequires.py", line 360, in generate_requires
    generate_run_requirements(backend, requirements)
  File "/usr/lib/rpm/anolis/pyproject_buildrequires.py", line 265, in generate_run_requirements
    dir_basename = prepare_metadata('.')
  File "/usr/lib64/python3.10/site-packages/maturin/__init__.py", line 210, in prepare_metadata_for_build_wheel
    sys.stdout.buffer.write(_output)
AttributeError: '_io.StringIO' object has no attribute 'buffer'
error: Bad exit status from /var/tmp/rpm-tmp.SoHxqG (%generate_buildrequires)

This exception turns out to be a known risk of using sys.stdout.buffer while the context of standard streams is unknown. By using sys.stdout.buffer, it is supposed that sys.stdout is io.TextIOWrapper. This is true for most of the common usages. However, in some cases, such as a library usage, sys.stdout can be io.StringIO, which does not have buffer, and here comes the exception.

It's been officially documented at here.

There is also similar issues for flake, at [1] [2].

Your maturin version (maturin --version)

1.3.0

Your Python version (python -V)

3.10.13

Your pip version (pip -V)

23.1

What bindings you're using

None

Does cargo build work?

  • Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

  • Yes

Steps to Reproduce

I have packed all the resources into a tarball. By downloading and extracting it you can reproduce this problem.

wget https://a.lind0.space/safetensors-0.4.0-bug.tar.gz
tar -xzf safetensors-0.4.0-bug.tar.gz
cd safetensors-0.4.0/
python3 -s anolis/pyproject_buildrequires.py --generate-extras --python3_pkgversion 3

Then, you will get this output:

Handling maturin>=1.0,<2.0 from build-system.requires
Requirement satisfied: maturin>=1.0,<2.0
   (installed: maturin 1.3.0)
(python3dist(maturin) < 2~~ with python3dist(maturin) >= 1)
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.10 at /usr/bin/python3
📡 Using build options features, bindings from pyproject.toml
Traceback (most recent call last):
  File "/root/python-safetensors/safetensors-0.4.0/anolis/pyproject_buildrequires.py", line 428, in main
    generate_requires(
  File "/root/python-safetensors/safetensors-0.4.0/anolis/pyproject_buildrequires.py", line 360, in generate_requires
    generate_run_requirements(backend, requirements)
  File "/root/python-safetensors/safetensors-0.4.0/anolis/pyproject_buildrequires.py", line 265, in generate_run_requirements
    dir_basename = prepare_metadata('.')
  File "/usr/lib64/python3.10/site-packages/maturin/__init__.py", line 210, in prepare_metadata_for_build_wheel
    sys.stdout.buffer.write(_output)
AttributeError: '_io.StringIO' object has no attribute 'buffer'
@zlind0 zlind0 added the bug Something isn't working label Oct 24, 2023
@messense
Copy link
Member

What's your proposed solution here? From reading the issues you linked, I'm leaning towards that if you are patching sys.stdout it's your responsibility to use io.TextIOWrapper.

@konstin What do you think?

@konstin
Copy link
Member

konstin commented Oct 24, 2023

Normally, i'd say maturin should support mocked sys.stdout, but for the case of PEP 517 specifically, the spec says:

Frontends should call each hook in a fresh subprocess, so that backends are free to change process global state (such as environment variables or the working directory). A Python library will be provided which frontends can use to easily call hooks this way.

I would argue that a linux distribution should follow the PEP 517 recommendation. You can use the build library which provides an easy integration.

@messense messense added the question Further information is requested label Oct 25, 2023
@zlind0
Copy link
Author

zlind0 commented Oct 25, 2023

What's your proposed solution here? From reading the issues you linked, I'm leaning towards that if you are patching sys.stdout it's your responsibility to use io.TextIOWrapper.

@konstin What do you think?

Sorry I am not that familiar with python so I am searching for solution as well. If I find an elegant solution, I will comment.

Actually in the case I am not patching the sys.stdout, the type of sys.stdout differs because the standard output is redirected.

@messense
Copy link
Member

Actually in the case I am not patching the sys.stdout, the type of sys.stdout differs because the standard output is redirected.

How exactly was the stdout redirected? I'm not sure why redirect stdout (like using > /dev/null in shell) would change it to io.StringIO.

@messense
Copy link
Member

Actually in the case I am not patching the sys.stdout, the type of sys.stdout differs because the standard output is redirected.

How exactly was the stdout redirected? I'm not sure why redirect stdout (like using > /dev/null in shell) would change it to io.StringIO.

Ok, found it in your code

@contextlib.contextmanager
def hook_call():
    captured_out = StringIO()
    with contextlib.redirect_stdout(captured_out):
        yield
    for line in captured_out.getvalue().splitlines():
        print_err('HOOK STDOUT:', line)

so you are effectively patching sys.stdout with io.StringIO().

I'm going to close this as wontfix.

@messense messense closed this as not planned Won't fix, can't repro, duplicate, stale Oct 25, 2023
@messense messense added wontfix This will not be worked on and removed question Further information is requested labels Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants