Skip to content

Sparse index: integrate with git stash #428

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.

Sign up for GitHub

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
vdye merged 6 commits into from
Sep 21, 2021
Merged

Sparse index: integrate with git stash #428

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f420f6
fixup! reset: expand test coverage in sparse checkout
vdye Sep 21, 2021
5946795
t/perf/run: fix bin-wrappers computation
derrickstolee Sep 21, 2021
4f5cb33
p2000: add performance test for `git stash`
vdye Sep 20, 2021
46fab37
stash: integrate with sparse index
vdye Sep 16, 2021
c531eab
merge-recursive: add merge function arg to `merge_recursive_generic`
vdye Sep 16, 2021
613c955
stash: use merge-ort rather than merge-recursive
vdye Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion builtin/am.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,7 +1585,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
if (state->quiet)
o.verbosity = 0;

if (merge_recursive_generic(&o, &our_tree, &their_tree, 1, bases, &result)) {
if (merge_recursive_generic(&o, &our_tree, &their_tree, 1, bases, merge_recursive, &result)) {
repo_rerere(the_repository, state->allow_rerere_autoupdate);
free(their_tree_name);
return error(_("Failed to merge in the changes."));
Expand Down
2 changes: 1 addition & 1 deletion builtin/merge-recursive.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
if (o.verbosity >= 3)
printf(_("Merging %s with %s\n"), o.branch1, o.branch2);

failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, &result);
failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, merge_recursive, &result);

free(better1);
free(better2);
Expand Down
2 changes: 1 addition & 1 deletion builtin/stash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
bases[0] = &info->b_tree;

ret = merge_recursive_generic(&o, &c_tree, &info->w_tree, 1, bases,
&result);
merge_recursive, &result);
if (ret) {
rerere(0);

Expand Down
3 changes: 2 additions & 1 deletion merge-ort.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4652,7 +4652,8 @@ void merge_incore_recursive(struct merge_options *opt,
trace2_region_enter("merge", "incore_recursive", opt->repo);

/* We set the ancestor label based on the merge_bases */
assert(opt->ancestor == NULL);
assert(opt->ancestor == NULL ||
!strcmp(opt->ancestor, "constructed merge base"));

trace2_region_enter("merge", "merge_start", opt->repo);
merge_start(opt, result);
Expand Down
4 changes: 2 additions & 2 deletions merge-recursive.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3785,6 +3785,7 @@ int merge_recursive_generic(struct merge_options *opt,
const struct object_id *merge,
int num_merge_bases,
const struct object_id **merge_bases,
recursive_merge_fn_t merge_fn,
struct commit **result)
{
int clean;
Expand All @@ -3808,8 +3809,7 @@ int merge_recursive_generic(struct merge_options *opt,
}

repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR);
clean = merge_recursive(opt, head_commit, next_commit, ca,
result);
clean = merge_fn(opt, head_commit, next_commit, ca, result);
Comment on lines 3811 to +3812

@derrickstolee derrickstolee Sep 21, 2021

Copy link
Copy Markdown

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.

if (clean < 0) {
rollback_lock_file(&lock);
return clean;
Expand Down
9 changes: 8 additions & 1 deletion merge-recursive.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ struct merge_options {
struct merge_options_internal *priv;
};

typedef int (*recursive_merge_fn_t)(struct merge_options *opt,
struct commit *h1,
struct commit *h2,
struct commit_list *merge_bases,
struct commit **result);

void init_merge_options(struct merge_options *opt, struct repository *repo);

/* parse the option in s and update the relevant field of opt */
Expand Down Expand Up @@ -103,7 +109,7 @@ int merge_recursive(struct merge_options *opt,

/*
* merge_recursive_generic can operate on trees instead of commits, by
* wrapping the trees into virtual commits, and calling merge_recursive().
* wrapping the trees into virtual commits, and calling the provided merge_fn.
* It also writes out the in-memory index to disk if the merge is successful.
*
* Outputs:
Expand All @@ -118,6 +124,7 @@ int merge_recursive_generic(struct merge_options *opt,
const struct object_id *merge,
int num_merge_bases,
const struct object_id **merge_bases,
recursive_merge_fn_t merge_fn,
struct commit **result);

#endif