Re: [RFC PATCH 0/3] grep: don't add subrepos to in-memory alternates

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Sep 21, 2019, 20:34 UTC
Message-ID: <CAHd-oW5iEQarYVxEXoTG-ua2zdoybTrSjCBKtO0YT292fm0NQQ@mail.gmail.com>
In-Reply-To: <xmqqh857vsqz.fsf@gitster-ct.c.googlers.com>

On Fri, Sep 20, 2019 at 1:26 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

>
> Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes:
>
> > Hmm, I may have gotten a little confused here. Are you talking
> > about the attributes stack (which contains .gitattributes and
> > info/attributes)?  If so, isn't this stack already rebuild for
> > every path? I mean, by the previous call chain it seems to me that
> > at least these two files are reread for every path.
>
> Yes, but for the switch that happens when coming out of a normal
> directory and then descending into another normal directory is just
> to pop the entries from the directory hierarchy we are getting out
> of, and then pushing the entries from the new directory hierarchy.
> We would not be discarding and rereading $GIT_DIR/info/ or the
> .gitattribute file from the top-level of the working tree.

Right, this would be the best way of doing it. However, I think this is not how it's currently implemented. I if correctly understood the code in this call chain:

grep_source_load_driver() > userdiff_find_by_path() > git_check_attr() > collect_some_attrs() > prepare_attr_stack() > bootstrap_attr_stack()

it seems that the whole stack is being rebuild for every path (even for paths descending in the same superproject or submodule). So $GIT_DIR/info/ and .gitattributes are being discarded and reread every time :(

Show 5 quoted lines

> Descending into a submodule is fundamentally and completely
> different.  None of the attributes defined in the superproject
> should affect the paths in the submodule, as it is a totally
> separate project, oblivious to the existence of enclosing the
> superproject.

I think we currently have a bug here as the attributes from the superproject *are* affecting the paths in the submodule. Here is a small script I wrote to test this: https://gitlab.com/snippets/1896951

The cause of this problem is that boostrap_attr_stack() doesn't read "<subrepo_prefix>/.gitattributes" but just ".gitattributes", always getting the superproject's file not the suprepo's. Yet another problem is that when this file is not present and we need to retrieve it from the index, this function calls read_attr() > read_attr_from_index() > read_blob_data_from_index(). The last one always reads from the_repository's odb, so it won't ever find the subrepo's ".gitattributes".

And a third bug is that when reading attributes of paths inside subrepo's directories, from index, we call read_attr_from_index() with a path such as "<subrepo_prefix>/<subdir>/.gitattributes". However, the subrepo_prefix should be stripped when looking in the subrepo's index, otherwise there will be no matches.

To fix these three problems, I think we would need to pass on a struct repository in these call chains. But this would require a very big modification as there are many places that can lead to one of them... And there're corner cases such as index_stream_convert_blob() which would need to receive a struct repository but it always writes to the_repo, which would be kind of inconsistent. Do you think this would be a good solution or should I try something else?