Merge branch 'ds/commit-graph-with-grafts' into maint
authorJunio C Hamano <gitster@pobox.com>
Wed, 21 Nov 2018 13:57:47 +0000 (22:57 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 21 Nov 2018 13:57:47 +0000 (22:57 +0900)
The recently introduced commit-graph auxiliary data is incompatible
with mechanisms such as replace & grafts that "breaks" immutable
nature of the object reference relationship. Disable optimizations
based on its use (and updating existing commit-graph) when these
incompatible features are in use in the repository.

* ds/commit-graph-with-grafts:
commit-graph: close_commit_graph before shallow walk
commit-graph: not compatible with uninitialized repo
commit-graph: not compatible with grafts
commit-graph: not compatible with replace objects
test-repository: properly init repo
commit-graph: update design document
refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
refs.c: migrate internal ref iteration to pass thru repository argument

16 files changed:
Documentation/technical/commit-graph.txt
builtin/commit-graph.c
builtin/replace.c
commit-graph.c
commit-graph.h
commit.c
commit.h
refs.c
refs.h
refs/iterator.c
refs/refs-internal.h
replace-object.c
replace-object.h
t/helper/test-repository.c
t/t5318-commit-graph.sh
upload-pack.c
index c664acbd765d06f000b2feeef1d7e98f1616ff62..001395e95071ec50d2fef65ead09cf5c49558a6a 100644 (file)
@@ -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
index 0bf0c486575a8557b729658ce05be02d28a1a8b8..da737df321528088d8aa20e3e87e803336b1f550 100644 (file)
@@ -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);
                return 0;
index 4f05791f3e895f78ba511dd6571bd09abab9269c..17868a92dcfccce335d69854f54f0ef5d3a9ca1b 100644 (file)
@@ -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));
index 8a1bec7b8aa420dd3d4ecadc95dee31029533c07..7cfa779dcbdcf9a3e2137cc54c1c9ef62c5f6895 100644 (file)
@@ -13,6 +13,8 @@
 #include "commit-graph.h"
 #include "object-store.h"
 #include "alloc.h"
+#include "hashmap.h"
+#include "replace-object.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -56,6 +58,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;
@@ -223,6 +247,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);
@@ -233,10 +260,10 @@ static int prepare_commit_graph(struct repository *r)
        return !!r->objects->commit_graph;
 }
 
-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)
@@ -693,6 +720,9 @@ void write_commit_graph(const char *obj_dir,
        int num_extra_edges;
        struct commit_list *parent;
 
+       if (!commit_graph_compatible(the_repository))
+               return;
+
        oids.nr = 0;
        oids.alloc = approximate_object_count() / 4;
 
@@ -845,7 +875,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);
 
index eea62f8c0ee53b56630a1a2b0c2c716b4cd63670..b62d1b08373633afd545f7d8734cbd0d61963bb6 100644 (file)
@@ -60,6 +60,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
index db679c2adfd766a385f5406e6413ad13ba1e27af..fb4c99cb8908b53c2bbba43ef08de0e2e217533e 100644 (file)
--- 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;
 
index 43f93b973dffe3bf8e56aa175e66304c72b0e30a..12b8b2d6543c758a6e96c0ba2ee287ef0d048e77 100644 (file)
--- 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);
 
 extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);
diff --git a/refs.c b/refs.c
index de81c7be7ca8d3ca033b34a61f33b0bff069932f..9a318c8456c3fe8d157cf6c7c6aca5b635d75230 100644 (file)
--- 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_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_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_ref_iterator(iter, fn, cb_data);
+       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 @@ int refs_verify_refname_available(struct ref_store *refs,
 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 bd52c1bbae3a68fe8ca8f9e6cae7cc54bdbf9852..6cc0397679fd55bfdfc72b7a7f4cbf3ee5d028a0 100644 (file)
--- 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);
index 2ac91ac3401c87108b9cdb4983acb7c165df82d9..629e00a122a7a867ae3350ce35469c2ceb1259a1 100644 (file)
@@ -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,
index 04425d6d1e45e2401454e7b89b0ed4b3959926de..89d7dd49cdce945d1bfe7277082d3d444b9d7e85 100644 (file)
@@ -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.
index 4ec77ce41848311a912256046bd2bf8dc9ee63c0..e295e87943102c2a1ad903aea1a634d34bf603a0 100644 (file)
@@ -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;
index 9345e105ddcaa38cf9e856df70d960245a3b696c..16528df942f79e3d7ce65d99e0103395d0c69f51 100644 (file)
@@ -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.
index 2762ca656262baa9494d96a2647248f4042a2e01..6a84a53efbf6919c83d3f1fd73786acd92ee7abf 100644 (file)
@@ -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);
 
index 0c500f7ca2641a2752f5d4819bb11efcf5f588bf..2799969f3e2a15fc636ed042c96d8a846e01ebe8 100755 (executable)
@@ -259,6 +259,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
index 82b393ec31917c0c2bcd904668a60b5aaef00633..2ae9d9bb475214070529e22cc3f186afebbc6c7c 100644 (file)
@@ -24,6 +24,7 @@
 #include "quote.h"
 #include "upload-pack.h"
 #include "serve.h"
+#include "commit-graph.h"
 
 /* Remember to update object flag allocation in object.h */
 #define THEY_HAVE      (1u << 11)
@@ -740,6 +741,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);