pypa / readme_renderer

Safely render long_description/README files in Warehouse
Apache License 2.0
158 stars 89 forks source link

RST cannot contain header only - cause silent crash #149

Closed kamichal closed 2 years ago

kamichal commented 5 years ago

If the source README.rst contains header only, e.g.:

Header
======

The twine check command claims the markup is invalid for no reason:

twine check 'dist/*'
Checking distribution dist/foo-0.0.1.tar.gz: Failed
The project's long_description has invalid markup which will not be rendered on PyPI. The following syntax errors were detected:

Without listing any errors. Thats the end of stdout/stderr.

If I change the README.rst by adding just single letter at end, e.g.:

Header
======
a

or clear it contents everything is ok:

twine check 'dist/*'
Checking distribution dist/grot-0.0.1.tar.gz: Passed

I debugged it a bit. And got to the line 99 in site-packages/readme_renderer/rst.py: rendered = parts.get("docinfo", "") + parts.get("fragment", "") and the rendered string is empty. (both docinfo and fragment contain empty strings). It corrupts the later if:

    if rendered:
        return clean(rendered)
    else:
        return None

...causing render function return None, which is interpreted by twine.check as error, but no warnings/errors are collected by used stream.

miketheman commented 2 years ago

Looked at this a bit, and discovered that a header-only .rst file emits an H1 html tag, which is filtered out by design via setting (from 8 years ago): https://github.com/pypa/readme_renderer/blob/5ad13fd852f4067518c9f4da2103d720fa90ca4e/readme_renderer/rst.py#L58-L59

(overrides docutils default: https://docutils.sourceforge.io/docs/user/config.html#writer-specific-defaults-2 )

It's possible that is intentional - to prevent duplication of the project title inside the body, but it does highlight a difference between the rst and markdown render behaviors. I found an example of this on my own library, when I switched from rst to md between versions https://pypi.org/project/pytest-socket/0.3.5/ vs https://pypi.org/project/pytest-socket/0.4.0/

But that's an issue for another time!


The bug here, as it appears to me, is that twine didn't emit a warning or error message to help the user understand what went wrong - as the reporter said - a "silent crash". And twine can't know if there's a warning, if this library doesn't emit one.

di commented 2 years ago

It's possible that is intentional - to prevent duplication of the project title inside the body, but it does highlight a difference between the rst and markdown render behaviors.

It's intentional! For exactly the reason you highlighted.

kamichal commented 2 years ago

It's intentional! For exactly the reason you highlighted.

Then the error should be given and possibly explained when it crashes with such an "intention".