]> git.ipfire.org Git - thirdparty/git.git/commitdiff
experimental: default to fetch.writeCommitGraph=false
authorJonathan Nieder <jrnieder@gmail.com>
Tue, 7 Jul 2020 06:20:39 +0000 (23:20 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 8 Jul 2020 23:37:43 +0000 (16:37 -0700)
The fetch.writeCommitGraph feature makes fetches write out a commit
graph file for the newly downloaded pack on fetch.  This improves the
performance of various commands that would perform a revision walk and
eventually ought to be the default for everyone.  To prepare for that
future, it's enabled by default for users that set
feature.experimental=true to experience such future defaults.

Alas, for --unshallow fetches from a shallow clone it runs into a
snag: by the time Git has fetched the new objects and is writing a
commit graph, it has performed a revision walk and r->parsed_objects
contains information about the shallow boundary from *before* the
fetch.  The commit graph writing code is careful to avoid writing a
commit graph file in shallow repositories, but the new state is not
shallow, and the result is that from that point on, commands like "git
log" make use of a newly written commit graph file representing a
fictional history with the old shallow boundary.

We could fix this by making the commit graph writing code more careful
to avoid writing a commit graph that could have used any grafts or
shallow state, but it is possible that there are other pieces of
mutated state that fetch's commit graph writing code may be relying
on.  So disable it in the feature.experimental configuration.

Google developers have been running in this configuration (by setting
fetch.writeCommitGraph=false in the system config) to work around this
bug since it was discovered in April.  Once the fix lands, we'll
enable fetch.writeCommitGraph=true again to give it some early testing
before rolling out to a wider audience.

In other words:

- this patch only affects behavior with feature.experimental=true

- it makes feature.experimental match the configuration Google has
  been using for the last few months, meaning it would leave users in
  a better tested state than without it

- this should improve testing for other features guarded by
  feature.experimental, by making feature.experimental safer to use

Reported-by: Jay Conrod <jayconrod@google.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/config/feature.txt
Documentation/config/fetch.txt
repo-settings.c

index 28c33602d527fa152b2702dab8a07ce3a5373c82..c0cbf2bb1cd58d426d05139d6068d1f6be5f9f50 100644 (file)
@@ -15,14 +15,6 @@ feature.experimental::
 * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
 skipping more commits at a time, reducing the number of round trips.
 +
-* `fetch.writeCommitGraph=true` writes a commit-graph after every `git fetch`
-command that downloads a pack-file from a remote. Using the `--split` option,
-most executions will create a very small commit-graph file on top of the
-existing commit-graph file(s). Occasionally, these files will merge and the
-write may take longer. Having an updated commit-graph file helps performance
-of many Git commands, including `git merge-base`, `git push -f`, and
-`git log --graph`.
-+
 * `protocol.version=2` speeds up fetches from repositories with many refs by
 allowing the client to specify which refs to list before the server lists
 them.
index b1a9b1461d30f4e79bd194d968cebbb8617b3181..b20394038d1eef83d7e454cf0fee7ceb8a457a0f 100644 (file)
@@ -90,5 +90,4 @@ fetch.writeCommitGraph::
        the existing commit-graph file(s). Occasionally, these files will
        merge and the write may take longer. Having an updated commit-graph
        file helps performance of many Git commands, including `git merge-base`,
-       `git push -f`, and `git log --graph`. Defaults to false, unless
-       `feature.experimental` is true.
+       `git push -f`, and `git log --graph`. Defaults to false.
index dc6817daa9537c3877dae686368ef58708ac0512..0918408b34436a02589cdb850da8345671e4a5f1 100644 (file)
@@ -51,14 +51,14 @@ void prepare_repo_settings(struct repository *r)
                UPDATE_DEFAULT_BOOL(r->settings.index_version, 4);
                UPDATE_DEFAULT_BOOL(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
        }
+
        if (!repo_config_get_bool(r, "fetch.writecommitgraph", &value))
                r->settings.fetch_write_commit_graph = value;
-       if (!repo_config_get_bool(r, "feature.experimental", &value) && value) {
-               UPDATE_DEFAULT_BOOL(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
-               UPDATE_DEFAULT_BOOL(r->settings.fetch_write_commit_graph, 1);
-       }
        UPDATE_DEFAULT_BOOL(r->settings.fetch_write_commit_graph, 0);
 
+       if (!repo_config_get_bool(r, "feature.experimental", &value) && value)
+               UPDATE_DEFAULT_BOOL(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
+
        /* Hack for test programs like test-dump-untracked-cache */
        if (ignore_untracked_cache_config)
                r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;