Fix CMake config filename

[dg0yt]

On case-sensitive filesystems, the default name enkiTS-config.cmake doesn't work. This PR implements the correct case-sensitive filename. (The case-insensitve name would be enkits-config.cmake.)

Could not find a package configuration file provided by "enkiTS" with any
  of the following names:

    enkiTSConfig.cmake
    enkits-config.cmake

[dougbinks]

Since I don't use cmake install, and since the install code was added by @pr0g in #96 I'm going to ask him if he can review this code as I can't test it myself.

[pr0g]

Thanks for tagging me @dougbinks. This looks reasonable to me, @dg0yt could you please include the platform you tested this on and repro steps to simulate the issue before/after. I would just like to verify the fix and double check there isn't a more idiomatic CMake way to resolve this (I'll be able to test this tomorrow). Thanks!

[dg0yt]

I tested with Linux, but any case-sensitive filesystem/OS is affected. You only need a plain

cmake_minimum_required(VERSION 3.1)
project(test)
find_package(enkiTS CONFIG REQUIRED)

Idiomatic is to use something like EXPORT enkits-targets, and generate+install a separate config file (and maybe version file) which has include("${CMAKE_CURRENT_LIST_DIR}/enkits-targets.cmake"). This config file can handle dependency lookup for transitive usage requirements. But is okay to keep it simple if there are no dependencies.

[pr0g]

That's great thank you @dg0yt! I think you're right, I'd just like to double check locally (out atm but will test tomorrow (Sunday UK time)). Thanks for the fix and sorry for the oversight on my part.

[pr0g]

Hey @dg0yt, apologies I just had time to test this.

I was able to reproduce the issue but found a slightly different solution that I think is preferable.

Instead of adding the new line FILE enkiTSConfig.cmake, I would instead modify lines 68 and 72 to be EXPORT enkits-config instead of EXPORT enkiTS-config.

 if( ENKITS_INSTALL )
     install(
         TARGETS enkiTS
-        EXPORT enkiTS-config
+        EXPORT enkits-config
         ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
     install(FILES ${ENKITS_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
     install(
-        EXPORT enkiTS-config
+        EXPORT enkits-config
         NAMESPACE enkiTS::
         DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/enkiTS)
 endif()

I think this should solve things in a slightly simpler way. Just to confirm I tested this with...

cd <your-dev-folder>
mkdir enkits-test # to be used later...
mkdir enkits
cd enkits
git clone https://github.com/dougbinks/enkiTS.git .
# ---
# apply changes shown above
# ---
cmake -B build -DCMAKE_INSTALL_PREFIX=../enkits-test/install -DENKITS_INSTALL=ON
cmake --build build --target install
cd ../enkits-test
touch CMakeLists.txt # see contents below
touch main.cpp # see contents below
cmake -B build -DCMAKE_PREFIX_PATH=install
cmake --build build # enkits is found as expected

# CMakeLists.txt
cmake_minimum_required(VERSION 3.1)
project(test)
find_package(enkiTS CONFIG REQUIRED)
add_executable(${PROJECT_NAME} main.cpp)
target_link_libraries(${PROJECT_NAME} PRIVATE enkiTS::enkiTS)

#include <TaskScheduler.h>

#include <cstdio>

int main(int argc, char** argv) {
  enki::TaskScheduler g_TS;
  g_TS.Initialize();

  enki::TaskSet task(
    1024, [](enki::TaskSetPartition range, uint32_t threadnum) {
      printf(
        "Thread %d, start %d, end %d\n", threadnum, range.start, range.end);
  });

  g_TS.AddTaskSetToPipe(&task);
  g_TS.WaitforTask(&task);

  return 0;
}

Let me know what you think. If you're able to make the update I proposed above and test it works for you that'd be great. I then thing @dougbinks can merge the PR. Thanks!

[dg0yt]

I know that changing the export name changes the default filename. But I can't see how it is preferable or (significantly) simpler. (One line, OMG!) It will need to be changed again as soon as the project needs a more elaborated config file (for components or dependencies).

I would like to point out that changing the filename to enkits-config.cmake will relax case requirements for the package name: It will match enkits, enkiTS, EnkiTS, eNKIts. And <nAmE>_FOUND will follow the requested name. This may be nice, or this may be a source for trouble... Given the target namespace, I would recommend enkiTSConfig.cmake which only matches find_package(enkiTS).

[pr0g]

I was not expecting that response @dg0yt, and I did not realize this was something you felt so strongly about.

If you want to only match on enkiTS, then switching to this has the same effect (for the reasons you described):

 if( ENKITS_INSTALL )
     install(
         TARGETS enkiTS
-        EXPORT enkiTS-config
+        EXPORT enkiTSConfig
         ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
     install(FILES ${ENKITS_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
     install(
-        EXPORT enkiTS-config
+        EXPORT enkiTSConfig
         NAMESPACE enkiTS::
         DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/enkiTS)
 endif()

If the install command did ever need to be made more sophisticated as you mention, the FILE command could be added then (I'll concede it is referenced in the official importing and exporting CMake guide, lending further weight to its use).

To be honest I don't mind either way, this is very much in @dougbinks's hands. I've given my two cents. Either approach is valid, I was coming at it from the perspective of updating what was there, rather than adding something new, but in the grand scheme of things it really isn't a big deal (maybe just make both changes to be completely consistent).

 if( ENKITS_INSTALL )
     install(
         TARGETS enkiTS
-        EXPORT enkiTS-config
+        EXPORT enkiTSConfig
         ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
     install(FILES ${ENKITS_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
     install(
-        EXPORT enkiTS-config
+        EXPORT enkiTSConfig
+        FILE enkiTSConfig.cmake
         NAMESPACE enkiTS::
         DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/enkiTS)
 endif()

[dougbinks]

I am not familiar with cmake install as I do not install libraries myself, but changing the export to enkiTSConfig as per @pr0g's request seems like a good approach, and I'm not against also explicitly stating the cmake file as well if you feel that's a good idea.

[dg0yt]

Now you must test. This branch only lives on GH.

[pr0g]

Tested locally with gh pr checkout 103.

I used the commands I listed earlier, works as expected.

[dougbinks]

Many thanks for this!

I've now merged this to dev and master.