| Message ID | f1fc89894b8832ab0f64f301b1621ae15654e08c.1628618950.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | In grep, no adding submodule ODB as alternates | expand |
On Tue, Aug 10, 2021 at 11:28:45AM -0700, Jonathan Tan wrote: > > The previous patches have made "git grep" no longer need to add > submodule ODBs as alternates, at least for the code paths tested in > t7814. Demonstrate this by making adding a submodule ODB as an alternate > fatal in this test. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > t/t7814-grep-recurse-submodules.sh | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh > index 828cb3ba58..3172f5b936 100755 > --- a/t/t7814-grep-recurse-submodules.sh > +++ b/t/t7814-grep-recurse-submodules.sh > @@ -8,6 +8,9 @@ submodules. > > . ./test-lib.sh > > +GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 > +export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB > + > test_expect_success 'setup directory structure and submodule' ' > echo "(1|2)d(3|4)" >a && > mkdir b && > -- > 2.33.0.rc1.237.g0d66db33f3-goog > This proof seems pretty handy, assuming nobody else is directly calling add_to_alternates_memory() (and therefore skipping the envvar check). I would feel slightly more convinced if that function was file-static somewhere, but I am not familiar with the problem space enough to say whether that's possible. This patch by itself, though: Reviewed-by: Emily Shaffer <emilyshaffer@google.com> - Emily
On Wed, Aug 11, 2021 at 6:55 PM Emily Shaffer <emilyshaffer@google.com> wrote: > > On Tue, Aug 10, 2021 at 11:28:45AM -0700, Jonathan Tan wrote: > > > > The previous patches have made "git grep" no longer need to add > > submodule ODBs as alternates, at least for the code paths tested in > > t7814. Demonstrate this by making adding a submodule ODB as an alternate > > fatal in this test. > > > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > > --- > > t/t7814-grep-recurse-submodules.sh | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh > > index 828cb3ba58..3172f5b936 100755 > > --- a/t/t7814-grep-recurse-submodules.sh > > +++ b/t/t7814-grep-recurse-submodules.sh > > @@ -8,6 +8,9 @@ submodules. > > > > . ./test-lib.sh > > > > +GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 > > +export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB > > + > > test_expect_success 'setup directory structure and submodule' ' > > echo "(1|2)d(3|4)" >a && > > mkdir b && > > -- > > 2.33.0.rc1.237.g0d66db33f3-goog > > > > This proof seems pretty handy, assuming nobody else is directly calling > add_to_alternates_memory() (and therefore skipping the envvar check). Hmm, there is at least one call chain in grep which might end up calling add_to_alternates_memory() directly (although it only seems to happen on a very specific case): grep_submodule > repo_read_gitmodules > config_from_gitmodules > add_to_alternates_memory We can check that with the following: git init A git init A/B git init A/B/C echo f >A/B/C/f git -C A/B/C add f git -C A/B/C commit -m f git -C A/B submodule add ./C git -C A/B commit -m C git -C A submodule add ./B git -C A commit -m B rm B/.gitmodules gdb -ex 'break add_to_alternates_memory' -ex 'run' --args \ git grep --recurse-submodules .
> Hmm, there is at least one call chain in grep which might end up > calling add_to_alternates_memory() directly (although it only seems to > happen on a very specific case): > > grep_submodule > repo_read_gitmodules > > config_from_gitmodules > add_to_alternates_memory > > We can check that with the following: [snip reproduction recipe] Thanks - it looks like my patch set is incomplete then. I'll make config_from_gitmodules() use my new function and if the existing grep test cases don't cover your reproduction recipe, I'll add yours in.
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh index 828cb3ba58..3172f5b936 100755 --- a/t/t7814-grep-recurse-submodules.sh +++ b/t/t7814-grep-recurse-submodules.sh @@ -8,6 +8,9 @@ submodules. . ./test-lib.sh +GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 +export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB + test_expect_success 'setup directory structure and submodule' ' echo "(1|2)d(3|4)" >a && mkdir b &&
The previous patches have made "git grep" no longer need to add submodule ODBs as alternates, at least for the code paths tested in t7814. Demonstrate this by making adding a submodule ODB as an alternate fatal in this test. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- t/t7814-grep-recurse-submodules.sh | 3 +++ 1 file changed, 3 insertions(+)