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

nix: use curl with blocking resolver #41619

Closed
wants to merge 3 commits into from
Closed

nix: use curl with blocking resolver #41619

wants to merge 3 commits into from

Conversation

orivej
Copy link
Contributor

@orivej orivej commented Jun 7, 2018

Motivation for this change

This is an alternative to #41452. Apparently curl threaded resolver enabled in 7.60 imposes additional restrictions on usage of its API that are not present with its blocking or asynchronous resolvers.

c-ares based asynchronous resolver does not support nsswitch plugins. This change makes Nix continue using curl with blocking resolver.

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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Tested that nix-channel --update does not wait for 10 seconds for DNS resolution.

@orivej
Revert "nix: point at curl 7.59.0 (NixOS#41452)"
036c131 
This reverts commit 5574df3.

Instead we will use curl 7.60 with c-ares resolver.
@orivej
curl: add threadedResolverSupport
5dd58ce 
Defaults to true since curl 7.60.
@orivej
nix: use curl with blocking resolver
d5b7e31
This was referenced Jun 7, 2018
Closed
Merged
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: curl, nix

Partial log (click to expand)

running test tests/fetchMercurial.sh... [SKIP]
running test tests/signing.sh... [PASS]
running test tests/run.sh... [PASS]
running test tests/brotli.sh... [PASS]
running test tests/pure-eval.sh... [PASS]
running test tests/check.sh... [PASS]
running test tests/plugins.sh... [PASS]
All tests succeeded
/nix/store/b3rih86lp6p3n11608l1w6wkm589pv6n-curl-7.60.0-bin
/nix/store/rc18nba0ryk3jyj0mx6rx2qa354xp137-nix-2.0.4

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: curl, nix

Partial log (click to expand)

running test tests/fetchMercurial.sh... [SKIP]
running test tests/signing.sh... [PASS]
running test tests/run.sh... [PASS]
running test tests/brotli.sh... [PASS]
running test tests/pure-eval.sh... [PASS]
running test tests/check.sh... [PASS]
running test tests/plugins.sh... [PASS]
All tests succeeded
/nix/store/bzspsgimak6r72wg615vky75jsfpfrvk-curl-7.60.0-bin
/nix/store/zqpncg11frcv05v0nzhaxlvxdsckx0pj-nix-2.0.4

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: curl, nix

Partial log (click to expand)

running test tests/fetchMercurial.sh... [SKIP]
running test tests/signing.sh... [PASS]
running test tests/run.sh... [PASS]
running test tests/brotli.sh... [PASS]
running test tests/pure-eval.sh... [PASS]
running test tests/check.sh... [PASS]
running test tests/plugins.sh... [PASS]
All tests succeeded
/nix/store/6s7vvjdsrw24mf16rm1mzcz4dycqpj9g-curl-7.60.0-bin
/nix/store/v5sxkc5pcnhjv5fbl161c0d2zc0p2k1b-nix-2.0.4

@dtzWill
Copy link
Member

dtzWill commented Jun 7, 2018

LGTM, and I agree we should leave curl using default resolver (threaded) for most users.

I haven't played with Nix when using blocking resolver-- does Nix work well with it, esp. since we use a single thread to drive downloads it'd be good to check things don't end up excessively serial or something.

FWIW 7.59 had threaded resolver enabled by default as well (we used it), so I'm not sure if anyone's tried the blocking resolver? Looks like threaded was enabled by default ~7.55 bouncing through commit history.

(Thanks for putting together these fixes, we certainly need to do something about this)

@orivej
Copy link
Contributor Author

orivej commented Jun 7, 2018

Looks like threaded was enabled by default ~7.55 bouncing through commit history.

Then curl/curl#2636 (comment) is not sufficient to explain our case, and I am not sure that this is not a curl bug after all.

@dtzWill
Copy link
Member

dtzWill commented Jun 9, 2018

FWIW here's a branch that patches Nix to reduce sleep time when items are queued to the recommended duration: https://github.com/dtzWill/nixpkgs/tree/fix/nix-curl-760

(which fixes the badness I was experiencing re:7.60 vs 7.59)

I'll make a PR on the nix repo to discuss the change/idea.

@dtzWill dtzWill mentioned this pull request Jun 9, 2018
Merged
@mmahut
Copy link
Member

mmahut commented Aug 7, 2019

Any updates on this pull request, please?

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@orivej-nixos
Copy link
Contributor

This is no longer relevant since 51ac3db.

@orivej orivej deleted the nix-curl-blocking branch March 29, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 8.has: clean-up 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@orivej @GrahamcOfBorg @dtzWill @mmahut @orivej-nixos @xeji