Fix SOURCE_DIR in HandleGTest.cmake

[olzhabay]

If benchmark added as cmake subproject, HandleGTest throws an error as ${CMAKE_SOURCE_DIR} does return absolute source dir.
Change it to ${CMAKE_CURRENT_SOURCE_DIR}, so it will be refering to it's own source dir.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

[olzhabay]

CLA signed

[googlebot]

CLAs look good, thanks!

[LebedevRI]

I think this is correct.

[coveralls]

Coverage remained the same at 89.224% when pulling 62809ea on olzhabay:master into 8503dfe on google:master.

[AppVeyorBot]

✅ Build benchmark 1521 completed (commit 15ead5e9da by @olzhabay)

[LebedevRI]

@olzhabay hm, thinking about it a bit more, you placed googletest into the benchmark/googletest?
And it is looking in yourproject'sroot/googletest.
I think the solution here is slightly incorrect.
I think there should be an option that will specify the path to the googletest directory.

[olzhabay]

@olzhabay hm, thinking about it a bit more, you placed googletest into the benchmark/googletest?
And it is looking in yourproject'sroot/googletest.
I think the solution here is slightly incorrect.
I think there should be an option that will specify the path to the googletest directory.

Yes, googletest is place in benchmark/googletest.
If it would be place outside of benchmark directory, then it cannot execute add_subdirectory.
Elif it fails to find googletest in benchmark subdir, it will simply download gtest or find it in among system installs.
What you think @LebedevRI ?

[LebedevRI]

If it would be place outside of benchmark directory, then it cannot execute add_subdirectory.

I'm not sure i follow. add_subdirectory() works with both the absolute and relative paths.
Currently; it expects googletest to be placed in the root of top-level project.
You expect it to look for googletest in it's own subdirectory.
Those are two expectations, and both of them might be the expected thing to happen.

I think the location where it looks for googletest should be a config option, thus any expectation can be valid. Also, i think you need to pass ${CMAKE_CURRENT_BINRY_DIR}/googletest as binary_dir into that add_subdirectory() call.

[EricWF]

This patch is correct.

CMAKE_SOURCE_DIR seems to always refer to the top level project directory,
while CMAKE_CURRENT_SOURCE_DIR refers to the most recent CMakeLists.txt being processed via add_subdirectory. Consider the following directory layout:

my_project/
  CMakeLists.txt
  benchmark/
    googletest/

Assuming my_project uses add_subdirectory(benchmark), then, when evaluating HandleGTest.cmake, the value of CMAKE_SOURCE_DIR will be my_project/ and CMAKE_CURRENT_SOURCE_DIR will be my_project/benchmark.

[LebedevRI]

I think i'm failing to convey my thoughts here.
The patch is correct if you, who use googlebenchmark as a subproject, expect for the googlebenchmark to look for the googletest in yourproject/googlebenchmark/googletest
The patch is not correct if you, who use googlebenchmark as a subproject, expect for the googlebenchmark to look for the googletest in yourproject/googletest.
I can see both cases being the expected behavior, thus i'm suggesting a config option.

[EricWF]

I think i'm failing to convey my thoughts here.

Ah, OK. Yes we could do a lot better in supporting other configurations for googletest. But that's a separate discussion, and we should file a bug to track it.

This patch is "correct" because it brings the implementation inline with the currently documented behavior, which says:

This dependency can be provided two ways:

• Checkout the Google Test sources into benchmark/googletest as above.
• Otherwise, if -DBENCHMARK_DOWNLOAD_DEPENDENCIES=ON is specified during configuration, > the library will automatically download and build any required dependencies.

I'm merging this patch to make the behavior inline with the documentation. Lets continue the broader discussion elsewhere.

[EricWF]

@olzhabay Could you re-open this with your latest changeset?

[olzhabay]

@LebedevRI Totally got your point. I added this option. So, now it can look to both myproject's dir and googlebenchmark's dir, if one of them has an googletest

[olzhabay]

@EricWF sure

[LebedevRI]

This patch is "correct" because it brings the implementation inline with the currently documented behavior, which says:

This dependency can be provided two ways:

• Checkout the Google Test sources into benchmark/googletest as above.
• Otherwise, if -DBENCHMARK_DOWNLOAD_DEPENDENCIES=ON is specified during configuration, > the library will automatically download and build any required dependencies.

I'm merging this patch to make the behavior inline with the documentation. Lets continue the broader discussion elsewhere.

Oh, i forgot about those docs. Ok, then that patch as it was is ok to go.

@LebedevRI Totally got your point. I added this option. So, now it can look to both myproject's dir and googlebenchmark's dir, if one of them has an googletest

Not really what i meant.
I was really talking about a config options, not hardcoding looking in two places.
But as @EricWF points out, that is not the scope of this PR.

[EricWF]

@olzhabay OK, so lets land your original fix first. And then we can explore improvements as a follow up PR.