Skip to content

Conversation

@codebytere
Copy link
Member

As in title. It's possible for NODE_EXTRA_CA_CERTS to work in a BoringSSL context, so there's no need to include that in the guard. This allows Electron users to leverage it.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 26, 2024
Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

This increases the number of #endif directives without changing the number of #if directives. You'll need to split the existing #if ... && ... into two separate directives in order to use separate #endif directives for the two conditions.

@codebytere
Copy link
Member Author

@tniessen oops, forgot to push that 😅 fixed.

@codebytere codebytere force-pushed the openssl-init-boringssl branch from 3b0f72c to 248c632 Compare March 26, 2024 11:04
Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

On a side note, the commit message does not adhere to the guidelines, I'd suggest crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL or something along those lines.

@codebytere codebytere force-pushed the openssl-init-boringssl branch from 248c632 to 452d594 Compare March 26, 2024 11:28
@codebytere codebytere requested a review from tniessen March 26, 2024 11:28
@tniessen tniessen changed the title crypto: BoringSSL can work with NODE_EXTRA_CA_CERTS crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL Mar 26, 2024
@codebytere codebytere force-pushed the openssl-init-boringssl branch from 452d594 to 8904a61 Compare March 26, 2024 12:02
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 11, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 11, 2024
@nodejs-github-bot nodejs-github-bot merged commit 28d68f3 into nodejs:main Apr 11, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in 28d68f3

aduh95 pushed a commit that referenced this pull request Apr 29, 2024
PR-URL: #52217 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@aduh95
Copy link
Contributor

aduh95 commented Apr 30, 2024

Should this be labelled semver-minor?

marco-ippolito pushed a commit that referenced this pull request May 2, 2024
PR-URL: #52217 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
PR-URL: #52217 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request May 8, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request May 13, 2024
* chore: bump node in DEPS to v20.13.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames - nodejs/node#51999 - nodejs/node#51927 * chore: bump node in DEPS to v20.13.1 * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: update patches --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
@codebytere codebytere deleted the openssl-init-boringssl branch May 31, 2024 13:32
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2024
* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.

8 participants