-
Notifications
You must be signed in to change notification settings - Fork 97
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
Sparse index: integrate with git stash
#428
Changes from all commits
5f420f6
5946795
4f5cb33
46fab37
c531eab
613c955
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,7 +74,7 @@ set_git_test_installed () { | |
mydir=$1 | ||
|
||
mydir_abs=$(cd $mydir && pwd) | ||
mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers" | ||
mydir_abs_wrappers="$mydir_abs/bin-wrappers" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an FYI: I sent this upstream via gitgitgadget#1044, since it is so simple. See https://lore.kernel.org/git/[email protected]/T/#u for the discussion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, should I remove it from this branch? Or will it disappear once There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will disappear during the rebase onto |
||
if test -d "$mydir_abs_wrappers" | ||
then | ||
GIT_TEST_INSTALLED=$mydir_abs_wrappers | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is odd that this doesn't already allow the ORT strategy if configured to do so.
I went hunting for how this was configured in
builtin/merge.c
, but that seems like a very complicated procedure.We could use something like the
pull.twohead
config here as a way to allow a user to modify the merge strategy. That gives us a safety valve in case the ORT strategy has a problem in one of these consumers.The other approach could be to just use the ORT algorithm here no matter what. It's a big change, but it might improve things more rapidly.
After having all of these thoughts I think you've found a good middle ground: the function pointer presents a way to use ORT only when we really need it. This works especially well for our early adoption of sparse index in
microsoft/git
, but we should call special attention to this when upstreaming the work.