Skip to content

Add MergeCommitsIntoIndex to ObjectDatabase #1534

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
merged 2 commits into from
Mar 5, 2018

Conversation

tyrielv
Copy link
Contributor

@tyrielv tyrielv commented Feb 12, 2018

Adds a new method MergeCommitsIntoIndex to ObjectDatabase to make the intermediate result of MergeCommits available (so that conflicts can be resolved directly against the produced index).

Also adds WriteToTree to Index so that the produced index can be converted to a tree just like MergeCommits currently does.

}
return null;
}
var result = new Index(indexHandle, repo);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not wild about returning a new Index object whose lifecycle is tied to the Repository. This is fine for services that are dealing with pull requests, since they'll spin up a repository, do a merge, then dispose the repository (and this merge result) but it's not going to be ideal for (say) GUI tools or anything that does multiple in-memory merges on the same Repository.

I think that we should create an Index subclass that's IDisposeable and doesn't register with the Repository for being disposed. TransientIndex, for lack of a better name...

I briefly (and by briefly, I mean I spent way too long on it) thought about making Index itself disposeable and replaceable, so that you could swap in a new index on the repository. I don't think that this is a path to success in the near term and that we should just have one type that represents the repository index and one that represents a transient index that's returned by merge / rebase / revert / etc / etc.

@ethomson
Copy link
Member

Thanks - it looks like the appveyor build is failing because of the GitHub TLS 1.2 upgrades 🙄 . (It's the gift that keeps on giving!)

I'll push an update and I'll merge this when it's fixed.

Thanks for this work!

@ethomson ethomson merged commit 4d40c50 into libgit2:master Mar 5, 2018
@ethomson
Copy link
Member

ethomson commented Mar 5, 2018

Thanks for working on this!

alex-weaver pushed a commit to alex-weaver/libgit2sharp that referenced this pull request Mar 23, 2023
Add MergeCommitsIntoIndex to ObjectDatabase
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.

2 participants