From 1b448f32a60c94b4c1617bdd522ce3e079c36526 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 10 Oct 2024 11:08:16 -0400 Subject: [PATCH] Add ability for RepoInfo to iter configs This will be necessary for building up a list of targets. --- source_dir_test/Sources/README.md | 2 + src/lib.rs | 95 +++++++++++++++++++------------ src/repo_info.rs | 45 +++++++++++++-- 3 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 source_dir_test/Sources/README.md diff --git a/source_dir_test/Sources/README.md b/source_dir_test/Sources/README.md new file mode 100644 index 0000000..64b946d --- /dev/null +++ b/source_dir_test/Sources/README.md @@ -0,0 +1,2 @@ +This directory exists to test that we correctly handle case on case-insensitive +file systems. diff --git a/src/lib.rs b/src/lib.rs index 45f2175..8c25b77 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ //! // get a list of repositories: //! //! let font_repo_cache = Path::new("~/where_i_want_to_checkout_fonts"); -//! let font_repos = google_fonts_sources::discover_sources(None, Some(font_repo_cache), false); +//! let font_repos = google_fonts_sources::discover_sources(font_repo_cache).unwrap(); //! //! // for each repo we find, do something with each source: //! @@ -274,32 +274,30 @@ fn config_files_and_rev_for_repo( // - otherwise try naive http requests first, // - and then finally clone the repo and look let local_git_dir = local_repo_dir.join(".git"); - if local_git_dir.exists() { - let rev = get_git_rev(&local_repo_dir).map_err(ConfigFetchIssue::GitFail)?; - let configs = get_config_paths(&local_repo_dir).ok_or(ConfigFetchIssue::NoConfigFound)?; - return Ok((configs, rev)); - } - - let naive = config_file_and_rev_from_remote_naive(repo_url).map(|(p, rev)| (vec![p], rev)); - // if not found, try checking out and looking; otherwise return the result - if !matches!(naive, Err(ConfigFetchIssue::NoConfigFound)) { - naive - } else { - let configs = config_files_from_local_checkout(repo_url, &local_repo_dir)?; - let rev = get_git_rev(&local_repo_dir).map_err(ConfigFetchIssue::GitFail)?; - Ok((configs, rev)) + let skip_http = local_git_dir.exists(); + + if !skip_http { + let config_from_http = + config_file_and_rev_from_remote_http(repo_url).map(|(p, rev)| (vec![p], rev)); + // if not found, try checking out and looking; otherwise return the result + if !matches!(config_from_http, Err(ConfigFetchIssue::NoConfigFound)) { + return config_from_http; + } } + let configs = config_files_from_local_checkout(repo_url, &local_repo_dir)?; + let rev = get_git_rev(&local_repo_dir).map_err(ConfigFetchIssue::GitFail)?; + Ok((configs, rev)) } -fn config_file_and_rev_from_remote_naive( +fn config_file_and_rev_from_remote_http( repo_url: &str, ) -> Result<(PathBuf, GitRev), ConfigFetchIssue> { - config_file_from_remote_naive(repo_url) + config_file_from_remote_http(repo_url) .and_then(|config| get_git_rev_remote(repo_url).map(|rev| (config, rev))) } // just check for the presence of the most common file names -fn config_file_from_remote_naive(repo_url: &str) -> Result { +fn config_file_from_remote_http(repo_url: &str) -> Result { for filename in ["config.yaml", "config.yml"] { let config_url = format!("{repo_url}/tree/HEAD/sources/{filename}"); let req = ureq::head(&config_url); @@ -336,7 +334,12 @@ fn config_files_from_local_checkout( std::fs::create_dir_all(local_repo_dir).unwrap(); clone_repo(repo_url, local_repo_dir).map_err(ConfigFetchIssue::GitFail)?; } - get_config_paths(local_repo_dir).ok_or(ConfigFetchIssue::NoConfigFound) + let configs: Vec<_> = iter_config_paths(local_repo_dir)?.collect(); + if configs.is_empty() { + Err(ConfigFetchIssue::NoConfigFound) + } else { + Ok(configs) + } } /// Look for a file like 'config.yaml' in a google fonts font checkout. @@ -344,7 +347,7 @@ fn config_files_from_local_checkout( /// This will look for all files that begin with 'config' and have either the /// 'yaml' or 'yml' extension; if multiple files match this pattern it will /// return the one with the shortest name. -fn get_config_paths(font_dir: &Path) -> Option> { +fn iter_config_paths(font_dir: &Path) -> Result, ConfigFetchIssue> { #[allow(clippy::ptr_arg)] // we don't use &Path so we can pass this to a closure below fn looks_like_config_file(path: &PathBuf) -> bool { let (Some(stem), Some(extension)) = @@ -355,19 +358,31 @@ fn get_config_paths(font_dir: &Path) -> Option> { stem.starts_with("config") && (extension == "yaml" || extension == "yml") } - let sources_dir = font_dir.join("sources"); - let contents = std::fs::read_dir(sources_dir).ok()?; - let mut config_files = contents - .filter_map(|entry| { - entry - .ok() - .and_then(|e| e.path().file_name().map(PathBuf::from)) - }) - .filter(looks_like_config_file) - .collect::>(); - - config_files.sort_by_key(|p| p.to_str().map(|s| s.len()).unwrap_or(usize::MAX)); - Some(config_files) + let sources_dir = find_sources_dir(font_dir).ok_or(ConfigFetchIssue::NoConfigFound)?; + let contents = std::fs::read_dir(sources_dir).map_err(|_| ConfigFetchIssue::NoConfigFound)?; + Ok(contents + .filter_map(|entry| entry.ok().map(|e| PathBuf::from(e.file_name()))) + .filter(looks_like_config_file)) +} + +fn find_sources_dir(font_dir: &Path) -> Option { + for case in ["sources", "Sources"] { + let path = font_dir.join(case); + if path.exists() { + // in order to handle case-insensitive file systems, we need to + // canonicalize this name, strip the canonical prefix, and glue + // it all back together + let canonical_font_dir = font_dir.canonicalize().ok()?; + let canonical_path = path.canonicalize().ok()?; + if let Ok(stripped) = canonical_path.strip_prefix(&canonical_font_dir) { + return Some(font_dir.join(stripped)); + } + // if that fails for some reason just return the unnormalized path, + // we'll survive + return Some(path); + } + } + None } fn update_google_fonts_checkout(path: &Path) -> Result<(), Error> { @@ -533,12 +548,12 @@ mod tests { use super::*; #[test] - fn naive_config() { + fn http_config() { assert!( - config_file_and_rev_from_remote_naive("https://github.com/PaoloBiagini/Joan").is_ok() + config_file_and_rev_from_remote_http("https://github.com/PaoloBiagini/Joan").is_ok() ); assert!(matches!( - config_file_and_rev_from_remote_naive("https://github.com/googlefonts/bangers"), + config_file_and_rev_from_remote_http("https://github.com/googlefonts/bangers"), Err(ConfigFetchIssue::NoConfigFound) )); } @@ -550,4 +565,12 @@ mod tests { assert!(rev.len() > 16); assert!(rev.chars().all(|c| c.is_ascii_hexdigit())); } + + #[test] + fn source_dir_case() { + assert_eq!( + find_sources_dir(Path::new("./source_dir_test")), + Some(PathBuf::from("./source_dir_test/Sources")) + ) + } } diff --git a/src/repo_info.rs b/src/repo_info.rs index e5a1494..8069197 100644 --- a/src/repo_info.rs +++ b/src/repo_info.rs @@ -5,7 +5,9 @@ use std::path::{Path, PathBuf}; use crate::{error::LoadRepoError, Config}; /// Information about a git repository containing font sources -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize)] +#[derive( + Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize, +)] #[non_exhaustive] pub struct RepoInfo { /// The repository's url @@ -63,12 +65,17 @@ impl RepoInfo { repo_path_for_url(&self.repo_url, cache_dir).unwrap() } - /// Return the a `Vec` of source files in this respository. + /// Attempt to checkout/update this repo to the provided `cache_dir`. /// - /// If necessary, this will create a new checkout of this repo at - /// '{git_cache_dir}/{repo_org}/{repo_name}'. - pub fn get_sources(&self, git_cache_dir: &Path) -> Result, LoadRepoError> { - let font_dir = self.repo_path(git_cache_dir); + /// The repo will be checked out to '{cache_dir}/{repo_org}/{repo_name}', + /// and HEAD will be set to the `self.git_rev()`. + /// + /// Returns the path to the checkout on success. + /// + /// Returns an error if the repo cannot be cloned, the git rev cannot be + /// found, or if there is an io error. + pub fn instantiate(&self, cache_dir: &Path) -> Result { + let font_dir = self.repo_path(cache_dir); if !font_dir.exists() { std::fs::create_dir_all(&font_dir)?; super::clone_repo(&self.repo_url, &font_dir)?; @@ -79,7 +86,33 @@ impl RepoInfo { sha: self.rev.clone(), }); } + Ok(font_dir) + } + /// Iterate paths to config files in this repo, checking it out if necessary + pub fn iter_configs( + &self, + cache_dir: &Path, + ) -> Result + '_, LoadRepoError> { + let font_dir = self.instantiate(cache_dir)?; + let (left, right) = match super::iter_config_paths(&font_dir) { + Ok(iter) => (Some(iter), None), + Err(_) => (None, None), + }; + let sources_dir = super::find_sources_dir(&font_dir).unwrap_or(font_dir); + Ok(left + .into_iter() + .flatten() + .chain(right) + .map(move |config| sources_dir.join(config))) + } + + /// Return a `Vec` of source files in this respository. + /// + /// If necessary, this will create a new checkout of this repo at + /// '{git_cache_dir}/{repo_org}/{repo_name}'. + pub fn get_sources(&self, git_cache_dir: &Path) -> Result, LoadRepoError> { + let font_dir = self.instantiate(git_cache_dir)?; let source_dir = font_dir.join("sources"); let configs = self .config_files