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.

Sign up for GitHub

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

python3Packages.werkzeug,python3Packages.aiohttp,python3Packages.async-upnp-client: fix build; pythonPackages.urllib3: disable failing test #115564

Closed
wants to merge 9 commits into from
Closed

python3Packages.werkzeug,python3Packages.aiohttp,python3Packages.async-upnp-client: fix build; pythonPackages.urllib3: disable failing test #115564

wants to merge 9 commits into from

Conversation

SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Mar 9, 2021 •
edited by FRidh
Loading

Motivation for this change

#115521

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@ofborg ofborg bot requested a review from mweinelt March 9, 2021 16:24
mweinelt
mweinelt reviewed Mar 9, 2021
View reviewed changes
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

Changes look reasonable.

@mweinelt
Copy link
Member

@SuperSandro2000 PTAL.

@mweinelt mweinelt mentioned this pull request Mar 12, 2021
Merged
10 tasks
@SuperSandro2000 SuperSandro2000 mentioned this pull request Mar 12, 2021
Open
@SuperSandro2000 SuperSandro2000 force-pushed the fix-werkzeug branch 2 times, most recently from a427266 to 92cf08f Compare March 12, 2021 22:29
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 12, 2021
@ofborg ofborg bot removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 12, 2021
@ofborg ofborg bot removed the 8.has: documentation This PR adds or changes documentation label Mar 12, 2021
@ofborg ofborg bot requested a review from mweinelt March 12, 2021 23:23
@ofborg ofborg bot requested review from costrouc and makefu March 12, 2021 23:44
@SuperSandro2000
Copy link
Member Author

SuperSandro2000 commented Mar 13, 2021 •
edited
Loading

I can't get urllib3 to pass the tests without disabling many tests.

@FRidh Can we skip this staging-next cycle and merge staging again? There where some commits for urllib3 on there already.

I got it building. aarch probably still fails and I and @mweinelt suspect that the tests are flaky.

@SuperSandro2000 SuperSandro2000 changed the title Fix werkzeug and home-assistant building python3Packages.werkzeug: fix building Mar 13, 2021
@SuperSandro2000

This comment has been minimized.

Sign in to view
@SuperSandro2000 SuperSandro2000 changed the title python3Packages.werkzeug: fix building python3Packages.werkzeug,pythonPackages.aiohttp,pythonPackages.async-upnp-client: fix build Mar 13, 2021
@SuperSandro2000 SuperSandro2000 changed the title python3Packages.werkzeug,pythonPackages.aiohttp,pythonPackages.async-upnp-client: fix build python3Packages.werkzeug,python3Packages.aiohttp,python3Packages.async-upnp-client: fix build Mar 13, 2021
@SuperSandro2000
pythonPackages.urllib3: disable failing test
f942c6c 
According to upstream the tests can be flaky.
If others are failing also add them or disable tests.

urllib3/urllib3#1554 (comment)
@SuperSandro2000 SuperSandro2000 changed the title python3Packages.werkzeug,python3Packages.aiohttp,python3Packages.async-upnp-client: fix build python3Packages.werkzeug,python3Packages.aiohttp,python3Packages.async-upnp-client: fix build; pythonPackages.urllib3: disable failing test Mar 13, 2021
@ofborg ofborg bot requested review from fabaff and Mic92 March 13, 2021 01:04
@SuperSandro2000
Copy link
Member Author

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 115564 run on x86_64-linux 1

4 packages built:
  • home-assistant
  • python3Packages.async-upnp-client
  • python3Packages.trio
  • python3Packages.werkzeug

FRidh
FRidh reviewed Mar 13, 2021
View reviewed changes
pkgs/development/python-modules/urllib3/default.nix
@@ -46,6 +46,10 @@ buildPythonPackage rec {
trustme
];

disabledTests = [
"test_connection_closed_on_read_timeout_preload_false"
Copy link
Member

Choose a reason for hiding this comment

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

fails only on aarch64. Disabling altogether will result in a mass rebuild.

Copy link
Member

Choose a reason for hiding this comment

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

disabledTests = if stdenv.hostPlatform.isAarch64 then [  "test_connection_closed_on_read_timeout_preload_false" ] else null;

Copy link
Member

Choose a reason for hiding this comment

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

pushed 2e0b261 to staging-next

Copy link
Member Author

Choose a reason for hiding this comment

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

I build urrlib3 on amd64 with --check and this was required to build the tests. I don't think we get around the mass rebuild.

@FRidh
Copy link
Member

FRidh commented Mar 13, 2021

Rebased and pushed to staging-next, thank you.

@FRidh FRidh closed this Mar 13, 2021
@SuperSandro2000 SuperSandro2000 deleted the fix-werkzeug branch March 13, 2021 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda left review comments

@FRidh FRidh FRidh left review comments

@jonringer jonringer Awaiting requested review from jonringer

@mweinelt mweinelt Awaiting requested review from mweinelt

@costrouc costrouc Awaiting requested review from costrouc

@makefu makefu Awaiting requested review from makefu

@fabaff fabaff Awaiting requested review from fabaff

@Mic92 Mic92 Awaiting requested review from Mic92

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SuperSandro2000 @mweinelt @FRidh @dotlambda