Simpler gmt_*_module.[ch] files

[PaulWessel]

Description of proposed changes

I have abstracted the key part of the various lookup functions and added them to the gmt_api.c. Now, the core or supplement-specific look-up function simply are shells that pass in the relevant array of module information into the main gmtapi functions. This reduced the code for gmt_module_*.[ch] by 22%.

[PaulWessel]

I also think we could avoid the #ifdef BUILD_SHARED_LIBS branch in gmt_core_module.h by only keeping the one with the list of function names, since they are only used in a call if BUILD_SHARED_LIBS is defined, otherwise it does not matter, no?
Builds for me and tests run, on macOS. So this is 35% savings in lines of code.

[PaulWessel]

I will simplify the shell script next, it ought to be even less code than 65%.

[PaulWessel]

We need a test on Windows here of course since we are messing with the dark shared library side.

[joa-quim]

I breaks my MB supplement again

[PaulWessel]

Can you be more specific? It did not break my gsfml supplement even though I did not make any changes there, only to GMT. My changes should not have any effect on other supplements since you are not calling things in gmt_core_*.c etc.

[PaulWessel]

There are still parts of the module.h files that are created by the script but that probably could just be part of a static include file. For instance, these prototypes:

/* Prototypes of all modules in the GMT core library */
EXTERN_MSC int GMT_psbasemap (void *API, int mode, void *args);
EXTERN_MSC int GMT_begin (void *API, int mode, void *args);
...

do not change, except when we very infrequently introduce a new module. It is not a problem for us to maintain this include file. What does change are things like the define statements at the top of each module: THIS_MODULE_PURPOSE, THIS_MODULE_KEYS, THIS_MODULE_NEEDS. For that reason we need a script to create the module structure. However, if we have the list of modules in that static file, the script would be much simplified since we can use the static file for the list of current modules and hence source files.
The question is could we make this a Cmake build thing: No gmt_core_module.c file in git but built on the fly.

[joa-quim]

since you are not calling things in gmt_core_*.c etc.

Yes, I am. I have a mbgmt_module.c|h. I have more MB modules with keys that use in Mirone.

But I have restored them. Now it builds again ... till next

[PaulWessel]

I know you have mbgmt_module.c|h but none of those are calling things in gmt_core_module.c, right?

[PaulWessel]

I would like to break up the gmt_supplements_module.c|h stuff since it is an artificial collection of some supplemental dirs. To simplify life for other supplements (mg, gsfml) it would be better if each supplement had these things and that cmake helped. We can still place stuff in a single supplements.so for the GMT approved supplements, but not make the code part special via gmt_supplements_*.c|h.

[PaulWessel]

Take that partially back: I think the only good solution for supplemental dirs will result in individual so/.dll files. If so then we are truly liberated from maintaining gmt_supplements_*.? files in the core directory.

[joa-quim]

No, they don't. But things were not working and didn't know why. Perhaps it was only the BUILD_EXTRA_DIRS thing.

[PaulWessel]

OK, I will use this branch to create modules.h files for core and for each supplement dir, then a small script that builds the glue.

[PaulWessel]

Hi @seisman, each of the supplement CMakeLists.txt files have lines like this:

set (SUPPL_NAME gsfml)

I would like to add the same to the main src file with name "core", and then have gmt_shared.h.in files in each source directory containing statements like this:

EXTERN_MSC void gmtlib_@SUPPLE_NAME@_module_show_all (void *API);

in order to automatically create the functions via cmake. Do you think the

configure_file (gmt_shared.h.in gmt_shared.h)

