find ruby gems & make coverage conditional on BUILD_COVERAGE
ClosedPublic

Authored by sitter on Mar 18 2019, 2:39 PM.

Details

Summary

new finding tech:

  • find_gem function configures gem-specific FindFoo files and wraps around find_package
  • FindFoo files look for ruby and then attempt to require the gem name i.e. it checks if the gem can be successfully loaded by the interpreter
  • since this is based on find_package it may be influenced in all the regular ways (e.g. forced found or disabled from finding altogether via CMAKE_DISABLE_FIND_PACKAGE_$PKGNAME)

various notes for the future:

  • technically this isn't 100% correct because the require name of a gem and the gem name may not be the same. e.g. the gem docker-api has the require name docker. for all currently used gems the names are however the same and so simply requiring the gem name is expected to work
  • the implementation doesn't care about versions, again because we don't need it to
  • test-unit is a bundled gem, some distributions (e.g. Arch) do split it out without making suitable dependency arrangements on a package level though

the tech is heavily inspired by Aleix Pol's tech for finding QML modules
as seen in extra-cmake-modules

Test Plan
  • ruby not found: none of the modules found
  • test-unit not present: error
  • simplecov not prseent: never errors
  • having a module installed or not is reported in the cmake output

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Mar 18 2019, 2:39 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptMar 18 2019, 2:39 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
sitter requested review of this revision.Mar 18 2019, 2:39 PM
elvisangelaccio added inline comments.
src/tests/CMakeLists.txt
8

Why REQUIRED? This gem is only needed for the test, right?

Would it make sense to make it optional and only run the test if the gem is found?

sitter added inline comments.Mar 19 2019, 10:48 AM
src/tests/CMakeLists.txt
8

My thinking is that its part of the test suite, so unless you build with BUILD_TESTING=OFF you must have the test run when you ctest the project. It also prevents accidentally not running the tests on CI (missing dep, broken ruby etc.).
Seeing as I am not the maintainer of dolphin I can also make it optional if that is preferred though :P

elvisangelaccio accepted this revision.Mar 20 2019, 7:25 PM
elvisangelaccio added inline comments.
src/tests/CMakeLists.txt
8

Good point. Go for it :)

This revision is now accepted and ready to land.Mar 20 2019, 7:25 PM
This revision was automatically updated to reflect the committed changes.

@sitter @bcooksley It seems the freebsd CI node doesn't have the test-unit gem, can we install it?

-- The following REQUIRED packages have not been found:
09:39:21  
09:39:21   * Gem:test-unit, Ruby gem 'test-unit' required for testing of servicemenu helpers.
09:39:21  
09:39:21  CMake Error at /usr/local/share/cmake/Modules/FeatureSummary.cmake:459 (message):
09:39:21    feature_summary() Error: REQUIRED package(s) are missing, aborting CMake
09:39:21    run.

@tcberner has sorted this out now.