Skip to content
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

maintaining-zlib.md is out of sync with chromium upstream #32856

Closed
sam-github opened this issue Apr 14, 2020 · 4 comments
Closed

maintaining-zlib.md is out of sync with chromium upstream #32856

sam-github opened this issue Apr 14, 2020 · 4 comments
Labels
zlib Issues and PRs related to the zlib subsystem.

Comments

@sam-github
Copy link
Contributor

I just ran through https://github.com/nodejs/node/blob/master/doc/guides/maintaining-zlib.md#updating-zlib, and it didn't produce a buildable zlib, see below.

This isn't an immediate problem, but if we have to update zlib for any reason (sec release, performance, portability), whoever is doing that will find the docs not quite sufficient.

If anyone wants to figure out what it would take to fix the docs, it'd be good to do it sooner than later.

Also, it might be useful to extend the docs to see how to determine what upstream SHA has actually been released in a chromium, so we don't grab pre-release/untested code.

cc: @mhdawson @richardlau @MylesBorins (to whom I mentioned that I don't think the guide is still in working order).

[19/1004] CC obj/deps/zlib/zlib.adler32_simd.o
FAILED: obj/deps/zlib/zlib.adler32_simd.o 
clang-11 -MMD -MF obj/deps/zlib/zlib.adler32_simd.o.d -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -D__STDC_FORMAT_MACROS -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS -DHAVE_HIDDE
N -DADLER32_SIMD_SSSE3 -DINFLATE_CHUNK_SIMD_SSE2 -DCRC32_SIMD_SSE42_PCLMUL -DINFLATE_CHUNK_READ_64LE -I../../deps/zlib -pthread -Wall -Wextra -Wno-unused-parameter -m64 -O3 -fno-omit-frame-po
inter   -c ../../deps/zlib/adler32_simd.c -o obj/deps/zlib/zlib.adler32_simd.o
../../deps/zlib/adler32_simd.c:116:34: error: always_inline function '_mm_maddubs_epi16' requires target feature 'ssse3', but would be inlined into function 'adler32_simd_' that is compiled w
ithout support for 'ssse3'
            const __m128i mad1 = _mm_maddubs_epi16(bytes1, tap1);
                                 ^
../../deps/zlib/adler32_simd.c:120:34: error: always_inline function '_mm_maddubs_epi16' requires target feature 'ssse3', but would be inlined into function 'adler32_simd_' that is compiled w
ithout support for 'ssse3'
            const __m128i mad2 = _mm_maddubs_epi16(bytes2, tap2);
                                 ^
2 errors generated.
[24/1003] CC obj/tools/icu/gen/icudata.icudt66_dat.o
@addaleax addaleax added the zlib Issues and PRs related to the zlib subsystem. label Apr 14, 2020
@addaleax
Copy link
Member

Err, yeah, this is my fault I guess. I assume that dccdc51 would need to be re-floated on top of zlib after updating, at least for the time being until somebody has the time to address that issue using gyp modifications only?

@sam-github
Copy link
Contributor Author

Maybe all that's needed is a note to cherry-pick that back? I just tried to check, but while it picks clean, v8 is rebuilding. That will take a while.

nodejs-github-bot pushed a commit that referenced this issue Dec 4, 2022
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@Neustradamus
Copy link

To follow

@cjihrig
Copy link
Contributor

cjihrig commented Dec 4, 2022

@Neustradamus if you are interested in following an issue, please use the "Subscribe" button in the GitHub UI instead of spamming everyone with comments. Just search for "subscribe" on the page to find the button.

targos pushed a commit that referenced this issue Dec 12, 2022
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
targos pushed a commit that referenced this issue Dec 12, 2022
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
This reverts commit 26991f7.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
Restructure the zlib.gyp file based on the upstream gn file, breaking
out the files with optimizations that need additional compiler flags.

Use a copy of the GN-scraper.py script to reduce the amount of hand
editing when the zlib dependency is updated.

PR-URL: #45589
Fixes: #32856
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@sam-github @Neustradamus @addaleax @cjihrig and others