Skip to content

sparse-checkout: list directories in cone mode #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

sparse-checkout: list directories in cone mode #500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d4295b
sparse-checkout: list directories in cone mode
derrickstolee Dec 26, 2019
74bbd0f
sparse-checkout: document interactions with submodules
derrickstolee Dec 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Documentation/git-sparse-checkout.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ THE FUTURE.
COMMANDS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> From: Derrick Stolee <[email protected]>

> Subject: Re: [PATCH 1/1] sparse-checkout: list folders in cone mode
s/folder/directory/ everywhere as the rest of Git.

> When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
> command taks a list of folders as input, then creates an ordered

"takes"

> list of sparse-checkout patterns such that those folders are
> recursively included and all sibling blobs along the parent folders

In this sentence, what does a "blob" really mean?  Do you mean a
filesystem entity, that is not a folder, that is immediately
contained in the "parent folder" (in other words, regular files
and symbolic links)?

How would this interact with a submodule by the way?

> are also included. Listing the patterns is less user-friendly than the
> folders themselves.
>
> In cone mode, and as long as the patterns match the expected cone-mode
> pattern types, change the output of 'git sparse-checkout list' to only
> show the folders that created the patterns.
> ...
> +In the cone mode case, the `git sparse-checkout list` subcommand will list the
> +folders that define the recursive patterns. For the example sparse-checkout file
> +above, the output is as follows:
> +
> +--------------------------
> +$ git sparse-checkout list
> +A/B/C
> +--------------------------
> +

Sounds like a worthwhile usability improvement.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 12/26/2019 4:17 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[email protected]> writes:
> 
>> From: Derrick Stolee <[email protected]>
> 
>> Subject: Re: [PATCH 1/1] sparse-checkout: list folders in cone mode
> s/folder/directory/ everywhere as the rest of Git.
> 
>> When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
>> command taks a list of folders as input, then creates an ordered
> 
> "takes"

Good catch.

>> list of sparse-checkout patterns such that those folders are
>> recursively included and all sibling blobs along the parent folders
> 
> In this sentence, what does a "blob" really mean?  Do you mean a
> filesystem entity, that is not a folder, that is immediately
> contained in the "parent folder" (in other words, regular files
> and symbolic links)?

You're right, I'm using strange wording here. How about "sibling
entries"?

> How would this interact with a submodule by the way?

I just checked with the Git repo by running:

	git submodule init
	git submodule update
	git sparse-checkout init --cone

The working directory then contains all blobs at root AND the
sha1collisiondetection submodule. Interesting that the sparse-
checkout feature ignores submodules. That seems like the best
approach since the user can already enlist in a subset of the
submodules.

>> are also included. Listing the patterns is less user-friendly than the
>> folders themselves.
>>
>> In cone mode, and as long as the patterns match the expected cone-mode
>> pattern types, change the output of 'git sparse-checkout list' to only
>> show the folders that created the patterns.
>> ...
>> +In the cone mode case, the `git sparse-checkout list` subcommand will list the
>> +folders that define the recursive patterns. For the example sparse-checkout file
>> +above, the output is as follows:
>> +
>> +--------------------------
>> +$ git sparse-checkout list
>> +A/B/C
>> +--------------------------
>> +
> 
> Sounds like a worthwhile usability improvement.
 
Thanks,
-Stolee

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Fri, Dec 27, 2019 at 1:48 PM Derrick Stolee via GitGitGadget
<[email protected]> wrote:
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> @@ -340,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
> +test_expect_success 'interaction with submodules' '
> +       ...
> +       cat >expect <<-EOF &&
> +               a
> +               folder1
> +               modules
> +       EOF

You would normally use \-EOF rather than -EOF to make it clear that no
interpolation is needed/expected within the here-doc body. However,
this script is already full of -EOF when \-EOF ought to be used, so
being consistent with existing tests may override an objection.

Likewise, please note for future reference that the usual way
here-docs are formatted in Git test scripts is to indent the body of
the here-doc to the same level as the command which opens it. That is:

    cat >expect <<\-EOF &&
    a
    folder1
    modules
    EOF

But, again, this script is already full of these malformatted
here-docs, so maintaining consistency with the existing test in the
script is probably okay.

