Doc comments, and manifest.pk_id -> pkg_id

src/dds/build.cpp

@@ -215,11 +215,11 @@ void add_sdist_to_build(build_plan&             plan,
                         build_env_ref           env,
                         usage_requirement_map&  ureqs,
                         sdist_names&            already_added) {
-    if (already_added.find(sd.manifest.pk_id.name) != already_added.end()) {
+    if (already_added.find(sd.manifest.pkg_id.name) != already_added.end()) {
         // This one has already been added
         return;
     }
-    spdlog::debug("Adding dependent build: {}", sd.manifest.pk_id.name);
+    spdlog::debug("Adding dependent build: {}", sd.manifest.pkg_id.name);
     // Ensure that ever dependency is loaded up first)
     for (const auto& dep : sd.manifest.dependencies) {
         auto other = sd_idx.find(dep.name);
@@ -228,9 +228,9 @@ void add_sdist_to_build(build_plan&             plan,
         add_sdist_to_build(plan, other->second, sd_idx, env, ureqs, already_added);
     }
     // Record that we have been processed
-    already_added.insert(sd.manifest.pk_id.name);
+    already_added.insert(sd.manifest.pkg_id.name);
     // Finally, actually add the package:
-    auto& pkg  = plan.add_package(package_plan(sd.manifest.pk_id.name, sd.manifest.namespace_));
+    auto& pkg  = plan.add_package(package_plan(sd.manifest.pkg_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();
@@ -250,7 +250,7 @@ void add_deps_to_build(build_plan&             plan,
                        build_env_ref           env) {
     auto sd_idx = params.dep_sdists  //
         | ranges::views::transform([](const auto& sd) {
-                      return std::pair(sd.manifest.pk_id.name, std::cref(sd));
+                      return std::pair(sd.manifest.pkg_id.name, std::cref(sd));
                   })  //
         | ranges::to<sdist_index_type>();
 
@@ -286,7 +286,7 @@ void dds::build(const build_params& params, const package_manifest& man) {
     }
 
     // Initialize the build plan for this project.
-    auto& pkg = plan.add_package(package_plan(man.pk_id.name, man.namespace_));
+    auto& pkg = plan.add_package(package_plan(man.pkg_id.name, man.namespace_));
 
     // assert(false && "Not ready yet!");
 

src/dds/catalog/get.cpp

@@ -52,12 +52,12 @@ temporary_sdist do_pull_sdist(const package_info& listing, const git_remote_list
 
 temporary_sdist dds::get_package_sdist(const package_info& pkg) {
     auto tsd = std::visit([&](auto&& remote) { return do_pull_sdist(pkg, remote); }, pkg.remote);
-    if (!(tsd.sdist.manifest.pk_id == pkg.ident)) {
+    if (!(tsd.sdist.manifest.pkg_id == pkg.ident)) {
         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 '{}')",
             pkg.ident.to_string(),
-            tsd.sdist.manifest.pk_id.to_string()));
+            tsd.sdist.manifest.pkg_id.to_string()));
     }
     return tsd;
 }

src/dds/dds.main.cpp

@@ -312,7 +312,7 @@ struct cli_repo {
         int run() {
             auto list_contents = [&](dds::repository repo) {
                 auto same_name = [](auto&& a, auto&& b) {
-                    return a.manifest.pk_id.name == b.manifest.pk_id.name;
+                    return a.manifest.pkg_id.name == b.manifest.pkg_id.name;
                 };
 
                 auto all         = repo.iter_sdists();
@@ -321,13 +321,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.pk_id.name, grp);
+                                       return std::pair(grp[0].manifest.pkg_id.name, grp);
                                    });
 
                 for (const auto& [name, grp] : grp_by_name) {
                     spdlog::info("{}:", name);
                     for (const dds::sdist& sd : grp) {
-                        spdlog::info("  - {}", sd.manifest.pk_id.version.to_string());
+                        spdlog::info("  - {}", sd.manifest.pkg_id.version.to_string());
                     }
                 }
 

src/dds/deps.cpp

@@ -56,7 +56,7 @@ void resolve_ureqs_(shared_compile_file_rules& rules,
             throw std::runtime_error(
                 fmt::format("Unable to resolve dependency '{}' (required by '{}')",
                             dep.name,
-                            man.pk_id.to_string()));
+                            man.pkg_id.to_string()));
         }
         resolve_ureqs_(rules, found->second.get().manifest, sd_idx);
         auto lib_src     = found->second.get().path / "src";
@@ -98,11 +98,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.pk_id.name) != already_added.end()) {
+    if (already_added.find(sd.manifest.pkg_id.name) != already_added.end()) {
         // We've already loaded this package into the plan.
         return;
     }
-    spdlog::debug("Add to plan: {}", sd.manifest.pk_id.name);
+    spdlog::debug("Add to plan: {}", sd.manifest.pkg_id.name);
     // First, load every dependency
     for (const auto& dep : sd.manifest.dependencies) {
         auto other = sd_idx.find(dep.name);
@@ -111,9 +111,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.pk_id.name);
+    already_added.insert(sd.manifest.pkg_id.name);
     // Add the package:
-    auto& pkg  = plan.add_package(package_plan(sd.manifest.pk_id.name, sd.manifest.namespace_));
+    auto& pkg  = plan.add_package(package_plan(sd.manifest.pkg_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();
@@ -129,7 +129,7 @@ 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.pk_id.name, std::cref(sd));
+                      return std::pair(sd.manifest.pkg_id.name, std::cref(sd));
                   })  //
         | ranges::to<sdist_index_type>();
 
