simpler boost detection on mac

[peter-d]

Fix for #395, but not sure if there are no unwanted side effects, only tested on Ventura 13.1 on M1.

[peter-d]

I don’t mind! Do you know why the CXX compiler version is set to 10 here? If I can take that out, it becomes even simpler: no more delta vs non-Mac systems. Op 3 feb. 2023 om 17:38 heeft Knute Lingaard ***@***.***> het volgende geschreven: @knute-sifive commented on this pull request. In sparta/cmake/sparta-config.cmake:

endif ()

+find_package (Boost 1.65.0 REQUIRED COMPONENTS ${_BOOST_COMPONENTS}) If you don't mind, let's move this to 1.76 —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[ghost]

I tested on a few machines with ALL of that code removed -- and it worked, so yes! Please remove it all! Thanks!

[peter-d]

Hmm - the cleaner include path set up seems to break in CI, though it works in the systems I have access too (MacOS and WSL2 Ubuntu). Not sure how to debug that without access to a system where it fails.

@knute-sifive I also noticed my WSL Ubuntu does not yet have Boost 1.76, only 1.74. Not sure which version you like to pin to? I now updated the cmake to require 1.76, but that might be overly aggressive for some distros... Not sure how this lines up with the systems you typically run on.

[ghost]

I'm ok with an (slightly) older of boost if that makes standard environments work. I'm looking at the CI failures and it appears that an include path to simdb was dropped. Odd.

[ghost]

I'm building now...

I like the direction this PR is going, but one concern I have is with regards to the users of sparta. The purpose of the sparta/cmake/sparta-config.cmake file is so that modelers that use Sparta don't have to explicitly set up SYSTEM includes or library paths/targets based on what Sparta requires. Instead, a modeler just includes this cmake file and dependencies are automatically set up.

With that hope said, I think the real issue is that the sparta/cmake/sparta-config.cmake is incomplete. In other words, it finds packages for you, sets up library paths/linking, but it did not set up include paths. This is missing.

I have a branch where I address that. Can you take a look at it and merge it with yours? I'm not seeing the simdb issue...

https://github.com/sparcians/map/tree/knutel/peter-d_cmake_include_fixes

[ghost]

I started a new PR: #397 to see if my changes with yours pass CI.

[ghost]

Ok, I can reproduce the failure locally. Apparently I had simdb installed locally and the paths were automatically found! I found simdb in /usr/local/include. Guess I was playing around with installation packages at one time.

[ghost]

Fixed the simdb issue on my other branch.

[peter-d]

I'm building now...

I like the direction this PR is going, but one concern I have is with regards to the users of sparta. The purpose of the sparta/cmake/sparta-config.cmake file is so that modelers that use Sparta don't have to explicitly set up SYSTEM includes or library paths/targets based on what Sparta requires. Instead, a modeler just includes this cmake file and dependencies are automatically set up.

I fully agree - it should be as simple as an include and (maybe) a find_package (which will allow you down the line to select versions If needed)

We actually made an internal FindSparta.cmake that does pretty much this, and will set up Sparta and all dependencies, nicely in Sparta:: cmake namespace, so you can also easily just add Sparta to target_link_libraries statements and the like. I'll need to check if I am allowed to share that publicly - should not be an issue, normally, but just need to be sure. If you like, I can contribute that, either directly or as inspiration.

With that hope said, I think the real issue is that the sparta/cmake/sparta-config.cmake is incomplete. In other words, it finds packages for you, sets up library paths/linking, but it did not set up include paths. This is missing.

Correct, this is now indeed spread over 2 files, part in the config, part in the ```CMakeLists.txt`

I have a branch where I address that. Can you take a look at it and merge it with yours? I'm not seeing the simdb issue...

https://github.com/sparcians/map/tree/knutel/peter-d_cmake_include_fixes

I'll have a look right away!

[ghost]

We actually made an internal FindSparta.cmake that does pretty much this, and will set up Sparta and all dependencies, nicely in Sparta:: cmake namespace, so you can also easily just add Sparta to target_link_libraries statements and the like. I'll need to check if I am allowed to share that publicly - should not be an issue, normally, but just need to be sure. If you like, I can contribute that, either directly or as inspiration.

🤤

That would be awesome!!

[ghost]

I'm slowly picking away the regression failures in the other PR and I think (🤞🏻 ) I got it working.

But a fundamental question on the cmake philosophy: Sparta is a package that depends on many other packages. How do you ensure that an end user (the modeler) is using the same packages that Sparta was built with?

As an example, Sparta was built with Boost 1.71. But the modeler is using Boost 1.80. What magic keeps everything in sync? 🤔

[peter-d]

Superseded by #397