diff --git a/Documentation/technical/commit-graph.txt b/Documentation/technical/commit-graph.txt index c664acbd76..001395e950 100644 --- a/Documentation/technical/commit-graph.txt +++ b/Documentation/technical/commit-graph.txt @@ -112,12 +112,24 @@ Design Details - The file format includes parameters for the object ID hash function, so a future change of hash algorithm does not require a change in format. +- Commit grafts and replace objects can change the shape of the commit + history. The latter can also be enabled/disabled on the fly using + `--no-replace-objects`. This leads to difficultly storing both possible + interpretations of a commit id, especially when computing generation + numbers. The commit-graph will not be read or written when + replace-objects or grafts are present. + +- Shallow clones create grafts of commits by dropping their parents. This + leads the commit-graph to think those commits have generation number 1. + If and when those commits are made unshallow, those generation numbers + become invalid. Since shallow clones are intended to restrict the commit + history to a very small set of commits, the commit-graph feature is less + helpful for these clones, anyway. The commit-graph will not be read or + written when shallow commits are present. + Future Work ----------- -- The commit graph feature currently does not honor commit grafts. This can - be remedied by duplicating or refactoring the current graft logic. - - After computing and storing generation numbers, we must make graph walks aware of generation numbers to gain the performance benefits they enable. This will mostly be accomplished by swapping a commit-date-ordered diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index bc0fa9ba52..22b974f4b4 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -120,6 +120,8 @@ static int graph_read(int argc, const char **argv) return 0; } +extern int read_replace_refs; + static int graph_write(int argc, const char **argv) { struct string_list *pack_indexes = NULL; @@ -150,6 +152,8 @@ static int graph_write(int argc, const char **argv) if (!opts.obj_dir) opts.obj_dir = get_object_directory(); + read_replace_refs = 0; + if (opts.reachable) { write_commit_graph_reachable(opts.obj_dir, opts.append, 1); return 0; diff --git a/builtin/replace.c b/builtin/replace.c index 8e67e09819..30a661ea0c 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -39,7 +39,8 @@ struct show_data { enum replace_format format; }; -static int show_reference(const char *refname, const struct object_id *oid, +static int show_reference(struct repository *r, const char *refname, + const struct object_id *oid, int flag, void *cb_data) { struct show_data *data = cb_data; @@ -56,9 +57,8 @@ static int show_reference(const char *refname, const struct object_id *oid, if (get_oid(refname, &object)) return error(_("failed to resolve '%s' as a valid ref"), refname); - obj_type = oid_object_info(the_repository, &object, - NULL); - repl_type = oid_object_info(the_repository, oid, NULL); + obj_type = oid_object_info(r, &object, NULL); + repl_type = oid_object_info(r, oid, NULL); printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type), oid_to_hex(oid), type_name(repl_type)); diff --git a/commit-graph.c b/commit-graph.c index 5908bd4e34..a686758603 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -13,6 +13,8 @@ #include "commit-graph.h" #include "object-store.h" #include "alloc.h" +#include "hashmap.h" +#include "replace-object.h" #include "progress.h" #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */ @@ -57,6 +59,28 @@ static struct commit_graph *alloc_commit_graph(void) return g; } +extern int read_replace_refs; + +static int commit_graph_compatible(struct repository *r) +{ + if (!r->gitdir) + return 0; + + if (read_replace_refs) { + prepare_replace_object(r); + if (hashmap_get_size(&r->objects->replace_map->map)) + return 0; + } + + prepare_commit_graft(r); + if (r->parsed_objects && r->parsed_objects->grafts_nr) + return 0; + if (is_repository_shallow(r)) + return 0; + + return 1; +} + struct commit_graph *load_commit_graph_one(const char *graph_file) { void *graph_map; @@ -225,6 +249,9 @@ static int prepare_commit_graph(struct repository *r) */ return 0; + if (!commit_graph_compatible(r)) + return 0; + obj_dir = r->objects->objectdir; prepare_commit_graph_one(r, obj_dir); prepare_alt_odb(r); @@ -253,10 +280,10 @@ int generation_numbers_enabled(struct repository *r) return !!first_generation; } -static void close_commit_graph(void) +void close_commit_graph(struct repository *r) { - free_commit_graph(the_repository->objects->commit_graph); - the_repository->objects->commit_graph = NULL; + free_commit_graph(r->objects->commit_graph); + r->objects->commit_graph = NULL; } static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos) @@ -737,6 +764,9 @@ void write_commit_graph(const char *obj_dir, struct commit_list *parent; struct progress *progress = NULL; + if (!commit_graph_compatible(the_repository)) + return; + oids.nr = 0; oids.alloc = approximate_object_count() / 4; oids.progress = NULL; @@ -908,7 +938,7 @@ void write_commit_graph(const char *obj_dir, write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr); write_graph_chunk_large_edges(f, commits.list, commits.nr); - close_commit_graph(); + close_commit_graph(the_repository); finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC); commit_lock_file(&lk); diff --git a/commit-graph.h b/commit-graph.h index 5678a8f4ca..9db40b4d3a 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -69,6 +69,7 @@ void write_commit_graph(const char *obj_dir, int verify_commit_graph(struct repository *r, struct commit_graph *g); +void close_commit_graph(struct repository *); void free_commit_graph(struct commit_graph *); #endif diff --git a/commit.c b/commit.c index d0f199e122..dc8a39d52a 100644 --- a/commit.c +++ b/commit.c @@ -209,7 +209,7 @@ static int read_graft_file(struct repository *r, const char *graft_file) return 0; } -static void prepare_commit_graft(struct repository *r) +void prepare_commit_graft(struct repository *r) { char *graft_file; diff --git a/commit.h b/commit.h index 2b1a734388..1d260d62f5 100644 --- a/commit.h +++ b/commit.h @@ -202,6 +202,7 @@ typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); struct commit_graft *read_graft_line(struct strbuf *line); int register_commit_graft(struct repository *r, struct commit_graft *, int); +void prepare_commit_graft(struct repository *r); struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid); /* largest positive number a signed 32-bit integer can contain */ diff --git a/refs.c b/refs.c index a7a75b4cc0..bbcac921b6 100644 --- a/refs.c +++ b/refs.c @@ -1394,17 +1394,50 @@ struct ref_iterator *refs_ref_iterator_begin( * non-zero value, stop the iteration and return that value; * otherwise, return 0. */ -static int do_for_each_ref(struct ref_store *refs, const char *prefix, - each_ref_fn fn, int trim, int flags, void *cb_data) +static int do_for_each_repo_ref(struct repository *r, const char *prefix, + each_repo_ref_fn fn, int trim, int flags, + void *cb_data) { struct ref_iterator *iter; + struct ref_store *refs = get_main_ref_store(r); if (!refs) return 0; iter = refs_ref_iterator_begin(refs, prefix, trim, flags); - return do_for_each_ref_iterator(iter, fn, cb_data); + return do_for_each_repo_ref_iterator(r, iter, fn, cb_data); +} + +struct do_for_each_ref_help { + each_ref_fn *fn; + void *cb_data; +}; + +static int do_for_each_ref_helper(struct repository *r, + const char *refname, + const struct object_id *oid, + int flags, + void *cb_data) +{ + struct do_for_each_ref_help *hp = cb_data; + + return hp->fn(refname, oid, flags, hp->cb_data); +} + +static int do_for_each_ref(struct ref_store *refs, const char *prefix, + each_ref_fn fn, int trim, int flags, void *cb_data) +{ + struct ref_iterator *iter; + struct do_for_each_ref_help hp = { fn, cb_data }; + + if (!refs) + return 0; + + iter = refs_ref_iterator_begin(refs, prefix, trim, flags); + + return do_for_each_repo_ref_iterator(the_repository, iter, + do_for_each_ref_helper, &hp); } int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) @@ -1449,12 +1482,11 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data); } -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data) +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data) { - return do_for_each_ref(get_main_ref_store(r), - git_replace_ref_base, fn, - strlen(git_replace_ref_base), - DO_FOR_EACH_INCLUDE_BROKEN, cb_data); + return do_for_each_repo_ref(r, git_replace_ref_base, fn, + strlen(git_replace_ref_base), + DO_FOR_EACH_INCLUDE_BROKEN, cb_data); } int for_each_namespaced_ref(each_ref_fn fn, void *cb_data) @@ -2033,10 +2065,12 @@ cleanup: int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data) { struct ref_iterator *iter; + struct do_for_each_ref_help hp = { fn, cb_data }; iter = refs->be->reflog_iterator_begin(refs); - return do_for_each_ref_iterator(iter, fn, cb_data); + return do_for_each_repo_ref_iterator(the_repository, iter, + do_for_each_ref_helper, &hp); } int for_each_reflog(each_ref_fn fn, void *cb_data) diff --git a/refs.h b/refs.h index bd52c1bbae..6cc0397679 100644 --- a/refs.h +++ b/refs.h @@ -276,6 +276,16 @@ struct ref_transaction; typedef int each_ref_fn(const char *refname, const struct object_id *oid, int flags, void *cb_data); +/* + * The same as each_ref_fn, but also with a repository argument that + * contains the repository associated with the callback. + */ +typedef int each_repo_ref_fn(struct repository *r, + const char *refname, + const struct object_id *oid, + int flags, + void *cb_data); + /* * The following functions invoke the specified callback function for * each reference indicated. If the function ever returns a nonzero @@ -309,7 +319,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, int for_each_tag_ref(each_ref_fn fn, void *cb_data); int for_each_branch_ref(each_ref_fn fn, void *cb_data); int for_each_remote_ref(each_ref_fn fn, void *cb_data); -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data); +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data); int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data); int for_each_glob_ref_in(each_ref_fn fn, const char *pattern, const char *prefix, void *cb_data); diff --git a/refs/iterator.c b/refs/iterator.c index 2ac91ac340..629e00a122 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -407,15 +407,15 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, struct ref_iterator *current_ref_iter = NULL; -int do_for_each_ref_iterator(struct ref_iterator *iter, - each_ref_fn fn, void *cb_data) +int do_for_each_repo_ref_iterator(struct repository *r, struct ref_iterator *iter, + each_repo_ref_fn fn, void *cb_data) { int retval = 0, ok; struct ref_iterator *old_ref_iter = current_ref_iter; current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - retval = fn(iter->refname, iter->oid, iter->flags, cb_data); + retval = fn(r, iter->refname, iter->oid, iter->flags, cb_data); if (retval) { /* * If ref_iterator_abort() returns ITER_ERROR, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 04425d6d1e..89d7dd49cd 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -474,8 +474,9 @@ extern struct ref_iterator *current_ref_iter; * adapter between the callback style of reference iteration and the * iterator style. */ -int do_for_each_ref_iterator(struct ref_iterator *iter, - each_ref_fn fn, void *cb_data); +int do_for_each_repo_ref_iterator(struct repository *r, + struct ref_iterator *iter, + each_repo_ref_fn fn, void *cb_data); /* * Only include per-worktree refs in a do_for_each_ref*() iteration. diff --git a/replace-object.c b/replace-object.c index 4ec77ce418..e295e87943 100644 --- a/replace-object.c +++ b/replace-object.c @@ -6,7 +6,8 @@ #include "repository.h" #include "commit.h" -static int register_replace_ref(const char *refname, +static int register_replace_ref(struct repository *r, + const char *refname, const struct object_id *oid, int flag, void *cb_data) { @@ -25,13 +26,13 @@ static int register_replace_ref(const char *refname, oidcpy(&repl_obj->replacement, oid); /* Register new object */ - if (oidmap_put(the_repository->objects->replace_map, repl_obj)) + if (oidmap_put(r->objects->replace_map, repl_obj)) die(_("duplicate replace ref: %s"), refname); return 0; } -static void prepare_replace_object(struct repository *r) +void prepare_replace_object(struct repository *r) { if (r->objects->replace_map) return; diff --git a/replace-object.h b/replace-object.h index 9345e105dd..16528df942 100644 --- a/replace-object.h +++ b/replace-object.h @@ -10,6 +10,8 @@ struct replace_object { struct object_id replacement; }; +void prepare_replace_object(struct repository *r); + /* * This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c index 2762ca6562..6a84a53efb 100644 --- a/t/helper/test-repository.c +++ b/t/helper/test-repository.c @@ -15,7 +15,10 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree, struct commit *c; struct commit_list *parent; - repo_init(&r, gitdir, worktree); + setup_git_env(gitdir); + + if (repo_init(&r, gitdir, worktree)) + die("Couldn't init repo"); c = lookup_commit(&r, commit_oid); @@ -38,7 +41,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir, struct commit *c; struct tree *tree; - repo_init(&r, gitdir, worktree); + setup_git_env(gitdir); + + if (repo_init(&r, gitdir, worktree)) + die("Couldn't init repo"); c = lookup_commit(&r, commit_oid); diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 75fe09521f..db8dcafca3 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -260,6 +260,66 @@ test_expect_success 'check that gc computes commit-graph' ' test_cmp_bin commit-graph-after-gc $objdir/info/commit-graph ' +test_expect_success 'replace-objects invalidates commit-graph' ' + cd "$TRASH_DIRECTORY" && + test_when_finished rm -rf replace && + git clone full replace && + ( + cd replace && + git commit-graph write --reachable && + test_path_is_file .git/objects/info/commit-graph && + git replace HEAD~1 HEAD~2 && + git -c core.commitGraph=false log >expect && + git -c core.commitGraph=true log >actual && + test_cmp expect actual && + git commit-graph write --reachable && + git -c core.commitGraph=false --no-replace-objects log >expect && + git -c core.commitGraph=true --no-replace-objects log >actual && + test_cmp expect actual && + rm -rf .git/objects/info/commit-graph && + git commit-graph write --reachable && + test_path_is_file .git/objects/info/commit-graph + ) +' + +test_expect_success 'commit grafts invalidate commit-graph' ' + cd "$TRASH_DIRECTORY" && + test_when_finished rm -rf graft && + git clone full graft && + ( + cd graft && + git commit-graph write --reachable && + test_path_is_file .git/objects/info/commit-graph && + H1=$(git rev-parse --verify HEAD~1) && + H3=$(git rev-parse --verify HEAD~3) && + echo "$H1 $H3" >.git/info/grafts && + git -c core.commitGraph=false log >expect && + git -c core.commitGraph=true log >actual && + test_cmp expect actual && + git commit-graph write --reachable && + git -c core.commitGraph=false --no-replace-objects log >expect && + git -c core.commitGraph=true --no-replace-objects log >actual && + test_cmp expect actual && + rm -rf .git/objects/info/commit-graph && + git commit-graph write --reachable && + test_path_is_missing .git/objects/info/commit-graph + ) +' + +test_expect_success 'replace-objects invalidates commit-graph' ' + cd "$TRASH_DIRECTORY" && + test_when_finished rm -rf shallow && + git clone --depth 2 "file://$TRASH_DIRECTORY/full" shallow && + ( + cd shallow && + git commit-graph write --reachable && + test_path_is_missing .git/objects/info/commit-graph && + git fetch origin --unshallow && + git commit-graph write --reachable && + test_path_is_file .git/objects/info/commit-graph + ) +' + # the verify tests below expect the commit-graph to contain # exactly the commits reachable from the commits/8 branch. # If the file changes the set of commits in the list, then the diff --git a/upload-pack.c b/upload-pack.c index 62a1000f44..540778d1dd 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -24,6 +24,7 @@ #include "quote.h" #include "upload-pack.h" #include "serve.h" +#include "commit-graph.h" #include "commit-reach.h" /* Remember to update object flag allocation in object.h */ @@ -692,6 +693,7 @@ static void deepen_by_rev_list(int ac, const char **av, { struct commit_list *result; + close_commit_graph(the_repository); result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW); send_shallow(result); free_commit_list(result);