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.

Sign up for GitHub

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

Jump to bottom

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

Closed
seisman opened this issue May 19, 2020 · 7 comments · Fixed by #3361
Closed

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

seisman opened this issue May 19, 2020 · 7 comments · Fixed by #3361

Comments

@seisman
Copy link
Member

seisman commented May 19, 2020

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:

gmt/src/gmt_init.c

Lines 3019 to 3026 in 28efd94

/* SHAREDIR still not found, make a smart guess based on runpath: */
if (gmt_guess_sharedir (path, GMT->init.runtime_bindir))
GMT->session.SHAREDIR = gmt_strdup_noquote (path);
else {
/* Still not found */
GMT_Report (GMT->parent, GMT_MSG_ERROR, "Could not locate share directory for GMT.\n");
GMT_exit (GMT, GMT_RUNTIME_ERROR); return GMT_RUNTIME_ERROR;
}
.

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.
This was referenced May 19, 2020
Merged
Closed
@PaulWessel
Copy link
Member

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
Copy link
Member Author

seisman commented May 20, 2020 •
edited
Loading

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.

gmt/src/gmt_common_runpath.c

Lines 358 to 367 in a1e72e6

char *gmt_guess_sharedir (char *sharedir, const char *runtime_bindir) {
/* 1. guess based on runtime_libdir */
if (sharedir_from_runtime_libdir (sharedir) == NULL) {
/* 2. guess based on runtime_bindir */
if (sharedir_from_runtime_bindir (sharedir, runtime_bindir) == NULL)
return NULL;
}
return sharedir;
}

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.

gmt/src/gmt_common_runpath.c

Lines 105 to 108 in a1e72e6

/* Get absolute path of executable */
if ( GetModuleFileName (NULL, path, PATH_MAX) == PATH_MAX )
/* Path to long */
return NULL;

@PaulWessel
Copy link
Member

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.

@PaulWessel PaulWessel mentioned this issue May 20, 2020
Closed
@seisman
Copy link
Member Author

seisman commented May 20, 2020

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
Copy link
Member

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

seisman added a commit that referenced this issue May 20, 2020
@seisman
Fix GMT_LIBDIR_RELATIVE for Windows
ad16c99 
On Windows, the gmt.dll is located in the `bin` directory, instead of
the usual `lib` directory. Thus, for windows, we need to set
`GMT_LIBDIR_RELATIVE` to `bin`.

Closes #3353.
@seisman seisman mentioned this issue May 20, 2020
Merged
@seisman
Copy link
Member Author

seisman commented May 20, 2020

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

@PaulWessel
Copy link
Member

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

seisman added a commit that referenced this issue May 20, 2020
@seisman
Fix GMT_LIBDIR_RELATIVE for Windows (#3361)
d5b80a5 
On Windows, the gmt.dll is located in the `bin` directory, instead of
the usual `lib` directory. Thus, for windows, we need to set
`GMT_LIBDIR_RELATIVE` to `bin`.

Closes #3353.
seisman added a commit to GenericMappingTools/pygmt that referenced this issue May 20, 2020
@seisman
Remove the variable GMT_SHAREDIR from CI settings
ec4f63a 
The variable GMT_SHAREDIR was needed due to a GMT bug.
See GenericMappingTools/gmt#3353 for the bug report,
and GenericMappingTools/gmt#3361 for the fix.

The conda GMT package was rebuilt in conda-forge/gmt-feedstock#94,
in which the patch was applied to GMT 6.0.0.

Now PyGMT should work well with no need to add the variable GMT_SHAREDIR.
@seisman seisman mentioned this issue May 20, 2020
Merged
5 tasks
seisman added a commit to GenericMappingTools/pygmt that referenced this issue May 20, 2020
@seisman
Remove the variable GMT_SHAREDIR from CI settings (#437)
deaa5e1 
The variable GMT_SHAREDIR was needed due to a GMT bug.
See GenericMappingTools/gmt#3353 for the bug report,
and GenericMappingTools/gmt#3361 for the fix.

The conda GMT package was rebuilt in conda-forge/gmt-feedstock#94,
in which the patch was applied to GMT 6.0.0.

Now PyGMT should work well with no need to add the variable GMT_SHAREDIR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@seisman @PaulWessel