[PATCH v2 0/2] builtin/sparse-checkout: add check-rules command

["William Sprent via GitGitGadget" <gitgitgadget@gmail.com>]

Hi

This v2 addresses comments from Elijah's review comments.

There's one thing worth highlighting. Elijah pointed out that the the
"check-rules cone mode is default" test would be stronger if the test itself
started with a 'git sparse-checkout set --no-cone' to explicitly test that
the default interpretation of the rules passed with the '--rules-file'
option is cone mode even though the current checkout is non-cone. I
implemented this and it exposed that the option did not actually behave that
way, and that the test only verified the default behaviour of a bare
repository.

I've modified the logic of the '--rules-file' option such that it defaults
to cone mode unless combined with '--no-cone', and I've added a line to the
documentation to make this more explicit.

The alternative would be to have '--rules-file' assume non cone mode when in
a non cone mode checkout, but I think this depends a bit on "how deprecated"
non-cone mode is vs. how important it is to have the option behave
consistently with 'sparse-checkout set' (which respects the current
checkout).

Changes since v1:

 * Explicitly state in documentation that '--rules-file' expects newline
   separated rules.
 * Explicitly state in documentation that '-z' does not affect the
   '--rules-file' input.
 * Fixup typo where 'When called with the --rules-file <file> flag' was
   missing "flag".
 * Fixup behaviour in 'check-rules --rules-file', such that it defaults to
   accepting cone mode patterns when in a non cone checkout.
 * Remember to release string buffers in 'check_rules()'.
 * Explicitly state in documentation that '--rules-file' defaults to cone
   mode unless combined with '--no-cone'.
 * Better test that the default of '--rules-file' is to expect '--cone-mode'
   by running 'check-rules' in a non-cone mode checkout.

William Sprent (2):
  builtin/sparse-checkout: remove NEED_WORK_TREE flag
  builtin/sparse-checkout: add check-rules command

 Documentation/git-sparse-checkout.txt |  25 +++-
 builtin/sparse-checkout.c             | 137 ++++++++++++++++++---
 git.c                                 |   2 +-
 t/t1091-sparse-checkout-builtin.sh    | 167 +++++++++++++++++++++++++-
 4 files changed, 307 insertions(+), 24 deletions(-)


base-commit: d15644fe0226af7ffc874572d968598564a230dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1488%2Fwilliams-unity%2Fsparse-doodle-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1488/williams-unity/sparse-doodle-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1488

Range-diff vs v1:

 1:  4b231e9beb4 = 1:  4b231e9beb4 builtin/sparse-checkout: remove NEED_WORK_TREE flag
 2:  21c8375efff ! 2:  ef6e5b4d786 builtin/sparse-checkout: add check-rules command
     @@ Documentation/git-sparse-checkout.txt: paths to pass to a subsequent 'set' or 'a
      +By default `check-rules` reads a list of paths from stdin and outputs only
      +the ones that match the current sparsity rules. The input is expected to consist
      +of one path per line, matching the output of `git ls-tree --name-only` including
     -+that pathnames that begin with a double quote (") are interpreted C-style quoted
     -+strings.
     ++that pathnames that begin with a double quote (") are interpreted as C-style
     ++quoted strings.
      ++
     -+When called with the `--rules-file <file>` the input files are matched against
     -+the sparse checkout rules found in `<file>` instead of the current ones. The
     -+rules in the files are expected to be in the same form as accepted by `git
     -+sparse-checkout set --stdin`.
     ++When called with the `--rules-file <file>` flag the input files are matched
     ++against the sparse checkout rules found in `<file>` instead of the current ones.
     ++The rules in the files are expected to be in the same form as accepted by `git
     ++sparse-checkout set --stdin` (in particular, they must be newline-delimited).
      ++
     -+The `--rules-file` flag can be combined with the `--[no]-cone` with the same
     -+effect as for the `set` command with the `--stdin` flag.
     ++By default, the rules passed to the `--rules-file` option are interpreted as
     ++cone mode directories. To pass non-cone mode patterns with `--rules-file`,
     ++combine the option with the `--no-cone` option.
      ++
     -+When called with the `-z` flag the input format and output format is \0
     -+terminated and not quoted.
     ++When called with the `-z` flag, the format of the paths input on stdin as well
     ++as the output paths are \0 terminated and not quoted. Note that this does not
     ++apply to the format of the rules passed with the `--rules-file` option.
      +
      +
       EXAMPLES
     @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch
      +		if (path_in_sparse_checkout(path, the_repository->index))
      +			write_name_quoted(path, stdout, line_terminator);
      +	}
     ++	strbuf_release(&line);
     ++	strbuf_release(&unquoted);
      +
      +	return 0;
      +}
     @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch
      +			     builtin_sparse_checkout_check_rules_usage,
      +			     PARSE_OPT_KEEP_UNKNOWN_OPT);
      +
     ++	if (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)
     ++		check_rules_opts.cone_mode = 1;
     ++
      +	update_cone_mode(&check_rules_opts.cone_mode);
      +	pl.use_cone_patterns = core_sparse_checkout_cone;
      +	if (check_rules_opts.rules_file) {
     @@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'disable fails outside w
      +	folder1/file
      +	EOF
      +
     -+	git -C bare sparse-checkout check-rules \
     ++	git -C repo sparse-checkout set --no-cone &&
     ++	git -C repo sparse-checkout check-rules \
      +		--rules-file ../rules >actual <all-files &&
      +
     -+	test_cmp expect actual
     ++	git -C bare sparse-checkout check-rules \
     ++		--rules-file ../rules >actual-bare <all-files &&
     ++
     ++	test_cmp expect actual &&
     ++	test_cmp expect actual-bare
      +'
      +
      +test_expect_success 'check-rules quoting' '

-- 
gitgitgadget