Fix issues in loading GMT's shared library

[seisman]

Description of proposed changes

The load_libgmt() function still has some bugs:

pygmt/pygmt/clib/loading.py

Lines 36 to 52 in 6a6171e

	lib_fullnames = []
	error = True
	for libname in clib_full_names():
	lib_fullnames.append(libname)
	try:
	libgmt = ctypes.CDLL(libname)
	check_libgmt(libgmt)
	error = False
	break
	except OSError as err:
	error = err
	if error:
	raise GMTCLibNotFoundError(
	"Error loading the GMT shared library "
	f"{', '.join(lib_fullnames)}.\n {error}."
	)
	return libgmt

1. At line 45, we only catch the OSError exception, but check_libgmt() may raise GMTCLibError exception. So if the library paths are ["/path/to/GMT5/libgmt.so", "/path/to/GMT6/libgmt.so", "libgmt.so"], the load_libgmt() function won't be able to the working library.
2. After Improve how PyGMT finds the GMT library #702, the clib_full_names() function returns a list of all possible paths. Some paths may be duplicated. So, if the paths are ["invalid_path_A", "invalid_path_A", "invalid_path_B"], we will check invalid_path_A twice.
3. The variable error only contains the error message of the last library path, but, each library path may fail due to different reasons.

This PR makes the following changes:

1. Catch the GMTCLibError error from calling check_libgmt() (Fixes point 1)
2. Skip a library path if it's known to fail in previous tries (Fixes point 2)
3. Improve the error message, by combine error message of all tries (Fixes point 3)
4. Add a new parameter lib_fullnames (default to clib_full_names()) to for easier testing
5. Add more tests to make sure it works.

Fixes #

Reminders

• Run make format and make check to make sure the code follows the style guide.
• Add tests for new features or tests that would have caught the bug that you're fixing.
• Add new public functions/methods/classes to doc/api/index.rst.
• Write detailed docstrings for all functions/methods.
• If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

• /format: automatically format and lint the code
• /test-gmt-dev: run full tests on the latest GMT development version

[weiji14]

Quite a few comments. After trying to have a go at refactoring the big 'test_load_libgmt_with_broken_libraries', I can see why you've coded it up this way, but it's not easy to follow the code and I worry about the poor maintainer in the future having to understand what is happening 😢

In a way, I wonder if we're overcomplicating the load_libgmt et al. functions in pygmt/clib/loading.py. It seems like a crazy amount of possible combinations we're testing (3 OSes, different GMT_LIBRARY_PATHs, libraries that may or may not load, etc). Probably need to have a hard think about whether we can actually simplify the whole loading logic.

[weiji14]

Could you change L78:L79 to read:

if os_name.startswith(("linux", "freebsd")):
    libnames = ["libgmt.so"]

Just gets rid of one elif statement.

[weiji14]

Thanks for adding the comprehensive tests @seisman, sorry for dragging this out for so long. Will be good to merge once all the tests pass and the Style Checks errors are resolved.

[seisman]

@weiji14 Thanks for your review.

Now the Style Checks reports errors like:

pygmt/tests/test_clib_loading.py:135:40: W0613: Unused argument 'mock_ctypes' (unused-argument)

The argument mock_ctypes is necessary, so I can't remove it. Should I just disable the pylint warning?

[weiji14]

Now the Style Checks reports errors like:

pygmt/tests/test_clib_loading.py:135:40: W0613: Unused argument 'mock_ctypes' (unused-argument)

The argument mock_ctypes is necessary, so I can't remove it. Should I just disable the pylint warning?

Yes.