New crate for calculating hunk dependencies

[mtsgrd]

Adds new crate with code for calculating dependencies between workspace changes and workspace commits. What we ultimately want to understand is: given an uncommitted change, do the old line numbers intersect with anything committed 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.

TODO:

• Create library
• Full test coverage
• Documentation for each struct

Follow-up PR for integration this code: #5252

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gitbutler-web	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 22, 2024 2:21pm

[krlvi]

@mtsgrd i see that this is still work in progress - one quick suggestions: it would be a good idea to make your internal members/methods/functions pub(crate). That way it is both easier to reason how the library is meant to be used, and also you are not leaking your internals

[mtsgrd]

@krlvi yes, work in progress. Also, thanks, I'll do that. If you have more of that kind of feedback then please keep it coming. :)

[Byron]

I do not envy you for this task, but also think that it's a great idea to solve this issue for good, pinning it down with as many tests as it takes.

Since blame is mentioned, I thought I'd CC @cruessler who is working on a first version of gix blame. Who knows, maybe there is some cross-pollination.

[krlvi]

@krlvi yes, work in progress. Also, thanks, I'll do that. If you have more of that kind of feedback then please keep it coming. :)

Cool i can take a look and leave some notes. What is the entry point of the library as far as usage goes?

[mtsgrd]

@krlvi the builder.rs test is probably the most logical place for you to start reading. *edit: in get_applied_status we'd construct a HunkDependencyBuilder, feed it diffs, and then get a PathDependencyLookup per path we're processing, and ask it for intersections.

Everything should be considered very experimental, I am at the stage where I managed to get it conceptually working, and tidied up a bit. I would expect there to be errors still, but like @Byron says we can lock it down with sufficient testing.

[krlvi]

Since this code is still in an early stage, instead of commenting on individual things, i would like to encourage you to consider this from an API design standpoint.

For example, instead of objects and builders, think about

• What result do you want to provide to the caller (what is the format of that result that makes sense outside of the internals of this implementation.
• What input do you need, at the highest level, to produce that result

Currently it appears that the caller would need to have a mutable HunkDependencyBuilder to which the user calls .add. It is not clear, how do i get the result? Which part is the result? (is it get_path?).

Instead a mutable struct you can consider getting the full structure as an arg. If you really want to use a mutable struct, perhaps the .add function can take a typed struct as an argument, so that you can document what is supposed to come in.

I hope this is helpful

[mtsgrd]

Instead a mutable struct you can consider getting the full structure as an arg.

Sounds obvious now that you say it, thanks.