in each directory (src, src/*) properly configure and build the correct sources? I am trying to find out that answer before doing all the work and really finding out...

[seisman]

Yes, configure_file can give exactly what you need.

[PaulWessel]

Here is my plan for eliminating users and developers from dealing with things like gmt_make_module_src.sh and the hundreds of lines of code it produces:

1. Each src dir should have a fixed include file called gmt_shared.h. It will list the prototypes for the modules in that directory, like for src:

EXTERN_MSC int GMT_psbasemap (void *API, int mode, void *args);
EXTERN_MSC int GMT_begin (void *API, int mode, void *args);
...

2. Each gmt_module.h file includes gmt_glue.h. It is auto-configured from gmt_glue.h.in and contains the four function prototypes needed to access the shared library; here is just one:

EXTERN_MSC void gmtlib_@SUPPLE_NAME@_module_show_all (void *API);

3. Each src dir also contains a file gmt_glue.c. It is configured from gmt_glue.c.in and it builds the functions exported by gmt_glue.h. Each gmt_glue.c file states #include gmt_moduleinfo.h which must be built from the parameters THIS_MODULE_PURPOSE, THIS_MODULE_KEYS, and THIS_MODULE_NEEDS and the list of modules in gmt_shared.h.

There are two steps that are nontrivial or unclear at this point.

• The building of gmt_moduleinfo.h per directory: This is currently a part of what gmt_make_module_src.sh does, but can it be recast as Cmake module? I can write this in bash first and then we can see how translatable it is. Cmake has lots of functions and string tools that FLorian used when he translated all the old code that maintained gmtmath.h.in (now gone).
• RIght now there is some unholy dependence of GMT core on the supplements prototypes: gmt_supplements_module.h is included by gmt_dev.h. But why? The MB or the GSFML prototypes are not included by gmt_dev.h, yet things are working fine. We need to break this chain.

Do @joa-quim or @seisman see anything flawed in this scheme? I think if this can work we avoid having to worry about running things like gmt_make_module_src.sh manually, and can much more easily document what it takes to make a new supplement dir (just copy gmt_shared.h and gmt_glue.?.in from any supplement and edit the first file.

[PaulWessel]

Yes, configure_file can give exactly what you need.

Perhaps it is possible to do

configure_file (../gmt_shared.h.in gmt_shared.h)

for the subdir CMakeLists.txt files so that there is no need to place a gmt_shared.h.in files (and gmt_shared.c.in) in each directory, only in src?

[seisman]

Yes, configure_file can give exactly what you need.

Perhaps it is possible to do

configure_file (../gmt_shared.h.in gmt_shared.h)

for the subdir CMakeLists.txt files so that there is no need to place a gmt_shared.h.in files (and gmt_shared.c.in) in each directory, only in src?

I'm a little confused. Are you talking about gmt_shared.h or gmt_glue.h?

[PaulWessel]

Sorry (names are changing in my head), above I mean gmt_glue.?.in since gmt_shared.h is a static file we maintain manually. The idea is to use the *.in files from src repeatedly in each subdir to make slightly different include files and C files.

[PaulWessel]

BTW, probably for similar reasons, I see this having snuck into src/CMakeLists.txt:

set (GMT_MBSYSTEM_SRCS gmt_mbsystem_module.c gmt_mbsystem_module.h)

That should not be necessary either.

[seisman]

Sorry (names are changing in my head), above I mean gmt_glue.?.in since gmt_shared.h is a static file we maintain manually. The idea is to use the *.in files from src repeatedly in each subdir to make slightly different include files and C files.

Yes, it's doable.

[PaulWessel]

OK, close to working but family insist on going swimming. Back in the afternoon. I committed all my changes to this branch in case you want to try to see. I still fail on a few externs but very close. No longer using gmt_core_module.?, but gmt_modules.h (static), gmt_glue.?.

[PaulWessel]

PS. I have not touched the supplements yet on this since the above scheme means breaking up supplements.so and the need to find those individual so files. So only core testing for now.

[seisman]

You didn't commit the gmt_shared.h file.

[joa-quim]

All julia tests pass again

[PaulWessel]

We are good to merge into master... Why is the WIP check in progress, @seisman ?

[seisman]

If SUPPL_NAME equals GMT_SUPPL_LIB_NAME then we append to the same moduleinfo.h file, otherwise we create one in the subdirectory instead? That way, this CMakeLists.txt file when used with the gsfml and similar supplements would build src/gsfml/gmt_gsfml_moduleinfo.h for us. Separately, the src/gmt_gsfml_glue.c is build by the GSFML CMakeLists.txt file from src/gmt_glue.c.in?

Do we still need this implemented?

[PaulWessel]

Not sure, maybe. There are three kinds of "supplements":

1. Official ones in GMT, like seis, spotter... These are built at the same time as GMT.
2. Unofficial ones, like gsfml, that take advantage of the GMT build machinery by setting a symbolic link newsuppl[?] to point to that supplements src dir. They are then also built at the same time as GMT.
3. Unofficial ones, like mbsystem, that has its own build system that expects GMT to be installed already.

See #3130 for some thoughts. For type 2 above to work, the idea was to use configure_file with gmt_glue.c.in and make src/newsuppl/gmt_gsfml_glue.c which means there has to be a gmt_gsfml_moduleinfo.h available. It would be slick if that was handled by GMT CMake, but if it adds to much complication... THen the 3rd option, and possibly the solution for type 3, is to let gmt --build-glue=supplementname create those pieces.

[PaulWessel]

I thought first of gmt-config, but it is a script that may not be ideal for all environments (e.g., Windows).

[seisman]

Are gmt_glue.c and gmt_moduleinfo.h required for unofficial supplements? As I understand it, if a supplement package doesn't have gmt_glue.c and gmt_moduleinfo.h, users still can access all modules in the supplement package, except that these modules are not listed in gmt --help or gmt --show-modules. Am I right?

[seisman]

The builds break after merging #3165 with following errors:

../../src/gmt_api.c:970:73: error: use of undeclared identifier 'GMT_SUPPL_LIB_NAME'
                                for (k = 0; list[k] && strncmp (list[k], GMT_SUPPL_LIB_NAME, strlen(GMT_SUPPL_LIB_NAME)); k++); /* Look for official supplements */
                                                                                                    ^
../../src/gmt_api.c:970:46: error: use of undeclared identifier 'GMT_SUPPL_LIB_NAME'
                                for (k = 0; list[k] && strncmp (list[k], GMT_SUPPL_LIB_NAME, strlen(GMT_SUPPL_LIB_NAME)); k++); /* Look for official supplements */
                                                                         ^
2 errors generated.

In commit d2016b4, you changed

for (k = 0; list[k] && strncmp (list[k], "supplements", 11U); k++);

to

for (k = 0; list[k] && strncmp (list[k], GMT_SUPPL_LIB_NAME, strlen(GMT_SUPPL_LIB_NAME)); k++);

[PaulWessel]

Yes, you are right. There are two things:

1. Visibility via gmt --help or gmt --show-modules. Without the glue functions we cannot discover modules in a shared library. You would need to know the module names and look up syntax online.
2. Building external interfaces. If GMT/MATLAB toolbox (or Julia I think) wanted to call some of these foreign modules and take advantage of matlab syntax, e.g.,

G = gmt ('surface', 'R0/30/0/30 -I0.1 mbdata.txt');  %grid a large MB dataset
plot = gmt ('mbgrdtiff', 'some options', G);  % Make tiff file via mbsystem

then we need to be able to access the KEYS for those modules in the library. Otherwise we cannot do the above since we would not know how to hook the G matrix into the mbgrdtiff command line options. This is the job of API functions GMT_Encode_Options and GMT_Expand_Option.

So that is why I want to encourage having the glue and making it super-painless for those developers to do this.

[PaulWessel]

Anything else for this branch you are thinking of, @seisman ?

[seisman]

This PR looks good to me.