From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"Junio C Hamano" <gitster@pobox.com>,
"Victoria Dye" <vdye@github.com>,
"Derrick Stolee" <stolee@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Derrick Stolee" <derrickstolee@github.com>,
"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH v2 2/5] ls-files: add --sparse option
Date: Wed, 8 Dec 2021 21:08:59 -0800 [thread overview]
Message-ID: <CABPp-BF5kw69ioheMvRPoVso8FJf45Fdm_Cf-P7PfqWsqHz9Gw@mail.gmail.com> (raw)
In-Reply-To: <58b5eca483502b0d45601490f6b4255f6419e6bd.1638992396.git.gitgitgadget@gmail.com>
On Wed, Dec 8, 2021 at 11:39 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> Existing callers to 'git ls-files' are expecting file names, not
> directories. It is best to expand a sparse index to show all of the
> contained files in this case.
>
> However, expert users may want to inspect the contents of the index
> itself including which directories are sparse. Add a --sparse option to
> allow users to request this information.
>
> During testing, I noticed that options such as --modified did not affect
> the output when the files in question were outside the sparse-checkout
> definition. Tests are added to document this preexisting behavior and
> how it remains unchanged with the sparse index and the --sparse option.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> Documentation/git-ls-files.txt | 4 ++
> builtin/ls-files.c | 12 +++-
> t/t1092-sparse-checkout-compatibility.sh | 90 ++++++++++++++++++++++++
> 3 files changed, 104 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index 6d11ab506b7..1c5d5f85ec5 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -187,6 +187,10 @@ Both the <eolinfo> in the index ("i/<eolinfo>")
> and in the working tree ("w/<eolinfo>") are shown for regular files,
> followed by the ("attr/<eolattr>").
>
> +--sparse::
> + If the index is sparse, show the sparse directories without expanding
> + to the contained files.
> +
> \--::
> Do not interpret any more arguments as options.
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 031fef1bcaa..c151eb1fb77 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -37,6 +37,7 @@ static int debug_mode;
> static int show_eol;
> static int recurse_submodules;
> static int skipping_duplicates;
> +static int show_sparse_dirs;
>
> static const char *prefix;
> static int max_prefix_len;
> @@ -315,8 +316,10 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
>
> if (!(show_cached || show_stage || show_deleted || show_modified))
> return;
> - /* TODO: audit for interaction with sparse-index. */
> - ensure_full_index(repo->index);
> +
> + if (!show_sparse_dirs)
> + ensure_full_index(repo->index);
> +
> for (i = 0; i < repo->index->cache_nr; i++) {
> const struct cache_entry *ce = repo->index->cache[i];
> struct stat st;
> @@ -670,6 +673,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
> OPT_BOOL(0, "debug", &debug_mode, N_("show debugging data")),
> OPT_BOOL(0, "deduplicate", &skipping_duplicates,
> N_("suppress duplicate entries")),
> + OPT_BOOL(0, "sparse", &show_sparse_dirs,
> + N_("show sparse directories in the presence of a sparse index")),
> OPT_END()
> };
> int ret = 0;
> @@ -677,6 +682,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
> if (argc == 2 && !strcmp(argv[1], "-h"))
> usage_with_options(ls_files_usage, builtin_ls_files_options);
>
> + prepare_repo_settings(the_repository);
> + the_repository->settings.command_requires_full_index = 0;
> +
> prefix = cmd_prefix;
> if (prefix)
> prefix_len = strlen(prefix);
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 3ba19ba1c14..bf2c6b169b9 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -802,6 +802,12 @@ test_expect_success 'sparse-index is expanded and converted back' '
> GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
> git -C sparse-index reset -- folder1/a &&
> test_region index convert_to_sparse trace2.txt &&
> + test_region index ensure_full_index trace2.txt &&
> +
> + # ls-files expands on read, but does not write.
> + rm trace2.txt &&
> + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
> + git -C sparse-index ls-files &&
> test_region index ensure_full_index trace2.txt
> '
>
> @@ -826,6 +832,7 @@ test_expect_success 'sparse-index is not expanded' '
> init_repos &&
>
> ensure_not_expanded status &&
> + ensure_not_expanded ls-files --sparse &&
> ensure_not_expanded commit --allow-empty -m empty &&
> echo >>sparse-index/a &&
> ensure_not_expanded commit -a -m a &&
> @@ -974,6 +981,89 @@ test_expect_success 'sparse index is not expanded: fetch/pull' '
> ensure_not_expanded pull full base
> '
>
> +test_expect_success 'ls-files' '
> + init_repos &&
> +
> + # Behavior agrees by default. Sparse index is expanded.
> + test_all_match git ls-files &&
> +
> + # With --sparse, the sparse index data changes behavior.
> + git -C sparse-index ls-files >dense &&
> + git -C sparse-index ls-files --sparse >sparse &&
> +
> + cat >expect <<-\EOF &&
> + @@ -13,13 +13,9 @@
> + e
> + folder1-
> + folder1.x
> + -folder1/0/0/0
> + -folder1/0/1
> + -folder1/a
> + +folder1/
> + folder10
> + -folder2/0/0/0
> + -folder2/0/1
> + -folder2/a
> + +folder2/
> + g
> + -x/a
> + +x/
> + z
> + EOF
> +
> + diff -u dense sparse | tail -n +3 >actual &&
> + test_cmp expect actual &&
> +
> + # With --sparse and no sparse index, nothing changes.
> + git -C sparse-checkout ls-files >dense &&
> + git -C sparse-checkout ls-files --sparse >sparse &&
> + test_cmp dense sparse &&
> +
I still believe this next section deserves a comment; something like
# Set up an erroneous condition: folder1/a present despite SKIP_WORKTREE.
# We do this just to verify consistency between sparse-index and
non-sparse-index
# with such files.
> + write_script edit-content <<-\EOF &&
> + mkdir folder1 &&
> + echo content >>folder1/a
> + EOF
> + run_on_sparse ../edit-content &&
> +
> + # ls-files does not notice modified files whose
> + # cache entries are marked SKIP_WORKTREE.
I also believe this still deserves a slight change, e.g.
# ls-files does not currently notice modified files whose
# cache entries are marked SKIP_WORKTREE; this
# may change in the future, but we test here that
# sparse-index and non-sparse-index at least match.
> + test_sparse_match git ls-files --modified &&
> + test_must_be_empty sparse-checkout-out &&
> + test_must_be_empty sparse-index-out &&
> +
> + git -C sparse-index ls-files --sparse --modified >sparse-index-out &&
> + test_must_be_empty sparse-index-out &&
> +
> + # Add folder1 to the sparse-checkout cone and
> + # check that ls-files shows the expanded files.
> + test_sparse_match git sparse-checkout add folder1 &&
> + test_sparse_match git ls-files --modified &&
> +
> + git -C sparse-index ls-files >dense &&
> + git -C sparse-index ls-files --sparse >sparse &&
> +
> + cat >expect <<-\EOF &&
> + @@ -17,9 +17,7 @@
> + folder1/0/1
> + folder1/a
> + folder10
> + -folder2/0/0/0
> + -folder2/0/1
> + -folder2/a
> + +folder2/
> + g
> + -x/a
> + +x/
> + z
> + EOF
> +
> + diff -u dense sparse | tail -n +3 >actual &&
> + test_cmp expect actual &&
> +
> + # Double-check index expansion is avoided
> + ensure_not_expanded ls-files --sparse
> +'
> +
> # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
> # in this scenario, but it shouldn't.
> test_expect_success 'reset mixed and checkout orphan' '
> --
> gitgitgadget
next prev parent reply other threads:[~2021-12-09 5:09 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-16 15:38 [PATCH 0/2] Sparse index: fetch, pull, ls-files Derrick Stolee via GitGitGadget
2021-11-16 15:38 ` [PATCH 1/2] fetch/pull: use the sparse index Derrick Stolee via GitGitGadget
2021-11-16 15:38 ` [PATCH 2/2] ls-files: add --sparse option Derrick Stolee via GitGitGadget
2021-11-22 18:36 ` Elijah Newren
2021-11-22 19:44 ` Derrick Stolee
2021-11-23 2:07 ` Ævar Arnfjörð Bjarmason
2021-12-08 15:14 ` Derrick Stolee
2021-12-08 15:20 ` Derrick Stolee
2021-12-08 17:04 ` Elijah Newren
2021-12-08 18:23 ` Derrick Stolee
2021-12-08 18:36 ` Elijah Newren
2021-12-08 19:06 ` Derrick Stolee
2021-12-09 12:50 ` Ævar Arnfjörð Bjarmason
2021-12-10 13:57 ` Derrick Stolee
2021-12-10 15:13 ` Ævar Arnfjörð Bjarmason
2021-12-13 19:16 ` Junio C Hamano
2021-12-16 14:11 ` Derrick Stolee
2021-11-17 9:29 ` [PATCH 0/2] Sparse index: fetch, pull, ls-files Junio C Hamano
2021-11-17 15:28 ` Derrick Stolee
2021-11-18 22:13 ` Junio C Hamano
2021-11-23 1:57 ` Ævar Arnfjörð Bjarmason
2021-12-08 19:39 ` [PATCH v2 0/5] " Derrick Stolee via GitGitGadget
2021-12-08 19:39 ` [PATCH v2 1/5] fetch/pull: use the sparse index Derrick Stolee via GitGitGadget
2021-12-08 19:39 ` [PATCH v2 2/5] ls-files: add --sparse option Derrick Stolee via GitGitGadget
2021-12-09 5:08 ` Elijah Newren [this message]
2021-12-10 13:51 ` Derrick Stolee
2021-12-08 19:39 ` [PATCH v2 3/5] t1092: replace 'read-cache --table' with 'ls-files --sparse' Derrick Stolee via GitGitGadget
2021-12-09 5:19 ` Elijah Newren
2021-12-08 19:39 ` [PATCH v2 4/5] t1091/t3705: remove 'test-tool read-cache --table' Derrick Stolee via GitGitGadget
2021-12-09 5:20 ` Elijah Newren
2021-12-08 19:39 ` [PATCH v2 5/5] test-read-cache: remove --table, --expand options Derrick Stolee via GitGitGadget
2021-12-09 5:23 ` [PATCH v2 0/5] Sparse index: fetch, pull, ls-files Elijah Newren
2021-12-10 15:13 ` [PATCH v3 " Derrick Stolee via GitGitGadget
2021-12-10 15:13 ` [PATCH v3 1/5] fetch/pull: use the sparse index Derrick Stolee via GitGitGadget
2021-12-10 15:13 ` [PATCH v3 2/5] ls-files: add --sparse option Derrick Stolee via GitGitGadget
2021-12-10 15:13 ` [PATCH v3 3/5] t1092: replace 'read-cache --table' with 'ls-files --sparse' Derrick Stolee via GitGitGadget
2021-12-10 15:13 ` [PATCH v3 4/5] t1091/t3705: remove 'test-tool read-cache --table' Derrick Stolee via GitGitGadget
2021-12-10 15:13 ` [PATCH v3 5/5] test-read-cache: remove --table, --expand options Derrick Stolee via GitGitGadget
2021-12-10 16:16 ` [PATCH v3 0/5] Sparse index: fetch, pull, ls-files Ævar Arnfjörð Bjarmason
2021-12-10 18:45 ` Elijah Newren
2021-12-11 2:24 ` Ævar Arnfjörð Bjarmason
2021-12-11 4:45 ` Elijah Newren
2021-12-10 18:53 ` Elijah Newren
2021-12-22 14:20 ` [PATCH v4 " Derrick Stolee via GitGitGadget
2021-12-22 14:20 ` [PATCH v4 1/5] fetch/pull: use the sparse index Derrick Stolee via GitGitGadget
2021-12-22 14:20 ` [PATCH v4 2/5] ls-files: add --sparse option Derrick Stolee via GitGitGadget
2021-12-22 14:20 ` [PATCH v4 3/5] t1092: replace 'read-cache --table' with 'ls-files --sparse' Derrick Stolee via GitGitGadget
2021-12-22 14:20 ` [PATCH v4 4/5] t1091/t3705: remove 'test-tool read-cache --table' Derrick Stolee via GitGitGadget
2021-12-22 14:20 ` [PATCH v4 5/5] test-read-cache: remove --table, --expand options Derrick Stolee via GitGitGadget
2021-12-22 19:17 ` [PATCH v4 0/5] Sparse index: fetch, pull, ls-files Elijah Newren
2021-12-22 23:56 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABPp-BF5kw69ioheMvRPoVso8FJf45Fdm_Cf-P7PfqWsqHz9Gw@mail.gmail.com \
--to=newren@gmail.com \
--cc=avarab@gmail.com \
--cc=derrickstolee@github.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=stolee@gmail.com \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).