Giter Club home page Giter Club logo

Comments (8)

dennisklein avatar dennisklein commented on June 1, 2024

FairMQ tests now depend on GTest. You should be able to disable the building of (FairMQ) tests via the BUILD_TESTING cmake option from ctest, see https://cmake.org/cmake/help/v3.0/module/CTest.html. It is on by default.

from fairroot.

dennisklein avatar dennisklein commented on June 1, 2024

I have tried the BUILD_TESTING switch again to verify and it basically does not create the test target if switched off. In addition, FairMQ tests are not built as intended.

from fairroot.

ktf avatar ktf commented on June 1, 2024

IMHO you should be very careful when adding dependencies like this, breaking backward compatibility. As a user of FairRoot I would have expected that if GTest was not present, only the tests depending on GTest would have been disabled, not building the whole project.

from fairroot.

dennisklein avatar dennisklein commented on June 1, 2024

As a developer I will not take blame for breaking things in the dev branch... (Why do we have tools like git revert and alibuild init if we do not use them)

As a user I expect the FairRoot build system to be declarative about which components are built (cmake build switches).

The fact that targets are only built depending on the environment is in my opinion broken and dangerous. Unfortunately, as a single developer in a single PR I can only touch a limited number of issues..

from fairroot.

ktf avatar ktf commented on June 1, 2024

I will not comment on your first statement, since I am leaving for holidays tomorrow and I do not want to start a flame war the night before... ;-)

For what concerns the second part, being declarative is fine, I am perfectly happy to specify -DGTEST_ROOT=$GTEST_ROOT if I want the GTest part being built. Actually I think every single dependency should be explicit and there should not be any environment involved (see issue #513). However, if I do not specify a given external, all its dependencies should be turned off. Having to different places which control the same feature is dangerous and does not make any particular sense:

  • specifying GTEST_ROOT and BUILD_TEST=NO does not make any sense
  • not specifying GTEST_ROOT and BUILD_TEST=YES does not work

So I remain of my idea that if GTEST_ROOT is not set, you should not build the tests depending on GTEST.

from fairroot.

dennisklein avatar dennisklein commented on June 1, 2024

not specifying GTEST_ROOT and BUILD_TEST=YES does not work

Why not? FindGTest could have picked up GTest from system paths.

Having two different places which control the same feature is dangerous (...)

  • *_ROOT: optional path hints for find_package modules
  • BUILD_*: build switches with default values

I do not see why we should break this CMake idiom. I also do not see how it is dangerous.

If I set -DBUILD_TESTING=ON, I will expect always the full test suite to be built and not just a subset implicitely depending on the environment. Giving GTEST_ROOT the same semantics as BUILD_TESTING, as you suggest, is IMHO problematic, because, as you said yourself, not all tests might depend on GTest and it makes the GTEST_ROOT variable mandatory.

So I remain of my idea that if GTEST_ROOT is not set, you should not build the tests depending on GTEST.

You consider the O2 aliBuild environment only which is not the whole picture. I can see few rare usecases, where you might want to build depending on *_FOUND family of variables though, e.g. to provide alternative implementations.

from fairroot.

dennisklein avatar dennisklein commented on June 1, 2024

I guess this discussion has died. Therefore closing, but reopen any time.

from fairroot.

ktf avatar ktf commented on June 1, 2024

I meant to reply but never found the time. Yes, indeed, I meant GTEST_FOUND, not GTEST_ROOT. If GTEST_FOUND=NO building tests should be disabled by default, IMHO, especially if this was the previous behaviour.

from fairroot.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.