Skip to content
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

tree-merge follow-up #1651

Merged
merged 10 commits into from
Nov 5, 2024
Merged

tree-merge follow-up #1651

merged 10 commits into from
Nov 5, 2024

Conversation

Byron
Copy link
Member

@Byron Byron commented Nov 2, 2024

This finishes up the tree-merge, with various tasks going into it as needed.

Follow-up of #1618.

Tasks

  • Repository::index_or_load_from_head_or_empty() - that way it also works in unborn repositories (typically seen in testing). Use it in in diff_resource_cache() and others that might not want to fail in an unborn repository.
  • Improve gix merge trees API to take as input and provide output that is fully wrapped. Particularly the tree-editor should be write for simple writes that are anyhow compatible.
  • an easy way to get the file paths of items that are conflicting (for producing error messages more easily maybe) - not really needed, and there is quickly a lot of paths involved anyway.

Next PR

  • A way to know if a textual merge would have resulted in a conflict without auto-resolution.
  • A way to resolve conflicts towards using our or their side. Either at conflict time, or post-conflict time, but fully tested as part of baseline expectations. Probably better to make it as part of the merge operation.
    • Note that we still want to record if a conflict occurred, and in case of text-merges we want to know if there would have been a conflict if we wouldn't have auto-resolved it. Lastly, for binary merges, we may assume a conflict, of course.
  • A way write the conflict declaration into an index or adjust an optional index as well (which might be easier) so we get all the stages right.

Possible Tasks

  • Repository::merge_commits()
  • A trivial gix merge commit
  • Submodule merges are also possible! Maybe outscope it though! libgit2 also doesn't try it.
  • textconv with context, see this gist for details.
    • There seem to be different 'tiers' of tools, some don't get GIT_DIR set, others do.
    • It also seems that diff-programs get too much context right now, but that depends on how much is passed to them by the caller as gix-command::Context.
  • How to model virtual-merge-bases? Can be none or many, user should have control over how this is done.
  • Actual tree-based merging
  • Make blob-merge based conflict handling possible in the tree merge from gix at least. - not needed for now
  • consider finding a way to more clearly say what happened, maybe with some additional explanation so it's more of a drill-down way of working, instead of a super-enum with a lot of very similar cases.

Everything is about MergeORT.

ability to re-use object caches of a repo that has seen the base-tree already, but overall, who knows*

  • merge-options passed with -X ours for instance don't affect tree-related auto-resolutions, just the ones related to content. This could be implemented when there is demand though.
  • it uses an empty tree if there is no merge-base - we must allow the same.
  • it allows for multiple merge-bases, creating a virtual one by merging all merge-bases together using the same algorithm, recursively.
  • merges can have conflicts without a individual files being involved, for instance when directory renames clash
  • Note that merge-ORT cannot properly handle renames within renamed directories, ending up with the source of the subdir-rename still present.
❯ git ls-tree -r $(git merge-tree main feat)
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    a
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    git-sec-renamed/2
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    git-sec-renamed/7
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    git-sec-renamed/subdir/6
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    git-sec/subdir-renamed/6
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    git-sequencer
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    gix/5
100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    h
  • Must make sure that possible types of conflicts are properly communicated, to not degenerate information
  • It puts conflict-markers in the blobs of the result tree, with annotations to provide additional context
  • Need resolution configuration, see git2::MergeOptions.
  • data stored by path, and is interned in the map to allow pointer-based comparisons
    • merge-info with everything one needs to know, also related to renames
    • or conflict information
    • it uses a memory-pool/arena to get memory for many paths all at once (and also release it like that)
  • paths start out as conflicted, and then can later be changed to non-conflicting if a content-based merged succeeded.
    • If it remains conflicts, the meta-data is used to produce an 'as merged as possible' version with conflict markers that can be checked out to the working tree.
  • hunks can partially overlap, but can also be resolved line-by line to some extend.

Byron added 4 commits November 5, 2024 15:51
After all, stopping the merge when there is any conflict is highly relevant.
It's useful to get a reasonable index in any case, even on unborn repositories.
It's for cases where the `HEAD` isn't setup at all, despite content being available,
and to avoid unnecessary restrictions on what works.
…tor back.

This can be useful to have more control over what gets written, or how.
Byron added 3 commits November 5, 2024 16:59
That way, more attached types are used for greater convenience.
By now, `blob-merge` is the lowest-level of features which is required
for both tree-merges and commit based merges. Hence it's better
to just call it `merge`.
@Byron Byron marked this pull request as ready for review November 5, 2024 18:21
@Byron Byron enabled auto-merge November 5, 2024 18:27
@Byron Byron merged commit a876533 into main Nov 5, 2024
16 checks passed
@Byron Byron deleted the merge branch November 5, 2024 18:48
@Byron Byron mentioned this pull request Nov 7, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant