Merge branch 'feature/pkg-id-type' into develop

azure-pipelines.yml

           sudo apt install -y python3-minimal g++-9 ccache
           python3 -m pip install pytest pytest-xdist
         displayName: Prepare System
-      - script: python3 -u tools/ci.py -B download --cxx g++-9 -T tools/gcc-9.dds
+      - script: python3 -u tools/ci.py -B download -T tools/gcc-9.dds
         displayName: Full CI
       - publish: _build/dds
         artifact: DDS Executable - Linux
       - script: |
           set -eu
           python3 -m pip install pytest pytest-xdist
-          python3 -u tools/ci.py -B download --cxx g++-9 -T tools/gcc-9.dds
+          python3 -u tools/ci.py -B download -T tools/gcc-9.dds
         displayName: Build and Run Unit Tests
       - publish: _build/dds
         artifact: DDS Executable - macOS

src/dds/build.cpp

 #include "./build.hpp"
 
+#include <dds/build/plan/compile_exec.hpp>
 #include <dds/catch2_embedded.hpp>
 #include <dds/compdb.hpp>
 #include <dds/usage_reqs.hpp>
 #include <dds/util/time.hpp>
 #include <libman/index.hpp>
 #include <libman/parse.hpp>
-#include <dds/build/plan/compile_exec.hpp>
 
 #include <spdlog/spdlog.h>
 
         fs::create_directories(catch_hpp.parent_path());
         auto hpp_strm = open(catch_hpp, std::ios::out | std::ios::binary);
         hpp_strm.write(detail::catch2_embedded_single_header_str,
-                    std::strlen(detail::catch2_embedded_single_header_str));
+                       std::strlen(detail::catch2_embedded_single_header_str));
         hpp_strm.close();
     }
 
     }
 
     build_plan plan;
-    auto&      pkg = plan.add_package(package_plan(man.name, man.namespace_));
+    auto&      pkg = plan.add_package(package_plan(man.pk_id.name, man.namespace_));
 
     usage_requirement_map ureqs
         = load_usage_requirements(params.root, params.out_root, params.lm_index);

