fix: Partial fix for FetchContent

[LecrisUT]

Partial fix for FetchContent issue regarding:

CMake Error in CMakeLists.txt:
  Target "fdict" contains relative path in its INTERFACE_INCLUDE_DIRECTORIES:

Issue is that generator expression should not have quotation marks there. A few other issues here are:

• Use CMake_Fortran_MODULE_DIRECTORY instead of setting the specific target property. This makes it consistent with downstream user's configuration
• target_include_directories should be PUBLIC, not INTERFACE there. I.e. the library should be using it as well. This is a compiler dependent behaviour because some might be automatically be using Fortran_MODULE_DIRECTOR or not, and this addresses the issue of the latter

Note on the "partial" fix. This is because the install interface is not fixed yet according to #32

[zerothi]

Add quotation for paths with spaces

[LecrisUT]

See earlier disccussion

[zerothi]

What do you meen here? Currently all modules from fdict is exposed?

[LecrisUT]

The convention in C/C++ projects is to install only 1 header file per project with a unique name. In cases where multiple header files are needed (header only libraries for example), than all the header files are in a subdirectory with the unique name of the project. Otherwise when you install another project that has similar header file names, they will overwrite each other and become unusable.

Same convention applies here. There is a potential of overlap with another project that uses variable.mod for example. In this case, simply add both ${CMAKE_Fortran_MODULE_DIRECTORY} and ${CMAKE_Fortran_MODULE_DIRECTORY}/fdict to the target_include_directories(), then depending on which target_link_libraries, the appropriate modules are used.

[zerothi]

Ah, I see.

[zerothi]

add quotation, to ensure paths with spaces

[LecrisUT]

I have tested locally and the quotation does not appear to be necessary. In general I would avoid quotations where unnecessary (in this case the variable CMAKE_CURRENT_BINARY_DIR is already sanitized to be a string) because sometimes the quotations are taken to be literal (as you've seen here in the generator expression). My rule of thumb is to sanitize everything at the set(), option() level rather than where the variables are used.

[zerothi]

I see, this is something where CMake is horrible, it is hard to know why some of the expansions works seamlessly, and others are problematic... Hence my guarding was just to put quotation everywhere.

[LecrisUT]

I fully agree. This is a relic of the original cmake design, and although there are rumors that the lead devs are working on a more robust, non text-based interface for camke 4, these are here to stay :(.

I was doing the same thing until I had various things break down because of the quotation. I realized though that most cmake commands are designed to work on variables (e.g. the if() command), so I work on the assumption that most commands are designed and tested to work for unquoted formats, and just use the quotation when it is needed to enforce it.

[zerothi]

Thanks for your PR! Some minor details

[LecrisUT]

• I have removed some redundant CMAKE_CURRENT_BINARY_DIR, CMAKE_CURRENT_SOURCE_DIR to make the listsfile more readable. Those are implicit where I have removed them
• Added a bit more TODOs about modern practices.
• There are more issues that I want to address in a separate PR, e.g. using project(VERSION), incorrect usage of write_basic_package_version_file(COMPATIBILITY)

[zerothi]

I'll fix the remaining issues.

[zerothi]

* There are more issues that I want to address in a separate PR, e.g. using `project(VERSION)`, incorrect usage of `write_basic_package_version_file(COMPATIBILITY)`

Don't add the project(VERSION) it is annoying it doesn't allow arbitrary version specification. For instance with git-hashes.

And why is the write_basic_package_version_file wrong? That is how it is documented: https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.html#generating-a-package-version-file

[LecrisUT]

* There are more issues that I want to address in a separate PR, e.g. using `project(VERSION)`, incorrect usage of `write_basic_package_version_file(COMPATIBILITY)`

Don't add the project(VERSION) it is annoying it doesn't allow arbitrary version specification. For instance with git-hashes.

Hmm good point. But the version tells you about the release intent with e.g. major change telling you that api has breaking changes. Git hashes are only for development, but I can see why that can be useful. How about exporting that information manually. It is highly advised to use project(VERSION) because that sets and resets both PROJECT_VERSION and fdict_VERSION as well as do any sanitization so that this can be used by any other cmake projects downstream.

(Btw I have a cmake project to get and set the project version directly from git/git archive, compatible with python's setuptools_scm setup files)

And why is the write_basic_package_version_file wrong? That is how it is documented: https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.html#generating-a-package-version-file

That ony controls the cmake find_package(fdict 0.3) syntax, telling the user if the apis are compatible or not. It does not control the actual linkage of the libraries so in that sense it is abi independent. The library linkage is done by the os and the compiler and cmake has no say or way to enforce library version compatibility. Or in other words, COMPATIBILITY tells if the cmake projects are compatible, not if the compilers or libraries used are compatible.