Inital support CMake build and Devcontainer configuration

[moschmdt]

@garfieldnate as discussed, this is the initial draft for CMake build without interfering with scons as far as possible.

This builds the soar library, excluding the SWIG interfaces and SVS, as well as the SoarCLI.
Additionally, the CMake build uses the conan package manager to fetch dependencies for all platforms, currently only SQLite3.

The only part where scons and CMake interfere is the following problem:
scons adds a file called build_time_date.h during the build process, which leads to failing builds for CMake. Is the build time still relevant that it should be behind a feature flag, is the feature rather deprecated and could be removed, or should this be replicated in CMake @garfieldnate?

[garfieldnate]

Wonderful, thank you very much!

Just to confirm, SWIG, SVS and SoarCLI are excluded not because they are impossible to build with CMake, but because they were not needed for the ROS2 package, correct?

I do think we need to generate the build_time_date.h file. I would also update it in the future to contain the commit SHA. I haven't tested this, but GPT suggested generation code that looks like this:

cmake_minimum_required(VERSION 3.16)
project(MyProject)

# ... other CMake code ...

# Get timestamp and Git SHA
string(TIMESTAMP CURRENT_TIMESTAMP "%Y-%m-%d %H:%M:%S")
execute_process(
  COMMAND git rev-parse HEAD
  WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
  OUTPUT_VARIABLE GIT_SHA
  OUTPUT_STRIP_TRAILING_WHITESPACE
  ERROR_QUIET
)

# Generate the version file
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/build_time_date.h.in" "${CMAKE_CURRENT_BINARY_DIR}/build_time_date.h")

# Add the generated header file to the include directories
include_directories("${CMAKE_CURRENT_BINARY_DIR}")

I will not have time to review this until December, but I'm very grateful for the contribution and look forward to improving the build system.

[garfieldnate]

Haven't reviewed it yet, but I do have a question: does this support both the SCU and non-SCU versions of the build? Or if only one, which one? The non-SCU version is supposed to be easier to debug (see #500). I'm not sure what the advantage of the SCU version is; perhaps compile time?

[moschmdt]

Just to confirm, SWIG, SVS and SoarCLI are excluded not because they are impossible to build with CMake, but because they were not needed for the ROS2 package, correct?

SWIG

SWIG is not built with CMake right now, but should be possible according to swig_add_library(..) CMake.

SVS

Not built.

SoarCLI

Already built with CMake, cf. SoarCLI/CMakeLists.txt as a subproject.

Unit Tests

I got them built with CMake but this is not ready to merge, thus not included in this PR.

SCU and non-SCU versions of the build

For now, this supports only the builds based on the cxx files. As far as I can tell, this means SCU compilation, since there was no benefit to collecting all source files individually for me. The same applies to shared vs. static library. The PR only adds the static library build for now - could be extended without much additional effort later if needed, e.g. for SWIG or Python bindings. I did some debugging and never had any debugging problems with the CMake VS Code extensions.

[moschmdt]

I do think we need to generate the build_time_date.h file.

Is there any specific reason why it is not tracked via git and generated during build time? The following code is from the generated build_time_date.h file, but the __TIME__ and __DATE__ all already macros replaced during build time. This looks to me like that was from a time before those macros existed?
If there are no opposing arguments, we could just add the file to the git repo and the problem is fixed? No further CMake required and SCons checks if the file is available - so no change needed here.

const char *kTimestamp = __TIME__;
const char *kDatestamp = __DATE__;
//* Last build of Soar 9.6.3 occurred at Fri Nov 22 17:20:20 2024 *//

[moschmdt]

I just had a little time to work on the Unit Tests. Currently 384/394 test are completed successfully. Most of the failing tests are SVS related. @garfieldnate should I include it in this PR?

Last test output:

https://github.com/moschmdt/Soar/actions/runs/12046432023/job/33586895049#step:11:1

[moschmdt]

• stdint.h header in Core/shared/msvs/ is installed and might lead to build issues in downstream projects. According to Generated msvs solution files do not appear to work #322 it might not be used any longer? (last edit according to git: 10 years ago)

[garfieldnate]

I just had a little time to work on the Unit Tests. Currently 384/394 test are completed successfully. Most of the failing tests are SVS related. @garfieldnate should I include it in this PR?

Last test output:

https://github.com/moschmdt/Soar/actions/runs/12046432023/job/33586895049#step:11:1

Sorry I missed that message before! Yes, would love to see it included. We do have to get the unit tests passing, but you'll see in our CI configuration that we disable a few of the tests.

[garfieldnate]

