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 issues for running on Windows #313

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 17 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: 0 additions & 1 deletion .azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ jobs:
########################################################################################
- job:
displayName: 'Windows'
condition: False

pool:
vmImage: 'vs2017-win2016'
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ install:
test:
# Run a tmp folder to make sure the tests are run on the installed version
mkdir -p $(TESTDIR)
@echo ""
@cd $(TESTDIR); python -c "import $(PROJECT); $(PROJECT).print_clib_info()"
@echo ""
#@echo ""
#@cd $(TESTDIR); python -c "import $(PROJECT); $(PROJECT).print_clib_info()"
#@echo ""
cd $(TESTDIR); pytest $(PYTEST_ARGS) $(PROJECT)
cp $(TESTDIR)/.coverage* .
rm -r $(TESTDIR)
Expand Down
84 changes: 23 additions & 61 deletions pygmt/clib/loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,85 +37,47 @@ 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.

break
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")