python-qt-tools / PyQt5-stubs

Stubs for PyQt5
GNU General Public License v3.0
68 stars 31 forks source link

Deprecate the docker build process #193

Open bluebird75 opened 2 years ago

bluebird75 commented 2 years ago

Hi,

Now that the PyQt5 stubs are released as part of PyQt5 pypi package, do we really need to keep this build process in place ?

A few facts :

@The-Compiler shared in an email that PyQtWebkit pyi generation depends on the Docker file for its stubs generation. However, the Docker file does not contain anything related to PyQtWebkit and PyQt5 does not ship QtWebkit modules.

So, I am under the impression that the Docker file can be removed because pyi provides the same services.

For QtWebkit, if it's not shipped with PyQt5, it might be a good idea to create a dedicated stub project file on pypi (PyQtWebkit-stubs) else people are unlikely to find the corresponding stubs.

The-Compiler commented 2 years ago

PyQt5 does ship with QtWebKit wrappers, the only place it's not shipped are the binary wheels. So far, the stubs here do contain QtWebKit stubs (and arguably this is the right place, as the code is part of the PyQt5 package). It seems odd to remove/split them so late in Qt 5.15's lifecycle.

altendky commented 2 years ago

What would it take to create an automated replacement for the docker process? I would generally look for a replacement before deletion.

bluebird75 commented 2 years ago

PyQt5 does ship with QtWebKit wrappers, the only place it's not shipped are the binary wheels.

Aaah, that's why I could not find them. I only looked at the wheels. And that's why there is a separate project shipping the built version, for Windows, it is probably not simple to build.

I am not imaginating dropping the stubs. The question is really how to maintain them. Do you remember how the stubs were built ? Because they are not part of the Docker process. This package is also ignored by default in the stubtest, so it really has a distinct status within PyQt5-stubs right now.

The-Compiler commented 2 years ago

Do you remember how the stubs were built ? Because they are not part of the Docker process.

They are. Like I said above, QtWebKit is part of the PyQt5 sources, and the build process builds PyQt from those sources.

This package is also ignored by default in the stubtest, so it really has a distinct status within PyQt5-stubs right now.

Why that is I don't know. Looks like @altendky moved them from "to review" to the allowlist in 881c597e100a32b29d9a7049e8bcebb5e2857fc9, but the commit message only talks about __init__ and __call__ methods.

altendky commented 2 years ago

We run against published wheels so if they don't provide QtWebKit then they won't be available to stubcheck so we have to ignore complaints about them. Well, 'have to' unless we build our own pyqt5 with webkit. At least I think that makes sense...

@bluebird75, have you run the docker process and seen what comes out of it? I forget, but if it builds into the PyQt5-stubs directory then delete the contents first so you can actually see what the docker run makes.

The-Compiler commented 2 years ago

Right, makes sense. I suppose the convenience of being able to run tox -e py310-linux or whatever beats the complexity of actually getting a proper environment where QtWebKit is available. Thus, I believe it's just a pragmatic decision to skip those.

altendky commented 2 years ago

We could build webkit-including wheels for our own reuse, or even publish them on PyPI. But, given the state of this project (starting to turn around with the new team members :]) and also pyqt-tools falling behind... maybe we're not going to get to that.

bluebird75 commented 2 years ago

Thanks for the clarification, I get it now. I got confused because there is no section dedicated to it in the Dockerfile.

Nowadays, we do have a wheel for QtWebkit on Windows though. So, I can probably stubtest it there at least.

bluebird75 commented 2 years ago

What would it take to create an automated replacement for the docker process?

Apart from QtWebkit, this already exists. @TilmanK built it in pyqt6-stubs: https://github.com/python-qt-tools/PyQt6-stubs/blob/f623a641cd5cdff53342177e4fbbf9cae8172336/generate_upstream.py#L28