• stdint.h header in Core/shared/msvs/ is installed and might lead to build issues in downstream projects. According to Generated msvs solution files do not appear to work #322 it might not be used any longer? (last edit according to git: 10 years ago)

Sorry, I don't see a reference to stdint.h in that ticket. Could you be more specific?

[garfieldnate]

If there are no opposing arguments, we could just add the file to the git repo

That sounds fine to me, as well.

[moschmdt]

Sorry, I don't see a reference to stdint.h in that ticket. Could you be more specific?

#322 (comment)

Sorry I missed that message before! Yes, would love to see it included. We do have to get the unit tests passing, but you'll see in our CI configuration that we disable a few of the tests.

The same unit tests are running, except the testCommandToFile because they keep failing with the following error in this line:

Soar/UnitTests/SoarUnitTests/FullTests.cpp

Line 1668 in d3ce90c

no_agent_assertTrue(result == resultString);

result = 0x7fa228012650 "Error changing to ./SoarUnitTests//\nError while sourcing file\n"

SVS tests are disabled because building with CMake is not implemented. Might happen in a following PR.

Unfortunately, I neither have a Windows machine nor a direct use case for it right now. Maybe someone from the Sour group can take the initial starting points and fix the build local and in CI.

[garfieldnate]

Why did you choose to use Conan as the package manager? I think vcpkg is the other popular one. I'm not familiar with either, so I'm just trying to understand the choices and trade-offs here a bit.

[moschmdt]

We started to use Conan in other projects, and so far we have had good experience with it and the amount of available packages.

There are multiple blog posts about the pro and cons, e.g. matgomes.com or reddit (comparison with deprecated Conan v1.x).

[moschmdt]

@garfieldnate What's your opinion on this?

[garfieldnate]

I'm already long past the time I meant to look at this, but I really do plan on doing it this month! I want to approve and merge it and build on it, eventually migrating to it. I'm sorry to make you wait.

[garfieldnate]

• It's not totally clear from looking at the CI build file what commands I need to run locally, given all of the extra environment variables used there. I used cmake -B build, though, and it worked fine. I think it would be worth it to have some basic notes in the readme on how to build this yourself.

• I don't like that we have to do an install in order to run the unit tests; I don't know what the reason for the install being required was, though, so maybe that can be fixed.

• I did not get the same unit test results locally as you did in your linked build. Mine fails with this error:

  testSpreadingActivation_AlphabetAgentAllOn: SoarDB| Unexpected sqlite result! result = 21. error = 0 (not an error)
  SoarDB|...in SQL statement: SELECT lti_id FROM smem_invalid_parents
  Assertion failed: (return_val != err), function execute, file soar_db.h, line 163.

I don't know what would cause this, so it would take some time to track down. However, it's also possible that I just ran it incorrectly; you tested on Mac, which I'm using, so there must be some difference in my environment or how I've built it, right? Maybe my cmake configuration command above was incorrect?

Installing cmake and conan is very easy, so no issues there for me. I am struggling to understand a bit of the security story, though. It seems like we should be able to specify a cryptographic hash or something to check that the downloaded dependencies (so far only SQLite) are correct, right? But I don't see anything for that.

[moschmdt]

I think adding additional manual building instructions makes sense with links to the Conan and CMake documentation. Should we add it to the website as "How to build Soar with CMake" and link to that page or would you like to have this in this Soar source code repository?

The reason for installing is to copy the agent files to the install directory in a predefined location to the UniTests executable. Otherwise, path reconstruction from the source tree to the installation tree would be required. Based on my experience with CMake (mostly ROS 2) this is how additional files (configs, data, etc.) are placed to be used in binaries (out of source build). As far as I investigated scons, this is fairly similar to what you did with the "out" directory?

There are still some errors in the CI tests that look similar, I don't know if that is based on the SQLite dependency, CMake build process or other errors. As you said that needs to be investigated.

So far I built Soar with CMake on Apple Arm and Linux (Ubuntu) and did not experience any problems. Relying more on CMake presents might be an interesting way forward, especially considering your first point and this error.

[moschmdt]

I am struggling to understand a bit of the security story, though. It seems like we should be able to specify a cryptographic hash or something to check that the downloaded dependencies (so far only SQLite) are correct, right? But I don't see anything for that.

Conan is working on a package signing feature but it is only in preview, yet. More information can be found in this conan issue.

[garfieldnate]

Should we add it to the website as "How to build Soar with CMake" and link to that page or would you like to have this in this Soar source code repository?

I would rather have it in a new section in the repository readme (with a notice that it is a work in progress!). Experience tells me that it's very easy for development instructions on the website to become out-of-date. If you write some basic instructions out, I will give it another try (I'm on an M2 Mac).

As far as I investigated scons, this is fairly similar to what you did with the "out" directory?

You're right, I was mistaken about the meaning of "install". It appears that CMake places the files in the build directory. I thought that "install" meant it would go in some permanent place on my computer. This looks perfect as-is 👍.

The unfinished security story is not ideal for all Soar users, but I guess it doesn't have to be resolved immediately. I would want to do something about it before completely switching away from SCons, which wouldn't be for a while, anyway.

[moschmdt]

You're right, I was mistaken about the meaning of "install". It appears that CMake places the files in the build directory. I thought that "install" meant it would go in some permanent place on my computer. This looks perfect as-is 👍.

Only half way - cmake install can install to the system with the GNUInstallDirs; I changed the default behavior to install into an install directory in the project root file. Additionally, I changed the way required test files are handled. They are now copied in the build step. As a result ctest does not depend on the install step.

I would rather have it in a new section in the repository readme (with a notice that it is a work in progress!). Experience tells me that it's very easy for development instructions on the website to become out-of-date. If you write some basic instructions out, I will give it another try (I'm on an M2 Mac).

Added; I referenced my build script, which has all the necessary commands to build it and switch to a different build type, e.g. Release, etc.

[garfieldnate]

I somehow didn't notice the build.sh 🤦 Thanks for updating the readme, and adding the performance tests and improving the UnitTest step!

It always cleans the build directory and re-builds everything from scratch, which would be difficult in a development situation. I suppose the VSCode plugin will allow rebuilding just the parts that need it though, right?

Do I need to set an environment variable or anything to make the build succeed? Running build.sh gives me this output:

Install finished successfully
-- Using Conan toolchain: /Users/nathanglenn/dev/workspaces/c_workspace/Soar/build/Debug/generators/conan_toolchain.cmake
-- Conan toolchain: Defining libcxx as C++ flags: -stdlib=libc++
-- Conan toolchain: C++ Standard 17 with extensions ON
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Conan: Component target declared 'SQLite::SQLite3'
-- Soar: Disabling SVS
CMake Warning at CMakeLists.txt:110 (message):
  clang-tidy not found!


CMake Error at UnitTests/CMakeLists.txt:40 (file):
  file COPY cannot set permissions on
  "/Users/nathanglenn/dev/workspaces/c_workspace/Soar/build/UnitTests//SoarTestAgents":
  Operation not permitted.


-- Configuring incomplete, errors occurred!
make: Makefile: No such file or directory
make: *** No rule to make target `Makefile'.  Stop.
CMake Error: Error processing file: /Users/nathanglenn/dev/workspaces/c_workspace/Soar/build/cmake_install.cmake

[moschmdt]

It always cleans the build directory and re-builds everything from scratch, which would be difficult in a development situation. I suppose the VSCode plugin will allow rebuilding just the parts that need it though, right?

Yes, in order to catch build type switches, e.g. release to debug, a clean build is required. In case of recompilation, either using the VS code extension or uncommenting the rm -rf ... is possible to skip a full rebuild.

Do I need to set an environment variable or anything to make the build succeed? Running build.sh gives me this output:

I don't think so. I suspect the two // in your path could be a problem, but cannot replicate the issue. I added a potential fix. Could you please test again? What CMake version are you using?

[moschmdt]

I tested building on Linux (inside Docker) and Mac (M1) and it worked in both cases. Maybe you had some scons build files in one of the directories?

[garfieldnate]

I don't know why, but I had to manually delete the build directory this time around to get rid of the error. But it does work! Thank you.

Note for later: really wish we could pass arguments through the ctest command to the test exe. There's a feature request for it here: https://gitlab.kitware.com/cmake/cmake/-/issues/20470.

I pushed a fix for one of the failing tests, and TODO notes for the others that await further build implementation.

There appears to be one final error with /Wpedantic being used on Windows for the debug build, and then some kind of linking errors for the release build.

[moschmdt]

Perfect! Would be interesting to figure out why that happens at some point in the future.

Thanks for fixing the tests!

Do you have a Windows machine at hand to fix the failing build?

[garfieldnate]

I don't have a quick Windows machine to use, unfortunately... We may have to do the long way by pushing to CI and seeing what the results are :D

I think the issue is related to the use of the EXPORT preprocessor macro, which expands to __declspec(dllexport) if _USRDLL is defined, or to __declspec(dllimport) otherwise. In the SCons build, _USRDLL is defined only when building the shared library, not the static one. The CI build of Soar always builds the shared library, and I don't think I've ever tested making the static one.

So I guess it's possible that the static library build on Windows is just broken and we never noticed because no one has built it in a while 🤔 . The next step would be to try running the SCons build with --static on Windows.