diff options
| -rw-r--r-- | builtin/log.c | 2 | ||||
| -rw-r--r-- | patch-ids.c | 32 | ||||
| -rw-r--r-- | patch-ids.h | 2 | ||||
| -rw-r--r-- | t/perf/p3400-rebase.sh | 36 | ||||
| -rwxr-xr-x | t/t6007-rev-list-cherry-pick-file.sh | 21 |
5 files changed, 85 insertions, 8 deletions
diff --git a/builtin/log.c b/builtin/log.c index 099f4f7..e364572 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1315,7 +1315,7 @@ static void prepare_bases(struct base_tree_info *bases, struct object_id *patch_id; if (commit->util) continue; - if (commit_patch_id(commit, &diffopt, sha1)) + if (commit_patch_id(commit, &diffopt, sha1, 0)) die(_("cannot get patch id")); ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id); patch_id = bases->patch_id + bases->nr_patch_id; diff --git a/patch-ids.c b/patch-ids.c index 69a14a3..082412a 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -5,7 +5,7 @@ #include "patch-ids.h" int commit_patch_id(struct commit *commit, struct diff_options *options, - unsigned char *sha1) + unsigned char *sha1, int diff_header_only) { if (commit->parents) diff_tree_sha1(commit->parents->item->object.oid.hash, @@ -13,13 +13,31 @@ int commit_patch_id(struct commit *commit, struct diff_options *options, else diff_root_tree_sha1(commit->object.oid.hash, "", options); diffcore_std(options); - return diff_flush_patch_id(options, sha1, 0); + return diff_flush_patch_id(options, sha1, diff_header_only); } +/* + * When we cannot load the full patch-id for both commits for whatever + * reason, the function returns -1 (i.e. return error(...)). Despite + * the "cmp" in the name of this function, the caller only cares about + * the return value being zero (a and b are equivalent) or non-zero (a + * and b are different), and returning non-zero would keep both in the + * result, even if they actually were equivalent, in order to err on + * the side of safety. The actual value being negative does not have + * any significance; only that it is non-zero matters. + */ static int patch_id_cmp(struct patch_id *a, struct patch_id *b, - void *keydata) + struct diff_options *opt) { + if (is_null_sha1(a->patch_id) && + commit_patch_id(a->commit, opt, a->patch_id, 0)) + return error("Could not get patch ID for %s", + oid_to_hex(&a->commit->object.oid)); + if (is_null_sha1(b->patch_id) && + commit_patch_id(b->commit, opt, b->patch_id, 0)) + return error("Could not get patch ID for %s", + oid_to_hex(&b->commit->object.oid)); return hashcmp(a->patch_id, b->patch_id); } @@ -43,11 +61,13 @@ static int init_patch_id_entry(struct patch_id *patch, struct commit *commit, struct patch_ids *ids) { + unsigned char header_only_patch_id[GIT_SHA1_RAWSZ]; + patch->commit = commit; - if (commit_patch_id(commit, &ids->diffopts, patch->patch_id)) + if (commit_patch_id(commit, &ids->diffopts, header_only_patch_id, 1)) return -1; - hashmap_entry_init(patch, sha1hash(patch->patch_id)); + hashmap_entry_init(patch, sha1hash(header_only_patch_id)); return 0; } @@ -60,7 +80,7 @@ struct patch_id *has_commit_patch_id(struct commit *commit, if (init_patch_id_entry(&patch, commit, ids)) return NULL; - return hashmap_get(&ids->patches, &patch, NULL); + return hashmap_get(&ids->patches, &patch, &ids->diffopts); } struct patch_id *add_commit_patch_id(struct commit *commit, diff --git a/patch-ids.h b/patch-ids.h index dea1ecd..0f34ea1 100644 --- a/patch-ids.h +++ b/patch-ids.h @@ -13,7 +13,7 @@ struct patch_ids { }; int commit_patch_id(struct commit *commit, struct diff_options *options, - unsigned char *sha1); + unsigned char *sha1, int); int init_patch_ids(struct patch_ids *); int free_patch_ids(struct patch_ids *); struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *); diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh new file mode 100644 index 0000000..b3e7d52 --- /dev/null +++ b/t/perf/p3400-rebase.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +test_description='Tests rebase performance' +. ./perf-lib.sh + +test_perf_default_repo + +test_expect_success 'setup' ' + git checkout -f -b base && + git checkout -b to-rebase && + git checkout -b upstream && + for i in $(seq 100) + do + # simulate huge diffs + echo change$i >unrelated-file$i && + seq 1000 >>unrelated-file$i && + git add unrelated-file$i && + test_tick && + git commit -m commit$i unrelated-file$i && + echo change$i >unrelated-file$i && + seq 1000 | tac >>unrelated-file$i && + git add unrelated-file$i && + test_tick && + git commit -m commit$i-reverse unrelated-file$i || + break + done && + git checkout to-rebase && + test_commit our-patch interesting-file +' + +test_perf 'rebase on top of a lot of unrelated changes' ' + git rebase --onto upstream HEAD^ && + git rebase --onto base HEAD^ +' + +test_done diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh index 28d4f6b..1408b60 100755 --- a/t/t6007-rev-list-cherry-pick-file.sh +++ b/t/t6007-rev-list-cherry-pick-file.sh @@ -207,4 +207,25 @@ test_expect_success '--count --left-right' ' test_cmp expect actual ' +# Corrupt the object store deliberately to make sure +# the object is not even checked for its existence. +remove_loose_object () { + sha1="$(git rev-parse "$1")" && + remainder=${sha1#??} && + firsttwo=${sha1%$remainder} && + rm .git/objects/$firsttwo/$remainder +} + +test_expect_success '--cherry-pick avoids looking at full diffs' ' + git checkout -b shy-diff && + test_commit dont-look-at-me && + echo Hello >dont-look-at-me.t && + test_tick && + git commit -m tip dont-look-at-me.t && + git checkout -b mainline HEAD^ && + test_commit to-cherry-pick && + remove_loose_object shy-diff^:dont-look-at-me.t && + git rev-list --cherry-pick ...shy-diff +' + test_done |