@@ -137,7 +137,7 @@ build_plan dds::create_deps_build_plan(const std::vector<sdist>& deps, build_env
     usage_requirement_map ureqs;
     sdist_names           already_added;
     for (const sdist& sd : deps) {
-        spdlog::info("Recording dependency: {}", sd.manifest.pk_id.name);
+        spdlog::info("Recording dependency: {}", sd.manifest.pkg_id.name);
         add_sdist_to_dep_plan(plan, sd, env, sd_idx, ureqs, already_added);
     }
     return plan;

src/dds/package/id.hpp

@@ -8,23 +8,40 @@
 
 namespace dds {
 
+/**
+ * Represents a unique package ID. We store this as a simple name-version pair.
+ *
+ * In text, this is represented with an `@` symbol in between. The `parse` and
+ * `to_string` method convert between this textual representation, and supports
+ * full round-trips.
+ */
 struct package_id {
-    std::string     name;
+    /// The name of the package
+    std::string name;
+    /// The version of the package
     semver::version version;
 
+    /// Default-initialize a package_id with a blank name and a default version
     package_id() = default;
+    /// Construct a package ID from a name-version pair
     package_id(std::string_view s, semver::version v);
 
+    /**
+     * Parse the given string into a package_id object.
+     */
     static package_id parse(std::string_view);
-    std::string       to_string() const noexcept;
 
-    auto tie() const noexcept { return std::tie(name, version); }
+    /**
+     * Convert this package_id into its corresponding textual representation.
+     * The returned string can be passed back to `parse()` for a round-trip
+     */
+    std::string to_string() const noexcept;
 
     friend bool operator<(const package_id& lhs, const package_id& rhs) noexcept {
-        return lhs.tie() < rhs.tie();
+        return std::tie(lhs.name, lhs.version) < std::tie(rhs.name, rhs.version);
     }
     friend bool operator==(const package_id& lhs, const package_id& rhs) noexcept {
-        return lhs.tie() == rhs.tie();
+        return std::tie(lhs.name, lhs.version) == std::tie(rhs.name, rhs.version);
     }
 };
 

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.pk_id.name),
+             lm::read_required("Name", ret.pkg_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.pk_id.name.empty()) {
+    if (ret.pkg_id.name.empty()) {
         throw std::runtime_error(
             fmt::format("'Name' field in [{}] may not be an empty string", fpath.string()));
     }
@@ -45,10 +45,10 @@ package_manifest package_manifest::load_from_file(const fs::path& fpath) {
     }
 
     if (ret.namespace_.empty()) {
-        ret.namespace_ = ret.pk_id.name;
+        ret.namespace_ = ret.pkg_id.name;
     }
 
-    ret.pk_id.version = semver::version::parse(version_str);
+    ret.pkg_id.version = semver::version::parse(version_str);
 
     ret.dependencies = depends_strs                                   //
         | ranges::views::transform(dependency::parse_depends_string)  //

src/dds/package/manifest.hpp

@@ -3,7 +3,6 @@
 #include <dds/deps.hpp>
 #include <dds/package/id.hpp>
 #include <dds/util/fs.hpp>
-#include <semver/version.hpp>
 
 #include <optional>
 #include <string>
@@ -11,16 +10,30 @@
 
 namespace dds {
 
+/**
+ * Possible values for Test-Driver in a package.dds
+ */
 enum class test_lib {
     catch_,
     catch_main,
 };
 
+/**
+ * Struct representing the contents of a `packaeg.dds` file.
+ */
 struct package_manifest {
-    package_id              pk_id;
-    std::string             namespace_;
+    /// The package ID, as determined by `Name` and `Version` together
+    package_id pkg_id;
+    /// The declared `Namespace` of the package. This directly corresponds with the libman Namespace
+    std::string namespace_;
+    /// The `Test-Driver` that this package declares, or `nullopt` if absent.
     std::optional<test_lib> test_driver;
+    /// The dependencies declared with the `Depends` fields, if any.
     std::vector<dependency> dependencies;
+
+    /**
+     * Load a package manifest from a file on disk.
+     */
     static package_manifest load_from_file(path_ref);
 };
 

src/dds/repo/repo.cpp

@@ -71,7 +71,7 @@ void repository::add_sdist(const sdist& sd, if_exists ife_action) {
             "repository, we'll hard-exit immediately.");
         std::terminate();
     }
-    auto sd_dest = _root / sd.manifest.pk_id.to_string();
+    auto sd_dest = _root / sd.manifest.pkg_id.to_string();
     if (fs::exists(sd_dest)) {
         auto msg = fmt::format("Source distribution '{}' is already available in the local repo",
                                sd.path.string());
@@ -95,7 +95,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.manifest.pk_id.to_string());
+    spdlog::info("Source distribution '{}' successfully exported", sd.manifest.pkg_id.to_string());
 }
 
 const sdist* repository::find(const package_id& pkg) const noexcept {
@@ -111,10 +111,10 @@ std::vector<sdist> repository::solve(const std::vector<dependency>& deps) const
                           [&](std::string_view name) -> std::vector<package_id> {
                               auto items = ranges::views::all(_sdists)  //
                                   | ranges::views::filter([&](const sdist& sd) {
-                                               return sd.manifest.pk_id.name == name;
+                                               return sd.manifest.pkg_id.name == name;
                                            })
                                   | ranges::views::transform(
-                                               [](const sdist& sd) { return sd.manifest.pk_id; })
+                                               [](const sdist& sd) { return sd.manifest.pkg_id; })
                                   | ranges::to_vector;
                               ranges::sort(items, std::less<>{});
                               return items;

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.pk_id.to_string());
+    spdlog::info("Generated export as {}", pkg_man.pkg_id.to_string());
 
     return sdist::from_directory(out);
 }

src/dds/sdist.hpp

@@ -28,13 +28,13 @@ struct sdist {
 
 inline constexpr struct sdist_compare_t {
     bool operator()(const sdist& lhs, const sdist& rhs) const {
-        return lhs.manifest.pk_id < rhs.manifest.pk_id;
+        return lhs.manifest.pkg_id < rhs.manifest.pkg_id;
     }
     bool operator()(const sdist& lhs, const package_id& rhs) const {
-        return lhs.manifest.pk_id < rhs;
+        return lhs.manifest.pkg_id < rhs;
     }
     bool operator()(const package_id& lhs, const sdist& rhs) const {
-        return lhs < rhs.manifest.pk_id;
+        return lhs < rhs.manifest.pkg_id;
     }
     using is_transparent = int;
 } sdist_compare;