GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [v6.x backport] src: guard bundled_ca/openssl_ca with HAVE_OPENSSL by danbev · Pull Request #17783 · nodejs/node

Conversation

@danbev

@danbev danbev commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

This is a backport of the following PRs:

  • src: guard bundled_ca/openssl_ca with HAVE_OPENSSL #12302
  • src: add --use-bundled-ca --use-openssl-ca check #12087

Please note that there was a previous PR for backporting this, but was closed as noted in this comment.

@MylesBorins I'm not sure if this should land due to your comment above. Could you take a look and let me know what you think?

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
danbev added 2 commits December 20, 2017 07:41
@danbev
src: add --use-bundled-ca --use-openssl-ca check
c3f35be 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: nodejs#12083
PR-URL: nodejs#12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@danbev
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
c7bea9f 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: nodejs#12087
PR-URL: nodejs#12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v6.x labels Dec 20, 2017
@danbev

danbev commented Dec 20, 2017

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/12224/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 20, 2017
@MylesBorins

MylesBorins commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

thanks for opening @danbev

my apologies if we have already reviewed this before. I'm defensively marking as semver minor, and we can discuss with the @nodejs/lts team
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins

MylesBorins commented Jan 11, 2018

Copy link
Copy Markdown
Contributor

Should potentially come with #12688 as well
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn gibfahn assigned MylesBorins Jan 12, 2018
@MylesBorins

MylesBorins commented Jan 17, 2018

Copy link
Copy Markdown
Contributor

landed in 5f6f460...64f0967
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins closed this Jan 17, 2018
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
@danbev @MylesBorins
src: add --use-bundled-ca --use-openssl-ca check
0d08e03 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: #12083
Backport-PR-URL: #17783
PR-URL: #12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
@danbev @MylesBorins
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
64f0967 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: #12087
Backport-PR-URL: #17783
PR-URL: #12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins reopened this Jan 17, 2018
@MylesBorins
MylesBorins force-pushed the v6.x-staging branch from 88b6795 to 3c4bb3c Compare January 17, 2018 08:10
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
@danbev @MylesBorins
src: add --use-bundled-ca --use-openssl-ca check
dd2102d 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: #12083
Backport-PR-URL: #17783
PR-URL: #12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
@danbev @MylesBorins
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
c3d5fb3 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: #12087
Backport-PR-URL: #17783
PR-URL: #12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins closed this Jan 17, 2018
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
@danbev @MylesBorins
src: add --use-bundled-ca --use-openssl-ca check
9ab8467 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: #12083
Backport-PR-URL: #17783
PR-URL: #12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
@danbev @MylesBorins
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
1b38147 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: #12087
Backport-PR-URL: #17783
PR-URL: #12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
@danbev @MylesBorins
src: add --use-bundled-ca --use-openssl-ca check
6d291ef 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: #12083
Backport-PR-URL: #17783
PR-URL: #12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
@danbev @MylesBorins
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
0e0a783 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: #12087
Backport-PR-URL: #17783
PR-URL: #12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
@danbev @MylesBorins
src: add --use-bundled-ca --use-openssl-ca check
758dc81 
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: #12083
Backport-PR-URL: #17783
PR-URL: #12087
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
@danbev @MylesBorins
src: guard bundled_ca/openssl_ca with HAVE_OPENSSL
2b29cea 
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: #12087
Backport-PR-URL: #17783
PR-URL: #12302
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@danbev
danbev deleted the backport-12302-v6.x branch March 27, 2018 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@MylesBorins MylesBorins

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@danbev @MylesBorins @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.