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

[v6.x backport] src: guard bundled_ca/openssl_ca with HAVE_OPENSSL #17783

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Dec 20, 2017

This is a backport of the following PRs:

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

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]>
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
Copy link
Contributor Author

danbev commented Dec 20, 2017

@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
Copy link
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

@MylesBorins
Copy link
Contributor

Should potentially come with #12688 as well

@MylesBorins
Copy link
Contributor

landed in 5f6f460...64f0967

MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
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
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 pushed a commit that referenced this pull request Jan 17, 2018
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
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 11, 2018
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
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
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
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
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
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
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
Development

Successfully merging this pull request may close these issues.

3 participants