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

Revisit libxml 2.9.13 compatibility #2479

Closed
voxik opened this issue Mar 18, 2022 · 5 comments
Closed

Revisit libxml 2.9.13 compatibility #2479

voxik opened this issue Mar 18, 2022 · 5 comments

Comments

@voxik
Copy link
Contributor

voxik commented Mar 18, 2022

This is related to upstream discussion [1] which was triggered by my report in Fedora [2] (and of course also related to #2461). The issue is that on Fedora, we use system version of libxml for Nokogiri and we don't have the option to revert the behavior (unless reverting this for whole system which would probably not make a sense and would be against upstream) and if upstream insist that the new "breaking" behavior is the right one, I wonder what Nokogiri is going to do about it?

@voxik voxik added the state/needs-triage Inbox for non-installation-related bug reports or help requests label Mar 18, 2022
@voxik
Copy link
Contributor Author

voxik commented Mar 18, 2022

Actually now I noticed the upstream ticket was opened much earlier, my bad 😊 Nevertheless, the point stays.

@voxik
Copy link
Contributor Author

voxik commented Mar 18, 2022

There is actually this proposed patch in libxml2:

https://gitlab.gnome.org/GNOME/libxml2/-/commit/4fd69f3e27e4ef2f8fafa091e723497017c40646

While this patch fixes ActionView test failures, it keeps the rails-html-sanitizer test suite broken, but differently

@voxik
Copy link
Contributor Author

voxik commented Mar 18, 2022

This is example of the rails-html-sanitizer failure with the patch above:

Failure:
SanitizersTest#test_strip_unclosed_cdata [/builddir/build/BUILD/rails-html-sanitizer-1.4.2/usr/share/gems/gems/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:86]:
--- expected
+++ actual
@@ -1,3 +1,3 @@
 # encoding: UTF-8
 #    valid: true
-"This has an unclosed ]] here..."
+"This has an unclosed <![CDATA[]] here..."

@flavorjones
Copy link
Member

flavorjones commented Mar 18, 2022

I believe this is a duplicate of #2468. If you agree, we can close this issue.

Please note that Nokogiri's test suite runs against a vanilla ubuntu image running the distro's libxml2 and libxslt. If you'd ilke to add coverage for a redhat/fedora system I would accept a PR.

Please also note that we run CI against upstream libxml2 and libxslt master, and the failing tests are linked-to in #2468.

If there are other things you think we should be doing to test and maintain compatibility with downstream distros, please feel free to make suggestions, because right now I think I'm providing adequate support and attention.

@flavorjones flavorjones added state/will-close meta/duplicate and removed state/needs-triage Inbox for non-installation-related bug reports or help requests labels Mar 18, 2022
@voxik
Copy link
Contributor Author

voxik commented Mar 18, 2022

Thx a lot. I'll follow the #2468.

I'll consider extending the CI!

@voxik voxik closed this as completed Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants