-
-
Notifications
You must be signed in to change notification settings - Fork 337
octopus-merge (part 2: blob-merge) #1585
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.
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ae27a5e
to
1d3d258
Compare
@EliahKagan Just as a note in case you'd like to submit a first patch to Git that will definitely be able to land, maybe to warm-up with the mailing-list workflow, then I have something for you. The attribute-documentation about three-way merges contains a reference to However, the git-config documentation doesn't mention it at all. |
0ae3791
to
ff0975c
Compare
Byron
commented
Sep 13, 2024
2f9c91b
to
a6678f9
Compare
ca465ab
to
46a86e9
Compare
…sts for correctness.
This makes it more usable as the value lives longer than the ref itself.
b599a89
to
e864aaa
Compare
efcab99
to
182f9ab
Compare
That way, the platform can be used to perform actual merges. This will also be a good chance to try the API.
This was referenced Sep 30, 2024
This was referenced Oct 12, 2024
Merged
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 6, 2024
They are not Rust code (they are text with conflict markers and a shell command, respectively) and they are not intended as doctests, but the absence of anything on the opening line caused them to be taken as doctests, so `cargo test --workspace --doc` would fail with parsing errors. (Doctests for all crates have not always been run automatically on CI, so this was not caught when these documentation comments were introduced in GitoxideLabs#1585.)
This was referenced Jan 23, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement an octopus merge based on trees, and (mostly) equivalent to
merge-ORT
in Git. The foundation, tree-editing, was implemented in #1566.Related to gitbutlerapp/gitbutler#4793.
Tasks
This PR was re-focussed on blob-based conflict detection and the generation of blob merge-results with conflict markers.
flattenBringmerge
intoplatform
API, similar to how it's done indiff
merge::State
to being able to execute the merge.Pipeline
implementation and testsPlatform
implementation and tests%P
argument for merge-drivers is needed for it to get through to the driver programGIT_REFLOG_ACTION
,GIT_EXEC_PATH
andGIT_PREFIX
can be passedmerge
stylediff3
stylezdiff3
stylediff3-conflict-markers.sh
(some special cases)figure out- skipped in interest of timecommon
case, where single-line changes are merged with the common middle, probably unrelated to the actual diff as zdiff3 and diff3 work.Note that 'union' is also affected by this, which seems detremental- skipped in interest of timemerge-file
(less simplistic input)<<<
lines that the text merge is adding? Probably configurable?gix-diff
.Questions
Next PR / Outscoped
textconv
with context, see this gist for details.GIT_DIR
set, others do.gix-command::Context
.diff3-conflict-markers
libgit2
also doesn't try it.diff3-conflict-markers.sh
- be sure to capture the 'empty tree' label , but also other special casesgix
at least.Research
Everything is about MergeORT.
git2::MergeOptions
.Crates
Mostly for performance optimization, also interesting for handling the index, and the tree::editor types.
Building Blocks for String Interning
yoke
.bumpalo
, but returns an index to the item, can also remove things. Strings aren't really supported though (i.e. variable length slices)String Interining
&str
https://lib.rs/crates/interned- also for&[u8]
- but leaks the data to get pointer comparisonArenaInterner
also for[u8]
Handle Special Cases
Questions
Is
git2::merge_trees()
a trivial merge? Does it handle all the cases of MergeORT?Maybe not, but it definitely handles rename tracking. See these
git2
flags for more information.How does rename-tracking affect a tree-merge?
The first round is done without renames, then there is a second round to find renames, and perform the merge of renamed items.
How is an octopus merge implemented, particularly with Merge ORT?
References
Removed Utility
Prints a patch with context 3, using bytes only, based on imara-diff.