Skip to content

crypto: fix build without scrypt#40613

Closed
shr-project wants to merge 1 commit intonodejs:masterfrom
shr-project:jansa/scrypt
Closed

crypto: fix build without scrypt#40613
shr-project wants to merge 1 commit intonodejs:masterfrom
shr-project:jansa/scrypt

Conversation

@shr-project
Copy link
Contributor

@shr-project shr-project commented Oct 26, 2021 •
edited
Loading

  • add missing semicolon to fix:
In file included from ../src/node_crypto.h:47,
                 from ../src/node.cc:46:
../src/crypto/crypto_scrypt.h:80:2: error: expected ';' after struct definition
   80 | }
      |  ^
      |  ;

and fix typo in the comment

Signed-off-by: Martin Jansa martin.jansa@lge.com

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Oct 26, 2021
@shr-project
crypto: fix build without scrypt
048203c 
* add missing semicolon to fix:
  In file included from ../src/node_crypto.h:47,
                   from ../src/node.cc:46:
  ../src/crypto/crypto_scrypt.h:80:2:
  error: expected ';' after struct definition
     80 | }
        |  ^
        |  ;

  and fix typo in the comment

Signed-off-by: Martin Jansa <martin.jansa@lge.com>
shr-project referenced this pull request Oct 26, 2021
@jasnell
crypto: refactoring internals, add WebCrypto
dae283d 
Fixes: #678
Refs: #26854

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #35093
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@VoltrexKeyva VoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@VoltrexKeyva VoltrexKeyva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Oct 27, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@github-actions github-actions bot mentioned this pull request Oct 28, 2021
Open
30 tasks
@richardlau richardlau mentioned this pull request Oct 28, 2021
Closed
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@github-actions github-actions bot mentioned this pull request Oct 29, 2021
Open
33 tasks
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@github-actions github-actions bot mentioned this pull request Oct 30, 2021
Open
35 tasks
@nodejs-github-bot

This comment has been minimized.

Sign in to view
This was referenced Oct 31, 2021
Open
Open
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 1, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/40635/

@github-actions github-actions bot mentioned this pull request Nov 2, 2021
Open
35 tasks
This was referenced Nov 3, 2021
Open
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 10, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/40816/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 10, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/40819/

@github-actions github-actions bot mentioned this pull request Nov 11, 2021
Open
31 tasks
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 11, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/40829/

@tniessen
Copy link
Member

tniessen commented Nov 11, 2021

@nodejs/node-core-utils git node land incorrectly picks up a Fixes line here:

----------------------------------- PR info ------------------------------------
Title      crypto: fix build without scrypt (#40613)
Author     Martin 'JaMa' Jansa <Martin.Jansa@gmail.com> (@shr-project, first-time contributor)
Branch     shr-project:jansa/scrypt -> nodejs:master
Labels     crypto, c++, author ready
Commits    1
 - crypto: fix build without scrypt
Committers 1
 - Martin Jansa <martin.jansa@lge.com>
PR-URL: https://github.com/nodejs/node/pull/40613
Fixes: https://github.com/In
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40613
Fixes: https://github.com/In
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------

tniessen pushed a commit that referenced this pull request Nov 11, 2021
@shr-project @tniessen
crypto: fix build without scrypt
a9b2d85 
* add missing semicolon to fix:
  In file included from ../src/node_crypto.h:47,
                   from ../src/node.cc:46:
  ../src/crypto/crypto_scrypt.h:80:2:
  error: expected ';' after struct definition
     80 | }
        |  ^
        |  ;

  and fix typo in the comment

Signed-off-by: Martin Jansa <martin.jansa@lge.com>

PR-URL: #40613
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@tniessen
Copy link
Member

tniessen commented Nov 11, 2021

Landed in a9b2d85, thank you for the contribution, @shr-project!

@tniessen tniessen closed this Nov 11, 2021
@tniessen tniessen mentioned this pull request Nov 11, 2021
Open
@Trott
Copy link
Member

Trott commented Nov 11, 2021

@nodejs/node-core-utils git node land incorrectly picks up a Fixes line here:

----------------------------------- PR info ------------------------------------
Title      crypto: fix build without scrypt (#40613)
Author     Martin 'JaMa' Jansa <Martin.Jansa@gmail.com> (@shr-project, first-time contributor)
Branch     shr-project:jansa/scrypt -> nodejs:master
Labels     crypto, c++, author ready
Commits    1
 - crypto: fix build without scrypt
Committers 1
 - Martin Jansa <martin.jansa@lge.com>
PR-URL: https://github.com/nodejs/node/pull/40613
Fixes: https://github.com/In
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40613
Fixes: https://github.com/In
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------

It's probably because fix: shows up in the commit message. If so, the regex or whatever could certainly be made more strict.

This was referenced Nov 12, 2021
Open
Open
Open
Open
Open
targos pushed a commit that referenced this pull request Nov 21, 2021
@shr-project @targos
crypto: fix build without scrypt
7f5931d 
* add missing semicolon to fix:
  In file included from ../src/node_crypto.h:47,
                   from ../src/node.cc:46:
  ../src/crypto/crypto_scrypt.h:80:2:
  error: expected ';' after struct definition
     80 | }
        |  ^
        |  ;

  and fix typo in the comment

Signed-off-by: Martin Jansa <martin.jansa@lge.com>

PR-URL: #40613
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@targos targos mentioned this pull request Nov 26, 2021
Merged
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
@shr-project @danielleadams
crypto: fix build without scrypt
fc9868c 
* add missing semicolon to fix:
  In file included from ../src/node_crypto.h:47,
                   from ../src/node.cc:46:
  ../src/crypto/crypto_scrypt.h:80:2:
  error: expected ';' after struct definition
     80 | }
        |  ^
        |  ;

  and fix typo in the comment

Signed-off-by: Martin Jansa <martin.jansa@lge.com>

PR-URL: #40613
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
@shr-project @danielleadams
crypto: fix build without scrypt
00fd7d2 
* add missing semicolon to fix:
  In file included from ../src/node_crypto.h:47,
                   from ../src/node.cc:46:
  ../src/crypto/crypto_scrypt.h:80:2:
  error: expected ';' after struct definition
     80 | }
        |  ^
        |  ;

  and fix typo in the comment

Signed-off-by: Martin Jansa <martin.jansa@lge.com>

PR-URL: #40613
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@tniessen tniessen tniessen approved these changes

@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes

+1 more reviewer

@ejcheng ejcheng ejcheng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@shr-project @nodejs-github-bot @tniessen @Trott @addaleax @VoltrexKeyva @ejcheng