Bug 2026630

This breaks plenty of other packages:

$ repoquery -q --repo=rawhide{,-source} --whatrequires python3-aiohttp
black+d-0:21.11~b0-1.fc36.noarch
electrum-0:4.1.5-1.fc35.noarch
gns3-server-0:2.2.26-1.fc36.noarch
go2rpm-0:1.5.0-3.fc35.noarch
home-assistant-cli-0:0.9.4-1.fc36.noarch
home-assistant-cli-0:0.9.4-1.fc36.src
postfix-mta-sts-resolver-0:1.0.0-6.fc35.noarch
postfix-mta-sts-resolver-0:1.0.0-6.fc35.src
python-accuweather-0:0.0.11-4.fc35.src
python-aioeafm-0:1.0.0-4.fc35.src
python-aioflo-0:0.4.2-4.fc35.src
python-aiogqlc-0:1.0.4-5.fc35.src
python-aiohttp-cors-0:0.7.0-14.fc35.src
python-aiohttp-sse-client-0:0.2.0-4.fc35.src
python-aionotion-0:2.0.3-5.fc35.src
python-aioresponses-0:0.7.2-3.fc35.src
python-aresponses-0:2.1.4-4.fc35.src
python-async-upnp-client-0:0.14.15-4.fc35.src
python-august-0:0.25.2-4.fc35.src
python-black-0:21.11~b0-1.fc36.src
python-cs-0:3.0.0-4.fc36.src
python-daikin-0:2.4.0-4.fc35.src
python-deconz-0:76-4.fc35.src
python-engineio-0:4.3.0-1.fc36.src
python-enturclient-0:0.2.1-5.fc35.src
python-epson-projector-0:0.2.3-4.fc35.src
python-fsspec-0:2021.10.1-1.fc36~bootstrap.src
python-geoip2-0:4.4.0-1.fc36.src
python-gios-0:0.1.4-5.fc35.src
python-google-resumable-media-0:2.1.0-1.fc36.src
python-haversion-0:21.11.1-1.fc36.src
python-insteon-0:1.0.8-4.fc35.src
python-matrix-nio-0:0.18.6-1.fc36.src
python-msrest-0:0.6.21-4.fc35.src
python-nikola-0:8.1.3-5.fc36.src
python-pyairnow-0:1.1.0-4.fc35.src
python-pyairvisual-0:5.0.5-4.fc35.src
python-pyopenuv-0:2.0.0-4.fc35.src
python-pysqueezebox-0:0.5.5-4.fc35.src
python-pytile-0:5.2.3-1.fc36.src
python-slackclient-0:3.11.2-1.fc36.src
python-socketio-0:5.5.0-1.fc36.src
python-teslajsonpy-0:0.11.0-3.fc35.src
python-vcrpy-0:4.1.1-4.fc35.src
python-wled-0:0.4.4-4.fc35.src
python3-accuweather-0:0.0.11-4.fc35.noarch
python3-afsapi-0:0.0.4-4.fc35.noarch
python3-aioambient-0:1.3.0-1.fc36.noarch
python3-aiocurrencylayer-0:0.1.3-4.fc35.noarch
python3-aioeafm-0:1.0.0-4.fc35.noarch
python3-aioflo-0:0.4.2-4.fc35.noarch
python3-aiogqlc-0:1.0.4-5.fc35.noarch
python3-aioguardian-0:1.0.4-4.fc35.noarch
python3-aiohttp-cors-0:0.7.0-14.fc35.noarch
python3-aiohttp-negotiate-0:0.11-16.fc35.noarch
python3-aiohttp-socks-0:0.6.0-3.fc35.noarch
python3-aiohttp-sse-client-0:0.2.0-4.fc35.noarch
python3-aiohue-0:2.2.0-6.fc35.noarch
python3-aioiotprov-0:0.0.7-4.fc35.noarch
python3-aionotion-0:2.0.3-5.fc35.noarch
python3-aioresponses-0:0.7.2-3.fc35.noarch
python3-aiorestapi-0:0.1.1-6.fc35.noarch
python3-aiounifi-0:23-4.fc35.noarch
python3-aresponses-0:2.1.4-4.fc35.noarch
python3-async-upnp-client-0:0.14.15-4.fc35.noarch
python3-connect-box-0:0.3.0-1.fc36.noarch
python3-coronavirus-0:1.1.1-4.fc35.noarch
python3-cs+async-0:3.0.0-4.fc36.noarch
python3-daikin-0:2.4.0-4.fc35.noarch
python3-deconz-0:76-4.fc35.noarch
python3-dingz-0:0.3.0-4.fc35.noarch
python3-discord-0:1.7.3-1.fc36.noarch
python3-engineio+asyncio_client-0:4.3.0-1.fc36.noarch
python3-enturclient-0:0.2.1-5.fc35.noarch
python3-epson-projector-0:0.2.3-4.fc35.noarch
python3-gekitchen-0:0.2.19-4.fc35.noarch
python3-geoip2-0:4.4.0-1.fc36.noarch
python3-gios-0:0.1.4-5.fc35.noarch
python3-glances-api-0:0.2.0-12.fc35.noarch
python3-haversion-0:21.11.1-1.fc36.noarch
python3-hole-0:0.6.0-1.fc36.noarch
python3-insteon-0:1.0.8-4.fc35.noarch
python3-luftdaten-0:0.6.5-1.fc36.noarch
python3-matrix-nio-0:0.18.6-1.fc36.noarch
python3-metno-0:0.8.1-4.fc35.noarch
python3-msrest-0:0.6.21-4.fc35.noarch
python3-mystrom-0:2.0.0-4.fc35.noarch
python3-netdata-0:0.2.0-6.fc35.noarch
python3-nikola-0:8.1.3-5.fc36.noarch
python3-omnilogic-0:0.4.5-1.fc36.noarch
python3-opendata-transport-0:0.2.1-8.fc35.noarch
python3-opensensemap-api-0:0.1.5-12.fc35.noarch
python3-pyairnow-0:1.1.0-4.fc35.noarch
python3-pyairvisual-0:5.0.5-4.fc35.noarch
python3-pyiqvia-0:0.3.3-3.fc35.noarch
python3-pyopenuv-0:2.0.0-4.fc35.noarch
python3-pysqueezebox-0:0.5.5-4.fc35.noarch
python3-pytest-aiohttp-0:0.3.0-13.fc35.noarch
python3-pytile-0:5.2.3-1.fc36.noarch
python3-regenmaschine-0:3.1.5-1.fc36.noarch
python3-slixmpp-0:1.7.1-3.fc35.x86_64
python3-socialscan-0:1.4.2-1.fc36.noarch
python3-socketio+asyncio_client-0:5.5.0-1.fc36.noarch
python3-subarulink-0:0.3.11-4.fc35.noarch
python3-teslajsonpy-0:0.11.0-3.fc35.noarch
python3-volkszaehler-0:0.2.1-4.fc35.noarch
python3-volvooncall-0:0.8.12-5.fc35.noarch
python3-waqiasync-0:1.0.0-4.fc35.noarch
python3-webthing-ws-0:0.1.0-8.fc35.noarch
python3-wled-0:0.4.4-4.fc35.noarch
xcat-0:1.0.4-6.fc35.noarch

