From 290fec881a4ba0bc3e02fcc4f7a5f1517642d2c5 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 --- Cargo.lock | 1 + helix-term/Cargo.toml | 4 +- helix-vcs/Cargo.toml | 3 + helix-vcs/src/jj.rs | 531 ++++++++++++++++++++++++++++++++++++++++ helix-vcs/src/lib.rs | 91 +++++-- helix-vcs/src/status.rs | 1 + 6 files changed, 611 insertions(+), 20 deletions(-) create mode 100644 helix-vcs/src/jj.rs diff --git a/Cargo.lock b/Cargo.lock index c74239d5..c825778e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1742,6 +1742,7 @@ dependencies = [ "helix-event", "imara-diff", "log", + "memchr", "parking_lot", "tempfile", "tokio", diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 2f3809da..bd4743a4 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -31,10 +31,12 @@ assets = [ ] [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 96f6206d..3ff74ebf 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -21,9 +21,12 @@ gix = { version = "0.85.0", features = ["attributes", "parallel", "status", "max imara-diff = "0.2.0" anyhow = "1" log = "0.4" +memchr = { version = "2.7", optional = true } +tempfile = { version = "3.13", optional = true } [features] git = ["gix"] +jj = ["memchr", "tempfile"] [dev-dependencies] tempfile.workspace = true diff --git a/helix-vcs/src/jj.rs b/helix-vcs/src/jj.rs new file mode 100644 index 00000000..473279dd --- /dev/null +++ b/helix-vcs/src/jj.rs @@ -0,0 +1,531 @@ +//! 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::ffi::OsStr; +use std::path::{Path, PathBuf}; +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")?; + + /// Helper function to run `jj diff` with the same default arguments + fn run_jj_diff( + repo: &Path, + file: &Path, + option: impl AsRef, + value: impl AsRef, + ) -> Result> { + let output = jj_command(repo) + .args([ + "diff".as_ref(), + // Work with current revision only. + "--revision".as_ref(), + "@".as_ref(), + option.as_ref(), + value.as_ref(), + // If the filepath starts with `-` or `--` it could be interpreted as an option or + // flag, which is not what the user will want and could at worst allow malicious + // repos to run destructive commands. + "--".as_ref(), + // Restrict the diff to the current file + file.as_ref(), + ]) + .output() + .context("failed to execute `jj diff` to get diff base")?; + + anyhow::ensure!( + output.status.success(), + "`jj diff` executed but failed for {file:?}" + ); + + Ok(output.stdout) + } + + let stdout = run_jj_diff( + repo, + file, + // Check if file is newly added, existing or unmodified + "--template", + "status_char", + )?; + + match stdout.trim_ascii().first().copied() { + // File existed in previous change and has no diff + None => std::fs::read(file).context("could not read jj diff base from existing file"), + // File is new from current change + Some(b'A' | b'C') => Ok(Vec::new()), + // File existed in previous change and has diff, in which case we need to get the diff base + Some(_) => { + 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 _stdout = run_jj_diff( + repo, + file, + // Pass custom diff configuration. + "--config", + format!( + "ui.diff-formatter=['{exe}', '$left/{base}', '{target}']", + exe = copy_bin, + base = file_relative_to_root.display(), + // Where to copy the jujutsu-provided file + target = tmppath.display(), + ), + )?; + std::fs::read(tmppath).context("could not read jj diff base from target") + } + } +} + +pub(crate) fn get_current_head_name(repo: &Path) -> Result>>> { + let out = jj_command(repo) + .args([ + "log", + "--no-graph", + // Includes from last immutable revision to current change. + "--revisions", + // + "immutable_heads()::@ & bookmarks()", + "--template", + // See + // + // This will produce the following: + // + // quvlrxss + // kmlpqmrv main-1 + // sxrrsnun main-2 main-3* + // kzqnuykl + // + // There will be a `*` when a bookmark has been modified compared to its remote. + // + // We use a short ID with 8 characters because in practice the change ID is extremely + // unlikely to conflict since we only consider mutable commits (like most jj commands + // will do by default) and this leaves space for bookmarks to appear in the status bar + // even on narrower screens. + r#"change_id.short(8) ++ " " ++ bookmarks ++ "\n""#, + ]) + .output()?; + + anyhow::ensure!(out.status.success(), "`jj log` executed but failed"); + + let output = String::from_utf8(out.stdout).context("`jj log` did not output valid UTF-8")?; + let head_text = extract_head_name(&output)?; + + Ok(Arc::new(ArcSwap::from_pointee(head_text.into()))) +} + +pub(crate) fn for_each_changed_file( + repo: &Path, + callback: impl Fn(Result) -> bool, +) -> Result<()> { + // The forward slash is the only character that is disallowed in both Unix and Windows paths, + // meaning `//` cannot ever appear in them on any platform. + // + // + // + // Lines will be of the following format (examples) + // + // ``` + // C:text.txt // + // D:added // copied.txt // // copied.txt // file // + // D:renamed // to-rename.txt // file // renamed.txt // file // + // D:removed // to-delete.txt // file // to-delete.txt // // + // D:modified // to-modify.txt // file // to-modify.txt // file // + // ``` + // + // Note we use `//\n` as the end delimiter to allow for files that contains `\n` in their name. + // + // For the file types, we will only concern ourselves with `file` and `symlink`, anything else + // will get dropped just like `git.rs` does. + let template = r#"concat( + conflicted_files + .map(|file| concat("C:", file.path().display(), " //\n")) + .join(""), + diff + .files() + .filter(|file| !conflict || conflicted_files.all(|c| c.path().display() != file.path().display())) + .map(|file| concat( + "D:", + file.status(), + " // ", + file.source().path().display(), + " // ", + file.source().file_type(), + " // ", + file.target().path().display(), + " // ", + file.target().file_type(), + " //\n", + )) + .join(""), + )"#; + + let out = jj_command(repo) + .args([ + "show", + // Work with current revision only. + "@", + "--no-patch", + "--template", + template, + ]) + .output()?; + + if !out.status.success() { + log::warn!("`jj show` executed but failed:\n{out:#?}"); + anyhow::bail!("`jj show` executed but failed"); + } + + for entry in split_double_slash(&out.stdout, true) { + let change = match entry { + [b'C', b':', rest @ ..] => FileChange::Conflict { + path: make_pathbuf(rest), + }, + [b'D', b':', rest @ ..] => match entry_to_change(rest) { + Some(change) => change, + None => continue, + }, + _ => continue, + }; + + if !callback(Ok(change)) { + return Ok(()); + } + } + + Ok(()) +} + +pub(crate) fn open_repo(repo_path: &Path) -> Result<()> { + assert!( + repo_path.join(".jj").exists(), + "no .jj where one was expected: {repo_path:?}", + ); + + // Checking that the .jj we found is actually a JJ repo + let status = jj_command(repo_path) + .args(["workspace", "root"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status()?; + + if status.success() { + Ok(()) + } else { + anyhow::bail!("not a valid JJ repo") + } +} + +/// Prepare a JJ command with the common boilerplate +fn jj_command(repo: &Path) -> Command { + // Prevents argument injection when the repo is named `--my-repo`. + // The public interface in `./lib.rs` will canonicalize before trying to look for a JJ repo + // so this is here to avoid issues in case of refactors + assert!( + repo.is_absolute(), + "only absolute paths can be used as jj repo paths" + ); + + let mut command = Command::new("jj"); + command.args([ + "--color", + "never", + // Will search for diffs and such normally but not update the underlying repository + "--no-integrate-operation", + "--no-pager", + "--quiet", + "--repository", + ]); + command.arg(repo); + command +} + +/// Helper function to make the extracting logic testable +fn extract_head_name(output: &str) -> Result { + let mut lines = output.lines(); + let mut next = || lines.next().and_then(|line| line.split_once(' ')); + + let (rev, exact_bookmarks) = 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")?; + + let head_text = if !exact_bookmarks.is_empty() { + // Parentheses: bookmarks are exactly on current change. + format!("{rev} ({exact_bookmarks})") + } else { + let ancestor_bookmarks = std::iter::from_fn(next) + .map(|e| e.1) + .filter(|s| !s.is_empty()) + .collect::>(); + if ancestor_bookmarks.is_empty() { + // Found no bookmarks amongst ancestors. + rev.to_string() + } else { + // Angle brackets: bookmarks are on ancestors. + format!("{rev} [{}]", ancestor_bookmarks.join(" ").trim()) + } + }; + + Ok(head_text) +} + +/// Associate a status to a `FileChange`. +/// +/// Gets something like `modified // conflict.txt // conflict // conflict.txt // file` as input. +fn entry_to_change(entry: &[u8]) -> Option { + let mut sections = split_double_slash(entry, false); + + let kind = sections.next()?; + + let source_path = sections.next()?; + let source_file_type = sections.next()?; + + let target_path = sections.next()?; + let target_file_type = sections.next()?; + + // Never generated in practice but let's be thourough in case that changes. + // + if target_file_type == b"conflict" { + return Some(FileChange::Conflict { + path: make_pathbuf(target_path), + }); + } + + let file_types = [ + // The empty file type is used when the file didn't exist before or doesn't exist now, + // e.g. when added or removed. + "".as_bytes(), + "conflict".as_bytes(), + "file".as_bytes(), + "symlink".as_bytes(), + ]; + if !file_types.contains(&source_file_type) || !file_types.contains(&target_file_type) { + return None; + } + + let change = match kind { + b"added" | b"copied" => FileChange::Untracked { + path: make_pathbuf(target_path), + }, + b"modified" => FileChange::Modified { + path: make_pathbuf(target_path), + }, + b"removed" => FileChange::Deleted { + path: make_pathbuf(target_path), + }, + b"renamed" => FileChange::Renamed { + from_path: make_pathbuf(source_path), + to_path: make_pathbuf(target_path), + }, + _ => return None, + }; + + Some(change) +} + +#[cfg(any(unix, target_os = "wasi"))] +fn make_pathbuf(sl: &[u8]) -> PathBuf { + #[cfg(unix)] + use std::os::unix::ffi::OsStrExt; + #[cfg(target_os = "wasi")] + use std::os::wasi::ffi::OsStrExt; + + PathBuf::from(std::ffi::OsStr::from_bytes(sl)) +} + +// Imperfect fallback for platforms where we don't know about an always-correct method. +// In practice, non-UTF8 paths are vanishingly rare and should not be an issue for anyone running a +// Rust binary like Helix. +#[cfg(not(any(unix, target_os = "wasi")))] +fn make_pathbuf(sl: &[u8]) -> PathBuf { + let s = String::from_utf8_lossy(sl); + PathBuf::from(s.into_owned()) +} + +/// Split a byte slice on either ` // ` or ` //\n` depending on `with_newline`. +fn split_double_slash(slice: &[u8], with_newline: bool) -> impl Iterator { + let mut done = false; + let mut rest = slice; + let needle = if with_newline { " //\n" } else { " // " }.as_bytes(); + std::iter::from_fn(move || { + if done { + return None; + } + let result = match memchr::memmem::find(rest, needle) { + Some(pos) => { + // We use the non-panicking variants to avoid adding the panic machinery here when + // we know it won't ever panic in practice (unless there is a bug in memchr, which + // is unlikely given how much the crate is used). + let (before, after) = rest.split_at_checked(pos).unwrap_or_default(); + rest = after.get(4..).unwrap_or_default(); + before + } + None => { + done = true; + rest + } + }; + Some(result) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_split_double_slash_no_newline() { + let input = b"modified // test.rs // file // test.rs // file //\n"; + let expected = [ + "modified".as_bytes(), + "test.rs".as_bytes(), + "file".as_bytes(), + "test.rs".as_bytes(), + "file //\n".as_bytes(), // Not trimmed since we're not splitting on newlines + ]; + + let result = split_double_slash(input, false).collect::>(); + + assert_eq!(result, expected); + } + + #[test] + fn test_split_double_slash_with_newline() { + let input = concat!( + "modified // test.rs // file // test.rs // file //\n", + "modified // test.rs // file // test.rs // file //\n", + ) + .as_bytes(); + let expected = [ + "modified // test.rs // file // test.rs // file".as_bytes(), + "modified // test.rs // file // test.rs // file".as_bytes(), + // We expect an empty slice after the last split + &[], + ]; + + let result = split_double_slash(input, true).collect::>(); + + assert_eq!(result, expected); + } + + #[test] + fn test_entry_to_change() { + let p = "helix-vcs/src/lib.rs"; + let pb = PathBuf::from(p); + + let entry = |kind, (t1, t2)| { + entry_to_change(format!("{kind} // {p} // {t1} // {p} // {t2}").as_bytes()) + }; + + for types in [ + ("conflict", "file"), + ("conflict", "symlink"), + ("file", "file"), + ("file", "symlink"), + ("symlink", "file"), + ("symlink", "symlink"), + ] { + assert_eq!( + entry("modified", types).unwrap(), + FileChange::Modified { path: pb.clone() } + ); + } + + for types in [("", "file"), ("", "symlink")] { + assert_eq!( + entry("added", types).unwrap(), + FileChange::Untracked { path: pb.clone() } + ); + } + + for types in [ + ("file", "file"), + ("file", "symlink"), + ("symlink", "file"), + ("symlink", "symlink"), + ] { + assert_eq!( + entry("copied", types).unwrap(), + FileChange::Untracked { path: pb.clone() } + ); + } + + for types in [("conflict", ""), ("file", ""), ("symlink", "")] { + assert_eq!( + entry("removed", types).unwrap(), + FileChange::Deleted { path: pb.clone() } + ); + } + + for types in [ + ("", "conflict"), + ("conflict", "conflict"), + ("file", "conflict"), + ("symlink", "conflict"), + ] { + assert_eq!( + entry("conflict", types).unwrap(), + FileChange::Conflict { path: pb.clone() } + ); + } + + for invalid_kind in ["invalid", ""] { + assert_eq!(entry(invalid_kind, ("file", "file")), None); + } + + for invalid_types in [ + ("tree", "file"), + ("submodule", "file"), + ("abcdef", "file"), + ("file", "tree"), + ("file", "submodule"), + ("file", "abcdef"), + ] { + assert_eq!(entry("modified", invalid_types), None); + } + } + + #[test] + fn test_extract_head_name() { + // No bookmarks. + let result = extract_head_name("abcdefgh \nijklmnop \n").unwrap(); + assert_eq!(result, "abcdefgh"); + + // Single exact bookmark. + let result = extract_head_name("abcdefgh bookmark*\nijklmnop other-bookmark*\n").unwrap(); + assert_eq!(result, "abcdefgh (bookmark*)"); + + // Multiple exact bookmarks. + let result = extract_head_name(concat!( + "abcdefgh bookmark bookmark-v2\n", + "ijklmnop other-ookmark\n", + )) + .unwrap(); + assert_eq!(result, "abcdefgh (bookmark bookmark-v2)"); + + // Single inexact bookmark. + let result = extract_head_name("abcdefgh \nijklmnop other-bookmark\n").unwrap(); + assert_eq!(result, "abcdefgh [other-bookmark]"); + + // Multiple inexact bookmarks. + let result = extract_head_name("abcdefgh \nijklmnop bookmark* bookmark-v2\n").unwrap(); + assert_eq!(result, "abcdefgh [bookmark* bookmark-v2]"); + } +} diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs index 3545b8b7..d0d266b7 100644 --- a/helix-vcs/src/lib.rs +++ b/helix-vcs/src/lib.rs @@ -8,6 +8,8 @@ use std::{collections::HashMap, path::Path, sync::Arc}; #[cfg(feature = "git")] mod git; +#[cfg(feature = "jj")] +mod jj; mod diff; @@ -81,7 +83,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, trust_full: bool) { @@ -99,9 +101,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, trust_full), + #[cfg(feature = "jj")] + PossibleDiffProvider::JJ => self.add_file_jj(repo_path, trust_full), }; match result { @@ -237,12 +241,48 @@ 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, + trust_full: bool, + ) -> Result<(Arc, PossibleDiffProvider)> { + use anyhow::Context; + + if !trust_full { + log::warn!(concat!( + "JJ repositories are only supported when the repository is trusted in Helix", + " because JJ itself does not expose a trust setting yet.\n", + "\n", + "See and ", + "." + )); + anyhow::bail!("Only fully trusted workspaces are supported for JJ repositories"); + } + let repo_path = std::fs::canonicalize(repo_path) + .context("failed to canonicalize potential jj repo path")?; + + // 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")] @@ -250,16 +290,22 @@ pub enum DiffProvider { repo: Box, trust_full: bool, }, + /// 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), } } @@ -267,6 +313,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), } } @@ -274,6 +322,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), } } } @@ -283,6 +333,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 @@ -290,20 +343,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 0240cad1..5ef301b8 100644 --- a/helix-vcs/src/status.rs +++ b/helix-vcs/src/status.rs @@ -1,6 +1,7 @@ use std::path::{Path, PathBuf}; /// States for a file having been changed. +#[derive(Debug, PartialEq, Eq)] pub enum FileChange { /// Not tracked by the VCS. Untracked { path: PathBuf }, -- 2.51.2