[BUG] RST formatted README checks correctly for mac and linux, but fails on windows

fakufaku commented 1 year ago

setuptools version

setuptools==65.5.1

Python version

3.7, 3.8, 3.9, 3.10

OS

Microsoft Windows Server 2022

Additional environment information

These are parts of the continuous integration tests on github actions.

Current runner version: '2.298.2'
Operating System
  Microsoft Windows Server 2022
  10.0.20348
  Datacenter
Runner Image
  Image: windows-2022
  Version: 20221027.1
  Included Software: https://github.com/actions/runner-images/blob/win22/20221027.1/images/win/Windows2022-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/win22%2F20221027.1

Description

The compiled bdist_wheel does not pass twine check on windows system only. The check passes without problem for universal wheel (on ubuntu-latest) and bdist_wheel on macos-latest. The error on windows is shown below in "Output" and is about image directive that is not permitted in restructured text markup.

The problem occurs in repo: https://github.com/LCAV/pyroomacoustics
Faulty CI run: https://github.com/LCAV/pyroomacoustics/actions/runs/3437045775
README.rst file: https://github.com/LCAV/pyroomacoustics/blob/master/README.rst

The faulty directive (at top of README.rst file, with context below) is shown here. I don't see any directive under image.

.. image:: https://github.com/LCAV/pyroomacoustics/raw/master/logo/pyroomacoustics_logo_horizontal.png
  ¦:scale: 80 %
  ¦:alt: Pyroomacoustics logo
  ¦:align: left

------------------------------------------------------------------------------

.. image:: https://travis-ci.org/LCAV/pyroomacoustics.svg?branch=pypi-release
  ¦ :target: https://travis-ci.org/LCAV/pyroomacoustics
.. image:: https://readthedocs.org/projects/pyroomacoustics/badge/?version=pypi-release
  ¦ :target: http://pyroomacoustics.readthedocs.io/en/pypi-release/
  ¦ :alt: Documentation Status
.. image:: https://mybinder.org/badge_logo.svg
  ¦ :target: https://mybinder.org/v2/gh/LCAV/pyroomacoustics/master?filepath=notebooks%2Fpyroomacoustics_demo.ipynb
  ¦ :alt: Test on mybinder
.. image:: https://img.shields.io/discord/829534160812245012?color=%237289DA&label=pyroomacoustics%20Discord&logo=discord&logoColor=white
  ¦ :target: https://discord.gg/HQ3evGYk2s
  ¦ :alt: Pyroomacoustics discord server

Summary
-------

Expected behavior

All build succeed, or all fail. In this case, I believe they should all succeed.

How to Reproduce

