-
Notifications
You must be signed in to change notification settings - Fork 30k
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
deps: add template for generated headers #42616
Conversation
47aaa16
to
a0b44c6
Compare
With these changes the size of $ 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 |
a0b44c6
to
b9babe6
Compare
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). |
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. |
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. |
@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. |
With the latest changes the sizes of the sub directories in $ 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 |
I'm wondering if we can get rid of some of these. e.g. |
I'm also not sure what |
I don't have the answer to that question, but I'd be all for removing them if they are no longer needed 👍 |
@miladfarca can you check with Vascili on which of we need? 1.5M aix64-gcc-as |
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. |
I've forwarded the link to Vasili to confirm. |
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: #42616 Fixes: #42081 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
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: #42616 Fixes: #42081 Refs: openssl/openssl@178fa72ed5 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
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: #42616 Fixes: #42081 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
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: #42616 Fixes: #42081 Refs: openssl/openssl@178fa72ed5 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
commit 7fae2c9 breaks C++ addons which use any
|
This is a dep of #42356, so, this can not be landed in v16.x |
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