From 2391c25e95f728ac25a9c6c0054294546ed460e6 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Mon, 4 Mar 2024 15:37:16 +0000 Subject: [PATCH] surf: add submodule support Git [submodules] are encoded as commits when walking a Git tree. To allow the support of them in `radicle-surf`, the tree entry is checked for this case. The entry will be treated such that it is has a name, prefix, Oid, and the submodule's URL. It's important to note that the Oid is not usable within the context of the repository that is being browsed, since they are entirely separate. Instead, the URL should be used in conjunction with the Oid for browsing purposes outside of `radicle-surf`. However, in a future iteration it may be considered to be able to browse the local contents of the submodule, if possible. N.b. getting the submodule and its URL is not supported for a bare repository, since it will complain about not having a working tree. [submodules]: https://git-scm.com/book/en/v2/Git-Tools-Submodules Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- .gitmodules | 3 + radicle-surf/Cargo.toml | 4 + radicle-surf/data/git-platinum | 1 + radicle-surf/examples/browsing.rs | 1 + radicle-surf/src/fs.rs | 138 +++++++++++++++++++++++++++- radicle-surf/src/repo.rs | 8 ++ radicle-surf/src/tree.rs | 20 +++- radicle-surf/t/src/code_browsing.rs | 1 + radicle-surf/t/src/submodule.rs | 21 ++++- 9 files changed, 186 insertions(+), 11 deletions(-) create mode 100644 .gitmodules create mode 160000 radicle-surf/data/git-platinum diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..524b7f3 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "git-platinum"] + path = radicle-surf/data/git-platinum + url = https://github.com/radicle-dev/git-platinum.git diff --git a/radicle-surf/Cargo.toml b/radicle-surf/Cargo.toml index e52dafa..58a3cae 100644 --- a/radicle-surf/Cargo.toml +++ b/radicle-surf/Cargo.toml @@ -26,6 +26,7 @@ doctest = false # to ignore the test on CI. gh-actions = [] minicbor = ["radicle-git-ext/minicbor"] +serde = ["dep:serde", "url/serde"] [dependencies] base64 = "0.13" @@ -52,6 +53,9 @@ version = "1" features = ["serde_derive"] optional = true +[dependencies.url] +version = "2.5" + [build-dependencies] anyhow = "1.0" flate2 = "1" diff --git a/radicle-surf/data/git-platinum b/radicle-surf/data/git-platinum new file mode 160000 index 0000000..27acd68 --- /dev/null +++ b/radicle-surf/data/git-platinum @@ -0,0 +1 @@ +Subproject commit 27acd68c7504755aa11023300890bb85bbd69d45 diff --git a/radicle-surf/examples/browsing.rs b/radicle-surf/examples/browsing.rs index 3df8c5a..c0abd39 100644 --- a/radicle-surf/examples/browsing.rs +++ b/radicle-surf/examples/browsing.rs @@ -55,6 +55,7 @@ fn print_directory(d: &Directory, repo: &Repository, indent_level: usize) { match entry { fs::Entry::File(f) => println!(" {}{}", &indent, f.name()), fs::Entry::Directory(d) => print_directory(&d, repo, indent_level + 1), + fs::Entry::Submodule(s) => println!(" {}{}", &indent, s.name()), } } } diff --git a/radicle-surf/src/fs.rs b/radicle-surf/src/fs.rs index e03518d..47e52c0 100644 --- a/radicle-surf/src/fs.rs +++ b/radicle-surf/src/fs.rs @@ -30,6 +30,7 @@ use std::{ use git2::Blob; use radicle_git_ext::{is_not_found_err, Oid}; use radicle_std_ext::result::ResultExt as _; +use url::Url; use crate::{Repository, Revision}; @@ -52,6 +53,8 @@ pub mod error { Utf8Error, #[error("the path {0} not found")] PathNotFound(PathBuf), + #[error(transparent)] + Submodule(#[from] Submodule), } #[derive(Debug, Error, PartialEq)] @@ -59,6 +62,23 @@ pub mod error { #[error(transparent)] Git(#[from] git2::Error), } + + #[derive(Debug, Error, PartialEq)] + pub enum Submodule { + #[error("URL is invalid utf-8 for submodule '{name}': {err}")] + Utf8 { + name: String, + #[source] + err: std::str::Utf8Error, + }, + #[error("failed to parse URL '{url}' for submodule '{name}': {err}")] + ParseUrl { + name: String, + url: String, + #[source] + err: url::ParseError, + }, + } } /// A `File` in a git repository. @@ -198,6 +218,8 @@ pub enum Entry { File(File), /// A sub-directory of a [`Directory`]. Directory(Directory), + /// An entry points to a submodule. + Submodule(Submodule), } impl PartialOrd for Entry { @@ -211,19 +233,25 @@ impl Ord for Entry { match (self, other) { (Entry::File(x), Entry::File(y)) => x.name().cmp(y.name()), (Entry::File(_), Entry::Directory(_)) => Ordering::Less, + (Entry::File(_), Entry::Submodule(_)) => Ordering::Less, (Entry::Directory(_), Entry::File(_)) => Ordering::Greater, + (Entry::Submodule(_), Entry::File(_)) => Ordering::Less, (Entry::Directory(x), Entry::Directory(y)) => x.name().cmp(y.name()), + (Entry::Directory(x), Entry::Submodule(y)) => x.name().cmp(y.name()), + (Entry::Submodule(x), Entry::Directory(y)) => x.name().cmp(y.name()), + (Entry::Submodule(x), Entry::Submodule(y)) => x.name().cmp(y.name()), } } } impl Entry { - /// Get a label for the `Entriess`, either the name of the [`File`] - /// or the name of the [`Directory`]. + /// Get a label for the `Entriess`, either the name of the [`File`], + /// the name of the [`Directory`], or the name of the [`Submodule`]. pub fn name(&self) -> &String { match self { Entry::File(file) => &file.name, Entry::Directory(directory) => directory.name(), + Entry::Submodule(submodule) => submodule.name(), } } @@ -231,6 +259,7 @@ impl Entry { match self { Entry::File(file) => file.path(), Entry::Directory(directory) => directory.path(), + Entry::Submodule(submodule) => submodule.path(), } } @@ -238,6 +267,7 @@ impl Entry { match self { Entry::File(file) => file.location(), Entry::Directory(directory) => directory.location(), + Entry::Submodule(submodule) => submodule.location(), } } @@ -254,6 +284,7 @@ impl Entry { pub(crate) fn from_entry( entry: &git2::TreeEntry, path: PathBuf, + repo: &Repository, ) -> Result { let name = entry.name().ok_or(error::Directory::Utf8Error)?.to_string(); let id = entry.id().into(); @@ -261,6 +292,12 @@ impl Entry { match entry.kind() { Some(git2::ObjectType::Tree) => Ok(Self::Directory(Directory::new(name, path, id))), Some(git2::ObjectType::Blob) => Ok(Self::File(File::new(name, path, id))), + Some(git2::ObjectType::Commit) => { + let submodule = (!repo.is_bare()) + .then(|| repo.find_submodule(&name)) + .transpose()?; + Ok(Self::Submodule(Submodule::new(name, path, submodule, id)?)) + }, _ => Err(error::Directory::InvalidType(path, "tree or blob")), } } @@ -354,7 +391,7 @@ impl Directory { // Walks only the first level of entries. And `_entry_path` is always // empty for the first level. tree.walk(git2::TreeWalkMode::PreOrder, |_entry_path, entry| { - match Entry::from_entry(entry, path.clone()) { + match Entry::from_entry(entry, path.clone(), repo) { Ok(entry) => match entry { Entry::File(_) => { entries.insert(entry.name().clone(), entry); @@ -365,6 +402,10 @@ impl Directory { // Skip nested directories git2::TreeWalkResult::Skip }, + Entry::Submodule(_) => { + entries.insert(entry.name().clone(), entry); + git2::TreeWalkResult::Ok + }, }, Err(err) => { error = Some(err); @@ -397,7 +438,7 @@ impl Directory { .ok_or_else(|| error::Directory::InvalidPath(path.to_path_buf()))?; let root_path = self.path().join(parent); - Entry::from_entry(&entry, root_path) + Entry::from_entry(&entry, root_path, repo) } /// Find the `Oid`, for a [`File`], found at `path`, if it exists. @@ -447,6 +488,7 @@ impl Directory { self.traverse(repo, 0, &mut |size, entry| match entry { Entry::File(file) => Ok(size + file.content(repo)?.size()), Entry::Directory(dir) => Ok(size + dir.size(repo)?), + Entry::Submodule(_) => Ok(size), }) } @@ -479,6 +521,7 @@ impl Directory { let acc = directory.traverse(repo, acc, f)?; f(acc, entry) }, + Entry::Submodule(_) => f(acc, entry), }) } } @@ -491,6 +534,93 @@ impl Revision for Directory { } } +/// A representation of a Git [submodule] when encountered in a Git +/// repository. +/// +/// [submodule]: https://git-scm.com/book/en/v2/Git-Tools-Submodules +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Submodule { + name: String, + prefix: PathBuf, + id: Oid, + url: Option, +} + +impl Submodule { + /// Construct a new `Submodule`. + /// + /// The `path` must be the prefix location of the directory, and + /// so should not end in `name`. + /// + /// The `id` is the commit pointer that Git provides when listing + /// a submodule. + pub fn new( + name: String, + prefix: PathBuf, + submodule: Option, + id: Oid, + ) -> Result { + let url = submodule + .and_then(|module| { + module + .opt_url_bytes() + .map(|bs| std::str::from_utf8(bs).map(|url| url.to_string())) + }) + .transpose() + .map_err(|err| error::Submodule::Utf8 { + name: name.clone(), + err, + })?; + let url = url + .map(|url| { + Url::parse(&url).map_err(|err| error::Submodule::ParseUrl { + name: name.clone(), + url, + err, + }) + }) + .transpose()?; + Ok(Self { + name, + prefix, + id, + url, + }) + } + + /// The name of this `Submodule`. + pub fn name(&self) -> &String { + &self.name + } + + /// Return the [`Path`] where this `Submodule` is located, relative to the + /// git repository root. + pub fn location(&self) -> &Path { + &self.prefix + } + + /// Return the exact path for this `Submodule`, including the + /// `name` of the submodule itself. + /// + /// The path is relative to the git repository root. + pub fn path(&self) -> PathBuf { + self.prefix.join(escaped_name(&self.name)) + } + + /// The object identifier of this `Submodule`. + /// + /// Note that this does not exist in the parent `Repository`. A + /// new `Repository` should be opened for the submodule. + pub fn id(&self) -> Oid { + self.id + } + + /// The URL for the submodule, if it is defined. + pub fn url(&self) -> &Option { + &self.url + } +} + /// When we need to escape "\" (represented as `\\`) for `PathBuf` /// so that it can be processed correctly. fn escaped_name(name: &str) -> String { diff --git a/radicle-surf/src/repo.rs b/radicle-surf/src/repo.rs index 7d98f6f..94624e4 100644 --- a/radicle-surf/src/repo.rs +++ b/radicle-surf/src/repo.rs @@ -440,6 +440,14 @@ impl Repository { // Private API, ONLY add `pub(crate) fn` or `fn` in here. // //////////////////////////////////////////////////////////// impl Repository { + pub(crate) fn is_bare(&self) -> bool { + self.inner.is_bare() + } + + pub(crate) fn find_submodule(&self, name: &str) -> Result { + self.inner.find_submodule(name) + } + pub(crate) fn find_blob(&self, oid: Oid) -> Result, git2::Error> { self.inner.find_blob(oid.into()) } diff --git a/radicle-surf/src/tree.rs b/radicle-surf/src/tree.rs index 4f4105b..3cac9b1 100644 --- a/radicle-surf/src/tree.rs +++ b/radicle-surf/src/tree.rs @@ -26,6 +26,7 @@ use serde::{ ser::{SerializeStruct as _, Serializer}, Serialize, }; +use url::Url; use crate::{fs, Commit}; @@ -108,10 +109,11 @@ impl Serialize for Tree { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum EntryKind { Tree(Oid), Blob(Oid), + Submodule { id: Oid, url: Option }, } impl PartialOrd for EntryKind { @@ -123,9 +125,14 @@ impl PartialOrd for EntryKind { impl Ord for EntryKind { fn cmp(&self, other: &Self) -> Ordering { match (self, other) { + (EntryKind::Submodule { .. }, EntryKind::Submodule { .. }) => Ordering::Equal, + (EntryKind::Submodule { .. }, EntryKind::Tree(_)) => Ordering::Equal, + (EntryKind::Tree(_), EntryKind::Submodule { .. }) => Ordering::Equal, (EntryKind::Tree(_), EntryKind::Tree(_)) => Ordering::Equal, (EntryKind::Tree(_), EntryKind::Blob(_)) => Ordering::Less, (EntryKind::Blob(_), EntryKind::Tree(_)) => Ordering::Greater, + (EntryKind::Submodule { .. }, EntryKind::Blob(_)) => Ordering::Less, + (EntryKind::Blob(_), EntryKind::Submodule { .. }) => Ordering::Greater, (EntryKind::Blob(_), EntryKind::Blob(_)) => Ordering::Equal, } } @@ -177,6 +184,7 @@ impl Entry { match self.entry { EntryKind::Blob(id) => id, EntryKind::Tree(id) => id, + EntryKind::Submodule { id, .. } => id, } } } @@ -209,6 +217,10 @@ impl From for EntryKind { match entry { fs::Entry::File(f) => EntryKind::Blob(f.id()), fs::Entry::Directory(d) => EntryKind::Tree(d.id()), + fs::Entry::Submodule(u) => EntryKind::Submodule { + id: u.id(), + url: u.url().clone(), + }, } } } @@ -241,7 +253,7 @@ impl Serialize for Entry { where S: Serializer, { - const FIELDS: usize = 4; + const FIELDS: usize = 5; let mut state = serializer.serialize_struct("TreeEntry", FIELDS)?; state.serialize_field("name", &self.name)?; state.serialize_field( @@ -249,8 +261,12 @@ impl Serialize for Entry { match self.entry { EntryKind::Blob(_) => "blob", EntryKind::Tree(_) => "tree", + EntryKind::Submodule { .. } => "submodule", }, )?; + if let EntryKind::Submodule { url: Some(url), .. } = &self.entry { + state.serialize_field("url", url)?; + }; state.serialize_field("oid", &self.object_id())?; state.serialize_field("lastCommit", &self.commit)?; state.end() diff --git a/radicle-surf/t/src/code_browsing.rs b/radicle-surf/t/src/code_browsing.rs index 1d9885f..054368f 100644 --- a/radicle-surf/t/src/code_browsing.rs +++ b/radicle-surf/t/src/code_browsing.rs @@ -31,6 +31,7 @@ fn iterate_root_dir_recursive() { match entry { fs::Entry::File(_) => Ok((count + 1, indent_level)), fs::Entry::Directory(_) => Ok((count + 1, indent_level + 1)), + fs::Entry::Submodule(_) => Ok((count + 1, indent_level)), } }, ) diff --git a/radicle-surf/t/src/submodule.rs b/radicle-surf/t/src/submodule.rs index f49abe1..a19d7fc 100644 --- a/radicle-surf/t/src/submodule.rs +++ b/radicle-surf/t/src/submodule.rs @@ -1,10 +1,21 @@ -#[cfg(not(feature = "gh-actions"))] +use std::path::Path; + +use radicle_surf::tree::EntryKind; + #[test] -// An issue with submodules, see: https://github.com/radicle-dev/radicle-surf/issues/54 -fn test_submodule_failure() { +fn test_submodule() { use radicle_git_ext::ref_format::refname; - use radicle_surf::{Branch, Repository}; + use radicle_surf::{fs, Branch, Repository}; let repo = Repository::discover(".").unwrap(); - repo.root_dir(Branch::local(refname!("main"))).unwrap(); + let branch = Branch::local(refname!("surf/submodule-support")); + let dir = repo.root_dir(&branch).unwrap(); + let platinum = dir + .find_entry(&Path::new("radicle-surf/data/git-platinum"), &repo) + .unwrap(); + assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some())); + + let surf = repo.tree(&branch, &Path::new("radicle-surf/data")).unwrap(); + let kind = EntryKind::from(platinum); + assert!(surf.entries().iter().any(|e| e.entry() == &kind)); }