deps: rewrite openssl x86asm.pl win32 workaround #60510
Open
+3,127
−3,093
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some sed lines were previously used to change from C-style #ifdef to nasm-style %ifdef in x86asm.pl for 32-bit Windows builds, but this creates problems when the C preprocessor is used before the assembler to build x86 assembly files. OpenSSL is using C preprocessor before nasm and uses #ifdef in this context. The perl line added to update-openssl.sh will work around the ifdef issue in a way that enables building for win32 and other x86. After update-openssl.sh script is run with "regenerate", x86asm.pl will end up with a modified "endbranch" subroutine that allows to use 2 types of ifdef (nasm-style %ifdef for win32 and gcc-style #ifdef for others). Then, after x86asm.pl is run during the node openssl build process, x86 assembly files may change their ifdef and endif lines depending on the system they are built for.
The file deps/openssl/openssl/crypto/perlasm/x86asm.pl that is creating assembly code for x86, now creates nasm-style %ifdef and %endif for win32 and gcc-style #ifdef and #endif for all other x86. A large number of files in this commit are here after re-running the openssl update script: tools/dep_updaters/update-openssl.sh regenerate Assembler files for non-win32 x86 get a %ifdef to #ifdef modification, buildinf.h files only see a DATE change, configdata.pm files only see a RANLIB change.
Using perl in not allowed from https://github.com/nodejs/node/blob/HEAD/doc/README.md#code-blocks although the ifdef syntax change always has been in perl. Until perl is enabled, use text.
|
Review requested:
|
nodejs-github-bot
added
dependencies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The win32 builds have been using a workaround for building the openssl dependency. This breaks building x86 assembler files for unsupported architectures (32-bit linux-elf etc).
These git commits aim to solve the situation in a more inclusive way by adding an if statement to the perl script
x86asm.plthat is used to create the assembler files. This way, win32 builds still get assembler files with%ifdefsyntax, which is how Node has been building them so far. Official OpenSSL has#ifdefand not%ifdefinx86asm.pl.There are actually only 2 necessary modifications:
tools/dep_updaters/update-openssl.sh- a perl line to rewritex86asm.plendbranch subroutine, not only#ifdefto%ifdefdoc/contributing/maintaining/maintaining-openssl.mdabout how to changex86asm.plOther changes in this pull request happen automatically after running
tools/dep_updaters/update-openssl.sh regenerate:deps/openssl/openssl/crypto/perlasm/x86asm.pl- changed directly from the update script%ifdefto#ifdefmodificationbuildinf.hfiles only see aDATEchangeconfigdata.pmfiles only see aRANLIBchangeThe
.Sfiles that openssl creates are supposed to go through the C preprocessor before using the assembler, andcppdoesn't understand the%ifdefsyntax, it needs#ifdef. Although 32-bit Linux is no longer supported, people still build for x86 linux (and maybe other x86) and the workaround for building for linux-elf has been so far to revert the%ifdefsyntax back to original (openssl source)#ifdef.In any case, if this pull request gets rejected, after Node drops support for all win32 releases, there will be no need to keep the ifdef modification, and after that, node openssl dependency will be building again for linux x86.
References: