SFTtech / openage

Free (as in freedom) open source clone of the Age of Empires II engine 🚀
http://openage.dev
Other
12.6k stars 1.11k forks source link

Create Nyan.cmake for nyan related processing inside /buildsystem #1240

Open simonsan opened 4 years ago

simonsan commented 4 years ago

Currently the nyan integration is done here somehow. What about separating things from above to mentioned buildsystem/modules/FindNyan.cmake and call find_package(nyan REQUIRED CONFIG) from libopenage/CMakeLists.txt?

TheJJ commented 4 years ago

The find procedure relies on the modern cmake package registry. Thus we don't need a find-implementation, nyan registers itself properly. The only thing needed there is find_package, and since all other find_package invocations are in libopenage/CMakeLists.txt, the nyan one is there too.

simonsan commented 4 years ago

@jj: I'm talking about that: That is a bit more than "find_package" this is basically giving the whole instructions for getting the whole project. Imho that doesn't belong into a random CMakeLists.txt but rather into the buildsystem subfolderstructure, no?

It's the logic behind that, if it would be just a simple find_package construct that would be fine. But like this it's actually cluttered between different files instead of collecting everything regarding nyan at one place and including it from there.

# if this didn't work, we can download nyan like a git submodule.
# this is the treeish to be checked out.
if(NOT DEFINED NYAN_CLONE_VERSION)
    set(NYAN_CLONE_VERSION origin/master)
endif()

option(
    DOWNLOAD_NYAN
    "whether to clone the nyan project in case it is not found"
    OFF
)

option(
    FORCE_DOWNLOAD_NYAN
    "Force the download and usage of the nyan project"
    OFF
)

option(
    DISABLE_SUBPROJECT_UPDATES
    "Disable the automatic update of subprojects over the internet"
    OFF
)

# if nyan was not found, consider downloading it as subproject
# only use the subproject mode if it was requested
# or if it was used before.
if((NOT nyan_FOUND AND DOWNLOAD_NYAN) OR FORCE_DOWNLOAD_NYAN)
    message(STATUS "Downloading nyan as submodule project...")

    if(DISABLE_SUBPROJECT_UPDATES)
        set(DISABLE_NYAN_UPDATES "DISABLE_UPDATES")
    endif()

    fetch_project(
        NAME nyan
        ${DISABLE_NYAN_UPDATES}
        GIT_REPOSITORY https://github.com/SFTtech/nyan
        GIT_TAG ${NYAN_CLONE_VERSION}
    )

    # don't register nyan to the userpackage-repo!
    set(REGISTER_USERPACKAGE OFF)
    # don't generate the `doc` target again (name conflict!)
    set(DOXYGEN_ENABLE OFF)

    # register the targets
    add_subdirectory(${nyan_SOURCE_DIR} ${nyan_BINARY_DIR})

    message(STATUS "nyan processed successfully!")

elseif(NOT nyan_FOUND)
    message(FATAL_ERROR "
  Could not find the cmake package configuration file \"nyanConfig.cmake\".
  To find it, you have several options:
  * If your distribution provides it, install \"nyan\" through the package manager.
  * If you want openage to automatically download \"nyan\", append `-DDOWNLOAD_NYAN=YES` to the cmake invocation or use `./configure --download-nyan`.
  * If you want to build nyan manually, follow the build instructions:
      [[  doc/building.md#nyan-installation  ]]
  * If you already built nyan but it still can't be found (cmake package repo fails):
    * Try to set \"nyan_DIR\" to the nyan build directory (it contains nyanConfig.cmake)
      either through:  \"./configure $youroptions -- -Dnyan_DIR=/home/dev/nyan/build\"
      or:              \"cmake $yourotheroptions -Dnyan_DIR=/home/dev/nyan/build ..\"
  In case of other problems, please try to figure them out (and tell us what you did).
  Contact information is in README.md.
")
endif()
simonsan commented 4 years ago

Let's discuss this first and understand what I mean, before closing it directly. I think you misunderstood the point of this issue.

Addition: It could be that I was also a bit unclear with my title, for sure. I mean more to collect the stuff regarding nyan at one place. If it's not FindNyan.cmake because of the package registry, i think buildsystem/nyan.cmake for the download and fetch procedure and options could be nice and we include(nyan) from there. Like this we unclutter this is a bit, no?

(If you still think that this is not the case and we should keep it like this, feel free to close ^^ )

TheJJ commented 4 years ago

yea, we can create buildsystem/nyan.cmake. in the libopenage/cmakelists.txt we can call regular find_package(nyan) and if it's not found, call the download function that is now in buildsystem/nyan.cmake. Basically we extract the downloading mechanism and place it into a separate file.