nghttp2 => 1.24.0 + --enable-app #28376

ixmatus · 2017-08-18T19:29:01Z

This change updates nghttp2 to the latest stable released version, 1.24.0. Additionally, a boolean argument, enableApp, is added to build nghttp2's applications (e.g: nghttpx, nghttpd, h2load, etc.)

Motivation for this change

The motivation for this change is two-fold:

Pick up the latest stable code for nghttp2.
Be able to build the apps provided by nghttp2, which is a prerequisite for a follow-on PR adding a new NixOS module for nghttpx.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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.

mention-bot · 2017-08-18T19:29:03Z

@ixmatus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @c0bw3b, @wkennington and @edolstra to be potential reviewers.

makefu · 2017-08-18T19:35:36Z

The PR Looks good but i recommend to squash the three commits into one which introduces the new feature and bumps the version number or rewrite the commits into:

Version bump "nghttp2: 1.20.0 -> 1.24.0"
Introduction of the new feature "nghttp2: add enableApp"

The commit messages should be in accordance with the CONTRIBUTING document.
I am looking forward for the nghttp2 module PR 👍

ixmatus · 2017-08-18T19:53:29Z

Thanks for the feedback @makefu.

ixmatus · 2017-08-18T19:57:00Z

Oh, technically my commit message still doesn't meet CONTRIBUTING.md, I'll fix again.

makefu · 2017-08-19T09:10:37Z

compiles and tested binaries, new 'app' related tools seem to be working.

@Mic92 would you be so kind? 👍

ixmatus · 2017-08-21T16:19:26Z

Thanks for the verification @makefu. Is it still the case that the Travis build failures can be ignored like it was a few months ago? If not, I will go investigate...

Otherwise, if we could get this merged that'd be nice so I can put up my PR for the nghttpx NixOS module.

CC: @domenkozar

makefu · 2017-08-21T18:00:27Z

Hi @ixmatus , you are right - a failing Travis bot has lost its meaning some time ago.

@qknight can you approve this PR?

Thanks!

fpletz · 2017-08-22T00:05:40Z

pkgs/development/libraries/nghttp2/default.nix

@@ -6,6 +6,7 @@
 , enableAsioLib ? false, boost ? null
 , enableGetAssets ? false, libxml2 ? null
 , enableJemalloc ? false, jemalloc ? null
+, enableApp ? false


You could alternatively put the apps into their own output. Adding bin to outputs might suffice.

I was trying to follow the derivation as it was as closely as possible but I'll admit I hadn't thought of your suggestion 😄.

I'll follow your suggestion and push a revision to the commit this evening. Thanks @fpletz.

Actually, this made $out basically empty, as libs are in $lib, so it's maybe a little "oversplit". ($out isn't normally referenced)

ixmatus · 2017-08-22T14:34:51Z

@fpletz I've addressed your feedback.

globin · 2017-08-22T18:11:00Z

I think we always have the bin output first in the list so that gets installed.. (@dezgeg?)

ixmatus · 2017-08-22T18:25:17Z

@globin I'm confused. I tested it and all of the application binaries were installed into the bin output. Is that what you're talking about?

globin · 2017-08-22T18:33:11Z

No this ensures that you can do e.g ${drvWithBinOutput}/bin/executable instead of having to add .bin :)
Sry that last comment was not clear at all..

Mic92 · 2017-08-22T19:26:14Z

I think you can also retarget staging for this one. nghttp2 -> curl -> quite a lot of packages.

ixmatus · 2017-08-22T20:11:53Z

@globin oh I see, I'll amend the commit with that, then.

@Mic92 I do not understand what you mean by retargeting for staging.

This change does three things: 1. Increases nghttp2's version from 1.20.0 to 1.24.0 2. Adds a `bin` output 3. Enables building of nghttp2's suite of applications

Mic92 · 2017-08-22T20:15:24Z

I mean changing the target branch to staging instead of master.

ixmatus · 2017-08-22T20:23:40Z

@Mic92 oh, I understand. Okay.

ixmatus · 2017-08-23T00:04:14Z

Don't merge yet. I have one more change to make to fix an issue I just noticed with nghttpx in our own test (ocsp cert stapling needs python, so I need to wrap nghttpx).

ixmatus · 2017-08-23T00:26:16Z

@fpletz @globin @Mic92 I'd appreciate your feedback on a1c6bfe. I didn't squash it with the other commit because I'm not positive it's the right thing to do here.

It fixes an issue I noticed with nghttpx by providing a python interpreter for ocsp stapling. However this means pulling in python for nghttp2 which I don't think is bad but it does make it heavier which is leaving me with the feeling that I might not be thinking about a second order effect given how central nghttp2 is in the build graph.

So I kept the commit separate in-case any of you have a really good reason for me to rethink how I'm solving this...

fpletz · 2017-08-30T20:24:18Z

pkgs/development/libraries/nghttp2/default.nix


  nativeBuildInputs = [ pkgconfig ];
-  buildInputs = [ openssl libev zlib c-ares ]
+  buildInputs = [ openssl libev zlib c-ares makeWrapper ]


makeWrapper is a nativeBuildInput.

... makeWrapper belongs in nativeBuildInputs

vcunat · 2017-09-02T17:12:46Z

Well, it would certainly be better to not to have build-time dependency fetchurl -> ... -> python.

Perhaps separate the wrapping into another derivation and pass only the unwrapped one to curl (as it uses only the libs anyway).

ixmatus · 2017-09-02T22:40:10Z

@vcunat thanks for taking a look. I'll try to address your concern this evening or tomorrow when I have some spare time.

Also --enable-app and use a $bin output.

vcunat · 2017-09-03T07:56:39Z

I merged the first commit to staging now, as it seems safe and there are many bugfixes within; we can improve later.

ixmatus · 2017-09-04T17:53:39Z

@vcunat I'll work on the improvements in b0a28f5 when I have some free time, I appreciate you taking the time to give me feedback on that commit and also merging something of the improvement for upstream 😄

c0bw3b · 2017-10-22T21:17:54Z

@ixmatus Actually we were already building the apps even when it wasn't wanted. See the builds history of libnghttp2 (ie nghttp2.lib) on Hydra. The --enable-apps configure option is automatic as long as openssl, libev, zlib and c-ares are in scope.

Moreover there is a confusion between nghttp2 the C library and nghttp2 apps suite. Trying to build both from one multi-outputs derivation just nurture that confusion.
Examples right in the nixpkgs tree are curl and apache : they use the full nghttp2 derivation as build inputs when they really need the lib alone... As a result, one will pull all nghttp2 apps when installing curlFull or apache. Which is an issue.

I agree with @vcunat that two different derivations would be much better.
We would need

a stable libnghttp2 that curl (and thus fetchurl) or apache could rely upon ;
a separate nghttp2 derivation for {h2load, nghttp, nghttpd, nghttpx} and we would be able to scope systemd or python or anything into this derivation, opening the road for a nghttpx service module.

FWIW Debian/Ubuntu/Arch/FreeBSD provides lib and apps as separate packages.

I'm willing to work on that and come back with a proper PR in time for 18.03.

vcunat · 2017-10-23T06:59:52Z

curlFull pulls only nghttp2.lib for runtime dependencies, but I suppose you meant build-time dependencies.

c0bw3b · 2017-10-24T13:21:20Z

Yes and no, I guess? 😼
It is just a build-time dependency in the curl derivation but :

[demo@nixos:~]$ nixos-version 
17.09.1812.36a4dc392a (Hummingbird)

[demo@nixos:~]$ find /nix/store/ -name "nghttpx"

[demo@nixos:~]$ nix-env -iA nixos.curlFull
installing ‘curl-7.56.0’
these paths will be fetched (2.80 MiB download, 5.24 MiB unpacked):
  /nix/store/3lg7jw4g54cy7l65bjk5qzwlb5m7cdxy-c-ares-1.13.0
  /nix/store/4z1n838nkpy42ns4ks749cjldgs5z4l2-openldap-2.4.45-dev
  /nix/store/a3ls83adr1dndnqm1r8d1rj50kq5rhl0-libev-4.24
  /nix/store/chkpwifxfmbkm4sdky2kdlhi2wb7ndld-libssh2-1.8.0-dev
>>/nix/store/dq226ai0lcr5kcp7d978q3ma71l6972i-nghttp2-1.20.0
  /nix/store/hsj75yi8k7m427xdg9xbb98qwc8xr92r-curl-7.56.0-devdoc
  /nix/store/jfd7rlwrkahyql7caknnnxpr1mcrfx3s-curl-7.56.0-dev
  /nix/store/kfiy6my66r0vl1kklwnb0q1x93mn04xq-nghttp2-1.20.0-dev
  /nix/store/lmm7dxr0dip3c545xn4gadb9h4jvij0m-curl-7.56.0-man
  /nix/store/pdlgahwrfx627ksx370difq2144y2wnz-curl-7.56.0-debug
  /nix/store/phhy9mspjwgzm4nv0c5c1j0vkpp9zapn-libidn-1.33-bin
  /nix/store/x03p60i19375ir9hsk6ac0wk1jiza30p-libidn-1.33-dev
