deps: update zlib to upstream 8bbd6c31 by lpinca · Pull Request #45387 · nodejs/node · GitHub
Skip to content

deps: update zlib to upstream 8bbd6c31 - #45387

Closed
lpinca wants to merge 2 commits into
nodejs:mainfrom
lpinca:update/zlib
Closed

deps: update zlib to upstream 8bbd6c31#45387
lpinca wants to merge 2 commits into
nodejs:mainfrom
lpinca:update/zlib

Conversation

@lpinca

@lpinca lpinca commented Nov 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Update zlib to upstream 8bbd6c31.

Supersedes #44412.

@nodejs-github-bot nodejs-github-bot added dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies. 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. Only starts once the PR has an approving review. label Nov 9, 2022
@lpinca

lpinca commented Nov 9, 2022

Copy link
Copy Markdown
Member Author

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. 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 reviewed changes

@RafaelGSS RafaelGSS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@Neustradamus

Neustradamus commented Nov 10, 2022

Copy link
Copy Markdown

To follow

@github-actions github-actions Bot mentioned this pull request Nov 10, 2022
Open
20 tasks
@mscdex

mscdex commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

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

lpinca commented Nov 10, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Build on Windows ARM fails, but I see no specific error.

@RaisinTen

RaisinTen commented Nov 10, 2022

Copy link
Copy Markdown
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

lpinca commented Nov 10, 2022

Copy link
Copy Markdown
Member Author

@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. Only starts once the PR has an approving review. label Nov 10, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. 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
@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 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>
@nappy

nappy commented Feb 20, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This (or maybe the other commit changing the gyp file) seems to break the Android arm64 build; zlib now references cpu-features.h on arm64 but this is not something provided by the platform it seems, but a module that needs to be compiled?!?

@Adenilson

Adenilson commented Mar 9, 2023

Copy link
Copy Markdown

@lpinca lpinca mentioned this pull request Mar 18, 2023
Closed
lpinca added a commit to lpinca/node that referenced this pull request Apr 4, 2023
@lpinca
deps: update zlib to upstream 5edb52d4
3e91e31 
Updated as described in doc/contributing/maintaining-zlib.md.

Refs: nodejs#45387 (comment)
lpinca added a commit that referenced this pull request Apr 6, 2023
@lpinca
deps: update zlib to upstream 5edb52d4
be2791b 
Updated as described in doc/contributing/maintaining-zlib.md.

Refs: #45387 (comment)
PR-URL: #47151
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
RafaelGSS pushed a commit that referenced this pull request Apr 13, 2023
@lpinca @RafaelGSS
deps: update zlib to upstream 5edb52d4
e7ca09f 
Updated as described in doc/contributing/maintaining-zlib.md.

Refs: #45387 (comment)
PR-URL: #47151
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@lpinca @danielleadams
deps: update zlib to upstream 5edb52d4
69a0050 
Updated as described in doc/contributing/maintaining-zlib.md.

Refs: #45387 (comment)
PR-URL: #47151
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
@lpinca @MoLow
deps: update zlib to upstream 5edb52d4
443a72e 
Updated as described in doc/contributing/maintaining-zlib.md.

Refs: nodejs#45387 (comment)
PR-URL: nodejs#47151
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@lpinca @nodejs-github-bot @Neustradamus @mscdex @RaisinTen @RafaelGSS @nappy @Adenilson @addaleax