feat: vcs: support Jujutsu as a diff-provider

pull/12022/head
Alexis (Poliorcetics) Bourget 2 weeks ago
parent 52c4a28fae
commit 422e761e73
No known key found for this signature in database

@ -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"

@ -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"

@ -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<Vec<u8>> {
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<Arc<ArcSwap<Box<str>>>> {
// See <https://github.com/martinvonz/jj/blob/main/docs/templates.md>
//
// 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<FileChange>) -> 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<FileChange> {
if let rename @ Some(_) = find_rename(path) {
return rename;
}
// Syntax: <https://github.com/martinvonz/jj/blob/f9cfa5c9ce0eacd38e961c954e461e5e73067d22/cli/src/diff_util.rs#L97-L101>
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<FileChange> {
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")),
);
}
}

@ -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<Path>, 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<Path>, 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<Path>),
}
#[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<Vec<u8>> {
// 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. `<path>/.git` exists)
#[cfg(feature = "git")]
Git,
/// Possibly a git repo rooted at the stored path (i.e. `<path>/.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)> {
// TODO(poliorcetics): make checking order configurable
let checks: &[(&str, PossibleDiffProvider)] = &[
#[cfg(feature = "jj")]
(".jj", PossibleDiffProvider::JJ),
#[cfg(feature = "git")]
if path.join(".git").try_exists().ok()? {
return Some((path, PossibleDiffProvider::Git));
}
None
}
(".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));
}
}
}
}

@ -1,5 +1,6 @@
use std::path::{Path, PathBuf};
#[derive(Debug, PartialEq, Eq)]
pub enum FileChange {
Untracked {
path: PathBuf,

Loading…
Cancel
Save