Recent CMake change exposed build error for MacOS

[jhiemstrawisc]

It looks like a recent change to CMakeLists.txt by @GregThain changed the default behavior of the SCITOKENS_BUILD_UNITTESTS option, switching it from OFF by default to ON.

Pelican regularly builds scitokens-cpp on MacOS as part of our unit testing framework, and I think this change in default behavior introduced a build error for us. I haven't decided if this is an issue with Pelican or Scitokens, because I haven't been able to reproduce the error locally on my Mac, and I'm no CMake wizard.

Here's how we build/install the library:

git clone --depth=1 https://github.com/scitokens/scitokens-cpp.git
pushd scitokens-cpp
mkdir build
cd build
export SCITOKENS_CPP_DIR=$PWD/release_dir
cmake .. -GNinja -DCMAKE_INSTALL_PREFIX="$PWD/release_dir"
ninja install
sudo ln -s "$PWD"/release_dir/lib/libSciTokens*.dylib /usr/local/lib
popd

Prior to the CMake changes on 04/28/25 this worked fine, but now it yields the following error:

 + ninja install
ninja: error: 'external/gtest/src/gtest-build/lib/libgtest.a', needed by 'test/scitokens-gtest', missing and no known rule to make it

I confirmed this is tied to the SCITOKENS_BUILD_UNITTESTS option by modifying our build script to pass -DSCITOKENS_BUILD_UNITTESTS=OFF, which resulted in a successful build. Plus there's no reason we need to be building the unit tests.

I also pinned the version we use so we're not building off the tip of master.

[djw8605]

Interesting catch. I do think you are right, building unittest should not be the default. Thoughts? @GregThain ?

[GregThain]

It is your call -- if you want to turn it off, I'm fine with that. I guess my thinking was always building and linking the tests shows that you haven't broken interfaces with changes, but if googletest on mac is hard to come by, maybe we should just leave it off by default.

[djw8605]

ChatGPT says, and it's always right:

In C++ projects using CMake, the most common and widely recommended practice is:
Unit tests should NOT be built by default — they are typically built only when a special flag is enabled, such as -DBUILD_TESTING=ON or a custom flag like -DENABLE_TESTS=ON.

Reasons why tests are off by default:

1. Separation of concerns: Tests are for developers. End users or production builds don’t need them.
2. Minimize build time and dependencies: Some test frameworks (e.g., GoogleTest) may be large or require extra setup.
3. Cleaner builds for deployment or packaging: Builds meant for release don’t include testing artifacts.
4. Standardization: Tools like CTest and many open-source projects follow this convention using the BUILD_TESTING flag.

[GregThain]

Our benevolent overlord has spoken!

[bbockelm]

This probably indicates broken CMake, no? Deps should be resolved at config time, not build.

If you need inspiration, here's the solution I recently came up with:

https://github.com/PelicanPlatform/xrootd-s3-http/blob/main/CMakeLists.txt#L156