(Of course, a preparatory patch fixing these here-doc issues would be
welcome, but might also be outside the scope of this submission. And,
fixing these very minor issues is quite low-priority, so I wouldn't
expect or demand it.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 12/27/2019 3:20 PM, Eric Sunshine wrote:
> On Fri, Dec 27, 2019 at 1:48 PM Derrick Stolee via GitGitGadget
> <[email protected]> wrote:
>> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
>> @@ -340,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
>> +test_expect_success 'interaction with submodules' '
>> +       ...
>> +       cat >expect <<-EOF &&
>> +               a
>> +               folder1
>> +               modules
>> +       EOF
> 
> You would normally use \-EOF rather than -EOF to make it clear that no
> interpolation is needed/expected within the here-doc body. However,
> this script is already full of -EOF when \-EOF ought to be used, so
> being consistent with existing tests may override an objection.
> 
> Likewise, please note for future reference that the usual way
> here-docs are formatted in Git test scripts is to indent the body of
> the here-doc to the same level as the command which opens it. That is:

Thanks for pointing out the difference, except...
 
>     cat >expect <<\-EOF &&

This should be <<-\EOF

>     a
>     folder1
>     modules
>     EOF
> 
> But, again, this script is already full of these malformatted
> here-docs, so maintaining consistency with the existing test in the
> script is probably okay.

Hm. Having these lines have the same tabbing hurts my eyes (it is
harder to see where the contents end, much like if we didn't tab
inside a subshell or an if block).

This is also a place where we are inconsistent, and it's not just
my fault for writing the test script in my own style. Here are a
few scripts that tab the same way as here:

	t0008-ignore.sh
	t4124-apply-ws-rule.sh
	t9400-diff-highlight.sh

These are definitely the minority. I just mention them so anyone
who does a cleanup of this whitespace inconsistency takes the time
to look for all examples.

For now, I'll fix the here-doc interpolation issue for this test,
but keep the whitespace matching the rest of the test script. I'll
add this concern to my next series.

Thanks,
-Stolee

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Elijah Newren wrote (reply to this):

On Fri, Dec 27, 2019 at 10:47 AM Derrick Stolee via GitGitGadget
<[email protected]> wrote:
>
> From: Derrick Stolee <[email protected]>
>
> When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
> command takes a list of folders as input, then creates an ordered
s/folders/directories/

> list of sparse-checkout patterns such that those folders are
s/folders/directories/

> recursively included and all sibling entries along the parent folders
again here.

> are also included. Listing the patterns is less user-friendly than the
> folders themselves.
and here.

>
> In cone mode, and as long as the patterns match the expected cone-mode
> pattern types, change the output of 'git sparse-checkout list' to only
> show the folders that created the patterns.
and here.

>
> With this change, the following piped commands would not change the
> working directory:
>
>         git sparse-checkout list | git sparse-checkout set --stdin
>
> The only time this would not work is if core.sparseCheckoutCone is
> true, but the sparse-checkout file contains patterns that do not
> match the expected pattern types for cone mode.
>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>  Documentation/git-sparse-checkout.txt | 11 ++++++++++-
>  builtin/sparse-checkout.c             | 21 +++++++++++++++++++++
>  t/t1091-sparse-checkout-builtin.sh    | 11 +++++++++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index 9c3c66cc37..dcca9ee826 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -28,7 +28,7 @@ THE FUTURE.
>  COMMANDS
>  --------
>  'list'::
> -       Provide a list of the contents in the sparse-checkout file.
> +       Describe the patterns in the sparse-checkout file.
>
>  'init'::
>         Enable the `core.sparseCheckout` setting. If the
> @@ -150,6 +150,15 @@ expecting patterns of these types. Git will warn if the patterns do not match.
>  If the patterns do match the expected format, then Git will use faster hash-
>  based algorithms to compute inclusion in the sparse-checkout.
>
> +In the cone mode case, the `git sparse-checkout list` subcommand will list the
> +folders that define the recursive patterns. For the example sparse-checkout file
and here.

> +above, the output is as follows:
> +
> +--------------------------
> +$ git sparse-checkout list
> +A/B/C
> +--------------------------
> +
>  If `core.ignoreCase=true`, then the pattern-matching algorithm will use a
>  case-insensitive check. This corrects for case mismatched filenames in the
>  'git sparse-checkout set' command to reflect the expected cone in the working
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 5d62f7a66d..b3bed891cb 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -53,6 +53,8 @@ static int sparse_checkout_list(int argc, const char **argv)
>
>         memset(&pl, 0, sizeof(pl));
>
> +       pl.use_cone_patterns = core_sparse_checkout_cone;
> +
>         sparse_filename = get_sparse_checkout_filename();
>         res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
>         free(sparse_filename);
> @@ -62,6 +64,25 @@ static int sparse_checkout_list(int argc, const char **argv)
>                 return 0;
>         }
>
> +       if (pl.use_cone_patterns) {
> +               int i;
> +               struct pattern_entry *pe;
> +               struct hashmap_iter iter;
> +               struct string_list sl = STRING_LIST_INIT_DUP;
> +
> +               hashmap_for_each_entry(&pl.recursive_hashmap, &iter, pe, ent) {
> +                       /* pe->pattern starts with "/", skip it */
> +                       string_list_insert(&sl, pe->pattern + 1);
> +               }
> +
> +               string_list_sort(&sl);
> +
> +               for (i = 0; i < sl.nr; i++)
> +                       printf("%s\n", sl.items[i].string);
> +
> +               return 0;
> +       }
> +
>         write_patterns_to_file(stdout, &pl);
>         clear_pattern_list(&pl);
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 6f7e2d0c9e..37f6d8fa90 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -246,6 +246,17 @@ test_expect_success 'cone mode: init and set' '
>         test_cmp expect dir
>  '
>
> +test_expect_success 'cone mode: list' '
> +       cat >expect <<-EOF &&
> +               folder1
> +               folder2
> +       EOF
> +       git -C repo sparse-checkout set --stdin <expect &&
> +       git -C repo sparse-checkout list >actual 2>err &&
> +       test_must_be_empty err &&
> +       test_cmp expect actual
> +'
> +
>  test_expect_success 'cone mode: set with nested folders' '
There's another one here but that's just the context area, so it'd be
a separate patch in a separate cleanup series...

>         git -C repo sparse-checkout set deep deep/deeper1/deepest 2>err &&
>         test_line_count = 0 err &&
> --

The rest looks good

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Elijah Newren wrote (reply to this):

On Fri, Dec 27, 2019 at 10:47 AM Derrick Stolee via GitGitGadget
<[email protected]> wrote:
>
> From: Derrick Stolee <[email protected]>
>
> Junio asked what the behavior is between the sparse-checkout feature
> and the submodule feature.

Does this first sentence help future readers?  It is what spurred you
to write the documentation, but it seems like something that could
just be left out.

> The sparse-checkout builtin has not changed
> the way these features interact, but we may as well document it in
> the builtin docs.
>
> Using 'git submodule (init|deinit)' a user can select a subset of
> submodules to populate. This behaves very similar to the sparse-checkout
> feature, but those directories contain their own .git directory
> including an object database and ref space. To have the sparse-checkout
> file also determine if those files should exist would easily cause
> problems. Therefore, keeping these features independent in this way
> is the best way forward.
>
> Also create a test that demonstrates this behavior to make sure
> it doesn't change as the sparse-checkout feature evolves.
>
> Reported-by: Junio C Hamano <[email protected]>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>  Documentation/git-sparse-checkout.txt | 10 ++++++++++
>  t/t1091-sparse-checkout-builtin.sh    | 28 +++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
>
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index dcca9ee826..2b7aaa0310 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -164,6 +164,16 @@ case-insensitive check. This corrects for case mismatched filenames in the
>  'git sparse-checkout set' command to reflect the expected cone in the working
>  directory.
>
> +
> +SUBMODULES
> +----------
> +
> +If your repository contains one or more submodules, then those submodules will
> +appear based on which you initialized with the `git submodule` command. If
> +your sparse-checkout patterns exclude an initialized submodule, then that
> +submodule will still appear in your working directory.
> +
> +
>  SEE ALSO
>  --------
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 37f6d8fa90..5572beeeca 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -340,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
>         test_cmp expect dir
>  '
>
> +test_expect_success 'interaction with submodules' '
> +       git clone repo super &&
> +       (
> +               cd super &&
> +               mkdir modules &&
> +               git submodule add ../repo modules/child &&
> +               git add . &&
> +               git commit -m "add submodule" &&
> +               git sparse-checkout init --cone &&
> +               git sparse-checkout set folder1
> +       ) &&
> +       list_files super >dir &&
> +       cat >expect <<-EOF &&
> +               a
> +               folder1
> +               modules
> +       EOF
> +       test_cmp expect dir &&
> +       list_files super/modules/child >dir &&
> +       cat >expect <<-EOF &&
> +               a
> +               deep
> +               folder1
> +               folder2
> +       EOF
> +       test_cmp expect dir
> +'
> +
>  test_done
> --