[...]

[demo@nixos:~]$ tree /nix/store/dq226ai0lcr5kcp7d978q3ma71l6972i-nghttp2-1.20.0
/nix/store/dq226ai0lcr5kcp7d978q3ma71l6972i-nghttp2-1.20.0
├── bin
│   ├── h2load
│   ├── nghttp
│   ├── nghttpd
│   └── nghttpx
└── share
    ├── doc
    │   └── nghttp2
    │       └── README.rst
    ├── man
    │   └── man1
    │       ├── h2load.1.gz
    │       ├── nghttp.1.gz
    │       ├── nghttpd.1.gz
    │       └── nghttpx.1.gz
    └── nghttp2
        └── fetch-ocsp-response

7 directories, 10 files

[demo@nixos:~]$ find /nix/store/ -name "nghttpx"
/nix/store/dq226ai0lcr5kcp7d978q3ma71l6972i-nghttp2-1.20.0/bin/nghttpx

ixmatus · 2017-10-24T15:29:08Z

I agree, at this point I think separating the derivations will make things clearer. I still have yet to contribute my nghttpx nixos module so when I have some free time to work on that, if no one has tackled this, I will.

vcunat · 2017-10-24T19:45:15Z

That's just curlFull.dev depending on it due to putting it into propagatedBuildInputs. EDIT: I tend to count .dev outputs more like build-time dependencies, as they shouldn't normally make it into runtime closures...

matthewbauer · 2018-04-21T20:34:43Z

@GrahamcOfBorg build nghttp2

GrahamcOfBorg · 2018-04-21T20:35:13Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: nghttp2

Partial log (click to expand)

