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

Integrate nokogiri-xmlsec into nokogiri #2888

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maths22
Copy link

@maths22 maths22 commented May 23, 2023

What problem is this PR intended to solve?

#2746 (comment)

Have you included adequate test coverage?
Tests have been ported over from the separate library, and some that were skipped
there have been fixed to be passing.

Does this change affect the behavior of either the C or the Java implementations?
This adds new functionality to the C implementation. In the draft version it doesn't
touch Java. We will either need Java to throw useful notimplemented exceptions or
add the features of this library to the Java side, presumably using a third-party library.

Additional Note
Definitely needs some work on how the bundled xmlsec build works, and I think some #ifdefs to handle a range of xmlsec versions. Also seems to occasionally segfault running the test suite, so that definitely needs resolving.

Definitely needs some work on how the library build works, and
I think some #ifdefs to handle a range of xmlsec versions.  Also
seems to occasionally segfault running the test suite, so that
definitely needs resolving first.
@flavorjones
Copy link
Member

Repeating what I said in #2547: @maths22 I appreciate you making the attempt to get xmlsec to work in JRuby, but I think the best first change we can make regarding xmlsec is simply to import the C code and tests. I'd prefer to defer tackling the introduction of JRuby support.

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.

2 participants