v10.20.0 N-API headers as shipped in the headers tarball disagree with the headers as tagged

[gabrielschulhof]

In the repository, for v10.20.0 we have https://github.com/nodejs/node/blob/v10.20.0/src/node_api.h#L702 which, at that line, has the correct guard.

When unpacking https://nodejs.org/dist/v10.20.0/node-v10.20.0-headers.tar.xz and examining node_api.h the guard found there is #ifdef NAPI_EXPERIMENTAL which is wrong, rather than #if NAPI_VERSION >= 6

[targos]

We can see here: https://nodejs.org/download/release/v10.20.0/

Some files are not dated from Apr 08 but Mar 26 instead. That includes the source headers tarballs and API documentation.

/cc @nodejs/build

[targos]

/cc @BethGriggs did you notice anything unusual when you promoted the release assets?

[gabrielschulhof]

@targos you're right. node-v10.20.0-linux-x64.tar.xz has the right header. This causes nvm to install a broken version of v10.20.0 though. I guess nvm must be downloading node-v10.20.0-headers.tar.gz.

[targos]

I don't know if it's related, but v13.12.0 was released on March 26.

[ljharb]

@gabrielschulhof nvm just calls configure, make, and make install on the source tarball, it doesn't download any headers tarball i'm aware of.

[rvagg]

More infra dramas, isn't this fun.

See nodejs/build#2282 for what I believe has happened. I don't have all the pieces (i.e. I only have a theory for the old headers and source tarball but I can explain why there aren't new ones as per that PR).

10.20.0 should probably be considered broken because of the mismatch here. I don't know what commit the source and headers tarballs come from but they aren't from what was tagged v10.20.0. So we should churn out a v10.20.1 ASAP.

Sorry @BethGriggs.

[rvagg]

/cc @nodejs/releasers FYI, see above comment.

a 10.20.1 just needs to be a version bump & changelog, everything else can stay the same. The release notes will have to come up with some language to describe what happened. Something like:

Due to release process failures, Node.js v10.20.0 shipped with source and header tarballs that did not properly match the final release commit that was used to build the binaries. We recommend that Node.js v10.20.0 not be used, particularly in any applications using native add-ons or where compiling Node.js from source is involved.

Node.js v10.20.1 is a clean release with the correct sources and is strongly recommended in place of v10.20.0.

[gabrielschulhof]

@ljharb you're right. It's not nvm. It's node-gyp. It downloads the headers, stores them in ~/.cache/node-gyp/v10.20.0 and then passes that as the include path when it builds:

$ /home/nix/.nvm/versions/node/v10.20.0/lib/node_modules/npm/bin/node-gyp-bin/node-gyp --verbose rebuild 
[...]
gyp verb get node dir no --target version specified, falling back to host node version: 10.20.0
gyp verb command install [ '10.20.0' ]
gyp verb install input version string "10.20.0"
gyp verb install installing version: 10.20.0
gyp verb install --ensure was passed, so won't reinstall if already installed
gyp verb install version not already installed, continuing with install 10.20.0
gyp verb ensuring nodedir is created /home/nix/.cache/node-gyp/10.20.0
gyp verb created nodedir /home/nix/.cache/node-gyp
gyp http GET https://nodejs.org/download/release/v10.20.0/node-v10.20.0-headers.tar.gz
gyp http 200 https://nodejs.org/download/release/v10.20.0/node-v10.20.0-headers.tar.gz
gyp verb extracted file from tarball node-v10.20.0/include/node/common.gypi
gyp verb extracted file from tarball node-v10.20.0/include/node/config.gypi
gyp verb extracted file from tarball node-v10.20.0/include/node/node.h
gyp verb extracted file from tarball node-v10.20.0/include/node/node_api.h
gyp verb extracted file from tarball node-v10.20.0/include/node/node_api_types.h
[...]
gyp verb get node dir target node version installed: 10.20.0
gyp verb build dir attempting to create "build" dir: /home/nix/node/node-addon-api/test/build
gyp verb build dir "build" dir needed to be created? /home/nix/node/node-addon-api/test/build
gyp verb build/config.gypi creating config file
gyp verb build/config.gypi writing out config file: /home/nix/node/node-addon-api/test/build/config.gypi
gyp verb config.gypi checking for gypi file: /home/nix/node/node-addon-api/test/config.gypi
gyp verb common.gypi checking for gypi file: /home/nix/node/node-addon-api/test/common.gypi
gyp verb gyp gyp format was not specified; forcing "make"
gyp info spawn /usr/bin/python
gyp info spawn args [ '/home/nix/.nvm/versions/node/v10.20.0/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/home/nix/node/node-addon-api/test/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/nix/.nvm/versions/node/v10.20.0/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/nix/.cache/node-gyp/10.20.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/home/nix/.cache/node-gyp/10.20.0',
gyp info spawn args   '-Dnode_gyp_dir=/home/nix/.nvm/versions/node/v10.20.0/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/home/nix/.cache/node-gyp/10.20.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/home/nix/node/node-addon-api/test',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
gyp verb command build []
gyp verb build type Release
gyp verb architecture x64
gyp verb node dev dir /home/nix/.cache/node-gyp/10.20.0
gyp verb `which` succeeded for `make` /usr/bin/make
gyp info spawn make
gyp info spawn args [ 'V=1', 'BUILDTYPE=Release', '-C', 'build' ]
make: Entering directory '/home/nix/node/node-addon-api/test/build'
  g++ '-DNODE_GYP_MODULE_NAME=binding' '-DUSING_UV_SHARED=1'                   \
  '-DUSING_V8_SHARED=1' '-DV8_DEPRECATION_WARNINGS=1' '-D_LARGEFILE_SOURCE'    \
  '-D_FILE_OFFSET_BITS=64' '-DOPENSSL_NO_PINSHARED'                            \
                                                                               \
  '-DNAPI_VERSION=6'                                                           \
                                                                               \
  '-DNAPI_CPP_EXCEPTIONS' '-DBUILDING_NODE_EXTENSION'                          \
                                                                               \
  -I/home/nix/.cache/node-gyp/10.20.0/include/node                             \ <
  -I/home/nix/.cache/node-gyp/10.20.0/src                                      \ <
  -I/home/nix/.cache/node-gyp/10.20.0/deps/openssl/config                      \ <
  -I/home/nix/.cache/node-gyp/10.20.0/deps/openssl/openssl/include             \ <
  -I/home/nix/.cache/node-gyp/10.20.0/deps/uv/include                          \ <
  -I/home/nix/.cache/node-gyp/10.20.0/deps/zlib                                \ <
  -I/home/nix/.cache/node-gyp/10.20.0/deps/v8/include                          \ <
  -I/home/nix/node/node-addon-api                                              \
                                                                               \
  -Wall -Wextra -Wpedantic -Wunused-parameter -fPIC -pthread -Wall -Wextra     \
  -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -Werror -Wall -Wextra \
  -Wpedantic -Wunused-parameter -fno-rtti -std=gnu++1y -MMD -MF                \
  ./Release/.deps/Release/obj.target/binding/arraybuffer.o.d.raw   -c -o       \
  Release/obj.target/binding/arraybuffer.o ../arraybuffer.cc

[BethGriggs]

A follow-up release v10.20.1 (#32768) has been prepared to resolve this, and will be released as soon as builds are available.

[gabrielschulhof]

A notable PR number ... 2^15 – had to be mentioned.

[BethGriggs]

Fixed in v10.20.1 (#32768) - https://nodejs.org/en/blog/release/v10.20.1/