Re: [PATCH v3 1/3] attr.c: read attributes in a sparse directory

From: Shuqi Liang <cheskaqiqi@gmail.com>
Date: Jul 13, 2023, 20:22 UTC
Message-ID: <CAMO4yUEh+HMZi8wC1aB=6oLCJkn3CNbw0reVAA-vULfVgF+=NA@mail.gmail.com>
In-Reply-To: <xmqqfs5uw178.fsf@gitster.g>

On Tue, Jul 11, 2023 at 6:08 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> -    if (!path_in_cone_mode_sparse_checkout(path, istate))
> >> -            return NULL;
> >> +    pos = index_name_pos_sparse(istate, path, strlen(path));
> >> +    pos = - pos - 2;
> >>
> >> -    buf = read_blob_data_from_index(istate, path, &size);
> >> -    if (!buf)
> >> -            return NULL;
> >> -    if (size >= ATTR_MAX_FILE_SIZE) {
> >> -            warning(_("ignoring overly large gitattributes blob '%s'"), path);
> >> -            return NULL;
> >> -    }
> >> +    if (!path_in_cone_mode_sparse_checkout(path, istate) && 0 <= pos) {
> >> +            if (!S_ISSPARSEDIR(istate->cache[pos]->ce_mode))
> >> +                    return NULL;
>
> Another thing I forgot to ask.  When we are asked to read
> ".gitattributes" at the top level, does this code work correctly?
> As ".gitattributes" is at the root level, it won't be hidden inside
> a sparsified directory in the index, and we do not have to search
> for its parent.  I just wanted to see if the relative_path computation
> and other things we see below will safely be skipped in such a case.

Yeah, this code works correctly. I added those tests in t1092 and they passed successfully.

test_expect_success 'check-attr with pathspec inside sparse definition' ' init_repos &&

    echo "a -crlf myAttr" >>.gitattributes &&
    run_on_all cp ../.gitattributes . &&

    test_all_match git check-attr -a -- deep/a &&

    test_all_match git add .gitattributes &&
    test_all_match git check-attr -a --cached -- deep/a
'
test_expect_success 'check-attr with pathspec inside sparse definition' '
init_repos &&

    echo "a -crlf myAttr" >>.gitattributes &&
    run_on_all cp ../.gitattributes . &&

    test_all_match git check-attr -a -- folder1/a &&

    test_all_match git add .gitattributes &&
    test_all_match git check-attr -a --cached -- folder1/a
'

Do I need to modify t1092 to include cases like this?