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

build: check without_ssl in warn openssl_no_asm #19934

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 11, 2018

Currently when configuring --without-ssl the following warning is
displayed:

WARNING: openssl_no_asm is enabled due to missed or old assembler.
            Please refer BUILDING.md

This commit adds a check of options.without_ssl to avoid this warning
when --without-ssl is used.

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

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Apr 11, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 11, 2018

@mscdex
Copy link
Contributor

mscdex commented Apr 11, 2018

s/witout_ssl/without_ssl/ in commit message

Currently when configuring --without-ssl  the following warning is
displayed:
WARNING: openssl_no_asm is enabled due to missed or old assembler.
            Please refer BUILDING.md

This commit adds a check of options.without_ssl to avoid this warning
when --without-ssl is used.
@danbev
Copy link
Contributor Author

danbev commented Apr 11, 2018

node-test-binary-windows failure looks unrelated

console output:

not ok 538 sequential/test-inspector-async-hook-setup-at-inspect-brk
  ---
  duration_ms: 0.498
  severity: fail
  stack: |-
    [test] Connecting to a child Node process
    [test] Testing /json/list
    [err] Debugger listening on ws://127.0.0.1:54669/e41bbf4a-fd83-4c28-9d77-a811ecc70db6
    [err] For help, see: https://nodejs.org/en/docs/inspector
    [err] 
    [test] Verify node waits for the frontend to disconnect
    [err] Debugger attached.
    [err] Waiting for the debugger to disconnect...
    [err] 
    [test] Verify basic properties of asyncStackTrace
    { AssertionError [ERR_ASSERTION]: 55 strictEqual 3221225477
        at runTests (c:\workspace\node-test-binary-windows\test\sequential\test-inspector-async-hook-setup-at-inspect-brk.js:48:10)
        at process._tickCallback (internal/process/next_tick.js:178:7)
      generatedMessage: true,
      name: 'AssertionError [ERR_ASSERTION]',
      code: 'ERR_ASSERTION',
      actual: 55,
      expected: 3221225477,
      operator: 'strictEqual' }
    1
  ...

@danbev danbev force-pushed the openssl-no-asm-without-ssl branch from 144ef8a to 67bb43c Compare April 11, 2018 09:38
@danbev danbev changed the title build: check witout_ssl in warn openssl_no_asm build: check without_ssl in warn openssl_no_asm Apr 11, 2018
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 11, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 14, 2018

Landed in 89cd749.

@danbev danbev closed this Apr 14, 2018
@danbev danbev deleted the openssl-no-asm-without-ssl branch April 14, 2018 11:52
danbev added a commit that referenced this pull request Apr 14, 2018
Currently when configuring --without-ssl  the following warning is
displayed:
WARNING: openssl_no_asm is enabled due to missed or old assembler.
            Please refer BUILDING.md

This commit adds a check of options.without_ssl to avoid this warning
when --without-ssl is used.

PR-URL: #19934
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
jasnell pushed a commit that referenced this pull request Apr 16, 2018
Currently when configuring --without-ssl  the following warning is
displayed:
WARNING: openssl_no_asm is enabled due to missed or old assembler.
            Please refer BUILDING.md

This commit adds a check of options.without_ssl to avoid this warning
when --without-ssl is used.

PR-URL: #19934
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@rvagg rvagg added v10.x and removed v10.x labels Apr 20, 2018
@BridgeAR
Copy link
Member

BridgeAR commented May 1, 2018

This seems to be based on a semver-major commit. I added the do not land labels accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants