Make libsolv wrappers into standalone library (#3181)

* Move solv-cpp into standalone library

* Remove mamba namespace from solv-cpp

* Switch solv-cpp to object library

* Install solv-cpp object library

* Fix doctest

* Fix windows path

.github/workflows/unix_impl.yml

@@ -73,6 +73,9 @@ jobs:
         with:
           environment-file: ./build/environment.lock
           environment-name: build_env
+      - name: Run solv-cpp tests
+        run: |
+          ./build/libmamba/ext/solv-cpp/tests/test_solv_cpp
       - name: Run libmamba tests
         run: |
           unset CONDARC  # Interferes with tests

.github/workflows/windows_impl.yml

@@ -79,6 +79,10 @@ jobs:
           environment-file: ./build/environment.lock
           environment-name: build_env
           init-shell: bash cmd.exe
+      - name: Run solv-cpp tests
+        shell: bash -elo pipefail {0}
+        run: |
+          ./build/libmamba/ext/solv-cpp/tests/test_solv_cpp
       - name: Run libmamba tests
         shell: bash -elo pipefail {0}
         run: |

Taskfile.dist.yml

@@ -154,6 +154,22 @@ tasks:
     cmds:
       - '"{{.TEST_MAMBA_EXE}}" {{.CLI_ARGS}}'
 
+  _test-solv-cpp:
+    internal: true
+    deps: [{task: _build, vars: {target: 'test_solv_cpp'}}]
+    dir: ''
+    cmds:
+      - '{{.CMAKE_BUILD_DIR}}/libmamba/ext/solv-cpp/tests/test_solv_cpp {{.args}}'
+  test-solv-cpp:
+    desc: Run `libmamba` C++ based tests.
+    summary: |
+      Run fast C++ tests of the libsolc C++ wrappers. Running this command will rebuild only
+      `solv-cpp` and its tests, so compilation error may still happen when building other targets.
+      Test options can be passed as extra command line arguments:
+
+          task test-solv-cpp -- --test-suite='util::*'
+    cmds: [{task: '_test-solv-cpp', vars: {args: '{{.CLI_ARGS}}'}}]
+
   _test-libmamba:
     internal: true
     deps: [{task: _build, vars: {target: 'test_libmamba'}}]

libmamba/CMakeLists.txt

@@ -176,13 +176,6 @@ set(
     ${LIBMAMBA_SOURCE_DIR}/specs/unresolved_channel.cpp
     ${LIBMAMBA_SOURCE_DIR}/specs/version.cpp
     ${LIBMAMBA_SOURCE_DIR}/specs/version_spec.cpp
-    # C++ wrapping of libsolv
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/pool.cpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/queue.cpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/repo.cpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/solvable.cpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/solver.cpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/transaction.cpp
     # Solver generic interface
     ${LIBMAMBA_SOURCE_DIR}/solver/helpers.cpp
     ${LIBMAMBA_SOURCE_DIR}/solver/problems_graph.cpp
@@ -393,18 +386,6 @@ set(
     ${LIBMAMBA_INCLUDE_DIR}/mamba/api/update.hpp
 )
 
-set(
-    LIBMAMBA_PRIVATE_HEADERS
-    # C++ wrapping of libsolv
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/queue.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/ids.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/pool.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/solvable.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/solver.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/repo.hpp
-    ${LIBMAMBA_SOURCE_DIR}/solv-cpp/transaction.hpp
-)
-
 # Targets and link
 # ================
 
@@ -417,6 +398,7 @@ find_package(yaml-cpp REQUIRED)
 find_package(reproc REQUIRED)
 find_package(reproc++ REQUIRED)
 find_package(Libsolv REQUIRED)
+add_subdirectory(ext/solv-cpp)
 
 macro(libmamba_create_target target_name linkage output_name)
     string(TOUPPER "${linkage}" linkage_upper)
@@ -426,10 +408,7 @@ macro(libmamba_create_target target_name linkage output_name)
 
     # Output
     # ======
-    add_library(
-        ${target_name}
-        ${linkage_upper} ${LIBMAMBA_PUBLIC_HEADERS} ${LIBMAMBA_PRIVATE_HEADERS} ${LIBMAMBA_SOURCES}
-    )
+    add_library(${target_name} ${linkage_upper} ${LIBMAMBA_PUBLIC_HEADERS} ${LIBMAMBA_SOURCES})
 
     # Header only libraries are always linked the same way
     target_link_libraries(${target_name} PUBLIC tl::expected nlohmann_json::nlohmann_json)
@@ -457,6 +436,7 @@ macro(libmamba_create_target target_name linkage output_name)
                 simdjson::simdjson_static
                 solv::libsolv_static
                 solv::libsolvext_static
+                solv::cpp
         )
 
         if(UNIX)
@@ -604,6 +584,7 @@ macro(libmamba_create_target target_name linkage output_name)
                 zstd::libzstd_shared
                 solv::libsolv
                 solv::libsolvext
+                solv::cpp
         )
     endif()
 

libmamba/ext/solv-cpp/CMakeLists.txt

@@ -0,0 +1,47 @@
+# Copyright (c) 2024, QuantStack and Mamba Contributors
+#
+# Distributed under the terms of the BSD 3-Clause License.
+#
+# The full license is in the file LICENSE, distributed with this software.
+
+cmake_minimum_required(VERSION 3.16)
+
+add_library(
+    solv-cpp OBJECT
+    src/pool.cpp src/queue.cpp src/repo.cpp src/solvable.cpp src/solver.cpp src/transaction.cpp
+)
+target_include_directories(
+    solv-cpp
+    PUBLIC $<INSTALL_INTERFACE:include> $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
+)
+
+find_package(tl-expected REQUIRED)
+find_package(Libsolv REQUIRED)
+
+if(BUILD_SHARED)
+    set(LIBSOLV_DEPS solv::libsolv solv::libsolvext)
+else()
+    set(LIBSOLV_DEPS solv::libsolv_static solv::libsolvext_static)
+endif()
+
+target_link_libraries(solv-cpp PUBLIC tl::expected ${LIBSOLV_DEPS})
+target_compile_features(solv-cpp PUBLIC cxx_std_17)
+
+mamba_target_add_compile_warnings(solv-cpp WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
+mamba_target_set_lto(solv-cpp MODE ${MAMBA_LTO})
+
+add_library(solv::cpp ALIAS solv-cpp)
+
+if(BUILD_LIBMAMBA_TESTS)
+    add_subdirectory(tests)
+endif()
+
+# Object libraries are installed as an interface library (in libmambaTargets) but do not install any
+# objects (.o files) or headers without the ``OBJETS DESTINATION`` property.
+install(
+    TARGETS solv-cpp
+    EXPORT ${PROJECT_NAME}Targets
+    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
+    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
+)

libmamba/ext/solv-cpp/include/solv-cpp/ids.hpp

@@ -9,7 +9,7 @@
 
 #include <solv/pooltypes.h>
 
-namespace mamba::solv
+namespace solv
 {
     using StringId = ::Id;
     using DependencyId = ::Id;

libmamba/ext/solv-cpp/include/solv-cpp/pool.hpp

@@ -22,7 +22,7 @@
 #include "solv-cpp/repo.hpp"
 #include "solv-cpp/solvable.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     /**
      * Pool of solvable involved in resolving en environment.

libmamba/ext/solv-cpp/include/solv-cpp/queue.hpp

@@ -11,7 +11,7 @@
 
 #include <solv/queue.h>
 
-namespace mamba::solv
+namespace solv
 {
     /**
      * A ``std::vector`` like structure used in libsolv.
@@ -147,6 +147,5 @@ namespace mamba::solv
         return C<value_type>(begin(), end());
     }
 
-}  // namespace mamba
-
+}
 #endif

libmamba/ext/solv-cpp/include/solv-cpp/repo.hpp

@@ -18,12 +18,7 @@
 #include "solv-cpp/ids.hpp"
 #include "solv-cpp/solvable.hpp"
 
-namespace mamba::fs
-{
-    class u8path;
-}
-
-namespace mamba::solv
+namespace solv
 {
 
     class ObjRepoViewConst

libmamba/ext/solv-cpp/include/solv-cpp/solvable.hpp

@@ -20,7 +20,7 @@ extern "C"
     using Solvable = struct s_Solvable;
 }
 
-namespace mamba::solv
+namespace solv
 {
     /**
      * We use solvable for all sort of things, including virtual packages and pins.

libmamba/ext/solv-cpp/include/solv-cpp/solver.hpp

@@ -30,7 +30,7 @@ extern "C"
     using Solver = struct s_Solver;
 }
 
-namespace mamba::solv
+namespace solv
 {
     class ObjPool;
     class ObjQueue;

libmamba/ext/solv-cpp/include/solv-cpp/transaction.hpp

@@ -17,7 +17,7 @@
 #include "solv-cpp/pool.hpp"
 #include "solv-cpp/queue.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     class ObjPool;
     class ObjSolver;

libmamba/ext/solv-cpp/src/pool.cpp

@@ -17,7 +17,7 @@
 
 #include "solv-cpp/pool.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     void ObjPool::PoolDeleter::operator()(::Pool* ptr)
     {

libmamba/ext/solv-cpp/src/queue.cpp

@@ -7,7 +7,6 @@
 
 #include <algorithm>
 #include <cassert>
-#include <exception>
 #include <limits>
 #include <sstream>
 
@@ -15,7 +14,7 @@
 
 #include "solv-cpp/queue.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     ObjQueue::ObjQueue(std::nullptr_t)
     {
@@ -300,6 +299,4 @@ namespace mamba::solv
     {
         return !(a == b);
     }
-
-
-}  // namespace mamba
+}

libmamba/ext/solv-cpp/src/repo.cpp

@@ -20,7 +20,7 @@ extern "C"  // Incomplete header in libsolv 0.7.23
 
 #include "solv-cpp/repo.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     /****************************************
      *  Implementation of ConstObjRepoView  *

libmamba/ext/solv-cpp/src/solvable.cpp

@@ -17,7 +17,7 @@
 
 #include "solv-cpp/solvable.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     /********************************************
      *  Implementation of ConstObjSolvableView  *

libmamba/ext/solv-cpp/src/solver.cpp

@@ -17,7 +17,7 @@
 #include "solv-cpp/queue.hpp"
 #include "solv-cpp/solver.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     auto enum_name(::SolverRuleinfo rule) -> std::string_view
     {

libmamba/ext/solv-cpp/src/transaction.cpp

@@ -13,7 +13,7 @@
 #include "solv-cpp/solver.hpp"
 #include "solv-cpp/transaction.hpp"
 
-namespace mamba::solv
+namespace solv
 {
     void ObjTransaction::TransactionDeleter::operator()(::Transaction* ptr)
     {

libmamba/ext/solv-cpp/tests/CMakeLists.txt

@@ -0,0 +1,25 @@
+# Copyright (c) 2024, QuantStack and Mamba Contributors
+#
+# Distributed under the terms of the BSD 3-Clause License.
+#
+# The full license is in the file LICENSE, distributed with this software.
+
+cmake_minimum_required(VERSION 3.16)
+
+add_executable(
+    test_solv_cpp
+    src/main.cpp
+    src/pool_data.cpp
+    src/test_pool.cpp
+    src/test_queue.cpp
+    src/test_repo.cpp
+    src/test_scenarios.cpp
+    src/test_solvable.cpp
+    src/test_solver.cpp
+    src/test_transaction.cpp
+)
+target_include_directories(test_solv_cpp PRIVATE src/)
+target_compile_definitions(test_solv_cpp PRIVATE DOCTEST_CONFIG_USE_STD_HEADERS)
+
+find_package(doctest REQUIRED)
+target_link_libraries(test_solv_cpp PRIVATE doctest::doctest solv::cpp)

libmamba/ext/solv-cpp/tests/src/main.cpp

@@ -0,0 +1,3 @@
+#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
+
+#include <doctest/doctest.h>

libmamba/ext/solv-cpp/tests/src/pool_data.cpp

@@ -8,7 +8,7 @@
 
 #include "pool_data.hpp"
 
-namespace mamba::test
+namespace solv::test
 {
     namespace
     {
@@ -61,5 +61,4 @@ namespace mamba::test
         solv.add_self_provide();
         return solv_id;
     }
-
 }

libmamba/ext/solv-cpp/tests/src/pool_data.hpp

@@ -15,7 +15,7 @@
 #include "solv-cpp/pool.hpp"
 #include "solv-cpp/repo.hpp"
 
-namespace mamba::test
+namespace solv::test
 {
     struct SimplePkg
     {
@@ -67,7 +67,5 @@ namespace mamba::test
         }
         return out;
     }
-
-
 }
 #endif

libmamba/ext/solv-cpp/tests/src/test_pool.cpp

@@ -16,9 +16,7 @@
 #include "solv-cpp/ids.hpp"
 #include "solv-cpp/pool.hpp"
 
-#include "doctest-printer/vector.hpp"
-
-using namespace mamba::solv;
+using namespace solv;
 
 TEST_SUITE("solv::ObjPool")
 {

libmamba/ext/solv-cpp/tests/src/test_queue.cpp

@@ -4,7 +4,6 @@
 //
 // The full license is in the file LICENSE, distributed with this software.
 
-#include <exception>
 #include <list>
 #include <stdexcept>
 #include <vector>
@@ -13,7 +12,7 @@
 
 #include "solv-cpp/queue.hpp"
 
-using namespace mamba::solv;
+using namespace solv;
 
 TEST_SUITE("solv::ObjQueue")
 {
@@ -86,7 +85,8 @@ TEST_SUITE("solv::ObjQueue")
         CHECK_EQ(q.at(0), q[0]);
         CHECK_EQ(q.at(1), q[1]);
         CHECK_EQ(q.at(2), q[2]);
-        CHECK_THROWS_AS(q.at(q.size()), std::out_of_range);
+        auto use_at = [&]() { [[maybe_unused]] auto const& x = q.at(q.size()); };
+        CHECK_THROWS_AS(use_at(), std::out_of_range);
     }
 
     TEST_CASE("clear")

libmamba/ext/solv-cpp/tests/src/test_repo.cpp

@@ -6,16 +6,48 @@
 
 #include <algorithm>
 #include <array>
+#include <chrono>
+#include <filesystem>
+#include <string>
 
 #include <doctest/doctest.h>
 
-#include "mamba/core/util.hpp"
-#include "mamba/util/cfile.hpp"
 #include "solv-cpp/pool.hpp"
 #include "solv-cpp/repo.hpp"
 
-using namespace mamba::solv;
-using namespace mamba;
+using namespace solv;
+
+/** Current timestamp in seconds. */
+auto
+timestamp() -> std::string
+{
+    const auto seconds = std::chrono::duration_cast<std::chrono::seconds>(
+        std::chrono::system_clock::now().time_since_epoch()
+    );
+    return std::to_string(seconds.count());
+}
+
+/** Unique  temporary directory deleted after tests. */
+struct TmpDir
+{
+    std::filesystem::path path;
+
+    TmpDir(std::filesystem::path path_)
+        : path(std::move(path_))
+    {
+        std::filesystem::create_directories(path);
+    }
+
+    TmpDir()
+        : TmpDir(std::filesystem::temp_directory_path() / "solv-cpp/tests" / timestamp())
+    {
+    }
+
+    ~TmpDir()
+    {
+        std::filesystem::remove_all(path);
+    }
+};
 
 TEST_SUITE("solv::ObjRepo")
 {
@@ -158,14 +190,15 @@ TEST_SUITE("solv::ObjRepo")
 
             SUBCASE("Write repo to file")
             {
-                auto dir = mamba::TemporaryDirectory();
-                const auto solv_file = dir.path() / "test-forge.hpp";
+                // Using only C OS encoding API for test.
+                auto dir = TmpDir();
+                const auto solv_file = (dir.path / "test-forge.solv").string();
                 {
-                    auto fptr = util::CFile::try_open(solv_file, "wb");
-                    REQUIRE(fptr);
-                    const auto written = repo.write(fptr->raw());
+                    std::FILE* fptr = std::fopen(solv_file.c_str(), "wb");
+                    REQUIRE_NE(fptr, nullptr);
+                    const auto written = repo.write(fptr);
                     REQUIRE(written);
-                    REQUIRE(fptr->try_close());
+                    REQUIRE_EQ(std::fclose(fptr), 0);
                 }
 
                 SUBCASE("Read repo from file")
@@ -176,11 +209,11 @@ TEST_SUITE("solv::ObjRepo")
 
                     // Create new repo from file
                     auto [repo_id2, repo2] = pool.add_repo("test-forge");
-                    auto fptr = util::CFile::try_open(solv_file, "rb");
-                    REQUIRE(fptr);
-                    const auto read = repo2.read(fptr->raw());
+                    std::FILE* fptr = std::fopen(solv_file.c_str(), "rb");
+                    REQUIRE_NE(fptr, nullptr);
+                    const auto read = repo2.read(fptr);
                     REQUIRE(read);
-                    REQUIRE(fptr->try_close());
+                    REQUIRE_EQ(std::fclose(fptr), 0);
 
                     CHECK_EQ(repo2.solvable_count(), n_solvables);
                     // True because we reused ids

libmamba/ext/solv-cpp/tests/src/test_scenarios.cpp

@@ -9,14 +9,13 @@
 
 #include "solv-cpp/pool.hpp"
 #include "solv-cpp/queue.hpp"
-#include "solv-cpp/repo.hpp"
 #include "solv-cpp/solver.hpp"
 #include "solv-cpp/transaction.hpp"
 
 #include "pool_data.hpp"
 
-using namespace mamba::solv;
-using namespace mamba::test;
+using namespace solv;
+using namespace solv::test;
 
 TEST_SUITE("solv::scenariso")
 {

libmamba/ext/solv-cpp/tests/src/test_solvable.cpp

@@ -10,7 +10,7 @@
 #include "solv-cpp/repo.hpp"
 #include "solv-cpp/solvable.hpp"
 
-using namespace mamba::solv;
+using namespace solv;
 
 TEST_SUITE("solv::ObjSolvable")
 {

libmamba/ext/solv-cpp/tests/src/test_solver.cpp

@@ -4,20 +4,16 @@
 //
 // The full license is in the file LICENSE, distributed with this software.
 
-#include <string>
-#include <vector>
-
 #include <doctest/doctest.h>
 #include <solv/solver.h>
 
 #include "solv-cpp/pool.hpp"
-#include "solv-cpp/repo.hpp"
 #include "solv-cpp/solver.hpp"
 
 #include "pool_data.hpp"
 
-using namespace mamba::solv;
-using namespace mamba::test;
+using namespace solv;
+using namespace solv::test;
 
 TEST_SUITE("solv::ObjSolver")
 {

libmamba/ext/solv-cpp/tests/src/test_transaction.cpp

@@ -17,8 +17,8 @@
 
 #include "pool_data.hpp"
 
-using namespace mamba::solv;
-using namespace mamba::test;
+using namespace solv;
+using namespace solv::test;
 
 TEST_SUITE("solv::ObjTransaction")
 {

libmamba/include/mamba/solver/libsolv/database.hpp

@@ -19,6 +19,11 @@
 #include "mamba/specs/package_info.hpp"
 #include "mamba/util/loop_control.hpp"
 
+namespace solv
+{
+    class ObjPool;
+}
+
 namespace mamba
 {
     namespace fs
@@ -26,11 +31,6 @@ namespace mamba
         class u8path;
     }
 
-    namespace solv
-    {
-        class ObjPool;
-    }
-
     namespace specs
     {
         class MatchSpec;

libmamba/include/mamba/solver/libsolv/unsolvable.hpp

@@ -14,14 +14,14 @@
 
 #include "mamba/solver/problems_graph.hpp"
 
+namespace solv
+{
+    class ObjSolver;
+}
+
 namespace mamba
 {
     class Palette;
-
-    namespace solv
-    {
-        class ObjSolver;
-    }
 }
 
 namespace mamba::solver::libsolv

libmamba/tests/CMakeLists.txt

@@ -12,15 +12,6 @@ set(
     LIBMAMBA_TEST_SRCS
     include/mambatests.hpp
     src/test_main.cpp
-    # C++ wrapping of libsolv
-    src/solv-cpp/pool_data.cpp
-    src/solv-cpp/test_pool.cpp
-    src/solv-cpp/test_queue.cpp
-    src/solv-cpp/test_repo.cpp
-    src/solv-cpp/test_scenarios.cpp
-    src/solv-cpp/test_solvable.cpp
-    src/solv-cpp/test_solver.cpp
-    src/solv-cpp/test_transaction.cpp
     # Utility library
     src/util/test_cast.cpp
     src/util/test_compare.cpp
@@ -104,12 +95,11 @@ target_include_directories(
 
 find_package(doctest REQUIRED)
 find_package(Threads REQUIRED)
-find_package(Libsolv REQUIRED)
 
 target_link_libraries(
     test_libmamba
     PUBLIC mamba::libmamba reproc reproc++
-    PRIVATE doctest::doctest Threads::Threads solv::libsolv solv::libsolvext
+    PRIVATE doctest::doctest Threads::Threads
 )
 
 # Copy data directory into binary dir to avoid modifications

libmamba/tests/src/test_main.cpp

@@ -1,2 +1,3 @@
 #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
-#include "doctest/doctest.h"
+
+#include <doctest/doctest.h>