★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/27283Nouvelle recherche | Portail wanayoo
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

deps: use nghttp2's config.h on all platforms #27283

Closed
wants to merge 1 commit into from

Conversation

Copy link
Contributor

@sam-github sam-github commented Apr 17, 2019

Fix warnings about use of htonl(), etc. by including config.h for all
platforms, defining HAVE_ARPA_INET_H on non-Windows, and therefore
including <arpa/inet.h>, which defines the host to network byte order
conversion functions.

--

This works on Linux, I'll see what ci says about the other platforms.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 17, 2019

@nodejs-github-bot nodejs-github-bot added build dont-land-on-v6.x http2 labels Apr 17, 2019
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 17, 2019

deps/nghttp2/nghttp2.gyp Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 18, 2019

@richardlau
Copy link
Member

@richardlau richardlau commented Apr 18, 2019

Travis failed with:
https://travis-ci.com/nodejs/node/jobs/193769803#L264-L285

=== release test-crypto-sign-verify ===
Path: parallel/test-crypto-sign-verify
--- stderr ---
assert.js:523
      throw err;
      ^
AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected
  Comparison {
+   message: 'error:0908F070:PEM routines:get_header_and_data:short header'
-   message: 'bye, bye, library'
  }
    at Object.<anonymous> (/home/travis/build/nodejs/node/test/parallel/test-crypto-sign-verify.js:42:10)
    at Module._compile (internal/modules/cjs/loader.js:766:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:777:10)
    at Module.load (internal/modules/cjs/loader.js:635:32)
    at Function.Module._load (internal/modules/cjs/loader.js:562:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:833:10)
    at internal/main/run_main_module.js:17:11
Command: out/Release/node /home/travis/build/nodejs/node/test/parallel/test-crypto-sign-verify.js

Unrelated?

Fix warnings about use of htonl(), etc. by including config.h for all
platforms, defining HAVE_ARPA_INET_H on non-Windows, and therefore
including <arpa/inet.h>, which defines the host to network byte order
conversion functions.
@sam-github
Copy link
Contributor Author

@sam-github sam-github commented Apr 18, 2019

Should not be related, I wrote that test in #27157, and it passed there, except travis's CI state isn't visible anymore.

I found some other recent travis jobs that include #27157, and they passed, like https://travis-ci.com/nodejs/node/builds/108825880

I rebased against master, and repushed.

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 18, 2019

@sam-github
Copy link
Contributor Author

@sam-github sam-github commented Apr 22, 2019

Landed in 90cf2d5

@sam-github sam-github closed this Apr 22, 2019
@sam-github sam-github deleted the use-nghttp2-config-h branch Apr 22, 2019
sam-github added a commit that referenced this issue Apr 22, 2019
Fix warnings about use of htonl(), etc. by including config.h for all
platforms, defining HAVE_ARPA_INET_H on non-Windows, and therefore
including <arpa/inet.h>, which defines the host to network byte order
conversion functions.

PR-URL: #27283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build http2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants