Skip to content

Commit

Permalink
deps: delete deps/v8/third_party/zlib
Browse files Browse the repository at this point in the history
We currently have two copies of Chromium's zlib: one in deps/zlib and
another in deps/v8/third_party/zlib. This has a couple of disadvantages:

1. There is an additional cost to keeping both dependencies up-to-date,
   and in fact they were already out-of-sync (see the refs).

2. People who compile with --shared-zlib (i.e. distro maintainers) will
   probably not be thrilled to learn that there is still a copy of zlib
   inside.

3. It's aesthetically unpleasing.

This diff (per discussion in the refs) centralizes on deps/zlib and
deletes deps/v8/third_party/zlib. That requires changing the include
headers in two files in deps/v8/src, which was pretty straightforward.

When the user requests compiling with a shared zlib build, we still need
to compile the contents of deps/zlib/google. This is not ideal but it's
also not a change in behavior: prior to this diff those files were
being compiled in the deps/v8/third_party/zlib version.

I tested this on Linux with the default build and a --shared-zlib build.
I checked that the shared-zlib build dynamically linked zlib according
to ldd, and that the regular build did not. I would appreciate if the
reviewers could suggest some other build configurations to try.

Refs: nodejs#47145
Refs: nodejs#47157
  • Loading branch information
kvakil committed Apr 10, 2023
1 parent 4b80a7b commit f4637a7
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 70 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.3',
'v8_embedder_string': '-node.4',

##### V8 defaults for Node.js #####

Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/deoptimizer/translation-array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "src/objects/fixed-array-inl.h"

#ifdef V8_USE_ZLIB
#include "third_party/zlib/google/compression_utils_portable.h"
#include "compression_utils_portable.h"
#endif // V8_USE_ZLIB

namespace v8 {
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/snapshot/snapshot-utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "src/base/sanitizer/msan.h"

#ifdef V8_USE_ZLIB
#include "third_party/zlib/zlib.h"
#include "zlib.h"
#endif

namespace v8 {
Expand Down
22 changes: 19 additions & 3 deletions deps/zlib/zlib.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,20 @@
{
'variables': {
'ZLIB_ROOT': '.',
'use_system_zlib%': 0,
'arm_fpu%': '',
'llvm_version%': '0.0',
},
'targets': [
{
'target_name': 'zlib_google',
'type': 'static_library',
'sources': [
'<!@pymod_do_main(GN-scraper "<(ZLIB_ROOT)/google/BUILD.gn" "\\"compression_utils_portable\\".*?sources = ")',
],
},
],
'conditions': [
['use_system_zlib==0', {
['node_shared_zlib=="false"', {
'targets': [
{
'target_name': 'zlib_adler32_simd',
Expand Down Expand Up @@ -174,12 +182,16 @@
{
'target_name': 'zlib',
'type': 'static_library',
'dependencies': [ 'zlib_google' ],
'sources': [
'<!@pymod_do_main(GN-scraper "<(ZLIB_ROOT)/BUILD.gn" "\\"zlib\\".*?sources = ")',
],
'include_dirs': [ '<(ZLIB_ROOT)' ],
'direct_dependent_settings': {
'include_dirs': [ '<(ZLIB_ROOT)' ],
'include_dirs': [
'<(ZLIB_ROOT)',
'<(ZLIB_ROOT)/google',
],
},
'conditions': [
['OS!="win"', {
Expand Down Expand Up @@ -248,10 +260,14 @@
{
'target_name': 'zlib',
'type': 'static_library',
'dependencies': [ 'zlib_google' ],
'direct_dependent_settings': {
'defines': [
'USE_SYSTEM_ZLIB',
],
'include_dirs': [
'<(ZLIB_ROOT)/google',
],
},
'defines': [
'USE_SYSTEM_ZLIB',
Expand Down
65 changes: 1 addition & 64 deletions tools/v8_gypfiles/v8.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@
'run_torque',
'v8_internal_headers',
'v8_maybe_icu',
'v8_zlib',
'../../deps/zlib/zlib.gyp:zlib',
],
'includes': ['inspector.gypi'],
'direct_dependent_settings': {
Expand Down Expand Up @@ -2022,68 +2022,5 @@
'sources': ['<(SHARED_INTERMEDIATE_DIR)/debug-support.cc', ],
},
}, # postmortem-metadata

{
'target_name': 'v8_zlib',
'type': 'static_library',
'toolsets': ['host', 'target'],
'conditions': [
['OS=="win"', {
'conditions': [
['"<(target_arch)"=="arm64" and _toolset=="target"', {
'defines': ['CPU_NO_SIMD']
}, {
'defines': ['X86_WINDOWS']
}]
]
}],
],
'direct_dependent_settings': {
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
],
},
'defines': [ 'ZLIB_IMPLEMENTATION' ],
'include_dirs': [
'<(V8_ROOT)/third_party/zlib',
'<(V8_ROOT)/third_party/zlib/google',
],
'sources': [
'<(V8_ROOT)/third_party/zlib/adler32.c',
'<(V8_ROOT)/third_party/zlib/chromeconf.h',
'<(V8_ROOT)/third_party/zlib/compress.c',
'<(V8_ROOT)/third_party/zlib/contrib/optimizations/insert_string.h',
'<(V8_ROOT)/third_party/zlib/contrib/optimizations/insert_string.h',
'<(V8_ROOT)/third_party/zlib/cpu_features.c',
'<(V8_ROOT)/third_party/zlib/cpu_features.h',
'<(V8_ROOT)/third_party/zlib/crc32.c',
'<(V8_ROOT)/third_party/zlib/crc32.h',
'<(V8_ROOT)/third_party/zlib/deflate.c',
'<(V8_ROOT)/third_party/zlib/deflate.h',
'<(V8_ROOT)/third_party/zlib/gzclose.c',
'<(V8_ROOT)/third_party/zlib/gzguts.h',
'<(V8_ROOT)/third_party/zlib/gzlib.c',
'<(V8_ROOT)/third_party/zlib/gzread.c',
'<(V8_ROOT)/third_party/zlib/gzwrite.c',
'<(V8_ROOT)/third_party/zlib/infback.c',
'<(V8_ROOT)/third_party/zlib/inffast.c',
'<(V8_ROOT)/third_party/zlib/inffast.h',
'<(V8_ROOT)/third_party/zlib/inffixed.h',
'<(V8_ROOT)/third_party/zlib/inflate.c',
'<(V8_ROOT)/third_party/zlib/inflate.h',
'<(V8_ROOT)/third_party/zlib/inftrees.c',
'<(V8_ROOT)/third_party/zlib/inftrees.h',
'<(V8_ROOT)/third_party/zlib/trees.c',
'<(V8_ROOT)/third_party/zlib/trees.h',
'<(V8_ROOT)/third_party/zlib/uncompr.c',
'<(V8_ROOT)/third_party/zlib/zconf.h',
'<(V8_ROOT)/third_party/zlib/zlib.h',
'<(V8_ROOT)/third_party/zlib/zutil.c',
'<(V8_ROOT)/third_party/zlib/zutil.h',
'<(V8_ROOT)/third_party/zlib/google/compression_utils_portable.cc',
'<(V8_ROOT)/third_party/zlib/google/compression_utils_portable.h',
],
}, # v8_zlib
],
}

0 comments on commit f4637a7

Please sign in to comment.