while evaluating the attribute 'C_INCLUDE_PATH' of the derivation 'python-2.7.14' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'C_INCLUDE_PATH' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:5:
while evaluating 'makeSearchPathOutput' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/lib/strings.nix�[0m:94:42, called from �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:22:
while evaluating 'makeSearchPath' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/lib/strings.nix�[0m:84:28, called from �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/lib/strings.nix�[0m:94:48:
while evaluating anonymous function at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/lib/strings.nix�[0m:85:32, called from undefined position:
while evaluating the attribute 'src' of the derivation 'db-5.3.28' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'nativeBuildInputs' of the derivation 'db-5.3.28.tar.gz' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'propagatedBuildInputs' of the derivation 'curl-7.59.0' at �[1m/private/var/lib/ofborg/builds/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
infinite recursion encountered, at undefined position

GrahamcOfBorg · 2018-04-21T20:35:15Z

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: nghttp2

Partial log (click to expand)

while evaluating the attribute 'C_INCLUDE_PATH' of the derivation 'python-2.7.14' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'C_INCLUDE_PATH' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:5:
while evaluating 'makeSearchPathOutput' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/lib/strings.nix�[0m:94:42, called from �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:22:
while evaluating 'makeSearchPath' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/lib/strings.nix�[0m:84:28, called from �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/lib/strings.nix�[0m:94:48:
while evaluating anonymous function at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/lib/strings.nix�[0m:85:32, called from undefined position:
while evaluating the attribute 'src' of the derivation 'db-5.3.28' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'nativeBuildInputs' of the derivation 'db-5.3.28.tar.gz' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'propagatedBuildInputs' of the derivation 'curl-7.59.0' at �[1m/home/ofborg/ofborg2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c2/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
infinite recursion encountered, at undefined position

GrahamcOfBorg · 2018-04-21T20:35:28Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: nghttp2

Partial log (click to expand)

while evaluating the attribute 'C_INCLUDE_PATH' of the derivation 'python-2.7.14' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'C_INCLUDE_PATH' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:5:
while evaluating 'makeSearchPathOutput' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/lib/strings.nix�[0m:94:42, called from �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/development/interpreters/python/cpython/2.7/default.nix�[0m:162:22:
while evaluating 'makeSearchPath' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/lib/strings.nix�[0m:84:28, called from �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/lib/strings.nix�[0m:94:48:
while evaluating anonymous function at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/lib/strings.nix�[0m:85:32, called from undefined position:
while evaluating the attribute 'patches' of the derivation 'libffi-3.2.1' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'nativeBuildInputs' of the derivation 'libffi-aarch64-rhbz1174037.patch' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
while evaluating the attribute 'propagatedBuildInputs' of the derivation 'curl-7.59.0' at �[1m/var/lib/gc-of-borg/nix-test-rs-14/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-14/pkgs/stdenv/generic/make-derivation.nix�[0m:153:11:
infinite recursion encountered, at undefined position

matthewbauer

See infinite recursion from @GrahamcOfBorg

ixmatus · 2018-04-23T13:46:49Z

I think we got what we wanted out of this. A few improvements were made (and merged) in subsequent commits too. I will close this.

ixmatus force-pushed the parnell/update-nghttp2 branch from 03deacc to 3efaa7a Compare August 18, 2017 19:52

ixmatus force-pushed the parnell/update-nghttp2 branch from 3efaa7a to 8cec113 Compare August 18, 2017 19:57

fpletz reviewed Aug 22, 2017

View reviewed changes

ixmatus force-pushed the parnell/update-nghttp2 branch from 8cec113 to 84ca88c Compare August 22, 2017 14:32

nghttp2: 1.20.0 -> 1.24.0

9dee190

This change does three things: 1. Increases nghttp2's version from 1.20.0 to 1.24.0 2. Adds a `bin` output 3. Enables building of nghttp2's suite of applications

ixmatus force-pushed the parnell/update-nghttp2 branch from 84ca88c to 9dee190 Compare August 22, 2017 20:12

ixmatus changed the base branch from master to staging August 22, 2017 20:24

fpletz reviewed Aug 30, 2017

View reviewed changes

Wrap nghttpx to provide the python interpreter

b0a28f5

... makeWrapper belongs in nativeBuildInputs

ixmatus force-pushed the parnell/update-nghttp2 branch from a1c6bfe to b0a28f5 Compare August 31, 2017 15:18

vcunat added a commit that referenced this pull request Sep 3, 2017

Merge: nghttp2: 1.20.0 -> 1.24.0 (#28376)

825c08e

Also --enable-app and use a $bin output.

c0bw3b mentioned this pull request Nov 25, 2017

nghttp2: 1.24.0 -> 1.28.0 + split lib and apps #32040

Closed

8 tasks

matthewbauer requested changes Apr 21, 2018

View reviewed changes

ixmatus closed this Apr 23, 2018

ixmatus deleted the parnell/update-nghttp2 branch April 23, 2018 13:46

nghttp2 => 1.24.0 + --enable-app #28376

nghttp2 => 1.24.0 + --enable-app #28376

Conversation

ixmatus commented Aug 18, 2017

Motivation for this change

Things done

mention-bot commented Aug 18, 2017

makefu commented Aug 18, 2017

ixmatus commented Aug 18, 2017

ixmatus commented Aug 18, 2017

makefu commented Aug 19, 2017

ixmatus commented Aug 21, 2017 • edited

makefu commented Aug 21, 2017

fpletz Aug 22, 2017

Choose a reason for hiding this comment

ixmatus Aug 22, 2017

Choose a reason for hiding this comment

vcunat Sep 2, 2017

Choose a reason for hiding this comment

ixmatus commented Aug 22, 2017

globin commented Aug 22, 2017

ixmatus commented Aug 22, 2017

globin commented Aug 22, 2017

Mic92 commented Aug 22, 2017

ixmatus commented Aug 22, 2017

Mic92 commented Aug 22, 2017 • edited

ixmatus commented Aug 22, 2017

ixmatus commented Aug 23, 2017

ixmatus commented Aug 23, 2017 • edited

fpletz Aug 30, 2017

Choose a reason for hiding this comment

vcunat commented Sep 2, 2017

ixmatus commented Sep 2, 2017

vcunat commented Sep 3, 2017

ixmatus commented Sep 4, 2017

c0bw3b commented Oct 22, 2017

vcunat commented Oct 23, 2017

c0bw3b commented Oct 24, 2017

ixmatus commented Oct 24, 2017

vcunat commented Oct 24, 2017 • edited

matthewbauer commented Apr 21, 2018

GrahamcOfBorg commented Apr 21, 2018

GrahamcOfBorg commented Apr 21, 2018

GrahamcOfBorg commented Apr 21, 2018

matthewbauer left a comment

Choose a reason for hiding this comment

ixmatus commented Apr 23, 2018

ixmatus commented Aug 21, 2017 •

edited

Mic92 commented Aug 22, 2017 •

edited

ixmatus commented Aug 23, 2017 •

edited

vcunat commented Oct 24, 2017 •

edited