Re: [PATCH 2/3] commit: write commit-graph with bloom filters

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 13, 2020, 22:11 UTC
Message-ID: <xmqqtv1navdd.fsf@gitster.c.googlers.com>
In-Reply-To: <8ef47003-784d-6a27-666d-5c916c4deb33@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

> If we keep this simple, or extract the process to a
> "write_commit_graph_for_tests()" macro inside builtin.h, then we could
> insert a commit-graph write in more places.

As long as the check necessary is cheap enough to realize that we are in production mode, we should be able to keep the run-time overhead to the minimum. Sprinkling such a call all over the place, however, might add to the overhead of reading code, though.

> However, I think there is value in testing the "not every commit is
> included in the commit-graph" case, which our current setup does quite
> nicely.

Yeah, that is also a good point.

> The one exception is that 'git merge' could benefit from this
> snippet, so we cap off the commit-graph whenever a test script
> "constructs" a repository to match a data shape.

Sounds good.

Show 5 quoted lines

> The problem here is that if GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1 and
> someone runs "git commit-graph write --no-changed-paths" then the
> negation of that option is ignored. But this is a GIT_TEST_* variable,
> and any test that requires that check could disable the enviroment
> variable first.

Yeah, that sounds good.