deps: add template for generated headers#42616

danbev · 2022-04-05T15:44:12Z

OpenSSL 3.0 has a number of header files that are generated, and
currently these headers are copied into the architecture specific
directories. This is done for each asm type, 'asm', 'asm_avx2', and
'no-asm' which has takes up quite a lot of disk space and also becomes
an issue with the headers.tar file which has increased due to this.

This commit adds copies the headers to a common directory for the
architecture, for example with linux-x86_64 there will be a directory
named deps/openssl/config/archs/linux-x86_64/common/include where the headers
will be copied (into subdirectories 'openssl' and 'crypto'. And in the
original locations a header file with the same name will be generated
which points (includes) the common header file.

Fixes: #42081

nodejs-github-bot · 2022-04-06T04:27:23Z

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

danbev · 2022-04-06T05:23:20Z

With these changes the size of include/node/openssl/archs is:

$ du -hs node-v18.0.0/include/node/openssl/archs/37M node-v18.0.0/include/node/openssl/archs/

I'm not sure if this is acceptable or not but at least it is an improvement current size which is around 61M.

nodejs-github-bot · 2022-04-06T15:22:45Z

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

mhdawson · 2022-04-06T21:02:29Z

I'm not sure if this is acceptable or not but at least it is an improvement current size which is around 61M.

Definitely a good improvement. I think we should compare to the pre-18.x size to judge whether this is a great first step or the complete fix.

richardlau · 2022-04-07T00:23:21Z

I'm not sure if this is acceptable or not but at least it is an improvement current size which is around 61M.
Definitely a good improvement. I think we should compare to the pre-18.x size to judge whether this is a great first step or the complete fix.

It's a step in the right direction but still much bigger than when we were using OpenSSL 1.1.1. For comparison the equivalent directories in the headers package for Node.js 16.14.0 is 2.7M (#42081).

danbev · 2022-04-07T09:09:52Z

For comparison the equivalent directories in the headers package for Node.js 16.14.0 is 2.7M (#42081).

I've added a comment to #42081 about the reason for the larger size in OpenSSL 3.x, compared to 1.1.1. While there might be more options to cut the size I don't think we will get it down to a size close to that of 1.1.1 as there are now more headers that are generated specifically for an architecture in 3.x than there were for 1.1.1. But I'll take another look and see if there is more that could be done.

danbev · 2022-04-08T03:21:45Z

I've been able to find one set of headers that are not being used anymore, and another set for providers which could be shared per architecture like is done in this PR. I'll make these changes and see what the size of the headers.tar is after that.

mhdawson · 2022-04-08T18:58:40Z

@danbev as an FYI some of the failures in the CI do look related to compiling openssl

danbev · 2022-04-09T06:22:38Z

@danbev as an FYI some of the failures in the CI do look related to compiling openssl

Thanks, I'll complete the changes I've got and then run through CI again on Monday.

nodejs-github-bot · 2022-04-11T04:42:19Z

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

nodejs-github-bot · 2022-04-11T11:40:21Z

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

nodejs-github-bot · 2022-04-11T15:05:32Z

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

danbev · 2022-04-12T07:03:20Z

With the latest changes the sizes of the sub directories in archs in the produced headers tar file looks like this:

$ ls | xargs du -sch 1.5M aix64-gcc-as 1.5M aix-gcc 1.5M BSD-x86 1.5M BSD-x86_64 1.5M darwin64-arm64-cc 1.5M darwin64-x86_64-cc 1.5M darwin-i386-cc 1.5M linux32-s390x 1.5M linux64-mips64 1.1M linux64-riscv64 1.5M linux64-s390x 1.5M linux-aarch64 1.5M linux-armv4 1.5M linux-elf 1.5M linux-ppc 1.5M linux-ppc64 1.5M linux-ppc64le 1.5M linux-x86_64 1.5M solaris64-x86_64-gcc 1.5M solaris-x86-gcc 2.8M VC-WIN32 2.8M VC-WIN64A 936K VC-WIN64-ARM 35M total

The windows variants are larger but they are also excluded from the headers handling that is part of this PR. We could look into them and see what can be done but if we assume that we can do something similar to what we have done to the others that would probably only get the total size down to something like 34M which I'm not really sure makes that much of a difference.

richardlau · 2022-04-12T11:59:06Z

With the latest changes the sizes of the sub directories in archs in the produced headers tar file looks like this:
$ ls | xargs du -sch 1.5M aix64-gcc-as 1.5M aix-gcc 1.5M BSD-x86 1.5M BSD-x86_64 1.5M darwin64-arm64-cc 1.5M darwin64-x86_64-cc 1.5M darwin-i386-cc 1.5M linux32-s390x 1.5M linux64-mips64 1.1M linux64-riscv64 1.5M linux64-s390x 1.5M linux-aarch64 1.5M linux-armv4 1.5M linux-elf 1.5M linux-ppc 1.5M linux-ppc64 1.5M linux-ppc64le 1.5M linux-x86_64 1.5M solaris64-x86_64-gcc 1.5M solaris-x86-gcc 2.8M VC-WIN32 2.8M VC-WIN64A 936K VC-WIN64-ARM 35M total 
The windows variants are larger but they are also excluded from the headers handling that is part of this PR. We could look into them and see what can be done but if we assume that we can do something similar to what we have done to the others that would probably only get the total size down to something like 34M which I'm not really sure makes that much of a difference.

I'm wondering if we can get rid of some of these. e.g. aix-gcc, darwin-i386-cc, linux32-s390x, linux-ppc, linux-ppc64, solaris-x86-gcc.

richardlau · 2022-04-12T12:13:37Z

I'm also not sure what linux-armv4 is.

danbev · 2022-04-12T14:02:18Z

I'm wondering if we can get rid of some of these. e.g. aix-gcc, darwin-i386-cc, linux32-s390x, linux-ppc, linux-ppc64, solaris-x86-gcc.

I don't have the answer to that question, but I'd be all for removing them if they are no longer needed 👍

mhdawson · 2022-04-12T18:13:42Z

@miladfarca can you check with Vascili on which of we need?

1.5M aix64-gcc-as
1.5M aix-gcc

mhdawson · 2022-04-12T18:15:26Z

I think we could get rid of linux-ppc and linux-ppc64 as we only support linux-ppc64le as long as that is what is used for linux-ppc64 builds.

miladfarca · 2022-04-12T19:23:55Z

@miladfarca can you check with Vascili on which of we need?
1.5M aix64-gcc-as 1.5M aix-gcc

I've forwarded the link to Vasili to confirm.
Assuming -maix64 is used during build with gcc (like how its done on the V8 side) then 64 bit headers might need to be used i.e aix64-gcc-as.

nodejs-github-bot · 2022-04-25T17:17:06Z

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

OpenSSL 3.0 has a number of header files that are generated, and currently these headers are copied into the architecture specific directories. This is done for each asm type, 'asm', 'asm_avx2', and 'no-asm' which has takes up quite a lot of disk space and also becomes an issue with the headers.tar file which has increased due to this. This commit adds copies the headers to a common directory for the architecture, for example with linux-x86_64 there will be a directory named deps/openssl/config/archs/linux-x86_64/common/include where the headers will be copied (into subdirectories 'openssl' and 'crypto'. And in the original locations a header file with the same name will be generated which points (includes) the common header file. PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

This arch was renamed to clarify that it used the aix assembler (as) and not the gnu assembler. It was removed from the Makefile and not being built but would still be picked up by make targets like the header-tar target. PR-URL: #42616Fixes: #42081 Refs: openssl/openssl@178fa72ed5 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

danbev · 2022-04-28T11:58:19Z

Landed in 33bc537, 4f1f14e, d8365ac, d68c4e9, 3f0b2a2, and 4ecfe3f

OpenSSL 3.0 has a number of header files that are generated, and currently these headers are copied into the architecture specific directories. This is done for each asm type, 'asm', 'asm_avx2', and 'no-asm' which has takes up quite a lot of disk space and also becomes an issue with the headers.tar file which has increased due to this. This commit adds copies the headers to a common directory for the architecture, for example with linux-x86_64 there will be a directory named deps/openssl/config/archs/linux-x86_64/common/include where the headers will be copied (into subdirectories 'openssl' and 'crypto'. And in the original locations a header file with the same name will be generated which points (includes) the common header file. PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

This arch was renamed to clarify that it used the aix assembler (as) and not the gnu assembler. It was removed from the Makefile and not being built but would still be picked up by make targets like the header-tar target. PR-URL: #42616Fixes: #42081 Refs: openssl/openssl@178fa72ed5 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

yanovich · 2022-05-05T14:56:30Z

commit 7fae2c9 breaks C++ addons which use any openssl header:

$ yarn build yarn run v1.22.18 $ node-gyp configure --silent && node-gyp build --silent make: Entering directory '/home/user/src/addon/build' CXX(target) Release/obj.target/addon/addon.o In file included from /home/s/.cache/node-gyp/18.1.0/include/node/openssl/opensslconf.h:9, from /home/s/.cache/node-gyp/18.1.0/include/node/openssl/macros.h:14, from /home/s/.cache/node-gyp/18.1.0/include/node/openssl/evp.h:14, from ../addon.cc:5: /home/s/.cache/node-gyp/18.1.0/include/node/openssl/./opensslconf_asm.h:97:11: fatal error: ./archs/linux-x86_64/asm/include/openssl/opensslconf.h: No such file or directory 97 | # include "./archs/linux-x86_64/asm/include/openssl/opensslconf.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make: *** [addon.target.mk:120: Release/obj.target/addon/addon.o] Error 1 $ head -n 5 addon.cc #include <stdlib.h> #include <string.h> #include <iostream> #include <openssl/evp.h>

juanarbol · 2022-05-30T06:15:56Z

This is a dep of #42356, so, this can not be landed in v16.x

nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. labels Apr 5, 2022

danbev force-pushed the openssl-3.0.0-header-files branch from 47aaa16 to a0b44c6Compare April 6, 2022 03:35

danbev force-pushed the openssl-3.0.0-header-files branch from a0b44c6 to b9babe6Compare April 6, 2022 14:27

github-actionsbot mentioned this pull request Apr 7, 2022
CI Reliability 2022-04-07 nodejs/reliability#244
Open
17 tasks

github-actionsbot mentioned this pull request Apr 8, 2022
CI Reliability 2022-04-08 nodejs/reliability#245
Open
22 tasks

github-actionsbot mentioned this pull request Apr 9, 2022
CI Reliability 2022-04-09 nodejs/reliability#246
Open
24 tasks

This was referenced Apr 10, 2022
CI Reliability 2022-04-10 nodejs/reliability#247
Open
CI Reliability 2022-04-11 nodejs/reliability#248
Open

github-actionsbot mentioned this pull request Apr 12, 2022
CI Reliability 2022-04-12 nodejs/reliability#249
Open
29 tasks

This was referenced Apr 26, 2022
CI Reliability 2022-04-26 nodejs/reliability#263
Open
CI Reliability 2022-04-27 nodejs/reliability#264
Open
CI Reliability 2022-04-28 nodejs/reliability#265
Open

richardlau approved these changes Apr 28, 2022
View reviewed changes

danbev added a commit that referenced this pull request Apr 28, 2022
deps: add note about removing asm archs
4f1f14e
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

danbev added a commit that referenced this pull request Apr 28, 2022
deps: remove aix-gcc architecture
d68c4e9
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

danbev added a commit that referenced this pull request Apr 28, 2022
deps: remove linux-ppc architecture
3f0b2a2
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

danbev added a commit that referenced this pull request Apr 28, 2022
deps: remove linux-ppc64 architecture
4ecfe3f
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

danbev closed this Apr 28, 2022

This was referenced Apr 29, 2022
CI Reliability 2022-04-29 nodejs/reliability#266
Open
CI Reliability 2022-04-30 nodejs/reliability#267
Open

targos pushed a commit that referenced this pull request May 2, 2022
deps: add note about removing asm archs
e8734a4
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

targos pushed a commit that referenced this pull request May 2, 2022
deps: remove aix-gcc architecture
8dc71f2
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

targos pushed a commit that referenced this pull request May 2, 2022
deps: remove linux-ppc architecture
4abe987
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

targos pushed a commit that referenced this pull request May 2, 2022
deps: remove linux-ppc64 architecture
2798929
PR-URL: #42616Fixes: #42081 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>

targos mentioned this pull request May 2, 2022
v18.1.0 release proposal #42943
Merged

yanovich mentioned this pull request May 5, 2022
deps: regression in openssl #42976
Closed

juanarbol added the dont-land-on-v16.x label May 30, 2022

juanarbol mentioned this pull request May 30, 2022
Revert "deps: add template for generated headers" #42978
Closed

richardlau mentioned this pull request Jun 28, 2022
deps: update OpenSSL 3.0.4+quic #43589
Closed

amyspark mentioned this pull request Sep 27, 2022
Regression in OpenSSL 3.0.2 vs. 1.1.1 on Android/NDK builds mesonbuild/wrapdb#427
Open

richardlau mentioned this pull request Jan 31, 2023
include/node/openssl/archs/ has ~50MB of (I hope?) unused headers #46451
Open

Uh oh!

deps: add template for generated headers#42616

deps: add template for generated headers #42616

Uh oh!

Conversation

danbev commented Apr 5, 2022

Uh oh!

nodejs-github-bot commented Apr 6, 2022

Uh oh!

danbev commented Apr 6, 2022

Uh oh!

nodejs-github-bot commented Apr 6, 2022

Uh oh!

mhdawson commented Apr 6, 2022

Uh oh!

richardlau commented Apr 7, 2022

Uh oh!

danbev commented Apr 7, 2022

Uh oh!

danbev commented Apr 8, 2022

Uh oh!

mhdawson commented Apr 8, 2022

Uh oh!

danbev commented Apr 9, 2022

Uh oh!

nodejs-github-bot commented Apr 11, 2022

Uh oh!

nodejs-github-bot commented Apr 11, 2022

Uh oh!

nodejs-github-bot commented Apr 11, 2022

Uh oh!

danbev commented Apr 12, 2022

Uh oh!

richardlau commented Apr 12, 2022

Uh oh!

richardlau commented Apr 12, 2022

Uh oh!

danbev commented Apr 12, 2022

Uh oh!

mhdawson commented Apr 12, 2022

Uh oh!

mhdawson commented Apr 12, 2022

Uh oh!

miladfarca commented Apr 12, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 25, 2022

Uh oh!

danbev commented Apr 28, 2022

Uh oh!

yanovich commented May 5, 2022

Uh oh!

juanarbol commented May 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

miladfarca commented Apr 12, 2022•
edited
Loading