From 422e761e73d1d71bbf897fe332243d203adab31f Mon Sep 17 00:00:00 2001 From: "Alexis (Poliorcetics) Bourget" Date: Thu, 7 Nov 2024 13:42:48 +0100 Subject: [PATCH] feat: vcs: support Jujutsu as a diff-provider --- helix-term/Cargo.toml | 4 +- helix-vcs/Cargo.toml | 2 + helix-vcs/src/jj.rs | 282 ++++++++++++++++++++++++++++++++++++++++ helix-vcs/src/lib.rs | 70 +++++++--- helix-vcs/src/status.rs | 1 + 5 files changed, 340 insertions(+), 19 deletions(-) create mode 100644 helix-vcs/src/jj.rs diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 1c399a47c..4fd4275ca 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -13,10 +13,12 @@ repository.workspace = true homepage.workspace = true [features] -default = ["git"] +default = ["git", "jj"] unicode-lines = ["helix-core/unicode-lines", "helix-view/unicode-lines"] integration = ["helix-event/integration_test"] +# VCS features git = ["helix-vcs/git"] +jj = ["helix-vcs/jj"] [[bin]] name = "hx" diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml index 6352eca7e..2a54012e8 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -24,9 +24,11 @@ imara-diff = "0.1.7" anyhow = "1" log = "0.4" +tempfile = { version = "3.13", optional = true } [features] git = ["gix"] +jj = ["tempfile"] [dev-dependencies] tempfile = "3.14" diff --git a/helix-vcs/src/jj.rs b/helix-vcs/src/jj.rs new file mode 100644 index 000000000..10d2b08b8 --- /dev/null +++ b/helix-vcs/src/jj.rs @@ -0,0 +1,282 @@ +//! Jujutsu works with several backends and could add new ones in the future. Private builds of +//! it could also have private backends. Those make it hard to use `jj-lib` since it won't have +//! access to newer or private backends and fail to compute the diffs for them. +//! +//! Instead in case there *is* a diff to base ourselves on, we copy it to a tempfile or just use the +//! current file if not. + +use std::path::Path; +use std::process::Command; +use std::sync::Arc; + +use anyhow::{Context, Result}; +use arc_swap::ArcSwap; + +use crate::FileChange; + +pub(super) fn get_diff_base(repo: &Path, file: &Path) -> Result> { + let file_relative_to_root = file + .strip_prefix(repo) + .context("failed to strip JJ repo root path from file")?; + + let tmpfile = tempfile::NamedTempFile::with_prefix("helix-jj-diff-") + .context("could not create tempfile to save jj diff base")?; + let tmppath = tmpfile.path(); + + let copy_bin = if cfg!(windows) { "copy.exe" } else { "cp" }; + + let status = Command::new("jj") + .arg("--repository") + .arg(repo) + .args([ + "--ignore-working-copy", + "diff", + "--revision", + "@", + "--config-toml", + ]) + // Copy the temporary file provided by jujutsu to a temporary path of our own, + // because the `$left` directory is deleted when `jj` finishes executing. + .arg(format!( + "ui.diff.tool = ['{exe}', '$left/{base}', '{target}']", + exe = copy_bin, + base = file_relative_to_root.display(), + // Where to copy the jujutsu-provided file + target = tmppath.display(), + )) + // Restrict the diff to the current file + .arg(file) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .context("failed to execute jj diff command")?; + + let use_jj_path = status.success() && std::fs::metadata(tmppath).map_or(false, |m| m.len() > 0); + // If the copy call inside `jj diff` succeeded, the tempfile is the one containing the base + // else it's just the original file (so no diff). We check for size since `jj` can return + // 0-sized files when there are no diffs to present for the file. + let diff_base_path = if use_jj_path { tmppath } else { file }; + + // If the command succeeded, it means we either copied the jujutsu base or the current file, + // so there should always be something to read and compare to. + std::fs::read(diff_base_path).context("could not read jj diff base from the target") +} + +pub(crate) fn get_current_head_name(repo: &Path) -> Result>>> { + // See + // + // This will produce the following: + // + // - If there are no branches: `vyvqwlmsvnlkmqrvqktpuluvuknuxpmm` + // - If there is a single branch: `vyvqwlmsvnlkmqrvqktpuluvuknuxpmm (master)` + // - If there are 2+ branches: `vyvqwlmsvnlkmqrvqktpuluvuknuxpmm (master, jj-diffs)` + // + // Always using the long id makes it easy to share it with others, which would not be the + // case for shorter ones: they could have a local change that renders it ambiguous. + let template = r#"separate(" ", change_id, surround("(", ")", branches.join(", ")))"#; + + let out = Command::new("jj") + .arg("--repository") + .arg(repo) + .args([ + "--ignore-working-copy", + "log", + "--color", + "never", + "--revisions", + "@", // Only display the current revision + "--no-graph", + "--no-pager", + "--template", + template, + ]) + .output()?; + + if !out.status.success() { + anyhow::bail!("jj log command executed but failed"); + } + + let out = String::from_utf8(out.stdout)?; + + let rev = out + .lines() + .next() + // Contrary to git, if a JJ repo exists, it always has at least two revisions: + // the root (zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz), which cannot be focused, and the current + // one, which exists even for brand new repos. + .context("should always find at least one line")?; + + Ok(Arc::new(ArcSwap::from_pointee(rev.into()))) +} + +pub(crate) fn for_each_changed_file( + repo: &Path, + callback: impl Fn(Result) -> bool, +) -> Result<()> { + let out = Command::new("jj") + .arg("--repository") + .arg(repo) + .args([ + "--ignore-working-copy", + "diff", + "--color", + "never", + "--revision", + "@", // Only display the current revision + "--no-pager", + "--types", + ]) + .output()?; + + if !out.status.success() { + anyhow::bail!("jj log command executed but failed"); + } + + let out = String::from_utf8(out.stdout)?; + + for line in out.lines() { + let Some((status, path)) = line.split_once(' ') else { + continue; + }; + + let Some(change) = status_to_change(status, path) else { + continue; + }; + + if !callback(Ok(change)) { + break; + } + } + + Ok(()) +} + +pub(crate) fn open_repo(repo_path: &Path) -> Result<()> { + assert!( + repo_path.join(".jj").exists(), + "no .jj where one was expected: {}", + repo_path.display(), + ); + + let status = Command::new("jj") + .args(["--ignore-working-copy", "root"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status()?; + + if status.success() { + Ok(()) + } else { + anyhow::bail!("not a valid JJ repo") + } +} + +/// Associate a status to a `FileChange`. +fn status_to_change(status: &str, path: &str) -> Option { + if let rename @ Some(_) = find_rename(path) { + return rename; + } + + // Syntax: + Some(match status { + "FF" | "LL" | "CF" | "CL" | "FL" | "LF" => FileChange::Modified { path: path.into() }, + "-F" | "-L" => FileChange::Untracked { path: path.into() }, + "F-" | "L-" => FileChange::Deleted { path: path.into() }, + "FC" | "LC" => FileChange::Conflict { path: path.into() }, + // We ignore gitsubmodules here since they not interesting in the context of + // a file editor. + _ => return None, + }) +} + +fn find_rename(path: &str) -> Option { + let (start, rest) = path.split_once('{')?; + let (from, rest) = rest.split_once(" => ")?; + let (to, end) = rest.split_once('}')?; + + Some(FileChange::Renamed { + from_path: format!("{start}{from}{end}").into(), + to_path: format!("{start}{to}{end}").into(), + }) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + #[test] + fn test_status_to_change() { + let p = "helix-vcs/src/lib.rs"; + let pb = PathBuf::from(p); + + for s in ["FF", "LL", "CF", "CL", "FL", "LF"] { + assert_eq!( + status_to_change(s, p).unwrap(), + FileChange::Modified { path: pb.clone() } + ); + } + for s in ["-F", "-L"] { + assert_eq!( + status_to_change(s, p).unwrap(), + FileChange::Untracked { path: pb.clone() } + ); + } + for s in ["F-", "L-"] { + assert_eq!( + status_to_change(s, p).unwrap(), + FileChange::Deleted { path: pb.clone() } + ); + } + for s in ["FC", "LC"] { + assert_eq!( + status_to_change(s, p).unwrap(), + FileChange::Conflict { path: pb.clone() } + ); + } + for s in ["GG", "LG", "ARO", "", " ", " "] { + assert_eq!(status_to_change(s, p), None); + } + } + + #[test] + fn test_find_rename() { + fn check(path: &str, expected: Option<(&str, &str)>) { + let result = find_rename(path); + + assert_eq!( + result, + expected.map(|(f, t)| FileChange::Renamed { + from_path: f.into(), + to_path: t.into() + }) + ) + } + + // No renames + check("helix-term/Cargo.toml", None); + check("helix-term/src/lib.rs", None); + + // Rename of first element in path + check( + "{helix-term => helix-term2}/Cargo.toml", + Some(("helix-term/Cargo.toml", "helix-term2/Cargo.toml")), + ); + // Rename of final element in path + check( + "helix-term/{Cargo.toml => Cargo.toml2}", + Some(("helix-term/Cargo.toml", "helix-term/Cargo.toml2")), + ); + // Rename of a single dir in the middle + check( + "helix-term/{src => src2}/lib.rs", + Some(("helix-term/src/lib.rs", "helix-term/src2/lib.rs")), + ); + // Rename of two dirs in the middle + check( + "helix-term/{src/ui => src2/ui2}/text.rs", + Some(("helix-term/src/ui/text.rs", "helix-term/src2/ui2/text.rs")), + ); + } +} diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs index 2508be6b3..b4b29a79e 100644 --- a/helix-vcs/src/lib.rs +++ b/helix-vcs/src/lib.rs @@ -4,6 +4,8 @@ use std::{collections::HashMap, path::Path, sync::Arc}; #[cfg(feature = "git")] mod git; +#[cfg(feature = "jj")] +mod jj; mod diff; @@ -72,7 +74,7 @@ impl DiffProviderRegistry { } /// Creation and update methods -#[cfg_attr(not(feature = "git"), allow(unused))] +#[cfg_attr(not(any(feature = "git", feature = "jj")), allow(unused))] impl DiffProviderRegistry { /// Register a provider (if any is found) for the given path. pub fn add(&mut self, path: &Path) { @@ -90,9 +92,11 @@ impl DiffProviderRegistry { return; }; - let result = match provider { + let result: Result<(Arc, PossibleDiffProvider)> = match provider { #[cfg(feature = "git")] PossibleDiffProvider::Git => self.add_file_git(repo_path), + #[cfg(feature = "jj")] + PossibleDiffProvider::JJ => self.add_file_jj(repo_path), }; match result { @@ -190,26 +194,49 @@ impl DiffProviderRegistry { Err(err) => Err(err), } } + + /// Add the JJ repo to the known providers *if* it isn't already known. + #[cfg(feature = "jj")] + fn add_file_jj(&mut self, repo_path: &Path) -> Result<(Arc, PossibleDiffProvider)> { + // Don't build a JJ repo object if there is already one for that path. + if let Some((key, DiffProvider::JJ(_))) = self.providers.get_key_value(repo_path) { + return Ok((Arc::clone(key), PossibleDiffProvider::JJ)); + } + + match jj::open_repo(repo_path) { + Ok(()) => { + let key = Arc::from(repo_path); + self.providers + .insert(Arc::clone(&key), DiffProvider::JJ(Arc::clone(&key))); + Ok((key, PossibleDiffProvider::JJ)) + } + Err(err) => Err(err), + } + } } /// A union type that includes all types that implement [DiffProvider]. We need this type to allow /// cloning [DiffProviderRegistry] as `Clone` cannot be used in trait objects. -/// -/// `Copy` is simply to ensure the `clone()` call is the simplest it can be. #[derive(Clone)] pub enum DiffProvider { #[cfg(feature = "git")] Git(gix::ThreadSafeRepository), + /// For [`jujutsu`](https://github.com/martinvonz/jj), we don't use the library but instead we + /// call the binary because it can dynamically load backends, which the JJ library doesn't know about. + #[cfg(feature = "jj")] + JJ(Arc), } -#[cfg_attr(not(feature = "git"), allow(unused))] +#[cfg_attr(not(any(feature = "git", feature = "jj")), allow(unused))] impl DiffProvider { fn get_diff_base(&self, file: &Path) -> Result> { // We need the */ref else we're matching on a reference and Rust considers all references - // inhabited. In our case + // inhabited. match *self { #[cfg(feature = "git")] Self::Git(ref repo) => git::get_diff_base(repo, file), + #[cfg(feature = "jj")] + Self::JJ(ref repo) => jj::get_diff_base(repo, file), } } @@ -217,6 +244,8 @@ impl DiffProvider { match *self { #[cfg(feature = "git")] Self::Git(ref repo) => git::get_current_head_name(repo), + #[cfg(feature = "jj")] + Self::JJ(ref repo) => jj::get_current_head_name(repo), } } @@ -224,6 +253,8 @@ impl DiffProvider { match *self { #[cfg(feature = "git")] Self::Git(ref repo) => git::for_each_changed_file(repo, f), + #[cfg(feature = "jj")] + Self::JJ(ref repo) => jj::for_each_changed_file(repo, f), } } } @@ -233,6 +264,9 @@ pub enum PossibleDiffProvider { /// Possibly a git repo rooted at the stored path (i.e. `/.git` exists) #[cfg(feature = "git")] Git, + /// Possibly a git repo rooted at the stored path (i.e. `/.jj` exists) + #[cfg(feature = "jj")] + JJ, } /// Does *possible* diff provider auto detection. Returns the 'root' of the workspace @@ -240,20 +274,20 @@ pub enum PossibleDiffProvider { /// We say possible because this function doesn't open the actual repository to check if that's /// actually the case. fn get_possible_provider(path: &Path) -> Option<(&Path, PossibleDiffProvider)> { - if cfg!(feature = "git") { - #[cfg_attr(not(feature = "git"), allow(unused))] - fn check_path(path: &Path) -> Option<(&Path, PossibleDiffProvider)> { - #[cfg(feature = "git")] - if path.join(".git").try_exists().ok()? { - return Some((path, PossibleDiffProvider::Git)); - } - - None - } + // TODO(poliorcetics): make checking order configurable + let checks: &[(&str, PossibleDiffProvider)] = &[ + #[cfg(feature = "jj")] + (".jj", PossibleDiffProvider::JJ), + #[cfg(feature = "git")] + (".git", PossibleDiffProvider::Git), + ]; + if !checks.is_empty() { for parent in path.ancestors() { - if let Some(path_and_provider) = check_path(parent) { - return Some(path_and_provider); + for &(repo_indic, pdp) in checks { + if let Ok(true) = parent.join(repo_indic).try_exists() { + return Some((parent, pdp)); + } } } } diff --git a/helix-vcs/src/status.rs b/helix-vcs/src/status.rs index f34334909..b221dc772 100644 --- a/helix-vcs/src/status.rs +++ b/helix-vcs/src/status.rs @@ -1,5 +1,6 @@ use std::path::{Path, PathBuf}; +#[derive(Debug, PartialEq, Eq)] pub enum FileChange { Untracked { path: PathBuf,