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

prettytoml: Catch StopIteration in AbstractTable._enumerate_items #2427

Merged
merged 2 commits into from
Jun 26, 2018

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented Jun 26, 2018

This makes PEP 479 enabled Pythons (such as 3.7) work again.

Otherwise you get:

RuntimeError: generator raised StopIteration

Fixes #2426


Untested. The vendoring script fails with unrelated problem.

@hroncok hroncok force-pushed the py37 branch 2 times, most recently from f4c6fa8 to 5e9d070 Compare June 26, 2018 20:11
@techalchemy
Copy link
Member

Ah yes. This is a bug I have attempted to patch for about 2 dozen times and I have no clue why it keeps unpatching itself...

Virtualenv location: /tmp/pipenv-b9_Jcc-project/.venv
Creating a Pipfile for this project...
Pipfile.lock not found, creating...
Locking [dev-packages] dependencies...
Locking [packages] dependencies...
  session=self.session,
  File "/var/lib/buildkite-agent/builds/build-1/kennethreitz/pipenv/pipenv/patched/notpip/_internal/index.py", line 180, in __init__
    impl=implementation,
  File "/var/lib/buildkite-agent/builds/build-1/kennethreitz/pipenv/pipenv/patched/notpip/_internal/pep425tags.py", line 282, in get_supported
    elif platform is None and is_manylinux1_compatible():
  File "/var/lib/buildkite-agent/builds/build-1/kennethreitz/pipenv/pipenv/patched/notpip/_internal/pep425tags.py", line 160, in is_manylinux1_compatible
    return pip._internal.utils.glibc.have_compatible_glibc(2, 5)
NameError: global name 'pip' is not defined
/var/lib/buildkite-agent/builds/build-1/kennethreitz/pipenv/pipenv/_compat.py:108: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/pipenv-ZMfLuA-requirements'>
  warnings.warn(warn_message, ResourceWarning)
 
/var/lib/buildkite-agent/builds/build-1/kennethreitz/pipenv/pipenv/_compat.py:108: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/pipenv-_pCZvZ-requirements'>
warnings.warn(warn_message, ResourceWarning)

Note the glibc failure. I'm pretty sure that should be caught by https://github.com/pypa/pipenv/blob/master/tasks/vendoring/patches/patched/_post-pip-update-pep425tags.patch ...

@hroncok
Copy link
Contributor Author

hroncok commented Jun 26, 2018

What does

- return pipenv.patched.notpip._internal.utils.glibc.have_compatible_glibc(2, 5)
+ return pipenv.patched.notpip._internal.utils.glibc.have_compatible_glibc(2, 5)
change?

@techalchemy
Copy link
Member

uh... that's an interesting question...

@techalchemy
Copy link
Member

-    return pip._internal.utils.glibc.have_compatible_glibc(2, 5)
+    return pipenv.patched.notpip._internal.utils.glibc.have_compatible_glibc(2, 5)

is what I'm guessing it is supposed to say...

@hroncok
Copy link
Contributor Author

hroncok commented Jun 26, 2018

testing with that.

This makes PEP 479 enabled Pythons (such as 3.7) work again.

Otherwise you get:

    RuntimeError: generator raised StopIteration

Fixes pypa#2426
@techalchemy
Copy link
Member

I'm going to merge this tonight (since I assume the tests will pass on this) -- thanks for the fix. I'll add a 3.7 build as well so we can iterate and get that working before next week. Thanks for keeping an eye on us, seriously appreciated

@techalchemy techalchemy added PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. Type: Vendored Dependencies This issue affects vendored dependencies within pipenv. labels Jun 26, 2018
@techalchemy techalchemy added this to the 2018.6.26 milestone Jun 26, 2018
@hroncok
Copy link
Contributor Author

hroncok commented Jun 26, 2018

How can I see the buildkite/pipenv CI result?

@techalchemy
Copy link
Member

Unfortunately Kenneth has that setup on a private account and they don't have the ability to make this publicly viewable yet, but it passed

@techalchemy
Copy link
Member

Someday appveyor will also run

@techalchemy techalchemy merged commit fa7e27e into pypa:master Jun 26, 2018
@techalchemy
Copy link
Member

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. Type: Vendored Dependencies This issue affects vendored dependencies within pipenv.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants