From fd4faf7a5d5d92ca3e50f5a6ee2f94676d83530b Mon Sep 17 00:00:00 2001 From: Shuqi Liang Date: Fri, 11 Aug 2023 10:22:09 -0400 Subject: [PATCH 1/3] t1092: add tests for 'git check-attr' Add tests for `git check-attr`, make sure attribute file does get read from index when path is either inside or outside of sparse-checkout definition. Add a test named 'diff --check with pathspec outside sparse definition'. It starts by disabling the trailing whitespace and space-before-tab checks using the core. whitespace configuration option. Then, it specifically re-enables the trailing whitespace check for a file located in a sparse directory by adding a whitespace=trailing-space rule to the .gitattributes file within that directory. Next, create and populate the folder1 directory, and then add the .gitattributes file to the index. Edit the contents of folder1/a, add it to the index, and proceed to "re-sparsify" 'folder1/' with 'git sparse-checkout reapply'. Finally, use 'git diff --check --cached' to compare the 'index vs. HEAD', ensuring the correct application of the attribute rules even when the file's path is outside the sparse-checkout definition. Mark the two tests 'check-attr with pathspec outside sparse definition' and 'diff --check with pathspec outside sparse definition' as 'test_expect_failure' to reflect an existing issue where the attributes inside a sparse directory are ignored. Ensure that the 'check-attr' command fails to read the required attributes to demonstrate this expected failure. Helped-by: Victoria Dye Signed-off-by: Shuqi Liang Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 8a95adf4b5..2d7fa65d81 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2259,4 +2259,53 @@ test_expect_success 'worktree is not expanded' ' ensure_not_expanded worktree remove .worktrees/hotfix ' +test_expect_success 'check-attr with pathspec inside sparse definition' ' + init_repos && + + echo "a -crlf myAttr" >>.gitattributes && + run_on_all cp ../.gitattributes ./deep && + + test_all_match git check-attr -a -- deep/a && + + test_all_match git add deep/.gitattributes && + test_all_match git check-attr -a --cached -- deep/a +' + +test_expect_failure 'check-attr with pathspec outside sparse definition' ' + init_repos && + + echo "a -crlf myAttr" >>.gitattributes && + run_on_sparse mkdir folder1 && + run_on_all cp ../.gitattributes ./folder1 && + run_on_all cp a folder1/a && + + test_all_match git check-attr -a -- folder1/a && + + git -C full-checkout add folder1/.gitattributes && + test_sparse_match git add --sparse folder1/.gitattributes && + test_all_match git commit -m "add .gitattributes" && + test_sparse_match git sparse-checkout reapply && + test_all_match git check-attr -a --cached -- folder1/a +' + +test_expect_failure 'diff --check with pathspec outside sparse definition' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo "a " >"$1" + EOF + + test_all_match git config core.whitespace -trailing-space,-space-before-tab && + + echo "a whitespace=trailing-space,space-before-tab" >>.gitattributes && + run_on_all mkdir -p folder1 && + run_on_all cp ../.gitattributes ./folder1 && + test_all_match git add --sparse folder1/.gitattributes && + run_on_all ../edit-contents folder1/a && + test_all_match git add --sparse folder1/a && + + test_sparse_match git sparse-checkout reapply && + test_all_match test_must_fail git diff --check --cached -- folder1/a +' + test_done From 4723ae1007f94cbb391c4fb5a042adccd038d9ae Mon Sep 17 00:00:00 2001 From: Shuqi Liang Date: Fri, 11 Aug 2023 10:22:10 -0400 Subject: [PATCH 2/3] attr.c: read attributes in a sparse directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch, git check-attr was unable to read the attributes from a .gitattributes file within a sparse directory. The original comment was operating under the assumption that users are only interested in files or directories inside the cones. Therefore, in the original code, in the case of a cone-mode sparse-checkout, we didn't load the .gitattributes file. However, this behavior can lead to missing attributes for files inside sparse directories, causing inconsistencies in file handling. To resolve this, revise 'git check-attr' to allow attribute reading for files in sparse directories from the corresponding .gitattributes files: 1.Utilize path_in_cone_mode_sparse_checkout() and index_name_pos_sparse to check if a path falls within a sparse directory. 2.If path is inside a sparse directory, employ the value of index_name_pos_sparse() to find the sparse directory containing path and path relative to sparse directory. Proceed to read attributes from the tree OID of the sparse directory using read_attr_from_blob(). 3.If path is not inside a sparse directory，ensure that attributes are fetched from the index blob with read_blob_data_from_index(). Change the test 'check-attr with pathspec outside sparse definition' to 'test_expect_success' to reflect that the attributes inside a sparse directory can now be read. Ensure that the sparse index case works correctly for git check-attr to illustrate the successful handling of attributes within sparse directories. Helped-by: Victoria Dye Signed-off-by: Shuqi Liang Signed-off-by: Junio C Hamano --- attr.c | 57 ++++++++++++++++-------- t/t1092-sparse-checkout-compatibility.sh | 10 ++++- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/attr.c b/attr.c index e5785c55db..d85fd6d2ed 100644 --- a/attr.c +++ b/attr.c @@ -808,35 +808,56 @@ static struct attr_stack *read_attr_from_blob(struct index_state *istate, static struct attr_stack *read_attr_from_index(struct index_state *istate, const char *path, unsigned flags) { + struct attr_stack *stack = NULL; char *buf; unsigned long size; + int sparse_dir_pos = -1; if (!istate) return NULL; /* - * The .gitattributes file only applies to files within its - * parent directory. In the case of cone-mode sparse-checkout, - * the .gitattributes file is sparse if and only if all paths - * within that directory are also sparse. Thus, don't load the - * .gitattributes file since it will not matter. - * - * In the case of a sparse index, it is critical that we don't go - * looking for a .gitattributes file, as doing so would cause the - * index to expand. + * When handling sparse-checkouts, .gitattributes files + * may reside within a sparse directory. We distinguish + * whether a path exists directly in the index or not by + * evaluating if 'pos' is negative. + * If 'pos' is negative, the path is not directly present + * in the index and is likely within a sparse directory. + * For paths not in the index, The absolute value of 'pos' + * minus 1 gives us the position where the path would be + * inserted in lexicographic order within the index. + * We then subtract another 1 from this value + * (sparse_dir_pos = -pos - 2) to find the position of the + * last index entry which is lexicographically smaller than + * the path. This would be the sparse directory containing + * the path. By identifying the sparse directory containing + * the path, we can correctly read the attributes specified + * in the .gitattributes file from the tree object of the + * sparse directory. */ - if (!path_in_cone_mode_sparse_checkout(path, istate)) - return NULL; + if (!path_in_cone_mode_sparse_checkout(path, istate)) { + int pos = index_name_pos_sparse(istate, path, strlen(path)); - 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 (pos < 0) + sparse_dir_pos = -pos - 2; } - return read_attr_from_buf(buf, path, flags); + if (sparse_dir_pos >= 0 && + S_ISSPARSEDIR(istate->cache[sparse_dir_pos]->ce_mode) && + !strncmp(istate->cache[sparse_dir_pos]->name, path, ce_namelen(istate->cache[sparse_dir_pos]))) { + const char *relative_path = path + ce_namelen(istate->cache[sparse_dir_pos]); + stack = read_attr_from_blob(istate, &istate->cache[sparse_dir_pos]->oid, relative_path, flags); + } else { + 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; + } + stack = read_attr_from_buf(buf, path, flags); + } + return stack; } static struct attr_stack *read_attr(struct index_state *istate, diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 2d7fa65d81..dc84b3e2e1 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2271,7 +2271,7 @@ test_expect_success 'check-attr with pathspec inside sparse definition' ' test_all_match git check-attr -a --cached -- deep/a ' -test_expect_failure 'check-attr with pathspec outside sparse definition' ' +test_expect_success 'check-attr with pathspec outside sparse definition' ' init_repos && echo "a -crlf myAttr" >>.gitattributes && @@ -2288,6 +2288,14 @@ test_expect_failure 'check-attr with pathspec outside sparse definition' ' test_all_match git check-attr -a --cached -- folder1/a ' +# NEEDSWORK: The 'diff --check' test is left as 'test_expect_failure' due +# to an underlying issue in oneway_diff() within diff-lib.c. +# 'do_oneway_diff()' is not called as expected for paths that could match +# inside of a sparse directory. Specifically, the 'ce_path_match()' function +# fails to recognize files inside a sparse directory (e.g., when 'folder1/' +# is a sparse directory, 'folder1/a' cannot be recognized). The goal is to +# proceed with 'do_oneway_diff()' if the pathspec could match inside of a +# sparse directory. test_expect_failure 'diff --check with pathspec outside sparse definition' ' init_repos && From f9815878c16892aae7c5be11ac67200aef6e4ff6 Mon Sep 17 00:00:00 2001 From: Shuqi Liang Date: Fri, 11 Aug 2023 10:22:11 -0400 Subject: [PATCH 3/3] check-attr: integrate with sparse-index Set the requires-full-index to false for "check-attr". Add a test to ensure that the index is not expanded whether the files are outside or inside the sparse-checkout cone when the sparse index is enabled. The `p2000` tests demonstrate a ~63% execution time reduction for 'git check-attr' using a sparse index. Test before after ----------------------------------------------------------------------- 2000.106: git check-attr -a f2/f4/a (full-v3) 0.05 0.05 +0.0% 2000.107: git check-attr -a f2/f4/a (full-v4) 0.05 0.05 +0.0% 2000.108: git check-attr -a f2/f4/a (sparse-v3) 0.04 0.02 -50.0% 2000.109: git check-attr -a f2/f4/a (sparse-v4) 0.04 0.01 -75.0% Helped-by: Victoria Dye Signed-off-by: Shuqi Liang Signed-off-by: Junio C Hamano --- builtin/check-attr.c | 3 +++ t/perf/p2000-sparse-operations.sh | 1 + t/t1092-sparse-checkout-compatibility.sh | 15 +++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/builtin/check-attr.c b/builtin/check-attr.c index b22ff748c3..c1da1d184e 100644 --- a/builtin/check-attr.c +++ b/builtin/check-attr.c @@ -122,6 +122,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, check_attr_options, check_attr_usage, PARSE_OPT_KEEP_DASHDASH); + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + if (repo_read_index(the_repository) < 0) { die("invalid cache"); } diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 96ed3e1d69..39e92b0841 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -134,5 +134,6 @@ test_perf_on_all git diff-files -- $SPARSE_CONE/a test_perf_on_all git diff-tree HEAD test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a test_perf_on_all "git worktree add ../temp && git worktree remove ../temp" +test_perf_on_all git check-attr -a -- $SPARSE_CONE/a test_done diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index dc84b3e2e1..2a4f35e984 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2316,4 +2316,19 @@ test_expect_failure 'diff --check with pathspec outside sparse definition' ' test_all_match test_must_fail git diff --check --cached -- folder1/a ' +test_expect_success 'sparse-index is not expanded: check-attr' ' + init_repos && + + echo "a -crlf myAttr" >>.gitattributes && + mkdir ./sparse-index/folder1 && + cp ./sparse-index/a ./sparse-index/folder1/a && + cp .gitattributes ./sparse-index/deep && + cp .gitattributes ./sparse-index/folder1 && + + git -C sparse-index add deep/.gitattributes && + git -C sparse-index add --sparse folder1/.gitattributes && + ensure_not_expanded check-attr -a --cached -- deep/a && + ensure_not_expanded check-attr -a --cached -- folder1/a +' + test_done