src/dds/dds.main.cpp

         int run() {
             auto list_contents = [&](dds::repository repo) {
                 auto same_name
-                    = [](auto&& a, auto&& b) { return a.manifest.name == b.manifest.name; };
+                    = [](auto&& a, auto&& b) { return a.manifest.pk_id.name == b.manifest.pk_id.name; };
 
                 auto all         = repo.iter_sdists();
                 auto grp_by_name = all                             //
                     | ranges::views::transform(ranges::to_vector)  //
                     | ranges::views::transform([](auto&& grp) {
                                        assert(grp.size() > 0);
-                                       return std::pair(grp[0].manifest.name, grp);
+                                       return std::pair(grp[0].manifest.pk_id.name, grp);
                                    });
 
                 for (const auto& [name, grp] : grp_by_name) {
                     spdlog::info("{}:", name);
                     for (const dds::sdist& sd : grp) {
-                        spdlog::info("  - {}", sd.manifest.version.to_string());
+                        spdlog::info("  - {}", sd.manifest.pk_id.version.to_string());
                     }
                 }
 

src/dds/deps.cpp

     auto insert_point = std::partition_point(sd.begin(), sd.end(), [&](const sdist& cand) {
         return cand.path < new_sd.path;
     });
-    if (insert_point != sd.end() && insert_point->manifest.name == new_sd.manifest.name) {
-        if (insert_point->manifest.version != new_sd.manifest.version) {
+    if (insert_point != sd.end()
+        && insert_point->manifest.pk_id.name == new_sd.manifest.pk_id.name) {
+        if (insert_point->manifest.pk_id.version != new_sd.manifest.pk_id.version) {
             assert(false && "Version conflict resolution not implemented yet");
             std::terminate();
         }
             throw std::runtime_error(
                 fmt::format("Unable to resolve dependency '{}' (required by '{}')",
                             dep.name,
-                            man.name));
+                            man.pk_id.to_string()));
         }
         resolve_ureqs_(rules, found->second.get().manifest, sd_idx);
         auto lib_src     = found->second.get().path / "src";
                            const sdist_index_type& sd_idx,
                            usage_requirement_map&  ureqs,
                            sdist_names&            already_added) {
-    if (already_added.find(sd.manifest.name) != already_added.end()) {
+    if (already_added.find(sd.manifest.pk_id.name) != already_added.end()) {
         // We've already loaded this package into the plan.
         return;
     }
-    spdlog::debug("Add to plan: {}", sd.manifest.name);
+    spdlog::debug("Add to plan: {}", sd.manifest.pk_id.name);
     // First, load every dependency
     for (const auto& dep : sd.manifest.dependencies) {
         auto other = sd_idx.find(dep.name);
         add_sdist_to_dep_plan(plan, other->second, env, sd_idx, ureqs, already_added);
     }
     // Record that we have been processed:
-    already_added.insert(sd.manifest.name);
+    already_added.insert(sd.manifest.pk_id.name);
     // Add the package:
-    auto& pkg  = plan.add_package(package_plan(sd.manifest.name, sd.manifest.namespace_));
+    auto& pkg  = plan.add_package(package_plan(sd.manifest.pk_id.name, sd.manifest.namespace_));
     auto  libs = collect_libraries(sd.path);
     for (const auto& lib : libs) {
         shared_compile_file_rules comp_rules = lib.base_compile_rules();
 
 build_plan dds::create_deps_build_plan(const std::vector<sdist>& deps, build_env_ref env) {
     auto sd_idx = deps  //
-        | ranges::views::transform(
-                      [](const auto& sd) { return std::pair(sd.manifest.name, std::cref(sd)); })  //
+        | ranges::views::transform([](const auto& sd) {
+                      return std::pair(sd.manifest.pk_id.name, std::cref(sd));
+                  })  //
         | ranges::to<sdist_index_type>();
 
     build_plan            plan;
     usage_requirement_map ureqs;
     sdist_names           already_added;
     for (const sdist& sd : deps) {
-        spdlog::info("Recording dependency: {}", sd.manifest.name);
+        spdlog::info("Recording dependency: {}", sd.manifest.pk_id.name);
         add_sdist_to_dep_plan(plan, sd, env, sd_idx, ureqs, already_added);
     }
     return plan;

src/dds/package_id.cpp

+#include <dds/package_id.hpp>
+
+#include <spdlog/fmt/fmt.h>
+
+#include <tuple>
+
+using namespace dds;
+
+package_id package_id::parse(std::string_view s) {
+    auto at_pos = s.find('@');
+    if (at_pos == s.npos) {
+        throw std::runtime_error(fmt::format("Invalid package ID string '{}'", s));
+    }
+
+    auto name    = s.substr(0, at_pos);
+    auto ver_str = s.substr(at_pos + 1);
+
+    return {std::string(name), semver::version::parse(ver_str)};
+}
+
+package_id::package_id(std::string_view n, semver::version v)
+    : name(n)
+    , version(std::move(v)) {
+    if (name.find('@') != name.npos) {
+        throw std::runtime_error(
+            fmt::format("Invalid package name '{}' (The '@' character is not allowed)"));
+    }
+}
+
+std::string package_id::to_string() const noexcept { return name + "@" + version.to_string(); }

src/dds/package_id.hpp

+#pragma once
+
+#include <semver/version.hpp>
+
+#include <string>
+#include <string_view>
+#include <tuple>
+
+namespace dds {
+
+struct package_id {
+    std::string     name;
+    semver::version version;
+
+    package_id() = default;
+    package_id(std::string_view s, semver::version v);
+
+    static package_id parse(std::string_view);
+    std::string       to_string() const noexcept;
+
+    auto tie() const noexcept { return std::tie(name, version); }
+
+    friend bool operator<(const package_id& lhs, const package_id& rhs) noexcept {
+        return lhs.tie() < rhs.tie();
+    }
+    friend bool operator==(const package_id& lhs, const package_id& rhs) noexcept {
+        return lhs.tie() == rhs.tie();
+    }
+};
+
+}  // namespace dds

src/dds/package_id.test.cpp

+#include <dds/package_id.hpp>
+
+#include <catch2/catch.hpp>
+
+TEST_CASE("Package package ID strings") {
+    struct case_ {
+        std::string_view string;
+        std::string_view expect_name;
+        std::string_view expect_version;
+    };
+    auto [id_str, exp_name, exp_ver] = GENERATE(Catch::Generators::values<case_>({
+        {"foo@1.2.3", "foo", "1.2.3"},
+        {"foo@1.2.3-beta", "foo", "1.2.3-beta"},
+        {"foo@1.2.3-alpha", "foo", "1.2.3-alpha"},
+    }));
+
+    auto pk_id = dds::package_id::parse(id_str);
+    CHECK(pk_id.to_string() == id_str);
+    CHECK(pk_id.name == exp_name);
+    CHECK(pk_id.version.to_string() == exp_ver);
+}
+
+TEST_CASE("Package ordering") {
+    enum order {
+        less_than,
+        equivalent_to,
+        greater_than,
+    };
+    struct case_ {
+        std::string_view lhs_pkg;
+        order            ord;
+        std::string_view rhs_pkg;
+    };
+
+    auto [lhs_str, ord, rhs_str] = GENERATE(Catch::Generators::values<case_>({
+        {"foo@1.2.3", greater_than, "bar@1.2.3"},
+        {"foo@1.2.3", greater_than, "foo@1.2.2"},
+        {"foo@1.2.3", equivalent_to, "foo@1.2.3"},
+        {"foo@1.2.3", equivalent_to, "foo@1.2.3+build-meta"},
+        {"foo@1.2.3", greater_than, "foo@1.2.3-alpha"},
+        {"foo@1.2.3-alpha.1", greater_than, "foo@1.2.3-alpha"},
+        {"foo@1.2.3-alpha.2", greater_than, "foo@1.2.3-alpha.1"},
+        {"foo@1.2.3-beta.2", greater_than, "foo@1.2.3-alpha.1"},
+        {"foo@0.1.2-alpha", less_than, "foo@1.0.0"},
+    }));
+
+    auto lhs = dds::package_id::parse(lhs_str);
+    auto rhs = dds::package_id::parse(rhs_str);
+
+    if (ord == less_than) {
+        CHECK(lhs < rhs);
+    } else if (ord == equivalent_to) {
+        CHECK(lhs == rhs);
+    } else if (ord == greater_than) {
+        CHECK_FALSE(lhs == rhs);
+        CHECK_FALSE(lhs < rhs);
+    }
+}

src/dds/package_manifest.cpp

     std::optional<std::string> opt_test_driver;
     lm::read(fmt::format("Reading package manifest '{}'", fpath.string()),
              kvs,
-             lm::read_required("Name", ret.name),
+             lm::read_required("Name", ret.pk_id.name),
              lm::read_opt("Namespace", ret.namespace_),
              lm::read_required("Version", version_str),
              lm::read_accumulate("Depends", depends_strs),
              lm::read_opt("Test-Driver", opt_test_driver),
              lm::reject_unknown());
 
-    if (ret.name.empty()) {
+    if (ret.pk_id.name.empty()) {
         throw std::runtime_error(
             fmt::format("'Name' field in [{}] may not be an empty string", fpath.string()));
     }
     }
 
     if (ret.namespace_.empty()) {
-        ret.namespace_ = ret.name;
+        ret.namespace_ = ret.pk_id.name;
     }
 
-    ret.version = semver::version::parse(version_str);
+    ret.pk_id.version = semver::version::parse(version_str);
 
     ret.dependencies = depends_strs                                   //
         | ranges::views::transform(dependency::parse_depends_string)  //

src/dds/package_manifest.hpp

 #pragma once
 
 #include <dds/deps.hpp>
+#include <dds/package_id.hpp>
 #include <dds/util/fs.hpp>
 #include <semver/version.hpp>
 
 };
 
 struct package_manifest {
-    std::string             name;
+    package_id              pk_id;
     std::string             namespace_;
     std::optional<test_lib> test_driver;
-    semver::version         version;
     std::vector<dependency> dependencies;
     static package_manifest load_from_file(path_ref);
 };

src/dds/repo/remote.cpp

             return false;
         }
 
-        auto nested        = lm::nested_kvlist::parse(value);
-        auto name_ver_pair = split_shell_string(nested.primary);
-        if (name_ver_pair.size() != 2) {
-            throw std::runtime_error(
-                fmt::format("{}: Invalid Remote-Package identity: '{}'", context, nested.primary));
-        }
-        auto name    = name_ver_pair[0];
-        auto version = semver::version::parse(name_ver_pair[1]);
-        put_listing(context, name, version, nested.pairs);
+        auto nested = lm::nested_kvlist::parse(value);
+        auto pk_id  = package_id::parse(nested.primary);
+        put_listing(context, std::move(pk_id), nested.pairs);
         return true;
     }
 
-    void put_listing(std::string_view     context,
-                     std::string          name,
-                     semver::version      version,
-                     const lm::pair_list& pairs) {
+    void put_listing(std::string_view context, package_id pk_id, const lm::pair_list& pairs) {
         if (pairs.find("git")) {
             std::string              url;
             std::string              ref;
                      lm::read_check_eq("git", ""),
                      lm::read_opt("auto", auto_id, &lm::split_usage_string),
                      lm::reject_unknown());
-            auto did_insert = out.emplace(remote_listing{std::move(name),
-                                                         version,
+            auto did_insert = out.emplace(remote_listing{std::move(pk_id),
                                                          git_remote_listing{url, ref, auto_id}})
                                   .second;
             if (!did_insert) {
-                spdlog::warn("Duplicate remote package defintion for {} {}",
-                             name,
-                             version.to_string());
+                spdlog::warn("Duplicate remote package defintion for {}", pk_id.to_string());
             }
         } else {
-            throw std::runtime_error(fmt::format("Unable to determine remote type of package {} {}",
-                                                 name,
-                                                 version.to_string()));
+            throw std::runtime_error(
+                fmt::format("Unable to determine remote type of package {}", pk_id.to_string()));
         }
     }
 };
     if (git.auto_lib.has_value()) {
         spdlog::info("Generating library data automatically");
         auto pkg_strm = dds::open(tmpdir.path() / "package.dds", std::ios::binary | std::ios::out);
-        pkg_strm << "Name: " << listing.name << '\n'                    //
-                 << "Version: " << listing.version.to_string() << '\n'  //
+        pkg_strm << "Name: " << listing.pk_id.name << '\n'                    //
+                 << "Version: " << listing.pk_id.version.to_string() << '\n'  //
                  << "Namespace: " << git.auto_lib->namespace_;
         auto lib_strm = dds::open(tmpdir.path() / "library.dds", std::ios::binary | std::ios::out);
         lib_strm << "Name: " << git.auto_lib->name;
 
 temporary_sdist remote_listing::pull_sdist() const {
     auto tsd = visit([&](auto&& actual) { return do_pull_sdist(*this, actual); });
-    if (tsd.sdist.manifest.name != name) {
-        throw std::runtime_error(
-            fmt::format("The name in the generated sdist ('{}') does not match the name listed in "
-                        "the remote listing file (expected '{}')",
-                        tsd.sdist.manifest.name,
-                        name));
-    }
-    if (tsd.sdist.manifest.version != version) {
-        throw std::runtime_error(
-            fmt::format("The version of the generated sdist is '{}', which does not match the "
-                        "expected version '{}'",
-                        tsd.sdist.manifest.version.to_string(),
-                        version.to_string()));
+    if (!(tsd.sdist.manifest.pk_id == pk_id)) {
+        throw std::runtime_error(fmt::format(
+            "The package name@version in the generated sdist does not match the name listed in "
+            "the remote listing file (expected '{}', but got '{}')",
+            pk_id.to_string(),
+            tsd.sdist.manifest.pk_id.to_string()));
     }
     return tsd;
 }

src/dds/repo/remote.hpp

 };
 
 struct remote_listing {
-    std::string                      name;
-    semver::version                  version;
+    package_id                       pk_id;
     std::variant<git_remote_listing> remote;
 
     template <typename Func>
 
 inline constexpr struct remote_listing_compare_t {
     using is_transparent = int;
-    auto tie(const remote_listing& rl) const { return std::tie(rl.name, rl.version); }
     bool operator()(const remote_listing& lhs, const remote_listing& rhs) const {
-        return tie(lhs) < tie(rhs);
+        return lhs.pk_id < rhs.pk_id;
     }
     template <typename Name, typename Version>
     bool operator()(const remote_listing& lhs, const std::tuple<Name, Version>& rhs) const {
-        return tie(lhs) < rhs;
+        auto&& [name, ver] = rhs;
+        return lhs.pk_id < package_id{name, ver};
     }
     template <typename Name, typename Version>
     bool operator()(const std::tuple<Name, Version>& lhs, const remote_listing& rhs) const {
-        return lhs < tie(rhs);
+        auto&& [name, ver] = lhs;
+        return package_id{name, ver} < rhs.pk_id;
     }
 } remote_listing_compare;
 

src/dds/repo/repo.cpp

             "repository, we'll hard-exit immediately.");
         std::terminate();
     }
-    auto sd_dest = _root / fmt::format("{}_{}", sd.manifest.name, sd.manifest.version.to_string());
+    auto sd_dest = _root / sd.manifest.pk_id.to_string();
     if (fs::exists(sd_dest)) {
         auto msg = fmt::format("Source distribution '{}' is already available in the local repo",
                                sd.path.string());
         fs::remove_all(sd_dest);
     }
     fs::rename(tmp_copy, sd_dest);
-    spdlog::info("Source distribution '{}' successfully exported", sd.ident());
+    spdlog::info("Source distribution '{}' successfully exported", sd.manifest.pk_id.to_string());
 }
 
 const sdist* repository::find(std::string_view name, semver::version ver) const noexcept {

src/dds/sdist.cpp

     sdist_export_file(out, params.project_dir, man_path);
     auto pkg_man = package_manifest::load_from_file(man_path);
 
-    spdlog::info("Generated export as {}_{}", pkg_man.name, pkg_man.version.to_string());
+    spdlog::info("Generated export as {}", pkg_man.pk_id.to_string());
 
     return sdist::from_directory(out);
 }

src/dds/sdist.hpp

         , path(path_) {}
 
     static sdist from_directory(path_ref p);
-
-    std::string ident() const noexcept {
-        return manifest.name + "_" + manifest.version.to_string();
-    }
 };
 
 inline constexpr struct sdist_compare_t {
     bool operator()(const sdist& lhs, const sdist& rhs) const {
-        return std::tie(lhs.manifest.name, lhs.manifest.version)
-            < std::tie(rhs.manifest.name, rhs.manifest.version);
+        return lhs.manifest.pk_id < rhs.manifest.pk_id;
     }
     template <typename Name, typename Version>
     bool operator()(const sdist& lhs, const std::tuple<Name, Version>& rhs) const {
-        return std::tie(lhs.manifest.name, lhs.manifest.version) < rhs;
+        auto&& [name, ver] = rhs;
+        return lhs.manifest.pk_id < package_id{name, ver};
     }
     template <typename Name, typename Version>
     bool operator()(const std::tuple<Name, Version>& lhs, const sdist& rhs) const {
-        return lhs < std::tie(rhs.manifest.name, rhs.manifest.version);
+        auto&& [name, ver] = lhs;
+        return package_id{name, ver} < rhs.manifest.pk_id;
     }
     using is_transparent = int;
 } sdist_compare;

tests/deps/git-remote/remote.dds

-Remote-Package: neo-buffer 0.1.0; git url=https://github.com/vector-of-bool/neo-buffer.git ref=develop
-Remote-Package: range-v3 0.9.1; git url=https://github.com/ericniebler/range-v3.git ref=0.9.1 auto=Niebler/range-v3
+Remote-Package: neo-buffer@0.1.0; git url=https://github.com/vector-of-bool/neo-buffer.git ref=develop
+Remote-Package: range-v3@0.9.1; git url=https://github.com/ericniebler/range-v3.git ref=0.9.1 auto=Niebler/range-v3

tests/deps/use-remote/remote.dds

-Remote-Package: nlohmann-json 3.7.1; git url=https://github.com/vector-of-bool/json.git ref=dds/3.7.1
+Remote-Package: nlohmann-json@3.7.1; git url=https://github.com/vector-of-bool/json.git ref=dds/3.7.1

tests/deps/use-spdlog/project/remote.dds

-Remote-Package: spdlog 1.4.2; git url=https://github.com/gabime/spdlog.git ref=v1.4.2 auto=spdlog/spdlog
+Remote-Package: spdlog@1.4.2; git url=https://github.com/gabime/spdlog.git ref=v1.4.2 auto=spdlog/spdlog

tests/sdist/sdist_test.py

     assert header_hpp.is_file()
     header_h = sd_dir / 'include/header.h'
     assert header_h.is_file()
+
+    dds.sdist_export()
+    assert (dds.repo_dir / 'foo@1.2.3').is_dir()

tests/test_drivers/catch/custom-runner/package.dds

+Name: Test
+Version: 0.0.0
+
+Test-Driver: Catch

tests/test_drivers/catch/custom-runner/src/testlib/calc.hpp

+#pragma once
+
+namespace stuff {
+
+int calculate(int a, int b) {
+    int result = a + b;
+    if (result == 42) {
+        return result;
+    } else {
+        return 42;
+    }
+}
+
+}  // namespace stuff

tests/test_drivers/catch/custom-runner/src/testlib/calc.test.cpp

+#define CATCH_CONFIG_RUNNER
+#include <catch2/catch.hpp>
+
+#include <testlib/calc.hpp>
+
+TEST_CASE("A simple test case") {
+    CHECK_FALSE(false);
+    CHECK(2 == 2);
+    CHECK(1 != 4);
+    CHECK(stuff::calculate(3, 11) == 42);
+}
+
+int main(int argc, char** argv) {
+    // We provide our own runner
+    return Catch::Session().run(argc, argv);
+}

tests/test_drivers/catch/main/package.dds

+Name: Test
+Version: 0.0.0
+
+Test-Driver: Catch-Main

tests/test_drivers/catch/main/src/testlib/calc.hpp

+#pragma once
+
+namespace stuff {
+
+int calculate(int a, int b) {
+    int result = a + b;
+    if (result == 42) {
+        return result;
+    } else {
+        return 42;
+    }
+}
+
+}  // namespace stuff

tests/test_drivers/catch/main/src/testlib/calc.test.cpp

+#include <catch2/catch.hpp>
+
+#include <testlib/calc.hpp>
+
+TEST_CASE("A simple test case") {
+    CHECK_FALSE(false);
+    CHECK(2 == 2);
+    CHECK(1 != 4);
+    CHECK(stuff::calculate(3, 11) == 42);
+}

tests/test_drivers/catch/test_catch.py

+from tests import DDS, dds_fixture_conf, DDSFixtureParams
+from dds_ci import proc
+
+
+@dds_fixture_conf(
+    DDSFixtureParams('main', 'main'),
+    DDSFixtureParams('custom-runner', 'custom-runner'),
+)
+def test_catch_testdriver(dds: DDS):
+    dds.build(tests=True)
+    test_exe = dds.build_dir / f'test/calc{dds.exe_suffix}'
+    assert test_exe.exists()
+    assert proc.run([test_exe]).returncode == 0