gjtorikian / mathematical

Convert mathematical equations to SVGs, PNGs, or MathML. A general wrapper to Lasem and mtex2MML.
https://gjtorikian.github.io/mathematical/
MIT License
166 stars 31 forks source link

Catalina updates #94

Closed gjtorikian closed 4 years ago

gjtorikian commented 4 years ago

Fixes https://github.com/gjtorikian/mathematical/issues/93, closes https://github.com/gjtorikian/mathematical/issues/89

Many thanks to @lorrden for the hint on using xcrun. I Observed the same problems as #89 after updating to Catalina, and was able to fix the libxml2 header linking.

...Unfortunately, there's a new problem. CMake can no link against any of the library (glib2.0, cairo, etc.). It finds them--this is one of the first steps:

- Found PkgConfig: /usr/local/bin/pkg-config (found version "0.29.2") 
-- Checking for module 'glib-2.0'
--   Found glib-2.0, version 2.62.3
-- Checking for module 'cairo'
--   Found cairo, version 1.14.6
-- Checking for module 'pango'
--   Found pango, version 1.44.7
-- Checking for module 'gdk-pixbuf-2.0'
--   Found gdk-pixbuf-2.0, version 2.40.0
-- Checking for module 'libxml-2.0'
--   Found libxml-2.0, version 2.9.4
-- Checking for module 'gio-2.0'
--   Found gio-2.0, version 2.62.3
-- Checking for module 'pangocairo'
--   Found pangocairo, version 1.44.7

But when it comes time to actually link them, there's an error:

[ 93%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmsvgtransformable.c.o
[ 94%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmsvgtspanelement.c.o
[ 95%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmsvguseelement.c.o
[ 96%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmsvgview.c.o
[ 97%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmtraits.c.o
[ 98%] Building C object CMakeFiles/lasem.dir/lasem/src/lsmutils.c.o
[ 99%] Building C object CMakeFiles/lasem.dir/lasem_overrides.c.o
/Users/gjtorikian/Development/mathematical/ext/mathematical/lasem_overrides.c:20:54: warning: control reaches end of non-void function [-Wreturn-type]
lsm_itex_to_mathml (const char *itex, gssize size) { }
                                                     ^
1 warning generated.
[100%] Linking C shared library liblasem.dylib
ld: library not found for -lglib-2.0
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [liblasem.dylib] Error 1
make[1]: *** [CMakeFiles/lasem.dir/all] Error 2
make: *** [all] Error 2
*** ../../../../ext/mathematical/extconf.rb failed ***

I will have to continue to explore why this is, but I do not have time at the moment. I'm opening this PR in the hopes that someone smarter than I sees the problem.

lorrden commented 4 years ago

The rake-updates result in:

mathematical git:(ce2600b) ✗ script/bootstrap 
==> Initing Git submodules
Submodule path 'ext/mathematical/lasem': checked out '79d2b9c0a27a420683634a222a818d4333c85fb7'
==> Installing gem dependencies…
Could not find gem 'rake (~> 13.0)' in any of the gem sources listed in your
Gemfile.

Which is kind of the same, but with a newer version. Anyone who have an idea of what may be the issue here?

gjtorikian commented 4 years ago

@lorrden Try with this branch now, I think that should be fixed.

lorrden commented 4 years ago

@lorrden Try with this branch now, I think that should be fixed.

Yeah, looks like that issue is fixed.

lorrden commented 4 years ago

Regarding the lasem build failure. Looks like there are no -L flags to point out the directory containing glib, Pango etc. It probably worked before by accident since sysroot was / and thus any use of /usr/local/lib (which will be a default search dir for clang) was picked up automatically by clang without needing to specify an L-flag.

I figured out how Lasem was built (I was mislead by the autoconf files in the directory), you need to add:

target_link_directories(${LIBRARY} PRIVATE 
  ${GLIB2_LIBRARY_DIRS}
  ${CAIRO_LIBRARY_DIRS}
  ${PANGO_LIBRARY_DIRS}
  ${GDK-PIXBUF2_LIBRARY_DIRS}
  ${LIBXML2_LIBRARY_DIRS}
  ${GIO2_LIBRARY_DIRS}
  ${PANGO_CAIRO_LIBRARY_DIRS}
  ${RUBY_LIBDIR}
)

To the lasem cmake file and the compile will pass. I am looking at the next error which is that the test suite fails with runtime dyld-error (rpath related).

Next error is:

/Users/lorrden/Projects/mathematical/lib/mathematical.rb:1:in `require': 
dlopen(/Users/lorrden/Projects/mathematical/lib/mathematical/mathematical.bundle, 0x0009): 
dependent dylib '@rpath/liblasem.dylib' not found for
 '/Users/lorrden/Projects/mathematical/lib/mathematical/mathematical.bundle'.
relative file paths not allowed '@rpath/liblasem.dylib' - 
/Users/lorrden/Projects/mathematical/lib/mathematical/mathematical.bundle (LoadError)
gjtorikian commented 4 years ago

@lorrden Oh, I missed your comment edit and went ahead and figured it out locally. Including the rpath issue. Let's see what Travis CI thinks.

lorrden commented 4 years ago

I saw the same issue as travis when I pulled the latest branch. I know that SIP (system integrity protection) made changes to @rpath handling in libraries and plugins.

I'll dig around a bit to see how to resolve this.

lorrden commented 4 years ago

Seems like no LC_RPATH command has been added to the built mathematical.bundle file. I hacked it manually using

install_name_tool -add_rpath @loader_path/../../ext/mathematical/lasem/build lib/mathematical/mathematical.bundle

That allowed the tests to pass at least, but the RPATH in that case is build specific. It should probably have a @loader_path based resolution as I tried (i.e. relative to the mathematical.bundle), but I am uncertain as to how the bundle is normally installed.

The test dir associated path could possibly be fixed by running install_name_tool with the -rpath old new flag which could change the test specific dir to install specific (this is similar what CMake does under the hood when running the install command).

What I am confused about is why the @rpath is used on Catalina without a corresponding LC_RPATH when this has apparently worked on previous macOSes.

lorrden commented 4 years ago

I think the solution is:

install_name_tool -add_rpath @loader_path/../../ext/mathematical/lib \
    lib/mathematical/mathematical.bundle

The directory layout assumptions works in the build dir and I think the gem install dir as well. The command needs to be executed after the build, and before the test.

Alternatively, clang could do that when linking the mathematical.bundle by adding -rpath @loader_path/../../ext/mathematical/lib but I have no clue how to change the bundle file's link flags.

gjtorikian commented 4 years ago

I need to think a bit on whether to continue to support old macOS releases with all of these changes, and if so, how.

gjtorikian commented 4 years ago

According to Travis, it uses 10.13 by default, which makes me even more confused as to how all of this broke in Catalina.

lorrden commented 4 years ago

Strange, I rebuilt it cleanly locally and the test failed with the same error as reported by travis below on the first invocation, but not the second one. Seems to be a bit random in behaviour.

gjtorikian commented 4 years ago

That error is definitely flakey and I'm ashamed to say I've seen it before, but never sorted a fix out.

lorrden commented 4 years ago

That error is definitely flakey and I'm ashamed to say I've seen it before, but never sorted a fix out.

I tried to disable different tests running the test command in a loop. Initially it seems to me that the delimiters_test is the main culprit, enabling and disabling the maliciousness_test affects the probability of the delimiters_test failing.

lorrden commented 4 years ago

FYI I enabled address sanitisers on mtex2MML and lasem and ran the test suite with them but this did not yield any results.

gjtorikian commented 4 years ago

I'm currently narrowing down whatever this flakey test is by passing in --seed=53660. No luck so far.

gjtorikian commented 4 years ago

Got it. 😄

gjtorikian commented 4 years ago

Thanks for your help @lorrden, I appreciate it.