Update to 5.12 - Githubissues

ccordoba12 commented 4 years ago

Checklist

[x] Used a fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[x] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

Closes #60.

conda-forge-linter commented 4 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

ccordoba12 commented 4 years ago

@conda-forge-admin, please rerender

conda-forge-linter commented 4 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to re-render for you, but it looks like there was nothing to do.

ccordoba12 commented 4 years ago

@isuruf, @ocefpaf, there are a couple of issues to consider here:

PyQt 5.12 split QtWebEngine into a different package (pyqtwebengine) to reduce size installation. That's good for packages like Matplotlib or qtconsole, which don't require to render web content in Qt widgets. Others (like Spyder) require that extra functionality.

However, I don't know how to handle conditional dependencies in conda to support previous Qt versions. That's because if PyQt < 5.12, then there's no need for packages like Spyder to require pyqtwebengine. But if PyQt >= 5.12, then we need to require that package or Spyder will fail to start.

What should we do? Should we create a different feedstock for pyqtwebengine or not?
There's a strange bug on Windows when running configure.py (I think), which seems to point out to a faulty Qt build:

Error: Failed to determine the detail of your Qt installation. Try again using the --verbose flag to see more detail about the problem.

ccordoba12 commented 4 years ago

There's a strange bug on Windows when running configure.py

Sorry, forget that comment. That's only seen in Azure but not in Appveyor (don't know why though).

isuruf commented 4 years ago

For 1, you can create an empty pyqtwebengine for qt<5.12 and require pyqtwebengine in spyder

ccordoba12 commented 4 years ago

For 1, you can create an empty pyqtwebengine for qt<5.12

For that we would have to split QtWebEngine from PyQt 5.9 too, right? But I don't know how to do that.

isuruf commented 4 years ago

For that we would have to split QtWebEngine from PyQt 5.9 too, right? But I don't know how to do that.

No. For Qt 5.9, pyqtwebengine=5.9 will require pyqt=5.9 and that's it. It'll be an empty package with just the requirements.

ccordoba12 commented 4 years ago

Ok, got it. Thanks for the advice @isuruf!

Unfortunately, I don't have time to submit pyqtwebengine to staged-recipes. Besides, I don't know how to compile it, i.e. if it requires PyQt5 sources or not.

Could someone help me with that?

isuruf commented 4 years ago

pyqtwebengine manylinux wheel without the qtwebengine libraries is only half a MB, so we might as well have pyqtwebengine in pyqt until we split qt

ccordoba12 commented 4 years ago

Actually pyqtwebengine weights ~50 Mb:

Selección_009

isuruf commented 4 years ago

If you download it, you'll see that most of that is a folder called PyQt5/Qt which is in our qt=5.12 package. The 3 other files in PyQt5 are about half an MB in total.

ccordoba12 commented 4 years ago

you'll see that most of that is a folder called PyQt5/Qt which is in our qt=5.12 package

Yeah, you're right. Those libraries are part of our Qt package, so there's no need to split this one.

I'll continue with this PR then when I have more free time next week.

pkgw commented 4 years ago

@ccordoba12 Is there anything people can do to help with this?

ccordoba12 commented 4 years ago

You could open a new PR for this, if you want, because I'm really swamped right now, sorry.

leofang commented 4 years ago

@ccordoba12 could you reopen #61 then?

ccordoba12 commented 4 years ago

@leofang, that was the wrong approach. You need to take what I've done here and try to finish it.

leofang commented 4 years ago

No, as I said there you need a private copy of sip. Your approach cannot achieve that. I can try to combine both approaches to support the WebEngine, but a special handling of sip is a must.

ccordoba12 commented 4 years ago

No, as I said there you need a private copy of sip

Why not an installed version of sip? And if a private copy is inevitable, is it not build as part of PyQt5?

leofang commented 4 years ago

No, as I said there you need a private copy of sip

Why not an installed version of sip? And if a private copy is inevitable, is it not build as part of PyQt5?

See conda-forge/sip-feedstock#16 for the references. PyQt changed the build requirements in 5.11. I didn’t invent it.

isuruf commented 4 years ago

@leofang, does a symlink from site-packages/sip to site-package/PyQt5/sip work?

isuruf commented 4 years ago

Or a simple python file PyQt5/sip.py that imports all the functions from sip?

leofang commented 4 years ago

I doubt a simple from sip import * statement in PyQt5/sip.py would work. Take a look at the file configure.py from sip v4.19.18, the build option --sip-module is converted to a macro SIP_MODULE_NAME in the generated makefile. Not sure how it'd be used during build time of sip and PyQt5. Also, files like .dist-info and sip.pyi also depend on the --sip-module option and thus need to be taken care of.

Symlinking all relevant files might work, but I don't know enough how to overwrite paths in a conda build script. I do not have time to explore along this path.

One thing I'd like to note: I admit the wget command in #61 isn't legit. On some systems it'd fail. But as I marked there, it was a WIP.

hhslepicka commented 4 years ago

@leofang if the wget is the issue, you could add wget as a build dependency and run a python command to just download the proper sip package. https://anaconda.org/conda-forge/wget

leofang commented 4 years ago

@isuruf @ccordoba12 Based on the above discussion on pyqtwebengine it seems the proposed workaround is to extract PyQt5/QtWebEngine*.so from the wheel, put them into the built PyQt5, and neglect the rest of the files (PyQt5/Qt/*).

However, this doesn't work, as the three .so shared libraries are linked to PyQt5/Qt/lib/libQt5*.so (checked using ldd). It seems to me one just has to pip-install the pyqtwebengine wheel, unless the linking can be redirected to conda-forge's Qt.

leofang commented 4 years ago

@leofang if the wget is the issue, you could add wget as a build dependency and run a python command to just download the proper sip package. https://anaconda.org/conda-forge/wget

Thanks for the suggestion, but I think this route is difficult to manage. I found that specifying multiple sources in the recipe is much simpler and robust.

ccordoba12 commented 4 years ago

Based on the above discussion on pyqtwebengine it seems the proposed workaround is to extract PyQt5/QtWebEngine*.so from the wheel

No, that's not what needs to be done. You need to extract pyqtwebengine sources in this recipe so that module is compiled and packaged as part of this package.

leofang commented 4 years ago

pyqtwebengine source is unavailable either on PyPi or on the official website if I'm not mistaken. Isn't this the reason you guys were looking at the wheel?

ccordoba12 commented 4 years ago

on the official website if I'm not mistaken

It's on the PyQt website, you need to look for it.

leofang commented 4 years ago

OK found it. I looked at the wrong page. Life is a bit easier with source code available. Thanks.

conda-forge / pyqt-feedstock

Update to 5.12 #59