finalize gix-merge integration #5722
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@Byron is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
Byron
force-pushed
the
finalize-merge-integration
branch
from
f873049 to
34e6042
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
34e6042 to
5856b9d
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
5856b9d to
b40b347
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
b40b347 to
a306692
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
a306692 to
85bf792
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
809e06a to
997720b
Compare
Byron
force-pushed
the
finalize-merge-integration
branch
from
997720b to
8dfbdc7
Compare
There is no good reason why we couldn't do the merge on the fly, and display the information that way. Especially because the result can be easily cached in the frontend. |
|
Other than a few small questions, this is incredibly accurate. Amazing stuff |
|
Thanks a lot for the review - thanks to it I was able to make a change which makes the code more readable. |
crates/gitbutler-repo/src/rebase.rs
Outdated
| let conflicting_entries = index.entries().iter().filter_map(|e| { | ||
| let stage = e.stage(); | ||
| if stage == gix::index::entry::Stage::Unconflicted { | ||
| None | ||
| } else { | ||
| Some((stage, e)) | ||
| } | ||
| }); | ||
| for (stage, entry) in conflicting_entries { | ||
| let storage = match stage { | ||
| Stage::Unconflicted => { | ||
| unreachable!("BUG: filtered above to not contain unconflicted entries") | ||
| } | ||
| Stage::Base => &mut ancestor_entries, | ||
| Stage::Ours => &mut our_entries, | ||
| Stage::Theirs => &mut their_entries, | ||
| }; |
There was a problem hiding this comment.
A great catch, I changed it and it's so much easier to read.
|
All looks good. We're going to merge after we release next |
|
Great! Let me add #5753 to the merge-queue then. |
Note that it also uses rebasing, but that's not supported natively in `gix` yet.
In many places, a test-module is defined to collect all integration tests. However, this doesn't prevent each of these loose modules to become their own test module by default. `autotest = false` prevents this automation.
That way one won't accidentally access private APIs, while staying consistent with a standard Rust crate layout. Personally, I really want to keep modules small and the `src/` tree free of any extras.
Having them makes it harder to search for and remove those added by oneself.
This change affects the entire edit mode, and every rebase.
It has minimal dependencies and can be used everywhere.
* simplify iteration in `extract_conflicted_files`
Byron
force-pushed
the
finalize-merge-integration
branch
from
c64b840 to
2610251
Compare
Replace the remaining uses of
git2::merge_treeswith the respectivegixfunction.This was made possible thanks to two additions:
Follow-up to #5416.
Merging/Cheery-picking/Rebasing is now 3.1x faster! I'd also expect it to be more correct, this means it should be able to resolve more unambiguous conflicts.
Tasks
gixupdategit2::merge_trees()get_commit_index@~10variant and find out what's going on there. Manual comparisons should be easier then.gixonmainNotes for the reviewer
There is one known niche condition in conjunction with tree-to-file or file-to-tree replacements where detected renames can throw of the resolution, leading to worse outcomes than what Git would produce. It's the only known case of this happening and it's unclear how it can be fixed. While writing this, I had an idea on how to fix it maybe, let's hope this comment doesn't have to stay long.- the issue was due to rename-detection in empty blobs, which now is deactivated by default if these renames happen to individual files.stacktests run serially, which was due to a shortcoming in howgix-testtoolslocks its script before execution. However, as these script invocations run in their own tempdir, nothing needs to be locked, so everything can run in parallel. When testing this, it showed that running the gitbutler-cli in debug mode in parallel with 16 cores isn't great, and the runtime increased to 25s compared to 22s while running it serially, while massively increasing the used CPU. Most of the time was kernel time due to filesystem access, so it seems the filesystem doesn't like what's happening. This won't be an issue asgix-testtoolsv0.16 is current, while we use v0.15 here. I also didn't release the patch yet, so there can't be any surprises. At some point though, one would probably try to make these tests better in creating their initial state.Validation
jgit, and set the base of the target-branch invirtual-branches.tomlto7b955048eb86e1c12114554beacb27b329252b15, about 500 commits in the past. Then I applied the6.10branch and hit the update button. This causes a lot of conflicting commits, which were then output to withgitbutler-cli branch status > git2|gix.romfor comparison.gix-mergeversion.git2andgix.Questions
conflicted_filesused? It's a list of ancestor, ours and their files taken directly from the index, but I can't imagine what these duplicate sets of files are good for. In theory, these wouldn't have to be stored - instead one re-does the merge without merge-strategy and then builds the conflicting index for presentation to the user.Performance
Cherry-Pick
The test setup is a
jgitrepository which rebases 500 commits by using a workspace update. The branch used isstable-6.10and the 'base' commit (invirtual_branches.toml) is7b955048eb86e1c12114554beacb27b329252b15.Before
integrate_upstreamtakes 13.80s. It's notable that the cherry-pick merge runs twice.After
integrate_upstreamis down to 4.43s, a 3.1x improvement.And according to the numbers with an updated diffing engine we land at 3.8s, a 3.6x improvement.