glaze icon indicating copy to clipboard operation
glaze copied to clipboard

Use FetchContent_MakeAvailable for asio and eigen

Open kaladron opened this issue 1 year ago • 10 comments

CMake 3.30 deprecates FetchContent_Populate as CMP0169. FetchContent_MakeAvailable is already used in tests/CMakeLists.txt so this should not cause any problems for people on older versions of CMake.

Tested: Built and ran tests using default container with LLVM-19 and CMake 3.30.2 on x86_64 without eigen installed on the image.

kaladron avatar Aug 17 '24 08:08 kaladron

@kaladron, as you can see these changes break the CI tests. Any idea of how to fix this?

stephenberry avatar Sep 04 '24 13:09 stephenberry

Thanks, let me take a look. I only clicked the tests from inside vscode, so I'll try troubleshoot with the same command line

The "clang-linux / build (16, Debug, 23) (pull_request) Failing after 27s" seems to be the image not having f95, but I saw that on some others as well so I think it's not me.

kaladron avatar Sep 04 '24 20:09 kaladron

No, it's not you, I'm pretty sure it is Eigen

stephenberry avatar Sep 04 '24 20:09 stephenberry

OK, it turns out I was seeing the failures in vscode, vscode just doesn't give ANY notice on ctests failing. Sorry about that. I've adjusted this so that Eigen's tests aren't recursed into. I've used C-f to look for "Not run", and also "failed" to ensure all of them say "0 tests failed".

kaladron avatar Sep 07 '24 09:09 kaladron

Thanks for working on this. There are still some issues with Eigen, as you can see in the Action results:

CMake Error at tests/eigen_test/CMakeLists.txt:15 (set_target_properties):
  set_target_properties can not be used on an ALIAS target.

stephenberry avatar Sep 07 '24 15:09 stephenberry

I think I just need to delete the out/ directory as a test every time before I submit. =( I'm just running tests on a fix.

kaladron avatar Sep 07 '24 19:09 kaladron

Please take another look. Thank you!

kaladron avatar Sep 07 '24 22:09 kaladron

This is driving me nuts. =( It's midnight here, I'll look again in the morning.

kaladron avatar Sep 07 '24 22:09 kaladron

Thanks for pounding your head against this. If you're not able to make progress I can take a look at it. But, I really appreciate you going through this.

stephenberry avatar Sep 08 '24 00:09 stephenberry

No worries. I avoided learning cmake for so long, and I've recently switched to trying to embrace it. It's just frustrating when it builds, you run tests and get all green checkmarks, and then it fails in presubmit! =)

Especially as this is a nothing patch to avoid a warning with a newer minimum cmake. My actual goal is to export a modules interface which only needs cmake 3.28, not the 3.30 I thought - which is only needed for import std. But I'll need to figure this out to submit a bigger patch, so...

kaladron avatar Sep 08 '24 07:09 kaladron

I've removed FetchContent_Populate with #1650, so this pull request can now be closed.

stephenberry avatar Mar 13 '25 15:03 stephenberry