[#8505] Run PyPy with Travis-CI #226

adiroiban · 2016-06-26T20:28:13Z

See https://twistedmatrix.com/trac/ticket/8505

ryanhiebert · 2016-06-27T04:11:05Z

.travis.yml

+        "$PYENV_ROOT/bin/pyenv" install --skip-existing "pypy-$PYPY_VERSION"
+        virtualenv --python="$PYENV_ROOT/versions/pypy-$PYPY_VERSION/bin/python" "$HOME/virtualenvs/pypy-$PYPY_VERSION"
+        source "$HOME/virtualenvs/pypy-$PYPY_VERSION/bin/activate"
+      fi
  - pip install tox tox-travis


You don't technically need to install tox explicitly, since tox-travis depends on tox... :-)

markrwilliams · 2016-06-28T13:09:54Z

@adiroiban it looks like travis did not to run on the last two commits -- was that intentional??

adiroiban · 2016-06-28T13:15:01Z

@adiroiban it looks like travis did not to run on the last two commits -- was that intentional??

Nope. Something is broken with Travis-CI as it was ignoring my push commits.

markrwilliams · 2016-06-28T13:17:22Z

@adiroiban fair enough. i'm happy to help if i can!

adiroiban · 2016-06-28T13:29:11Z

I have other PR and travis-ci is not triggered ... so I need to investigate ...I have sent an email to Travis-CI support team

anyway we need to fix the coverage issue and this will also need to be deployed on our buildbot env

BanzaiMan · 2016-06-28T14:19:46Z

.travis.yml

+        virtualenv --python="$PYENV_ROOT/versions/pypy-$PYPY_VERSION/bin/python" "$HOME/virtualenvs/pypy-$PYPY_VERSION"
+        source "$HOME/virtualenvs/pypy-$PYPY_VERSION/bin/activate"
+        echo "import sys; sys.setrecursionlimit(10000)" > $HOME/virtualenvs/pypy-$PYPY_VERSION/
+/lib-python/2.7/sitecustomize.py


What is this line supposed to do? This line introduces YAML syntax error, so Travis CI build will not run.

markrwilliams · 2016-06-29T04:20:34Z

@adiroiban so it didn't fix the issue but it did seem to let the code advance a little farther. if you're willing to give it another shot, you can bump the recursion limit to 10000. otherwise i'll report back after i've gotten a chance to run coverage & trial under a debug build of pypy.

adiroiban · 2016-06-29T11:47:22Z

if you're willing to give it another shot, you can bump the recursion limit to 10000. otherwise i'll report back after i've gotten a chance to run coverage & trial under a debug build of pypy.

The limit is already at 10.000 ... as I have just copy pasted from your comment :)

Maybe is worth getting in touch with coverage people to see if something special is needed for PyPy

markrwilliams · 2016-06-29T13:06:06Z

@adiroiban sorry, i meant 100,000 - github ate my comment and i hastily retyped it.

i can reach out to the coverage people, but i'm pretty sure this is a pypy specific issue :( i'll update this PR when i know more.

rodrigc · 2016-07-04T21:50:12Z

@markrwilliams: take a look at https://travis-ci.org/twisted/twisted/jobs/142311357

markrwilliams · 2016-07-05T00:50:09Z

@rodrigc Sorry, I should have mentioned that I've filed an issue with PyPy about this: https://bitbucket.org/pypy/pypy/issues/2335/maximum-recursion-depth-exceeded-with
I'll update this PR when a solution is found.

rodrigc · 2016-07-05T07:02:58Z

@markrwilliams How many Pypy patches for Twisted do you have?
Can you extract a subset of those changes which do not require if PYPY, and
can run completely on Python 2.7, and pass the coverage tests on Python 2.7?
If we can get a subset of those patches committed into Twisted, that will allow some forward progress.

Getting blocked because the coverage tool itself doesn't work under Pypy is :(

adiroiban · 2016-07-05T12:17:15Z

The codecov patch protection was lifted see twisted-infra/braid#213

so you can no merge pypy patches even if their coverage is not reported

markrwilliams · 2016-07-09T20:34:54Z

Armin Rigo just pushed a fix for the coverage issue. Looks like we can revist it this after the next PyPy release!

codecov-io · 2016-07-24T22:20:22Z

Codecov Report

Merging #226 into trunk will decrease coverage by -45.47%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##            trunk     #226       +/-   ##
===========================================
- Coverage   91.28%   45.82%   -45.47%     
===========================================
  Files         844      156      -688     
  Lines      147616    19990   -127626     
  Branches    13059     2898    -10161     
===========================================
- Hits       134756     9160   -125596     
+ Misses      10623    10222      -401     
+ Partials     2237      608     -1629

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8e8e8a...93f7ec5. Read the comment docs.

adiroiban · 2016-07-25T10:51:54Z

I have pushed a version with 100.000 recursion limit... but not luck.... so I am reverting that change.

Rather than having this code unmerged, I have pushed it for review as at least it can run pypy on travis ci... once a new pypy release is ready, in theory we should only update the version in the travis install.sh

When is the new pypy version planned to be released?

I had a quick look at the pypy website, but the release notes are done without any date ... very strange.

Similar with the bad habit that we have in Twisted, when a ticket is closed as fixed there is no note about when it will be released ... or in which version it was released

glyph · 2016-08-09T00:17:54Z

@adiroiban - can you update your branch please? hopefully the push of the merge commit will also fix whatever broke the documentation buildbot…

adiroiban · 2016-08-09T09:46:13Z

@glyph updated. thanks!

glyph · 2016-08-12T21:19:24Z

Updating again in the hopes that the CI gods will be with us this time…

glyph · 2016-08-29T21:14:44Z

Ugh. They were, but now conflicts need to be resolved.

glyph · 2016-09-10T19:23:34Z

@adiroiban - the build is failing because the topfile is in the wrong place. Move it into src/ and I think we can merge this…

glyph · 2016-09-10T19:24:32Z

.travis/install.sh

    source ~/.venv/bin/activate
 fi

 # Temporary workaround for https://github.com/pypa/setuptools/issues/776;
-# install (and thereby cache a built wheel of) cryptography.  (NB: We're


Why delete this comment? Seems irrelevant to the change in question.

I have removed it as for pypy this is no longer valid.

In travis we have pypy 2.5 and in venv we have pypy 5.4

adiroiban · 2016-09-11T16:53:31Z

Thanks @glyph for the review. I have moved to src and also updated the travis cache functionality

It would be nice to have a command in pip which can remove packages which are not used for a long time.

for pyenv, I went for this simple pyenv uninstall command, but we might need a .travis/pyenv.cleanup.sh script which will remove any version, other than PYPY_VERSION ... and in the future we might want to keep other versions if pyenv

adiroiban · 2016-09-11T17:04:32Z

we are caching on travis the full ~/.cache/pip , but maybe we should only cache ~/.cache/pip/wheels

njsmith · 2017-02-21T11:06:57Z

Instead of doing all this work with pyenv and caching, wouldn't it be easier to install squeaky's portable pypy from scratch on each build? (The travis cache has to be downloaded to the build worker every time anyway, so it's only really useful for artifacts that you're building on travis.)

glyph · 2017-02-21T20:36:00Z

@njsmith - An alternate PR would be appreciated, but given that "all this work" is already done, is there much benefit to getting rid of it?

njsmith · 2017-02-21T20:42:14Z

I don't know. I just happened to glance at this out of curiosity and on a quick skim it looked like there was some struggle with getting the cache state management to work correctly -- the advantage of my suggestion is that it'd be stateless.

markrwilliams · 2017-11-29T23:22:31Z

@adiroiban I agree with @njsmith and would prefer to use squeaky's portable PyPy. I can take up this PR if that approach seems OK to you.

adiroiban · 2017-11-30T03:29:17Z

@markrwilliams go for it. Thanks!

I have never used pypy so my work from here was just trying to pay it forward for all the good things I use in Twisted basically great python 2.7 as I don't care for the rest :)

I think that I have just checked what Travis-CI was doing and I went on those lines.

ulidtko · 2019-08-09T08:29:31Z

/me zaps a blessed wand of dead turning on the "Twisted on PyPy" thread. The PR rises from the dead.

glyph · 2019-08-09T09:10:51Z

@ulidtko You have successfully nerd-sniped me.

Julian · 2019-08-10T00:01:45Z

What's wrong with the travis-provided PyPy that means pyenv needs to be involved?

glyph · 2019-08-10T00:10:01Z

Historically it has been unspeakably ancient, and only the most recent pypys are really worth testing. If travis is on the latest we could revert that part.

glyph · 2019-08-10T00:10:27Z

(I’d rather just land this as-is and do that as a follow-up though)

Julian · 2019-08-10T00:24:38Z

Got it -- yup sounds good (me personally I've [not had any issues recently with it being terribly out of date]( https://travis-ci.org/Julian/jsonschema/jobs/569829619#L273), but yeah think doing it as a follow up is fine)

…

On Fri, Aug 9, 2019 at 8:10 PM Glyph ***@***.***> wrote: (I’d rather just land this as-is and do that as a follow-up though) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#226?email_source=notifications&email_token=AACQQXU343MJ3HVQOGQFVA3QDYBPZA5CNFSM4CH2RFWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4ABSUA#issuecomment-520100176>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACQQXWRAASMCHULIW5LX6LQDYBPZANCNFSM4CH2RFWA> .

Julian · 2019-08-10T00:30:19Z

src/twisted/python/compat.py

+    realFromFD = socket.fromfd
+    if realFromFD.__name__ == fromFDWithoutModifyingFlags.__name__:
+        return
+    socket.fromfd = fromFDWithoutModifyingFlags


So... personally I have experienced cases where twisted has monkey patched globally a thing and that monkey patch introduced bugs in non-twisted code...

I realize how tempting this is over instead just changing code to use a level of indirection, since I presume fromfd is reasonably widely used but... severe :(

Tell me to go away though I guess, and that this is the best way.

This is the best way, at least in this specific case.

It's only monkeypatched on pypy3

The behavior on pypy3 is buggy as heck and this will fix it for other programs as well.

The monkeypatch is harmless (even if the monkeypatch were to get invoked on a different cpython, it would at worst be slightly inefficient unless someone is doing something truly batty with threads).

I expect this to be relatively short-lived; it links to the bug, the bug is super straightforward and has a reproducer that's only a half a dozen lines. The other bug I filed today, Armin has already fixed.

Julian

I really dislike the monkey patching, but other than that, lgtm.

This was referenced Jun 26, 2016

PyPy is ignored tox-dev/tox-travis#16

Closed

Adds _PYPY to twisted.python.compat #223

Closed

ryanhiebert reviewed Jun 27, 2016
View reviewed changes

BanzaiMan reviewed Jun 28, 2016
View reviewed changes

glyph reviewed Sep 10, 2016
View reviewed changes

glyph added 2 commits August 9, 2019 14:58

make pypy/pypy3 work in travis configuration

d508f6a

various lint failures

0d06116

glyph force-pushed the 8505-pypy-travis branch from c2b8d9c to 0d06116 Compare August 9, 2019 22:08

glyph added 2 commits August 9, 2019 15:20

remove pypy3 from allowed failures now that it doesn't fail

2d74921

don't break on windows

e6f0ecb

Julian reviewed Aug 10, 2019

View reviewed changes

Julian approved these changes Aug 10, 2019

View reviewed changes

glyph merged commit c55c778 into trunk Aug 10, 2019

glyph deleted the 8505-pypy-travis branch August 10, 2019 01:02

[#8505] Run PyPy with Travis-CI #226

[#8505] Run PyPy with Travis-CI #226

Conversation

adiroiban commented Jun 26, 2016

ryanhiebert Jun 27, 2016

Choose a reason for hiding this comment

markrwilliams commented Jun 28, 2016

adiroiban commented Jun 28, 2016

markrwilliams commented Jun 28, 2016

adiroiban commented Jun 28, 2016

BanzaiMan Jun 28, 2016

Choose a reason for hiding this comment

markrwilliams commented Jun 29, 2016

adiroiban commented Jun 29, 2016

markrwilliams commented Jun 29, 2016

rodrigc commented Jul 4, 2016

markrwilliams commented Jul 5, 2016

rodrigc commented Jul 5, 2016 • edited Loading

adiroiban commented Jul 5, 2016

markrwilliams commented Jul 9, 2016

codecov-io commented Jul 24, 2016 • edited Loading

Codecov Report

adiroiban commented Jul 25, 2016

glyph commented Aug 9, 2016

adiroiban commented Aug 9, 2016

glyph commented Aug 12, 2016

glyph commented Aug 29, 2016

glyph commented Sep 10, 2016

glyph Sep 10, 2016

Choose a reason for hiding this comment

adiroiban Sep 11, 2016

Choose a reason for hiding this comment

adiroiban commented Sep 11, 2016 • edited Loading

adiroiban commented Sep 11, 2016

njsmith commented Feb 21, 2017

glyph commented Feb 21, 2017

njsmith commented Feb 21, 2017 via email

markrwilliams commented Nov 29, 2017

adiroiban commented Nov 30, 2017

ulidtko commented Aug 9, 2019 • edited Loading

glyph commented Aug 9, 2019

Julian commented Aug 10, 2019

glyph commented Aug 10, 2019

glyph commented Aug 10, 2019

Julian commented Aug 10, 2019 via email

Julian Aug 10, 2019

Choose a reason for hiding this comment

glyph Aug 10, 2019

Choose a reason for hiding this comment

Julian left a comment

Choose a reason for hiding this comment

rodrigc commented Jul 5, 2016 •

edited

Loading

codecov-io commented Jul 24, 2016 •

edited

Loading

adiroiban commented Sep 11, 2016 •

edited

Loading

ulidtko commented Aug 9, 2019 •

edited

Loading