Skip to content

Conversation

@targos
Copy link
Member

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Nov 14, 2022
@targostargos added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Nov 14, 2022
@targos
Copy link
MemberAuthor

I used https://github.com/nodejs/node/blob/main/tools/update-nghttp2.sh but somehow config.h is removed while still being included by other files.

@targos
Copy link
MemberAuthor

@yashLadha is the script missing a step?

Copy link
Contributor

@ShogunPandaShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@targos
Copy link
MemberAuthor

@ShogunPanda but it doesn't work!

@ShogunPanda
Copy link
Contributor

@targos I haven't executed it :) Changes on source code seemed reasonable, I was not really looking in the build script.

@yashLadha
Copy link
Contributor

@yashLadha is the script missing a step?

No @targos last time I checked it worked completely fine.

Can you paste the log about what is the exact error. Might be due to recent changes in upstream dependencies.

@targos
Copy link
MemberAuthor

You can check any of the failing builds here

@targostargos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Nov 15, 2022
@gengjiawen
Copy link
Member

@ShogunPanda but it doesn't work!

It's auto-generated, you need to copy it from build folder.
image

@gengjiawen
Copy link
Member

gengjiawen commented Nov 15, 2022

Original config.h for v.1.51.0. But from the comment we may need to revert current one since the file comments like
/* Edited to match src/node.h. */

config.h
/* Hint to the compiler that a function never returns */#defineNGHTTP2_NORETURN __attribute__((noreturn)) /* Define to `int' if <sys/types.h> does not define. *//* #undef ssize_t *//* Define to 1 if you have the `std::map::emplace`. */#defineHAVE_STD_MAP_EMPLACE 1 /* Define to 1 if you have `libjansson` library. */#defineHAVE_JANSSON 1 /* Define to 1 if you have `libxml2` library. */#defineHAVE_LIBXML2 1 /* Define to 1 if you have `mruby` library. *//* #undef HAVE_MRUBY *//* Define to 1 if you have `neverbleed` library. *//* #undef HAVE_NEVERBLEED *//* sizeof(int *) */#defineSIZEOF_INT_P 8 /* sizeof(time_t) */#defineSIZEOF_TIME_T 8 /* Define to 1 if you have the `_Exit` function. */#defineHAVE__EXIT 1 /* Define to 1 if you have the `accept4` function. *//* #undef HAVE_ACCEPT4 *//* Define to 1 if you have the `mkostemp` function. */#defineHAVE_MKOSTEMP 1 /* Define to 1 if you have the `initgroups` function. */#defineHAVE_DECL_INITGROUPS 1 /* Define to 1 to enable debug output. *//* #undef DEBUGBUILD *//* Define to 1 if you want to disable threads. *//* #undef NOTHREADS *//* Define to 1 if you have the <arpa/inet.h> header file. */#defineHAVE_ARPA_INET_H 1 /* Define to 1 if you have the <fcntl.h> header file. */#defineHAVE_FCNTL_H 1 /* Define to 1 if you have the <inttypes.h> header file. */#defineHAVE_INTTYPES_H 1 /* Define to 1 if you have the <limits.h> header file. */#defineHAVE_LIMITS_H 1 /* Define to 1 if you have the <netdb.h> header file. */#defineHAVE_NETDB_H 1 /* Define to 1 if you have the <netinet/in.h> header file. */#defineHAVE_NETINET_IN_H 1 /* Define to 1 if you have the <pwd.h> header file. */#defineHAVE_PWD_H 1 /* Define to 1 if you have the <sys/socket.h> header file. */#defineHAVE_SYS_SOCKET_H 1 /* Define to 1 if you have the <sys/time.h> header file. */#defineHAVE_SYS_TIME_H 1 /* Define to 1 if you have the <syslog.h> header file. */#defineHAVE_SYSLOG_H 1 /* Define to 1 if you have the <time.h> header file. */#defineHAVE_TIME_H 1 /* Define to 1 if you have the <unistd.h> header file. */#defineHAVE_UNISTD_H 1 /* Define to 1 if HTTP/3 is enabled. *//* #undef ENABLE_HTTP3 *//* Define to 1 if you have `libbpf` library. *//* #undef HAVE_LIBBPF *//* Define to 1 if you have enum bpf_stats_type in linux/bpf.h. *//* #undef HAVE_BPF_STATS_TYPE *//* Define to 1 if you have `libngtcp2_crypto_openssl` library. *//* #undef HAVE_LIBNGTCP2_CRYPTO_OPENSSL */

/* Hint to the compiler that a function never returns */
#define NGHTTP2_NORETURN

/* Edited to match src/node.h. */
Copy link
Member

Choose a reason for hiding this comment

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

More likely we need to revert this file than using the auto-gen one.

@targos
Copy link
MemberAuthor

@yashLadha If I run the script with the current version (./tools/update-nghttp2.sh 1.47.0), the same problem happens (config.h is deleted):

$ git status On branch main Your branch is up to date with 'origin/main'. nothing to commit, working tree clean $ ./tools/update-nghttp2.sh 1.47.0 Making temporary workspace Fetching nghttp2 source archive Removing everything, except lib/ and COPYING Copying existing gyp files Replacing existing nghttp2 All done! Please git add nghttp2, commit the new version: $ git add -A deps/nghttp2 $ git commit -m "deps: update nghttp2 to 1.47.0" $ git status On branch main Your branch is up to date with 'origin/main'. Changes not staged for commit: (use "git add/rm <file>..." to update what will be committed) (use "git restore <file>..." to discard changes in working directory) deleted: deps/nghttp2/lib/includes/config.h no changes added to commit (use "git add" and/or "git commit -a") 

@yashLadha
Copy link
Contributor

Will check today, and close on this.

@yashLadha
Copy link
Contributor

I checked the 1.47.0 tag as well and there were no include.h file present in the tree, yet the builds passed #42127. I don't see anything changed in the script.

@targostargos closed this Nov 22, 2022
@targostargos deleted the nghttp2-1.51.0 branch November 22, 2022 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wantedIssues that need assistance from volunteers or PRs that need help to proceed.http2Issues or PRs related to the http2 subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@targos@nodejs-github-bot@ShogunPanda@yashLadha@gengjiawen