Re: [PATCH v5 9/9] submodule: support reading .gitmodules when it's not in the working tree

From: Junio C Hamano <gitster@pobox.com>
To: Antonio Ospite <ao2@ao2.it>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	git@vger.kernel.org, "Brandon Williams" <bmwill@google.com>,
	"Daniel Graña" <dangra@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Richard Hartmann" <richih.mailinglist@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v5 9/9] submodule: support reading .gitmodules when it's not in the working tree
Date: Fri, 21 Sep 2018 09:19:45 -0700	[thread overview]
Message-ID: <xmqq5zyyyg9q.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180920173552.6109014827a062dcf3821632@ao2.it> (Antonio Ospite's message of "Thu, 20 Sep 2018 17:35:52 +0200")

Antonio Ospite <ao2@ao2.it> writes:

> Protecting the problematic submodules function could work for now, but
> I'd like to have more comments, my proposal is:
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 601f801158..52b45de749 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -427,6 +427,11 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
>         if (repo_submodule_init(&submodule, superproject, path))
>                 return 0;
>
> +       grep_read_lock();
> +       /*
> +        * NEEDSWORK: repo_read_gitmodules accesses the object store which is
> +        * global, thus it needs to be protected.
> +        */
>         repo_read_gitmodules(&submodule);
>
>         /*
> @@ -439,7 +444,6 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
>          * store is no longer global and instead is a member of the repository
>          * object.
>          */
> -       grep_read_lock();
>         add_to_alternates_memory(submodule.objects->objectdir);
>         grep_read_unlock();

I think this is in line with how the grep codepath protects itself
when doing anything that accesses the object store.

Thanks.