oplog review (part 2)

[Byron]

Follow up to #3867

Tasks

• assure snapshot doesn't happen outside of the integration branch (figure out why it's flaky)
  • don't know why it was flaky though.
• fix worktree-merge when creating a snapshot
• store commit data directly (as object) - not possible with git2, besides, it's safe to assume newline-separation between headers and message.
• assure test index restore
• assure test vbranch-commit restore
• assure test vbranch-tree restore
• assure test integration restore
• review everything else of oplog.rs and assure important parts have basic tests at least

Notes for the Reviewer

• When restoring a snapshot, I have made the integration branch mandatory when restoring it due to the way set_head() is used unconditionally. The prepare-snapshot code is still flexible and treats it as optional.
• The tests are still needed, but I have trouble to align the state of the integration branch with what it should be, so it's hard for me to make assertions as they aren't what I think it should be. I would like to take a break from that.

Open questions

Can the usage of oplog be more abstract?

Right now it's hand-rolled prepare-commit steps everywhere, could that be more 'automatic'?
TBD❗️

General Thoughts

On core.git

• When encountering core::git::* and all its wrapped types I was wondering if this is going to be a god-sent or a burden. Time will tell.
• The git wrapped types don't seem to be used much/consistently within ops, which makes it a potential burden.
• Somehow I think that it's easier to go with git2 while it's there as long as repositories aren't kept around but opened mostly when needed. This is what I have seen. I have also seen a simple API wrapper for configuration handling (get/set) and though that such improvements could be done with so that we'd have git2::Repository::open()?.config().easy().use_easy_api() - this is just a general idea. Right now I can't tell, but I will keep an eye out for that.
• I chose not to replace git2 usage with git except for in the API (where git::Oid is used instead of git2::Oid) until I know what's the right choice here.

On worktree-handling

Due to variable naming, it's not always clear what Project::path is. It's a worktree-directory, and whenever it's used like project.path.join(".git/gitbutler") it's a problem that will prevent GB from dealing with ALL the repository configurations out there as .git doesn't have to be a directory, nor does it have to be there at all. The only way to handle this correctly is to open a repo with gix::open() and then use the git_dir() and worktree_dir() methods respectively to get the actual directories. These do have to be treated separately, but it's definitely a good idea start from a working-tree directory as it matches with the single-worktree paradigm of GB.

When the is a repo already, definitely use its path() (which is its git_dir), instead of doing wd.join(".git/gitbutler/...").

Avoid sha in variable names

As everything should be strongly typed, as opposed to stringly typed, by the type it's already clear that it's a hash. What's not usually clear is what the hash points to. If I read target_sha I don't know much, especially because target is GB lingo for trunk to merge with. Instead, I'd call it target_commit, which tells me something important that's not obvious from the type otherwise.

Don't predict the compiler

Sometimes code feels like it tries to anticipate what the compiler would say, even though the most direct (or intuitive) way of doing things would actually work. It's usually easiest to try first, and then make it work if needed. Sometimes, it just works.

Don't repeat module-names in test-functions

That way, they read well both on the command-line and in IDEs.

From there, it's also easier to find more descriptive names.