Fixing linking order and flags for mdx

[piyushrpt]

Fixes #155

1. disable openmp flag explicitly for mdx
2. linking order of libraries - I would like to use the nice target management but that seems to make a mess of the order in which mdx expects libs.

This PR should probably be updated with changes to TargetX11 and TargetMotif. Feel free to push to this branch and make changes.

[piyushrpt]

Is there a better solution? The solution here does not change affect "X11::" targets at all. It explicitly sets up "X11isce::" and uses it. So, this will not get in the way of anything else expecting "X11::"

[rtburns-jpl]

I like that you gave your specialized targets different names but I'd like the X11:: targets to still exist for use in other targets if needed in the future. Since we only need them for mdx, why not something like this inside contrib/mdx/CMakeLists.txt?

add_library(isce2::motif IMPORTED INTERFACE)
target_link_libraries(isce2::motif INTERFACE
        X11::X11
        Motif::Motif
        X11::Xt)
target_link_libraries(mdx PRIVATE isce2::motif)

But taking a step back, I still feel there is a still undiagnosed underlying issue behind this and the CMAKE_Fortran_FLAGS being set improperly. I'd like to see if there is a more idiomatic solution for this but still haven't been able to reproduce your errors. Do you have a dockerfile or more details about your environment I can use to recreate this?

[piyushrpt]

The Fortran Flags is a conda issue (where it is set by default) - their argument, why are you using fortran compilers in something meant for C/C++/Python. I think the solution above runs into same issue, the order wont be preserved in new cmake - due to interface_libraries being setup in FindX11.

Another option is to explicitly using X11_{COMPONENT}INCLUDE_PATH and X11{COMPONENT}_LIB directly without using the interfaces like @lijun99 mentioned.

[rtburns-jpl]

Yeah, are the fortran flags being set automatically in a conda build? I suspect this could be due to an environment variable and would like to repro this. Knowing what distro, packages, etc you're using would be a big help.

I'd be fine using the explicit FindX11 variables if we can't use the targets. But as far as I can tell the target_link_libraries command should preserve order, whether the arguments are library names or targets.

[piyushrpt]

conda build sets env variable FFLAGS that includes -fopenmp.
The order of libraries need to be necessarily preserved if the interfaces themselves define other interface_link_libraries

https://github.com/Kitware/CMake/blob/78df084c7abc9dfa2561b16f9d319c417a3cf0d5/Modules/FindX11.cmake#L724-L730

Then cmake's internal algorithm kicks in to try and resolve the linking order - which is fairly complicated from couple of blog posts that i came across.

Sounds like, using the FindX11 variables might be a good compromise that works across different versions of cmake.

[rtburns-jpl]

Agreed. That's weird to me that conda-forge sets all those variables, but I guess we're the weird case that simply enabling openmp breaks our build. I'd probably prefer that the -fopenmp is patched out in the recipe build.sh, but I'm fine with doing it here instead if that's more convenient.

I think you're right about the FindX11 variables. I just pushed a commit to motif-deps - let me know if that enforces linking order properly.

[rtburns-jpl]

Looks great! Thanks for taking the time to track this down