On 11/21/2019 6:21 AM, SZEDER Gábor wrote:

On Mon, Oct 21, 2019 at 01:56:14PM +0000, Derrick Stolee via GitGitGadget wrote:

quoted

From: Derrick Stolee <redacted>

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 <redacted>
---
 builtin/sparse-checkout.c          | 35 ++++++++++++++++++++++++++++--
 t/t1091-sparse-checkout-builtin.sh | 20 +++++++++++++++++

No documentation update.

quoted

 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>).

quoted

+	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.

I will add this to the list of things to do as a follow-up. There are
lots of ways users can abuse the command-line right now, especially in
cone mode.

The goal right now is to get the typical usage figured out, then we can
look for atypical cases (such as newlines in filenames, which...who even
does that?).

quoted

+			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.

quoted

+			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

I'll add this to the list, too.

Thanks,
-Stolee