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

Jump to bottom

h11: add pytest5 compatability #67896

Merged
merged 1 commit into from Sep 13, 2019
Merged

h11: add pytest5 compatability #67896

merged 1 commit into from Sep 13, 2019

Conversation

B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented Sep 1, 2019

Motivation for this change

The python37Packages.h11 build is currently failing.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

@B4dM4n B4dM4n requested a review from FRidh as a code owner September 1, 2019 19:09
@arcnmx
Copy link
Member

arcnmx commented Sep 2, 2019

@GrahamcOfBorg build python3Packages.h11 pythonPackages.h11

Looks good, I'm using this locally to partially unblock broken builds from the recent python fallout.

@wd15
Copy link
Contributor

wd15 commented Sep 3, 2019

Nix-review doesn't build for me, it just hangs with the message below.

h11 is a uvicorn dependency, but I'm not sure what's going on here.

$ nix-review pr 67896
remote: Enumerating objects: 323, done.
remote: Counting objects: 100% (323/323), done.
...
builder for '/nix/store/8z9b1665n08hx9ha7fdswl8z02qasq3f-python3.7-uvicorn-0.8.4.drv' failed with exit code 1; last 10 log lines:
  adding 'uvicorn-0.8.4.dist-info/top_level.txt'
  adding 'uvicorn-0.8.4.dist-info/RECORD'
  removing build/bdist.linux-x86_64/wheel
  installing
  /build/uvicorn-0.8.4/dist /build/uvicorn-0.8.4
  Processing ./uvicorn-0.8.4-py3-none-any.whl
  Requirement already satisfied: httptools==0.0.13; sys_platform != "win32" and sys_platform != "cygwin" and platform_python_implementation != "pypy" in /nix/store/pdizzgdn34frmiwh8kxzxxyih9kmxqjb-python3.7-httptools-0.0.13/lib/python3.7/site-packages (from uvicorn==0.8.4) (0.0.13)
  Collecting uvloop==0.12.*; sys_platform != "win32" and sys_platform != "cygwin" and platform_python_implementation != "pypy" (from uvicorn==0.8.4)
  ERROR: Could not find a version that satisfies the requirement uvloop==0.12.*; sys_platform != "win32" and sys_platform != "cygwin" and platform_python_implementation != "pypy" (from uvicorn==0.8.4) (from versions: none)
ERROR: No matching distribution found for uvloop==0.12.*; sys_platform != "win32" and sys_platform != "cygwin" and platform_python_implementation != "pypy" (from uvicorn==0.8.4)
cannot build derivation '/nix/store/spic0gp923dh0xkrn5p12zgn4yycz73q-python3.7-starlette-0.12.4.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/cnjixkvlqgvf563qlf4k1aw6v6dgwclf-python3.7-fastapi-0.33.0.drv': 2 dependencies couldn't be built
[1/0/2 built (1 failed), 0.0 MiB DL] building python3.7-mitmproxy-4.0.4 (installCheckPhase): test/mitmproxy/net/websockets/t[0 built (1 failed), 0.0 MiB DL]

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Sep 3, 2019

mitmproxy is also affected by the pytest5 issue. Many of the TCP related tests hang or fail.

I also created a patch for it:

master...B4dM4n:mitmproxy-pytest5

@arcnmx
Copy link
Member

arcnmx commented Sep 3, 2019 •
edited

ERROR: Could not find a version that satisfies the requirement uvloop==0.12.*; sys_platform != "win32" and sys_platform != "cygwin" and platform_python_implementation != "pypy" (from uvicorn==0.8.4) (from versions: none)

That seems unrelated to this PR? uvloop was updated to 0.13 yesterday. The only other package that appears to depend on h11 in nixpkgs is:

@GrahamcOfBorg build python3Packages.wsproto pythonPackages.wsproto

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Sep 10, 2019

The uvicorn build failure will be fixed here: master...B4dM4n:websockets-8

The branch also contains the changes from this PR and nix-review can build everything except mitmproxy.

@lheckemann lheckemann added this to the 19.09 milestone Sep 10, 2019
@B4dM4n B4dM4n mentioned this pull request Sep 13, 2019
Closed
10 tasks
@d-goldin
Copy link
Contributor

d-goldin commented Sep 13, 2019 •
edited

Let's reference #68361 to make tracking this easier and to add some visibility

@d-goldin
Copy link
Contributor

I just tested this as well for python 3.7 and 2.7 and the tests pass fine.

@arcnmx
Copy link
Member

arcnmx commented Sep 13, 2019

fwiw I've been using this patch locally for the past week, given that it only touches one line of a test I think it should be pretty good to go!

@Mic92 Mic92 merged commit f595f28 into NixOS:master Sep 13, 2019
@B4dM4n B4dM4n deleted the h11-pytest branch September 13, 2019 14:50
@jonringer
Copy link
Contributor

fixes python3Packages.h11 and python3Packages.wsproto for #68361

@Mic92
Copy link
Member

Mic92 commented Sep 13, 2019

backport:

[detached HEAD 09e9586] h11: add pytest5 compatability
Author: Fabian Möller fabianm88@gmail.com
Date: Sun Sep 1 18:11:16 2019 +0200
1 file changed, 9 insertions(+), 1 deletion(-)

@B4dM4n B4dM4n mentioned this pull request Sep 13, 2019
Merged
10 tasks
@wd15 wd15 mentioned this pull request Sep 16, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
19.09
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@B4dM4n @arcnmx @wd15 @d-goldin @jonringer @Mic92 @lheckemann