-
Notifications
You must be signed in to change notification settings - Fork 510
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
New crate for calculating hunk dependencies
- Loading branch information
Showing
12 changed files
with
853 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
[package] | ||
name = "gitbutler-hunk-dependency" | ||
version = "0.0.0" | ||
edition = "2021" | ||
authors = ["GitButler <[email protected]>"] | ||
publish = false | ||
|
||
[dependencies] | ||
anyhow = "1.0.86" | ||
git2.workspace = true | ||
gix = { workspace = true, features = [] } | ||
gitbutler-reference.workspace = true | ||
gitbutler-serde.workspace = true | ||
gitbutler-stack.workspace = true | ||
gitbutler-id.workspace = true | ||
itertools = "0.13" | ||
serde = { workspace = true, features = ["std"] } | ||
bstr.workspace = true | ||
tracing.workspace = true | ||
tokio.workspace = true | ||
uuid = { workspace = true, features = ["v4", "fast-rng"] } | ||
|
||
[[test]] | ||
name = "blame" | ||
path = "tests/mod.rs" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
use std::{ | ||
collections::HashMap, | ||
path::{Path, PathBuf}, | ||
}; | ||
|
||
use gitbutler_stack::StackId; | ||
|
||
use crate::{diff::Diff, hunk::DependencyHunk, stack::DependencyStack}; | ||
|
||
/// Calculates dependencies between workspace changes and workspace commits. | ||
/// | ||
/// What we ultimately want to understand is, given an uncommitted change | ||
/// in some file, do the old line numbers intersect with any commmit(s) in | ||
/// the workspace? | ||
/// | ||
/// The problem we have to overcome is that we the workspace changes are | ||
/// produced by diffing the working directory against the workspace commit. | ||
/// It means changes from one stack can offset line numbers in changes from | ||
/// a different stack. The most intuitive way of checking if they touch | ||
/// the same lines is to use regular git blame, but it suffers from two | ||
/// problems, 1) speed and 2) lack of --reverse flag in git2. The latter | ||
/// means we can't detect intersections with deleted lines. | ||
/// | ||
/// If we don't calculate these dependencies correctly it means a user | ||
/// might be able to move a hunk into a stack where it cannot be committed. | ||
/// | ||
/// So the solution here is that we build up the same information we would | ||
/// get from blame by adding diffs together. | ||
#[derive(Debug, Default, PartialEq, Clone)] | ||
pub struct HunkDependencyBuilder { | ||
stacks: HashMap<StackId, DependencyStack>, | ||
} | ||
|
||
impl HunkDependencyBuilder { | ||
pub fn add( | ||
&mut self, | ||
stack_id: StackId, | ||
commit_id: git2::Oid, | ||
path: &PathBuf, | ||
diffs: Vec<Diff>, | ||
) -> anyhow::Result<()> { | ||
if let Some(lane) = self.stacks.get_mut(&stack_id) { | ||
lane.add(stack_id, commit_id, path, diffs)?; | ||
} else { | ||
let mut lane_deps = DependencyStack::default(); | ||
lane_deps.add(stack_id, commit_id, path, diffs)?; | ||
self.stacks.insert(stack_id, lane_deps); | ||
} | ||
Ok(()) | ||
} | ||
|
||
/// Gets an object that can be used to lookup dependencies for a given path. | ||
/// | ||
/// The reasoning for combining the stacks/lanes here, rather than including | ||
/// it where diffs are combined within the branch, is/was to keep the logic | ||
/// simple. In iterating on the code, however, it feels like it might make | ||
/// more sense to go directly to "global" line numbers. | ||
/// | ||
/// The constraint we would need to introduce is that diffs from different | ||
/// stacks cannot intersect with each other. Doing so would mean the workspace | ||
/// is corrupt. | ||
/// | ||
/// TODO: Consider moving most of the code below to path.rs | ||
pub fn get_path(&mut self, path: &Path) -> anyhow::Result<PathDependencyLookup> { | ||
let path_deps = self | ||
.stacks | ||
.values() | ||
.filter(|s| s.contains_path(path)) | ||
.filter_map(|value| value.get_by_path(path)) | ||
.collect::<Vec<_>>(); | ||
// Tracks the cumulative lines added/removed. | ||
let mut line_shift = 0; | ||
// Next hunk to consider for each branch containing path. | ||
let mut pointer_pos: Vec<usize> = vec![0; path_deps.len()]; | ||
let mut result = vec![]; | ||
|
||
loop { | ||
let next_start_lines = path_deps | ||
.iter() | ||
.enumerate() | ||
.map(|(i, path_dep)| path_dep.hunks.get(pointer_pos[i])) | ||
.map(|hunk_dep| hunk_dep.map(|hunk_dep| hunk_dep.start as u32)) | ||
.collect::<Vec<_>>(); | ||
|
||
// Find the | ||
let path_deps_index = next_start_lines | ||
.iter() | ||
.enumerate() // We want to filter out None values, but keep their index. | ||
.filter(|(_, start_line)| start_line.is_some()) | ||
.min_by_key(|&(index, &value)| { | ||
value.unwrap() + next_start_lines[index].unwrap_or(0) | ||
}) | ||
.map(|(index, _)| index); | ||
|
||
if path_deps_index.is_none() { | ||
break; // No more items to process. | ||
} | ||
let stack_index = path_deps_index.unwrap(); | ||
|
||
let pos = pointer_pos[stack_index]; | ||
pointer_pos[stack_index] += 1; | ||
|
||
let path_dep = &path_deps[stack_index]; | ||
let hunk_dep = &path_dep.hunks[pos]; | ||
|
||
result.push(DependencyHunk { | ||
start: hunk_dep.start + line_shift, | ||
..hunk_dep.clone() | ||
}); | ||
line_shift += hunk_dep.line_shift; | ||
} | ||
Ok(PathDependencyLookup { hunk_deps: result }) | ||
} | ||
} | ||
|
||
#[derive(Debug, Default, PartialEq, Clone)] | ||
pub struct PathDependencyLookup { | ||
hunk_deps: Vec<DependencyHunk>, | ||
} | ||
|
||
impl PathDependencyLookup { | ||
pub fn find(self, start: i32, lines: i32) -> Vec<DependencyHunk> { | ||
self.hunk_deps | ||
.into_iter() | ||
.filter(|range| range.intersects(start, lines)) | ||
.collect::<Vec<_>>() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
use anyhow::{anyhow, Context}; | ||
|
||
#[derive(Debug, PartialEq, Clone)] | ||
pub struct Diff { | ||
pub old_start: i32, | ||
pub old_lines: i32, | ||
pub new_start: i32, | ||
pub new_lines: i32, | ||
} | ||
|
||
impl Diff { | ||
pub fn net_lines(&self) -> i32 { | ||
self.new_lines - self.old_lines | ||
} | ||
} | ||
|
||
fn count_context_lines<I, S>(iter: I) -> i32 | ||
where | ||
I: Iterator<Item = S>, | ||
S: AsRef<str>, | ||
{ | ||
iter.take_while(|line| { | ||
let line_ref = line.as_ref(); // Convert to &str | ||
!line_ref.starts_with('-') && !line_ref.starts_with('+') | ||
}) | ||
.fold(0i32, |acc, _| acc + 1) | ||
} | ||
|
||
impl TryFrom<String> for Diff { | ||
fn try_from(value: String) -> Result<Self, anyhow::Error> { | ||
parse_unidiff(value) | ||
} | ||
|
||
type Error = anyhow::Error; | ||
} | ||
|
||
impl TryFrom<&str> for Diff { | ||
fn try_from(value: &str) -> Result<Self, anyhow::Error> { | ||
parse_unidiff(value) | ||
} | ||
|
||
type Error = anyhow::Error; | ||
} | ||
|
||
fn parse_unidiff(value: impl AsRef<str>) -> Result<Diff, anyhow::Error> { | ||
let value = value.as_ref(); | ||
let header = value.lines().next().context("No header found")?; | ||
if !header.starts_with("@@") { | ||
return Err(anyhow!("Malformed undiff hunk")); | ||
} | ||
let parts: Vec<&str> = header.split_whitespace().collect(); | ||
let (old_start, old_lines) = parse_hunk_info(parts[1]); | ||
let (new_start, new_lines) = parse_hunk_info(parts[2]); | ||
let head_context_lines = count_context_lines(value.lines().skip(1).take(3)); | ||
let tail_context_lines = count_context_lines(value.rsplit_terminator('\n').take(3)); | ||
let context_lines = head_context_lines + tail_context_lines; | ||
|
||
Ok(Diff { | ||
old_start: old_start + head_context_lines, | ||
old_lines: old_lines - context_lines, | ||
new_start: new_start + head_context_lines, | ||
new_lines: new_lines - context_lines, | ||
}) | ||
} | ||
|
||
fn parse_hunk_info(hunk_info: &str) -> (i32, i32) { | ||
let hunk_info = hunk_info.trim_start_matches(&['-', '+'][..]); // Remove the leading '-' or '+' | ||
let parts: Vec<&str> = hunk_info.split(',').collect(); | ||
let start = parts[0].parse().unwrap(); | ||
let lines = if parts.len() > 1 { | ||
parts[1].parse().unwrap() | ||
} else { | ||
1 | ||
}; | ||
(start, lines) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
use gitbutler_stack::StackId; | ||
|
||
#[derive(Debug, PartialEq, Clone)] | ||
pub struct DependencyHunk { | ||
pub stack_id: StackId, | ||
pub commit_id: git2::Oid, | ||
pub start: i32, | ||
pub lines: i32, | ||
pub line_shift: i32, | ||
} | ||
|
||
impl DependencyHunk { | ||
fn end(&self) -> i32 { | ||
self.start + self.lines - 1 | ||
} | ||
|
||
pub fn intersects(&self, start: i32, lines: i32) -> bool { | ||
self.end() >= start && self.start < start + lines | ||
} | ||
|
||
pub fn contains_new_lines(&self, start: i32, lines: i32) -> bool { | ||
start > self.start && start + lines <= self.end() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pub mod builder; | ||
pub mod diff; | ||
pub mod hunk; | ||
pub mod path; | ||
pub mod stack; |
Oops, something went wrong.