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

node 17.1.0 #89068

Closed
wants to merge 6 commits into from
Closed

node 17.1.0 #89068

wants to merge 6 commits into from

Conversation

tuzi3040
Copy link
Contributor

@tuzi3040 tuzi3040 commented Nov 9, 2021

Created with brew bump-formula-pr.

resource blocks may require updates.

npm updated to 8.1.2.

@BrewTestBot BrewTestBot added bump-formula-pr PR was created using `brew bump-formula-pr` python Python use is a significant feature of the PR or issue labels Nov 9, 2021
@tuzi3040 tuzi3040 closed this Nov 9, 2021
@tuzi3040 tuzi3040 reopened this Nov 9, 2021
@carlocab
Copy link
Member

carlocab commented Nov 10, 2021

Probably needs OpenSSL 3.0. We'd need to make [email protected] a build dependency (and probably make it [email protected] while we're at it), and add jupyterlab, anime-downloader and emscripten to the versioned conflicts allowlist.

lanraragi will also be a problem.

@carlocab
Copy link
Member

I think this will fail the tap syntax job for conflicting versioned dependencies.

@BrewTestBot BrewTestBot added automerge-skip `brew pr-automerge` will skip this pull request and removed python Python use is a significant feature of the PR or issue labels Nov 10, 2021
Formula/node.rb Outdated Show resolved Hide resolved
Formula/node.rb Outdated Show resolved Hide resolved
@tuzi3040
Copy link
Contributor Author

Also, in the previous check, clang raised error since SSL_QUIC_METHOD is not found.

  clang -o /private/tmp/node-20211109-60450-k5ll9i/node-v17.1.0/out/Release/obj.target/openssl/deps/openssl/openssl/ssl/bio_ssl.o ../deps/openssl/openssl/ssl/bio_ssl.c '-DV8_DEPRECATION_WARNINGS' '-DV8_IMMINENT_DEPRECATION_WARNINGS' '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CERT_STORE' '-D_DARWIN_USE_64_BIT_INODE=1' '-DOPENSSL_NO_HW' '-DOPENSSL_API_COMPAT=0x10100001L' '-DSTATIC_LEGACY' '-DNDEBUG' '-DL_ENDIAN' '-DOPENSSL_BUILDING_OPENSSL' '-DECP_NISTZ256_ASM' '-DKECCAK1600_ASM' '-DOPENSSL_BN_ASM_MONT' '-DOPENSSL_CPUID_OBJ' '-DPOLY1305_ASM' '-DSHA1_ASM' '-DSHA256_ASM' '-DSHA512_ASM' '-DVPAES_ASM' '-DOPENSSL_PIC' '-DENGINESDIR="/dev/null"' -I/opt/homebrew/opt/libuv/include -I/opt/homebrew/opt/brotli/include -I/opt/homebrew/opt/c-ares/include -I/opt/homebrew/opt/libnghttp2/include -I/opt/homebrew/opt/openssl@3/include -I/opt/homebrew/Cellar/icu4c/69.1/include -I../deps/openssl/openssl -I../deps/openssl/openssl/include -I../deps/openssl/openssl/crypto -I../deps/openssl/openssl/crypto/include -I../deps/openssl/openssl/crypto/modes -I../deps/openssl/openssl/crypto/ec/curve448 -I../deps/openssl/openssl/crypto/ec/curve448/arch_32 -I../deps/openssl/openssl/providers/common/include -I../deps/openssl/openssl/providers/implementations/include -I../deps/openssl/config -I../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2 -I../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/include -I../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/crypto -I../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/crypto/include/internal -I../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/providers/common/include  -O3 -gdwarf-2 -flto -mmacosx-version-min=10.13 -arch arm64 -Wall -Wendif-labels -W -Wno-unused-parameter -Wno-missing-field-initializers -fno-strict-aliasing -MMD -MF /private/tmp/node-20211109-60450-k5ll9i/node-v17.1.0/out/Release/.deps//private/tmp/node-20211109-60450-k5ll9i/node-v17.1.0/out/Release/obj.target/openssl/deps/openssl/openssl/ssl/bio_ssl.o.d.raw   -c

In file included from ../deps/openssl/openssl/ssl/bio_ssl.c:17:
../deps/openssl/openssl/ssl/ssl_local.h:1213:11: error: unknown type name 'SSL_QUIC_METHOD'; did you mean 'SSL_METHOD'?
    const SSL_QUIC_METHOD *quic_method;
          ^~~~~~~~~~~~~~~
          SSL_METHOD
/opt/homebrew/opt/openssl@3/include/openssl/ssl.h:229:30: note: 'SSL_METHOD' declared here
typedef struct ssl_method_st SSL_METHOD;
                             ^

This is because QUIC is not yet implemented in official release of OpenSSL 3.
node used quictls/openssl to support QUIC in node@17.
nodejs/node#38512
node@17 release note

We might need to switch to quictls to build.

@carlocab
Copy link
Member

carlocab commented Nov 10, 2021

Do we not just need to pass --openssl-legacy-provider?

You'll also need to squash your commits together, because @BrewTestBot can't do that for you here (hence the automerge-skip label).

@tuzi3040
Copy link
Contributor Author

Do we not just need to pass --openssl-legacy-provider?

Honestly, I don't think this would work, since this option is aimed at bypassing default restriction in OpenSSL 3.0. This probably has nothing to do with the QUIC stuff.

You'll also need to squash your commits together, because @BrewTestBot can't do that for you here (hence the automerge-skip label).

Got you, will do when ready to merge.

@carlocab
Copy link
Member

Honestly, I don't think this would work, since this option is aimed at bypassing default restriction in OpenSSL 3.0. This probably has nothing to do with the QUIC stuff.

We should ask upstream if they no longer support building with a non-forked OpenSSL then.

Got you, will do when ready to merge.

Waiting until then will mean re-running CI another time, so sooner is probably better.

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Nov 12, 2021
@github-actions github-actions bot closed this Nov 13, 2021
@tuzi3040
Copy link
Contributor Author

tuzi3040 commented Nov 13, 2021

Also, in the previous check, clang raised error since SSL_QUIC_METHOD is not found.
We might need to switch to quictls to build.

Seems related to nodejs/node#40783.

We should ask upstream if they no longer support building with a non-forked OpenSSL then.

According to nodejs/node#38512, specifically the pr description,

We will still be able to dynamically link to OpenSSL 1.1.1 and we have a CI job which dynamically links to OpenSSL 1.1.1 which is run for every pull request to make sure that we maintain backward compatibility.

We probably don't need to use openssl@3 yet. Dynamic linked openssl 1.1 is still supported. Thus no dependence conflict.

@derrabus
Copy link
Contributor

I think, we should leave this PR open until it's resolved, dear bot.

@tuzi3040
Copy link
Contributor Author

I reverted two changes to switch back to openssl 1.1.1, would you please reopen this pr please? @carlocab

@carlocab
Copy link
Member

Unfortunately, I can't reopen this either. The reopen button is greyed out with a comment that says "The bump-node-17.1.0 branch was force-pushed or recreated." when my mouse pointer hovers over it.

@tuzi3040
Copy link
Contributor Author

tuzi3040 commented Nov 15, 2021

Unfortunately, I can't reopen this either. The reopen button is greyed out with a comment that says "The bump-node-17.1.0 branch was force-pushed or recreated." when my mouse pointer hovers over it.

Oh, my bad. Opening another pr #89459.

@tuzi3040 tuzi3040 mentioned this pull request Nov 15, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge-skip `brew pr-automerge` will skip this pull request bump-formula-pr PR was created using `brew bump-formula-pr` outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants