{"thread":{"id":"59359","subject":"[PATCH 0/2] builtin/sparse-checkout: add check-rules command","startedAt":"2023-03-08T13:49:29Z","lastAt":"2023-04-07T16:38:14Z","messageCount":18,"participants":["William Sprent via GitGitGadget","Junio C Hamano","Elijah Newren","William Sprent","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"473193","messageId":"pull.1488.git.1678283349.gitgitgadget@gmail.com","threadId":"59359","inReplyTo":null,"subject":"[PATCH 0/2] builtin/sparse-checkout: add check-rules command","fromName":"William Sprent via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-08T13:49:07Z","receivedAt":"2023-03-08T13:49:29Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"Hi\n\nBack in January I submitted a patch which suggested to teach 'ls-tree' to be\nable to filter its output based on sparse checkout specifications [1]. My\nmain motivation for doing so was (is) to enable building more tooling on top\nof sparse checkouts, which is currently hampered a bit by the fact that git\ndoesn't expose the pattern matching rules for the sparse checkouts.\n\nI think the main point from that thread was that the 'ls-tree' change was\nconceptually a larger change that I had initially thought it was. It was\nsuggested that perhaps it would be more straight-forward to initially add a\ncommand in the vein of 'git-check-ignore' before teaching all the other\ncommands about sparse checkout specifics, and I think that makes sense. So I\nam proposing here a new 'check-rules' sub-command to 'sparse-checkout'. This\nexposes the sparse checkout pattern matching logic while still keeping the\npattern specification local to the sparse-checkout command.\n\nSince the intention is that this new behavior would not need a work tree as\nit allows the user to supply a set of rules to verify the paths against, the\nchange that introduces the sub-command is preceded by one that removes the\n'NEEDS_WORK_TREE' flag for 'sparse-checkout' and replaces it with calls to\n'setup_work_tree()' to keep current behavior.\n\n1:\nhttps://public-inbox.org/git/pull.1459.git.1673456518993.gitgitgadget@gmail.com/\n\nWilliam Sprent (2):\n builtin/sparse-checkout: remove NEED_WORK_TREE flag\n builtin/sparse-checkout: add check-rules command\n\n Documentation/git-sparse-checkout.txt | 23 +++-\n builtin/sparse-checkout.c | 132 +++++++++++++++++----\n git.c | 2 +-\n t/t1091-sparse-checkout-builtin.sh | 162 +++++++++++++++++++++++++-\n 4 files changed, 295 insertions(+), 24 deletions(-)\n\n\nbase-commit: d15644fe0226af7ffc874572d968598564a230dd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1488%2Fwilliams-unity%2Fsparse-doodle-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1488/williams-unity/sparse-doodle-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1488\n-- \ngitgitgadget\n"},{"id":"473194","messageId":"4b231e9beb43e4fac6457b9bf86e4c1db39c4238.1678283349.git.gitgitgadget@gmail.com","threadId":"59359","inReplyTo":"pull.1488.git.1678283349.gitgitgadget@gmail.com","subject":"[PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"William Sprent via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-08T13:49:08Z","receivedAt":"2023-03-08T13:49:31Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"From: William Sprent \n\nIn preparation for adding a sub-command to 'sparse-checkout' that can be\nrun in a bare repository, remove the 'NEED_WORK_TREE' flag from its\nentry in the 'commands' array of 'git.c'.\n\nTo avoid that this changes any behaviour, add calls to\n'setup_work_tree()' to all of the 'sparse-checkout' sub-commands and add\ntests that verify that 'sparse-checkout ' still fail with a clear\nerror message telling the user that the command needs a work tree.\n\nSigned-off-by: William Sprent \n---\n builtin/sparse-checkout.c | 6 ++++++\n git.c | 2 +-\n t/t1091-sparse-checkout-builtin.sh | 33 ++++++++++++++++++++++++++++++\n 3 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex c3738154918..5fdc3d9aab5 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -57,6 +57,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)\n \tchar *sparse_filename;\n \tint res;\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"this worktree is not sparse\"));\n \n@@ -448,6 +449,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \trepo_read_index(the_repository);\n \n \tinit_opts.cone_mode = -1;\n@@ -760,6 +762,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"no sparse-checkout to add to\"));\n \n@@ -806,6 +809,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \trepo_read_index(the_repository);\n \n \tset_opts.cone_mode = -1;\n@@ -855,6 +859,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"must be in a sparse-checkout to reapply sparsity patterns\"));\n \n@@ -898,6 +903,7 @@ static int sparse_checkout_disable(int argc, const char **argv,\n \t * forcibly return to a dense checkout regardless of initial state.\n \t */\n \n+\tsetup_work_tree();\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_sparse_checkout_disable_options,\n \t\t\t builtin_sparse_checkout_disable_usage, 0);\ndiff --git a/git.c b/git.c\nindex 6171fd6769d..5adc835cf10 100644\n--- a/git.c\n+++ b/git.c\n@@ -583,7 +583,7 @@ static struct cmd_struct commands[] = {\n \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t{ \"show-index\", cmd_show_index, RUN_SETUP_GENTLY },\n \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n-\t{ \"sparse-checkout\", cmd_sparse_checkout, RUN_SETUP | NEED_WORK_TREE },\n+\t{ \"sparse-checkout\", cmd_sparse_checkout, RUN_SETUP },\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stash\", cmd_stash, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 627267be153..7216267aec7 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -882,4 +882,37 @@ test_expect_success 'by default, non-cone mode will warn on individual files' '\n \tgrep \"pass a leading slash before paths.*if you want a single file\" warning\n '\n \n+test_expect_success 'setup bare repo' '\n+\tgit clone --bare \"file://$(pwd)/repo\" bare\n+'\n+test_expect_success 'list fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout list 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'add fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout add deeper 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'set fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout set deeper 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'init fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout init 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'reapply fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout reapply 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'disable fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout disable 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"473195","messageId":"21c8375efff5c20e299c52aa579e6b10dafcbc4f.1678283349.git.gitgitgadget@gmail.com","threadId":"59359","inReplyTo":"pull.1488.git.1678283349.gitgitgadget@gmail.com","subject":"[PATCH 2/2] builtin/sparse-checkout: add check-rules command","fromName":"William Sprent via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-08T13:49:09Z","receivedAt":"2023-03-08T13:49:32Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"From: William Sprent \n\nThere exists no direct way to interrogate git about which paths are\nmatched by a given set of sparsity rules. It is possible to get this\ninformation from git, but it includes checking out the commit that\ncontains the paths, applying the sparse checkout patterns and then using\nsomething like 'git ls-files -t' to check if the skip worktree bit is\nset. This works in some case, but there are cases where it is awkward or\ninfeasible to generate a checkout for this purpose.\n\nExposing the pattern matching of sparse checkout enables more tooling to\nbe built and avoids a situation where tools that want to reason about\nsparse checkouts start containing parallel implementation of the rules.\nTo accommodate this, add a 'check-rules' subcommand to the\n'sparse-checkout' builtin along the lines of the 'git check-ignore' and\n'git check-attr' commands. The new command accepts a list of paths on\nstdin and outputs just the ones the match the sparse checkout.\n\nTo allow for use in a bare repository and to allow for interrogating\nabout other patterns than the current ones, include a '--rules-file'\noption which allows the caller to explicitly pass sparse checkout rules\nin the format accepted by 'sparse-checkout set --stdin'.\n\nTo allow for reuse of the handling of input patterns for the\n'--rules-file' flag, modify 'add_patterns_from_input()' to be able to\nread from a 'FILE' instead of just stdin.\n\nTo allow for reuse of the logic which decides whether or not rules\nshould be interpreted as cone-mode patterns, split that part out of\n'update_modes()' such that can be called without modifying the config.\n\nAn alternative could have been to create a new 'check-sparsity' command.\nHowever, placing it under 'sparse-checkout' allows for a) more easily\nre-using the sparse checkout pattern matching and cone/non-code mode\nhandling, and b) keeps the documentation for the command next to the\nexperimental warning and the cone-mode discussion.\n\nSigned-off-by: William Sprent \n---\n Documentation/git-sparse-checkout.txt | 23 ++++-\n builtin/sparse-checkout.c | 126 +++++++++++++++++++++----\n t/t1091-sparse-checkout-builtin.sh | 129 +++++++++++++++++++++++++-\n 3 files changed, 255 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex 68392d2a56e..8fdde8f53be 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -9,7 +9,7 @@ git-sparse-checkout - Reduce your working tree to a subset of tracked files\n SYNOPSIS\n --------\n [verse]\n-'git sparse-checkout' (init | list | set | add | reapply | disable) []\n+'git sparse-checkout' (init | list | set | add | reapply | disable | check-rules) []\n \n \n DESCRIPTION\n@@ -135,6 +135,27 @@ paths to pass to a subsequent 'set' or 'add' command. However,\n the disable command, so the easy restore of calling a plain `init`\n decreased in utility.\n \n+'check-rules'::\n+\tCheck whether sparsity rules match one or more paths.\n++\n+By default `check-rules` reads a list of paths from stdin and outputs only\n+the ones that match the current sparsity rules. The input is expected to consist\n+of one path per line, matching the output of `git ls-tree --name-only` including\n+that pathnames that begin with a double quote (\") are interpreted C-style quoted\n+strings.\n++\n+When called with the `--rules-file ` the input files are matched against\n+the sparse checkout rules found in `` instead of the current ones. The\n+rules in the files are expected to be in the same form as accepted by `git\n+sparse-checkout set --stdin`.\n++\n+The `--rules-file` flag can be combined with the `--[no]-cone` with the same\n+effect as for the `set` command with the `--stdin` flag.\n++\n+When called with the `-z` flag the input format and output format is \\0\n+terminated and not quoted.\n+\n+\n EXAMPLES\n --------\n `git sparse-checkout set MY/DIR1 SUB/DIR2`::\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 5fdc3d9aab5..969ae14a415 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -20,7 +20,7 @@\n static const char *empty_base = \"\";\n \n static char const * const builtin_sparse_checkout_usage[] = {\n-\tN_(\"git sparse-checkout (init | list | set | add | reapply | disable) []\"),\n+\tN_(\"git sparse-checkout (init | list | set | add | reapply | disable | check-rules) []\"),\n \tNULL\n };\n \n@@ -384,13 +384,7 @@ static int set_config(enum sparse_checkout_mode mode)\n \treturn 0;\n }\n \n-static int update_modes(int *cone_mode, int *sparse_index)\n-{\n-\tint mode, record_mode;\n-\n-\t/* Determine if we need to record the mode; ensure sparse checkout on */\n-\trecord_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n-\n+static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {\n \t/* If not specified, use previous definition of cone mode */\n \tif (*cone_mode == -1 && core_apply_sparse_checkout)\n \t\t*cone_mode = core_sparse_checkout_cone;\n@@ -398,12 +392,21 @@ static int update_modes(int *cone_mode, int *sparse_index)\n \t/* Set cone/non-cone mode appropriately */\n \tcore_apply_sparse_checkout = 1;\n \tif (*cone_mode == 1 || *cone_mode == -1) {\n-\t\tmode = MODE_CONE_PATTERNS;\n \t\tcore_sparse_checkout_cone = 1;\n-\t} else {\n-\t\tmode = MODE_ALL_PATTERNS;\n-\t\tcore_sparse_checkout_cone = 0;\n+\t\treturn MODE_CONE_PATTERNS;\n \t}\n+\tcore_sparse_checkout_cone = 0;\n+\treturn MODE_ALL_PATTERNS;\n+}\n+\n+static int update_modes(int *cone_mode, int *sparse_index)\n+{\n+\tint mode, record_mode;\n+\n+\t/* Determine if we need to record the mode; ensure sparse checkout on */\n+\trecord_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n+\n+\tmode = update_cone_mode(cone_mode);\n \tif (record_mode && set_config(mode))\n \t\treturn 1;\n \n@@ -547,7 +550,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n \n static void add_patterns_from_input(struct pattern_list *pl,\n \t\t\t\t int argc, const char **argv,\n-\t\t\t\t int use_stdin)\n+\t\t\t\t FILE *file)\n {\n \tint i;\n \tif (core_sparse_checkout_cone) {\n@@ -557,9 +560,9 @@ static void add_patterns_from_input(struct pattern_list *pl,\n \t\thashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);\n \t\tpl->use_cone_patterns = 1;\n \n-\t\tif (use_stdin) {\n+\t\tif (file) {\n \t\t\tstruct strbuf unquoted = STRBUF_INIT;\n-\t\t\twhile (!strbuf_getline(&line, stdin)) {\n+\t\t\twhile (!strbuf_getline(&line, file)) {\n \t\t\t\tif (line.buf[0] == '\"') {\n \t\t\t\t\tstrbuf_reset(&unquoted);\n \t\t\t\t\tif (unquote_c_style(&unquoted, line.buf, NULL))\n@@ -581,10 +584,10 @@ static void add_patterns_from_input(struct pattern_list *pl,\n \t\t\t}\n \t\t}\n \t} else {\n-\t\tif (use_stdin) {\n+\t\tif (file) {\n \t\t\tstruct strbuf line = STRBUF_INIT;\n \n-\t\t\twhile (!strbuf_getline(&line, stdin)) {\n+\t\t\twhile (!strbuf_getline(&line, file)) {\n \t\t\t\tsize_t len;\n \t\t\t\tchar *buf = strbuf_detach(&line, &len);\n \t\t\t\tadd_pattern(buf, empty_base, 0, pl, 0);\n@@ -611,7 +614,8 @@ static void add_patterns_cone_mode(int argc, const char **argv,\n \tstruct pattern_list existing;\n \tchar *sparse_filename = get_sparse_checkout_filename();\n \n-\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\tadd_patterns_from_input(pl, argc, argv,\n+\t\t\t\tuse_stdin ? stdin : NULL);\n \n \tmemset(&existing, 0, sizeof(existing));\n \texisting.use_cone_patterns = core_sparse_checkout_cone;\n@@ -648,7 +652,7 @@ static void add_patterns_literal(int argc, const char **argv,\n \t\t\t\t\t pl, NULL, 0))\n \t\tdie(_(\"unable to load existing sparse-checkout patterns\"));\n \tfree(sparse_filename);\n-\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\tadd_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);\n }\n \n static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n@@ -667,7 +671,8 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n \t\tbreak;\n \n \tcase REPLACE:\n-\t\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\t\tadd_patterns_from_input(pl, argc, argv,\n+\t\t\t\t\tuse_stdin ? stdin : NULL);\n \t\tbreak;\n \t}\n \n@@ -929,6 +934,86 @@ static int sparse_checkout_disable(int argc, const char **argv,\n \treturn set_config(MODE_NO_PATTERNS);\n }\n \n+static char const * const builtin_sparse_checkout_check_rules_usage[] = {\n+\tN_(\"git sparse-checkout check-rules [-z] [--skip-checks]\"\n+\t \"[--[no-]cone] [--rules-file ]\"),\n+\tNULL\n+};\n+\n+static struct sparse_checkout_check_rules_opts {\n+\tint cone_mode;\n+\tint null_termination;\n+\tchar *rules_file;\n+} check_rules_opts;\n+\n+static int check_rules(struct pattern_list *pl, int null_terminated) {\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tchar *path;\n+\tint line_terminator = null_terminated ? 0 : '\\n';\n+\tstrbuf_getline_fn getline_fn = null_terminated ? strbuf_getline_nul\n+\t\t: strbuf_getline;\n+\tthe_repository->index->sparse_checkout_patterns = pl;\n+\twhile (!getline_fn(&line, stdin)) {\n+\t\tpath = line.buf;\n+\t\tif (!null_terminated && line.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, line.buf, NULL))\n+\t\t\t\tdie(_(\"unable to unquote C-style string '%s'\"),\n+\t\t\t\t\tline.buf);\n+\n+\t\t\tpath = unquoted.buf;\n+\t\t}\n+\n+\t\tif (path_in_sparse_checkout(path, the_repository->index))\n+\t\t\twrite_name_quoted(path, stdout, line_terminator);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix)\n+{\n+\tstatic struct option builtin_sparse_checkout_check_rules_options[] = {\n+\t\tOPT_BOOL('z', NULL, &check_rules_opts.null_termination,\n+\t\t\t N_(\"terminate input and output files by a NUL character\")),\n+\t\tOPT_BOOL(0, \"cone\", &check_rules_opts.cone_mode,\n+\t\t\t N_(\"when used with --rules-file interpret patterns as cone mode patterns\")),\n+\t\tOPT_FILENAME(0, \"rules-file\", &check_rules_opts.rules_file,\n+\t\t\t N_(\"use patterns in instead of the current ones.\")),\n+\t\tOPT_END(),\n+\t};\n+\n+\tFILE *fp;\n+\tint ret;\n+\tstruct pattern_list pl = {0};\n+\tchar *sparse_filename;\n+\tcheck_rules_opts.cone_mode = -1;\n+\n+\targc = parse_options(argc, argv, prefix,\n+\t\t\t builtin_sparse_checkout_check_rules_options,\n+\t\t\t builtin_sparse_checkout_check_rules_usage,\n+\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT);\n+\n+\tupdate_cone_mode(&check_rules_opts.cone_mode);\n+\tpl.use_cone_patterns = core_sparse_checkout_cone;\n+\tif (check_rules_opts.rules_file) {\n+\t\tfp = xfopen(check_rules_opts.rules_file, \"r\");\n+\t\tadd_patterns_from_input(&pl, argc, argv, fp);\n+\t\tfclose(fp);\n+\t} else {\n+\t\tsparse_filename = get_sparse_checkout_filename();\n+\t\tif (add_patterns_from_file_to_list(sparse_filename, \"\", 0, &pl,\n+\t\t\t\t\t\t NULL, 0))\n+\t\t\tdie(_(\"unable to load existing sparse-checkout patterns\"));\n+\t\tfree(sparse_filename);\n+\t}\n+\n+\tret = check_rules(&pl, check_rules_opts.null_termination);\n+\tclear_pattern_list(&pl);\n+\treturn ret;\n+}\n+\n int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n {\n \tparse_opt_subcommand_fn *fn = NULL;\n@@ -939,6 +1024,7 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n \t\tOPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n \t\tOPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n+\t\tOPT_SUBCOMMAND(\"check-rules\", &fn, sparse_checkout_check_rules),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 7216267aec7..521dc914fa7 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -555,7 +555,7 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '\n \tcheck_files repo a folder1\n '\n \n-test_expect_success 'interaction with submodules' '\n+test_expect_success 'setup submodules' '\n \tgit clone repo super &&\n \t(\n \t\tcd super &&\n@@ -566,11 +566,22 @@ test_expect_success 'interaction with submodules' '\n \t\tgit commit -m \"add submodule\" &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set folder1\n-\t) &&\n+\t)\n+'\n+\n+test_expect_success 'interaction with submodules' '\n \tcheck_files super a folder1 modules &&\n \tcheck_files super/modules/child a deep folder1 folder2\n '\n \n+test_expect_success 'check-rules interaction with submodules' '\n+\tgit -C super ls-tree --name-only -r HEAD >all-files &&\n+\tgit -C super sparse-checkout check-rules >check-rules-matches rules <<-\\EOF &&\n+\tfolder1\n+\tdeep/deeper1/deepest\n+\tEOF\n+\n+\tgit -C bare ls-tree -r --name-only HEAD >all-files &&\n+\tgit -C bare sparse-checkout check-rules --cone \\\n+\t\t--rules-file ../rules >check-rules-file out &&\n+\tsed -n \"/^S /!s/^. //p\" out >ls-files &&\n+\n+\tgit -C repo sparse-checkout check-rules >check-rules-default rules <<-\\EOF &&\n+\tdeep/deeper1/deepest/a\n+\tEOF\n+\n+\tgit -C bare ls-tree -r --name-only HEAD >all-files &&\n+\tgit -C bare sparse-checkout check-rules --no-cone --rules-file ../rules\\\n+\t\t>check-rules-file out &&\n+\tsed -n \"/^S /!s/^. //p\" out >ls-files &&\n+\n+\tgit -C repo sparse-checkout check-rules >check-rules-default expect <<-\\EOF &&\n+\tdeep/deeper1/deepest/a\n+\tEOF\n+\n+\ttest_cmp expect check-rules-file &&\n+\ttest_cmp check-rules-file ls-files &&\n+\ttest_cmp check-rules-file check-rules-default\n+'\n+\n+test_expect_success 'check-rules cone mode is default' '\n+\tcat >rules <<-\\EOF &&\n+\tfolder1\n+\tEOF\n+\n+\tcat >all-files <<-\\EOF &&\n+\ttoplevel\n+\tfolder2/file\n+\tfolder1/file\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\ttoplevel\n+\tfolder1/file\n+\tEOF\n+\n+\tgit -C bare sparse-checkout check-rules \\\n+\t\t--rules-file ../rules >actual rules <<-EOF &&\n+\t\"folder\\\" a\"\n+\tEOF\n+\tcat >files <<-EOF &&\n+\t\"folder\\\" a/file\"\n+\t\"folder\\\" b/file\"\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\t\"folder\\\" a/file\"\n+\tEOF\n+\tgit sparse-checkout check-rules --cone \\\n+\t\t--rules-file rules >actual rules <<-EOF &&\n+\t\"folder\\\" a\"\n+\tEOF\n+\n+\tlf_to_nul >files <<-EOF &&\n+\tfolder\" a/a\n+\tfolder\" a/b\n+\tfolder\" b/fileQ\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tfolder\" a/aQfolder\" a/bQ\n+\tEOF\n+\n+\tgit sparse-checkout check-rules --cone -z \\\n+\t\t--rules-file rules >actual.nul actual &&\n+\techo >>actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"473208","messageId":"xmqqmt4nt8k3.fsf@gitster.g","threadId":"59359","inReplyTo":"4b231e9beb43e4fac6457b9bf86e4c1db39c4238.1678283349.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-08T18:14:04Z","receivedAt":"2023-03-08T18:14:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Sprent via GitGitGadget\" writes:\n\n> In preparation for adding a sub-command to 'sparse-checkout' that can be\n> run in a bare repository, remove the 'NEED_WORK_TREE' flag from its\n> entry in the 'commands' array of 'git.c'.\n\nGiven that \"sparse-checkout\" is about affecting which part of the\ndirectory hierarchies are to be materialized in the working tree,\nthe idea to add a subcommand that does not require a working tree to\nthe command by itself smells very iffy, and the changes necessary to\nprotect the existing commands from the breakage caused by the\ndecision to drop NEED_WORK_TREE bit, i.e. sprinkling many\nsetup_work_tree() calls, somehow looks like they are solving a\nproblem that did not have to get created in the first place.\n\nBut you did not find a place the feature you wanted to add with\n[2/2] would fit better, perhaps, in which case, somebody else may be\nable to suggest an alternative in their reviews of that step,\nhopefully.\n\n\n"},{"id":"473215","messageId":"xmqqilfbt552.fsf@gitster.g","threadId":"59359","inReplyTo":"xmqqmt4nt8k3.fsf@gitster.g","subject":"Re: [PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-08T19:27:53Z","receivedAt":"2023-03-08T19:28:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> But you did not find a place the feature you wanted to add with\n> [2/2] would fit better, perhaps, in which case, somebody else may be\n> able to suggest an alternative in their reviews of that step,\n> hopefully.\n\n... oh, or perhaps the list would reach a consensus that mixing a\nsubcommand that does not require a working tree is not all that bad,\nwhich I am personally OK with, too. I didn't at all mean to say:\n\"this shouldn't be a subcommand there, do it somewhere else\".\n\nThanks.\n"},{"id":"473287","messageId":"CABPp-BGCgD+9s788YYmzvFHah0kik5QxbYPvWWf6mG--Pd5KPw@mail.gmail.com","threadId":"59359","inReplyTo":"xmqqilfbt552.fsf@gitster.g","subject":"Re: [PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-03-09T15:31:37Z","receivedAt":"2023-03-09T15:32:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Mar 8, 2023 at 11:27 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > But you did not find a place the feature you wanted to add with\n> > [2/2] would fit better, perhaps, in which case, somebody else may be\n> > able to suggest an alternative in their reviews of that step,\n> > hopefully.\n>\n> ... oh, or perhaps the list would reach a consensus that mixing a\n> subcommand that does not require a working tree is not all that bad,\n> which I am personally OK with, too. I didn't at all mean to say:\n> \"this shouldn't be a subcommand there, do it somewhere else\".\n\nI'm of the opinion that having sparse-checkout-related functionality\nall fall under the 'sparse-checkout' command makes sense. In the\npast, all such subcommands happened to require a working tree, but\nWilliam has come up with a reasonable one that doesn't. So, I'd be in\nfavor of this mixing.\n\nI'll try to get a chance to review these patches soon, perhaps this weekend.\n"},{"id":"473322","messageId":"xmqqbkl18t5d.fsf@gitster.g","threadId":"59359","inReplyTo":"CABPp-BGCgD+9s788YYmzvFHah0kik5QxbYPvWWf6mG--Pd5KPw@mail.gmail.com","subject":"Re: [PATCH 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T22:19:26Z","receivedAt":"2023-03-09T22:20:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> I'm of the opinion that having sparse-checkout-related functionality\n> all fall under the 'sparse-checkout' command makes sense. In the\n> past, all such subcommands happened to require a working tree, but\n> William has come up with a reasonable one that doesn't. So, I'd be in\n> favor of this mixing.\n\nYup, I am OK with that reasoning myself.\n\n> I'll try to get a chance to review these patches soon, perhaps this weekend.\n\nThanks.\n"},{"id":"473696","messageId":"CABPp-BE51HR9jvi_VxYHk8aWdGw+m=zCOgdYn8rQzYhht+bVJA@mail.gmail.com","threadId":"59359","inReplyTo":"21c8375efff5c20e299c52aa579e6b10dafcbc4f.1678283349.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] builtin/sparse-checkout: add check-rules command","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-03-19T04:26:37Z","receivedAt":"2023-03-19T04:26:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Mar 8, 2023 at 5:49 AM William Sprent via GitGitGadget\n wrote:\n>\n> From: William Sprent \n>\n> There exists no direct way to interrogate git about which paths are\n> matched by a given set of sparsity rules. It is possible to get this\n> information from git, but it includes checking out the commit that\n> contains the paths, applying the sparse checkout patterns and then using\n> something like 'git ls-files -t' to check if the skip worktree bit is\n> set. This works in some case, but there are cases where it is awkward or\n> infeasible to generate a checkout for this purpose.\n>\n> Exposing the pattern matching of sparse checkout enables more tooling to\n> be built and avoids a situation where tools that want to reason about\n> sparse checkouts start containing parallel implementation of the rules.\n> To accommodate this, add a 'check-rules' subcommand to the\n> 'sparse-checkout' builtin along the lines of the 'git check-ignore' and\n> 'git check-attr' commands. The new command accepts a list of paths on\n> stdin and outputs just the ones the match the sparse checkout.\n>\n> To allow for use in a bare repository and to allow for interrogating\n> about other patterns than the current ones, include a '--rules-file'\n> option which allows the caller to explicitly pass sparse checkout rules\n> in the format accepted by 'sparse-checkout set --stdin'.\n>\n> To allow for reuse of the handling of input patterns for the\n> '--rules-file' flag, modify 'add_patterns_from_input()' to be able to\n> read from a 'FILE' instead of just stdin.\n>\n> To allow for reuse of the logic which decides whether or not rules\n> should be interpreted as cone-mode patterns, split that part out of\n> 'update_modes()' such that can be called without modifying the config.\n>\n> An alternative could have been to create a new 'check-sparsity' command.\n> However, placing it under 'sparse-checkout' allows for a) more easily\n> re-using the sparse checkout pattern matching and cone/non-code mode\n> handling, and b) keeps the documentation for the command next to the\n> experimental warning and the cone-mode discussion.\n>\n> Signed-off-by: William Sprent \n> ---\n> Documentation/git-sparse-checkout.txt | 23 ++++-\n> builtin/sparse-checkout.c | 126 +++++++++++++++++++++----\n> t/t1091-sparse-checkout-builtin.sh | 129 +++++++++++++++++++++++++-\n> 3 files changed, 255 insertions(+), 23 deletions(-)\n>\n> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\n> index 68392d2a56e..8fdde8f53be 100644\n> --- a/Documentation/git-sparse-checkout.txt\n> +++ b/Documentation/git-sparse-checkout.txt\n> @@ -9,7 +9,7 @@ git-sparse-checkout - Reduce your working tree to a subset of tracked files\n> SYNOPSIS\n> --------\n> [verse]\n> -'git sparse-checkout' (init | list | set | add | reapply | disable) []\n> +'git sparse-checkout' (init | list | set | add | reapply | disable | check-rules) []\n>\n>\n> DESCRIPTION\n> @@ -135,6 +135,27 @@ paths to pass to a subsequent 'set' or 'add' command. However,\n> the disable command, so the easy restore of calling a plain `init`\n> decreased in utility.\n>\n> +'check-rules'::\n> + Check whether sparsity rules match one or more paths.\n> ++\n> +By default `check-rules` reads a list of paths from stdin and outputs only\n> +the ones that match the current sparsity rules. The input is expected to consist\n> +of one path per line, matching the output of `git ls-tree --name-only` including\n> +that pathnames that begin with a double quote (\") are interpreted C-style quoted\n> +strings.\n> ++\n> +When called with the `--rules-file ` the input files are matched against\n> +the sparse checkout rules found in `` instead of the current ones. The\n> +rules in the files are expected to be in the same form as accepted by `git\n> +sparse-checkout set --stdin`.\n\nCould we add a \"(in particular, they must be newline-delimited)\" to\nthe end of that paragraph?\n\nI built your series locally to test, and my first attempt was:\n\n$ git sparse-checkout set t reftable\n$ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n| git sparse-checkout check-rules\nt/helper/test-userdiff.c\nreftable/basics.c\nmerge-ort.c\n\n...which did exactly what I expected and wanted. So, I decided to\nundo the sparse checkout and use the --rules-file option:\n\n$ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n| git sparse-checkout check-rules --rules-file <(echo t reftable)\nmerge-ort.c\n\nI was perplexed. I thought it was a bug and was typing up explaining\nwhy this was a problem, before I re-read this sentence of your\ndocumentation closer and realized I had just flubbed it. I should\nhave ran:\n$ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n| git sparse-checkout check-rules --rules-file <(echo -e\n\"t\\nreftable\")\nt/helper/test-userdiff.c\nreftable/basics.c\nmerge-ort.c\n\nGiven that this wasn't obvious to me after reading the Documentation\n(it probably should have been but just wasn't), might it be worth\ncalling out?\n\n> ++\n> +The `--rules-file` flag can be combined with the `--[no]-cone` with the same\n> +effect as for the `set` command with the `--stdin` flag.\n> ++\n> +When called with the `-z` flag the input format and output format is \\0\n> +terminated and not quoted.\n\nI believe this does not affect the format of the --rules-file\noption...but maybe it should? Whether it does or doesn't, do we want\nto call it out when explaining -z's behavior?\n\n> +\n> EXAMPLES\n> --------\n> `git sparse-checkout set MY/DIR1 SUB/DIR2`::\n> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> index 5fdc3d9aab5..969ae14a415 100644\n> --- a/builtin/sparse-checkout.c\n> +++ b/builtin/sparse-checkout.c\n> @@ -20,7 +20,7 @@\n> static const char *empty_base = \"\";\n>\n> static char const * const builtin_sparse_checkout_usage[] = {\n> - N_(\"git sparse-checkout (init | list | set | add | reapply | disable) []\"),\n> + N_(\"git sparse-checkout (init | list | set | add | reapply | disable | check-rules) []\"),\n> NULL\n> };\n>\n> @@ -384,13 +384,7 @@ static int set_config(enum sparse_checkout_mode mode)\n> return 0;\n> }\n>\n> -static int update_modes(int *cone_mode, int *sparse_index)\n> -{\n> - int mode, record_mode;\n> -\n> - /* Determine if we need to record the mode; ensure sparse checkout on */\n> - record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n> -\n> +static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {\n> /* If not specified, use previous definition of cone mode */\n> if (*cone_mode == -1 && core_apply_sparse_checkout)\n> *cone_mode = core_sparse_checkout_cone;\n> @@ -398,12 +392,21 @@ static int update_modes(int *cone_mode, int *sparse_index)\n> /* Set cone/non-cone mode appropriately */\n> core_apply_sparse_checkout = 1;\n> if (*cone_mode == 1 || *cone_mode == -1) {\n> - mode = MODE_CONE_PATTERNS;\n> core_sparse_checkout_cone = 1;\n> - } else {\n> - mode = MODE_ALL_PATTERNS;\n> - core_sparse_checkout_cone = 0;\n> + return MODE_CONE_PATTERNS;\n> }\n> + core_sparse_checkout_cone = 0;\n> + return MODE_ALL_PATTERNS;\n> +}\n> +\n> +static int update_modes(int *cone_mode, int *sparse_index)\n> +{\n> + int mode, record_mode;\n> +\n> + /* Determine if we need to record the mode; ensure sparse checkout on */\n> + record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n> +\n> + mode = update_cone_mode(cone_mode);\n> if (record_mode && set_config(mode))\n> return 1;\n>\n> @@ -547,7 +550,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n>\n> static void add_patterns_from_input(struct pattern_list *pl,\n> int argc, const char **argv,\n> - int use_stdin)\n> + FILE *file)\n> {\n> int i;\n> if (core_sparse_checkout_cone) {\n> @@ -557,9 +560,9 @@ static void add_patterns_from_input(struct pattern_list *pl,\n> hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);\n> pl->use_cone_patterns = 1;\n>\n> - if (use_stdin) {\n> + if (file) {\n> struct strbuf unquoted = STRBUF_INIT;\n> - while (!strbuf_getline(&line, stdin)) {\n> + while (!strbuf_getline(&line, file)) {\n> if (line.buf[0] == '\"') {\n> strbuf_reset(&unquoted);\n> if (unquote_c_style(&unquoted, line.buf, NULL))\n> @@ -581,10 +584,10 @@ static void add_patterns_from_input(struct pattern_list *pl,\n> }\n> }\n> } else {\n> - if (use_stdin) {\n> + if (file) {\n> struct strbuf line = STRBUF_INIT;\n>\n> - while (!strbuf_getline(&line, stdin)) {\n> + while (!strbuf_getline(&line, file)) {\n> size_t len;\n> char *buf = strbuf_detach(&line, &len);\n> add_pattern(buf, empty_base, 0, pl, 0);\n> @@ -611,7 +614,8 @@ static void add_patterns_cone_mode(int argc, const char **argv,\n> struct pattern_list existing;\n> char *sparse_filename = get_sparse_checkout_filename();\n>\n> - add_patterns_from_input(pl, argc, argv, use_stdin);\n> + add_patterns_from_input(pl, argc, argv,\n> + use_stdin ? stdin : NULL);\n>\n> memset(&existing, 0, sizeof(existing));\n> existing.use_cone_patterns = core_sparse_checkout_cone;\n> @@ -648,7 +652,7 @@ static void add_patterns_literal(int argc, const char **argv,\n> pl, NULL, 0))\n> die(_(\"unable to load existing sparse-checkout patterns\"));\n> free(sparse_filename);\n> - add_patterns_from_input(pl, argc, argv, use_stdin);\n> + add_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);\n> }\n>\n> static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n> @@ -667,7 +671,8 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n> break;\n>\n> case REPLACE:\n> - add_patterns_from_input(pl, argc, argv, use_stdin);\n> + add_patterns_from_input(pl, argc, argv,\n> + use_stdin ? stdin : NULL);\n> break;\n> }\n>\n> @@ -929,6 +934,86 @@ static int sparse_checkout_disable(int argc, const char **argv,\n> return set_config(MODE_NO_PATTERNS);\n> }\n>\n> +static char const * const builtin_sparse_checkout_check_rules_usage[] = {\n> + N_(\"git sparse-checkout check-rules [-z] [--skip-checks]\"\n> + \"[--[no-]cone] [--rules-file ]\"),\n> + NULL\n> +};\n> +\n> +static struct sparse_checkout_check_rules_opts {\n> + int cone_mode;\n> + int null_termination;\n> + char *rules_file;\n> +} check_rules_opts;\n> +\n> +static int check_rules(struct pattern_list *pl, int null_terminated) {\n> + struct strbuf line = STRBUF_INIT;\n> + struct strbuf unquoted = STRBUF_INIT;\n> + char *path;\n> + int line_terminator = null_terminated ? 0 : '\\n';\n> + strbuf_getline_fn getline_fn = null_terminated ? strbuf_getline_nul\n> + : strbuf_getline;\n> + the_repository->index->sparse_checkout_patterns = pl;\n> + while (!getline_fn(&line, stdin)) {\n\nstrbuf_getline_nul() and strbuf_getline() are documented as\noverwriting their strbuf, so there is no need to clear line.\n\n> + path = line.buf;\n> + if (!null_terminated && line.buf[0] == '\"') {\n> + strbuf_reset(&unquoted);\n> + if (unquote_c_style(&unquoted, line.buf, NULL))\n\nunquote_c_style() is documented as appending to its strbuf, but you\nhandle that via the strbuf_reset() on the line before.\n\n> + die(_(\"unable to unquote C-style string '%s'\"),\n> + line.buf);\n> +\n> + path = unquoted.buf;\n> + }\n> +\n> + if (path_in_sparse_checkout(path, the_repository->index))\n> + write_name_quoted(path, stdout, line_terminator);\n> + }\n\nDo you need to call strbuf_release() for both line and unquoted here?\nI think you might have a small leak.\n\n> +\n> + return 0;\n> +}\n> +\n> +static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix)\n> +{\n> + static struct option builtin_sparse_checkout_check_rules_options[] = {\n> + OPT_BOOL('z', NULL, &check_rules_opts.null_termination,\n> + N_(\"terminate input and output files by a NUL character\")),\n> + OPT_BOOL(0, \"cone\", &check_rules_opts.cone_mode,\n> + N_(\"when used with --rules-file interpret patterns as cone mode patterns\")),\n> + OPT_FILENAME(0, \"rules-file\", &check_rules_opts.rules_file,\n> + N_(\"use patterns in instead of the current ones.\")),\n> + OPT_END(),\n> + };\n> +\n> + FILE *fp;\n> + int ret;\n> + struct pattern_list pl = {0};\n> + char *sparse_filename;\n> + check_rules_opts.cone_mode = -1;\n> +\n> + argc = parse_options(argc, argv, prefix,\n> + builtin_sparse_checkout_check_rules_options,\n> + builtin_sparse_checkout_check_rules_usage,\n> + PARSE_OPT_KEEP_UNKNOWN_OPT);\n> +\n> + update_cone_mode(&check_rules_opts.cone_mode);\n> + pl.use_cone_patterns = core_sparse_checkout_cone;\n> + if (check_rules_opts.rules_file) {\n> + fp = xfopen(check_rules_opts.rules_file, \"r\");\n> + add_patterns_from_input(&pl, argc, argv, fp);\n> + fclose(fp);\n> + } else {\n> + sparse_filename = get_sparse_checkout_filename();\n> + if (add_patterns_from_file_to_list(sparse_filename, \"\", 0, &pl,\n> + NULL, 0))\n> + die(_(\"unable to load existing sparse-checkout patterns\"));\n> + free(sparse_filename);\n> + }\n> +\n> + ret = check_rules(&pl, check_rules_opts.null_termination);\n> + clear_pattern_list(&pl);\n> + return ret;\n> +}\n> +\n> int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n> {\n> parse_opt_subcommand_fn *fn = NULL;\n> @@ -939,6 +1024,7 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n> OPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n> OPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n> OPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n> + OPT_SUBCOMMAND(\"check-rules\", &fn, sparse_checkout_check_rules),\n> OPT_END(),\n> };\n>\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index 7216267aec7..521dc914fa7 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -555,7 +555,7 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '\n> check_files repo a folder1\n> '\n>\n> -test_expect_success 'interaction with submodules' '\n> +test_expect_success 'setup submodules' '\n> git clone repo super &&\n> (\n> cd super &&\n> @@ -566,11 +566,22 @@ test_expect_success 'interaction with submodules' '\n> git commit -m \"add submodule\" &&\n> git sparse-checkout init --cone &&\n> git sparse-checkout set folder1\n> - ) &&\n> + )\n> +'\n> +\n> +test_expect_success 'interaction with submodules' '\n> check_files super a folder1 modules &&\n> check_files super/modules/child a deep folder1 folder2\n> '\n>\n> +test_expect_success 'check-rules interaction with submodules' '\n> + git -C super ls-tree --name-only -r HEAD >all-files &&\n> + git -C super sparse-checkout check-rules >check-rules-matches +\n> + test_i18ngrep ! \"modules/\" check-rules-matches &&\n> + test_i18ngrep \"folder1/\" check-rules-matches\n\nThis file already uses test_i18ngrep, so continuing to use it is okay\njust for consistency. But in general test_i18ngrep is deprecated\n(\"Should not be used and will be removed soon.\" according to the docs;\nsoon is relative) and we prefer to just use grep.\n\n> +'\n> +\n> test_expect_success 'different sparse-checkouts with worktrees' '\n> git -C repo sparse-checkout set --cone deep folder1 &&\n> git -C repo worktree add --detach ../worktree &&\n> @@ -915,4 +926,118 @@ test_expect_success 'disable fails outside work tree' '\n> test_i18ngrep \"this operation must be run in a work tree\" err\n> '\n>\n> +test_expect_success 'setup clean' '\n> + git -C repo clean -fdx\n> +'\n> +\n> +test_expect_success 'check-rules cone mode' '\n> + cat >rules <<-\\EOF &&\n> + folder1\n> + deep/deeper1/deepest\n> + EOF\n> +\n> + git -C bare ls-tree -r --name-only HEAD >all-files &&\n> + git -C bare sparse-checkout check-rules --cone \\\n> + --rules-file ../rules >check-rules-file +\n> + git -C repo sparse-checkout set --cone --stdin + git -C repo ls-files -t >out &&\n> + sed -n \"/^S /!s/^. //p\" out >ls-files &&\n> +\n> + git -C repo sparse-checkout check-rules >check-rules-default +\n> + test_i18ngrep \"deep/deeper1/deepest/a\" check-rules-file &&\n> + test_i18ngrep ! \"deep/deeper2\" check-rules-file &&\n> +\n> + test_cmp check-rules-file ls-files &&\n> + test_cmp check-rules-file check-rules-default\n> +'\n> +\n> +test_expect_success 'check-rules non-cone mode' '\n> + cat >rules <<-\\EOF &&\n> + deep/deeper1/deepest/a\n> + EOF\n> +\n> + git -C bare ls-tree -r --name-only HEAD >all-files &&\n> + git -C bare sparse-checkout check-rules --no-cone --rules-file ../rules\\\n> + >check-rules-file +\n> + cat rules | git -C repo sparse-checkout set --no-cone --stdin &&\n> + git -C repo ls-files -t >out &&\n> + sed -n \"/^S /!s/^. //p\" out >ls-files &&\n> +\n> + git -C repo sparse-checkout check-rules >check-rules-default +\n> + cat >expect <<-\\EOF &&\n> + deep/deeper1/deepest/a\n> + EOF\n> +\n> + test_cmp expect check-rules-file &&\n> + test_cmp check-rules-file ls-files &&\n> + test_cmp check-rules-file check-rules-default\n> +'\n> +\n> +test_expect_success 'check-rules cone mode is default' '\n> + cat >rules <<-\\EOF &&\n> + folder1\n> + EOF\n> +\n> + cat >all-files <<-\\EOF &&\n> + toplevel\n> + folder2/file\n> + folder1/file\n> + EOF\n> +\n> + cat >expect <<-\\EOF &&\n> + toplevel\n> + folder1/file\n> + EOF\n> +\n> + git -C bare sparse-checkout check-rules \\\n> + --rules-file ../rules >actual +\n> + test_cmp expect actual\n\nThis test is checking that despite whatever sparse-checkout setting we\nhave, that the --rules-files option makes it ignore those rules and\nuse the specified one instead, and in particular that cone mode is\nassumed when nothing is specified with --rules-file. The test is\nstronger because the test right before this one did a non-cone-mode\ncheckout. But since this test didn't itself do a non-cone-mode\ncheckout, that extra strength of the test could be lost by a future\ncontributor inserting another test between these. It's not a big\ndeal, but perhaps it'd make sense to have this test add a `git\nsparse-checkout set --no-cone ...` call near the beginning?\n\n> +'\n> +\n> +test_expect_success 'check-rules quoting' '\n> + cat >rules <<-EOF &&\n> + \"folder\\\" a\"\n> + EOF\n> + cat >files <<-EOF &&\n> + \"folder\\\" a/file\"\n> + \"folder\\\" b/file\"\n> + EOF\n> + cat >expect <<-EOF &&\n> + \"folder\\\" a/file\"\n> + EOF\n> + git sparse-checkout check-rules --cone \\\n> + --rules-file rules >actual +\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'check-rules null termination' '\n> + cat >rules <<-EOF &&\n> + \"folder\\\" a\"\n> + EOF\n> +\n> + lf_to_nul >files <<-EOF &&\n> + folder\" a/a\n> + folder\" a/b\n> + folder\" b/fileQ\n> + EOF\n> +\n> + cat >expect <<-EOF &&\n> + folder\" a/aQfolder\" a/bQ\n> + EOF\n> +\n> + git sparse-checkout check-rules --cone -z \\\n> + --rules-file rules >actual.nul + nul_to_q actual &&\n> + echo >>actual &&\n> +\n> + test_cmp expect actual\n> +'\n> +\n> +\n> test_done\n> --\n> gitgitgadget\n\nOverall, nicely done. I only found minor things to comment on, and\nlike the overall design, explanation, and implementation.\n"},{"id":"473697","messageId":"CABPp-BGBBptR-BLGzd43w26uzKazK77f9goPbVCXv+Y0-FzbvQ@mail.gmail.com","threadId":"59359","inReplyTo":"pull.1488.git.1678283349.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] builtin/sparse-checkout: add check-rules command","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-03-19T04:28:52Z","receivedAt":"2023-03-19T04:29:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Mar 8, 2023 at 5:49 AM William Sprent via GitGitGadget\n wrote:\n>\n> Hi\n>\n> Back in January I submitted a patch which suggested to teach 'ls-tree' to be\n> able to filter its output based on sparse checkout specifications [1]. My\n> main motivation for doing so was (is) to enable building more tooling on top\n> of sparse checkouts, which is currently hampered a bit by the fact that git\n> doesn't expose the pattern matching rules for the sparse checkouts.\n>\n> I think the main point from that thread was that the 'ls-tree' change was\n> conceptually a larger change that I had initially thought it was. It was\n> suggested that perhaps it would be more straight-forward to initially add a\n> command in the vein of 'git-check-ignore' before teaching all the other\n> commands about sparse checkout specifics, and I think that makes sense. So I\n> am proposing here a new 'check-rules' sub-command to 'sparse-checkout'. This\n> exposes the sparse checkout pattern matching logic while still keeping the\n> pattern specification local to the sparse-checkout command.\n>\n> Since the intention is that this new behavior would not need a work tree as\n> it allows the user to supply a set of rules to verify the paths against, the\n> change that introduces the sub-command is preceded by one that removes the\n> 'NEEDS_WORK_TREE' flag for 'sparse-checkout' and replaces it with calls to\n> 'setup_work_tree()' to keep current behavior.\n>\n> 1:\n> https://public-inbox.org/git/pull.1459.git.1673456518993.gitgitgadget@gmail.com/\n\nSorry for the delay in reviewing. Patch 1 looks good to me. Patch 2\nlooks really good as well, but there were a few minor things that we\nmight be able to touch up.\n\nAnd now that I've played with it and tested a little bit locally, I\nreally like the idea of having this functionality around. Thanks for\nsending it in.\n\n> William Sprent (2):\n> builtin/sparse-checkout: remove NEED_WORK_TREE flag\n> builtin/sparse-checkout: add check-rules command\n>\n> Documentation/git-sparse-checkout.txt | 23 +++-\n> builtin/sparse-checkout.c | 132 +++++++++++++++++----\n> git.c | 2 +-\n> t/t1091-sparse-checkout-builtin.sh | 162 +++++++++++++++++++++++++-\n> 4 files changed, 295 insertions(+), 24 deletions(-)\n>\n>\n> base-commit: d15644fe0226af7ffc874572d968598564a230dd\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1488%2Fwilliams-unity%2Fsparse-doodle-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1488/williams-unity/sparse-doodle-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1488\n> --\n> gitgitgadget\n"},{"id":"473775","messageId":"8c0b4c56-45a2-865c-fe6e-367d3d5f8a02@unity3d.com","threadId":"59359","inReplyTo":"CABPp-BE51HR9jvi_VxYHk8aWdGw+m=zCOgdYn8rQzYhht+bVJA@mail.gmail.com","subject":"Re: [PATCH 2/2] builtin/sparse-checkout: add check-rules command","fromName":"William Sprent","fromEmail":"williams@unity3d.com","sentAt":"2023-03-20T15:49:00Z","receivedAt":"2023-03-20T15:59:10Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"On 19/03/2023 05.26, Elijah Newren wrote:\n> Hi,\n> \n> On Wed, Mar 8, 2023 at 5:49 AM William Sprent via GitGitGadget\n> wrote:\n>>\n>> From: William Sprent \n>>\n>> There exists no direct way to interrogate git about which paths are\n>> matched by a given set of sparsity rules. It is possible to get this\n>> information from git, but it includes checking out the commit that\n>> contains the paths, applying the sparse checkout patterns and then using\n>> something like 'git ls-files -t' to check if the skip worktree bit is\n>> set. This works in some case, but there are cases where it is awkward or\n>> infeasible to generate a checkout for this purpose.\n>>\n>> Exposing the pattern matching of sparse checkout enables more tooling to\n>> be built and avoids a situation where tools that want to reason about\n>> sparse checkouts start containing parallel implementation of the rules.\n>> To accommodate this, add a 'check-rules' subcommand to the\n>> 'sparse-checkout' builtin along the lines of the 'git check-ignore' and\n>> 'git check-attr' commands. The new command accepts a list of paths on\n>> stdin and outputs just the ones the match the sparse checkout.\n>>\n>> To allow for use in a bare repository and to allow for interrogating\n>> about other patterns than the current ones, include a '--rules-file'\n>> option which allows the caller to explicitly pass sparse checkout rules\n>> in the format accepted by 'sparse-checkout set --stdin'.\n>>\n>> To allow for reuse of the handling of input patterns for the\n>> '--rules-file' flag, modify 'add_patterns_from_input()' to be able to\n>> read from a 'FILE' instead of just stdin.\n>>\n>> To allow for reuse of the logic which decides whether or not rules\n>> should be interpreted as cone-mode patterns, split that part out of\n>> 'update_modes()' such that can be called without modifying the config.\n>>\n>> An alternative could have been to create a new 'check-sparsity' command.\n>> However, placing it under 'sparse-checkout' allows for a) more easily\n>> re-using the sparse checkout pattern matching and cone/non-code mode\n>> handling, and b) keeps the documentation for the command next to the\n>> experimental warning and the cone-mode discussion.\n>>\n>> Signed-off-by: William Sprent \n>> ---\n>> Documentation/git-sparse-checkout.txt | 23 ++++-\n>> builtin/sparse-checkout.c | 126 +++++++++++++++++++++----\n>> t/t1091-sparse-checkout-builtin.sh | 129 +++++++++++++++++++++++++-\n>> 3 files changed, 255 insertions(+), 23 deletions(-)\n>>\n>> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\n>> index 68392d2a56e..8fdde8f53be 100644\n>> --- a/Documentation/git-sparse-checkout.txt\n>> +++ b/Documentation/git-sparse-checkout.txt\n>> @@ -9,7 +9,7 @@ git-sparse-checkout - Reduce your working tree to a subset of tracked files\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> -'git sparse-checkout' (init | list | set | add | reapply | disable) []\n>> +'git sparse-checkout' (init | list | set | add | reapply | disable | check-rules) []\n>>\n>>\n>> DESCRIPTION\n>> @@ -135,6 +135,27 @@ paths to pass to a subsequent 'set' or 'add' command. However,\n>> the disable command, so the easy restore of calling a plain `init`\n>> decreased in utility.\n>>\n>> +'check-rules'::\n>> + Check whether sparsity rules match one or more paths.\n>> ++\n>> +By default `check-rules` reads a list of paths from stdin and outputs only\n>> +the ones that match the current sparsity rules. The input is expected to consist\n>> +of one path per line, matching the output of `git ls-tree --name-only` including\n>> +that pathnames that begin with a double quote (\") are interpreted C-style quoted\n>> +strings.\n>> ++\n>> +When called with the `--rules-file ` the input files are matched against\n>> +the sparse checkout rules found in `` instead of the current ones. The\n>> +rules in the files are expected to be in the same form as accepted by `git\n>> +sparse-checkout set --stdin`.\n> \n> Could we add a \"(in particular, they must be newline-delimited)\" to\n> the end of that paragraph?\n> \n> I built your series locally to test, and my first attempt was:\n> \n> $ git sparse-checkout set t reftable\n> $ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n> | git sparse-checkout check-rules\n> t/helper/test-userdiff.c\n> reftable/basics.c\n> merge-ort.c\n> \n> ...which did exactly what I expected and wanted. So, I decided to\n> undo the sparse checkout and use the --rules-file option:\n> \n> $ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n> | git sparse-checkout check-rules --rules-file <(echo t reftable)\n> merge-ort.c\n> \n> I was perplexed. I thought it was a bug and was typing up explaining\n> why this was a problem, before I re-read this sentence of your\n> documentation closer and realized I had just flubbed it. I should\n> have ran:\n> $ echo -e \"t/helper/test-userdiff.c\\nreftable/basics.c\\nmerge-ort.c\\nDocumentation/give-up.txt\"\n> | git sparse-checkout check-rules --rules-file <(echo -e\n> \"t\\nreftable\")\n> t/helper/test-userdiff.c\n> reftable/basics.c\n> merge-ort.c\n> \n> Given that this wasn't obvious to me after reading the Documentation\n> (it probably should have been but just wasn't), might it be worth\n> calling out?\n> \n\nSure. I'll add that to the end. That gives us:\n\n\"The rules in the files are expected to be in the same form as accepted by `git\nsparse-checkout set --stdin` (in particular, they must be newline-delimited).\"\n\n>> ++\n>> +The `--rules-file` flag can be combined with the `--[no]-cone` with the same\n>> +effect as for the `set` command with the `--stdin` flag.\n>> ++\n>> +When called with the `-z` flag the input format and output format is \\0\n>> +terminated and not quoted.\n> \n> I believe this does not affect the format of the --rules-file\n> option...but maybe it should? Whether it does or doesn't, do we want\n> to call it out when explaining -z's behavior?\n>\n\nIt does not affect the --rules-file option. That's right.\nI think it makes sense to make it such that you can feed the exact same rules\ninto 'check-rules' as you can into 'set --stdin', and 'set --stdin' does not have\na -z option. And since rules are a user facing concept, I can't think of a\nsituation where you would want to store them as null separated records.\n\nIf we _do_ want to have a -z for the --rules-file option, I think that it should\nprobably be a separate option given that it isn't unlikely that you would want\nto have rules stored newline separated as they are first and foremost for users\nbut still would prefer to not have to deal with quoted paths when authoring\na tool or a script.\n\nBut I do think it makes sense to call it out in the documentation. Maybe something\nalong the lines of:\n\n\"When called with the `-z` flag, the format of the paths input on stdin as well as\nthe output paths are \\0 terminated and not quoted. Note that this does not apply\nto the format of the rules passed with the `--rules-file` option.\"\n\n \n>> +\n>> EXAMPLES\n>> --------\n>> `git sparse-checkout set MY/DIR1 SUB/DIR2`::\n>> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n>> index 5fdc3d9aab5..969ae14a415 100644\n>> --- a/builtin/sparse-checkout.c\n>> +++ b/builtin/sparse-checkout.c\n>> @@ -20,7 +20,7 @@\n>> static const char *empty_base = \"\";\n>>\n>> static char const * const builtin_sparse_checkout_usage[] = {\n>> - N_(\"git sparse-checkout (init | list | set | add | reapply | disable) []\"),\n>> + N_(\"git sparse-checkout (init | list | set | add | reapply | disable | check-rules) []\"),\n>> NULL\n>> };\n>>\n>> @@ -384,13 +384,7 @@ static int set_config(enum sparse_checkout_mode mode)\n>> return 0;\n>> }\n>>\n>> -static int update_modes(int *cone_mode, int *sparse_index)\n>> -{\n>> - int mode, record_mode;\n>> -\n>> - /* Determine if we need to record the mode; ensure sparse checkout on */\n>> - record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n>> -\n>> +static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {\n>> /* If not specified, use previous definition of cone mode */\n>> if (*cone_mode == -1 && core_apply_sparse_checkout)\n>> *cone_mode = core_sparse_checkout_cone;\n>> @@ -398,12 +392,21 @@ static int update_modes(int *cone_mode, int *sparse_index)\n>> /* Set cone/non-cone mode appropriately */\n>> core_apply_sparse_checkout = 1;\n>> if (*cone_mode == 1 || *cone_mode == -1) {\n>> - mode = MODE_CONE_PATTERNS;\n>> core_sparse_checkout_cone = 1;\n>> - } else {\n>> - mode = MODE_ALL_PATTERNS;\n>> - core_sparse_checkout_cone = 0;\n>> + return MODE_CONE_PATTERNS;\n>> }\n>> + core_sparse_checkout_cone = 0;\n>> + return MODE_ALL_PATTERNS;\n>> +}\n>> +\n>> +static int update_modes(int *cone_mode, int *sparse_index)\n>> +{\n>> + int mode, record_mode;\n>> +\n>> + /* Determine if we need to record the mode; ensure sparse checkout on */\n>> + record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n>> +\n>> + mode = update_cone_mode(cone_mode);\n>> if (record_mode && set_config(mode))\n>> return 1;\n>>\n>> @@ -547,7 +550,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n>>\n>> static void add_patterns_from_input(struct pattern_list *pl,\n>> int argc, const char **argv,\n>> - int use_stdin)\n>> + FILE *file)\n>> {\n>> int i;\n>> if (core_sparse_checkout_cone) {\n>> @@ -557,9 +560,9 @@ static void add_patterns_from_input(struct pattern_list *pl,\n>> hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);\n>> pl->use_cone_patterns = 1;\n>>\n>> - if (use_stdin) {\n>> + if (file) {\n>> struct strbuf unquoted = STRBUF_INIT;\n>> - while (!strbuf_getline(&line, stdin)) {\n>> + while (!strbuf_getline(&line, file)) {\n>> if (line.buf[0] == '\"') {\n>> strbuf_reset(&unquoted);\n>> if (unquote_c_style(&unquoted, line.buf, NULL))\n>> @@ -581,10 +584,10 @@ static void add_patterns_from_input(struct pattern_list *pl,\n>> }\n>> }\n>> } else {\n>> - if (use_stdin) {\n>> + if (file) {\n>> struct strbuf line = STRBUF_INIT;\n>>\n>> - while (!strbuf_getline(&line, stdin)) {\n>> + while (!strbuf_getline(&line, file)) {\n>> size_t len;\n>> char *buf = strbuf_detach(&line, &len);\n>> add_pattern(buf, empty_base, 0, pl, 0);\n>> @@ -611,7 +614,8 @@ static void add_patterns_cone_mode(int argc, const char **argv,\n>> struct pattern_list existing;\n>> char *sparse_filename = get_sparse_checkout_filename();\n>>\n>> - add_patterns_from_input(pl, argc, argv, use_stdin);\n>> + add_patterns_from_input(pl, argc, argv,\n>> + use_stdin ? stdin : NULL);\n>>\n>> memset(&existing, 0, sizeof(existing));\n>> existing.use_cone_patterns = core_sparse_checkout_cone;\n>> @@ -648,7 +652,7 @@ static void add_patterns_literal(int argc, const char **argv,\n>> pl, NULL, 0))\n>> die(_(\"unable to load existing sparse-checkout patterns\"));\n>> free(sparse_filename);\n>> - add_patterns_from_input(pl, argc, argv, use_stdin);\n>> + add_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);\n>> }\n>>\n>> static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n>> @@ -667,7 +671,8 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n>> break;\n>>\n>> case REPLACE:\n>> - add_patterns_from_input(pl, argc, argv, use_stdin);\n>> + add_patterns_from_input(pl, argc, argv,\n>> + use_stdin ? stdin : NULL);\n>> break;\n>> }\n>>\n>> @@ -929,6 +934,86 @@ static int sparse_checkout_disable(int argc, const char **argv,\n>> return set_config(MODE_NO_PATTERNS);\n>> }\n>>\n>> +static char const * const builtin_sparse_checkout_check_rules_usage[] = {\n>> + N_(\"git sparse-checkout check-rules [-z] [--skip-checks]\"\n>> + \"[--[no-]cone] [--rules-file ]\"),\n>> + NULL\n>> +};\n>> +\n>> +static struct sparse_checkout_check_rules_opts {\n>> + int cone_mode;\n>> + int null_termination;\n>> + char *rules_file;\n>> +} check_rules_opts;\n>> +\n>> +static int check_rules(struct pattern_list *pl, int null_terminated) {\n>> + struct strbuf line = STRBUF_INIT;\n>> + struct strbuf unquoted = STRBUF_INIT;\n>> + char *path;\n>> + int line_terminator = null_terminated ? 0 : '\\n';\n>> + strbuf_getline_fn getline_fn = null_terminated ? strbuf_getline_nul\n>> + : strbuf_getline;\n>> + the_repository->index->sparse_checkout_patterns = pl;\n>> + while (!getline_fn(&line, stdin)) {\n> \n> strbuf_getline_nul() and strbuf_getline() are documented as\n> overwriting their strbuf, so there is no need to clear line.\n> \n>> + path = line.buf;\n>> + if (!null_terminated && line.buf[0] == '\"') {\n>> + strbuf_reset(&unquoted);\n>> + if (unquote_c_style(&unquoted, line.buf, NULL))\n> \n> unquote_c_style() is documented as appending to its strbuf, but you\n> handle that via the strbuf_reset() on the line before.\n> \n>> + die(_(\"unable to unquote C-style string '%s'\"),\n>> + line.buf);\n>> +\n>> + path = unquoted.buf;\n>> + }\n>> +\n>> + if (path_in_sparse_checkout(path, the_repository->index))\n>> + write_name_quoted(path, stdout, line_terminator);\n>> + }\n> \n> Do you need to call strbuf_release() for both line and unquoted here?\n> I think you might have a small leak.\n> \n\nI think you are right. I'll make sure to do that.\n\n>> +\n>> + return 0;\n>> +}\n>> +\n>> +static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix)\n>> +{\n>> + static struct option builtin_sparse_checkout_check_rules_options[] = {\n>> + OPT_BOOL('z', NULL, &check_rules_opts.null_termination,\n>> + N_(\"terminate input and output files by a NUL character\")),\n>> + OPT_BOOL(0, \"cone\", &check_rules_opts.cone_mode,\n>> + N_(\"when used with --rules-file interpret patterns as cone mode patterns\")),\n>> + OPT_FILENAME(0, \"rules-file\", &check_rules_opts.rules_file,\n>> + N_(\"use patterns in instead of the current ones.\")),\n>> + OPT_END(),\n>> + };\n>> +\n>> + FILE *fp;\n>> + int ret;\n>> + struct pattern_list pl = {0};\n>> + char *sparse_filename;\n>> + check_rules_opts.cone_mode = -1;\n>> +\n>> + argc = parse_options(argc, argv, prefix,\n>> + builtin_sparse_checkout_check_rules_options,\n>> + builtin_sparse_checkout_check_rules_usage,\n>> + PARSE_OPT_KEEP_UNKNOWN_OPT);\n>> +\n>> + update_cone_mode(&check_rules_opts.cone_mode);\n>> + pl.use_cone_patterns = core_sparse_checkout_cone;\n>> + if (check_rules_opts.rules_file) {\n>> + fp = xfopen(check_rules_opts.rules_file, \"r\");\n>> + add_patterns_from_input(&pl, argc, argv, fp);\n>> + fclose(fp);\n>> + } else {\n>> + sparse_filename = get_sparse_checkout_filename();\n>> + if (add_patterns_from_file_to_list(sparse_filename, \"\", 0, &pl,\n>> + NULL, 0))\n>> + die(_(\"unable to load existing sparse-checkout patterns\"));\n>> + free(sparse_filename);\n>> + }\n>> +\n>> + ret = check_rules(&pl, check_rules_opts.null_termination);\n>> + clear_pattern_list(&pl);\n>> + return ret;\n>> +}\n>> +\n>> int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n>> {\n>> parse_opt_subcommand_fn *fn = NULL;\n>> @@ -939,6 +1024,7 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n>> OPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n>> OPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n>> OPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n>> + OPT_SUBCOMMAND(\"check-rules\", &fn, sparse_checkout_check_rules),\n>> OPT_END(),\n>> };\n>>\n>> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n>> index 7216267aec7..521dc914fa7 100755\n>> --- a/t/t1091-sparse-checkout-builtin.sh\n>> +++ b/t/t1091-sparse-checkout-builtin.sh\n>> @@ -555,7 +555,7 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '\n>> check_files repo a folder1\n>> '\n>>\n>> -test_expect_success 'interaction with submodules' '\n>> +test_expect_success 'setup submodules' '\n>> git clone repo super &&\n>> (\n>> cd super &&\n>> @@ -566,11 +566,22 @@ test_expect_success 'interaction with submodules' '\n>> git commit -m \"add submodule\" &&\n>> git sparse-checkout init --cone &&\n>> git sparse-checkout set folder1\n>> - ) &&\n>> + )\n>> +'\n>> +\n>> +test_expect_success 'interaction with submodules' '\n>> check_files super a folder1 modules &&\n>> check_files super/modules/child a deep folder1 folder2\n>> '\n>>\n>> +test_expect_success 'check-rules interaction with submodules' '\n>> + git -C super ls-tree --name-only -r HEAD >all-files &&\n>> + git -C super sparse-checkout check-rules >check-rules-matches > +\n>> + test_i18ngrep ! \"modules/\" check-rules-matches &&\n>> + test_i18ngrep \"folder1/\" check-rules-matches\n> \n> This file already uses test_i18ngrep, so continuing to use it is okay\n> just for consistency. But in general test_i18ngrep is deprecated\n> (\"Should not be used and will be removed soon.\" according to the docs;\n> soon is relative) and we prefer to just use grep.\n> \n\nAh. Thanks. I didn't know. I'll leave it for consistency.\n\n>> +'\n>> +\n>> test_expect_success 'different sparse-checkouts with worktrees' '\n>> git -C repo sparse-checkout set --cone deep folder1 &&\n>> git -C repo worktree add --detach ../worktree &&\n>> @@ -915,4 +926,118 @@ test_expect_success 'disable fails outside work tree' '\n>> test_i18ngrep \"this operation must be run in a work tree\" err\n>> '\n>>\n>> +test_expect_success 'setup clean' '\n>> + git -C repo clean -fdx\n>> +'\n>> +\n>> +test_expect_success 'check-rules cone mode' '\n>> + cat >rules <<-\\EOF &&\n>> + folder1\n>> + deep/deeper1/deepest\n>> + EOF\n>> +\n>> + git -C bare ls-tree -r --name-only HEAD >all-files &&\n>> + git -C bare sparse-checkout check-rules --cone \\\n>> + --rules-file ../rules >check-rules-file > +\n>> + git -C repo sparse-checkout set --cone --stdin > + git -C repo ls-files -t >out &&\n>> + sed -n \"/^S /!s/^. //p\" out >ls-files &&\n>> +\n>> + git -C repo sparse-checkout check-rules >check-rules-default > +\n>> + test_i18ngrep \"deep/deeper1/deepest/a\" check-rules-file &&\n>> + test_i18ngrep ! \"deep/deeper2\" check-rules-file &&\n>> +\n>> + test_cmp check-rules-file ls-files &&\n>> + test_cmp check-rules-file check-rules-default\n>> +'\n>> +\n>> +test_expect_success 'check-rules non-cone mode' '\n>> + cat >rules <<-\\EOF &&\n>> + deep/deeper1/deepest/a\n>> + EOF\n>> +\n>> + git -C bare ls-tree -r --name-only HEAD >all-files &&\n>> + git -C bare sparse-checkout check-rules --no-cone --rules-file ../rules\\\n>> + >check-rules-file > +\n>> + cat rules | git -C repo sparse-checkout set --no-cone --stdin &&\n>> + git -C repo ls-files -t >out &&\n>> + sed -n \"/^S /!s/^. //p\" out >ls-files &&\n>> +\n>> + git -C repo sparse-checkout check-rules >check-rules-default > +\n>> + cat >expect <<-\\EOF &&\n>> + deep/deeper1/deepest/a\n>> + EOF\n>> +\n>> + test_cmp expect check-rules-file &&\n>> + test_cmp check-rules-file ls-files &&\n>> + test_cmp check-rules-file check-rules-default\n>> +'\n>> +\n>> +test_expect_success 'check-rules cone mode is default' '\n>> + cat >rules <<-\\EOF &&\n>> + folder1\n>> + EOF\n>> +\n>> + cat >all-files <<-\\EOF &&\n>> + toplevel\n>> + folder2/file\n>> + folder1/file\n>> + EOF\n>> +\n>> + cat >expect <<-\\EOF &&\n>> + toplevel\n>> + folder1/file\n>> + EOF\n>> +\n>> + git -C bare sparse-checkout check-rules \\\n>> + --rules-file ../rules >actual > +\n>> + test_cmp expect actual\n> \n> This test is checking that despite whatever sparse-checkout setting we\n> have, that the --rules-files option makes it ignore those rules and\n> use the specified one instead, and in particular that cone mode is\n> assumed when nothing is specified with --rules-file. The test is\n> stronger because the test right before this one did a non-cone-mode\n> checkout. But since this test didn't itself do a non-cone-mode\n> checkout, that extra strength of the test could be lost by a future\n> contributor inserting another test between these. It's not a big\n> deal, but perhaps it'd make sense to have this test add a `git\n> sparse-checkout set --no-cone ...` call near the beginning?\n> \n\nThat's a good point. I'll add that.\n\n>> +'\n>> +\n>> +test_expect_success 'check-rules quoting' '\n>> + cat >rules <<-EOF &&\n>> + \"folder\\\" a\"\n>> + EOF\n>> + cat >files <<-EOF &&\n>> + \"folder\\\" a/file\"\n>> + \"folder\\\" b/file\"\n>> + EOF\n>> + cat >expect <<-EOF &&\n>> + \"folder\\\" a/file\"\n>> + EOF\n>> + git sparse-checkout check-rules --cone \\\n>> + --rules-file rules >actual > +\n>> + test_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'check-rules null termination' '\n>> + cat >rules <<-EOF &&\n>> + \"folder\\\" a\"\n>> + EOF\n>> +\n>> + lf_to_nul >files <<-EOF &&\n>> + folder\" a/a\n>> + folder\" a/b\n>> + folder\" b/fileQ\n>> + EOF\n>> +\n>> + cat >expect <<-EOF &&\n>> + folder\" a/aQfolder\" a/bQ\n>> + EOF\n>> +\n>> + git sparse-checkout check-rules --cone -z \\\n>> + --rules-file rules >actual.nul > + nul_to_q actual &&\n>> + echo >>actual &&\n>> +\n>> + test_cmp expect actual\n>> +'\n>> +\n>> +\n>> test_done\n>> --\n>> gitgitgadget\n> \n> Overall, nicely done. I only found minor things to comment on, and\n> like the overall design, explanation, and implementation.\n\nThanks for taking the time to review.\n"},{"id":"474217","messageId":"pull.1488.v2.git.1679903703.gitgitgadget@gmail.com","threadId":"59359","inReplyTo":"pull.1488.git.1678283349.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] builtin/sparse-checkout: add check-rules command","fromName":"William Sprent via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-27T07:55:01Z","receivedAt":"2023-03-27T07:55:10Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"Hi\n\nThis v2 addresses comments from Elijah's review comments.\n\nThere's one thing worth highlighting. Elijah pointed out that the the\n\"check-rules cone mode is default\" test would be stronger if the test itself\nstarted with a 'git sparse-checkout set --no-cone' to explicitly test that\nthe default interpretation of the rules passed with the '--rules-file'\noption is cone mode even though the current checkout is non-cone. I\nimplemented this and it exposed that the option did not actually behave that\nway, and that the test only verified the default behaviour of a bare\nrepository.\n\nI've modified the logic of the '--rules-file' option such that it defaults\nto cone mode unless combined with '--no-cone', and I've added a line to the\ndocumentation to make this more explicit.\n\nThe alternative would be to have '--rules-file' assume non cone mode when in\na non cone mode checkout, but I think this depends a bit on \"how deprecated\"\nnon-cone mode is vs. how important it is to have the option behave\nconsistently with 'sparse-checkout set' (which respects the current\ncheckout).\n\nChanges since v1:\n\n * Explicitly state in documentation that '--rules-file' expects newline\n separated rules.\n * Explicitly state in documentation that '-z' does not affect the\n '--rules-file' input.\n * Fixup typo where 'When called with the --rules-file flag' was\n missing \"flag\".\n * Fixup behaviour in 'check-rules --rules-file', such that it defaults to\n accepting cone mode patterns when in a non cone checkout.\n * Remember to release string buffers in 'check_rules()'.\n * Explicitly state in documentation that '--rules-file' defaults to cone\n mode unless combined with '--no-cone'.\n * Better test that the default of '--rules-file' is to expect '--cone-mode'\n by running 'check-rules' in a non-cone mode checkout.\n\nWilliam Sprent (2):\n builtin/sparse-checkout: remove NEED_WORK_TREE flag\n builtin/sparse-checkout: add check-rules command\n\n Documentation/git-sparse-checkout.txt | 25 +++-\n builtin/sparse-checkout.c | 137 ++++++++++++++++++---\n git.c | 2 +-\n t/t1091-sparse-checkout-builtin.sh | 167 +++++++++++++++++++++++++-\n 4 files changed, 307 insertions(+), 24 deletions(-)\n\n\nbase-commit: d15644fe0226af7ffc874572d968598564a230dd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1488%2Fwilliams-unity%2Fsparse-doodle-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1488/williams-unity/sparse-doodle-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1488\n\nRange-diff vs v1:\n\n 1: 4b231e9beb4 = 1: 4b231e9beb4 builtin/sparse-checkout: remove NEED_WORK_TREE flag\n 2: 21c8375efff ! 2: ef6e5b4d786 builtin/sparse-checkout: add check-rules command\n @@ Documentation/git-sparse-checkout.txt: paths to pass to a subsequent 'set' or 'a\n +By default `check-rules` reads a list of paths from stdin and outputs only\n +the ones that match the current sparsity rules. The input is expected to consist\n +of one path per line, matching the output of `git ls-tree --name-only` including\n -+that pathnames that begin with a double quote (\") are interpreted C-style quoted\n -+strings.\n ++that pathnames that begin with a double quote (\") are interpreted as C-style\n ++quoted strings.\n ++\n -+When called with the `--rules-file ` the input files are matched against\n -+the sparse checkout rules found in `` instead of the current ones. The\n -+rules in the files are expected to be in the same form as accepted by `git\n -+sparse-checkout set --stdin`.\n ++When called with the `--rules-file ` flag the input files are matched\n ++against the sparse checkout rules found in `` instead of the current ones.\n ++The rules in the files are expected to be in the same form as accepted by `git\n ++sparse-checkout set --stdin` (in particular, they must be newline-delimited).\n ++\n -+The `--rules-file` flag can be combined with the `--[no]-cone` with the same\n -+effect as for the `set` command with the `--stdin` flag.\n ++By default, the rules passed to the `--rules-file` option are interpreted as\n ++cone mode directories. To pass non-cone mode patterns with `--rules-file`,\n ++combine the option with the `--no-cone` option.\n ++\n -+When called with the `-z` flag the input format and output format is \\0\n -+terminated and not quoted.\n ++When called with the `-z` flag, the format of the paths input on stdin as well\n ++as the output paths are \\0 terminated and not quoted. Note that this does not\n ++apply to the format of the rules passed with the `--rules-file` option.\n +\n +\n EXAMPLES\n @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch\n +\t\tif (path_in_sparse_checkout(path, the_repository->index))\n +\t\t\twrite_name_quoted(path, stdout, line_terminator);\n +\t}\n ++\tstrbuf_release(&line);\n ++\tstrbuf_release(&unquoted);\n +\n +\treturn 0;\n +}\n @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch\n +\t\t\t builtin_sparse_checkout_check_rules_usage,\n +\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT);\n +\n ++\tif (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)\n ++\t\tcheck_rules_opts.cone_mode = 1;\n ++\n +\tupdate_cone_mode(&check_rules_opts.cone_mode);\n +\tpl.use_cone_patterns = core_sparse_checkout_cone;\n +\tif (check_rules_opts.rules_file) {\n @@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'disable fails outside w\n +\tfolder1/file\n +\tEOF\n +\n -+\tgit -C bare sparse-checkout check-rules \\\n ++\tgit -C repo sparse-checkout set --no-cone &&\n ++\tgit -C repo sparse-checkout check-rules \\\n +\t\t--rules-file ../rules >actual actual-bare \n\nIn preparation for adding a sub-command to 'sparse-checkout' that can be\nrun in a bare repository, remove the 'NEED_WORK_TREE' flag from its\nentry in the 'commands' array of 'git.c'.\n\nTo avoid that this changes any behaviour, add calls to\n'setup_work_tree()' to all of the 'sparse-checkout' sub-commands and add\ntests that verify that 'sparse-checkout ' still fail with a clear\nerror message telling the user that the command needs a work tree.\n\nSigned-off-by: William Sprent \n---\n builtin/sparse-checkout.c | 6 ++++++\n git.c | 2 +-\n t/t1091-sparse-checkout-builtin.sh | 33 ++++++++++++++++++++++++++++++\n 3 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex c3738154918..5fdc3d9aab5 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -57,6 +57,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)\n \tchar *sparse_filename;\n \tint res;\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"this worktree is not sparse\"));\n \n@@ -448,6 +449,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \trepo_read_index(the_repository);\n \n \tinit_opts.cone_mode = -1;\n@@ -760,6 +762,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"no sparse-checkout to add to\"));\n \n@@ -806,6 +809,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \trepo_read_index(the_repository);\n \n \tset_opts.cone_mode = -1;\n@@ -855,6 +859,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,\n \t\tOPT_END(),\n \t};\n \n+\tsetup_work_tree();\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"must be in a sparse-checkout to reapply sparsity patterns\"));\n \n@@ -898,6 +903,7 @@ static int sparse_checkout_disable(int argc, const char **argv,\n \t * forcibly return to a dense checkout regardless of initial state.\n \t */\n \n+\tsetup_work_tree();\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_sparse_checkout_disable_options,\n \t\t\t builtin_sparse_checkout_disable_usage, 0);\ndiff --git a/git.c b/git.c\nindex 6171fd6769d..5adc835cf10 100644\n--- a/git.c\n+++ b/git.c\n@@ -583,7 +583,7 @@ static struct cmd_struct commands[] = {\n \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t{ \"show-index\", cmd_show_index, RUN_SETUP_GENTLY },\n \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n-\t{ \"sparse-checkout\", cmd_sparse_checkout, RUN_SETUP | NEED_WORK_TREE },\n+\t{ \"sparse-checkout\", cmd_sparse_checkout, RUN_SETUP },\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stash\", cmd_stash, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 627267be153..7216267aec7 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -882,4 +882,37 @@ test_expect_success 'by default, non-cone mode will warn on individual files' '\n \tgrep \"pass a leading slash before paths.*if you want a single file\" warning\n '\n \n+test_expect_success 'setup bare repo' '\n+\tgit clone --bare \"file://$(pwd)/repo\" bare\n+'\n+test_expect_success 'list fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout list 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'add fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout add deeper 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'set fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout set deeper 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'init fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout init 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'reapply fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout reapply 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n+test_expect_success 'disable fails outside work tree' '\n+\ttest_must_fail git -C bare sparse-checkout disable 2>err &&\n+\ttest_i18ngrep \"this operation must be run in a work tree\" err\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"474219","messageId":"ef6e5b4d786933b5c94cf82e63719ce4e88fbbba.1679903703.git.gitgitgadget@gmail.com","threadId":"59359","inReplyTo":"pull.1488.v2.git.1679903703.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] builtin/sparse-checkout: add check-rules command","fromName":"William Sprent via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-27T07:55:03Z","receivedAt":"2023-03-27T07:55:14Z","isPatch":true,"sender":{"key":"williams@unity3d.com","avatar":"https://avatars.githubusercontent.com/u/77490534?v=4"},"body":"From: William Sprent \n\nThere exists no direct way to interrogate git about which paths are\nmatched by a given set of sparsity rules. It is possible to get this\ninformation from git, but it includes checking out the commit that\ncontains the paths, applying the sparse checkout patterns and then using\nsomething like 'git ls-files -t' to check if the skip worktree bit is\nset. This works in some case, but there are cases where it is awkward or\ninfeasible to generate a checkout for this purpose.\n\nExposing the pattern matching of sparse checkout enables more tooling to\nbe built and avoids a situation where tools that want to reason about\nsparse checkouts start containing parallel implementation of the rules.\nTo accommodate this, add a 'check-rules' subcommand to the\n'sparse-checkout' builtin along the lines of the 'git check-ignore' and\n'git check-attr' commands. The new command accepts a list of paths on\nstdin and outputs just the ones the match the sparse checkout.\n\nTo allow for use in a bare repository and to allow for interrogating\nabout other patterns than the current ones, include a '--rules-file'\noption which allows the caller to explicitly pass sparse checkout rules\nin the format accepted by 'sparse-checkout set --stdin'.\n\nTo allow for reuse of the handling of input patterns for the\n'--rules-file' flag, modify 'add_patterns_from_input()' to be able to\nread from a 'FILE' instead of just stdin.\n\nTo allow for reuse of the logic which decides whether or not rules\nshould be interpreted as cone-mode patterns, split that part out of\n'update_modes()' such that can be called without modifying the config.\n\nAn alternative could have been to create a new 'check-sparsity' command.\nHowever, placing it under 'sparse-checkout' allows for a) more easily\nre-using the sparse checkout pattern matching and cone/non-code mode\nhandling, and b) keeps the documentation for the command next to the\nexperimental warning and the cone-mode discussion.\n\nSigned-off-by: William Sprent \n---\n Documentation/git-sparse-checkout.txt | 25 ++++-\n builtin/sparse-checkout.c | 131 +++++++++++++++++++++----\n t/t1091-sparse-checkout-builtin.sh | 134 +++++++++++++++++++++++++-\n 3 files changed, 267 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex 68392d2a56e..53dc17aa77a 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -9,7 +9,7 @@ git-sparse-checkout - Reduce your working tree to a subset of tracked files\n SYNOPSIS\n --------\n [verse]\n-'git sparse-checkout' (init | list | set | add | reapply | disable) []\n+'git sparse-checkout' (init | list | set | add | reapply | disable | check-rules) []\n \n \n DESCRIPTION\n@@ -135,6 +135,29 @@ paths to pass to a subsequent 'set' or 'add' command. However,\n the disable command, so the easy restore of calling a plain `init`\n decreased in utility.\n \n+'check-rules'::\n+\tCheck whether sparsity rules match one or more paths.\n++\n+By default `check-rules` reads a list of paths from stdin and outputs only\n+the ones that match the current sparsity rules. The input is expected to consist\n+of one path per line, matching the output of `git ls-tree --name-only` including\n+that pathnames that begin with a double quote (\") are interpreted as C-style\n+quoted strings.\n++\n+When called with the `--rules-file ` flag the input files are matched\n+against the sparse checkout rules found in `` instead of the current ones.\n+The rules in the files are expected to be in the same form as accepted by `git\n+sparse-checkout set --stdin` (in particular, they must be newline-delimited).\n++\n+By default, the rules passed to the `--rules-file` option are interpreted as\n+cone mode directories. To pass non-cone mode patterns with `--rules-file`,\n+combine the option with the `--no-cone` option.\n++\n+When called with the `-z` flag, the format of the paths input on stdin as well\n+as the output paths are \\0 terminated and not quoted. Note that this does not\n+apply to the format of the rules passed with the `--rules-file` option.\n+\n+\n EXAMPLES\n --------\n `git sparse-checkout set MY/DIR1 SUB/DIR2`::\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 5fdc3d9aab5..8243d866817 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -20,7 +20,7 @@\n static const char *empty_base = \"\";\n \n static char const * const builtin_sparse_checkout_usage[] = {\n-\tN_(\"git sparse-checkout (init | list | set | add | reapply | disable) []\"),\n+\tN_(\"git sparse-checkout (init | list | set | add | reapply | disable | check-rules) []\"),\n \tNULL\n };\n \n@@ -384,13 +384,7 @@ static int set_config(enum sparse_checkout_mode mode)\n \treturn 0;\n }\n \n-static int update_modes(int *cone_mode, int *sparse_index)\n-{\n-\tint mode, record_mode;\n-\n-\t/* Determine if we need to record the mode; ensure sparse checkout on */\n-\trecord_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n-\n+static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {\n \t/* If not specified, use previous definition of cone mode */\n \tif (*cone_mode == -1 && core_apply_sparse_checkout)\n \t\t*cone_mode = core_sparse_checkout_cone;\n@@ -398,12 +392,21 @@ static int update_modes(int *cone_mode, int *sparse_index)\n \t/* Set cone/non-cone mode appropriately */\n \tcore_apply_sparse_checkout = 1;\n \tif (*cone_mode == 1 || *cone_mode == -1) {\n-\t\tmode = MODE_CONE_PATTERNS;\n \t\tcore_sparse_checkout_cone = 1;\n-\t} else {\n-\t\tmode = MODE_ALL_PATTERNS;\n-\t\tcore_sparse_checkout_cone = 0;\n+\t\treturn MODE_CONE_PATTERNS;\n \t}\n+\tcore_sparse_checkout_cone = 0;\n+\treturn MODE_ALL_PATTERNS;\n+}\n+\n+static int update_modes(int *cone_mode, int *sparse_index)\n+{\n+\tint mode, record_mode;\n+\n+\t/* Determine if we need to record the mode; ensure sparse checkout on */\n+\trecord_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;\n+\n+\tmode = update_cone_mode(cone_mode);\n \tif (record_mode && set_config(mode))\n \t\treturn 1;\n \n@@ -547,7 +550,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n \n static void add_patterns_from_input(struct pattern_list *pl,\n \t\t\t\t int argc, const char **argv,\n-\t\t\t\t int use_stdin)\n+\t\t\t\t FILE *file)\n {\n \tint i;\n \tif (core_sparse_checkout_cone) {\n@@ -557,9 +560,9 @@ static void add_patterns_from_input(struct pattern_list *pl,\n \t\thashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);\n \t\tpl->use_cone_patterns = 1;\n \n-\t\tif (use_stdin) {\n+\t\tif (file) {\n \t\t\tstruct strbuf unquoted = STRBUF_INIT;\n-\t\t\twhile (!strbuf_getline(&line, stdin)) {\n+\t\t\twhile (!strbuf_getline(&line, file)) {\n \t\t\t\tif (line.buf[0] == '\"') {\n \t\t\t\t\tstrbuf_reset(&unquoted);\n \t\t\t\t\tif (unquote_c_style(&unquoted, line.buf, NULL))\n@@ -581,10 +584,10 @@ static void add_patterns_from_input(struct pattern_list *pl,\n \t\t\t}\n \t\t}\n \t} else {\n-\t\tif (use_stdin) {\n+\t\tif (file) {\n \t\t\tstruct strbuf line = STRBUF_INIT;\n \n-\t\t\twhile (!strbuf_getline(&line, stdin)) {\n+\t\t\twhile (!strbuf_getline(&line, file)) {\n \t\t\t\tsize_t len;\n \t\t\t\tchar *buf = strbuf_detach(&line, &len);\n \t\t\t\tadd_pattern(buf, empty_base, 0, pl, 0);\n@@ -611,7 +614,8 @@ static void add_patterns_cone_mode(int argc, const char **argv,\n \tstruct pattern_list existing;\n \tchar *sparse_filename = get_sparse_checkout_filename();\n \n-\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\tadd_patterns_from_input(pl, argc, argv,\n+\t\t\t\tuse_stdin ? stdin : NULL);\n \n \tmemset(&existing, 0, sizeof(existing));\n \texisting.use_cone_patterns = core_sparse_checkout_cone;\n@@ -648,7 +652,7 @@ static void add_patterns_literal(int argc, const char **argv,\n \t\t\t\t\t pl, NULL, 0))\n \t\tdie(_(\"unable to load existing sparse-checkout patterns\"));\n \tfree(sparse_filename);\n-\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\tadd_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);\n }\n \n static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n@@ -667,7 +671,8 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,\n \t\tbreak;\n \n \tcase REPLACE:\n-\t\tadd_patterns_from_input(pl, argc, argv, use_stdin);\n+\t\tadd_patterns_from_input(pl, argc, argv,\n+\t\t\t\t\tuse_stdin ? stdin : NULL);\n \t\tbreak;\n \t}\n \n@@ -929,6 +934,91 @@ static int sparse_checkout_disable(int argc, const char **argv,\n \treturn set_config(MODE_NO_PATTERNS);\n }\n \n+static char const * const builtin_sparse_checkout_check_rules_usage[] = {\n+\tN_(\"git sparse-checkout check-rules [-z] [--skip-checks]\"\n+\t \"[--[no-]cone] [--rules-file ]\"),\n+\tNULL\n+};\n+\n+static struct sparse_checkout_check_rules_opts {\n+\tint cone_mode;\n+\tint null_termination;\n+\tchar *rules_file;\n+} check_rules_opts;\n+\n+static int check_rules(struct pattern_list *pl, int null_terminated) {\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tchar *path;\n+\tint line_terminator = null_terminated ? 0 : '\\n';\n+\tstrbuf_getline_fn getline_fn = null_terminated ? strbuf_getline_nul\n+\t\t: strbuf_getline;\n+\tthe_repository->index->sparse_checkout_patterns = pl;\n+\twhile (!getline_fn(&line, stdin)) {\n+\t\tpath = line.buf;\n+\t\tif (!null_terminated && line.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, line.buf, NULL))\n+\t\t\t\tdie(_(\"unable to unquote C-style string '%s'\"),\n+\t\t\t\t\tline.buf);\n+\n+\t\t\tpath = unquoted.buf;\n+\t\t}\n+\n+\t\tif (path_in_sparse_checkout(path, the_repository->index))\n+\t\t\twrite_name_quoted(path, stdout, line_terminator);\n+\t}\n+\tstrbuf_release(&line);\n+\tstrbuf_release(&unquoted);\n+\n+\treturn 0;\n+}\n+\n+static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix)\n+{\n+\tstatic struct option builtin_sparse_checkout_check_rules_options[] = {\n+\t\tOPT_BOOL('z', NULL, &check_rules_opts.null_termination,\n+\t\t\t N_(\"terminate input and output files by a NUL character\")),\n+\t\tOPT_BOOL(0, \"cone\", &check_rules_opts.cone_mode,\n+\t\t\t N_(\"when used with --rules-file interpret patterns as cone mode patterns\")),\n+\t\tOPT_FILENAME(0, \"rules-file\", &check_rules_opts.rules_file,\n+\t\t\t N_(\"use patterns in instead of the current ones.\")),\n+\t\tOPT_END(),\n+\t};\n+\n+\tFILE *fp;\n+\tint ret;\n+\tstruct pattern_list pl = {0};\n+\tchar *sparse_filename;\n+\tcheck_rules_opts.cone_mode = -1;\n+\n+\targc = parse_options(argc, argv, prefix,\n+\t\t\t builtin_sparse_checkout_check_rules_options,\n+\t\t\t builtin_sparse_checkout_check_rules_usage,\n+\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT);\n+\n+\tif (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)\n+\t\tcheck_rules_opts.cone_mode = 1;\n+\n+\tupdate_cone_mode(&check_rules_opts.cone_mode);\n+\tpl.use_cone_patterns = core_sparse_checkout_cone;\n+\tif (check_rules_opts.rules_file) {\n+\t\tfp = xfopen(check_rules_opts.rules_file, \"r\");\n+\t\tadd_patterns_from_input(&pl, argc, argv, fp);\n+\t\tfclose(fp);\n+\t} else {\n+\t\tsparse_filename = get_sparse_checkout_filename();\n+\t\tif (add_patterns_from_file_to_list(sparse_filename, \"\", 0, &pl,\n+\t\t\t\t\t\t NULL, 0))\n+\t\t\tdie(_(\"unable to load existing sparse-checkout patterns\"));\n+\t\tfree(sparse_filename);\n+\t}\n+\n+\tret = check_rules(&pl, check_rules_opts.null_termination);\n+\tclear_pattern_list(&pl);\n+\treturn ret;\n+}\n+\n int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n {\n \tparse_opt_subcommand_fn *fn = NULL;\n@@ -939,6 +1029,7 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n \t\tOPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n \t\tOPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n+\t\tOPT_SUBCOMMAND(\"check-rules\", &fn, sparse_checkout_check_rules),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 7216267aec7..9ceb17f9118 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -555,7 +555,7 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '\n \tcheck_files repo a folder1\n '\n \n-test_expect_success 'interaction with submodules' '\n+test_expect_success 'setup submodules' '\n \tgit clone repo super &&\n \t(\n \t\tcd super &&\n@@ -566,11 +566,22 @@ test_expect_success 'interaction with submodules' '\n \t\tgit commit -m \"add submodule\" &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set folder1\n-\t) &&\n+\t)\n+'\n+\n+test_expect_success 'interaction with submodules' '\n \tcheck_files super a folder1 modules &&\n \tcheck_files super/modules/child a deep folder1 folder2\n '\n \n+test_expect_success 'check-rules interaction with submodules' '\n+\tgit -C super ls-tree --name-only -r HEAD >all-files &&\n+\tgit -C super sparse-checkout check-rules >check-rules-matches rules <<-\\EOF &&\n+\tfolder1\n+\tdeep/deeper1/deepest\n+\tEOF\n+\n+\tgit -C bare ls-tree -r --name-only HEAD >all-files &&\n+\tgit -C bare sparse-checkout check-rules --cone \\\n+\t\t--rules-file ../rules >check-rules-file out &&\n+\tsed -n \"/^S /!s/^. //p\" out >ls-files &&\n+\n+\tgit -C repo sparse-checkout check-rules >check-rules-default rules <<-\\EOF &&\n+\tdeep/deeper1/deepest/a\n+\tEOF\n+\n+\tgit -C bare ls-tree -r --name-only HEAD >all-files &&\n+\tgit -C bare sparse-checkout check-rules --no-cone --rules-file ../rules\\\n+\t\t>check-rules-file out &&\n+\tsed -n \"/^S /!s/^. //p\" out >ls-files &&\n+\n+\tgit -C repo sparse-checkout check-rules >check-rules-default expect <<-\\EOF &&\n+\tdeep/deeper1/deepest/a\n+\tEOF\n+\n+\ttest_cmp expect check-rules-file &&\n+\ttest_cmp check-rules-file ls-files &&\n+\ttest_cmp check-rules-file check-rules-default\n+'\n+\n+test_expect_success 'check-rules cone mode is default' '\n+\tcat >rules <<-\\EOF &&\n+\tfolder1\n+\tEOF\n+\n+\tcat >all-files <<-\\EOF &&\n+\ttoplevel\n+\tfolder2/file\n+\tfolder1/file\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\ttoplevel\n+\tfolder1/file\n+\tEOF\n+\n+\tgit -C repo sparse-checkout set --no-cone &&\n+\tgit -C repo sparse-checkout check-rules \\\n+\t\t--rules-file ../rules >actual actual-bare rules <<-EOF &&\n+\t\"folder\\\" a\"\n+\tEOF\n+\tcat >files <<-EOF &&\n+\t\"folder\\\" a/file\"\n+\t\"folder\\\" b/file\"\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\t\"folder\\\" a/file\"\n+\tEOF\n+\tgit sparse-checkout check-rules --cone \\\n+\t\t--rules-file rules >actual rules <<-EOF &&\n+\t\"folder\\\" a\"\n+\tEOF\n+\n+\tlf_to_nul >files <<-EOF &&\n+\tfolder\" a/a\n+\tfolder\" a/b\n+\tfolder\" b/fileQ\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tfolder\" a/aQfolder\" a/bQ\n+\tEOF\n+\n+\tgit sparse-checkout check-rules --cone -z \\\n+\t\t--rules-file rules >actual.nul actual &&\n+\techo >>actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"474250","messageId":"xmqqr0taaxrr.fsf@gitster.g","threadId":"59359","inReplyTo":"4b231e9beb43e4fac6457b9bf86e4c1db39c4238.1679903703.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-27T17:51:04Z","receivedAt":"2023-03-27T17:51:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Sprent via GitGitGadget\" writes:\n\n> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> index c3738154918..5fdc3d9aab5 100644\n> --- a/builtin/sparse-checkout.c\n> +++ b/builtin/sparse-checkout.c\n> @@ -57,6 +57,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)\n> \tchar *sparse_filename;\n> \tint res;\n> \n> +\tsetup_work_tree();\n> \tif (!core_apply_sparse_checkout)\n> \t\tdie(_(\"this worktree is not sparse\"));\n> ...\n> @@ -898,6 +903,7 @@ static int sparse_checkout_disable(int argc, const char **argv,\n> \t * forcibly return to a dense checkout regardless of initial state.\n> \t */\n> \n> +\tsetup_work_tree();\n> \targc = parse_options(argc, argv, prefix,\n> \t\t\t builtin_sparse_checkout_disable_options,\n> \t\t\t builtin_sparse_checkout_disable_usage, 0);\n\nI am throwing this out not as \"we must tackle this ugliness before\nthis patch can proceed\" but as \"this is ugly, I wish somebody can\nfigure it out in a cleaner way\", so do not take this as a blocking\ncomment or objection, but more as something to think about improving\nif possible as a bonus point.\n\nIt really is a shame that we have a nice \"dispatch\" table at the\nbeginning of the single caller:\n\n int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n {\n parse_opt_subcommand_fn *fn = NULL;\n struct option builtin_sparse_checkout_options[] = {\n OPT_SUBCOMMAND(\"list\", &fn, sparse_checkout_list),\n OPT_SUBCOMMAND(\"init\", &fn, sparse_checkout_init),\n OPT_SUBCOMMAND(\"set\", &fn, sparse_checkout_set),\n OPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n OPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n OPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n OPT_END(),\n };\n\nyet we have to sprinkle setup_work_tree() to all of these functions'\nimplementation. If we were able to describe which selected ones do\nnot need the setup call, we could let the parse-options API to look\nup the function and then before calling \"fn\" we could make the setup\ncall. That would allow us to maintain the subcommands much nicely.\n\nThanks.\n"},{"id":"474634","messageId":"CABPp-BHhjjO32EAzi_Deck6fxsyG=uMrvAnJLaD0e9qMtrcDEQ@mail.gmail.com","threadId":"59359","inReplyTo":"pull.1488.v2.git.1679903703.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] builtin/sparse-checkout: add check-rules command","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-04-01T18:49:42Z","receivedAt":"2023-04-01T18:50:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Mon, Mar 27, 2023 at 12:55 AM William Sprent via GitGitGadget\n wrote:\n>\n> Hi\n>\n> This v2 addresses comments from Elijah's review comments.\n>\n> There's one thing worth highlighting. Elijah pointed out that the the\n> \"check-rules cone mode is default\" test would be stronger if the test itself\n> started with a 'git sparse-checkout set --no-cone' to explicitly test that\n> the default interpretation of the rules passed with the '--rules-file'\n> option is cone mode even though the current checkout is non-cone. I\n> implemented this and it exposed that the option did not actually behave that\n> way, and that the test only verified the default behaviour of a bare\n> repository.\n>\n> I've modified the logic of the '--rules-file' option such that it defaults\n> to cone mode unless combined with '--no-cone', and I've added a line to the\n> documentation to make this more explicit.\n>\n> The alternative would be to have '--rules-file' assume non cone mode when in\n> a non cone mode checkout, but I think this depends a bit on \"how deprecated\"\n> non-cone mode is vs. how important it is to have the option behave\n> consistently with 'sparse-checkout set' (which respects the current\n> checkout).\n\nOoh, nice catch.\n\nGiven that the whole point of check-rules and --rules-file is to\ndetermine how a _different_ sparse checkout would behave, I agree with\nyour decision here that the current sparse checkout should not affect\nhow --rules-file functions.\n\n>\n> Changes since v1:\n>\n> * Explicitly state in documentation that '--rules-file' expects newline\n> separated rules.\n> * Explicitly state in documentation that '-z' does not affect the\n> '--rules-file' input.\n> * Fixup typo where 'When called with the --rules-file flag' was\n> missing \"flag\".\n> * Fixup behaviour in 'check-rules --rules-file', such that it defaults to\n> accepting cone mode patterns when in a non cone checkout.\n> * Remember to release string buffers in 'check_rules()'.\n> * Explicitly state in documentation that '--rules-file' defaults to cone\n> mode unless combined with '--no-cone'.\n> * Better test that the default of '--rules-file' is to expect '--cone-mode'\n> by running 'check-rules' in a non-cone mode checkout.\n\nThis round addresses all my issues, and I agree with the other change\nmade in this round. So, v2 is:\n\nReviewed-by: Elijah Newren \n\nThanks for sending this in, William!\n"},{"id":"474695","messageId":"xmqqttxw6gin.fsf@gitster.g","threadId":"59359","inReplyTo":"CABPp-BHhjjO32EAzi_Deck6fxsyG=uMrvAnJLaD0e9qMtrcDEQ@mail.gmail.com","subject":"Re: [PATCH v2 0/2] builtin/sparse-checkout: add check-rules command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-03T17:07:44Z","receivedAt":"2023-04-03T17:07:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> This round addresses all my issues, and I agree with the other change\n> made in this round. So, v2 is:\n>\n> Reviewed-by: Elijah Newren \n>\n> Thanks for sending this in, William!\n\nThanks.\n"},{"id":"475007","messageId":"20230407161655.GB3117@szeder.dev","threadId":"59359","inReplyTo":"xmqqr0taaxrr.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2023-04-07T16:16:55Z","receivedAt":"2023-04-07T16:17:02Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Mar 27, 2023 at 10:51:04AM -0700, Junio C Hamano wrote:\n> \"William Sprent via GitGitGadget\" writes:\n> \n> > diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> > index c3738154918..5fdc3d9aab5 100644\n> > --- a/builtin/sparse-checkout.c\n> > +++ b/builtin/sparse-checkout.c\n> > @@ -57,6 +57,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)\n> > \tchar *sparse_filename;\n> > \tint res;\n> > \n> > +\tsetup_work_tree();\n> > \tif (!core_apply_sparse_checkout)\n> > \t\tdie(_(\"this worktree is not sparse\"));\n> > ...\n> > @@ -898,6 +903,7 @@ static int sparse_checkout_disable(int argc, const char **argv,\n> > \t * forcibly return to a dense checkout regardless of initial state.\n> > \t */\n> > \n> > +\tsetup_work_tree();\n> > \targc = parse_options(argc, argv, prefix,\n> > \t\t\t builtin_sparse_checkout_disable_options,\n> > \t\t\t builtin_sparse_checkout_disable_usage, 0);\n> \n> I am throwing this out not as \"we must tackle this ugliness before\n> this patch can proceed\" but as \"this is ugly, I wish somebody can\n> figure it out in a cleaner way\", so do not take this as a blocking\n> comment or objection, but more as something to think about improving\n> if possible as a bonus point.\n> \n> It really is a shame that we have a nice \"dispatch\" table at the\n> beginning of the single caller:\n> \n> int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)\n> {\n> parse_opt_subcommand_fn *fn = NULL;\n> struct option builtin_sparse_checkout_options[] = {\n> OPT_SUBCOMMAND(\"list\", &fn, sparse_checkout_list),\n> OPT_SUBCOMMAND(\"init\", &fn, sparse_checkout_init),\n> OPT_SUBCOMMAND(\"set\", &fn, sparse_checkout_set),\n> OPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n> OPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n> OPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n> OPT_END(),\n> };\n> \n> yet we have to sprinkle setup_work_tree() to all of these functions'\n> implementation. If we were able to describe which selected ones do\n> not need the setup call, we could let the parse-options API to look\n> up the function and then before calling \"fn\" we could make the setup\n> call. That would allow us to maintain the subcommands much nicely.\n\nIt's easy enough to do in this particular case: there is an\nOPT_SUBCOMMAND_F() variant which takes an additional flags parameter,\nso we could add a PARSE_OPT_SETUP_WORK_TREE flag, check it in e.g.\nparse_subcommand(), and act accordingly if it's set.\n\nHowever, this wouldn't work when the command has a default operation\nmode and is invoked without any subcommands. And I'm not sure about\ndoing this in parse-options, because it's about, well, parsing\noptions, not about doing fancy setup stuff.\n\n\n"},{"id":"475009","messageId":"xmqqttxrk5qn.fsf@gitster.g","threadId":"59359","inReplyTo":"20230407161655.GB3117@szeder.dev","subject":"Re: [PATCH v2 1/2] builtin/sparse-checkout: remove NEED_WORK_TREE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-07T16:38:08Z","receivedAt":"2023-04-07T16:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> struct option builtin_sparse_checkout_options[] = {\n>> OPT_SUBCOMMAND(\"list\", &fn, sparse_checkout_list),\n>> OPT_SUBCOMMAND(\"init\", &fn, sparse_checkout_init),\n>> OPT_SUBCOMMAND(\"set\", &fn, sparse_checkout_set),\n>> OPT_SUBCOMMAND(\"add\", &fn, sparse_checkout_add),\n>> OPT_SUBCOMMAND(\"reapply\", &fn, sparse_checkout_reapply),\n>> OPT_SUBCOMMAND(\"disable\", &fn, sparse_checkout_disable),\n>> OPT_END(),\n>> };\n>> \n>> yet we have to sprinkle setup_work_tree() to all of these functions'\n>> implementation. If we were able to describe which selected ones do\n>> not need the setup call, we could let the parse-options API to look\n>> up the function and then before calling \"fn\" we could make the setup\n>> call. That would allow us to maintain the subcommands much nicely.\n>\n> It's easy enough to do in this particular case: there is an\n> OPT_SUBCOMMAND_F() variant which takes an additional flags parameter,\n> so we could add a PARSE_OPT_SETUP_WORK_TREE flag, check it in e.g.\n> parse_subcommand(), and act accordingly if it's set.\n>\n> However, this wouldn't work when the command has a default operation\n> mode and is invoked without any subcommands. And I'm not sure about\n> doing this in parse-options, because it's about, well, parsing\n> options, not about doing fancy setup stuff.\n\nYes, exactly. What I was imagining was more along the lines of\n\n\tparse_opt_subcommand_fn *fn = NULL;\n\tparse_opt_subcommand_fn *fn_with_setup = NULL;\n\toptions[] = {\n\t\tOPT_SUBCOMMAND(\"list\", &fn_with_setup, sparse_checkout_list),\n\t\t...\n\t\tOPT_SUBCOMMAND(\"check-rules\", &fn, sparse_check_rules),\n\t};\n\n\tparse_options(...);\n\n if (fn_with_setup) {\n\t\tsetup_worktree();\n\t\tfn = fn_with_setup;\n\t}\n\tfn(...);\n\nBut of course as a \"safety\" measure, one options[] array can all\npoint at the same \"fn\" variable or parse_options() becomes unhappy,\nso the above does not work out of the box.\n"}]}