Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
437 changes: 255 additions & 182 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ resolver = "2"
[workspace.dependencies]
bstr = "1.11.1"
# Add the `tracing` or `tracing-detail` features to see more of gitoxide in the logs. Useful to see which programs it invokes.
gix = { git = "https://github.com/GitoxideLabs/gitoxide", rev = "faa0cdeb35a8135ff9513a1c9884126f6b080f4a", default-features = false, features = [] }
gix = { git = "https://github.com/GitoxideLabs/gitoxide", rev = "af704f57bb9480c47cdd393465264d586f1d4562", default-features = false, features = [] }
git2 = { version = "0.20.0", features = [
"vendored-openssl",
"vendored-libgit2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use gitbutler_error::error::Marker;
use gitbutler_oplog::SnapshotExt;
use gitbutler_oxidize::GixRepositoryExt;
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_project::AUTO_TRACK_LIMIT_BYTES;
use gitbutler_reference::{Refname, RemoteRefname};
use gitbutler_repo::logging::{LogUntil, RepositoryExt as _};
use gitbutler_repo::{
Expand Down Expand Up @@ -305,7 +306,7 @@ impl BranchManager<'_> {

// We don't support having two branches applied that conflict with each other
{
let uncommited_changes_tree_id = repo.create_wd_tree()?.id();
let uncommited_changes_tree_id = repo.create_wd_tree(AUTO_TRACK_LIMIT_BYTES)?.id();
let gix_repo = self.ctx.gix_repository_for_merging_non_persisting()?;
let merges_cleanly = gix_repo
.merges_cleanly_compat(
Expand Down
3 changes: 2 additions & 1 deletion crates/gitbutler-branch-actions/src/virtual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use gitbutler_oxidize::{
git2_signature_to_gix_signature, git2_to_gix_object_id, gix_to_git2_oid, GixRepositoryExt,
};
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_project::AUTO_TRACK_LIMIT_BYTES;
use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname};
use gitbutler_repo::{
logging::{LogUntil, RepositoryExt as _},
Expand Down Expand Up @@ -1089,7 +1090,7 @@ pub fn is_remote_branch_mergeable(

let base_tree = find_base_tree(ctx.repo(), &branch_commit, &target_commit)?;

let wd_tree = ctx.repo().create_wd_tree()?;
let wd_tree = ctx.repo().create_wd_tree(AUTO_TRACK_LIMIT_BYTES)?;

let branch_tree = branch_commit.tree().context("failed to find branch tree")?;
let gix_repo_in_memory = ctx.gix_repository_for_merging()?.with_object_memory();
Expand Down
4 changes: 3 additions & 1 deletion crates/gitbutler-edit-mode/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,9 @@ pub(crate) fn save_and_return_to_workspace(
let parents = commit.parents().collect::<Vec<_>>();

// Recommit commit
let tree = repository.create_wd_tree()?;
// While we perform hard resets we should pick up everything to avoid loosing worktree state.
let pick_up_untracked_files_of_any_size = 0;
let tree = repository.create_wd_tree(pick_up_untracked_files_of_any_size)?;

let (_, committer) = repository.signatures()?;
let commit_headers = commit
Expand Down
10 changes: 4 additions & 6 deletions crates/gitbutler-oplog/src/oplog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use gitbutler_oxidize::{
};
use gitbutler_project::{
access::{WorktreeReadPermission, WorktreeWritePermission},
Project,
Project, AUTO_TRACK_LIMIT_BYTES,
};
use gitbutler_repo::RepositoryExt;
use gitbutler_repo::SignaturePurpose;
Expand All @@ -30,8 +30,6 @@ use gix::object::tree::diff::Change;
use gix::prelude::ObjectIdExt;
use tracing::instrument;

const SNAPSHOT_FILE_LIMIT_BYTES: u64 = 32 * 1024 * 1024;

/// The Oplog allows for crating snapshots of the current state of the project as well as restoring to a previous snapshot.
/// Snapshots include the state of the working directory as well as all additional GitButler state (e.g. virtual branches, conflict state).
/// The data is stored as git trees in the following shape:
Expand Down Expand Up @@ -312,7 +310,7 @@ impl OplogExt for Project {
let old_wd_tree_id = tree_from_applied_vbranches(&gix_repo, commit.parent(0)?.id())?;
let old_wd_tree = repo.find_tree(old_wd_tree_id)?;

repo.ignore_large_files_in_diffs(SNAPSHOT_FILE_LIMIT_BYTES)?;
repo.ignore_large_files_in_diffs(AUTO_TRACK_LIMIT_BYTES)?;

let mut diff_opts = git2::DiffOptions::new();
diff_opts
Expand Down Expand Up @@ -602,7 +600,7 @@ fn restore_snapshot(
let workdir_tree_id = tree_from_applied_vbranches(&gix_repo, snapshot_commit_id)?;
let workdir_tree = repo.find_tree(workdir_tree_id)?;

repo.ignore_large_files_in_diffs(SNAPSHOT_FILE_LIMIT_BYTES)?;
repo.ignore_large_files_in_diffs(AUTO_TRACK_LIMIT_BYTES)?;

// Define the checkout builder
let mut checkout_builder = git2::build::CheckoutBuilder::new();
Expand Down Expand Up @@ -739,7 +737,7 @@ fn lines_since_snapshot(project: &Project, repo: &git2::Repository) -> Result<us
// This looks at the diff between the tree of the currently selected as 'default' branch (where new changes go)
// and that same tree in the last snapshot. For some reason, comparing workdir to the workdir subree from
// the snapshot simply does not give us what we need here, so instead using tree to tree comparison.
repo.ignore_large_files_in_diffs(SNAPSHOT_FILE_LIMIT_BYTES)?;
repo.ignore_large_files_in_diffs(AUTO_TRACK_LIMIT_BYTES)?;

let oplog_state = OplogHandle::new(&project.gb_dir());
let Some(oplog_commit_id) = oplog_state.oplog_head()? else {
Expand Down
4 changes: 4 additions & 0 deletions crates/gitbutler-project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@ pub fn configure_git2() {
// These settings are only changed from `main` of applications.
git2::opts::strict_object_creation(false);
}

/// The maximum size of files to automatically start tracking, i.e. untracked files we pick up for tree-creation.
/// **Inactive for now** while it's hard to tell if it's safe *not* to pick up everything.
pub const AUTO_TRACK_LIMIT_BYTES: u64 = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are cases where not disregarding huge files will lock up the app. For instance the case where somebody has a databse in the repo root - not ignored, but never intend to stage and commit that. Or an accidentally moving a large file in the repo.
Perhaps we can teach the edit mode to perform hard resets excluding these files? In terms of frequency of occurrence, it is much more likely to hit the case of an accidentally trying to add a huge file to a tree than to reset it via edit mode...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't agree more.

For this PR, I'd hope that maintaining the current behaviour of create_wd_tree() picking up everything independently of size will be acceptable, as it at least won't make anything worse.

Then, for the future, I'd hope GB can be made to…

  • …use the .git/index when creating commits for the user (but won't use the .git/index when creating internal commits, like for the oplog as it's more efficient)
  • …never do hard resets
  • …to leave untracked files alone just like Git does
    • …while barking if there is some clash with an untracked file

It's the "good gitizen" principle so basic expectations that people have built up towards Git will be met.
Maybe there are better ways to do that even, there should be some room for innovation, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that the creation of the wd tree is slow when large files are present.

In my mind, the slowness was with both generating and displaying diffs of large files. As such, we only needed to put the limits in there.

never do hard resets

Is that value compatible with edit mode?

to leave untracked files alone just like Git does

Could you elaborate on what cases you're concerned about here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that the creation of the wd tree is slow when large files are present.

Independently of the UI, creating objects from content of disk will always be limited to 20-30MB/s per core due to ZLIB. This is horribly inefficient especially when said large file is a database that changes all the time while compressed by nature.

Is that value compatible with edit mode?

Git has one mechanism to deal with this, being git stash --include-untracked. But even that I think should only be done once it's clear the untracked files are in the way. The content of the index would need to be stashed anyway to avoid loosing it.

Could you elaborate on what cases you're concerned about here?

It's the expectation that a Git client behaves like Git, and Git won't touch untracked files unless it's explicitly told to (or sometime less explicitly, but it's opt-in always).jj has the advantage of being its own thing so it can redefine what's 'normal' to great effect. To my mind, that's something that GitButler can't do, at least not by default, merely to align with the expectations brought towards it.
Going beyond the 'normal' for Git, of course, is another form on innovation which should be explored to push the envelope, but that can't ever come at the cost of the safety of the user's data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see. I also mixed up the code path that is being changed here with this one

let diff = repo.diff_tree_to_workdir_with_index(Some(&old_tree), Some(&mut diff_opts))?;

1 change: 1 addition & 0 deletions crates/gitbutler-repo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ path = "tests/mod.rs"
gitbutler-testsupport.workspace = true
gitbutler-user.workspace = true
serde_json = { version = "1.0", features = ["std", "arbitrary_precision"] }
insta = "1.41.1"
Loading
Loading