Incorporate the new package_id type, as use `@` as a name separator

src/dds/build.cpp

@@ -1,5 +1,6 @@
 #include "./build.hpp"
 
+#include <dds/build/plan/compile_exec.hpp>
 #include <dds/catch2_embedded.hpp>
 #include <dds/compdb.hpp>
 #include <dds/usage_reqs.hpp>
@@ -7,7 +8,6 @@
 #include <dds/util/time.hpp>
 #include <libman/index.hpp>
 #include <libman/parse.hpp>
-#include <dds/build/plan/compile_exec.hpp>
 
 #include <spdlog/spdlog.h>
 
@@ -127,7 +127,7 @@ void prepare_catch2_driver(library_build_params& lib_params,
         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();
     }
 
@@ -198,7 +198,7 @@ void dds::build(const build_params& params, const package_manifest& man) {
     }
 
     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

@@ -119,7 +119,7 @@ struct cli_repo {
         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                             //
@@ -127,13 +127,13 @@ struct cli_repo {
                     | 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

@@ -63,8 +63,9 @@ void detail::do_find_deps(const repository& repo, const dependency& dep, std::ve
     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();
         }
@@ -87,7 +88,7 @@ void resolve_ureqs_(shared_compile_file_rules& rules,
             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";
@@ -129,11 +130,11 @@ void add_sdist_to_dep_plan(build_plan&             plan,
                            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);
@@ -142,9 +143,9 @@ void add_sdist_to_dep_plan(build_plan&             plan,
         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();
@@ -159,15 +160,16 @@ void add_sdist_to_dep_plan(build_plan&             plan,
 
 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

@@ -18,4 +18,13 @@ package_id package_id::parse(std::string_view s) {
     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

@@ -12,6 +12,9 @@ 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;
 

src/dds/package_manifest.cpp

@@ -18,14 +18,14 @@ package_manifest package_manifest::load_from_file(const fs::path& fpath) {
     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()));
     }
@@ -47,10 +47,10 @@ package_manifest package_manifest::load_from_file(const fs::path& fpath) {
     }
 
     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

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <dds/deps.hpp>
+#include <dds/package_id.hpp>
 #include <dds/util/fs.hpp>
 #include <semver/version.hpp>
 
@@ -17,10 +18,9 @@ enum class test_lib {
 };
 
 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

@@ -114,18 +114,18 @@ temporary_sdist do_pull_sdist(const remote_listing& listing, const git_remote_li
 
 temporary_sdist remote_listing::pull_sdist() const {
     auto tsd = visit([&](auto&& actual) { return do_pull_sdist(*this, actual); });
-    if (tsd.sdist.manifest.name != name) {
+    if (tsd.sdist.manifest.pk_id.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,
+                        tsd.sdist.manifest.pk_id.name,
                         name));
     }
-    if (tsd.sdist.manifest.version != version) {
+    if (tsd.sdist.manifest.pk_id.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(),
+                        tsd.sdist.manifest.pk_id.version.to_string(),
                         version.to_string()));
     }
     return tsd;

src/dds/repo/repo.cpp

@@ -69,7 +69,7 @@ void repository::add_sdist(const sdist& sd, if_exists ife_action) {
             "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());
@@ -93,7 +93,7 @@ void repository::add_sdist(const sdist& sd, if_exists ife_action) {
         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

@@ -93,7 +93,7 @@ sdist dds::create_sdist_in_dir(path_ref out, const sdist_params& params) {
     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

@@ -24,24 +24,21 @@ struct sdist {
         , 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/sdist/sdist_test.py

@@ -11,3 +11,6 @@ def test_create_sdist(dds: DDS):
     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()