The Wayback Machine - https://web.archive.org/web/20230204001323/https://github.com/nodejs/node/pull/45387
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

Jump to bottom

deps: update zlib to upstream 8bbd6c31 #45387

Closed
wants to merge 2 commits into from
Closed

deps: update zlib to upstream 8bbd6c31 #45387

wants to merge 2 commits into from

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Nov 9, 2022 •
edited

Update zlib to upstream 8bbd6c31.

Supersedes #44412.
@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Nov 9, 2022
@lpinca lpinca mentioned this pull request Nov 9, 2022
Closed
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2022
@lpinca
Copy link
Member Author

lpinca commented Nov 9, 2022

Benchmark CI: https://ci.nodejs.org/job/benchmark-node-micro-benchmarks/1215/
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
RafaelGSS
RafaelGSS approved these changes Nov 9, 2022
View changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
@Neustradamus
Copy link

To follow
@github-actions github-actions bot mentioned this pull request Nov 10, 2022
Open
20 tasks
@mscdex
Copy link
Contributor

mscdex commented Nov 10, 2022

FWIW it looks like there is no difference in results after re-running the deflate benchmarks.
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@lpinca
Copy link
Member Author

lpinca commented Nov 10, 2022 •
edited

Build on Windows ARM fails, but I see no specific error.
@RaisinTen
Copy link
Member

If you view the logs as plain text and search for the last occurrence of error: you'll see:

C:\workspace\node-compile-windows\node\deps\zlib\cpu_features.c(55,1): fatal error C1189: #error:  cpu_features.c CPU feature detection in not defined for your platform [C:\workspace\node-compile-windows\node\deps\zlib\zlib.vcxproj]
@lpinca
Copy link
Member Author

lpinca commented Nov 10, 2022

@RaisinTen thanks. I'll take a look. We might need to patch the gyp file as suggested here #44412 (comment).
@lpinca lpinca force-pushed the update/zlib branch 2 times, most recently from 5fe09af to 02dfc58 Compare November 10, 2022 09:21
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@RafaelGSS RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 16, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 16, 2022
@nodejs-github-bot
Copy link
Contributor

Landed in 8f9dc27...26991f7
nodejs-github-bot pushed a commit that referenced this pull request Nov 16, 2022
@lpinca @nodejs-github-bot
deps: update zlib to upstream 8bbd6c31
b6c108f 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 16, 2022
@addaleax @nodejs-github-bot
deps: fix zlib compilation for CPUs without SIMD features
26991f7 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@lpinca lpinca deleted the update/zlib branch November 16, 2022 19:59
@lpinca lpinca mentioned this pull request Nov 16, 2022
Open
@github-actions github-actions bot mentioned this pull request Nov 17, 2022
Open
13 tasks
This was referenced Nov 17, 2022
Closed
Closed
Closed
Closed
@github-actions github-actions bot mentioned this pull request Nov 18, 2022
Open
12 tasks
This was referenced Nov 21, 2022
Closed
Closed
ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
@lpinca @ruyadorno
deps: update zlib to upstream 8bbd6c31
c41e67f 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
@addaleax @ruyadorno
deps: fix zlib compilation for CPUs without SIMD features
45ba14b 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@ruyadorno ruyadorno mentioned this pull request Nov 24, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@lpinca @danielleadams
deps: update zlib to upstream 8bbd6c31
71e8723 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@addaleax @danielleadams
deps: fix zlib compilation for CPUs without SIMD features
93b9cc4 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@lpinca @danielleadams
deps: update zlib to upstream 8bbd6c31
17893de 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@addaleax @danielleadams
deps: fix zlib compilation for CPUs without SIMD features
bbd60e2 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@lpinca @danielleadams
deps: update zlib to upstream 8bbd6c31
978cfad 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@addaleax @danielleadams
deps: fix zlib compilation for CPUs without SIMD features
43419ad 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@lpinca @danielleadams
deps: update zlib to upstream 8bbd6c31
f2f853f 
Updated as described in doc/contributing/maintaining-zlib.md.

PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@addaleax @danielleadams
deps: fix zlib compilation for CPUs without SIMD features
5b52e92 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: #32553

PR-URL: #32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45387
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.
7 participants
@lpinca @nodejs-github-bot @Neustradamus @mscdex @RaisinTen @RafaelGSS @addaleax