I read over the rest, and not being a submodule user I'm not sure what
I'd expect.  But it certainly seems reasonable to document how these
features interact and that you haven't made any modifications in the
area.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Elijah Newren <[email protected]> writes:

> On Fri, Dec 27, 2019 at 10:47 AM Derrick Stolee via GitGitGadget
> <[email protected]> wrote:
>>
>> From: Derrick Stolee <[email protected]>
>>
>> Junio asked what the behavior is between the sparse-checkout feature
>> and the submodule feature.
>
> Does this first sentence help future readers?  It is what spurred you
> to write the documentation, but it seems like something that could
> just be left out.

I tend to agree.  It can safely go below the three-dash lines.

--------
'list'::
Provide a list of the contents in the sparse-checkout file.
Describe the patterns in the sparse-checkout file.

'init'::
Enable the `core.sparseCheckout` setting. If the
Expand Down Expand Up @@ -150,11 +150,30 @@ expecting patterns of these types. Git will warn if the patterns do not match.
If the patterns do match the expected format, then Git will use faster hash-
based algorithms to compute inclusion in the sparse-checkout.

In the cone mode case, the `git sparse-checkout list` subcommand will list the
directories that define the recursive patterns. For the example sparse-checkout
file above, the output is as follows:

--------------------------
$ git sparse-checkout list
A/B/C
--------------------------

If `core.ignoreCase=true`, then the pattern-matching algorithm will use a
case-insensitive check. This corrects for case mismatched filenames in the
'git sparse-checkout set' command to reflect the expected cone in the working
directory.


SUBMODULES
----------

If your repository contains one or more submodules, then those submodules will
appear based on which you initialized with the `git submodule` command. If
your sparse-checkout patterns exclude an initialized submodule, then that
submodule will still appear in your working directory.


SEE ALSO
--------

Expand Down
21 changes: 21 additions & 0 deletions builtin/sparse-checkout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ static int sparse_checkout_list(int argc, const char **argv)

memset(&pl, 0, sizeof(pl));

pl.use_cone_patterns = core_sparse_checkout_cone;

sparse_filename = get_sparse_checkout_filename();
res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
free(sparse_filename);
Expand All @@ -62,6 +64,25 @@ static int sparse_checkout_list(int argc, const char **argv)
return 0;
}

if (pl.use_cone_patterns) {
int i;
struct pattern_entry *pe;
struct hashmap_iter iter;
struct string_list sl = STRING_LIST_INIT_DUP;

hashmap_for_each_entry(&pl.recursive_hashmap, &iter, pe, ent) {
/* pe->pattern starts with "/", skip it */
string_list_insert(&sl, pe->pattern + 1);
}

string_list_sort(&sl);

for (i = 0; i < sl.nr; i++)
printf("%s\n", sl.items[i].string);

return 0;
}

write_patterns_to_file(stdout, &pl);
clear_pattern_list(&pl);

Expand Down
39 changes: 39 additions & 0 deletions t/t1091-sparse-checkout-builtin.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,17 @@ test_expect_success 'cone mode: init and set' '
test_cmp expect dir
'

test_expect_success 'cone mode: list' '
cat >expect <<-EOF &&
folder1
folder2
EOF
git -C repo sparse-checkout set --stdin <expect &&
git -C repo sparse-checkout list >actual 2>err &&
test_must_be_empty err &&
test_cmp expect actual
'

test_expect_success 'cone mode: set with nested folders' '
git -C repo sparse-checkout set deep deep/deeper1/deepest 2>err &&
test_line_count = 0 err &&
Expand Down Expand Up @@ -329,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
test_cmp expect dir
'

test_expect_success 'interaction with submodules' '
git clone repo super &&
(
cd super &&
mkdir modules &&
git submodule add ../repo modules/child &&
git add . &&
git commit -m "add submodule" &&
git sparse-checkout init --cone &&
git sparse-checkout set folder1
) &&
list_files super >dir &&
cat >expect <<-\EOF &&
a
folder1
modules
EOF
test_cmp expect dir &&
list_files super/modules/child >dir &&
cat >expect <<-\EOF &&
a
deep
folder1
folder2
EOF
test_cmp expect dir
'

test_done