GenericMappingTools / gmt

The Generic Mapping Tools
https://www.generic-mapping-tools.org
Other
843 stars 352 forks source link

A potential bug in guessing GMT's share directory #3353

Closed seisman closed 4 years ago

seisman commented 4 years ago

PyGMT crashes with the conda GMT package on Windows. After some debugging, it turns out GMT cannot determine its sharedir for the conda GMT package.

Unlike GMT's official installers, the conda GMT package doesn't define environmental variables GMT6_SHAREDIR, GMT5_SHAREDIR or GMT_SHAREDIR. Thus, GMT has to guess the share directory by calling gmt_guess_sharedir:

https://github.com/GenericMappingTools/gmt/blob/28efd94884247a693a6799f8b176c9d432620f73/src/gmt_init.c#L3019-L3026.

The function sharedir_from_runtime_libdir tries to guess sharedir from runtime_libdir. For Windows, because the gmt.dll library is in the bin directory, the variable runtime_libdir ends with bin, e.g., D:/bld/gmt_1589870953348/_test_env/Library/bin. As the default value of GMT_LIBDIR_RELATIVE is lib, sharedir_from_runtime_libdir can't determine the sharedir by replacing GMT_LIBDIR_RELATIVE with GMT_SHARE_DIR_RELATIVE (e.g., conda uses share/gmt). Thus, the function sharedir_from_runtime_libdir returns NULL.

Then GMT tries to guess sharedir from runtime_bindir. When running GMT command line, the variable runtime_bindir has a value D:/bld/gmt_1589870953348/_test_env/Library/bin, then it can replace bin with share/gmt. However, when PyGMT loads the GMT library, the variable's value is D:/bld/gmt_1589870953348/_test_env.

PaulWessel commented 4 years ago

Unless I misunderstand the above, seems like under Windows the GMT_LIBDIR_RELATIVE should be bin? Also, do you understand why D:/bld/gmt_1589870953348/_test_env is returned? What is returning that? Do you mean runtime_bindir returns that path? Is that the path to the gmtPy script?

seisman commented 4 years ago

Unless I misunderstand the above, seems like under Windows the GMT_LIBDIR_RELATIVE should be bin?

YES, I think so.

Also, do you understand why D:/bld/gmt_1589870953348/_test_env is returned? What is returning that? Do you mean runtime_bindir returns that path? Is that the path to the gmtPy script?

I added a print statement before line 362. The value of the variable runtime_bindir is D:/bld/gmt_1589870953348/_test_env. https://github.com/GenericMappingTools/gmt/blob/a1e72e6c380c84f6502fb8f561de293b481aac77/src/gmt_common_runpath.c#L358-L367

The value of the variable runtime_bindir is determined in the function GMT_runtime_bindir_win32. In function GMT_runtime_bindir_win32, GetModuleFileName set the variable path to D:\bld\gmt_1589944374446\_test_env\python.exe, which is the path of the python executable.

https://github.com/GenericMappingTools/gmt/blob/a1e72e6c380c84f6502fb8f561de293b481aac77/src/gmt_common_runpath.c#L105-L108

PaulWessel commented 4 years ago

When @ldldr wrote that, he was not anticipating any other program that gmt.c calling it (I think). So here you get the path to the Python interpreter instead, which may or may not have a directory that is useful for this fishing expedition.

Perhaps a better approach is to recognized that when API->external is true (and we are calling GMT from MEX, Julia, Python) taht we instead do a quick system call to get the output of

gmt --show-sharedir

as long as gmt is installed in the path (which it surely will be), you get the right dir. I will hack up a branch for testing.

seisman commented 4 years ago

In src/config.h.in, changing

#define GMT_LIBDIR_RELATIVE "@GMT_LIBDIR@"

to

#ifndef WIN32
    #define GMT_LIBDIR_RELATIVE "@GMT_LIBDIR@"
#else
    #define GMT_LIBDIR_RELATIVE "@GMT_BINDIR@"
#endif

works for pygmt. Seems a good solution if gmt is not in PATH as in #3360.

PaulWessel commented 4 years ago

Great, should we leave my external code as a backup in case the fishing fails?

seisman commented 4 years ago

Perhaps not necessary now. I want to avoid calling external command when loading libraries.

PaulWessel commented 4 years ago

OK, will count on you to remember this scheme should it become necessary in the future. I will delete the branch.