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

dev-python/*: bump twisted and some packages that depend on it to python 3.10 #20835

Closed
wants to merge 6 commits into from

Conversation

vaartis
Copy link
Contributor

@vaartis vaartis commented May 16, 2021

Separated from #20828, and will be rebased after it's merged. Waiting on the twisted keyword for ~alpha (https://bugs.gentoo.org/773451)

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @vaartis
Areas affected: ebuilds
Packages affected: dev-python/PySocks, dev-python/asgiref, dev-python/automat, dev-python/constantly, dev-python/cryptography...

dev-python/PySocks: @gentoo/python
dev-python/asgiref: @gentoo/python
dev-python/automat: @gentoo/python
dev-python/constantly: @gentoo/python
dev-python/cryptography: @gentoo/python
dev-python/cython-test-exception-raiser: @gentoo/python
dev-python/django: @gentoo/python
dev-python/dnspython: @gentoo/python
dev-python/hpack: @gentoo/python
dev-python/hyper-h2: @gentoo/python
dev-python/hyperframe: @gentoo/python
dev-python/jinja: @gentoo/python
dev-python/m2r: @gentoo/python
dev-python/pycurl: @gentoo/python
dev-python/pyopenssl: @gentoo/python
dev-python/pytest-asyncio: @gentoo/python
dev-python/pytest-subtests: @gentoo/python
dev-python/pyyaml: @gentoo/python
dev-python/selenium: @gentoo/python
dev-python/semantic_version: @prometheanfire, @gentoo/python
dev-python/service_identity: @gentoo/python
dev-python/tblib: @gentoo/python
dev-python/twisted: @gentoo/python
dev-python/urllib3: @gentoo/python
www-servers/tornado: @gentoo/python

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels May 16, 2021
@vaartis vaartis marked this pull request as draft May 16, 2021 13:25
@mgorny
Copy link
Member

mgorny commented May 16, 2021

Did you manage to test PySocks? This bleeping thing technically requires ancient Tornado that can't work with py3.10 but you might be able to get them mostly working by collecting random patches from the Internet. I've basically started pinging people to stop using it as it's abandoned.

@mgorny
Copy link
Member

mgorny commented May 16, 2021

(I'm entirely happy if you managed to test it locally/in venv and not bother with making the tests work inside the ebuild)

@vaartis
Copy link
Contributor Author

vaartis commented May 16, 2021

Did you manage to test PySocks? This bleeping thing technically requires ancient Tornado that can't work with py3.10 but you might be able to get them mostly working by collecting random patches from the Internet. I've basically started pinging people to stop using it as it's abandoned.
(I'm entirely happy if you managed to test it locally/in venv and not bother with making the tests work inside the ebuild)

Tested it locally with requests on both 3.8 and 3.10, both seem to work fine.

@vaartis
Copy link
Contributor Author

vaartis commented May 16, 2021

Was able to get the tests to actually run locally, they work with 3.8 fine, in 3.10 tornado needs a single patch to run successfully and then the tests pass also.

@mgorny
Copy link
Member

mgorny commented May 16, 2021

Was able to get the tests to actually run locally, they work with 3.8 fine, in 3.10 tornado needs a single patch to run successfully and then the tests pass also.

How much effort did you need to put into it? I'm wondering if there's a point in trying to make them ebuild-friendly after all.

@vaartis
Copy link
Contributor Author

vaartis commented May 17, 2021

How much effort did you need to put into it? I'm wondering if there's a point in trying to make them ebuild-friendly after all.

Changed the version in requirements_dev.txt for test server to 0.0.29 and after tox fetched the dependencies changed a single line in tornado from collections.MutableMapping to collections.abc.MutableMapping. That was enought for PySocks tests to pass.

We'd need to bring back old tornado and old test server. Current test server is very different, they changed the whole thing in 0.0.30.

@mgorny
Copy link
Member

mgorny commented May 18, 2021

BTW I'm surprised that Twisted works with py3.10. Back when I was doing py3.9 (and it was after py3.9 final), I had to fix a lot of issues upstream.

@vaartis
Copy link
Contributor Author

vaartis commented May 18, 2021

BTW I'm surprised that Twisted works with py3.10. Back when I was doing py3.9 (and it was after py3.9 final), I had to fix a lot of issues upstream.

Surprisingly enough, yes, it seems to work fine, except those tests that depend on warning count. Perhaps thanks to your work on 3.9, but also may be because it's a relatively recent release (just two months ago).

@vaartis vaartis force-pushed the twisted-dependant-bumps branch 4 times, most recently from 8fa7a53 to 4ca432f Compare May 18, 2021 10:34
@mgorny
Copy link
Member

mgorny commented May 18, 2021

Good news! Jinja has just been released in Python 3.10-compatible version, and I've pushed it to ::gentoo. Please remove jinja from the PR when rebasing ;-).

@vaartis
Copy link
Contributor Author

vaartis commented May 19, 2021

Good news! Jinja has just been released in Python 3.10-compatible version, and I've pushed it to ::gentoo. Please remove jinja from the PR when rebasing ;-).

Great. Rebased and removed it, that's one problem less. Django seems content with the new jinja version.

@vaartis vaartis force-pushed the twisted-dependant-bumps branch 2 times, most recently from d28861c to ed8c2ea Compare May 19, 2021 18:28
@mgorny
Copy link
Member

mgorny commented May 20, 2021

FYI, I've reverted asgiref because the warnings from the code break revdeps. Normally I'd say it's revdeps' fault for relying on warning-free dependencies but fighting 'fatal warnings' all over the place sounds like fighting windmills, so it's probably better to fix the issue in asgiref.

@mgorny
Copy link
Member

mgorny commented May 20, 2021

That said, I've forced keywords on twisted, so you may want to try rebasing and seeing where we can go from there.

@vaartis
Copy link
Contributor Author

vaartis commented May 20, 2021

Will look into it later today, probably going to also look into asigref stuff and patch whatever problems they have in their code and suggesting the patch upstream, depending on how much work it is.

@vaartis
Copy link
Contributor Author

vaartis commented May 20, 2021

django/asgiref#262

Gathered a patch for asgiref to fix all their deprecation warnings, let's see what upstream says about them.

@mgorny, I saw in the bugzilla issue for alpha keywording that you have chose to instead mask 3.10 on alpha. Should I revbump all the ebuilds affected removing the keyword?

@mgorny
Copy link
Member

mgorny commented May 20, 2021

Don't remove keywords. The mask makes them irrelevant.

@vaartis
Copy link
Contributor Author

vaartis commented May 20, 2021

Now that upstream has approved the asgiref patches, I've re-added the ebuild here.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-05-21 08:19 UTC
Newest commit scanned: 8063ab0
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/fd2796148d/output.html

@@ -3,7 +3,7 @@

EAPI=7

PYTHON_COMPAT=( python3_{7..9} )
PYTHON_COMPAT=( python3_{7..10} )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've lost the django dep here. But then, this makes it possible to merge it as-is for the time being, and add django when it's 3.10-ready. I'll file a bug not to forget about it.

Copy link
Contributor Author

@vaartis vaartis May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you link the bug here?
I removed it because it was broken for you, or at least seemed so, so yes, let's wait on this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgorny
Copy link
Member

mgorny commented May 21, 2021

I'm sorry, I was wrong. I've just retested and the twisted errors/hang is happening on 3.7 too. I'm going to investigate.

@mgorny
Copy link
Member

mgorny commented May 21, 2021

===============================================================================                                                                   [ERROR]                                                                                                                                           
Traceback (most recent call last):                                                                                                                
  File "/tmp/portage/dev-python/twisted-21.2.0/work/twisted-twisted-21.2.0-python3_7/test/lib/twisted/internet/test/test_gireactor.py", line 100, 
in test_gtkApplicationActivate                                                                                                                    
    reactor = gtk3reactor.Gtk3Reactor()                                                                                                           
  File "/tmp/portage/dev-python/twisted-21.2.0/work/twisted-twisted-21.2.0-python3_7/test/lib/twisted/internet/gtk3reactor.py", line 35, in __init
__                                                                                                                                                
    gireactor.GIReactor.__init__(self, useGtk=True)                                                                                               
  File "/tmp/portage/dev-python/twisted-21.2.0/work/twisted-twisted-21.2.0-python3_7/test/lib/twisted/internet/gireactor.py", line 73, in __init__
    _glibbase.GlibReactorBase.__init__(self, GLib, _gtk, useGtk=useGtk)                                                                             File "/tmp/portage/dev-python/twisted-21.2.0/work/twisted-twisted-21.2.0-python3_7/test/lib/twisted/internet/_glibbase.py", line 115, in __init_
_                                                                                                                                                     self._pending = self._gtk.events_pending                                                                                                        File "/usr/lib/python3.7/site-packages/gi/overrides/__init__.py", line 32, in __getattr__                                                       
    return getattr(self._introspection_module, name)                                                                                              
  File "/usr/lib/python3.7/site-packages/gi/module.py", line 124, in __getattr__                         
    self.__name__, name))                                                                                                                         
builtins.AttributeError: 'gi.repository.Gtk' object has no attribute 'events_pending'                                

twisted.internet.test.test_gireactor.GApplicationRegistrationTests.test_gtkApplicationActivate

What GTK+ version do you have?

@mgorny
Copy link
Member

mgorny commented May 21, 2021

Ok, I'm 90% convinced that it's GTK+4 getting in the way. I've unmerged it and I'm trying again. If all tests pass this time, I'm going to merge it.

@mgorny
Copy link
Member

mgorny commented May 21, 2021

No such luck:

  RunnersTests                                                                                                                           [35/1864]
    test_basicTrialIntegration ...                                         [OK]
    test_expectedResults ... /tmp/portage/dev-python/twisted-21.2.0/temp/environment: line 2913:   127 Segmentation fault      (core dumped) "${EPYTHON}" -m twisted.trial twisted

@vaartis
Copy link
Contributor Author

vaartis commented May 21, 2021

Ok, I'm 90% convinced that it's GTK+4 getting in the way. I've unmerged it and I'm trying again. If all tests pass this time, I'm going to merge it.

That's curious. I don't have GTK installed at all. Emerging it now just to make sure.

Yep, now it hangs for me too. Seems like GTK 4 breaks the tests pretty badly. A lot of errors too. No idea how to report it upstream, their bug reporting process seems to be a bit complicated.

    testUnfeaturefulMessage ...                                         [ERROR]
  DefaultSearchTests
    test_searchAndMessageSet ...                                           [OK]
    test_searchInvalidCriteria ...                                         [OK]
    test_searchMessageSet ...                                              [OK]
    test_searchMessageSetUIDWithStar ...                                   [OK]
    test_searchMessageSetUIDWithStarAndHighStart ...                       [OK]
    test_searchMessageSetUIDWithStarFirst ...                              [OK]
    test_searchMessageSetWithList ...                                      [OK]
    test_searchMessageSetWithStar ...                                      [OK]
    test_searchMessageSetWithStarFirst ...                                 [OK]
    test_searchNot ...                                                     [OK]
    test_searchNotMessageSet ...                                           [OK]
    test_searchOr ...                                                      [OK]
    test_searchOrMessageSet ...                                            [OK]
  DisconnectionTests
    testClientDisconnectFailsDeferreds ...                                 [OK]
  FetchSearchStoreTests
    testSearch ... 

@mgorny
Copy link
Member

mgorny commented May 21, 2021

I've filed https://twistedmatrix.com/trac/ticket/10200 for GTK+4, and I'll submit a patch to force GTK+3 shortly. If I don't forget, I'm going to test git twisted against py3.10. Or you can try (uninstall gtk:4 first):

git clone https://github.com/twisted/twisted
cd twisted
python3.10 setup.py build
PYTHONPATH=build/lib python3.10 -m twisted.trial twisted

@vaartis
Copy link
Contributor Author

vaartis commented May 22, 2021

I've filed https://twistedmatrix.com/trac/ticket/10200 for GTK+4, and I'll submit a patch to force GTK+3 shortly. If I don't forget, I'm going to test git twisted against py3.10. Or you can try (uninstall gtk:4 first):

Thank you. I have tested the git twisted version and the test suite seems to pass fine, without GTK 4 that is. That said, I only just found out that there are two GTK packages in different categories. Turns out I did have GTK3 installed, so the tests do indeed work fine with it.

@vaartis
Copy link
Contributor Author

vaartis commented May 22, 2021

I've applied your patch to force gtk 3 to the newest version of twisted, and also tried to apply it to an older one, however it seems that older versions' tests no longer pass at all, because dev-python/pyopenssl is too new and we only have 20.0.1 in tree. However, the code itself actually test if the function is available and if not makes it as such, so the problem only exists for one test thing, so I'll patch that one, it's a single line change.

builtins.AttributeError: 'Context' object has no attribute 'set_npn_advertise_callback'

UPD: nevermind, there's more than just that. Not sure about what to do with this.

@mgorny
Copy link
Member

mgorny commented May 22, 2021

So you can't reproduce the segfault?

@vaartis
Copy link
Contributor Author

vaartis commented May 22, 2021

So you can't reproduce the segfault?

Don't think so. It doesn't segfault for me, the test isn't skipped, and passes fine. It's very strange since it seems like a fairly simple test without any C calls or anything.

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-05-22 10:24 UTC
Newest commit scanned: 09708eb
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/14fadd49fa/output.html

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-05-22 10:34 UTC
Newest commit scanned: 2920027
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/5b7f0ea6c8/output.html

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-05-22 11:14 UTC
Newest commit scanned: 0d389bc
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/47d3a8b637/output.html

@mgorny
Copy link
Member

mgorny commented Jun 1, 2021

So I've given it more thought and I've decided to merge it and stop blocking other packages on tornado. Worst case, other packages will hit the warnings and we'll either decide to wait or to ignore them there as well, and so on.

That is, 'it' without dnspython since that still fails for me and doesn't seem to be needed for the rest.

@gentoo-bot gentoo-bot closed this in 02872e7 Jun 1, 2021
@vaartis
Copy link
Contributor Author

vaartis commented Jun 1, 2021

So I've given it more thought and I've decided to merge it and stop blocking other packages on tornado. Worst case, other packages will hit the warnings and we'll either decide to wait or to ignore them there as well, and so on.

That is, 'it' without dnspython since that still fails for me and doesn't seem to be needed for the rest.

I see. Would be good to figure out what the problem with dnspython is..

@mgorny
Copy link
Member

mgorny commented Jun 1, 2021

I think it's related to output capture. I can get the test to pass by passing -s to pytest.

Could you try different versions of pytest, to see if this is some kind of regression?

@vaartis
Copy link
Contributor Author

vaartis commented Jun 2, 2021

I think it's related to output capture. I can get the test to pass by passing -s to pytest.

Could you try different versions of pytest, to see if this is some kind of regression?

The only version of pytest that works with 3.10 is the latest one, others are broken on 3.10. It works on 8, 9 and 10 for me. The older versions of pytest work with 8 and 9 for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits.
Projects
None yet
4 participants