-
Notifications
You must be signed in to change notification settings - Fork 129
Use CMake find macro instead of pulling in googletest as a submodule #402
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
Conversation
Picking up from #400 (comment), I wonder why gtest was giving you trouble on 22.04, since that's what our CI runs on? I'm unmarking this as a draft because I'd like to see what our CI does with it, though it'll need a prod to try running the tests. |
enable_testing() | ||
find_package(GTest REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgive me, I'm still trying to comprehend CMake - does this mean GTest maintains a file in the CMake distribution for finding it's install location on various architectures? Does this mean any project where find_package
is available has some distribution on Linux, Mac, and Windows? That would be pretty convenient - I wonder how many of our submodules have that?
The part of this change I have the feeling will be trickier is updating manifold.yml
to get our CI to install the required packages on each platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a search path which cmake will try to find the package file, similar to a compiler trying to search for header files. I think this is fine with gtest as it is widely used and is not a critical dependency for us. For other dependencies, I think the current submodule approach would be better as it allows us to control the version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@t-paul You tend to be good at explaining reasons for decoupling dependencies, adding you here in case you want to chip in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elalish CMake comes with a bunch of find macros. I believe these macros mostly deal with offering a way of interacting with the components of the package, namespaced library names etc. A lot of these macros will use a more structured approach, like pkg-config, to locate the actual software. Since these interact with binaries, it also doesn't matter which build system the dependencies were built with.
The caveat (there is always a caveat) is that on systems without decent options for package management, coughwindowscough, the find macros sometimes need help.
..but to get best of both worlds, build all your dependencies with separate CMake invocations, then use standard find macros (with necessary hints) to find the software, then it should be easy for anyone to build manifold binaries which suit their environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kintel Thanks for the explanation, that does indeed sound like the best of both worlds: basically allow use of the binary distro if available (and if it's version is compatible) and fall back to our submodule otherwise, but keep its CMake process somehow separated. I've never accomplished that - do you have examples? Preferably that demonstrate it working on a CI across Linux, Mac and Windows when the package isn't available on them all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of any good examples; a lot of this comes down to strategies for software distribution vs. CI vs. development, which may all have different goals and challenges.
I expect most of the pain of build systems and package management to happen once a project grows out of the "lab" and gets external users, so you don't necessarily have to solve all problems at once.
..but spending some time to learn and experiment with CMake is always a good thing :)
Also, I'm not saying that this will make things easier for you; it's about making it easier for downstream consumers, which invariably adds more work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, well I tried updating our manifold.yml
to include libgtest-dev
, but it seems to have only worked for the mac build. I have no idea why Ubuntu is angry, and I don't even know where to pull gtest for Windows. Any thoughts? Currently this is convincing me submodules is a better way to go...
For OpenSCAD, we use msys2 on Windows. It's a way of making Windows appear more friendly (i.e. Unix-like), and it comes with a package manager where we can install software in a structured fashion: https://github.com/openscad/openscad/blob/master/.github/workflows/windows.yml I've heard things about vcpkg as well, but it's rumored to be very slow. |
For the Ubuntu breakage, I cannot tell; looks like some interaction between gtest and glm |
As I say, I'd love to make our library easier to consume, and no one will know better how to do that than the users themselves. The only reason I didn't use CMake's |
I am revisiting this. What do you think about using system provided package when available, and use CMake |
Implemented in #523. |
Proof of concept @elalish
Prerequisites (Ubuntu):
sudo apt install libgtest-dev