Re: [PATCH] experimental: default to fetch.writeCommitGraph=false
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 7, 2020, 15:09 UTC
- Message-ID
- <xmqq8sfv745r.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200707062039.GC784740@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> The fetch.writeCommitGraph feature makes fetches write out a commit
Show 10 quoted lines
> 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
In other words, fetch.writeCommitGraph in its current form is too broken to be recommended even for brave souls with "experimental" bit on.
I wonder if we perhaps wnat to add to the documentation for writeCommitGraph configuration that its use is currently not recommended in a shallow clone or something (I know it is not a problem just to use it with shallow but the breakage needs to involve unshallowing, but by definition those who do not use shallow would not hit the unshallowing bug, so...).
Show 9 quoted lines
> Reported-by: Jay Conrod <jayconrod@google.com> > Helped-by: Taylor Blau <me@ttaylorr.com> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> > --- > I realize this is late to send. That said, as described above, I > think it's a good way to buy time by minimizing user exposure to > fetch.writeCommitGraph=true until a fix for it is well cooked. > > In other words, I'd like to see this patch in Git 2.28-rc0.
Yes, I do, too.
Thanks.
Show 34 quoted lines
> diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
> index b1a9b1461d3..b20394038d1 100644
> --- a/Documentation/config/fetch.txt
> +++ b/Documentation/config/fetch.txt
> @@ -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.
> diff --git a/repo-settings.c b/repo-settings.c
> index dc6817daa95..0918408b344 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -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;