Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How about all lowercase installation directories? #14

Open
schuhschuh opened this issue Dec 13, 2015 · 4 comments
Open

How about all lowercase installation directories? #14

schuhschuh opened this issue Dec 13, 2015 · 4 comments

Comments

@schuhschuh
Copy link
Member

On Unix, it is quite common to keep directory names all lowercase. Header files of modules are, however, installed in directories irtk/Common et al., i.e., mixing lowercase with CamelCase. It should be either IRTK/Common or irtk/common. For Unix, I prefer the latter. On Windows, I would actually choose the first. Given that NTFS is not case sensitive (at least not normally...), this wouldn't make a difference. But if a common case should be chosen for all platforms, I vote for all lowercase. Not saying that I am not open to use uppercase instead. But if we do use upper case letters, then let's use IRTK instead in directory names as well as library names (i.e., libIRTKCommon.so).

@ghisvail
Copy link
Member

I don't have a strong stance on one proposal or the other, since appropriate usage of CMake targets will render this cosmetic issue transparent to the user. A few things I'd like to point out:

  • The header files should not be installed blindly under /usr/include considering the large volume of them. Hence the need for a subdirectory.
  • Choice of whether the format should be lowercase or camelcase does not matter much in our case. It would matter if we were using namespaced includes, for instance #include <irtk/$MODULE/$HEADER>
  • The install path and naming of the CMake package files will matter, depending on whether we want the API to be find_package(IRTK COMPONENTS Common) or find_package(irtk COMPONENTS common).

@schuhschuh
Copy link
Member Author

Choice of whether the format should be lowercase or camelcase does not matter much in our case. It would matter if we were using namespaced includes, for instance #include <irtk/$MODULE/$HEADER>

That's my point. If headers are installed in a subdirectory, they should be included using such "namespaced" includes.

transparent to the user.

It's not actually transparent. You have to advocate a specific way how headers should be included and whether they have a file name prefix or not (cf. #15).

Either

#include <irtkHEADER.h>

or

#include <irtk/HEADER.h>
#include <irtk/MODULE/HEADER.h>

As mentioned in #15, when we install headers in an "irtk" subdirectory and switch to a "irtk" C++ namespace (which I think would be desirable), I clearly prefer the second approach because the include path should only contain the XXX/include/ directory itself and in a standard Linux installation, the /usr/include and /usr/local/include directories are already in the search path.

@schuhschuh
Copy link
Member Author

The install path and naming of the CMake package files will matter, depending on whether we want the API to be find_package(IRTK COMPONENTS Common) or find_package(irtk COMPONENTS common).

The capitalisation of the module name does in fact not matter to CMake. This you have to handle in your package file yourself. It's easy to just convert all module names to lowercase in the package configuration file included by find_package so a user can capitalise it as they wish. Only the package name (and directory containing the package configuration file) matters to some degree.

I vote for find_package(IRTK) because it's an acronym and referred to it as "IRTK" in texts. Even in this case you can have a irtk-config.cmake file, but I would indeed choose IRTKConfig.cmake given the CamelCase we use for source files (incl. CMakeLists).

To help find_package to find the IRTKConfig.cmake file, it should be either installed in <prefix>/lib/cmake, <prefix>/lib/cmake/IRTK or <prefix>/lib/cmake/irtk with the location be registered upon installation if the lowercase hinders CMake from finding it (never tested this) or when not installed in a standard <prefix> (optional registration users can disable). But even when we choose lowercase for all installation directories, we can still make a single exception for <prefix>/lib/cmake/IRTK if it makes it easier for CMake to find it as find_package(IRTK).

Also, it would be common practice to call all variables set by a package configuration file as <name>_VARIABLE, where the capitalisation of <name> should be identical to the find_package(<name>) call. This was not so in early CMake days, but for "modern" CMake. Thus, if we would choose find_package(irtk), our constants (variables) would be named irtk_VARIABLE which isn't nice... lowercase variable names in CMake are common for local variables in loops or functions, but shouldn't be used for (global) constants.

@schuhschuh
Copy link
Member Author

Also, it would be common practice to call all variables set by a package configuration file as _VARIABLE, where the capitalisation of should be identical to the find_package() call. This was not so in early CMake days, but for "modern" CMake.

After having had a look at many of the refactored CMake 3 Find modules, it seems CMake developers now (i.e., since version 3) made the choice to always capitalise the package name in the variable name prefix no matter how the Find module is called. So I guess that's what "modern" CMake prefers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants