Re: [RFC PATCH 5/6] fetch: destroy commit graph on shallow parameters
- From
Stefan Beller <sbeller@google.com>
- Date
- May 31, 2018, 19:29 UTC
- Message-ID
- <CAGZ79kZp3SG8D_aap7k7TDUaGsA7_8ufhkiJK7VR4nGQohVDDQ@mail.gmail.com>
- In-Reply-To
- <20180531174024.124488-6-dstolee@microsoft.com>
On Thu, May 31, 2018 at 10:41 AM, Derrick Stolee <dstolee@microsoft.com> wrote:
> The commit-graph feature is not compatible with history-rewriting > features such as shallow clones.
I associate "history rewriting" with changing objects in the history. For example interactive rebase or the BFG cleaner[1] / filter-branch to remove certain commits from other commits as parents. This history rewriting leads to different sha1s, and the commit graph feature is compatible with that in the sense that you can add all the new sha1s /commits to the graph and prune out the old unreferenced commits.
Shallow clones are not rewriting history IMHO, as the sha1s do not change. What changes is the assumption of presence of the parent commits (which makes it hard to compute the generation number), by the grafting trick, that "overlays" (?) history.
This is more of a nit, though.
[1] https://rtyley.github.io/bfg-repo-cleaner/
> When running a 'git fetch' with > any of the shallow/unshallow options, destroy the commit-graph > file and override core.commitGraph to be false.
We do that *before* the actual fetch happens such that the improved negotiation of the future cannot even try to benefit from generation numbers?
We do it at fetch time instead of other local operations as that is an entry point to commit-graph incompatible features. Would this also be needed in clone?
I was about to ask if a more fine grained inclusion to lookup_commit would make sense, but that is explicitely called out in the cover letter as 'too hard for now'.
Show 18 quoted lines
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> builtin/fetch.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index af896e4b74..2001dca881 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1452,6 +1452,12 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> }
> }
>
> + if (update_shallow || depth || deepen_since || deepen_not.nr ||
> + deepen_relative || unshallow || update_shallow || is_repository_shallow()) {
> + destroy_commit_graph(get_object_directory());
> + core_commit_graph = 0;