Skip to content

fix: assure we convert worktree data to Git before writing it (#5558) #5564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 17, 2024
Merged

fix: assure we convert worktree data to Git before writing it (#5558) #5564

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4645ad2
fix: assure we convert worktree data to Git before writing it (#5558)
Byron Nov 15, 2024
081b76e
Assure all configuration is picked-up, particularly on Windows.
Byron Nov 15, 2024
0eab950
Optimize the way converted data is read back.
Byron Nov 16, 2024
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
22 changes: 19 additions & 3 deletions crates/gitbutler-repo/src/repository_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@ impl RepositoryExt for git2::Repository {
/// or if the HEAD branch has no commits
#[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))]
fn create_wd_tree(&self) -> Result<Tree> {
let gix_repo = gix::open_opts(
self.path(),
gix::open::Options::default().permissions(gix::open::Permissions {
config: gix::open::permissions::Config {
// Whenever we deal with worktree filters, we'd want to have the installation configuration as well.
git_binary: cfg!(windows),
..Default::default()
},
..Default::default()
}),
)?;
let (mut pipeline, index) = gix_repo.filter_pipeline(None)?;
let mut tree_update_builder = git2::build::TreeUpdateBuilder::new();

let worktree_path = self.workdir().context("Could not find worktree path")?;
Expand All @@ -220,6 +232,7 @@ impl RepositoryExt for git2::Repository {
// | add | modify | upsert |
// | modify | modify | upsert |

let mut buf = Vec::with_capacity(1024);
for status_entry in &statuses {
let status = status_entry.status();
let path = status_entry.path().context("Failed to get path")?;
Expand All @@ -241,16 +254,19 @@ impl RepositoryExt for git2::Repository {
let blob = self.blob(path_str.as_bytes())?;
tree_update_builder.upsert(path, blob, git2::FileMode::Link);
} else {
let file = std::fs::read(&file_path)?;
let blob = self.blob(&file)?;
let mut file_for_git =
pipeline.convert_to_git(std::fs::File::open(&file_path)?, path, &index)?;
buf.clear();
std::io::copy(&mut file_for_git, &mut buf)?;
let blob_id = self.blob(&buf)?;
Copy link
Contributor

@Caleb-T-Owens Caleb-T-Owens Nov 15, 2024

Choose a reason for hiding this comment

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

Why do we want to copy the data into the buffer and not just directly pass it to self.blob?

Copy link
Collaborator Author

@Byron Byron Nov 16, 2024

Choose a reason for hiding this comment

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

file_for_git isn't a buffer, it's something that implements std::io::Read, while blob() takes &[u8].

This is the version of the code that is easiest to write, but also slowest as two out of three cases already have a buffer underneath which could be passed directly.

I will adjust it to be the most efficient possible.


let file_type = if is_executable(&file_path.metadata()?) {
git2::FileMode::BlobExecutable
} else {
git2::FileMode::Blob
};

tree_update_builder.upsert(path, blob, file_type);
tree_update_builder.upsert(path, blob_id, file_type);
}
}
}
Expand Down
Loading