From bbf1096b1539a77043c7782925f37dbce7230474 Mon Sep 17 00:00:00 2001
From: Ken Matsui <26405363+ken-matsui@users.noreply.github.com>
Date: Thu, 30 Jan 2025 21:05:40 -0500
Subject: [PATCH] Manifest: move findManifest into Manifest class (#1134)

---
 src/Cmd/Add.cc    |  2 +-
 src/Cmd/Clean.cc  |  2 +-
 src/Cmd/Remove.cc |  2 +-
 src/Manifest.cc   | 51 ++++++++++++++++++++++-------------------------
 src/Manifest.hpp  |  9 +++++----
 5 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/Cmd/Add.cc b/src/Cmd/Add.cc
index ae91d4307..899b6d640 100644
--- a/src/Cmd/Add.cc
+++ b/src/Cmd/Add.cc
@@ -129,7 +129,7 @@ addDependencyToManifest(
   }
 
   // Keep the order of the tables.
-  const fs::path manifestPath = Try(findManifest());
+  const fs::path manifestPath = Try(Manifest::findPath());
   auto data = toml::parse<toml::ordered_type_config>(manifestPath);
 
   // Check if the dependencies table exists, if not create it.
diff --git a/src/Cmd/Clean.cc b/src/Cmd/Clean.cc
index fe3d182c7..4f51da5ba 100644
--- a/src/Cmd/Clean.cc
+++ b/src/Cmd/Clean.cc
@@ -25,7 +25,7 @@ const Subcmd CLEAN_CMD =  //
 static Result<void>
 cleanMain(CliArgsView args) noexcept {
   // TODO: share across sources
-  fs::path outDir = Try(findManifest()).parent_path() / "cabin-out";
+  fs::path outDir = Try(Manifest::findPath()).parent_path() / "cabin-out";
 
   // Parse args
   for (auto itr = args.begin(); itr != args.end(); ++itr) {
diff --git a/src/Cmd/Remove.cc b/src/Cmd/Remove.cc
index 8395a9d7c..7b1895e47 100644
--- a/src/Cmd/Remove.cc
+++ b/src/Cmd/Remove.cc
@@ -31,7 +31,7 @@ removeMain(const CliArgsView args) {
   Ensure(!args.empty(), "`cabin remove` requires at least one argument");
 
   std::vector<std::string_view> removedDeps = {};
-  const fs::path manifestPath = Try(findManifest());
+  const fs::path manifestPath = Try(Manifest::findPath());
   auto data = toml::parse<toml::ordered_type_config>(manifestPath);
   auto& deps = data["dependencies"];
 
diff --git a/src/Manifest.cc b/src/Manifest.cc
index 22bbceaa6..407c543ad 100644
--- a/src/Manifest.cc
+++ b/src/Manifest.cc
@@ -465,37 +465,12 @@ SystemDependency::install() const {
   return Ok(DepMetadata{ .includes = cflags, .libs = libs });
 }
 
-Result<fs::path>
-findManifest(fs::path candidateDir) noexcept {
-  const fs::path origCandDir = candidateDir;
-  while (true) {
-    const fs::path configPath = candidateDir / Manifest::NAME;
-    logger::trace("Finding manifest: {}", configPath.string());
-    if (fs::exists(configPath)) {
-      return Ok(configPath);
-    }
-
-    const fs::path parentPath = candidateDir.parent_path();
-    if (candidateDir.has_parent_path()
-        && parentPath != candidateDir.root_directory()) {
-      candidateDir = parentPath;
-    } else {
-      break;
-    }
-  }
-
-  Bail(
-      "{} not find in `{}` and its parents", Manifest::NAME,
-      origCandDir.string()
-  );
-}
-
 Result<Manifest>
 Manifest::tryParse(fs::path path, const bool findParents) noexcept {
   if (findParents) {
-    path = Try(findManifest(path.parent_path()));
+    path = Try(findPath(path.parent_path()));
   }
-  return Manifest::tryFromToml(toml::parse(path), path);
+  return tryFromToml(toml::parse(path), path);
 }
 
 Result<Manifest>
@@ -514,6 +489,28 @@ Manifest::tryFromToml(const toml::value& data, fs::path path) noexcept {
   ));
 }
 
+Result<fs::path>
+Manifest::findPath(fs::path candidateDir) noexcept {
+  const fs::path origCandDir = candidateDir;
+  while (true) {
+    const fs::path configPath = candidateDir / FILE_NAME;
+    logger::trace("Finding manifest: {}", configPath.string());
+    if (fs::exists(configPath)) {
+      return Ok(configPath);
+    }
+
+    const fs::path parentPath = candidateDir.parent_path();
+    if (candidateDir.has_parent_path()
+        && parentPath != candidateDir.root_directory()) {
+      candidateDir = parentPath;
+    } else {
+      break;
+    }
+  }
+
+  Bail("{} not find in `{}` and its parents", FILE_NAME, origCandDir.string());
+}
+
 Result<std::vector<DepMetadata>>
 Manifest::installDeps(const bool includeDevDeps) const {
   std::vector<DepMetadata> installed;
diff --git a/src/Manifest.hpp b/src/Manifest.hpp
index eaa05ed93..b3dc06ced 100644
--- a/src/Manifest.hpp
+++ b/src/Manifest.hpp
@@ -190,7 +190,7 @@ using Dependency =
 
 class Manifest {
 public:
-  static constexpr const char* NAME = "cabin.toml";
+  static constexpr const char* FILE_NAME = "cabin.toml";
 
   const fs::path path;
   const Package package;
@@ -200,11 +200,14 @@ class Manifest {
   const Lint lint;
 
   static Result<Manifest> tryParse(
-      fs::path path = fs::current_path() / NAME, bool findParents = true
+      fs::path path = fs::current_path() / FILE_NAME, bool findParents = true
   ) noexcept;
   static Result<Manifest>
   tryFromToml(const toml::value& data, fs::path path = "unknown") noexcept;
 
+  static Result<fs::path>
+  findPath(fs::path candidateDir = fs::current_path()) noexcept;
+
   Result<std::vector<DepMetadata>> installDeps(bool includeDevDeps) const;
 
 private:
@@ -219,8 +222,6 @@ class Manifest {
         profiles(std::move(profiles)), lint(std::move(lint)) {}
 };
 
-Result<fs::path>
-findManifest(fs::path candidateDir = fs::current_path()) noexcept;
 Result<void> validatePackageName(std::string_view name) noexcept;
 
 }  // namespace cabin