mirror of
https://github.com/git/git
synced 2024-08-25 10:45:59 +00:00
![Jeff King](/assets/img/avatar_default.png)
The patch-id code which powers "log --cherry-pick" doesn't look at whether each commit is a merge or not. It just feeds the commit's first parent to the diff, and ignores any additional parents. In theory, this might be useful if you wanted to find equivalence between, say, a merge commit and a squash-merge that does the same thing. But it also promotes a false equivalence between distinct merges. For example, every "merge -s ours" would look identical to an empty commit (which is true in a sense, but presumably there was a value in merging in the discarded history). Since patch-ids are meant for throwing away duplicates, we should err on the side of _not_ matching such merges. Moreover, we may spend a lot of extra time computing these merge diffs. In the case that inspired this patch, a "git format-patch --cherry-pick" dropped from over 3 minutes to less than 3 seconds. This seems pretty drastic, but is easily explained. The command was invoked by a "git rebase" of an older topic branch; there had been tens of thousands of commits on the upstream branch in the meantime. In addition, this project used a topic-branch workflow with occasional "back-merges" from "master" to each topic (to resolve conflicts on the topics rather than in the merge commits). So there were not only extra merges, but the diffs for these back-merges were generally quite large (because they represented _everything_ that had been merged to master since the topic branched). This patch treats a merge fed to commit_patch_id() or add_commit_patch_id() as an error, and a lookup for such a merge via has_commit_patch_id() will always return NULL. An earlier version of the patch tried to distinguish between "error" and "patch id for merges not defined", but that becomes unnecessarily complicated. The only callers are: 1. revision traversals which want to do --cherry-pick; they call add_commit_patch_id(), but do not care if it fails. They only want to add what we can, look it up later with has_commit_patch_id(), and err on the side of not-matching. 2. format-patch --base, which calls commit_patch_id(). This _does_ notice errors, but should never feed a merge in the first place (and if it were to do so accidentally, then this patch is a strict improvement; we notice the bug rather than generating a bogus patch-id). So in both cases, this does the right thing. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
115 lines
3.1 KiB
C
115 lines
3.1 KiB
C
#include "cache.h"
|
|
#include "diff.h"
|
|
#include "commit.h"
|
|
#include "sha1-lookup.h"
|
|
#include "patch-ids.h"
|
|
|
|
static int patch_id_defined(struct commit *commit)
|
|
{
|
|
/* must be 0 or 1 parents */
|
|
return !commit->parents || !commit->parents->next;
|
|
}
|
|
|
|
int commit_patch_id(struct commit *commit, struct diff_options *options,
|
|
unsigned char *sha1, int diff_header_only)
|
|
{
|
|
if (!patch_id_defined(commit))
|
|
return -1;
|
|
|
|
if (commit->parents)
|
|
diff_tree_sha1(commit->parents->item->object.oid.hash,
|
|
commit->object.oid.hash, "", options);
|
|
else
|
|
diff_root_tree_sha1(commit->object.oid.hash, "", options);
|
|
diffcore_std(options);
|
|
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,
|
|
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);
|
|
}
|
|
|
|
int init_patch_ids(struct patch_ids *ids)
|
|
{
|
|
memset(ids, 0, sizeof(*ids));
|
|
diff_setup(&ids->diffopts);
|
|
ids->diffopts.detect_rename = 0;
|
|
DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
|
|
diff_setup_done(&ids->diffopts);
|
|
hashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp, 256);
|
|
return 0;
|
|
}
|
|
|
|
int free_patch_ids(struct patch_ids *ids)
|
|
{
|
|
hashmap_free(&ids->patches, 1);
|
|
return 0;
|
|
}
|
|
|
|
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, header_only_patch_id, 1))
|
|
return -1;
|
|
|
|
hashmap_entry_init(patch, sha1hash(header_only_patch_id));
|
|
return 0;
|
|
}
|
|
|
|
struct patch_id *has_commit_patch_id(struct commit *commit,
|
|
struct patch_ids *ids)
|
|
{
|
|
struct patch_id patch;
|
|
|
|
if (!patch_id_defined(commit))
|
|
return NULL;
|
|
|
|
memset(&patch, 0, sizeof(patch));
|
|
if (init_patch_id_entry(&patch, commit, ids))
|
|
return NULL;
|
|
|
|
return hashmap_get(&ids->patches, &patch, &ids->diffopts);
|
|
}
|
|
|
|
struct patch_id *add_commit_patch_id(struct commit *commit,
|
|
struct patch_ids *ids)
|
|
{
|
|
struct patch_id *key = xcalloc(1, sizeof(*key));
|
|
|
|
if (!patch_id_defined(commit))
|
|
return NULL;
|
|
|
|
if (init_patch_id_entry(key, commit, ids)) {
|
|
free(key);
|
|
return NULL;
|
|
}
|
|
|
|
hashmap_add(&ids->patches, key);
|
|
return key;
|
|
}
|