On Mon, Oct 21, 2019 at 01:56:14PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> The 'git sparse-checkout set' subcommand takes a list of patterns
> and places them in the sparse-checkout file. Then, it updates the
> working directory to match those patterns. For a large list of
> patterns, the command-line call can get very cumbersome.
>
> Add a '--stdin' option to instead read patterns over standard in.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> builtin/sparse-checkout.c | 35 ++++++++++++++++++++++++++++--
> t/t1091-sparse-checkout-builtin.sh | 20 +++++++++++++++++
No documentation update.
> 2 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 834ee421f0..f2e2bd772d 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -154,6 +154,15 @@ static int write_patterns_and_update(struct pattern_list *pl)
> return update_working_directory();
> }
>
> +static char const * const builtin_sparse_checkout_set_usage[] = {
> + N_("git sparse-checkout set [--stdin|<patterns>]"),
In the usage string [...] denotes optional command line parameters.
However, they are not optional, but either '--stdin' or at least one
pattern is required:
$ git sparse-checkout set
error: Sparse checkout leaves no entry on working directory
error: Sparse checkout leaves no entry on working directory
$ echo $?
255
So this should be (--stdin | <patterns>).
> + NULL
> +};
> +
> +static struct sparse_checkout_set_opts {
> + int use_stdin;
> +} set_opts;
> +
> static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> {
> static const char *empty_base = "";
> @@ -161,10 +170,32 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> struct pattern_list pl;
> int result;
> int set_config = 0;
> +
> + static struct option builtin_sparse_checkout_set_options[] = {
> + OPT_BOOL(0, "stdin", &set_opts.use_stdin,
> + N_("read patterns from standard in")),
> + OPT_END(),
> + };
> +
> memset(&pl, 0, sizeof(pl));
>
> - for (i = 1; i < argc; i++)
> - add_pattern(argv[i], empty_base, 0, &pl, 0);
> + argc = parse_options(argc, argv, prefix,
> + builtin_sparse_checkout_set_options,
> + builtin_sparse_checkout_set_usage,
> + PARSE_OPT_KEEP_UNKNOWN);
> +
> + if (set_opts.use_stdin) {
> + struct strbuf line = STRBUF_INIT;
> +
> + while (!strbuf_getline(&line, stdin)) {
This reads patterns separated by a newline character.
What if someone is doomed with pathnames containing newline
characters, should we provide a '-z' option for \0-separated patterns?
$ touch foo bar $'foo\nbar'
$ git add .
$ git commit -m 'filename with newline'
[master (root-commit) 5cd7369] filename with newline
3 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 bar
create mode 100644 foo
create mode 100644 "foo\nbar"
$ git sparse-checkout set foo
$ ls
foo
$ git sparse-checkout set 'foo*'
$ ls
foo foo?bar
$ git sparse-checkout set $'foo\nbar'
$ ls
foo?bar
# Looks good so far, but...
$ cat .git/info/sparse-checkout
foo
bar
$ git read-tree -um HEAD
$ ls
bar foo
# Not so good after all.
# Let's try to hand-edit the sparse-checkout file.
$ echo $'"foo\\nbar"' >.git/info/sparse-checkout
$ git read-tree -um HEAD
error: Sparse checkout leaves no entry on working directory
$ echo $'foo\\nbar'
>.git/info/sparse-checkout
$ git read-tree -um HEAD
error: Sparse checkout leaves no entry on working directory
$ echo $'foo\\\nbar'
>.git/info/sparse-checkout
$ git read-tree -um HEAD
$ ls
bar
# OK, I give up :)
So, it seems that the sparse-checkout file format doesn't support
paths/patterns containing a newline character (or if it does, I
couldn't figure out how), and thus there is no use for a '-z' option.
However, as shown above a newline in a pattern given as parameter
eventually leads to undesired behavior, so I think 'git
sparse-checkout set $'foo\nbar' should error out upfront.
> + size_t len;
> + char *buf = strbuf_detach(&line, &len);
Nit: this 'len' variable is unnecessary, as it's only used as an out
variable of this strbuf_detach() call, but strbuf_detach() accepts a
NULL as its second parameter.
> + add_pattern(buf, empty_base, 0, &pl, 0);
> + }
> + } else {
> + for (i = 0; i < argc; i++)
> + add_pattern(argv[i], empty_base, 0, &pl, 0);
> + }
According to the usage string this subcommand needs either '--stdin'
or a set of patterns, but not both, which is in line with my
interpretation of the commit message. I think this makes sense, and
it's consistent with the '--stdin' option of other git commands.
However, the above option parsing and if-else conditions allow both
'--stdin' and patterns, silently ignoring the patterns, without
erroring out:
$ echo README | git sparse-checkout set --stdin Makefile
$ echo $?
0
$ find . -name README |wc -l
51
$ find . -name Makefile |wc -l
0
> if (!core_apply_sparse_checkout) {
> sc_set_config(MODE_ALL_PATTERNS);
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index bf2dc55bb1..a9ff5eb9ec 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -128,4 +128,24 @@ test_expect_success 'set sparse-checkout using builtin' '
> test_cmp expect dir
> '
>
> +test_expect_success 'set sparse-checkout using --stdin' '
> + cat >expect <<-EOF &&
> + /*
> + !/*/
> + /folder1/
> + /folder2/
> + EOF
> + git -C repo sparse-checkout set --stdin <expect &&
> + git -C repo sparse-checkout list >actual &&
> + test_cmp expect actual &&
> + test_cmp expect repo/.git/info/sparse-checkout &&
> + ls repo >dir &&
> + cat >expect <<-EOF &&
> + a
> + folder1
> + folder2
> + EOF
> + test_cmp expect dir
> +'
> +
> test_done
> --
> gitgitgadget
>
