test(3p/nix): Set up scaffolding & fix up tests for value-to-{json|xml} r/1369

Configures the CMake build to load & run the GoogleTest tests.

I (grfn) also updated this to get the tests running as part of the nix
derivation, which required defining our own manual configurePhase and
installCheckPhase, rather than depending on the one provided by stdenv.
Not doing this would cause cmake to attempt to *run* the tests as part
of the buildPhase, which wouldn't work because the dynamic libraries
hadn't been put into a place where the test executables knew where to
find them. We're not sure *why* this fixes it, and for some reason
fixing this also breaks the automatic behavior of nixpkgs of passing
-j$NIX_BUILD_CORES -l$NIX_BUILD_CORES to make, but that's eaasy enough
to fix manually in a preBuild

Paired-With: Griffin Smith <grfn@gws.fyi>
Change-Id: I79d61854a3ff47301cdce8a40c76820a97bdf901
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1240
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>

6 files changed, 97 insertions, 46 deletions

diff --git a/third_party/nix/CMakeLists.txt b/third_party/nix/CMakeLists.txt
index 4e6b209c3699..f796ccdee4d6 100644
--- a/third_party/nix/CMakeLists.txt
+++ b/third_party/nix/CMakeLists.txt
@@ -38,4 +38,12 @@ INSTALL(DIRECTORY corepkgs
     PATTERN "*.nix")
 INSTALL(FILES "${PROJECT_BINARY_DIR}/config.nix" DESTINATION "${CMAKE_INSTALL_DATADIR}/nix/corepkgs")
 
+# Conditionally run tests
+option(PACKAGE_TESTS "Build the tests" ON)
+if (PACKAGE_TESTS)
+  enable_testing()
+  find_package(GTest)
+  include(GoogleTest)
+endif()
+
 add_subdirectory(src)
