eclipse / mosquitto

Eclipse Mosquitto - An open source MQTT broker
https://mosquitto.org
Other
8.91k stars 2.37k forks source link

manpages are not built on linux when using cmake #1184

Closed peterhoeg closed 3 years ago

peterhoeg commented 5 years ago

DOCUMENTATION is set by default to ON but still the man pages are not being built causing the install to fail. The workaround is to run this before starting the build:

    pushd man
    make
    popd

We do this on NixOS as we are building with cmake for uniformity.

karlp commented 5 years ago

Or, you know, use makefiles like you're directed to on linux.

ralight commented 5 years ago

If you build from the released tars then the man pages are already built as well.

peterhoeg commented 5 years ago

Or, you know, use makefiles like you're directed to on linux.

On NixOS we use a single expression to build for 32/64 linux as well as 64 bit darwin (using normal nix, obviously not nixos) so it's of great benefit to us if we can build using the same stack. And since the cmake build variant works perfectly fine on linux (except for this minor issue), that's what we use.

Is there any particular reason that cmake on linux shouldn't work?

And along the same lines, any particular reason you don't just use cmake by default regardless of platform instead of having to maintain to build systems?

If you build from the released tars then the man pages are already built as well.

We currently fetch based on the version tag from GH which indeed does come without the pregenerated man pages.

Would you accept a PR that generates the man pages when building with cmake?

ralight commented 5 years ago

Anybody can use whatever build system they want, on whichever platform, as long as they are aware that the makefiles are the canonical build system, and the cmake files are only directly supported for the purpose of making it easier to compile on Windows and Mac.

I use the makefiles every day, and that's not going to change. cmake adds too much friction for me when I frequently want to make a quick clone, build and test, for example :) By contrast, the cmake files get updated at least before a release, but possibly not for a while before. So they should work for Linux, but that's not always guaranteed. There are also gotchas, like the man pages not being built and the linker symbol versioning not being built into the library.

As long as building man pages in cmake doesn't break the mac or windows builds then, yes, I'd accept a PR for it.

peterhoeg commented 5 years ago

Thank you for taking the time to explain the situation. For now we will simply work around this on NixOS as that is already in place. I suggest we leave this open in case some enterprising soul with cmake experience comes by looking for a good issue to handle.

gdt commented 5 years ago

So if cmake is unofficial, is the makefile system going to be improved so that it is portable and does all the things that a build system is expected to do, like not have hardcoded paths that people have to change, check features, do things differently based on OS, etc.? As I see it mosquitto has a recommended build system which does not meet requirements and a cmake system which has serious bugs. Maybe autoconf :-)

wrmbaron commented 4 years ago

You can easily fix the manpage issue with a man/CMakeLists.txt file like noted below. Feel free to use that or modify it. If you want to, I can post a PR, if that is allowed.

set(outfiles "")

function(add_manpage file suffix)
    set(input "${CMAKE_CURRENT_SOURCE_DIR}/${file}.${suffix}")
    set(output "${CMAKE_CURRENT_BINARY_DIR}/${file}.${suffix}")
    add_custom_command(
        OUTPUT "${output}"
        COMMAND xsltproc --nonet "${input}.xml"
        DEPENDS "${input}.xml" "${input}.meta"
        WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
        VERBATIM)
    list(APPEND outfiles "${output}")
    set(outfiles "${outfiles}" PARENT_SCOPE)
    install(FILES "${output}" DESTINATION "${CMAKE_INSTALL_MANDIR}/man${suffix}")
endfunction()

add_manpage(libmosquitto 3)
add_manpage(mosquitto 8)
add_manpage(mosquitto.conf 5)
add_manpage(mosquitto_passwd 1)
add_manpage(mosquitto_pub 1)
add_manpage(mosquitto_rr 1)
add_manpage(mosquitto_sub 1)
add_manpage(mosquitto-tls 7)
add_manpage(mqtt 7)

add_custom_target(man ALL DEPENDS "${outfiles}")
peterhoeg commented 4 years ago

If you want to, I can post a PR, if that is allowed.

I think a PR would be a great idea.

ralight commented 3 years ago

This should be fixed now.