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

dep: libxml 2.12.0 upgrade (with libxml2 patch for weakref) #3032

Merged
merged 6 commits into from
Nov 18, 2023

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented Nov 18, 2023

What problem is this PR intended to solve?

#3031

This approach uses the patches submitted upstream at:

and turns off libxml2's thread-local storage feature, which is an issue when precompiling for musl on glibc systems. If and when we can build a separate musl native gem, we can revisit this decision.

@flavorjones flavorjones force-pushed the fj-dep-libxml-2.12.0_patch-weakref branch 2 times, most recently from 7f4a743 to 9c9c99c Compare November 18, 2023 20:11
@flavorjones
Copy link
Member Author

CI didn't kick off, so the manual run is https://github.com/sparklemotion/nokogiri/actions/runs/6916120729

@flavorjones
Copy link
Member Author

OK, so even with the patches applied, we're still pulling in ld-linux-x86-64.so.2 because the new thread local storage feature ("TLS") results in a reference to __tls_get_addr that is non-weak. I am assuming something similar is happening on windows with libwinpthread.

You can read more about TLS at https://maskray.me/blog/2021-02-14-all-about-thread-local-storage

Turning off TLS by configuring libxml2 with --without-tls avoids this problem. We'll try that next.

Using thread-local variables in position-independent code (i.e.,
combining `__thread` variables with the `-fPIC` compiler option)
introduces a (non-weak) reference to `__tls_get_addr` which resolves
to `ld-linux` on glibc systems and `libwinpthread` on windows systems.

This is an issue when precompiling for musl on glibc systems. If and
when we can build a separate musl native gem, we can revisit this
decision.
@flavorjones flavorjones force-pushed the fj-dep-libxml-2.12.0_patch-weakref branch from 9c9c99c to 004e6a6 Compare November 18, 2023 21:20
@flavorjones
Copy link
Member Author

This seems like the winner.

@flavorjones flavorjones merged commit c7dca6a into main Nov 18, 2023
138 checks passed
@flavorjones flavorjones deleted the fj-dep-libxml-2.12.0_patch-weakref branch November 18, 2023 21:56
tagliala added a commit to tagliala/premailer that referenced this pull request Jan 18, 2024
There have been two recent changes in libxml2 2.11 and than 2.12 which
broke two specs related to the conversion of UTF-8 chars.

This commit sets the minimum required Nokogiri version to 1.16 which
bundles libxml2 2.12, in order to have a predictable behaviour.

References:
- premailer#430
- sparklemotion/nokogiri#2965
- sparklemotion/nokogiri#3032
- GNOME/libxml2@f1c1f5c
- GNOME/libxml2@37cedc0

Close premailer#443
tagliala added a commit to tagliala/premailer that referenced this pull request Jan 18, 2024
There have been two recent changes in libxml2 2.11 and than 2.12 which
broke two specs related to the conversion of UTF-8 chars.

This commit sets the minimum required Nokogiri version to 1.16 which
bundles libxml2 2.12, in order to have a predictable behaviour.

References:
- premailer#430
- sparklemotion/nokogiri#2965
- sparklemotion/nokogiri#3032
- GNOME/libxml2@f1c1f5c
- GNOME/libxml2@37cedc0

Close premailer#443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant