Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to 5.12 #59

Closed
wants to merge 4 commits into from
Closed

Update to 5.12 #59

wants to merge 4 commits into from

Conversation

ccordoba12
Copy link
Contributor

@ccordoba12 ccordoba12 commented Aug 8, 2019

Checklist

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

Closes #60.

@conda-forge-linter
Copy link

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
Copy link
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-linter
Copy link

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Member

isuruf commented Aug 8, 2019

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

recipe/meta.yaml Outdated Show resolved Hide resolved
@ccordoba12
Copy link
Contributor Author

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
Copy link
Member

isuruf commented Aug 8, 2019

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
Copy link
Contributor Author

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
Copy link
Member

isuruf commented Aug 8, 2019

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
Copy link
Contributor Author

Actually pyqtwebengine weights ~50 Mb:

Selección_009

@isuruf
Copy link
Member

isuruf commented Aug 8, 2019

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.

image

@ccordoba12
Copy link
Contributor Author

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.

@ccordoba12 ccordoba12 mentioned this pull request Aug 9, 2019
@leofang leofang mentioned this pull request Aug 9, 2019
5 tasks
@leofang leofang mentioned this pull request Aug 20, 2019
@pkgw
Copy link

pkgw commented Sep 16, 2019

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

@ccordoba12
Copy link
Contributor Author

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

@leofang
Copy link
Member

leofang commented Sep 17, 2019

@ccordoba12 could you reopen #61 then?

@ccordoba12
Copy link
Contributor Author

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

@leofang
Copy link
Member

leofang commented Sep 17, 2019

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
Copy link
Contributor Author

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
Copy link
Member

leofang commented Sep 17, 2019

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
Copy link
Member

isuruf commented Sep 17, 2019

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

@isuruf
Copy link
Member

isuruf commented Sep 17, 2019

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

@leofang
Copy link
Member

leofang commented Sep 17, 2019

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
Copy link

@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
Copy link
Member

leofang commented Sep 19, 2019

@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
Copy link
Member

leofang commented Sep 19, 2019

@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
Copy link
Contributor Author

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
Copy link
Member

leofang commented Sep 19, 2019

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
Copy link
Contributor Author

on the official website if I'm not mistaken

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

@leofang
Copy link
Member

leofang commented Sep 19, 2019

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

@isuruf isuruf closed this in #61 Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants