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

Implement xpath error message handling #135

Closed
wants to merge 1 commit into from
Closed

Implement xpath error message handling #135

wants to merge 1 commit into from

Conversation

hurricane-socrates
Copy link

See #133 for context.

Two issues so far:

  1. The first test isn't correct yet. I wanted to start this PR.
  2. After each source change I have to redefine LIBXML2. I'm not sure how to tell Cargo where the correct libxml2 resides. I think it's finding the Apple version, not the brew version. There's also a MacPorts version that might be in the search path.

@anwaralameddin
Copy link
Contributor

On macOS, pkg-config-0.3.2 is used in the build script to locate libxml2 when LIBXML2 is not set. Setting the atleast_version and replacing the function find_library with the method find might help, but I have not tested it. For local tests, it might be simpler to reorder the entries in the library search paths.

Anyway, this breaks tests for anyone using a released version of libxml2 regardless of OS. Can the PR objective be achieved using xmlSetStructuredErrorFunc instead of xmlXPathSetErrorHandler? If so, you could consider using it until version 2.13.0 is resealed and widely adopted. For reference, ubuntu's latest release still uses libxml2 version 2.9.14. Otherwise, if xmlSetStructuredErrorFunc does not work here, you could consider conditional compilation on the libxml2 version.

@hurricane-socrates
Copy link
Author

hurricane-socrates commented May 20, 2024

Yeah, I was going around on the question of version support. I have no problem waiting. I see Debian Stable is also on the 2.9 branch

I couldn't get the build script to work with the updated HEAD version. I can see it uses pkg-config, and brew creates a correct definition. Nevertheless... I'll try pounding on it some more. It's probably got something to do with brew not symlinking libxml2 b/c Apple

In the meantime, the whole point of this task for me was that libxml2 reported line & col. It has stopped doing that reporting. Very annoying.

I'll look at xmlSetStructuredErrorFunc. It doesn't ring a bell right now. I'm sure it'll be as useful as xmlXPathSetErrorHandler has turned out to be. Right now I am annoyed at developing OSS on macos. At least three versions of libxml: Apple's, MacPort's, Brew's.

@hurricane-socrates
Copy link
Author

hurricane-socrates commented May 22, 2024

I'd like to withdraw this PR.

  1. Implementing xmlXPathSetErrorHandler for this task increases this project's technical debt;
  2. libxml has stopped reporting detailed xpath errors. I'll look into why this is happening. The change nullifies my reasons for using xmlSetStructuredErrorFunc.

@dginev
Copy link
Member

dginev commented May 22, 2024

Thank you for investigating this @hurricane-socrates and feel free to close here if you've chosen to do so.

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.

4 participants