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

nodejs16/17: link against system zlib #12996

Merged
merged 2 commits into from Nov 24, 2021
Merged

Conversation

Tatsh
Copy link
Contributor

@Tatsh Tatsh commented Nov 16, 2021

Description

Fixes https://trac.macports.org/ticket/63774

My guess is that some code from Node's variant of zlib is getting optimised incorrectly with the new Apple Clang version, but I did not look into it much. Normally we link against system libs anyway, and this fixes the issue we are having.

Also there are more opportunties for system lib linking:

  • brotli
  • c-ares
  • http-parser
  • libuv
  • nghttp2
  • nghttp3 (no port yet)
  • ngtcp2 (no port yet)

Only bumping these two versions as these are the versions supported around the time of Monterey's release and only Monterey is affected by the zlib static code issue.

  • nodejs16: link against system zlib
  • nodejs17: link against system zlib

Revbump is to force a rebuild for those affected (and unfortunately those who aren't, but I have never seen revision used conditionally).

I do not know how far back the patch should go if we decide to patch nodejs15 and before that. However, linking against system libs is still seen as an improvement.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 12.0.1 21A559 x86_64
Xcode 13.1 13A1030d

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

Fixes #63774
Revbumping because the prior version would build successfully, but the
statically linked zlib code never worked on Monterey.
Revbumping because the prior version would build successfully, but the
statically linked zlib code never worked on Monterey.
@macportsbot
Copy link

Notifying maintainers:
@ci42 for port nodejs16, nodejs17.

@macportsbot macportsbot added maintainer: open Affects an openmaintainer port type: bugfix labels Nov 16, 2021
@mascguy mascguy self-assigned this Nov 24, 2021
@mascguy
Copy link
Member

mascguy commented Nov 24, 2021

Folks have had eight days to provide feedback, so merging.

@mascguy mascguy merged commit 5a77f0d into macports:master Nov 24, 2021
@Tatsh Tatsh deleted the nodejs-63774 branch November 25, 2021 03:31
rhapsodyv added a commit to rhapsodyv/macports-ports that referenced this pull request Feb 22, 2022
Fixes zlib issue with nodejs 12 and 14, linking against system zlib.
Same of https://trac.macports.org/ticket/63774 and macports#12996, but for nodejs 12 and 14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
4 participants