git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "William Sprent via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Victoria Dye <vdye@github.com>, Elijah Newren <newren@gmail.com>,
	William Sprent <williams@unity3d.com>
Subject: [PATCH v2 0/2] builtin/sparse-checkout: add check-rules command
Date: Mon, 27 Mar 2023 07:55:01 +0000	[thread overview]
Message-ID: <pull.1488.v2.git.1679903703.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1488.git.1678283349.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

  parent reply	other threads:[~2023-03-27  7:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08 13:49 [PATCH 0/2] builtin/sparse-checkout: add check-rules command William Sprent via GitGitGadget
2023-03-08 13:49 ` [PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag William Sprent via GitGitGadget
2023-03-08 18:14   ` Junio C Hamano
2023-03-08 19:27     ` Junio C Hamano
2023-03-09 15:31       ` Elijah Newren
2023-03-09 22:19         ` Junio C Hamano
2023-03-08 13:49 ` [PATCH 2/2] builtin/sparse-checkout: add check-rules command William Sprent via GitGitGadget
2023-03-19  4:26   ` Elijah Newren
2023-03-20 15:49     ` William Sprent
2023-03-19  4:28 ` [PATCH 0/2] " Elijah Newren
2023-03-27  7:55 ` William Sprent via GitGitGadget [this message]
2023-03-27  7:55   ` [PATCH v2 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag William Sprent via GitGitGadget
2023-03-27 17:51     ` Junio C Hamano
2023-04-07 16:16       ` SZEDER Gábor
2023-04-07 16:38         ` Junio C Hamano
2023-03-27  7:55   ` [PATCH v2 2/2] builtin/sparse-checkout: add check-rules command William Sprent via GitGitGadget
2023-04-01 18:49   ` [PATCH v2 0/2] " Elijah Newren
2023-04-03 17:07     ` 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1488.v2.git.1679903703.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=vdye@github.com \
    --cc=williams@unity3d.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).