From b0ef0427f86bd8b4a286843ef34f95a2d6f28b8a Mon Sep 17 00:00:00 2001
From: Colin Rofls <colin@cmyr.net>
Date: Thu, 19 Sep 2024 13:47:37 -0400
Subject: [PATCH] Include org in local repo checkout

This addresses an issue where certain fonts exist in multiple locations
(e.g. under the original author as well as the googlefotns org)
---
 src/lib.rs       | 23 ++---------------------
 src/repo_info.rs | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 0a5fb76..f17e228 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -287,10 +287,8 @@ fn config_files_and_rev_for_repo(
     repo_url: &str,
     checkout_font_dir: &Path,
 ) -> Result<(Vec<PathBuf>, GitRev), ConfigFetchIssue> {
-    let repo_name = repo_name_from_url(repo_url)
-        .ok_or_else(|| ConfigFetchIssue::BadRepoUrl(repo_url.into()))?;
-
-    let local_repo_dir = checkout_font_dir.join(repo_name);
+    let local_repo_dir = repo_info::repo_path_for_url(repo_url, checkout_font_dir)
+        .ok_or_else(|| ConfigFetchIssue::BadRepoUrl(repo_url.to_owned()))?;
     // - if local repo already exists, then look there
     // - otherwise try naive http requests first,
     // - and then finally clone the repo and look
@@ -528,11 +526,6 @@ fn clone_repo(url: &str, to_dir: &Path) -> Result<(), GitFail> {
     Ok(())
 }
 
-fn repo_name_from_url(url: &str) -> Option<&str> {
-    let url = url.trim_end_matches('/');
-    url.rsplit_once('/').map(|(_, tail)| tail)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -548,18 +541,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn name_from_url() {
-        assert_eq!(
-            repo_name_from_url("https://github.com/hyper-type/hahmlet/"),
-            Some("hahmlet"),
-        );
-        assert_eq!(
-            repo_name_from_url("https://github.com/hyper-type/Advent"),
-            Some("Advent"),
-        );
-    }
-
     #[test]
     fn remote_sha() {
         let rev = get_git_rev_remote("https://github.com/googlefonts/fontations").unwrap();
diff --git a/src/repo_info.rs b/src/repo_info.rs
index 5e41aa6..f97f21a 100644
--- a/src/repo_info.rs
+++ b/src/repo_info.rs
@@ -55,13 +55,20 @@ impl RepoInfo {
         &self.rev
     }
 
+    /// Given a root cache directory, return the local path this repo.
+    ///
+    /// This is in the format, `{cache_dir}/{repo_org}/{repo_name}`
+    pub fn repo_path(&self, cache_dir: &Path) -> PathBuf {
+        // unwrap is okay because we already know the url is well formed
+        repo_path_for_url(&self.repo_url, cache_dir).unwrap()
+    }
+
     /// Return the a `Vec` of source files in this respository.
     ///
     /// If necessary, this will create a new checkout of this repo at
-    /// '{git_cache_dir}/{repo_name}'.
+    /// '{git_cache_dir}/{repo_org}/{repo_name}'.
     pub fn get_sources(&self, git_cache_dir: &Path) -> Result<Vec<PathBuf>, LoadRepoError> {
-        let font_dir = git_cache_dir.join(self.repo_name());
-
+        let font_dir = self.repo_path(git_cache_dir);
         if !font_dir.exists() {
             std::fs::create_dir_all(&font_dir)?;
             super::clone_repo(&self.repo_url, &font_dir)?;
@@ -107,3 +114,27 @@ fn repo_name_and_org_from_url(url: &str) -> Option<(&str, &str)> {
     let (_, org) = rest.rsplit_once('/')?;
     Some((org, name))
 }
+
+pub(super) fn repo_path_for_url(url: &str, base_cache_dir: &Path) -> Option<PathBuf> {
+    let (org, name) = repo_name_and_org_from_url(url)?;
+    let mut path = base_cache_dir.join(org);
+    path.push(name);
+    Some(path)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn org_and_name_from_url() {
+        assert_eq!(
+            repo_name_and_org_from_url("https://github.com/hyper-type/hahmlet/"),
+            Some(("hyper-type", "hahmlet")),
+        );
+        assert_eq!(
+            repo_name_and_org_from_url("https://github.com/hyper-type/Advent"),
+            Some(("hyper-type", "Advent")),
+        );
+    }
+}