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

Fix compiling on Mac OS X 10.14 (and presumably anywhere else that LIBXML_THREAD_ENABLED is set) #48

Merged
merged 1 commit into from
Mar 26, 2019

Conversation

caldwell
Copy link
Contributor

The problem is that LIBXML_THREAD_ENABLED changes the xmlIndentTreeOutput global from a simple global variable into a function call that returns an lvalue. The only proper way to detect and handle this is to just use the macro in C.

This patch adds c_wrappers.c and compiles it in the build.rs. I tested this with a recent nightly rust on Mac OS X 10.14 and Debian testing.

@dginev
Copy link
Member

dginev commented Mar 25, 2019

Hi @caldwell , thanks a lot for this PR and problem report!

Would you mind helping me check if we can solve this using the bindgen-generated low-level helpers in bindings.rs, to avoid adding new C files?

Two questions:

  • Can you get the .travis.yml setup to fail in CI, by setting the env var, so that we can test this setup in the future? I can't seem to reproduce the issue locally for now on my Ubuntu setup.
  • Have you tried accessing the global state from c_helpers.rs via e.g.
pub fn setIndentTreeOutput(indent: c_int) {
  unsafe {
    let mut global_state = xmlGetGlobalState();
    (*global_state).xmlIndentTreeOutput = indent;
  }
}

I would consider it a win if we can keep the repo clean of .c files, we had a range of those early on, and bindgen was a huge help in clearing them out.

Edit: Ah, I see that this is a compile-time flag for building libxml2, so it won't be at all easy to reproduce on a linux box, we'd have to rebuild the package from scratch... Hm.

@caldwell
Copy link
Contributor Author

caldwell commented Mar 25, 2019

Hi, thanks for taking a look. I agree less C would be great. Your suggestion works on my setup, but I don't think it's quite right. In particular, after looking at the libxml2 sources, there are 2 places to get the globals. The struct returned from xmlGetGlobalState() is one of them, but there's also a plain global int defined in globals.c. They have a function __xmlIndentTreeOutput() that chooses the struct or the global int based on whether it's running in the main thread (which is always true if threads aren't enabled at all). I was hesitant to use __xmlIndentTreeOutput() because it's named as an internal function and I wasn't sure if it was always there. But after looking at the code it seems to always be defined.

So I suggest this:

pub fn setIndentTreeOutput(indent: c_int) {
  unsafe {
    let xml_indent_tree_output_ptr = __xmlIndentTreeOutput();
    (*xml_indent_tree_output_ptr) = indent;
  }
}

This works on my Mac. I want to test on my linux machine and then I'll push an update.

Fixes building on Mac OS X (and presumably anywhere else that
LIBXML_THREAD_ENABLED is set).
@caldwell caldwell force-pushed the master branch 2 times, most recently from bce3928 to 8550d54 Compare March 25, 2019 23:34
@dginev
Copy link
Member

dginev commented Mar 26, 2019

Wonderful, thank you for taking a detailed look and refining the PR. Looks good to go! I'll release a minor version in case you want to have a cargo dependency.

@dginev dginev merged commit 18955f0 into KWARC:master Mar 26, 2019
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.

None yet

2 participants