Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ jobs:
CONDA_REQUIREMENTS_DEV: requirements-dev.txt
CONDA_INSTALL_EXTRA: "codecov"
CONDA_EXTRA_CHANNEL: "conda-forge/label/dev"
GMT_LIBRARY_PATH: "C:\Miniconda\envs\testing\lib"

strategy:
matrix:
Expand Down
83 changes: 22 additions & 61 deletions pygmt/clib/loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,85 +37,46 @@ def load_libgmt(env=None):
couldn't access the functions).

"""
libpath = get_clib_path(env)
try:
libgmt = ctypes.CDLL(libpath)
check_libgmt(libgmt)
except OSError as err:
msg = "\n".join(
[
"Error loading the GMT shared library '{}':".format(libpath),
"{}".format(str(err)),
]
)
raise GMTCLibNotFoundError(msg)
return libgmt


def get_clib_path(env):
"""
Get the path to the libgmt shared library.

Determine the file name and extension and append to the path set by
``GMT_LIBRARY_PATH``, if any.

Parameters
----------
env : dict or None
A dictionary containing the environment variables. If ``None``, will
default to ``os.environ``.

Returns
-------
libpath : str
The path to the libgmt shared library.

"""
libname = clib_name()
if env is None:
env = os.environ
if "GMT_LIBRARY_PATH" in env:
libpath = os.path.join(env["GMT_LIBRARY_PATH"], libname)
else:
libpath = libname
return libpath
libnames = clib_name(os_name=sys.platform)
libpath = env.get("GMT_LIBRARY_PATH", "")
error = False
for libname in libnames:
try:
libgmt = ctypes.CDLL(os.path.join(libpath, libname))
check_libgmt(libgmt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both ctypes.CDLL and check_libgmt don't raise any errors, it means we find the library, then we should break out of the loop. Right?

Suggested change
check_libgmt(libgmt)
check_libgmt(libgmt)
break

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course! Thanks, Dongdong. I stopped coding yesterday because I couldn't even see this basic error anymore :)

Copy link
Member

@weiji14 weiji14 Nov 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that this change is part of what's been causing our Windows builds to fail... Since we have a list of dll files to check, if one of the dll fails, the variable error turns True and stays True even if a later dll works. Our Linux and MacOS tests work because there's only one item in the list.

except OSError as err:
error = err
if error:
raise GMTCLibNotFoundError(
"Error loading the GMT shared library '{}':".format(libname)
)
return libgmt


def clib_name(os_name=None, is_64bit=None):
def clib_name(os_name):
"""
Return the name of GMT's shared library for the current OS.

Parameters
----------
os_name : str or None
The operating system name as given by ``sys.platform``
(the default if None).
is_64bit : bool or None
Whether or not the OS is 64bit. Only used if the OS is ``win32``. If None, will
determine automatically.
os_name : str
The operating system name as given by ``sys.platform``.

Returns
-------
libname : str
The name of GMT's shared library.
libname : list of str
List of possible names of GMT's shared library.

"""
if os_name is None:
os_name = sys.platform

if is_64bit is None:
is_64bit = sys.maxsize > 2 ** 32

if os_name.startswith("linux"):
libname = "libgmt.so"
libname = ["libgmt.so"]
elif os_name == "darwin":
# Darwin is macOS
libname = "libgmt.dylib"
libname = ["libgmt.dylib"]
elif os_name == "win32":
if is_64bit:
libname = "gmt_w64.dll"
else:
libname = "gmt_w32.dll"
libname = ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
else:
raise GMTOSError('Operating system "{}" not supported.'.format(sys.platform))
return libname
Expand Down
51 changes: 0 additions & 51 deletions pygmt/tests/test_clib.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,9 @@

from .. import clib
from ..clib.session import FAMILIES, VIAS
from ..clib.loading import clib_name, load_libgmt, check_libgmt, get_clib_path
from ..clib.conversion import dataarray_to_matrix
from ..exceptions import (
GMTCLibError,
GMTOSError,
GMTCLibNotFoundError,
GMTCLibNoSessionError,
GMTInvalidInput,
GMTVersionError,
Expand Down Expand Up @@ -70,54 +67,6 @@ def mock_get_libgmt_func(name, argtypes=None, restype=None):
setattr(session, "get_libgmt_func", get_libgmt_func)


def test_load_libgmt():
"Test that loading libgmt works and doesn't crash."
load_libgmt()


def test_load_libgmt_fail():
"Test that loading fails when given a bad library path."
env = {"GMT_LIBRARY_PATH": "not/a/real/path"}
with pytest.raises(GMTCLibNotFoundError):
load_libgmt(env=env)


def test_get_clib_path():
"Test that the correct path is found when setting GMT_LIBRARY_PATH."
# Get the real path to the library first
with clib.Session() as lib:
libpath = lib.info["library path"]
libdir = os.path.dirname(libpath)
# Assign it to the environment variable but keep a backup value to restore
# later
env = {"GMT_LIBRARY_PATH": libdir}

# Check that the path is determined correctly
path_used = get_clib_path(env=env)
assert os.path.samefile(path_used, libpath)
assert os.path.dirname(path_used) == libdir

# Check that loading libgmt works
load_libgmt(env=env)


def test_check_libgmt():
"Make sure check_libgmt fails when given a bogus library"
with pytest.raises(GMTCLibError):
check_libgmt(dict())


def test_clib_name():
"Make sure we get the correct library name for different OS names"
for linux in ["linux", "linux2", "linux3"]:
assert clib_name(linux) == "libgmt.so"
assert clib_name("darwin") == "libgmt.dylib"
assert clib_name("win32", is_64bit=True) == "gmt_w64.dll"
assert clib_name("win32", is_64bit=False) == "gmt_w32.dll"
with pytest.raises(GMTOSError):
clib_name("meh")


def test_getitem():
"Test that I can get correct constants from the C lib"
ses = clib.Session()
Expand Down
35 changes: 35 additions & 0 deletions pygmt/tests/test_clib_loading.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""
Test the functions that load libgmt
"""
import pytest

from ..clib.loading import clib_name, load_libgmt, check_libgmt
from ..exceptions import GMTCLibError, GMTOSError, GMTCLibNotFoundError


def test_check_libgmt():
"Make sure check_libgmt fails when given a bogus library"
with pytest.raises(GMTCLibError):
check_libgmt(dict())


def test_load_libgmt():
"Test that loading libgmt works and doesn't crash."
check_libgmt(load_libgmt())


def test_load_libgmt_fail():
"Test that loading fails when given a bad library path."
env = {"GMT_LIBRARY_PATH": "not/a/real/path"}
with pytest.raises(GMTCLibNotFoundError):
load_libgmt(env=env)


def test_clib_name():
"Make sure we get the correct library name for different OS names"
for linux in ["linux", "linux2", "linux3"]:
assert clib_name(linux) == ["libgmt.so"]
assert clib_name("darwin") == ["libgmt.dylib"]
assert clib_name("win32") == ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
with pytest.raises(GMTOSError):
clib_name("meh")