30e02b7 Merge remote-tracking branch 'han/update-design' — radicle-git

radicle-git

rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt

Git libraries for Radicle

Merge remote-tracking branch 'han/update-design'

Fintan Halpenny committed 3 years ago

commit 30e02b756fba672efabcb1392b54e25b4b10da3e
parent 502053d

2 files changed +80 -95

modified radicle-surf/docs/refactor-design.md

@@ -1,26 +1,31 @@

 # An updated design for radicle-surf
 This is a design blueprint for the new `radicle-git/radicle-surf` crate. The
 actual design details and implemenation are described and updated in its
 documentation comments, viewable via `cargo doc`.
 ## Introduction
 Now we have ported the `radicle-surf` crate from its own github repo to be
 part of the `radicle-git` repo. We are taking this opportunity to refactor
 its design as well. Intuitively, `radicle-surf` provides an API so that one
 can use it to create a GitHub-like UI for a git repo:
 In September 2022, we have ported the [`radicle-surf` crate](https://github.com/radicle-dev/radicle-surf)
 from its own github repo to be part of the [`radicle-git` repo](https://github.com/radicle-dev/radicle-git).
 We are taking this opportunity to refactor its design as well. Intuitively,
 `radicle-surf` provides an API so that one can use it to create a GitHub-like
 UI for a git repo:
 . Code browsing: given a specific commit/ref, browse files and directories.
 . Diff between two revisions that resolve into two commits.
 . Retrieve the history of commits with a given head, and optionally a file.
 . List refs and retrieve their metadata: Branches, Tags, Remotes,
 Notes and user-defined "categories", where a category is: refs/<category>/<...>.
 Notes and user-defined "categories", where a category is: `refs/<category>/<...>`.
 ## Motivation
 The `radicle-surf` crate aims to provide a safe and easy-to-use API that
 supports the features listed in [Introduction]. Based on the existing API,
 the main goals of the refactoring are:
 supports the features listed in [Introduction](#introduction). Based on the
 existing API, the main goals of the refactoring are:
 - API review: identify the issues with the current API.
 - New API: propose a new API that could reuse parts of the existing API.
 - Updated API: propose an updated API that reuses parts of the existing API.
 - Address open issues in the original `radicle-surf` repo.
 - Be `git` specific. (i.e. no need to support other VCS systems)
 - Remove `git2` from the public API. The use of `git2` should be an

@@ -31,14 +36,14 @@ implementation detail.

 In this section, we review some core types in the current API and propose
 changes to them. The main theme is to make the API simpler and easier to use.
 ### Remove the `Browser`
 ### Remove the `Browser` type
 The type `Browser` is awkward as of today:
 The type [`Browser`](https://github.com/radicle-dev/radicle-surf/blob/b85d2183d786e5fa447aab9d2f420a32f1061bfa/surf/src/vcs.rs#L145) is awkward:
 - it is not a source of truth of any information. For example, `list_branches`
 method is just a wrapper of `Repository::list_branches`.
 - it takes in `History`, but really works at the `Snapshot` level.
 - it is mutable but its state does not help much.
 - it takes in `History`, but really operates at the `Snapshot` level.
 - it is mutable but its state mutations are not used much.
 Can we just remove `Browser` and implement its functionalities using other
 types?

@@ -47,18 +52,19 @@ types?

 - For generating `Directory`, use `Repository` directly given a `Rev`.
 - For accessing `Branch`, `Tag` or `Commit`, use `Repository`.
 ## Remove the `Snapshot` type
 ### Remove the `Snapshot` type
 A `Snapshot` should be really just a tree (or `Directory`) of a `Commit` in
 git. Currently it is a function that returns a `Directory`. Because it is OK
 to be git specific, we don't need to have this generic function to create a
 snapshot across different VCS systems.
 A [`Snapshot`](https://github.com/radicle-dev/radicle-surf/blob/b85d2183d786e5fa447aab9d2f420a32f1061bfa/surf/src/vcs.rs#L140) should be really
 just a tree (or `Directory`) of a `Commit` in git. Currently it is a function
 that returns a `Directory`. As we are moving to be git specific, we don't need
 to have this generic function to create a snapshot across different VCS systems.
 The snapshot function can be easily implement as a method of `RepositoryRef`.
 ## Simplify `Directory` and remove the `Tree` and `Forest` types
 ### Simplify `Directory` and remove the `Tree` and `Forest` types
 The `Directory` type represents the file system view of a snapshot. Its field
 The [`Directory`](https://github.com/radicle-dev/radicle-surf/blob/b85d2183d786e5fa447aab9d2f420a32f1061bfa/surf/src/file_system/directory.rs#L144)
 type represents the file system view of a snapshot. Its field
 `sub_directories` is defined a `Forest` based on `Tree`. The types are
 over-engineered from such a simple concept. We could refactor `Directory` to
 use `DirectoryContents` for its items and not to use `Tree` or `Forest` at all.

@@ -66,7 +72,7 @@ use `DirectoryContents` for its items and not to use `Tree` or `Forest` at all.

 We also found the `list_directory()` method duplicates with `iter()` method.
 Hence `list_directory()` is removed, together with `SystemType` type.
 ## Remove `Vcs` trait
 ### Remove `Vcs` trait
 The `Vcs` trait was introduced to support different version control backends,
 for example both Git and Pijul, and potentially others. However, since this

@@ -80,9 +86,17 @@ would look like and how they meet the requirements.

 ### Basic types
 #### Repository
 `Repository` is kept as the entry point of the API, even though its methods
 would change due to changes in other types. Also, we would like to consolidate
 [`Repository`](https://github.com/radicle-dev/radicle-surf/blob/b85d2183d786e5fa447aab9d2f420a32f1061bfa/surf/src/vcs/git/repo.rs#L53) and
  [`RepositoryRef`](https://github.com/radicle-dev/radicle-surf/blob/b85d2183d786e5fa447aab9d2f420a32f1061bfa/surf/src/vcs/git/repo.rs#L63) to
  simplify the API.
 #### Revision and Commit
 In Git, `Revision` commonly resolves into a `Commit` but could refers to other
 In Git, `Revision` commonly resolves into a `Commit` but could refer to other
 objects for example a `Blob`. Hence we need to keep both concepts in the API.
 Currently we have multiple types to identify a `Commit` or `Revision`.

@@ -91,7 +105,7 @@ Currently we have multiple types to identify a `Commit` or `Revision`.

 - Rev
 The relations between them are: all `Rev` and `Commit` can resolve into `Oid`,
 and most `Rev`s can resolve into `Commit`.
 and in most cases `Rev` can resolve into `Commit`.
 On one hand, `Oid` is the ultimate unique identifer but it is more machine-
 friendly than human-friendly. On the other hand, `Revision` is most human-

@@ -148,8 +162,7 @@ enum FilterBy {

 }
 ```
 For the methods provided by `History`, please see section [Retrieve the history]
 (#retrieve-the-history) below.
 For the methods provided by `History`, please see section [Retrieve the history](#retrieve-the-history) below.
 #### Commit

@@ -173,10 +186,10 @@ pub struct Commit {

 }
 ```
 To get the content (i.e. the tree object) of the commit, the user should use
 `snapshot` method described in [Code browsing](#code-browsing) section.
 To get the file system snapshot of the commit, the user should use
 `root_dir` method described in [Code browsing](#code-browsing) section.
 To get the diff of the commit, the user should use `diff_from_parent` method
 To get the diff of the commit, the user should use `diff_commit` method
 described in [Diffs](#diffs) section. Note that we might move that method to
 `Commit` type itself.

@@ -188,7 +201,7 @@ commit. The core API is:

 - Create a root Directory:
 ```Rust
 impl RepositoryRef {
     pub fn snapshot<C: ToCommit>(&self, commit: C) -> Result<Directory, Error>;
     pub fn root_dir<C: ToCommit>(&self, commit: C) -> Result<Directory, Error>;
 }
 ```

@@ -228,25 +241,13 @@ impl RepositoryRef {

 }
 ```
 We used to have the following method:
 ```Rust
     /// Returns the diff between a revision and the initial state of the repo.
     pub fn initial_diff<R: Revision>(&self, rev: R) -> Result<Diff, Error>;
 ```
 However, it is not comparing with any other commit so the output is basically
 the snapshot of `R`. I am not sure if it is necessary. My take is that we can
 remove this method.
 We also have the following method:
 To obtain the diff of a particular commit, we will have:
 ```Rust
     /// Returns the diff of a specific commit.
     pub fn diff_from_parent<C: ToCommit>(&self, commit: C) -> Result<Diff, Error>;
 impl RepositoryRef {
     pub fn diff_commit(&self, commit: impl ToCommit) -> Result<Diff, Error>;
 }
 ```
 I think it is probably better to instead define the above method as
 `Commit::diff()` as its output is associated with a `Commit`.
 ### Retrieve the history
 The user would be able to get the list of previous commits reachable from a

@@ -259,13 +260,14 @@ impl RepositoryRef {

 }
 ```
 `History` implements `Iterator` that produces `Result<Commit, Error>`, and
 also provides these methods:
 To access the `History`, the user simply iterates `History` as it implements the
 `Iterator` trait that produces `Result<Commit, Error>`.
 `History` provides a method to filter based on a path and other helper methods:
 ```Rust
 impl<'a> History<'a> {
     pub fn new<C: ToCommit>(repo: RepositoryRef<'a>, head: C) -> Result<Self, Error>;
     /// Returns the head commit of a history.
     pub fn head(&self) -> &Commit;
     // Modifies a history with a filter by `path`.

@@ -299,58 +301,41 @@ but did not implement it in the current code mainly because the libgit2 doc says

 ### List refs and retrieve their metadata
 Git refs are simple names that point to objects using object IDs. `radicle-surf`
 support refs by its `Ref` type.
 Git refs are simple names that point to objects using object IDs. In this new
 design, we no longer group different refs into a single enum. Instead, each kind
 of ref would be their own type, e.g. `Tag`, `Branch`, `Namespace`, etc.
 To retrieve the refs, the user would call the `<ref_type>s()` method of a
 repo, e.g. `branches()`, `tags()`. The result is an iterator of available refs,
 e.g. `Branches`.
 ```Rust
 pub enum Ref {
     /// A git tag, which can be found under `.git/refs/tags/`.
     Tag {
         /// The name of the tag, e.g. `v1.0.0`.
         name: TagName,
     },
     /// A git branch, which can be found under `.git/refs/heads/`.
     LocalBranch {
         /// The name of the branch, e.g. `master`.
         name: BranchName,
     },
     /// A git branch, which can be found under `.git/refs/remotes/`.
     RemoteBranch {
         /// The remote name, e.g. `origin`.
         remote: String,
         /// The name of the branch, e.g. `master`.
         name: BranchName,
     },
     /// A git namespace, which can be found under `.git/refs/namespaces/`.
     ///
     /// Note that namespaces can be nested.
     Namespace {
         /// The name value of the namespace.
         namespace: String,
         /// The reference under that namespace, e.g. The
         /// `refs/remotes/origin/master/ portion of `refs/namespaces/
         /// moi/refs/remotes/origin/master`.
         reference: Box<Ref>,
     },
     /// A git notes, which can be found under `.git/refs/notes`
     Notes {
         /// The default name is "commits".
         name: String,
     }
 impl RepositoryRef {
     /// Lists branch names with `filter`.
     pub fn branches<G>(&self, pattern: G) -> Result<Branches, Error>
     where
         G: Into<Glob<Branch>>
 }
 ```
 ### Git Objects
 ### Git Tree and Blob
 In Git, a `Blob` object represents the content of a file, and a `Tree` object
 represents the content of a listing (in a directory). A `Tree` or a `Blob` does
 not have its own name because there could be different names (paths) pointing
 to the same `Tree` or `Blob`. In contrast, a `Directory` or a `File` has names
 included and possible other attributes.
 Git has four kinds of objects: Blob, Tree, Commit and Tag. We have already
 discussed `Commit` (a struct) and `Tag` (`Ref::Tag`) types. For `blob`, we
 use `File` type to represent, and for `tree`, we use `Directory` type to
 represent. The motivation is to let the user "surf" a repo as a file system
 as much as possible, and to avoid Git internal concepts in our API if possible.
 In `radicle-surf` API, we would expose `Tree` and `Blob` as defined in Git, i.e.
 as the content only, with some extra helper methods, for example `commit()` that
 returns the commit that created the object.
 Open question: there could be some cases where the names `blob` and `tree`
 shall be used. We need to define such cases clearly if they exist.
 The `Repository` provides methods to retrieve `Tree` or `Blob` for a given path.
 ## Error handling
 ## Summary
 TBD
 This design document was created as a guideline for refactoring the
 `radicle-surf` crate when we move it into the `radicle-git` repo. As the code
 evolves, this document would not be matching the actual code exactly. However
 we can still come back to this document to learn about the history of the
 design and update it when needed.

modified radicle-surf/src/history.rs

@@ -38,7 +38,7 @@ enum FilterBy {

 impl<'a> History<'a> {
     /// Creates a new history starting from `head`, in `repo`.
     pub fn new<C: ToCommit>(repo: &'a Repository, head: C) -> Result<Self, Error> {
     pub(crate) fn new<C: ToCommit>(repo: &'a Repository, head: C) -> Result<Self, Error> {
         let head = head
             .to_commit(repo)
             .map_err(|err| Error::ToCommit(err.into()))?;