Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GTEST_ROOT is not present if not building GTest #126

Closed
paskino opened this issue Jun 25, 2018 · 4 comments · Fixed by #659
Closed

GTEST_ROOT is not present if not building GTest #126

paskino opened this issue Jun 25, 2018 · 4 comments · Fixed by #659
Assignees

Comments

@paskino
Copy link
Contributor

paskino commented Jun 25, 2018

https://github.com/CCPPETMR/SIRF-SuperBuild/blob/master/SuperBuild/External_GTest.cmake#L75 will not set GTEST_ROOT rather GTEST_LIBRARIES.

@paskino paskino self-assigned this Jun 25, 2018
paskino added a commit that referenced this issue Jun 25, 2018
…nto conda

Added a bunch of conda recipes to build:
1. boost
2. gtest
3. ismrmrd
4. gadgetron
5. armadillo
6. stir (incomplete)

fixes #126
@paskino
Copy link
Contributor Author

paskino commented Jan 28, 2019

reading better this link it seems that FindGTest will (or better may???) set the GTEST_ROOT cached variable.

@KrisThielemans
Copy link
Member

I think GTEST_ROOT "may be set" by the user, not by the FindGTest.cmake. Could check on the source.

Using GTEST_LIBRARIES is fraught with difficulties as you don't know what's in there, an din any case can't be passed on.

This is a common problem in the SuperBuild: we need to ensure that "ours" is found if we build it, but otherwise we have to rely on usual CMake stuff.

We've tried mark_as_superbuild to work-around this, but it wouldn't cope with the USE_SYSTEM_*=OFF case where the user sets GTEST_ROOT.

I suppose the thing to do is to only pass GTEST_ROOT if it is actually set (either by us, or by the user). That'd need a change here for Gadgetron.

I think the best strategy is probably to create a variable GTEST_CMAKE_FLAGS in External_GTest.cmake which would be setting GTEST_ROOT if it needs to, and otherwise be empty, and use that in dependent packages. That should work for all packages I believe.

@paskino
Copy link
Contributor Author

paskino commented Jan 28, 2019

I'm struggling to understand the problem...

@KrisThielemans
Copy link
Member

maybe you're right. cases:

  • USE_SYSTEM_GTEST=ON: we set GTEST_ROOT
  • USE_SYSTEM_GTEST=OFF:
    • the user sets GTEST_ROOT, so we'll pass it on
    • the user doesn't set GTEST_ROOT, so we set it to empty when calling CMake for Gadgetron.

Actually seems fine. If you agree, please close this issue.

I still think that a strategy if introducing GTEST_CMAKE_FLAGS, STIR_CMAKE_FLAGS etc would be nice and clean, but that's for another issue then (if you agree with it, please create a new one)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants