EventBus icon indicating copy to clipboard operation
EventBus copied to clipboard

Add CPack support

Open Aang23 opened this issue 5 years ago • 15 comments

Is your feature request related to a problem? Please describe. On Linux system it's always easier and cleaner to install everything through the package manager, rather than "make install".

Describe the solution you'd like The solution I would consider to be the best, since you're already using CMake, is using CPack to build .rpm & .deb files, using the "make package" command.

Describe alternatives you've considered Alternatives I considered are, evidently, running "make install", or building those package myself. Those ways works, but the first one isn't that clean or recommended, while the second's only issue is not being "official", which would be a shame since it's quite easy to implement.

CMake & CPack documentation

Aang23 avatar Apr 09 '19 18:04 Aang23

Thanks I will look into this in near future. Of course PR are welcome :)

gelldur avatar Apr 09 '19 18:04 gelldur

So expect a PR soon !

Aang23 avatar Apr 09 '19 19:04 Aang23

#24 pull request is done :) Thx a lot!

Need to improve:

  • [Nice to have] Format CMakeLists.txt via Clion (need to publish some autoformater / Clion formater?)
  • [Must have] New line on end
  • [Must have] Update contact & vendor
  • [Must have] Consider what if someone doesn't have CPack (should it be surrounded as option ?)
  • [Nice to have] Maybe split file so CPack could be in e.g. EventBus_CPack.cmake (Is file to long ?)
  • [Nice to have] Add travis section ?

gelldur avatar Apr 10 '19 06:04 gelldur

You're welcome !

If you've got a custom formatting configuration, yes, publishing it could be quite useful. My main IDE isn't Clion, but I got installed just it case, so that's not an issue.

CPack is bundled with CMake, even on Windows systems, however an option to disable / enable CPack totally, and enabling / disabling specific package types, could be a great plus. Would you prefer that to be done through something like cmake -DCPAK_PACKAGES, or CPACK_PACKAGE_RPM, CPACK_PACKAGE_DEB, etc ? (I already implemented an ENABLE_CPACK option)

I'll add a travis section and split the file too, but where would you like the EventBus_CPack.cmake file to be ? (Maybe lib/cmake)

Aang23 avatar Apr 10 '19 19:04 Aang23

My format of cmake isn't so important. As you can see there is already .clang-format so simply I want everything to be autoformated.

  • lib/cmake sounds ok! Thanks
  • ENABLE_CPACK is good enough I think
  • Thanks for the info about CPack (never used intentionally)

Of course you change can be merged in current state as it is ok. I add approve already. I didn't merge it only because lack of time to test it by my own (just for fun) and I would like to add those above improvements so I don't require them from you :) Of course, if you want to improve it, you are welcome :) If you have better ideas, those are also welcome. I'm open to discussion.

gelldur avatar Apr 11 '19 07:04 gelldur

I formatted EventBus_CPack.cmake through Clion. I kept the package types configuration in the top level CMakeList. I think that will be enough for now, CPack already disables package that requires missing tools to be built. If you want travis to be able to build all currently defined package types, you'll have to install the "rpm" package (at least on ubuntu).

Aang23 avatar Apr 11 '19 08:04 Aang23

Thanks! It looks great :) I will try it out & merge on the evening :+1:

gelldur avatar Apr 11 '19 08:04 gelldur

Need to update:

  • EventBusDev to EventBus (different target ?)
  • "RPM;DEB;TGZ" should be passed by variable ? E.g. on my system I don't have rpmbuild
  • Update README about how to do this
  • Add Travis section + upload it somewhere maybe ?
  • Check how other open source projects are dealing with "RPM;DEB;TGZ"

gelldur avatar Apr 11 '19 19:04 gelldur

Well since I moved the CPack config in the top level cmake file, it takes its config there, I'll move it back to the EventBus (/lib) CMake. I could also check if rpmbuild / etc is present on the system ? If you want to distribute the packages it needs to be built for each distribution, preferably (unless that's very similar ones like Ubuntu / Debian).

Aang23 avatar Apr 12 '19 10:04 Aang23

I didn't thought about distribution this small lib but why not to try & learn something new :)

gelldur avatar Apr 12 '19 10:04 gelldur

I just moved the CPack option back into /lib's CMakeLists, now everything works fine. Here's what would be installed by one of those packages (The built project still appears to be EventBusDev, but it is only since it's the project defined in the top cmake file). By default CPack will only build TGZ / TZ / STGZ, and sometime depending on the distribution, also for the package manager.

/usr/local/include/eventbus
/usr/local/include/eventbus/AsyncEventBus.h
/usr/local/include/eventbus/EventBus.h
/usr/local/include/eventbus/EventCollector.h
/usr/local/include/eventbus/TokenHolder.h
/usr/local/include/eventbus/internal
/usr/local/include/eventbus/internal/AsyncCallbackVector.h
/usr/local/include/eventbus/internal/CallbackVector.h
/usr/local/include/eventbus/internal/TransactionCallbackVector.h
/usr/local/include/eventbus/internal/common.h
/usr/local/lib64/cmake
/usr/local/lib64/cmake/EventBus
/usr/local/lib64/cmake/EventBus/EventBusConfig.cmake
/usr/local/lib64/cmake/EventBus/EventBusConfigVersion.cmake
/usr/local/lib64/cmake/EventBus/EventBusTargets-noconfig.cmake
/usr/local/lib64/cmake/EventBus/EventBusTargets.cmake
/usr/local/lib64/libEventBus.a

Aang23 avatar Apr 12 '19 10:04 Aang23

So, what I ended up doing is adding set(CPACK_GENERATOR "" CACHE STRING "Set packages CPack should build") in the top CMakeLists, and removing any reference to it in the enable_cpack() function & /lib's CmakeLists. Now the package generator can be configured through : cmake .. -DCPACK_GENERATOR="DEB"

That's also what most other projects using CPack uses.

I'll make another pull request for those changes.

Aang23 avatar Apr 12 '19 11:04 Aang23

I think issue is generally solved but I will leave it for some time to think about Travis upload or other package distribution. (Maybe add as AUR ?)

gelldur avatar Apr 21 '19 12:04 gelldur

The only thing I would change is removed the ${CPACK_GENERATOR} argument in enable_cpack(), as passing ${CPACK_GENERATOR} into enable_cpack() is quite useless, since all it used for is setting ${CPACK_GENERATOR}. That would be great if Travis could at least build the package. An AUR could be quite useful, and I was also thinking about a Copr repository. Launchpad may also be an option for APT.

Aang23 avatar Apr 23 '19 13:04 Aang23

Yeah you are right enable_cpack should be renamed, but passing what to build to function looks ok for me. Of course I would like add this support to travis but need some time after work to do that :)

gelldur avatar Apr 24 '19 12:04 gelldur