diff --git a/third_party/nix/default.nix b/third_party/nix/default.nix
index f9989602143b..ad1ff5beebfb 100644
--- a/third_party/nix/default.nix
+++ b/third_party/nix/default.nix
@@ -52,6 +52,7 @@ in pkgs.llvmPackages.libcxxStdenv.mkDerivation {
     flex
     glog
     grpc
+    gtest
     libseccomp
     libsodium
     openssl
@@ -60,11 +61,37 @@ in pkgs.llvmPackages.libcxxStdenv.mkDerivation {
     xz
   ];
 
+  doCheck = false;
+  doInstallCheck = true;
+
   propagatedBuildInputs = with pkgs; [
     boost
     largeBoehm
   ];
 
+  configurePhase = ''
+    mkdir build
+    cd build
+    cmake .. \
+      -DCMAKE_INSTALL_PREFIX=$out \
+      -DCMAKE_BUILD_TYPE=Release \
+      -DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF \
+      -DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF \
+      -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON
+  '';
+
+  installCheckPhase = ''
+    export NIX_DATA_DIR=$out/share
+    make test
+  '';
+
+  preBuild = ''
+    if [ -n "$NIX_BUILD_CORES" ]; then
+      makeFlags+="-j$NIX_BUILD_CORES "
+      makeFlags+="-l$NIX_BUILD_CORES "
+    fi
+  '';
+
   # Forward the location of the generated Protobuf / gRPC files so
   # that they can be included by CMake.
   NIX_PROTO_SRCS = protoSrcs;
diff --git a/third_party/nix/src/CMakeLists.txt b/third_party/nix/src/CMakeLists.txt
index 05386fa2dd7c..3775333ee313 100644
--- a/third_party/nix/src/CMakeLists.txt
+++ b/third_party/nix/src/CMakeLists.txt
@@ -11,6 +11,10 @@ add_subdirectory(libstore)
 add_subdirectory(libmain)
 add_subdirectory(libexpr)
 
+if (PACKAGE_TESTS)
+  add_subdirectory(tests)
+endif()
+
 add_executable(nix)
 set_property(TARGET nix PROPERTY CXX_STANDARD 17)
 include_directories(${PROJECT_BINARY_DIR})
diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc
index f28b13c83d68..dd1b199d927d 100644
--- a/third_party/nix/src/libstore/store-api.cc
+++ b/third_party/nix/src/libstore/store-api.cc
@@ -446,7 +446,7 @@ PathSet Store::queryValidPaths(const PathSet& paths,
    responsibility of the caller to provide a closure. */
 std::string Store::makeValidityRegistration(const PathSet& paths,
                                             bool showDerivers, bool showHash) {
-  std::string s = s;
+  std::string s;
 
   for (auto& i : paths) {
     s += i + "\n";
diff --git a/third_party/nix/src/tests/CMakeLists.txt b/third_party/nix/src/tests/CMakeLists.txt
new file mode 100644
index 000000000000..0cbb8c7747e8
--- /dev/null
+++ b/third_party/nix/src/tests/CMakeLists.txt
@@ -0,0 +1,11 @@
+# -*- mode: cmake; -*-
+include_directories(${PROJECT_BINARY_DIR}) # for 'generated/'
+
+add_executable(value-to-json value-to-json.cc)
+target_link_libraries(value-to-json
+  nixexpr
+  nixstore
+  GTest::gtest_main
+)
+
+gtest_discover_tests(value-to-json)
diff --git a/third_party/nix/src/tests/value-to-json.cc b/third_party/nix/src/tests/value-to-json.cc
index 462d2fda0170..744698733415 100644
--- a/third_party/nix/src/tests/value-to-json.cc
+++ b/third_party/nix/src/tests/value-to-json.cc
@@ -1,12 +1,13 @@
-#include "value-to-json.hh"
+#include "libexpr/value-to-json.hh"
 
+#include <set>
 #include <sstream>
 
 #include <gtest/gtest.h>
 
-#include "store-api.hh"
-#include "value-to-xml.hh"
-#include "value.hh"
+#include "libexpr/value-to-xml.hh"
+#include "libexpr/value.hh"
+#include "libstore/store-api.hh"
 
 class ValueTest : public ::testing::Test {
  protected:
@@ -20,46 +21,46 @@ class XMLValueTest : public ValueTest {};
 
 namespace nix {
 
-class DummyStore : public Store {
+class DummyStore final : public Store {
  public:
   explicit DummyStore() : Store(Store::Params{}) {}
 
   std::string getUri() { return ""; }
-  virtual void queryPathInfoUncached(const StorePath&) {}
-  virtual void queryPathInfoUncached(
-      const StorePath&,
-      nix::Callback<std::shared_ptr<const nix::ValidPathInfo>>) noexcept {}
-  std::optional<StorePath> queryPathFromHashPart(const std::string& hashPart) {
-    return {};
-  }
-  StorePath addToStore(const std::string&, const std::string&,
-                       const StorePathSet&, nix::RepairFlag) {
-    return StorePath::dummy.clone();
-  }
-  StorePath addToStore(const std::string&, const Path&, bool, nix::HashType,
-                       nix::PathFilter&, nix::RepairFlag) {
-    return StorePath::dummy.clone();
+
+  void queryPathInfoUncached(
+      const Path& path,
+      Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept {}
+
+  Path queryPathFromHashPart(const std::string& hashPart) { return ""; }
+
+  Path addToStore(const std::string& name, const Path& srcPath,
+                  bool recursive = true, HashType hashAlgo = htSHA256,
+                  PathFilter& filter = defaultPathFilter,
+                  RepairFlag repair = NoRepair) {
+    return "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x";
   }
-  StorePath addTextToStore(const std::string&, const std::string&,
-                           const StorePathSet&, nix::RepairFlag) {
-    return StorePath::dummy.clone();
+
+  Path addTextToStore(const std::string& name, const std::string& s,
+                      const PathSet& references, RepairFlag repair = NoRepair) {
+    return "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x";
   }
 
-  void narFromPath(const StorePath&, Sink&) {}
-  void ensurePath(const StorePath&) {}
+  void narFromPath(const Path& path, Sink& sink) {}
 
-  BuildResult buildDerivation(const StorePath&, const BasicDerivation&,
-                              BuildMode) {
+  BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv,
+                              BuildMode buildMode = bmNormal) {
     return BuildResult{};
   }
+
+  void ensurePath(const Path& path) {}
 };
 
 TEST_F(JSONValueTest, null) {
   std::stringstream ss;
   Value v;
   PathSet ps;
-  auto store = std::make_shared<DummyStore>();
-  EvalState s({}, ref<Store>(store), false);
+  std::shared_ptr<Store> store = std::make_shared<DummyStore>();
+  EvalState s({}, ref<Store>(store));
 
   mkNull(v);
   printValueAsJSON(s, true, v, ss, ps);
@@ -69,7 +70,7 @@ TEST_F(JSONValueTest, null) {
 TEST_F(JSONValueTest, BoolFalse) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -81,7 +82,7 @@ TEST_F(JSONValueTest, BoolFalse) {
 TEST_F(JSONValueTest, BoolTrue) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -93,7 +94,7 @@ TEST_F(JSONValueTest, BoolTrue) {
 TEST_F(JSONValueTest, IntPositive) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -105,7 +106,7 @@ TEST_F(JSONValueTest, IntPositive) {
 TEST_F(JSONValueTest, IntNegative) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -117,7 +118,7 @@ TEST_F(JSONValueTest, IntNegative) {
 TEST_F(JSONValueTest, String) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -129,7 +130,7 @@ TEST_F(JSONValueTest, String) {
 TEST_F(JSONValueTest, StringQuotes) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -141,7 +142,7 @@ TEST_F(JSONValueTest, StringQuotes) {
 TEST_F(JSONValueTest, Path) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -153,7 +154,7 @@ TEST_F(JSONValueTest, Path) {
 TEST_F(JSONValueTest, PathNoCopy) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -174,7 +175,7 @@ TEST_F(XMLValueTest, null) {
   Value v;
   PathSet ps;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({}, ref<Store>(store), false);
+  EvalState s({}, ref<Store>(store));
 
   mkNull(v);
   printValueAsXML(s, true, true, v, ss, ps);
@@ -184,7 +185,7 @@ TEST_F(XMLValueTest, null) {
 TEST_F(XMLValueTest, BoolFalse) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -196,7 +197,7 @@ TEST_F(XMLValueTest, BoolFalse) {
 TEST_F(XMLValueTest, BoolTrue) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -208,7 +209,7 @@ TEST_F(XMLValueTest, BoolTrue) {
 TEST_F(XMLValueTest, IntPositive) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -220,7 +221,7 @@ TEST_F(XMLValueTest, IntPositive) {
 TEST_F(XMLValueTest, IntNegative) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -232,7 +233,7 @@ TEST_F(XMLValueTest, IntNegative) {
 TEST_F(XMLValueTest, String) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -244,7 +245,7 @@ TEST_F(XMLValueTest, String) {
 TEST_F(XMLValueTest, StringQuotes) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -260,7 +261,7 @@ TEST_F(XMLValueTest, StringQuotes) {
 TEST_F(XMLValueTest, Path) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;
 
@@ -276,7 +277,7 @@ TEST_F(XMLValueTest, Path) {
 TEST_F(XMLValueTest, PathNoCopy) {
   std::stringstream ss;
   auto store = std::make_shared<DummyStore>();
-  EvalState s({"."}, ref<Store>(store), false);
+  EvalState s({"."}, ref<Store>(store));
   Value v;
   PathSet ps;