From 2cc5a0d63bb5a9ce79531dfd26062ffc0d593c74 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 27 Oct 2020 17:04:02 -0400 Subject: [PATCH 01/10] Improve how PyGMT find the GMT library In the old codes, PyGMT first checks if the environmental variable `GMT_LIBRARY_PATH` is defined. If yes, it tries to find the GMT library in the specified path. Otherwise, it will search for the GMT library in standard library paths (LD_LIBRARY_PATH on Linux; DYLD_LIBRARY_PATH on macOS; PATH on Windows). This PR improve how PyGMT find the library, by search for all possible paths. The paths from the highest priority to the lowest priority are: 1. the path defined by `GMT_LIBRARY_PATH` 2. the path returned by command `gmt --show-library` 3. On Windows, also check the path returned by `find_library` function 4. the standard system paths Thus, the function `clib_full_names` now returns a list of possible names, for example: ``` ["/path/defined/by/gmtlibrarypath/libgmt.so", "/usr/local/lib/libgmt.so", "libgmt.so"] ``` **Pros**: 1. More ways to find the library, so less "GMTCLibNotFoundError" errors **Cons**: 1. It calls the "gmt" command in a new process 2. A much longer list returned by `clib_full_names`. Maybe slower loading? 3. Difficult to test. Closes #628. --- pygmt/clib/loading.py | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/pygmt/clib/loading.py b/pygmt/clib/loading.py index ee9332e86be..cc455c41cfc 100644 --- a/pygmt/clib/loading.py +++ b/pygmt/clib/loading.py @@ -8,6 +8,7 @@ import sys import ctypes from ctypes.util import find_library +import subprocess as sp from ..exceptions import GMTOSError, GMTCLibError, GMTCLibNotFoundError @@ -97,16 +98,37 @@ def clib_full_names(env=None): """ if env is None: env = os.environ + libnames = clib_names(os_name=sys.platform) # e.g. libgmt.so, libgmt.dylib, gmt.dll - libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib - lib_fullnames = [os.path.join(libpath, libname) for libname in libnames] - # Search for DLLs in PATH if GMT_LIBRARY_PATH is not defined [Windows only] - if not libpath and sys.platform == "win32": + # list of libraries paths to search, sort by priority from high to low + lib_fullnames = [] + + # Search for libraries in GMT_LIBRARY_PATH if defined. + libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib + if libpath: + for libname in libnames: + lib_fullnames.append(os.path.join(libpath, libname)) + + # Search for the library returned by command "gmt --show-library" + try: + lib_fullpath = sp.check_output( + ["gmt", "--show-library"], encoding="utf-8" + ).rstrip("\n") + lib_fullnames.append(lib_fullpath) + except FileNotFoundError: # command not found + pass + + # Search for DLLs in PATH (done by calling "find_library") + if sys.platform == "win32": for libname in libnames: libfullpath = find_library(libname) if libfullpath: lib_fullnames.append(libfullpath) + + # Search for library names in the system default path [the lowest priority] + lib_fullnames.extend(libnames) + return lib_fullnames @@ -133,10 +155,5 @@ def check_libgmt(libgmt): functions = ["Create_Session", "Get_Enum", "Call_Module", "Destroy_Session"] for func in functions: if not hasattr(libgmt, "GMT_" + func): - msg = " ".join( - [ - "Error loading libgmt.", - "Couldn't access function GMT_{}.".format(func), - ] - ) + msg = f"Error loading libgmt. Couldn't access function GMT_{func}." raise GMTCLibError(msg) From 50dae173a358fb44b7ec18e19bf8723ac795e2d1 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 29 Nov 2020 16:30:16 -0500 Subject: [PATCH 02/10] Check library names for FreeBSD --- pygmt/tests/test_clib_loading.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 9c11cb698c1..b3984fcdb6e 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -41,5 +41,7 @@ def test_clib_names(): assert clib_names(linux) == ["libgmt.so"] assert clib_names("darwin") == ["libgmt.dylib"] assert clib_names("win32") == ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"] + for freebsd in ["freebsd10", "freebsd11", "freebsd12"]: + assert clib_names(freebsd) == ["libgmt.so"] with pytest.raises(GMTOSError): clib_names("meh") From cbc4eb5eea1ced10ee096c098faa3e9f15870277 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 29 Nov 2020 16:34:42 -0500 Subject: [PATCH 03/10] loading works even though the given library path is invalid --- pygmt/tests/test_clib_loading.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index b3984fcdb6e..870808d116f 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -19,14 +19,13 @@ def test_load_libgmt(): check_libgmt(load_libgmt()) -def test_load_libgmt_fail(): - "Test that loading fails when given a bad library path." +def test_load_libgmt_with_a_bad_library_path(): + "Test that loading still works when given a bad library path." # save the old value (if any) before setting a fake "GMT_LIBRARY_PATH" old_gmt_library_path = os.environ.get("GMT_LIBRARY_PATH") os.environ["GMT_LIBRARY_PATH"] = "/not/a/real/path" - with pytest.raises(GMTCLibNotFoundError): - load_libgmt() + check_libgmt(load_libgmt()) # revert back to the original status (if any) if old_gmt_library_path: From c9e142e63331e054db3406e55fa64ae7aebb6089 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 29 Nov 2020 16:39:24 -0500 Subject: [PATCH 04/10] Fix a lint issue --- pygmt/tests/test_clib_loading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 870808d116f..6cbb9024314 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -5,7 +5,7 @@ import pytest from ..clib.loading import clib_names, load_libgmt, check_libgmt -from ..exceptions import GMTCLibError, GMTOSError, GMTCLibNotFoundError +from ..exceptions import GMTCLibError, GMTOSError def test_check_libgmt(): From 5f17f1e96ec9cc2d33dd41ad86beba77c7a2675f Mon Sep 17 00:00:00 2001 From: Wei Ji Date: Fri, 15 Jan 2021 17:10:30 +1300 Subject: [PATCH 05/10] Use generator yield in clib_full_names function --- pygmt/clib/loading.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pygmt/clib/loading.py b/pygmt/clib/loading.py index 4b38a26c969..f6ffa681073 100644 --- a/pygmt/clib/loading.py +++ b/pygmt/clib/loading.py @@ -6,9 +6,9 @@ """ import ctypes import os +import subprocess as sp import sys from ctypes.util import find_library -import subprocess as sp from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError @@ -33,9 +33,10 @@ def load_libgmt(): couldn't access the functions). """ - lib_fullnames = clib_full_names() + lib_fullnames = [] error = True - for libname in lib_fullnames: + for libname in clib_full_names(): + lib_fullnames.append(libname) try: libgmt = ctypes.CDLL(libname) check_libgmt(libgmt) @@ -100,22 +101,20 @@ def clib_full_names(env=None): env = os.environ libnames = clib_names(os_name=sys.platform) # e.g. libgmt.so, libgmt.dylib, gmt.dll + libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib # list of libraries paths to search, sort by priority from high to low - lib_fullnames = [] - # Search for libraries in GMT_LIBRARY_PATH if defined. - libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib if libpath: for libname in libnames: - lib_fullnames.append(os.path.join(libpath, libname)) + yield os.path.join(libpath, libname) # Search for the library returned by command "gmt --show-library" try: lib_fullpath = sp.check_output( ["gmt", "--show-library"], encoding="utf-8" ).rstrip("\n") - lib_fullnames.append(lib_fullpath) + yield lib_fullpath except FileNotFoundError: # command not found pass @@ -124,12 +123,11 @@ def clib_full_names(env=None): for libname in libnames: libfullpath = find_library(libname) if libfullpath: - lib_fullnames.append(libfullpath) + yield libfullpath # Search for library names in the system default path [the lowest priority] - lib_fullnames.extend(libnames) - - return lib_fullnames + for libname in libnames: + yield libname def check_libgmt(libgmt): From 20728f9d6a33e60b81cbc06558a1b6465c28e845 Mon Sep 17 00:00:00 2001 From: Wei Ji Date: Wed, 20 Jan 2021 16:53:56 +1300 Subject: [PATCH 06/10] Refactor test_load_libgmt_with_a_bad_library_path to use monkeypatch Cleaner to use pytest's `monkeypatch.setenv` fixture to set an environment variable, see https://docs.pytest.org/en/4.6.x/monkeypatch.html#monkeypatching-environment-variables --- pygmt/tests/test_clib_loading.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index d0228a8758e..ff6ed0692a0 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -1,8 +1,6 @@ """ Test the functions that load libgmt """ -import os - import pytest from pygmt.clib.loading import check_libgmt, clib_names, load_libgmt from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError @@ -19,19 +17,11 @@ def test_load_libgmt(): check_libgmt(load_libgmt()) -def test_load_libgmt_with_a_bad_library_path(): +def test_load_libgmt_with_a_bad_library_path(monkeypatch): "Test that loading still works when given a bad library path." - # save the old value (if any) before setting a fake "GMT_LIBRARY_PATH" - old_gmt_library_path = os.environ.get("GMT_LIBRARY_PATH") - - os.environ["GMT_LIBRARY_PATH"] = "/not/a/real/path" - check_libgmt(load_libgmt()) - - # revert back to the original status (if any) - if old_gmt_library_path: - os.environ["GMT_LIBRARY_PATH"] = old_gmt_library_path - else: - del os.environ["GMT_LIBRARY_PATH"] + # Set a fake "GMT_LIBRARY_PATH" + monkeypatch.setenv("GMT_LIBRARY_PATH", "/not/a/real/path") + assert check_libgmt(load_libgmt()) is None def test_clib_names(): From 243ad13d094431e1d977d7aea0dfd0dc94609787 Mon Sep 17 00:00:00 2001 From: Wei Ji Date: Wed, 20 Jan 2021 16:56:29 +1300 Subject: [PATCH 07/10] Monkeypatch test to check that GMTCLibNotFoundError is raised properly --- pygmt/clib/loading.py | 5 +++-- pygmt/tests/test_clib_loading.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pygmt/clib/loading.py b/pygmt/clib/loading.py index f6ffa681073..ffd85575d49 100644 --- a/pygmt/clib/loading.py +++ b/pygmt/clib/loading.py @@ -77,7 +77,7 @@ def clib_names(os_name): elif os_name.startswith("freebsd"): # FreeBSD libnames = ["libgmt.so"] else: - raise GMTOSError(f'Operating system "{sys.platform}" not supported.') + raise GMTOSError(f'Operating system "{os_name}" not supported.') return libnames @@ -114,8 +114,9 @@ def clib_full_names(env=None): lib_fullpath = sp.check_output( ["gmt", "--show-library"], encoding="utf-8" ).rstrip("\n") + assert os.path.exists(lib_fullpath) yield lib_fullpath - except FileNotFoundError: # command not found + except (FileNotFoundError, AssertionError): # command not found pass # Search for DLLs in PATH (done by calling "find_library") diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index ff6ed0692a0..44c4dfd4cff 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -1,6 +1,9 @@ """ Test the functions that load libgmt """ +import subprocess +import sys + import pytest from pygmt.clib.loading import check_libgmt, clib_names, load_libgmt from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError @@ -17,6 +20,21 @@ def test_load_libgmt(): check_libgmt(load_libgmt()) +@pytest.mark.skipif(sys.platform == "win32", reason="run on UNIX platforms only") +def test_load_libgmt_fails(monkeypatch): + """ + Test that GMTCLibNotFoundError is raised when GMT's shared library cannot + be found. + """ + with monkeypatch.context() as mpatch: + mpatch.setattr(sys, "platform", "win32") # pretend to be on Windows + mpatch.setattr( + subprocess, "check_output", lambda cmd, encoding: "libfakegmt.so" + ) + with pytest.raises(GMTCLibNotFoundError): + check_libgmt(load_libgmt()) + + def test_load_libgmt_with_a_bad_library_path(monkeypatch): "Test that loading still works when given a bad library path." # Set a fake "GMT_LIBRARY_PATH" From dbfd7e29b4233c56262921877146fc85470a2f49 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 11 Feb 2021 23:51:14 -0500 Subject: [PATCH 08/10] Check if the GMT shared library exists in GMT_LIBRARY_PATH --- pygmt/clib/loading.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pygmt/clib/loading.py b/pygmt/clib/loading.py index ce192a64718..5f72f19c7bd 100644 --- a/pygmt/clib/loading.py +++ b/pygmt/clib/loading.py @@ -97,21 +97,23 @@ def clib_full_names(env=None): env = os.environ libnames = clib_names(os_name=sys.platform) # e.g. libgmt.so, libgmt.dylib, gmt.dll - libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib # list of libraries paths to search, sort by priority from high to low # Search for libraries in GMT_LIBRARY_PATH if defined. + libpath = env.get("GMT_LIBRARY_PATH", "") # e.g. $HOME/miniconda/envs/pygmt/lib if libpath: for libname in libnames: - yield os.path.join(libpath, libname) + libfullpath = os.path.join(libpath, libname) + if os.path.exists(libfullpath): + yield libfullpath # Search for the library returned by command "gmt --show-library" try: - lib_fullpath = sp.check_output( + libfullpath = sp.check_output( ["gmt", "--show-library"], encoding="utf-8" ).rstrip("\n") - assert os.path.exists(lib_fullpath) - yield lib_fullpath + assert os.path.exists(libfullpath) + yield libfullpath except (FileNotFoundError, AssertionError): # command not found pass From 6769dc3d5c6af3685352e6e7633901ae4dd01945 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 11 Feb 2021 23:52:30 -0500 Subject: [PATCH 09/10] Format codes --- pygmt/tests/test_clib_loading.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 620ced80e79..0d2edc9b60e 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -40,7 +40,9 @@ def test_load_libgmt_fails(monkeypatch): def test_load_libgmt_with_a_bad_library_path(monkeypatch): - "Test that loading still works when given a bad library path." + """ + Test that loading still works when given a bad library path. + """ # Set a fake "GMT_LIBRARY_PATH" monkeypatch.setenv("GMT_LIBRARY_PATH", "/not/a/real/path") assert check_libgmt(load_libgmt()) is None From c3887ce25e45e23cf6d50cfb7644898c3d7c581c Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 12 Feb 2021 01:55:09 -0500 Subject: [PATCH 10/10] Change Returns to Yields --- pygmt/clib/loading.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/clib/loading.py b/pygmt/clib/loading.py index 5f72f19c7bd..ee46c9ebd5c 100644 --- a/pygmt/clib/loading.py +++ b/pygmt/clib/loading.py @@ -88,8 +88,8 @@ def clib_full_names(env=None): A dictionary containing the environment variables. If ``None``, will default to ``os.environ``. - Returns - ------- + Yields + ------ lib_fullnames: list of str List of possible full names of GMT's shared library. """