git clone --recursive https://github.com/LCAV/pyroomacoustics.git
cd pyroomacoustics
pip install -U numpy Cython pybind11>=2.2 setuptools wheel twine
python setup.py bdist_wheel
twine check dist/*

I can't check outside of CI for windows since I don't own a windows system.

Output

The output on windows has a lot of compiler warnings and has several 10000 lines. The logs are available on github or upon demand. For the output of the twine check command only, see below.

On windows-latest with python 3.10. Also happens for python 3.7, 3.8, and 3.9.

2022-11-10T13:15:54.3257116Z Checking dist\pyroomacoustics-0.7.0-cp310-cp310-win_amd64.whl: FAILED
2022-11-10T13:15:54.3281582Z ERROR    `long_description` has syntax errors in markup and would not be
2022-11-10T13:15:54.3282037Z          rendered on PyPI.
2022-11-10T13:15:54.3282568Z          line 1: Error: Error in "image" directive:
2022-11-10T13:15:54.3282955Z          no content permitted.
2022-11-10T13:15:54.3283296Z
2022-11-10T13:15:54.3283654Z          .. image::
2022-11-10T13:15:54.3284070Z          https://github.com/LCAV/pyroomacoustics/raw/master/logo/pyroomacoustic
2022-11-10T13:15:54.3284450Z          s_logo_horizontal.png
2022-11-10T13:15:54.3284781Z
2022-11-10T13:15:54.3285092Z             :scale: 80 %
2022-11-10T13:15:54.3285831Z
2022-11-10T13:15:54.3286179Z             :alt: Pyroomacoustics logo
2022-11-10T13:15:54.3286910Z
2022-11-10T13:15:54.3287299Z             :align: left

On macos-latest with python 3.10

2022-11-10T13:13:48.3407300Z Checking dist/pyroomacoustics-0.7.0-cp310-cp310-macosx_10_15_x86_64.whl: ^[[32mPASSED^[[0m

abravalheri commented 1 year ago

Hi @fakufaku, thank you very much for the report.

I am having some trouble to run your reproducer:

I noticed that the lines pip install -U pytest setuptools wheel twine pytest & python setup.py bdist_wheel, will result in error because pybind11 is not installed.

If I install pybind11>=2.2 and try again, I get the following error:

...
pyroomacoustics\.venv\lib\site-packages\pybind11\include\pybind11/eigen.h(35): fatal error C1083: Cannot open include file: 'Eigen/Core': No such file or directory
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio\\2022\\BuildTools\\VC\\Tools\\MSVC\\14.30.30705\\bin\\HostX86\\x64\\cl.exe' failed with exit code 2

Probably there is a library missing?

Could you please simplify your reproducer to the minimum steps necessary and remove everything that is not related to the error you are seeing? (You can find more information about how to create a minimal reproducer in https://stackoverflow.com/help/minimal-reproducible-example). If possible, also try to use a virtualenv in your reproducer.

In an attempt to simplify the reproducer I tried the following (Windows 10, Python 3.9.13, PowerShell 5.1.19041.1682):

git clone https://github.com/LCAV/pyroomacoustics.git
cd pyroomacoustics

Rename-Item setup.py _orig_setup.py

@"
from setuptools import setup
from os import path

here = path.abspath(path.dirname(__file__))

with open(path.join(here, "README.rst"), encoding="utf-8") as f:
    long_description = f.read()

setup(
    name="pyroomacoustics",
    version="0.42",
    long_description=long_description,
    long_description_content_type="text/x-rst",
    author_email="fakufaku@gmail.ch",
    packages=[],
)
"@ | out-file -nonewline -encoding utf8 setup.py

python -m venv .venv
.venv\Scripts\python -m pip install -U setuptools wheel twine
.venv\Scripts\python setup.py bdist_wheel
.venv\Scripts\python -m twine check dist/*

As a result I get a PASSED score from twine:

Checking dist\pyroomacoustics-0.42-py3-none-any.whl: PASSED

fakufaku commented 1 year ago

Hi @abravalheri , thanks for your feedback. Sorry, the --recursive option is needed to bring some library. I have edited the original post, but here is the updated procedure.

git clone --recursive https://github.com/LCAV/pyroomacoustics.git
cd pyroomacoustics
pip install -U numpy Cython pybind11>=2.2 setuptools wheel twine
python setup.py bdist_wheel
twine check dist/*

fakufaku commented 1 year ago

And, by the way, the CI script is here. https://github.com/LCAV/pyroomacoustics/blob/master/.github/workflows/pythonpackage.yml

abravalheri commented 1 year ago

What seems to be happening here is a combination of several factors:

Your README.rst file seems to follow a Windows encoding (\r\n line endings)
You are using codecs.open instead of open to read the README.rst file, and according to Python docs:
No automatic conversion of '\n' is done on reading and writing.
- This means that the value that you are giving to setuptools has line endings in the form of \r\n.
- Setuptools simply passes these line endings on to the final metadata file.
- However, they clash with the rest of the metadata file which uses \n (I am also not 100% sure this kind of line endings are accepted in the packaging standards).
I suspect that on Linux/macOS, git might be automatically converting the files line endings to \n during the checkout/pull.

Could you please try replacing codecs.open with open or changing the encoding of your README.rst file to use \n line endings?

I think that guessing the line endings of long_description and correcting them automatically is outside of the scope of setuptools[^1].

[^1]: Please note that when long_description is left at setuptools care (e.g. by setting [metadata]\nlong_description = file: README.rst in the setup.cfg file), setuptools will use the built-in open function that automatically converts \r\n to \n.

fakufaku commented 1 year ago

@abravalheri Thank you very much for finding out the problem! 🙏 That would have been very hard to debug on my end. Replacing codecs.open by the default open has solved the problem. 👍 I am not sure why I was using it in the first place. I think it may have been for some python 2.7 compatibility issue, when it was still relevant... 🤷

pypa / setuptools