aurora-opensource / au

A C++14-compatible physical units library with no dependencies and a single-file delivery option. Emphasis on safety, accessibility, performance, and developer experience.
Apache License 2.0
329 stars 21 forks source link

Make building unit tests optional to simplify packaging #297

Closed ericriff closed 1 month ago

ericriff commented 1 month ago

I intent to package this library with Conan.

I don't want the conan package to depend on GTest, so instead of patching it out on the conan recipe I decided to propose this here instead.

I maintained the tests enabled by default, but a user can opt out of them with -DENABLE_TESTING:BOOL=OFF.

I don't think that gtest has any business on the installed targets. I disabled the find_package() call on the generated config file if we're building unit tests, but there are other references to gtest on the generated files. That's unusual. I think GTest should be treated as an internal tool, used for testing, and do not get propagated downstream.

-Eric

CLAassistant commented 1 month ago

CLA assistant check
All committers have signed the CLA.

chiphogg commented 1 month ago

Thanks for making this PR! Conan support has been a longstanding wishlist item of mine, and it's really exciting to see folks from the community stepping up to help make it happen. :rocket:

As I mentioned in the conversation for #215, we do have one target that legitimately depends on GTest. Not sure how we want to handle that here. As I also mentioned there, since we don't use CMake/conan on a day to day basis, there's a high prior probability of us just making weird choices out of ignorance. :slightly_smiling_face:

LMK your thoughts on some of our best options. I hope I'll get a chance to look more closely at this either later today or sometime in the next couple days.

chiphogg commented 1 month ago

Superseded by #303, which includes all of your commits here (and I've confirmed that your authorship shows up in the commit log in main).

Thanks a ton for your contributions @ericriff! It's because of your efforts (and those of @AbrilRBS) that Au is now finally available on conan. :sunglasses: