curl: run tests in passthru by Artturin · Pull Request #178869 · NixOS/nixpkgs

Artturin · 2022-06-24T14:03:48Z

curl is built many times during a stdenv rebuild

they are a bottleneck

Description of changes

Things done

allows using the correct package in passthru remove unneeded replaceStrings in urls

roberth

Awesome!

roberth · 2022-06-25T16:32:15Z

pkgs/tools/networking/curl/default.nix

In the future (#176557), nixosTests.nginx-http3.extend { defaults = { /* NixOS module */ }; }

(fyi)

roberth · 2022-06-25T16:36:02Z

pkgs/tools/networking/curl/default.nix

fetchpatch = tests.fetchpatch.simple.override { fetchpatch = fetchpatch.override { fetchurl = useThisCurl fetchurl; }; };

where tests is pkgs.tests.

Haven't tried this, but would be pretty great to have something like this.
Also would be nice to have a direct fetchurl test. 🤷

I didn't actually want to repeat .simple here, but I don't think we have an overridePackages to complement callPackages yet. That could use recurseForDerivations to do the right thing.

As rigorous as it is to ensure all these test-packages are built with this curl, I question slightly how relevant it is - most of these packages are only ever built with one curl, and may not actually build with esoteric/minimal curl builds. How is the tester to know what's supposed to build and what isn't?

It also slightly raises the question whether we're going to do this with all the reverse-dependency passthru.tests we've added. Strictly we should, but in reality it would end up with testers building all kinds of package variants that aren't usually expected.

pkgs/tools/networking/curl/default.nix

SuperSandro2000 · 2022-06-26T17:45:15Z

ofborg is failing for some reason but I cannot look it up because the log is not loading.

curl is built many times during a stdenv rebuild they are a bottleneck

Mindavi · 2022-06-29T06:24:21Z

I knew someone would come in and change running the tests again.

See #170828 for some previous discussion and why they were enabled.

cc @risicle

roberth · 2022-06-29T07:57:43Z

pkgs/tools/networking/curl/default.nix

-      ocaml-curly = ocamlPackages.curly;
-      php-curl = phpExtensions.curl;
-      pycurl = python3.pkgs.pycurl;
+      withCheck = finalAttrs.finalPackage.overrideAttrs (_: { doCheck = true; });


This one should be a channel blocker if it fails. It should be added to release.nix for the nixpkgs and nixos jobs.

not sure i did it correctly

error: attribute 'tests' missing at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-4/pkgs/top-level/release.nix:100:15: 99| jobs.stdenv.x86_64-linux 100| jobs.curl.tests.withCheck.x86_64-linux | ^ 101| jobs.cargo.x86_64-linux

are passthru attributes stripped?

yeah release.nix is kind of a mess (or maybe I'm unfamiliar) and it strips all packages down using hydraJobs. It's supposed to help that evaluator release memory.
If you add the passthru tests as "normal" packages somewhere in pkgs, you won't have to deal with the release.nix code as much.

Maybe do something like this in pkgs/test/default.nix?

corePackageTests = recurseIntoAttrs { curl = recurseIntoAttrs pkgs.curl.tests; };

That should give you pkgs.tests.corePackageTests, which you can use in the release files.

pkgs/tools/networking/curl/default.nix

risicle · 2022-06-29T19:10:02Z

Notionally approve, haven't tested.

pkgs/tools/networking/curl/default.nix

roberth · 2022-07-01T08:16:02Z

pkgs/test/default.nix

Suggested change

# we can't add 'nixpkgs.curl.tests' to hydra jobs due to 'passthru' being stripped

passthruTestsForChannelBlockers = recurseIntoAttrs {

# we can't add 'nixpkgs.curl.tests' to hydra jobs due to 'tests' (and 'passthru') being stripped

packageTestsForChannelBlockers = recurseIntoAttrs {

The fact that they're on a package is more significant than passthru, which is just an implementation detail of mkDerivation.

passthru was introduced in 1014ca2 (2006) and only exposed the attrs inside of it. The behavior passthru.passthru = passthru was only added later for inspection 84fba68:

to distinguish after the fact which attributes were part of the derivation and which came from passthru.

However, the root cause of any such ambiguity comes from the mistake that mkDerivation exposes all its argument attributes in the returned package attrset.

Since RFC 92, not all packages will be defined through mkDerivation anymore, so we should try not to perpetuate its jargon and keep things simple.

SuperSandro2000 · 2022-07-01T19:22:37Z

@ofborg eval

Artturin · 2022-07-01T19:28:22Z

the hydra jobs for tests.packageTestsForChannelBlockers look like this

$ ./maintainers/scripts/rebuild-amount.sh  --print HEAD~1
Estimating rebuild amount by counting changed Hydra jobs (parallel=unset).
      5 x86_64-darwin
     11 x86_64-linux

nixos-install-tools.x86_64-linux                                                      /nix/store/mddaircl7p6ka6v30sl3hnbkp30xp22n-nixos-install-tools-22.11pre-git
tests.nixos-functions.nixos-test.x86_64-linux                                         /nix/store/g5wq3p43akl3c0zszp676q4nq5nyh5vv-nixos-system-nixos-test
tests.packageTestsForChannelBlockers.curl.coeurl.x86_64-darwin                        /nix/store/x3x1mq7bfd6rb8hi2r84g5n1mfafpf4h-coeurl-0.2.0
tests.packageTestsForChannelBlockers.curl.coeurl.x86_64-linux                         /nix/store/ja1wpybr47p53il1hrqnajmiisrg56si-coeurl-0.2.0
tests.packageTestsForChannelBlockers.curl.curlpp.x86_64-linux                         /nix/store/mlqrdavazq0pkfvaxz9ksr540pfy7bdb-curlpp-0.8.1
tests.packageTestsForChannelBlockers.curl.fetchpatch.x86_64-linux                     /nix/store/gh62y23d9xjx38xn4mkwfmy6gmpplyw2-source-salted-z09f6i1ny721
tests.packageTestsForChannelBlockers.curl.haskell-curl.x86_64-darwin                  doc=/nix/store/iv8lyrn13x472gxfiv4bbfx2fp007bmi-curl-1.3.8-doc;/nix/store/k2q9dm1sd7pc16ydn3xkn02h700vmkpd-curl-1.3.8
tests.packageTestsForChannelBlockers.curl.haskell-curl.x86_64-linux                   doc=/nix/store/i26k3nsqm5i4w29ywizngkmnyvd9rydd-curl-1.3.8-doc;/nix/store/8id3zbivfiiy856w24675v2i7vrsa7v7-curl-1.3.8
tests.packageTestsForChannelBlockers.curl.nginx-http3.x86_64-linux                    /nix/store/isd4bnd78js04zm4mhhxlbfbdlczf01j-vm-test-run-nginx-http3
tests.packageTestsForChannelBlockers.curl.ocaml-curly.x86_64-darwin                   /nix/store/vs7zsly4b9ynkmky48hipbvrmdamxnxa-ocaml4.13.1-curly-0.2.0
tests.packageTestsForChannelBlockers.curl.ocaml-curly.x86_64-linux                    /nix/store/pr8128y509li57qqzf6misk0a7h18ci4-ocaml4.13.1-curly-0.2.0
tests.packageTestsForChannelBlockers.curl.php-curl.x86_64-linux                       dev=/nix/store/f7pa423hkdpzqvii0xdbk7s3r0ljqczw-php-curl-8.1.7-dev;/nix/store/aijj4f0f6iskri7j0i955f6rvg7f43ls-php-curl-8.1.7
tests.packageTestsForChannelBlockers.curl.pycurl.x86_64-darwin                        /nix/store/8rim82j9akdcq55jrnaj9ggwsz6400bg-python3.10-pycurl-7.45.1
tests.packageTestsForChannelBlockers.curl.pycurl.x86_64-linux                         /nix/store/dyr6m5ac757jvgpvj72w4k6fi3i6r9n6-python3.10-pycurl-7.45.1
tests.packageTestsForChannelBlockers.curl.withCheck.x86_64-darwin                     bin=/nix/store/bc5z41yg99n8f58hjfvrcpp552n872qs-curl-7.83.1-bin;dev=/nix/store/ly0q61iaf2gfssf9aab4mqb3qrgbkil1-curl-7.83.1-dev;devdoc=/nix/store/wik2qchahxksfmdcrgamc5pmdpzhz811-curl-7.83.1-devdoc;man=/nix/store/ccrjr4z4p7l9b5c2h465xbyiwna80w6h-curl-7.83.1-man;/nix/store/81axcdapacnw0dxjrw0f00brqh2x3b9c-curl-7.83.1
tests.packageTestsForChannelBlockers.curl.withCheck.x86_64-linux                      bin=/nix/store/8w50x3ygiymlxmy36gj7y6xsg9kqvnjv-curl-7.83.1-bin;debug=/nix/store/zpf9j93lc3br13wg9adh2la9ksay0npz-curl-7.83.1-debug;dev=/nix/store/zv467z7cq9yl23igax527z64krp4vr4r-curl-7.83.1-dev;devdoc=/nix/store/qfa4ifwbm6sq11qz1q2b924nb986k2q0-curl-7.83.1-devdoc;man=/nix/store/03kkz40l7yc4qgdaf3kp4b86z6zjzzx3-curl-7.83.1-man;/nix/store/qz9lczmn7h5l9gz7yw57kjl1q7msy275-curl-7.83.1

… channel blocker we can't add 'nixpkgs.curl.tests' to hydra jobs due to 'tests' (and 'passthru') being stripped TODO: add a function in lib-release.nix to get derivations and add `.x86_64-linux` to them then we can just point release files to nixpkgs.tests.packageTestsForChannelBlockers instead of nixpkgs.tests.packageTestsForChannelBlockers.curl.withCheck

vcunat · 2022-07-11T16:26:50Z

Error: https://hydra.nixos.org/jobset/nixos/staging-small#tabs-errors

vcunat · 2022-07-11T16:27:27Z

BTW, this is in staging-next now, PR #181133

Artturin · 2022-07-12T12:04:28Z

reverted the release tests for now in #181220

phpPackage.mkExtension: makeOverridable

67d19f4

Artturin force-pushed the curlies branch from 54b64ac to efb9de6 Compare June 24, 2022 14:35

Artturin requested review from Ma27, aanderse, etu, globin, jtojnar and talyz as code owners June 24, 2022 14:35

Artturin requested a review from roberth June 24, 2022 14:37

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jun 24, 2022

curl: use finalAttrs

6bce3fc

allows using the correct package in passthru remove unneeded replaceStrings in urls

Artturin requested a review from SuperSandro2000 June 24, 2022 16:26

Artturin force-pushed the curlies branch 2 times, most recently from e73468f to 11a8dd3 Compare June 24, 2022 16:48

roberth reviewed Jun 25, 2022

View reviewed changes

Artturin changed the title ~~curl: use finalAttrs~~ curl: run tests in passthru Jun 25, 2022

Artturin force-pushed the curlies branch from 11a8dd3 to c562082 Compare June 25, 2022 20:22

curl: put tests in tests.withCheck

47b58ff

curl is built many times during a stdenv rebuild they are a bottleneck

Artturin force-pushed the curlies branch from c562082 to 47b58ff Compare June 27, 2022 23:17

Ma27 approved these changes Jun 28, 2022

View reviewed changes

roberth reviewed Jun 29, 2022

View reviewed changes

Artturin commented Jun 29, 2022

View reviewed changes

pkgs/tools/networking/curl/default.nix Outdated Show resolved Hide resolved

github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jun 29, 2022

Artturin commented Jun 29, 2022

View reviewed changes

pkgs/tools/networking/curl/default.nix Outdated Show resolved Hide resolved

Artturin force-pushed the curlies branch from bbfac90 to e21a2a7 Compare July 1, 2022 00:34

roberth reviewed Jul 1, 2022

View reviewed changes

Artturin force-pushed the curlies branch from e21a2a7 to d351819 Compare July 1, 2022 15:17

Artturin force-pushed the curlies branch from d351819 to 0c93a11 Compare July 2, 2022 00:37

Artturin force-pushed the curlies branch from 0c93a11 to 7141ab0 Compare July 2, 2022 00:38

ofborg bot added the 8.has: package (new) This PR adds a new package label Jul 2, 2022

Artturin force-pushed the curlies branch from b06f7af to 7141ab0 Compare July 3, 2022 13:04

roberth approved these changes Jul 5, 2022

View reviewed changes

Artturin merged commit fb6816e into NixOS:staging Jul 6, 2022

Artturin deleted the curlies branch July 6, 2022 14:30

Artturin mentioned this pull request Jul 12, 2022

Revert "release: add tests.packageTestsForChannelBlockers.curl.withCh… #181214

Merged

13 tasks

risicle mentioned this pull request Aug 7, 2022

[Backport staging-22.05] zlib: add patch for CVE-2022-37434 #185578

Closed

1 task

-  # we can't add 'nixpkgs.curl.tests' to hydra jobs due to 'passthru' being stripped
-  passthruTestsForChannelBlockers = recurseIntoAttrs {
+  # we can't add 'nixpkgs.curl.tests' to hydra jobs due to 'tests' (and 'passthru') being stripped
+  packageTestsForChannelBlockers = recurseIntoAttrs {

Uh oh!

Conversation

Artturin commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SuperSandro2000 commented Jun 26, 2022

Uh oh!

Mindavi commented Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

risicle commented Jun 29, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 commented Jul 1, 2022

Uh oh!

Artturin commented Jul 1, 2022

Uh oh!

vcunat commented Jul 11, 2022

Uh oh!

vcunat commented Jul 11, 2022

Uh oh!

Artturin commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Artturin commented Jun 24, 2022 •

edited

Loading

Mindavi commented Jun 29, 2022 •

edited

Loading

Artturin commented Jul 12, 2022 •

edited

Loading