It was caused by this update https://src.fedoraproject.org/rpms/python-async-timeout/c/ff75bbc0f2d92cb4c3b8b65131de7de11794aeba?branch=rawhide

This is resolved with 3.8.1, but I'm having issues with the unbundling patch.

Build is failing unpatched:

gcc -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/tmp/build-env-bfeoxgtp/include -I/usr/include/python3.10 -c aiohttp/_websocket.c -o build/temp.linux-x86_64-3.10/aiohttp/_websocket.o
cc1: fatal error: aiohttp/_websocket.c: No such file or directory
compilation terminated.
error: command '/usr/lib64/ccache/gcc' failed with exit code 1

A quick investigation shows that upstream switched from http-parser to llhttp[1] in 3.8.0[2].

This dependency is kind of exotic in that it generates C code from TypeScript. There’s a decent chance it could be packaged successfully by collecting the build dependencies with nodejs-packaging-bundler. It will surely be a bit fussy.

Even leaving aside the desire to unbundle, adding an additional source archive corresponding to the git submodule for llhttp or switching from the GitHub source archive to the PyPI source archive will not solve the problem, because the NodeJS/TypeScript code generators still need to be run during the build in order to obtain the llhttp C sources.

[1] https://github.com/nodejs/llhttp
[2] https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst#380-2021-10-31

aiohttp >= 3.8.0 has new dependencies. Those dependencies are not available at the moment.

Should we roll back to python-async-timeout 3?

(In reply to Ben Beasley from comment #4)
> A quick investigation shows that upstream switched from http-parser to
> llhttp[1] in 3.8.0[2].
> 
> This dependency is kind of exotic in that it generates C code from
> TypeScript. There’s a decent chance it could be packaged successfully by
> collecting the build dependencies with nodejs-packaging-bundler. It will
> surely be a bit fussy.
> 
> Even leaving aside the desire to unbundle, adding an additional source
> archive corresponding to the git submodule for llhttp or switching from the
> GitHub source archive to the PyPI source archive will not solve the problem,
> because the NodeJS/TypeScript code generators still need to be run during
> the build in order to obtain the llhttp C sources.
> 
> [1] https://github.com/nodejs/llhttp
> [2]
> https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst#380-2021-10-31

I’ve posted a review request for llhttp:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2029461

As mentioned, the build is somewhat exotic since TypeScript is transpiled to C within the NodeJS ecosystem, and the resulting sources are used to build a plain C library. The spec file therefore “partially” uses the NodeJS packaging guidelines.

I think the result is defensible based on the spirit of the guidelines, but a reviewer will need to exercise judgement to decide if they agree. Hopefully someone following this issue is willing to take the review.

----

It looks like 3.8.0+ will also require https://pypi.org/project/frozenlist/ and https://pypi.org/project/aiosignal/, which should be more straightforward to package.

Now that llhttp is in the repos (thanks for the review), I’ve submitted the remaining two new dependencies for review:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2029651
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2029682

Both of these were split out from aiohttp.

*** Bug 2029837 has been marked as a duplicate of this bug. ***

Thanks for doing all that work, Ben.

(In reply to Major Hayden 🤠 from comment #10)
> Thanks for doing all that work, Ben.

You’re welcome. I think that takes care of all the missing new dependencies.

I haven’t started working on a PR to update python-aiohttp, and to be honest I’d be just as happy if I didn’t have to. However, I’ll probably revisit it soon if nobody else picks up the torch right away.

I'll give it a try, Ben.

LGTM in COPR: https://copr.fedorainfracloud.org/coprs/mhayden/aiohttp-fix/builds/

I think we just need to do a build and a bodhi update for python-aiohttp and we should be all set. I kicked off a build in Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=80029143

Koji build looks good. Fabian, would you be able to look over the Koji build and submit an update for python-aiohttp?

https://koji.fedoraproject.org/koji/taskinfo?taskID=80029143

Rawhide updates are created automatically: https://bodhi.fedoraproject.org/updates/FEDORA-2021-1d0af83cbb

Hello,

Please note that this comment was generated automatically. If you feel that this output has mistakes, please contact me via email (mhroncok).

All subpackages of a package against which this bug was filled are now installable or removed from Fedora 36.

Thanks for taking care of it!

The update "broke" python-discord: bz2032935. But it was already broken by this BZ, so it is now a net benefit.

Also note that the package now bundles llhttp and the bundled(llhttp) provide is in the wrong place in the spec, so it is not provided by the actually built python3-aiohttp package.

I’ll take a look at unbundling llhttp. It should be a pretty small patch.

It seems there is a LLHTTP_STRICT_MODE preprocessor macro that controls the parsing mode (strict or loose) and needs to be set at compile time. I didn’t notice that when I packaged it. The macro defaults to 0, so that is how the the current separate llhttp library package is compiled. There isn’t really any clear upstream documentation for this parsing mode.

Surveying existing llhttp bundlers:
- python-aiohttp sets LLHTTP_STRICT_MODE 1 on its bundled llhttp
- python-httptools does not
- nodejs does not, as far as I can tell

It’s almost as if the llhttp package should be providing two versions of the library, one with strict parsing enabled and one without. That seems a little tricky (one flavor would have to be renamed, like “llhttp_strict”, I guess), but maybe it’s the way to go.

(In reply to Ben Beasley from comment #19)
> It’s almost as if the llhttp package should be providing two versions of the
> library, one with strict parsing enabled and one without. That seems a
> little tricky (one flavor would have to be renamed, like “llhttp_strict”, I
> guess), but maybe it’s the way to go.

In the original http_parser.c, the corresponding option says:

https://github.com/nodejs/http-parser/blob/main/http_parser.h#L50

> /* Compile with -DHTTP_PARSER_STRICT=0 to make less checks, but run
>  * faster
>  */
> #ifndef HTTP_PARSER_STRICT
> # define HTTP_PARSER_STRICT 1
> #endif

and it defaults to strict mode (vs. llhttp, which defaults to loose mode).

At the moment, I’m thinking that if performance is the only reason for loose mode, the best course of action is probably to switch the Fedora llhttp package to build with LLHTTP_STRICT_MODE=1, and let any packages that *really* need loose mode for performance figure out how to build with bundled llhttp.

However, I’m going to wait a few days before doing anything, in case someone has useful input.

(In reply to Ben Beasley from comment #20)
> At the moment, I’m thinking that if performance is the only reason for loose
> mode, the best course of action is probably to switch the Fedora llhttp
> package to build with LLHTTP_STRICT_MODE=1, and let any packages that
> *really* need loose mode for performance figure out how to build with
> bundled llhttp.
> 
> However, I’m going to wait a few days before doing anything, in case someone
> has useful input.

I’m proceeding with enabling LLHTTP_STRICT_MODE in the separate llhttp package.

> - python-aiohttp sets LLHTTP_STRICT_MODE 1 on its bundled llhttp

I don’t know why I said this; aiohttp actually disables strict mode, LLHTTP_STRICT_MODE 0.

Since none of the packages that currently bundle llhttp want strict mode, I suppose I should go back to disabling it in the separate llhttp package.

PR created: https://src.fedoraproject.org/rpms/python-aiohttp/pull-request/2

- Stop disabling debuginfo
- Unbundle `llhttp`
- Run the test suite