{"thread":{"id":"53425","subject":"[PATCH 00/10] [RFC] In-tree sparse-checkout definitions","startedAt":"2020-05-07T13:17:48Z","lastAt":"2020-06-18T08:18:37Z","messageCount":32,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Derrick Stolee","Elijah Newren","Son Luong Ngoc"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"397296","messageId":"pull.627.git.1588857462.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":null,"subject":"[PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:32Z","receivedAt":"2020-05-07T13:17:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is based on ds/sparse-allow-empty-working-tree.\n\nBACKGROUND\n==========\n\nAfter discussing the future of sparse-checkout at the Git Contributor\nSummit, Minh Tai caught me during the break to introduce a very interesting\nidea: in-tree sparse-checkout definitions.\n\nHere is the context: I was talking about how the Office team is building a\ntool to update the sparse-checkout definition dynamically as their build\ndefinitions change. They have a very compartmentalized build system, and\ncertain \"projects\" depend on each other. To ensure that builds do not break\nas dependencies change, the plan was to have this tool run as a hook when\nHEAD changes. This is a bit expensive to do, and we were discussing how much\nwe need this to be automated versus a part of the build process.\n\nThis kind of idea would need to be replicated for every team that wants to\nuse sparse-checkout in this way. That doesn't bode well for wide adoption of\nthe feature.\n\nIN-TREE SPARSE-CHECKOUT DEFINITIONS\n===================================\n\nMinh's idea was simple: have sparse-checkout files in the working directory\nand use config to point to them. As these in-tree files update, we can\nautomatically update the sparse-checkout definition accordingly. Now, the\nonly thing to do would be to ensure that the sparse-checkout files are\nupdated when someone updates the build definitions. This requires some extra\nbuild validation, but would not require special tools built on every client.\n\nAs soon as I sat down to implement this idea, I discovered some\ncomplications to that basic idea. Each of these sparse-checkout definitions\nshould fit a \"role\" on the team. To continue with the Office analogy,\nsuppose we had a definition for developers working on each of Word,\nPowerPoint, and Excel. What happens when someone wants to port a feature\nfrom Excel to Word? That user would want to take the union of the two\nsparse-checkout definitions. But what does that mean when we are working\nwith arbitrary sparse-checkout files?\n\nThis leads to the restriction that I built into this feature: we only care\nabout \"cone mode\" definitions. These rely on directory-based prefix matches\nthat are very fast. With this restriction, it is simple to understand what\nthe \"union\" operation should do: take all directories that would be included\nby either.\n\nOnce we restrict to cone mode, there is no reason to continue storing files\nusing the sparse-checkout file format. The list of patterns is larger than\nthe input to \"git sparse-checkout set\" or output from \"git sparse-checkout\nlist\" in cone mode, and more prone to user error. Thus, let's be simpler and\nuse a list of directories to specify the sparse-checkout definition.\n\nThis leads to the second issue that complicates things. If a build\ndependency changes to a core library, but there are N \"roles\" that depend on\nthat library, then the person changing that library also needs to change and\nvalidate N sparse-checkout definitions! This does not scale well. So, let's\ntake a page from build dependencies and allow these in-tree sparse-checkout\ndefinitions to depend on each other!\n\nThe end result is a mechanism that should be flexible enough to future needs\nbut simple enough to build using existing Git features. The format re-uses\nthe config file format. This provides good re-use of code as well as being\nsomething easy to extend in the future. In Patch 4, I tried to describe\nseveral alternatives and why this is the best of those options. I'm open to\nsuggestions here.\n\nOUTLINE\n=======\n\n * Patch 1 is a bugfix that should work on its own. I caught it in tests\n after Patch 5.\n \n \n * Patches 2-7 implement the feature.\n \n \n * Patches 8-9 make the Git build system a bit more robust to missing\n directories.\n \n \n * Patch 10 adds a .sparse directory to Git as well as two in-tree\n sparse-checkout definitions. One is for a bare-bones Linux developer and\n the other adds the compat code for Windows on top.\n \n \n\nAs mentioned in patch 10, this allows the following workflow for Git\ncontributors that want to \"eat our own dogfood\" with the partial clone and\nsparse-checkout features:\n\n $ git clone --sparse --filter=blob:none https://github.com/git/git$ cd git\n$ git sparse-checkout set --in-tree .sparse/base.deps\n\nThen, we have a smaller working directory but can still build and test the\ncode.\n\nFUTURE DIRECTIONS\n=================\n\nThere are definitely ways to make this feature more seamless, or more useful\nto us.\n\n * It would be nice to extend git clone to have a string value to \n --sparse= and possibly have it multi-valued. This would initialize\n the in-tree sparse-checkout definition immediately upon cloning.\n \n \n * I think there are some ways we could reorganize the Git codebase to make\n our \"most basic\" sparse-checkout file have even fewer files. Mostly, it\n would require modifying the build to ask \"do these files exist on-disk?\n if not, then disable this build option.\" This would also require adding\n test dependencies that also disable some tests when those do not exist.\n \n \n * Currently, if we have an in-tree sparse-checkout definition, then we\n can't add more directories to it manually. If someone runs git\n sparse-checkout set --in-tree followed by git sparse-checkout add\n , then will disappear as it is overridden by the in-tree definition.\n The only way I can think to get around this would be to add these\n directories through Git config, since we were using the sparse-checkout\n file as the storage of \"what directories do I care about?\" but it is no\n longer the source of truth in this situation. I'm open to ideas here, and\n it would be nice to have this interaction hardened before moving the\n series out of RFC status.\n \n \n\nThanks, -Stolee\n\nDerrick Stolee (10):\n unpack-trees: avoid array out-of-bounds error\n sparse-checkout: move code from builtin\n sparse-checkout: move code from unpack-trees.c\n sparse-checkout: allow in-tree definitions\n sparse-checkout: automatically update in-tree definition\n sparse-checkout: use oidset to prevent repeat blobs\n sparse-checkout: define in-tree dependencies\n Makefile: skip git-gui if dir is missing\n Makefile: disable GETTEXT when 'po' is missing\n .sparse: add in-tree sparse-checkout for Git\n\n .sparse/base.deps | 19 ++\n .sparse/windows.deps | 3 +\n Documentation/config.txt | 2 +\n Documentation/config/sparse.txt | 15 +\n Documentation/git-sparse-checkout.txt | 70 ++++-\n Makefile | 19 +-\n builtin/commit.c | 4 +-\n builtin/read-tree.c | 4 +\n builtin/sparse-checkout.c | 268 +++--------------\n read-cache.c | 8 +-\n sparse-checkout.c | 418 ++++++++++++++++++++++++++\n sparse-checkout.h | 30 ++\n t/lib-gettext.sh | 1 -\n t/t0200-gettext-basic.sh | 6 +-\n t/t1091-sparse-checkout-builtin.sh | 143 +++++++++\n t/test-lib.sh | 7 +-\n unpack-trees.c | 10 +-\n 17 files changed, 785 insertions(+), 242 deletions(-)\n create mode 100644 .sparse/base.deps\n create mode 100644 .sparse/windows.deps\n create mode 100644 Documentation/config/sparse.txt\n create mode 100644 sparse-checkout.c\n create mode 100644 sparse-checkout.h\n\n\nbase-commit: ace224ac5fb120e9cae894e31713ab60e91f141f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-627%2Fderrickstolee%2Fsparse-checkout-in-tree-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-627/derrickstolee/sparse-checkout-in-tree-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/627\n-- \ngitgitgadget\n"},{"id":"397297","messageId":"5bfe3f3fc8a99b3d4fdd4286da17cd935090c614.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 01/10] unpack-trees: avoid array out-of-bounds error","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:33Z","receivedAt":"2020-05-07T13:17:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe loop in warn_conflicted_path() that checks for the count of entries\nwith the same path uses \"i+count\" for the array entry. However, the loop\nonly verifies that the value of count is below the array size. Fix this\nby adding i to the condition.\n\nI hit this condition during a test of the in-tree sparse-checkout\nfeature, so it is exercised by the end of the series.\n\nSigned-off-by: Derrick Stolee \n---\n unpack-trees.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9a3ccd9d083..4f880f2da90 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -563,10 +563,11 @@ static int warn_conflicted_path(struct index_state *istate,\n \tadd_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n \n \t/* Find out how many higher stage entries at same path */\n-\twhile (++count < istate->cache_nr &&\n+\twhile (i + ++count < istate->cache_nr &&\n \t !strcmp(conflicting_path,\n \t\t istate->cache[i+count]->name))\n \t\t/* do nothing */;\n+\n \treturn count;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"397298","messageId":"0181a134bfb6986dc0e54ae624c478446a1324a9.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 02/10] sparse-checkout: move code from builtin","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:34Z","receivedAt":"2020-05-07T13:17:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn anticipation of some new features in the sparse-checkout space,\ncreate a new sparse-checkout.c file at root to store the implementation\nof these features. Several helper methods from builtin/sparse-checkout.c\nare moved to this file.\n\nSigned-off-by: Derrick Stolee \n---\n Makefile | 1 +\n builtin/sparse-checkout.c | 225 +-----------------------------------\n sparse-checkout.c | 233 ++++++++++++++++++++++++++++++++++++++\n sparse-checkout.h | 16 +++\n 4 files changed, 251 insertions(+), 224 deletions(-)\n create mode 100644 sparse-checkout.c\n create mode 100644 sparse-checkout.h\n\ndiff --git a/Makefile b/Makefile\nindex 9804a0758b2..2e8029b8026 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -985,6 +985,7 @@ LIB_OBJS += sha1-name.o\n LIB_OBJS += shallow.o\n LIB_OBJS += sideband.o\n LIB_OBJS += sigchain.o\n+LIB_OBJS += sparse-checkout.o\n LIB_OBJS += split-index.o\n LIB_OBJS += stable-qsort.o\n LIB_OBJS += strbuf.o\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 95d08824172..fd247e428e4 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -9,11 +9,11 @@\n #include \"string-list.h\"\n #include \"cache.h\"\n #include \"cache-tree.h\"\n-#include \"lockfile.h\"\n #include \"resolve-undo.h\"\n #include \"unpack-trees.h\"\n #include \"wt-status.h\"\n #include \"quote.h\"\n+#include \"sparse-checkout.h\"\n \n static const char *empty_base = \"\";\n \n@@ -22,30 +22,6 @@ static char const * const builtin_sparse_checkout_usage[] = {\n \tNULL\n };\n \n-static char *get_sparse_checkout_filename(void)\n-{\n-\treturn git_pathdup(\"info/sparse-checkout\");\n-}\n-\n-static void write_patterns_to_file(FILE *fp, struct pattern_list *pl)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < pl->nr; i++) {\n-\t\tstruct path_pattern *p = pl->patterns[i];\n-\n-\t\tif (p->flags & PATTERN_FLAG_NEGATIVE)\n-\t\t\tfprintf(fp, \"!\");\n-\n-\t\tfprintf(fp, \"%s\", p->pattern);\n-\n-\t\tif (p->flags & PATTERN_FLAG_MUSTBEDIR)\n-\t\t\tfprintf(fp, \"/\");\n-\n-\t\tfprintf(fp, \"\\n\");\n-\t}\n-}\n-\n static int sparse_checkout_list(int argc, const char **argv)\n {\n \tstruct pattern_list pl;\n@@ -92,153 +68,6 @@ static int sparse_checkout_list(int argc, const char **argv)\n \treturn 0;\n }\n \n-static int update_working_directory(struct pattern_list *pl)\n-{\n-\tenum update_sparsity_result result;\n-\tstruct unpack_trees_options o;\n-\tstruct lock_file lock_file = LOCK_INIT;\n-\tstruct repository *r = the_repository;\n-\n-\tmemset(&o, 0, sizeof(o));\n-\to.verbose_update = isatty(2);\n-\to.update = 1;\n-\to.head_idx = -1;\n-\to.src_index = r->index;\n-\to.dst_index = r->index;\n-\to.skip_sparse_checkout = 0;\n-\to.pl = pl;\n-\n-\tsetup_work_tree();\n-\n-\trepo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);\n-\n-\tsetup_unpack_trees_porcelain(&o, \"sparse-checkout\");\n-\tresult = update_sparsity(&o);\n-\tclear_unpack_trees_porcelain(&o);\n-\n-\tif (result == UPDATE_SPARSITY_WARNINGS)\n-\t\t/*\n-\t\t * We don't do any special handling of warnings from untracked\n-\t\t * files in the way or dirty entries that can't be removed.\n-\t\t */\n-\t\tresult = UPDATE_SPARSITY_SUCCESS;\n-\tif (result == UPDATE_SPARSITY_SUCCESS)\n-\t\twrite_locked_index(r->index, &lock_file, COMMIT_LOCK);\n-\telse\n-\t\trollback_lock_file(&lock_file);\n-\n-\treturn result;\n-}\n-\n-static char *escaped_pattern(char *pattern)\n-{\n-\tchar *p = pattern;\n-\tstruct strbuf final = STRBUF_INIT;\n-\n-\twhile (*p) {\n-\t\tif (is_glob_special(*p))\n-\t\t\tstrbuf_addch(&final, '\\\\');\n-\n-\t\tstrbuf_addch(&final, *p);\n-\t\tp++;\n-\t}\n-\n-\treturn strbuf_detach(&final, NULL);\n-}\n-\n-static void write_cone_to_file(FILE *fp, struct pattern_list *pl)\n-{\n-\tint i;\n-\tstruct pattern_entry *pe;\n-\tstruct hashmap_iter iter;\n-\tstruct string_list sl = STRING_LIST_INIT_DUP;\n-\tstruct strbuf parent_pattern = STRBUF_INIT;\n-\n-\thashmap_for_each_entry(&pl->parent_hashmap, &iter, pe, ent) {\n-\t\tif (hashmap_get_entry(&pl->recursive_hashmap, pe, ent, NULL))\n-\t\t\tcontinue;\n-\n-\t\tif (!hashmap_contains_parent(&pl->recursive_hashmap,\n-\t\t\t\t\t pe->pattern,\n-\t\t\t\t\t &parent_pattern))\n-\t\t\tstring_list_insert(&sl, pe->pattern);\n-\t}\n-\n-\tstring_list_sort(&sl);\n-\tstring_list_remove_duplicates(&sl, 0);\n-\n-\tfprintf(fp, \"/*\\n!/*/\\n\");\n-\n-\tfor (i = 0; i < sl.nr; i++) {\n-\t\tchar *pattern = escaped_pattern(sl.items[i].string);\n-\n-\t\tif (strlen(pattern))\n-\t\t\tfprintf(fp, \"%s/\\n!%s/*/\\n\", pattern, pattern);\n-\t\tfree(pattern);\n-\t}\n-\n-\tstring_list_clear(&sl, 0);\n-\n-\thashmap_for_each_entry(&pl->recursive_hashmap, &iter, pe, ent) {\n-\t\tif (!hashmap_contains_parent(&pl->recursive_hashmap,\n-\t\t\t\t\t pe->pattern,\n-\t\t\t\t\t &parent_pattern))\n-\t\t\tstring_list_insert(&sl, pe->pattern);\n-\t}\n-\n-\tstrbuf_release(&parent_pattern);\n-\n-\tstring_list_sort(&sl);\n-\tstring_list_remove_duplicates(&sl, 0);\n-\n-\tfor (i = 0; i < sl.nr; i++) {\n-\t\tchar *pattern = escaped_pattern(sl.items[i].string);\n-\t\tfprintf(fp, \"%s/\\n\", pattern);\n-\t\tfree(pattern);\n-\t}\n-}\n-\n-static int write_patterns_and_update(struct pattern_list *pl)\n-{\n-\tchar *sparse_filename;\n-\tFILE *fp;\n-\tint fd;\n-\tstruct lock_file lk = LOCK_INIT;\n-\tint result;\n-\n-\tsparse_filename = get_sparse_checkout_filename();\n-\n-\tif (safe_create_leading_directories(sparse_filename))\n-\t\tdie(_(\"failed to create directory for sparse-checkout file\"));\n-\n-\tfd = hold_lock_file_for_update(&lk, sparse_filename,\n-\t\t\t\t LOCK_DIE_ON_ERROR);\n-\n-\tresult = update_working_directory(pl);\n-\tif (result) {\n-\t\trollback_lock_file(&lk);\n-\t\tfree(sparse_filename);\n-\t\tclear_pattern_list(pl);\n-\t\tupdate_working_directory(NULL);\n-\t\treturn result;\n-\t}\n-\n-\tfp = xfdopen(fd, \"w\");\n-\n-\tif (core_sparse_checkout_cone)\n-\t\twrite_cone_to_file(fp, pl);\n-\telse\n-\t\twrite_patterns_to_file(fp, pl);\n-\n-\tfflush(fp);\n-\tcommit_lock_file(&lk);\n-\n-\tfree(sparse_filename);\n-\tclear_pattern_list(pl);\n-\n-\treturn 0;\n-}\n-\n enum sparse_checkout_mode {\n \tMODE_NO_PATTERNS = 0,\n \tMODE_ALL_PATTERNS = 1,\n@@ -339,58 +168,6 @@ static int sparse_checkout_init(int argc, const char **argv)\n \treturn write_patterns_and_update(&pl);\n }\n \n-static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path)\n-{\n-\tstruct pattern_entry *e = xmalloc(sizeof(*e));\n-\te->patternlen = path->len;\n-\te->pattern = strbuf_detach(path, NULL);\n-\thashmap_entry_init(&e->ent,\n-\t\t\t ignore_case ?\n-\t\t\t strihash(e->pattern) :\n-\t\t\t strhash(e->pattern));\n-\n-\thashmap_add(&pl->recursive_hashmap, &e->ent);\n-\n-\twhile (e->patternlen) {\n-\t\tchar *slash = strrchr(e->pattern, '/');\n-\t\tchar *oldpattern = e->pattern;\n-\t\tsize_t newlen;\n-\n-\t\tif (slash == e->pattern)\n-\t\t\tbreak;\n-\n-\t\tnewlen = slash - e->pattern;\n-\t\te = xmalloc(sizeof(struct pattern_entry));\n-\t\te->patternlen = newlen;\n-\t\te->pattern = xstrndup(oldpattern, newlen);\n-\t\thashmap_entry_init(&e->ent,\n-\t\t\t\t ignore_case ?\n-\t\t\t\t strihash(e->pattern) :\n-\t\t\t\t strhash(e->pattern));\n-\n-\t\tif (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))\n-\t\t\thashmap_add(&pl->parent_hashmap, &e->ent);\n-\t}\n-}\n-\n-static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n-{\n-\tstrbuf_trim(line);\n-\n-\tstrbuf_trim_trailing_dir_sep(line);\n-\n-\tif (strbuf_normalize_path(line))\n-\t\tdie(_(\"could not normalize path %s\"), line->buf);\n-\n-\tif (!line->len)\n-\t\treturn;\n-\n-\tif (line->buf[0] != '/')\n-\t\tstrbuf_insertstr(line, 0, \"/\");\n-\n-\tinsert_recursive_pattern(pl, line);\n-}\n-\n static char const * const builtin_sparse_checkout_set_usage[] = {\n \tN_(\"git sparse-checkout (set|add) (--stdin | )\"),\n \tNULL\ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nnew file mode 100644\nindex 00000000000..5ada307b42c\n--- /dev/null\n+++ b/sparse-checkout.c\n@@ -0,0 +1,233 @@\n+#include \"cache.h\"\n+#include \"config.h\"\n+#include \"dir.h\"\n+#include \"lockfile.h\"\n+#include \"quote.h\"\n+#include \"repository.h\"\n+#include \"sparse-checkout.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+#include \"unpack-trees.h\"\n+\n+char *get_sparse_checkout_filename(void)\n+{\n+\treturn git_pathdup(\"info/sparse-checkout\");\n+}\n+\n+void write_patterns_to_file(FILE *fp, struct pattern_list *pl)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < pl->nr; i++) {\n+\t\tstruct path_pattern *p = pl->patterns[i];\n+\n+\t\tif (p->flags & PATTERN_FLAG_NEGATIVE)\n+\t\t\tfprintf(fp, \"!\");\n+\n+\t\tfprintf(fp, \"%s\", p->pattern);\n+\n+\t\tif (p->flags & PATTERN_FLAG_MUSTBEDIR)\n+\t\t\tfprintf(fp, \"/\");\n+\n+\t\tfprintf(fp, \"\\n\");\n+\t}\n+}\n+\n+int update_working_directory(struct pattern_list *pl)\n+{\n+\tenum update_sparsity_result result;\n+\tstruct unpack_trees_options o;\n+\tstruct lock_file lock_file = LOCK_INIT;\n+\tstruct repository *r = the_repository;\n+\n+\tmemset(&o, 0, sizeof(o));\n+\to.verbose_update = isatty(2);\n+\to.update = 1;\n+\to.head_idx = -1;\n+\to.src_index = r->index;\n+\to.dst_index = r->index;\n+\to.skip_sparse_checkout = 0;\n+\to.pl = pl;\n+\n+\tsetup_work_tree();\n+\n+\trepo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);\n+\n+\tsetup_unpack_trees_porcelain(&o, \"sparse-checkout\");\n+\tresult = update_sparsity(&o);\n+\tclear_unpack_trees_porcelain(&o);\n+\n+\tif (result == UPDATE_SPARSITY_WARNINGS)\n+\t\t/*\n+\t\t * We don't do any special handling of warnings from untracked\n+\t\t * files in the way or dirty entries that can't be removed.\n+\t\t */\n+\t\tresult = UPDATE_SPARSITY_SUCCESS;\n+\tif (result == UPDATE_SPARSITY_SUCCESS)\n+\t\twrite_locked_index(r->index, &lock_file, COMMIT_LOCK);\n+\telse\n+\t\trollback_lock_file(&lock_file);\n+\n+\treturn result;\n+}\n+\n+static char *escaped_pattern(char *pattern)\n+{\n+\tchar *p = pattern;\n+\tstruct strbuf final = STRBUF_INIT;\n+\n+\twhile (*p) {\n+\t\tif (is_glob_special(*p))\n+\t\t\tstrbuf_addch(&final, '\\\\');\n+\n+\t\tstrbuf_addch(&final, *p);\n+\t\tp++;\n+\t}\n+\n+\treturn strbuf_detach(&final, NULL);\n+}\n+\n+static void write_cone_to_file(FILE *fp, struct pattern_list *pl)\n+{\n+\tint i;\n+\tstruct pattern_entry *pe;\n+\tstruct hashmap_iter iter;\n+\tstruct string_list sl = STRING_LIST_INIT_DUP;\n+\tstruct strbuf parent_pattern = STRBUF_INIT;\n+\n+\thashmap_for_each_entry(&pl->parent_hashmap, &iter, pe, ent) {\n+\t\tif (hashmap_get_entry(&pl->recursive_hashmap, pe, ent, NULL))\n+\t\t\tcontinue;\n+\n+\t\tif (!hashmap_contains_parent(&pl->recursive_hashmap,\n+\t\t\t\t\t pe->pattern,\n+\t\t\t\t\t &parent_pattern))\n+\t\t\tstring_list_insert(&sl, pe->pattern);\n+\t}\n+\n+\tstring_list_sort(&sl);\n+\tstring_list_remove_duplicates(&sl, 0);\n+\n+\tfprintf(fp, \"/*\\n!/*/\\n\");\n+\n+\tfor (i = 0; i < sl.nr; i++) {\n+\t\tchar *pattern = escaped_pattern(sl.items[i].string);\n+\n+\t\tif (strlen(pattern))\n+\t\t\tfprintf(fp, \"%s/\\n!%s/*/\\n\", pattern, pattern);\n+\t\tfree(pattern);\n+\t}\n+\n+\tstring_list_clear(&sl, 0);\n+\n+\thashmap_for_each_entry(&pl->recursive_hashmap, &iter, pe, ent) {\n+\t\tif (!hashmap_contains_parent(&pl->recursive_hashmap,\n+\t\t\t\t\t pe->pattern,\n+\t\t\t\t\t &parent_pattern))\n+\t\t\tstring_list_insert(&sl, pe->pattern);\n+\t}\n+\n+\tstrbuf_release(&parent_pattern);\n+\n+\tstring_list_sort(&sl);\n+\tstring_list_remove_duplicates(&sl, 0);\n+\n+\tfor (i = 0; i < sl.nr; i++) {\n+\t\tchar *pattern = escaped_pattern(sl.items[i].string);\n+\t\tfprintf(fp, \"%s/\\n\", pattern);\n+\t\tfree(pattern);\n+\t}\n+}\n+\n+int write_patterns_and_update(struct pattern_list *pl)\n+{\n+\tchar *sparse_filename;\n+\tFILE *fp;\n+\tint fd;\n+\tstruct lock_file lk = LOCK_INIT;\n+\tint result;\n+\n+\tsparse_filename = get_sparse_checkout_filename();\n+\n+\tif (safe_create_leading_directories(sparse_filename))\n+\t\tdie(_(\"failed to create directory for sparse-checkout file\"));\n+\n+\tfd = hold_lock_file_for_update(&lk, sparse_filename,\n+\t\t\t\t LOCK_DIE_ON_ERROR);\n+\n+\tresult = update_working_directory(pl);\n+\tif (result) {\n+\t\trollback_lock_file(&lk);\n+\t\tfree(sparse_filename);\n+\t\tclear_pattern_list(pl);\n+\t\tupdate_working_directory(NULL);\n+\t\treturn result;\n+\t}\n+\n+\tfp = xfdopen(fd, \"w\");\n+\n+\tif (core_sparse_checkout_cone)\n+\t\twrite_cone_to_file(fp, pl);\n+\telse\n+\t\twrite_patterns_to_file(fp, pl);\n+\n+\tfflush(fp);\n+\tcommit_lock_file(&lk);\n+\n+\tfree(sparse_filename);\n+\tclear_pattern_list(pl);\n+\n+\treturn 0;\n+}\n+\n+void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path)\n+{\n+\tstruct pattern_entry *e = xmalloc(sizeof(*e));\n+\te->patternlen = path->len;\n+\te->pattern = strbuf_detach(path, NULL);\n+\thashmap_entry_init(&e->ent,\n+\t\t\t ignore_case ?\n+\t\t\t strihash(e->pattern) :\n+\t\t\t strhash(e->pattern));\n+\n+\thashmap_add(&pl->recursive_hashmap, &e->ent);\n+\n+\twhile (e->patternlen) {\n+\t\tchar *slash = strrchr(e->pattern, '/');\n+\t\tchar *oldpattern = e->pattern;\n+\t\tsize_t newlen;\n+\n+\t\tif (slash == e->pattern)\n+\t\t\tbreak;\n+\n+\t\tnewlen = slash - e->pattern;\n+\t\te = xmalloc(sizeof(struct pattern_entry));\n+\t\te->patternlen = newlen;\n+\t\te->pattern = xstrndup(oldpattern, newlen);\n+\t\thashmap_entry_init(&e->ent,\n+\t\t\t\t ignore_case ?\n+\t\t\t\t strihash(e->pattern) :\n+\t\t\t\t strhash(e->pattern));\n+\n+\t\tif (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))\n+\t\t\thashmap_add(&pl->parent_hashmap, &e->ent);\n+\t}\n+}\n+\n+void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n+{\n+\tstrbuf_trim(line);\n+\n+\tstrbuf_trim_trailing_dir_sep(line);\n+\n+\tif (strbuf_normalize_path(line))\n+\t\tdie(_(\"could not normalize path %s\"), line->buf);\n+\n+\tif (!line->len)\n+\t\treturn;\n+\n+\tif (line->buf[0] != '/')\n+\t\tstrbuf_insertstr(line, 0, \"/\");\n+\n+\tinsert_recursive_pattern(pl, line);\n+}\ndiff --git a/sparse-checkout.h b/sparse-checkout.h\nnew file mode 100644\nindex 00000000000..4148832760d\n--- /dev/null\n+++ b/sparse-checkout.h\n@@ -0,0 +1,16 @@\n+#ifndef SPARSE_CHECKOUT_H\n+#define SPARSE_CHECKOUT_H\n+\n+#include \"cache.h\"\n+#include \"repository.h\"\n+\n+struct pattern_list;\n+\n+char *get_sparse_checkout_filename(void);\n+void write_patterns_to_file(FILE *fp, struct pattern_list *pl);\n+int update_working_directory(struct pattern_list *pl);\n+int write_patterns_and_update(struct pattern_list *pl);\n+void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path);\n+void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl);\n+\n+#endif\n-- \ngitgitgadget\n\n"},{"id":"397300","messageId":"2188577cd848d7cee77f06f1ad2b181864e5e36d.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:36Z","receivedAt":"2020-05-07T13:17:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nOne of the difficulties of using the sparse-checkout feature is not\nknowing which directories are absolutely needed for working in a portion\nof the repository. Some of this can be documented in README files or\nincluded in a bootstrapping tool along with the repository. This is done\nin an ad-hoc way by every project that wants to use it.\n\nLet's make this process easier for users by creating a way to define a\nuseful sparse-checkout definition inside the Git tree data. This has\nseveral benefits. In particular, the data is available to anyone who has\na copy of the repository without needing a different data source.\nSecond, the needs of the repository can change over time and Git can\npresent a way to automatically update the working directory as these\nsparse-checkout definitions change over time.\n\nWhen sitting down to design this feature, there were several possible\noptions.\n\nThe first option is to literally include files in the repository that\ncould replace the sparse-checkout file. This presents full generality\nfor the repository, but the sparse-checkout patterns may look strange to\na non-expert. While the general case could be useful to some, we are\nactively working to make \"cone mode\" the generally accepted way to work\nwith sparse-checkout. In cone mode, the user selects a set of\ndirectories to include and all matches are directory-based prefix\nmatches. This is much faster but also much easier to understand.\n\nAnother option would be to interpret a file as a list of necessary\ndirectories. This would match the input to 'git sparse-checkout set'\nand output of 'git sparse-checkout list' when in cone mode. However,\nthere are subtleties around custom data parsers that may not make this\nthe safest approach.\n\nBoth of the above options also suffer from a drawback that the file\nformat is too simple to allow for extensions in the future. For example,\nin a repository with many dependent parts, changing a necessary\ndirectory in a core component would require updating every in-tree\nsparse-checkout definition. Since those files are likely very long, this\npresents a lot of noise for users to handle as they update these\ndependencies.\n\nInstead, let's create a format that can easily be extended to satisfy\nthis need and any other needs we may want to add in the future. For this\nreason, I selected the config file format for these in-tree\nsparse-checkout definitions.\n\nTo use this feature, add the \"--in-tree\" option when setting or adding\ndirectories to the sparse-checkout definition. For example:\n\n $ git sparse-checkout set --in-tree .sparse/base\n $ git sparse-checkout add --in-tree .sparse/extra\n\nThese commands add values to the multi-valued config setting\n\"sparse.inTree\". When updating the sparse-checkout definition, these\nvalues describe paths in the repository to find the sparse-checkout\ndata. After the commands listed earlier, we expect to see the following\nin .git/config.worktree:\n\n\t[sparse]\n\t\tintree = .sparse/base\n\t\tintree = .sparse/extra\n\nWhen applying the sparse-checkout definitions from this config, the\nblobs at HEAD:.sparse/base and HEAD:.sparse/extra are loaded. In those\nfiles, the multi-valued config values \"sparse.dir\" are considered as\nthe directories to construct a cone mode sparse-checkout file. The end\nresult is as if these paths were provided to \"git sparse-checkout set\"\nin cone mode.\n\nFor example, suppose .sparse/base had the following content:\n\n\t[sparse]\n\t\tdir = A\n\t\tdir = B/C\n\t\tdir = D/E/F\n\nand .sparse/extra had the following content:\n\n\t[sparse]\n\t\tdir = D\n\t\tdir = X\n\nThen, the output of \"git sparse-checkout list\" would be\n\n\tA\n\tB/C\n\tD\n\tX\n\nNote that since \"D\" contains \"D/E/F\", that directory replaces the\nposition of \"D/E/F\" in the list.\n\nSince these are parsed using the config library, the parser is robust\nenough to understand comments and complicated string values.\n\nThe key benefit to this approach is that it can be extended by defining\nnew config values. In a later change, we will introduce \"sparse.inherit\"\nto point to another file in the tree. This will solve the problem of\nediting many files when core dependencies change.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/config.txt | 2 +\n Documentation/config/sparse.txt | 15 +++\n Documentation/git-sparse-checkout.txt | 43 ++++++++-\n builtin/sparse-checkout.c | 43 +++++++++\n sparse-checkout.c | 128 +++++++++++++++++++++++++-\n sparse-checkout.h | 11 +++\n t/t1091-sparse-checkout-builtin.sh | 101 ++++++++++++++++++++\n 7 files changed, 337 insertions(+), 6 deletions(-)\n create mode 100644 Documentation/config/sparse.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 08b13ba72be..40f44948229 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -435,6 +435,8 @@ include::config/sequencer.txt[]\n \n include::config/showbranch.txt[]\n \n+include::config/sparse.txt[]\n+\n include::config/splitindex.txt[]\n \n include::config/ssh.txt[]\ndiff --git a/Documentation/config/sparse.txt b/Documentation/config/sparse.txt\nnew file mode 100644\nindex 00000000000..c1fce87cd33\n--- /dev/null\n+++ b/Documentation/config/sparse.txt\n@@ -0,0 +1,15 @@\n+sparse.inTree::\n+\tThe `core.sparseCheckout` config option enables the `sparse-checkout`\n+\tfeature, but if there are any values for the multi-valued\n+\t`sparse.inTree` config option, then the sparse-checkout patterns are\n+\tdefined by parsing the files listed in these values. See\n+\tlinkgit:git-sparse-checkout[1] for more information.\n+\n+sparse.dir::\n+\tThis config setting is ignored if present in the repository config.\n+\tInstead, this multi-valued option is present in the files listed by\n+\t`sparse.inTree` and specifies the directories needed in the\n+\tworking directory. The union of all `sparse.dir` values across all\n+\t`sparse.inTree` files forms the input for `git sparse-checkout set`\n+\tin cone mode. See linkgit:git-sparse-checkout[1] for more\n+\tinformation.\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex 1a3ace60820..da9322c5e41 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -62,13 +62,20 @@ directories (recursively) as well as files that are siblings of ancestor\n directories. The input format matches the output of `git ls-tree --name-only`.\n This includes interpreting pathnames that begin with a double quote (\") as\n C-style quoted strings.\n++\n+When the `--in-tree` option is provided, the paths provided are interpreted\n+as files within the working directory that are used to construct the\n+`sparse-checkout` patterns. See 'IN-TREE PATTERN SET' below.\n \n 'add'::\n \tUpdate the sparse-checkout file to include additional patterns.\n \tBy default, these patterns are read from the command-line arguments,\n \tbut they can be read from stdin using the `--stdin` option. When\n \t`core.sparseCheckoutCone` is enabled, the given patterns are interpreted\n-\tas directory names as in the 'set' subcommand.\n+\tas directory names as in the 'set' subcommand. When the `--in-tree`\n+\toption is provided, the input is interpreted as locations of files\n+\tdescribing a sparse-checkout definition as in the 'set' subcommand\n+\tand the 'IN-TREE PATTERN SET' section below.\n \n 'reapply::\n \tReapply the sparsity pattern rules to paths in the working tree.\n@@ -197,6 +204,40 @@ case-insensitive check. This corrects for case mismatched filenames in the\n directory.\n \n \n+IN-TREE PATTERN SET\n+-------------------\n+\n+As your project changes, your sparse-checkout pattern sets may also change.\n+It is important to be able to construct a valid sparse-checkout pattern set\n+when switching between points in history. The in-tree pattern sets allow\n+versioning cone-mode sparse-checkout patterns next to your other artifacts.\n+\n+To enable the feature, create a sparse-checkout definition using the Git\n+config format. The file should specify the multi-valued config variable\n+`sparse.dir` to a list of directories to include in the sparse-checkout\n+definition. If multiple files are specified, the resulting sparse-checkout\n+definition is the union of all directories from all such files. For\n+example, the following file contains a list of three directories, `A`,\n+`B/C`, and `D/E/F`:\n+\n+----------------------------------\n+[sparse]\n+\tdir = A\n+\tdir = B/C\n+# Comments are allowed to describe\n+# why a directory is necessary\n+\tdir = D/E/F\n+----------------------------------\n+\n+Use `git sparse-checkout set --in-tree ` to initialize the patterns\n+to those included in the file at ``. This will override any existing\n+patterns you have in your sparse-checkout file.\n+\n+After switching between commits with different versions of this file, run\n+`git sparse-checkout reapply` to adjust the sparse-checkout patterns to\n+the new definition.\n+\n+\n SUBMODULES\n ----------\n \ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex fd247e428e4..621f1801c03 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -175,6 +175,7 @@ static char const * const builtin_sparse_checkout_set_usage[] = {\n \n static struct sparse_checkout_set_opts {\n \tint use_stdin;\n+\tint in_tree;\n } set_opts;\n \n static void add_patterns_from_input(struct pattern_list *pl,\n@@ -312,12 +313,52 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)\n \treturn result;\n }\n \n+static int modify_in_tree_list(int argc, const char **argv, enum modify_type m)\n+{\n+\tint result = 0;\n+\tint i;\n+\tstruct string_list sl = STRING_LIST_INIT_DUP;\n+\tstruct pattern_list pl;\n+\n+\tmemset(&pl, 0, sizeof(pl));\n+\n+\tswitch(m) {\n+\tcase ADD:\n+\t\tif (load_in_tree_list_from_config(the_repository, &sl))\n+\t\t\treturn 1;\n+\t\tif (!sl.nr)\n+\t\t\twarning(_(\"the existing in-tree config has no entries; this overwrites the existing sparse-checkout definition.\"));\n+\t\tpopulate_sparse_checkout_patterns(&pl);\n+\t\tbreak;\n+\n+\tcase REPLACE:\n+\t\thashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);\n+\t\thashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);\n+\t\tbreak;\n+\t}\n+\n+\tfor (i = 0; i < argc; i++)\n+\t\tstring_list_insert(&sl, argv[i]);\n+\n+\tif (load_in_tree_pattern_list(the_repository, &sl, &pl) ||\n+\t set_sparse_in_tree_config(the_repository, &sl) ||\n+\t write_patterns_and_update(&pl))\n+\t\tresult = 1;\n+\n+\tstring_list_clear(&sl, 0);\n+\tclear_pattern_list(&pl);\n+\n+\treturn result;\n+}\n+\n static int sparse_checkout_set(int argc, const char **argv, const char *prefix,\n \t\t\t enum modify_type m)\n {\n \tstatic struct option builtin_sparse_checkout_set_options[] = {\n \t\tOPT_BOOL(0, \"stdin\", &set_opts.use_stdin,\n \t\t\t N_(\"read patterns from standard in\")),\n+\t\tOPT_BOOL(0, \"in-tree\", &set_opts.in_tree,\n+\t\t\t N_(\"define the sparse-checkout from files in the tree\")),\n \t\tOPT_END(),\n \t};\n \n@@ -328,6 +369,8 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,\n \t\t\t builtin_sparse_checkout_set_usage,\n \t\t\t PARSE_OPT_KEEP_UNKNOWN);\n \n+\tif (set_opts.in_tree)\n+\t\treturn modify_in_tree_list(argc, argv, m);\n \treturn modify_pattern_list(argc, argv, m);\n }\n \ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nindex 875b620568d..d6c27ca19c4 100644\n--- a/sparse-checkout.c\n+++ b/sparse-checkout.c\n@@ -8,6 +8,7 @@\n #include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"unpack-trees.h\"\n+#include \"object-store.h\"\n \n char *get_sparse_checkout_filename(void)\n {\n@@ -33,14 +34,113 @@ void write_patterns_to_file(FILE *fp, struct pattern_list *pl)\n \t}\n }\n \n+int load_in_tree_list_from_config(struct repository *r,\n+\t\t\t\t struct string_list *sl)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *cl;\n+\n+\tcl = repo_config_get_value_multi(r, SPARSE_CHECKOUT_IN_TREE);\n+\n+\tif (!cl)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, cl)\n+\t\tstring_list_insert(sl, item->string);\n+\n+\treturn 0;\n+}\n+\n+static int sparse_dir_cb(const char *var, const char *value, void *data)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct pattern_list *pl = (struct pattern_list *)data;\n+\n+\tif (!strcmp(var, SPARSE_CHECKOUT_DIR)) {\n+\t\tstrbuf_addstr(&path, value);\n+\t\tstrbuf_to_cone_pattern(&path, pl);\n+\t\tstrbuf_release(&path);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int load_in_tree_from_blob(struct pattern_list *pl,\n+\t\t\t\t struct object_id *oid)\n+{\n+\treturn git_config_from_blob_oid(sparse_dir_cb,\n+\t\t\t\t\tSPARSE_CHECKOUT_DIR,\n+\t\t\t\t\toid, pl);\n+}\n+\n+int load_in_tree_pattern_list(struct repository *r,\n+\t\t\t struct string_list *sl,\n+\t\t\t struct pattern_list *pl)\n+{\n+\tstruct index_state *istate = r->index;\n+\tstruct string_list_item *item;\n+\tstruct strbuf path = STRBUF_INIT;\n+\n+\tpl->use_cone_patterns = 1;\n+\n+\tfor_each_string_list_item(item, sl) {\n+\t\tstruct object_id *oid;\n+\t\tenum object_type type;\n+\t\tint pos = index_name_pos(istate, item->string, strlen(item->string));\n+\n+\t\t/*\n+\t\t * Exit silently, as this is likely the case where Git\n+\t\t * changed branches to a location where the inherit file\n+\t\t * does not exist. Do not update the sparse-checkout.\n+\t\t */\n+\t\tif (pos < 0)\n+\t\t\treturn 1;\n+\n+\t\toid = &istate->cache[pos]->oid;\n+\t\ttype = oid_object_info(r, oid, NULL);\n+\n+\t\tif (type != OBJ_BLOB) {\n+\t\t\twarning(_(\"expected a file at '%s'; not updating sparse-checkout\"),\n+\t\t\t\toid_to_hex(oid));\n+\t\t\treturn 1;\n+\t\t}\n+\n+\t\tload_in_tree_from_blob(pl, oid);\n+\t}\n+\n+\tstrbuf_release(&path);\n+\n+\treturn 0;\n+}\n+\n int populate_sparse_checkout_patterns(struct pattern_list *pl)\n {\n \tint result;\n-\tchar *sparse = get_sparse_checkout_filename();\n-\n-\tpl->use_cone_patterns = core_sparse_checkout_cone;\n-\tresult = add_patterns_from_file_to_list(sparse, \"\", 0, pl, NULL);\n-\tfree(sparse);\n+\tconst char *in_tree;\n+\n+\tif (!git_config_get_value(SPARSE_CHECKOUT_IN_TREE, &in_tree) &&\n+\t in_tree) {\n+\t\tstruct string_list paths = STRING_LIST_INIT_DUP;\n+\t\t/* If we do not have this config, skip this step! */\n+\t\tif (load_in_tree_list_from_config(the_repository, &paths) ||\n+\t\t !paths.nr)\n+\t\t\treturn 1;\n+\n+\t\t/* Check diff for paths over from/to. If any changed, reload. */\n+\t\t/* or for now, reload always! */\n+\t\thashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);\n+\t\thashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);\n+\t\tpl->use_cone_patterns = 1;\n+\n+\t\tresult = load_in_tree_pattern_list(the_repository, &paths, pl);\n+\t\tstring_list_clear(&paths, 0);\n+\t} else {\n+\t\tchar *sparse = get_sparse_checkout_filename();\n+\n+\t\tpl->use_cone_patterns = core_sparse_checkout_cone;\n+\t\tresult = add_patterns_from_file_to_list(sparse, \"\", 0, pl, NULL);\n+\t\tfree(sparse);\n+\t}\n \n \treturn result;\n }\n@@ -243,3 +343,21 @@ void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n \n \tinsert_recursive_pattern(pl, line);\n }\n+\n+int set_sparse_in_tree_config(struct repository *r, struct string_list *sl)\n+{\n+\tstruct string_list_item *item;\n+\tconst char *config_path = git_path(\"config.worktree\");\n+\n+\t/* clear existing values */\n+\tgit_config_set_multivar_in_file_gently(config_path,\n+\t\t\t\t\t SPARSE_CHECKOUT_IN_TREE,\n+\t\t\t\t\t NULL, NULL, 1);\n+\n+\tfor_each_string_list_item(item, sl)\n+\t\tgit_config_set_multivar_in_file_gently(\n+\t\t\tconfig_path, SPARSE_CHECKOUT_IN_TREE,\n+\t\t\titem->string, CONFIG_REGEX_NONE, 0);\n+\n+\treturn 0;\n+}\ndiff --git a/sparse-checkout.h b/sparse-checkout.h\nindex e0c840f07f9..993a5701a60 100644\n--- a/sparse-checkout.h\n+++ b/sparse-checkout.h\n@@ -4,14 +4,25 @@\n #include \"cache.h\"\n #include \"repository.h\"\n \n+#define SPARSE_CHECKOUT_DIR \"sparse.dir\"\n+#define SPARSE_CHECKOUT_IN_TREE \"sparse.intree\"\n+\n struct pattern_list;\n \n char *get_sparse_checkout_filename(void);\n int populate_sparse_checkout_patterns(struct pattern_list *pl);\n void write_patterns_to_file(FILE *fp, struct pattern_list *pl);\n int update_working_directory(struct pattern_list *pl);\n+int write_patterns(struct pattern_list *pl, int and_update);\n int write_patterns_and_update(struct pattern_list *pl);\n void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path);\n void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl);\n \n+int load_in_tree_list_from_config(struct repository *r,\n+\t\t\t\t struct string_list *sl);\n+int load_in_tree_pattern_list(struct repository *r,\n+\t\t\t struct string_list *sl,\n+\t\t\t struct pattern_list *pl);\n+int set_sparse_in_tree_config(struct repository *r, struct string_list *sl);\n+\n #endif\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 88cdde255cd..1040bf9c261 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -604,4 +604,105 @@ test_expect_success MINGW 'cone mode replaces backslashes with slashes' '\n \tcheck_files repo/deep a deeper1\n '\n \n+test_expect_success 'basis of --in-tree' '\n+\tgit -C repo config auto.crlf false &&\n+\tcat >folder1 <<-\\EOF &&\n+\t[sparse]\n+\t\tdir = folder1\n+\tEOF\n+\tcat >folder2 <<-\\EOF &&\n+\t[sparse]\n+\t\tdir = folder2\n+\tEOF\n+\tcat >deep <<-\\EOF &&\n+\t[sparse]\n+\t\tdir = deep\n+\tEOF\n+\tcat >deeper1 <<-\\EOF &&\n+\t[sparse]\n+\t\tdir = deep/deeper1\n+\tEOF\n+\tcat >sparse <<-\\EOF &&\n+\t[sparse]\n+\t\tdir = .sparse\n+\tEOF\n+\tmkdir repo/.sparse &&\n+\tfor file in folder1 folder2 deep deeper1 sparse\n+\tdo\n+\t\tcp $file repo/.sparse/ || return 1\n+\tdone &&\n+\tgit -C repo add .sparse &&\n+\tgit -C repo commit -m \"Add sparse specifications\" &&\n+\n+\tgit -C repo sparse-checkout set --in-tree .sparse/folder1 &&\n+\tcheck_files repo a folder1 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\techo .sparse/folder1 >expect-config &&\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a folder1 &&\n+\n+\tgit -C repo sparse-checkout set --in-tree .sparse/folder2 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\techo .sparse/folder2 >expect-config &&\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a folder2 &&\n+\n+\tgit -C repo sparse-checkout set --in-tree .sparse/deeper1 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\techo .sparse/deeper1 >expect-config &&\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a deep &&\n+\tcheck_files repo/deep a deeper1 &&\n+\n+\tgit -C repo sparse-checkout set --in-tree .sparse/deeper1 .sparse/deep .sparse/folder1 &&\n+\tcheck_files repo a deep folder1 &&\n+\tcheck_files repo/deep a deeper1 deeper2 &&\n+\tcat >expect-list <<-EOF &&\n+\tdeep\n+\tfolder1\n+\tEOF\n+\tgit -C repo sparse-checkout list >actual-list &&\n+\ttest_cmp expect-list actual-list &&\n+\n+\tgit -C repo sparse-checkout set --in-tree .sparse/folder1 .sparse/deeper1 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\tcat >expect-config <<-\\EOF &&\n+\t.sparse/deeper1\n+\t.sparse/folder1\n+\tEOF\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a deep folder1\n+'\n+\n+test_expect_success '\"add\" with --in-tree' '\n+\tgit -C repo sparse-checkout set --in-tree .sparse/folder1 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\techo .sparse/folder1 >expect-config &&\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a folder1 &&\n+\tgit -C repo sparse-checkout add --in-tree .sparse/deeper1 &&\n+\tgit -C repo config --get-all sparse.inTree >actual-config &&\n+\tcat >expect-config <<-\\EOF &&\n+\t.sparse/deeper1\n+\t.sparse/folder1\n+\tEOF\n+\ttest_cmp expect-config actual-config &&\n+\tcheck_files repo a deep folder1\n+'\n+\n+test_expect_success 'reapply after updating in-tree file' '\n+\tgit -C repo sparse-checkout set --in-tree .sparse/sparse &&\n+\tcheck_files repo a &&\n+\ttest_path_is_dir repo/.sparse &&\n+\techo \"\\tdir = folder1\" >>repo/.sparse/sparse &&\n+\tgit -C repo commit -a -m \"Update sparse file\" &&\n+\tgit -C repo sparse-checkout reapply &&\n+\tcheck_files repo a folder1 &&\n+\ttest_path_is_dir repo/.sparse &&\n+\tgit -C repo checkout HEAD~1 &&\n+\tgit -C repo sparse-checkout reapply &&\n+\tcheck_files repo a &&\n+\ttest_path_is_dir repo/.sparse\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"397299","messageId":"f55cd0fb3897db9ca22156347293ca830cdf018c.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 07/10] sparse-checkout: define in-tree dependencies","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:39Z","receivedAt":"2020-05-07T13:17:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs mentioned in the definition of the in-tree sparse-checkout\ndefinitions, a large repository could have many different roles for the\nusers of that repository. That means that many different in-tree\nsparse-checkout definitions need to exist. If a directory needs to be\nadded to many of these roles, then many of these files need to be\nedited. This is too much work to scale properly.\n\nInstead, let's make these definitions easier to maintain by following\nbasic principles of build dependencies. Add simple links between these\nin-tree files using the new \"sparse.inherit\" config key. This multi-\nvalued config setting specifies more files that are needed to define the\nsparse-checkout for a role. By separating out the common directories\ninto a base file, the more specialized roles can focus their files on\nthe directories only needed by those roles and leave the common\ndirectories to that base file.\n\nFor example, suppose that .sparse/base has the following data:\n\n\t[sparse]\n\t\tdir = A\n\t\tdir = B/C\n\t\tdir = D/E/F\n\nand .sparse/extra has the following data:\n\n\t[sparse]\n\t\tdir = D\n\t\tdir = X\n\t\tinherit = .sparse/base\n\nNow, a user can run \"git sparse-checkout set --in-tree .sparse/extra\"\nand the resulting directories will define their cone-mode\nsparse-checkout patterns:\n\n\tA\n\tB/C\n\tD\n\tX\n\nThus, the resulting sparse-checkout definition is the union of the\ndefinitions from .sparse/base and .sparse/extra, but the user only has\none value of sparse.inTree of \".sparse/extra\".\n\nIt is simple to modify our existing logic to explore the directed graph\ncreated by the sparse.inherit values. We simply need to append to the\nstring_list containing the list of in-tree files. Since we never repeat\nparsing on the same blob oid, this will not lead to infinite loops or\nexponential blowups in the parsing time.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-sparse-checkout.txt | 26 ++++++++++++++++++++++\n sparse-checkout.c | 27 ++++++++++++++++++-----\n sparse-checkout.h | 1 +\n t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++\n 4 files changed, 80 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex c1713ebb1d2..941658e0011 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -238,6 +238,32 @@ definition according to the files available at the new commit. If any of\n the specified files do not exist at the new commit, then the sparse-checkout\n definition will not change.\n \n+In a very large repository, there may be need to have multiple of these\n+in-tree sparse-checkout definitions to fit the roles of multiple types of\n+users. As each definition grows and the number of user types grow, it can\n+be difficult to manage updating all of the definitions when a common core\n+is modified somehow. For this reason, the in-tree pattern sets can inherit\n+the directories from other in-tree pattern sets. Use the `sparse.inherit`\n+option to specify other files in the tree.\n+\n+For example, suppose the file listed earlier is at `.sparse/core`. Another\n+file could be stored as `.sparse/extra` with contents\n+\n+----------------------------------\n+[sparse]\n+\tdir = X\n+\tdir = Y/Z\n+\tinherit = .sparse/core\n+----------------------------------\n+\n+Then, if you run `git sparse-checkout set --in-tree .sparse/extra`, the\n+sparse-checkout definition will include `X` and `Y/Z` from `.sparse/extra`\n+as well as `A`, `B/C`, and `D/E/F` from `.sparse/core`. This is similar\n+to specifying both `.sparse/core` and `.sparse/extra` in the `set`\n+subcommand, but has a slight advantage. If the `.sparse/extra` file changes\n+the set of `inherit` files, then your sparse-checkout definition will\n+update accordingly as you switch between commits.\n+\n \n SUBMODULES\n ----------\ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nindex d01f4d7b525..4edeab49a10 100644\n--- a/sparse-checkout.c\n+++ b/sparse-checkout.c\n@@ -66,12 +66,29 @@ static int sparse_dir_cb(const char *var, const char *value, void *data)\n \treturn 0;\n }\n \n+static int sparse_inherit_cb(const char *var, const char *value, void *data)\n+{\n+\tstruct string_list *sl = (struct string_list *)data;\n+\n+\tif (!strcmp(var, SPARSE_CHECKOUT_INHERIT))\n+\t\tstring_list_append(sl, value);\n+\n+\treturn 0;\n+}\n+\n static int load_in_tree_from_blob(struct pattern_list *pl,\n-\t\t\t\t struct object_id *oid)\n+\t\t\t\t struct object_id *oid,\n+\t\t\t\t struct string_list *inherit)\n {\n-\treturn git_config_from_blob_oid(sparse_dir_cb,\n-\t\t\t\t\tSPARSE_CHECKOUT_DIR,\n-\t\t\t\t\toid, pl);\n+\tif (git_config_from_blob_oid(sparse_dir_cb,\n+\t\t\t\t SPARSE_CHECKOUT_DIR,\n+\t\t\t\t oid, pl))\n+\t\treturn 1;\n+\tif (git_config_from_blob_oid(sparse_inherit_cb,\n+\t\t\t\t SPARSE_CHECKOUT_INHERIT,\n+\t\t\t\t oid, inherit))\n+\t\treturn 1;\n+\treturn 0;\n }\n \n int load_in_tree_pattern_list(struct repository *r,\n@@ -121,7 +138,7 @@ int load_in_tree_pattern_list(struct repository *r,\n \t\t\tgoto cleanup;\n \t\t}\n \n-\t\tload_in_tree_from_blob(pl, oid);\n+\t\tload_in_tree_from_blob(pl, oid, sl);\n \t}\n \n cleanup:\ndiff --git a/sparse-checkout.h b/sparse-checkout.h\nindex fb0ba48524a..8b766ea38fb 100644\n--- a/sparse-checkout.h\n+++ b/sparse-checkout.h\n@@ -5,6 +5,7 @@\n #include \"repository.h\"\n \n #define SPARSE_CHECKOUT_DIR \"sparse.dir\"\n+#define SPARSE_CHECKOUT_INHERIT \"sparse.inherit\"\n #define SPARSE_CHECKOUT_IN_TREE \"sparse.intree\"\n \n struct pattern_list;\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex fdaafba5377..b2389e5b5e6 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -716,4 +716,35 @@ test_expect_success 'keep definition when in-tree file is missing' '\n \ttest_path_is_dir repo/.sparse\n '\n \n+test_expect_success 'inherit definition from other files' '\n+\tcat >repo/.sparse/inherit <<-EOF &&\n+\t[sparse]\n+\t\tinherit = .sparse/sparse\n+\t\tinherit = .sparse/deep\n+\t\tinherit = .sparse/deeper1\n+\tEOF\n+\tgit -C repo add .sparse &&\n+\tgit -C repo commit -m \"Add inherited file\" &&\n+\tgit -C repo sparse-checkout set --in-tree .sparse/inherit &&\n+\tcheck_files repo a deep folder1 &&\n+\tcheck_files repo/deep a deeper1 deeper2 &&\n+\ttest_path_is_dir repo/.sparse &&\n+\tcat >repo/.sparse/sparse <<-EOF &&\n+\t[sparse]\n+\t\tdir = .sparse\n+\tEOF\n+\tgit -C repo commit -a -m \"drop folder1 from sparse\" &&\n+\tcheck_files repo a deep &&\n+\tcheck_files repo/deep a deeper1 deeper2 &&\n+\ttest_path_is_dir repo/.sparse\n+'\n+\n+test_expect_success 'inherit files can have cycles' '\n+\techo \"\\tinherit = .sparse/inherit\" >>repo/.sparse/sparse &&\n+\tgit -C repo commit -a -m \"create inherit cycle\" &&\n+\tcheck_files repo a deep &&\n+\tcheck_files repo/deep a deeper1 deeper2 &&\n+\ttest_path_is_dir repo/.sparse\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"397301","messageId":"444a6b5f894f28e96f713e5caccba18e1ea3b3eb.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 03/10] sparse-checkout: move code from unpack-trees.c","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:35Z","receivedAt":"2020-05-07T13:17:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThere is no need to have two ways to load the sparse-checkout patterns,\nespecially duplicating the info/sparse-checkout filename. Move this code\nout of unpack-trees.c and into sparse-checkout.c.\n\nSigned-off-by: Derrick Stolee \n---\n sparse-checkout.c | 12 ++++++++++++\n sparse-checkout.h | 1 +\n unpack-trees.c | 7 ++-----\n 3 files changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nindex 5ada307b42c..875b620568d 100644\n--- a/sparse-checkout.c\n+++ b/sparse-checkout.c\n@@ -33,6 +33,18 @@ void write_patterns_to_file(FILE *fp, struct pattern_list *pl)\n \t}\n }\n \n+int populate_sparse_checkout_patterns(struct pattern_list *pl)\n+{\n+\tint result;\n+\tchar *sparse = get_sparse_checkout_filename();\n+\n+\tpl->use_cone_patterns = core_sparse_checkout_cone;\n+\tresult = add_patterns_from_file_to_list(sparse, \"\", 0, pl, NULL);\n+\tfree(sparse);\n+\n+\treturn result;\n+}\n+\n int update_working_directory(struct pattern_list *pl)\n {\n \tenum update_sparsity_result result;\ndiff --git a/sparse-checkout.h b/sparse-checkout.h\nindex 4148832760d..e0c840f07f9 100644\n--- a/sparse-checkout.h\n+++ b/sparse-checkout.h\n@@ -7,6 +7,7 @@\n struct pattern_list;\n \n char *get_sparse_checkout_filename(void);\n+int populate_sparse_checkout_patterns(struct pattern_list *pl);\n void write_patterns_to_file(FILE *fp, struct pattern_list *pl);\n int update_working_directory(struct pattern_list *pl);\n int write_patterns_and_update(struct pattern_list *pl);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 4f880f2da90..6f9bdeb57a0 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -16,6 +16,7 @@\n #include \"fsmonitor.h\"\n #include \"object-store.h\"\n #include \"promisor-remote.h\"\n+#include \"sparse-checkout.h\"\n \n /*\n * Error messages expected by scripts out of plumbing commands such as\n@@ -1550,14 +1551,10 @@ static void mark_new_skip_worktree(struct pattern_list *pl,\n static void populate_from_existing_patterns(struct unpack_trees_options *o,\n \t\t\t\t\t struct pattern_list *pl)\n {\n-\tchar *sparse = git_pathdup(\"info/sparse-checkout\");\n-\n-\tpl->use_cone_patterns = core_sparse_checkout_cone;\n-\tif (add_patterns_from_file_to_list(sparse, \"\", 0, pl, NULL) < 0)\n+\tif (populate_sparse_checkout_patterns(pl) < 0)\n \t\to->skip_sparse_checkout = 1;\n \telse\n \t\to->pl = pl;\n-\tfree(sparse);\n }\n \n \n-- \ngitgitgadget\n\n"},{"id":"397304","messageId":"4ccc5ecbf3d8106ea55b8eb17cd8ad14df3c2680.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 08/10] Makefile: skip git-gui if dir is missing","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:40Z","receivedAt":"2020-05-07T13:17:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs an effort to promote \"dogfooding\" the sparse-checkout feature within\nthe Git codebase, it is helpful to explore which portions of the\ncodebase are optional.\n\nThe NO_TCLTK variable in the Makefile allows ignoring the git-gui\napplication at build time. If the local sparse-checkout removes that\ndirectory, then the build will fail without manually defining that\nvariable. Instead, check to see if the directory exists at build time to\nsee if we can automatically ignore this directory.\n\nWith this change, the following list of directories can be supplied to\n\"git sparse-checkout set\" and Git will build and test on Linux:\n\n\tDocumentation\n\tbuiltin\n\tcompat/.depend\n\tcontrib\n\tewah\n\tmergetools\n\tnegotiator\n\tperl\n\trefs\n\tsha1dc\n\tsha256\n\tt\n\ttemplates\n\ttrace2\n\tvcs-svn\n\txdiff\n\nThe \"make install\" command requires the \"gitweb\" and \"po\" directories.\nThe \"po\" directory will become optional in the next change.\n\nSigned-off-by: Derrick Stolee \n---\n Makefile | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 2e8029b8026..70760d315cb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -313,7 +313,8 @@ all::\n #\n # Define NO_PYTHON if you do not want Python scripts or libraries at all.\n #\n-# Define NO_TCLTK if you do not want Tcl/Tk GUI.\n+# Define NO_TCLTK if you do not want Tcl/Tk GUI. This will also be defined\n+# if the git-gui directory is missing.\n #\n # Define SANE_TEXT_GREP to \"-a\" if you use recent versions of GNU grep\n # and egrep that are pickier when their input contains non-ASCII data.\n@@ -1860,6 +1861,12 @@ ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\n \n+ifndef NO_TCLTK\n+\tifeq ($(ls git-gui),)\n+\t\tNO_TCLTK = NoThanks\n+\tendif\n+endif\n+\n ifeq ($(PERL_PATH),)\n NO_PERL = NoThanks\n endif\n-- \ngitgitgadget\n\n"},{"id":"397302","messageId":"9078d0872831bd51157b7623070412a3d6f3a1ad.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 10/10] .sparse: add in-tree sparse-checkout for Git","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:42Z","receivedAt":"2020-05-07T13:17:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe in-tree sparse-checkout feature allows architects to adapt their\ndependentent subsystems into sparse-checkout specificiations. This helps\nthe typical users who do not know the full build system to use the\nsparse-chekcout in a pain-free way. In particular, if the\nsparse-checkout dependencies update at the same times as build\ndependencies, then the users automatically get new sparse-checkout\ndefinitions as they switch branches.\n\nFor the Git codebase, it is not immediately obvious which directories\nare absolutely required for building Git. From my estimation, the\nnecessary directories for building and testing Git on Linux are listed\nin the .sparse/base.deps file in this change. A few more directories in\nthe compat/ dir are required for building on Windows.\n\nThis presents a new possible workflow for Git contributors, especially\nthose that want to test several new features in their workflow. The\nfollowing allows a user to set up working on Git with partial clone and\nsparse-checkout:\n\n $ git clone --sparse --filter=blob:none https://github.com/git/git\n $ cd git\n $ git sparse-checkout set --in-tree .sparse/base.deps\n $ make test\n\nPerhaps there are ways we can further reduce the size of the \"bare\nnecessities\" by rearranging code and adjusting the Makefile to match. In\nparticular, I noticed that the vcs-svn directory is required for the\nbuild. It could be helpful to create a way to build a very small version\nof \"core Git\" that doesn't include integrations with other version\ncontrol systems, and having them automatically disabled if they are\nmissing from the sparse-checkout definition would be a great way to make\nthat more accessible for contributors.\n\nSigned-off-by: Derrick Stolee \n---\n .sparse/base.deps | 19 +++++++++++++++++++\n .sparse/windows.deps | 3 +++\n 2 files changed, 22 insertions(+)\n create mode 100644 .sparse/base.deps\n create mode 100644 .sparse/windows.deps\n\ndiff --git a/.sparse/base.deps b/.sparse/base.deps\nnew file mode 100644\nindex 00000000000..b0682175dc3\n--- /dev/null\n+++ b/.sparse/base.deps\n@@ -0,0 +1,19 @@\n+[sparse]\n+\tdir = Documentation\n+\tdir = block-sha1\n+\tdir = builtin\n+\tdir = compat/.depend\n+\tdir = contrib\n+\tdir = ewah\n+\tdir = gitweb\n+\tdir = mergetools\n+\tdir = negotiator\n+\tdir = perl\n+\tdir = refs\n+\tdir = sha1dc\n+\tdir = sha256\n+\tdir = t\n+\tdir = templates\n+\tdir = trace2\n+\tdir = vcs-svn\n+\tdir = xdiff\ndiff --git a/.sparse/windows.deps b/.sparse/windows.deps\nnew file mode 100644\nindex 00000000000..6c9bf1df335\n--- /dev/null\n+++ b/.sparse/windows.deps\n@@ -0,0 +1,3 @@\n+[sparse]\n+\tinherit = .sparse/base.deps\n+\tdir = compat\n-- \ngitgitgadget\n"},{"id":"397303","messageId":"5392cb7a1065fae766b6ef3c6f59728263ec6342.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 09/10] Makefile: disable GETTEXT when 'po' is missing","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:41Z","receivedAt":"2020-05-07T13:18:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs an effort to promote \"dogfooding\" the sparse-checkout feature within\nthe Git codebase, it is helpful to explore which portions of the\ncodebase are optional.\n\nThe NO_GETTEXT build variable can disable the translation libraries.\nThe test suite then uses the NO_GETTEXT environment variable to disable\nthe GETTEXT prerequisite for some tests.\n\nThe 'po' directory contains translations for strings in the Git\ncodebase. While this stores extremely important data for Git users, the\ndata is not typically used by Git contributors in their daily work.\nThus, it could be removed from the working directory using\nsparse-checkout.\n\nHowever, doing such a removal causes some tests to fail. Part of the\nfailures are related to the GETTEXT prerequisite being enabled when it\nprobably should not be. The other part is that some tests in\nt0200-gettext-basic.sh depend on the existence of files in the po\ndirectory.\n\nIn test-lib, disable the GETTEXT prerequisite when the po directory does\nnot exist, then add the GETTEXT prerequisite to these tests that use\nfiles in that directory.\n\nIn Makefile, define NO_GETTEXT when the po directory does not exist.\nThis is necessary for \"make install\" to work correctly.\n\nSigned-off-by: Derrick Stolee \n---\n Makefile | 9 ++++++++-\n t/lib-gettext.sh | 1 -\n t/t0200-gettext-basic.sh | 6 +++---\n t/test-lib.sh | 7 ++++++-\n 4 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 70760d315cb..38c2d54349a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -72,7 +72,8 @@ all::\n #\n # Define NO_GETTEXT if you don't want Git output to be translated.\n # A translated Git requires GNU libintl or another gettext implementation,\n-# plus libintl-perl at runtime.\n+# plus libintl-perl at runtime. This will also be defined if the 'po'\n+# directory is missing.\n #\n # Define USE_GETTEXT_SCHEME and set it to 'fallthrough', if you don't trust\n # the installed gettext translation of the shell scripts output.\n@@ -1861,6 +1862,12 @@ ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\n \n+ifndef NO_GETTEXT\n+\tifeq ($(ls po),)\n+\t\tNO_GETTEXT = NoThanks\n+\tendif\n+endif\n+\n ifndef NO_TCLTK\n \tifeq ($(ls git-gui),)\n \t\tNO_TCLTK = NoThanks\ndiff --git a/t/lib-gettext.sh b/t/lib-gettext.sh\nindex 2139b427ca1..beeb45a1387 100644\n--- a/t/lib-gettext.sh\n+++ b/t/lib-gettext.sh\n@@ -7,7 +7,6 @@\n . ./test-lib.sh\n \n GIT_TEXTDOMAINDIR=\"$GIT_BUILD_DIR/po/build/locale\"\n-GIT_PO_PATH=\"$GIT_BUILD_DIR/po\"\n export GIT_TEXTDOMAINDIR GIT_PO_PATH\n \n if test -n \"$GIT_TEST_INSTALLED\"\ndiff --git a/t/t0200-gettext-basic.sh b/t/t0200-gettext-basic.sh\nindex 8853d8afb92..0f6bc941cbb 100755\n--- a/t/t0200-gettext-basic.sh\n+++ b/t/t0200-gettext-basic.sh\n@@ -15,17 +15,17 @@ test_expect_success 'sanity: $TEXTDOMAIN is git' '\n test $TEXTDOMAIN = \"git\"\n '\n \n-test_expect_success 'xgettext sanity: Perl _() strings are not extracted' '\n+test_expect_success GETTEXT 'xgettext sanity: Perl _() strings are not extracted' '\n ! grep \"A Perl string xgettext will not get\" \"$GIT_PO_PATH\"/is.po\n '\n \n-test_expect_success 'xgettext sanity: Comment extraction with --add-comments' '\n+test_expect_success GETTEXT 'xgettext sanity: Comment extraction with --add-comments' '\n grep \"TRANSLATORS: This is a test\" \"$TEST_DIRECTORY\"/t0200/* | wc -l >expect &&\n grep \"TRANSLATORS: This is a test\" \"$GIT_PO_PATH\"/is.po | wc -l >actual &&\n test_cmp expect actual\n '\n \n-test_expect_success 'xgettext sanity: Comment extraction with --add-comments stops at statements' '\n+test_expect_success GETTEXT 'xgettext sanity: Comment extraction with --add-comments stops at statements' '\n ! grep \"This is a phony\" \"$GIT_PO_PATH\"/is.po &&\n ! grep \"the above comment\" \"$GIT_PO_PATH\"/is.po\n '\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0ea1e5a05ed..ca22d23f0d2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1474,7 +1474,12 @@ test -z \"$NO_PYTHON\" && test_set_prereq PYTHON\n test -n \"$USE_LIBPCRE1$USE_LIBPCRE2\" && test_set_prereq PCRE\n test -n \"$USE_LIBPCRE1\" && test_set_prereq LIBPCRE1\n test -n \"$USE_LIBPCRE2\" && test_set_prereq LIBPCRE2\n-test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n+\n+GIT_PO_PATH=\"$GIT_BUILD_DIR/po\"\n+if test -d \"$GIT_PO_PATH\"\n+then\n+\ttest -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n+fi\n \n if test -n \"$GIT_TEST_GETTEXT_POISON_ORIG\"\n then\n-- \ngitgitgadget\n\n"},{"id":"397305","messageId":"fcf948bda7aebcc5f88c17f5b308b2ce0cc285f5.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 06/10] sparse-checkout: use oidset to prevent repeat blobs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:38Z","receivedAt":"2020-05-07T13:18:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs we parse the in-tree config files that store the sparse.dir values\nused to create an in-tree sparse-checkout definition, we can easily\navoid parsing the same file multiple times by using an oidset on those\nblobs. We only parse if the oid is new to the oidset.\n\nThis is unlikely to have a major performance benefit right now, but will\nbe extremely important when we introduce the sparse.inherit options to\nlink multiple files in a directed graph. This oidset will prevent\ninfinite loops when cycles exist in that digraph, or exponential blowups\neven in the case of a directed acyclic graph.\n\nSigned-off-by: Derrick Stolee \n---\n sparse-checkout.c | 24 +++++++++++++++++++-----\n 1 file changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nindex 6c58fda9722..d01f4d7b525 100644\n--- a/sparse-checkout.c\n+++ b/sparse-checkout.c\n@@ -9,6 +9,7 @@\n #include \"string-list.h\"\n #include \"unpack-trees.h\"\n #include \"object-store.h\"\n+#include \"oidset.h\"\n \n char *get_sparse_checkout_filename(void)\n {\n@@ -77,9 +78,12 @@ int load_in_tree_pattern_list(struct repository *r,\n \t\t\t struct string_list *sl,\n \t\t\t struct pattern_list *pl)\n {\n+\tint result = 0;\n \tstruct index_state *istate = r->index;\n \tstruct string_list_item *item;\n \tstruct strbuf path = STRBUF_INIT;\n+\tstruct oidset set;\n+\toidset_init(&set, 16);\n \n \tpl->use_cone_patterns = 1;\n \n@@ -96,24 +100,34 @@ int load_in_tree_pattern_list(struct repository *r,\n \t\t * Use -1 return to ensure populate_from_existing_patterns()\n \t\t * skips the sparse-checkout updates.\n \t\t */\n-\t\tif (pos < 0)\n-\t\t\treturn -1;\n+\t\tif (pos < 0) {\n+\t\t\tresult = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \n \t\toid = &istate->cache[pos]->oid;\n+\n+\t\tif (oidset_contains(&set, oid))\n+\t\t\tcontinue;\n+\n+\t\toidset_insert(&set, oid);\n+\n \t\ttype = oid_object_info(r, oid, NULL);\n \n \t\tif (type != OBJ_BLOB) {\n \t\t\twarning(_(\"expected a file at '%s'; not updating sparse-checkout\"),\n \t\t\t\toid_to_hex(oid));\n-\t\t\treturn 1;\n+\t\t\tresult = 1;\n+\t\t\tgoto cleanup;\n \t\t}\n \n \t\tload_in_tree_from_blob(pl, oid);\n \t}\n \n+cleanup:\n \tstrbuf_release(&path);\n-\n-\treturn 0;\n+\toidset_clear(&set);\n+\treturn result;\n }\n \n int populate_sparse_checkout_patterns(struct pattern_list *pl)\n-- \ngitgitgadget\n\n"},{"id":"397306","messageId":"9a2cb7bb5ed7a5dc039d4b47bfee83c589252a45.1588857462.git.gitgitgadget@gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"[PATCH 05/10] sparse-checkout: automatically update in-tree definition","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T13:17:37Z","receivedAt":"2020-05-07T13:18:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA benefit of having sparse-checkouts defined in the tree data is that\nthe sparse-checkout definition can change over time. To take advantage\nof that data, a user could run \"git sparse-checkout reapply\" whenever\nthey thought the sparse-checkout definition was out of date. That is\nlikely too manual of a process, since a user would probably only\ndiscover this after a failure to accomplish their goal, such as a build\nfailure.\n\nPrevent user frustration by automatically updating the sparse-checkout\ndefinition after changing the index when an in-tree definition is\nprovided by config. This will happen during every index change,\nincluding every step of a rebase, including conflict states.\n\nSpecial care was needed around the --no-sparse-checkout option in \"git\nread-tree\". This previously relied on changing the skip_sparse_checkout\noption in \"struct unpack_trees_options\" to prevent applying the\nskip-worktree bits. However, now that we make a second update to the\nindex focusing on the skip-worktree bits, this needs to be prevented in\nanother way. The simplest thing to do was disable the feature through\nthe core_apply_sparse_checkout global variable.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-sparse-checkout.txt | 7 ++++---\n builtin/commit.c | 4 +++-\n builtin/read-tree.c | 4 ++++\n read-cache.c | 8 +++++---\n sparse-checkout.c | 26 +++++++++++++++++++++++++-\n sparse-checkout.h | 1 +\n t/t1091-sparse-checkout-builtin.sh | 19 +++++++++++++++----\n 7 files changed, 57 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex da9322c5e41..c1713ebb1d2 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -233,9 +233,10 @@ Use `git sparse-checkout set --in-tree ` to initialize the patterns\n to those included in the file at ``. This will override any existing\n patterns you have in your sparse-checkout file.\n \n-After switching between commits with different versions of this file, run\n-`git sparse-checkout reapply` to adjust the sparse-checkout patterns to\n-the new definition.\n+As Git switches between commits, it will update the in-tree sparse-checkout\n+definition according to the files available at the new commit. If any of\n+the specified files do not exist at the new commit, then the sparse-checkout\n+definition will not change.\n \n \n SUBMODULES\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 7ba33a3bec4..0eab8d74469 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -36,6 +36,7 @@\n #include \"help.h\"\n #include \"commit-reach.h\"\n #include \"commit-graph.h\"\n+#include \"sparse-checkout.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [] [--] ...\"),\n@@ -222,7 +223,8 @@ static int commit_index_files(void)\n \tcase COMMIT_AS_IS:\n \t\tbreak; /* nothing to do */\n \tcase COMMIT_NORMAL:\n-\t\terr = commit_lock_file(&index_lock);\n+\t\terr = commit_lock_file(&index_lock) ||\n+\t\t update_in_tree_sparse_checkout();\n \t\tbreak;\n \tcase COMMIT_PARTIAL:\n \t\terr = commit_lock_file(&index_lock);\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex af7424b94c8..9ae81ffffa1 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -247,6 +247,10 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)\n \t\tparse_tree(tree);\n \t\tinit_tree_desc(t+i, tree->buffer, tree->size);\n \t}\n+\n+\tif (opts.skip_sparse_checkout)\n+\t\tcore_apply_sparse_checkout = 0;\n+\n \tif (unpack_trees(nr_trees, t, &opts))\n \t\treturn 128;\n \ndiff --git a/read-cache.c b/read-cache.c\nindex aa427c5c170..150e73feb0d 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -25,6 +25,7 @@\n #include \"fsmonitor.h\"\n #include \"thread-utils.h\"\n #include \"progress.h\"\n+#include \"sparse-checkout.h\"\n \n /* Mask for the name length in ce_flags in the on-disk index */\n \n@@ -3074,9 +3075,10 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n \n \tif (ret)\n \t\treturn ret;\n-\tif (flags & COMMIT_LOCK)\n-\t\tret = commit_locked_index(lock);\n-\telse\n+\tif (flags & COMMIT_LOCK) {\n+\t\tret = commit_locked_index(lock) ||\n+\t\t update_in_tree_sparse_checkout();\n+\t} else\n \t\tret = close_lock_file_gently(lock);\n \n \trun_hook_le(NULL, \"post-index-change\",\ndiff --git a/sparse-checkout.c b/sparse-checkout.c\nindex d6c27ca19c4..6c58fda9722 100644\n--- a/sparse-checkout.c\n+++ b/sparse-checkout.c\n@@ -92,9 +92,12 @@ int load_in_tree_pattern_list(struct repository *r,\n \t\t * Exit silently, as this is likely the case where Git\n \t\t * changed branches to a location where the inherit file\n \t\t * does not exist. Do not update the sparse-checkout.\n+\t\t *\n+\t\t * Use -1 return to ensure populate_from_existing_patterns()\n+\t\t * skips the sparse-checkout updates.\n \t\t */\n \t\tif (pos < 0)\n-\t\t\treturn 1;\n+\t\t\treturn -1;\n \n \t\toid = &istate->cache[pos]->oid;\n \t\ttype = oid_object_info(r, oid, NULL);\n@@ -145,6 +148,7 @@ int populate_sparse_checkout_patterns(struct pattern_list *pl)\n \treturn result;\n }\n \n+static int updating_sparse_checkout = 0;\n int update_working_directory(struct pattern_list *pl)\n {\n \tenum update_sparsity_result result;\n@@ -152,6 +156,10 @@ int update_working_directory(struct pattern_list *pl)\n \tstruct lock_file lock_file = LOCK_INIT;\n \tstruct repository *r = the_repository;\n \n+\tif (updating_sparse_checkout)\n+\t\treturn 0;\n+\tupdating_sparse_checkout = 1;\n+\n \tmemset(&o, 0, sizeof(o));\n \to.verbose_update = isatty(2);\n \to.update = 1;\n@@ -180,9 +188,24 @@ int update_working_directory(struct pattern_list *pl)\n \telse\n \t\trollback_lock_file(&lock_file);\n \n+\tupdating_sparse_checkout = 0;\n \treturn result;\n }\n \n+int update_in_tree_sparse_checkout(void)\n+{\n+\tconst char *first_value;\n+\n+\tif (!core_apply_sparse_checkout)\n+\t\treturn 0;\n+\n+\t/* only update if doing so due to sparse.inTree. */\n+\tif (!git_config_get_value(SPARSE_CHECKOUT_IN_TREE, &first_value) &&\n+\t first_value)\n+\t \treturn update_working_directory(NULL);\n+\treturn 0;\n+}\n+\n static char *escaped_pattern(char *pattern)\n {\n \tchar *p = pattern;\n@@ -273,6 +296,7 @@ int write_patterns_and_update(struct pattern_list *pl)\n \t\tfree(sparse_filename);\n \t\tclear_pattern_list(pl);\n \t\tupdate_working_directory(NULL);\n+\t\tupdating_sparse_checkout = 0;\n \t\treturn result;\n \t}\n \ndiff --git a/sparse-checkout.h b/sparse-checkout.h\nindex 993a5701a60..fb0ba48524a 100644\n--- a/sparse-checkout.h\n+++ b/sparse-checkout.h\n@@ -13,6 +13,7 @@ char *get_sparse_checkout_filename(void);\n int populate_sparse_checkout_patterns(struct pattern_list *pl);\n void write_patterns_to_file(FILE *fp, struct pattern_list *pl);\n int update_working_directory(struct pattern_list *pl);\n+int update_in_tree_sparse_checkout(void);\n int write_patterns(struct pattern_list *pl, int and_update);\n int write_patterns_and_update(struct pattern_list *pl);\n void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path);\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 1040bf9c261..fdaafba5377 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -605,6 +605,7 @@ test_expect_success MINGW 'cone mode replaces backslashes with slashes' '\n '\n \n test_expect_success 'basis of --in-tree' '\n+\tgit -C repo branch no-in-tree &&\n \tgit -C repo config auto.crlf false &&\n \tcat >folder1 <<-\\EOF &&\n \t[sparse]\n@@ -690,18 +691,28 @@ test_expect_success '\"add\" with --in-tree' '\n \tcheck_files repo a deep folder1\n '\n \n-test_expect_success 'reapply after updating in-tree file' '\n+test_expect_success 'automatically change after updating in-tree file' '\n \tgit -C repo sparse-checkout set --in-tree .sparse/sparse &&\n \tcheck_files repo a &&\n \ttest_path_is_dir repo/.sparse &&\n-\techo \"\\tdir = folder1\" >>repo/.sparse/sparse &&\n+\tprintf \"\\tdir = folder1\\n\" >>repo/.sparse/sparse &&\n \tgit -C repo commit -a -m \"Update sparse file\" &&\n-\tgit -C repo sparse-checkout reapply &&\n \tcheck_files repo a folder1 &&\n \ttest_path_is_dir repo/.sparse &&\n \tgit -C repo checkout HEAD~1 &&\n-\tgit -C repo sparse-checkout reapply &&\n \tcheck_files repo a &&\n+\ttest_path_is_dir repo/.sparse &&\n+\tgit -C repo checkout - &&\n+\tcheck_files repo a folder1 &&\n+\ttest_path_is_dir repo/.sparse\n+'\n+\n+test_expect_success 'keep definition when in-tree file is missing' '\n+\tgit -C repo checkout no-in-tree &&\n+\tcheck_files repo a folder1 &&\n+\ttest_path_is_missing repo/.sparse &&\n+\tgit -C repo checkout - &&\n+\tcheck_files repo a folder1 &&\n \ttest_path_is_dir repo/.sparse\n '\n \n-- \ngitgitgadget\n\n"},{"id":"397365","messageId":"xmqqk11n2xf3.fsf@gitster.c.googlers.com","threadId":"53425","inReplyTo":"5bfe3f3fc8a99b3d4fdd4286da17cd935090c614.1588857462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/10] unpack-trees: avoid array out-of-bounds error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-07T22:27:28Z","receivedAt":"2020-05-07T22:27:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The loop in warn_conflicted_path() that checks for the count of entries\n> with the same path uses \"i+count\" for the array entry. However, the loop\n> only verifies that the value of count is below the array size. Fix this\n> by adding i to the condition.\n>\n> I hit this condition during a test of the in-tree sparse-checkout\n> feature, so it is exercised by the end of the series.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> unpack-trees.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 9a3ccd9d083..4f880f2da90 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -563,10 +563,11 @@ static int warn_conflicted_path(struct index_state *istate,\n> \tadd_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n> \n> \t/* Find out how many higher stage entries at same path */\n> -\twhile (++count < istate->cache_nr &&\n> +\twhile (i + ++count < istate->cache_nr &&\n> \t !strcmp(conflicting_path,\n> \t\t istate->cache[i+count]->name))\n> \t\t/* do nothing */;\n\nEek. Yes, it is obvious that the original is wrong once you point\nit out. But \"i + ++count\" looks like a line noise, and funny way\nthat lines are wrapped in the original does not help X-<.\n\nWe may want to fix the style and the grammar while we are at it,\nperhaps like the attached.\n\nIn any case, thanks for a fix.\n\n unpack-trees.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 6bbf58d28e..c38938d96c 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -562,11 +562,11 @@ static int warn_conflicted_path(struct index_state *istate,\n \n \tadd_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n \n-\t/* Find out how many higher stage entries at same path */\n-\twhile (++count < istate->cache_nr &&\n-\t !strcmp(conflicting_path,\n-\t\t istate->cache[i+count]->name))\n-\t\t/* do nothing */;\n+\t/* Find out how many higher stage entries are at same path */\n+\twhile ((++count) + i < istate->cache_nr &&\n+\t !strcmp(conflicting_path, istate->cache[count + i]->name))\n+\t\t; /* do nothing */\n+\n \treturn count;\n }\n \n"},{"id":"397366","messageId":"xmqqeerv2w01.fsf@gitster.c.googlers.com","threadId":"53425","inReplyTo":"2188577cd848d7cee77f06f1ad2b181864e5e36d.1588857462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-07T22:58:06Z","receivedAt":"2020-05-07T22:58:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> One of the difficulties of using the sparse-checkout feature is not\n> knowing which directories are absolutely needed for working in a portion\n> of the repository. Some of this can be documented in README files or\n> included in a bootstrapping tool along with the repository. This is done\n> in an ad-hoc way by every project that wants to use it.\n>\n> Let's make this process easier for users by creating a way to define a\n> useful sparse-checkout definition inside the Git tree data. This has\n> several benefits. In particular, the data is available to anyone who has\n> a copy of the repository without needing a different data source.\n> Second, the needs of the repository can change over time and Git can\n> present a way to automatically update the working directory as these\n> sparse-checkout definitions change over time.\n\nAnd two lines of development can merge them together?\n\nAny time a new \"feature\" pops up that would eventually affect how\n\"git clone\" and \"git checkout\" work based on untrusted user data, we\nneed to make sure there is no negative security implications. \n\nIf it only boils down to \"we have files that can record list of\nleading directory names and without offering extra 'flexibility'\", I\nguess there aren't all that much that a malicious sparse definition\ncan do and we would be safe, though.\n\n> To use this feature, add the \"--in-tree\" option when setting or adding\n> directories to the sparse-checkout definition. For example:\n>\n> $ git sparse-checkout set --in-tree .sparse/base\n> $ git sparse-checkout add --in-tree .sparse/extra\n>\n> These commands add values to the multi-valued config setting\n> \"sparse.inTree\". When updating the sparse-checkout definition, these\n> values describe paths in the repository to find the sparse-checkout\n> data. After the commands listed earlier, we expect to see the following\n> in .git/config.worktree:\n>\n> \t[sparse]\n> \t\tintree = .sparse/base\n> \t\tintree = .sparse/extra\n\nWhat does this say in human words? \"These two tracked files specify\nwhich paths should be in the working tree\"? Spelling it out here\nwould help readers of this commit.\n\n> When applying the sparse-checkout definitions from this config, the\n> blobs at HEAD:.sparse/base and HEAD:.sparse/extra are loaded. \n\nOK, so end-user edit to the working tree copy or what is added to\nthe index does not count and only the committed version gets used.\n\nThat makes it simple---I was wondering how we would operate when\nmerging a branch with different contents in the .sparse/* files\nuntil the conflicts are resolved.\n\n> In those\n> files, the multi-valued config values \"sparse.dir\" are considered as\n> the directories to construct a cone mode sparse-checkout file. The end\n> result is as if these paths were provided to \"git sparse-checkout set\"\n> in cone mode.\n\nOK.\n\n> For example, suppose .sparse/base had the following content:\n>\n> \t[sparse]\n> \t\tdir = A\n> \t\tdir = B/C\n> \t\tdir = D/E/F\n>\n> and .sparse/extra had the following content:\n>\n> \t[sparse]\n> \t\tdir = D\n> \t\tdir = X\n>\n> Then, the output of \"git sparse-checkout list\" would be\n>\n> \tA\n> \tB/C\n> \tD\n> \tX\n>\n> Note that since \"D\" contains \"D/E/F\", that directory replaces the\n> position of \"D/E/F\" in the list.\n>\n> Since these are parsed using the config library, the parser is robust\n> enough to understand comments and complicated string values.\n>\n> The key benefit to this approach is that it can be extended by defining\n> new config values. In a later change, we will introduce \"sparse.inherit\"\n> to point to another file in the tree. This will solve the problem of\n> editing many files when core dependencies change.\n\nWith only a multi-valued sparse.dir elements allowed in these\nin-tree .sparse/* files, I guess there isn't much mischeaf a\nmalicious .sparse/* file can do. Can it try to [includeIf] some\npaths external to the repository to cause a remote attacker to learn\nabout the paths on the local system, perhaps? \n\n> diff --git a/Documentation/config/sparse.txt b/Documentation/config/sparse.txt\n> new file mode 100644\n> index 00000000000..c1fce87cd33\n> --- /dev/null\n> +++ b/Documentation/config/sparse.txt\n> @@ -0,0 +1,15 @@\n> +sparse.inTree::\n> +\tThe `core.sparseCheckout` config option enables the `sparse-checkout`\n> +\tfeature, but if there are any values for the multi-valued\n> +\t`sparse.inTree` config option, then the sparse-checkout patterns are\n> +\tdefined by parsing the files listed in these values. See\n> +\tlinkgit:git-sparse-checkout[1] for more information.\n\nDoes \"... but if ... then\" mean \"by parsing the files and ignoring\nall other things that may otherwise define patterns\"?\n\n> +sparse.dir::\n> +\tThis config setting is ignored if present in the repository config.\n> +\tInstead, this multi-valued option is present in the files listed by\n> +\t`sparse.inTree` and specifies the directories needed in the\n> +\tworking directory. The union of all `sparse.dir` values across all\n> +\t`sparse.inTree` files forms the input for `git sparse-checkout set`\n> +\tin cone mode. See linkgit:git-sparse-checkout[1] for more\n> +\tinformation.\n\nIf this is *not* a config in the usual sense, we probably should not\ninclude it in this document and in the \"git config --help\" output.\nThat will allow us to drop the first sentence.\n\nThose .sparse/* in-tree files are like .gitmodules in the sense that\nthey happen to use the same syntax so that the parser can be shared,\nbut they are not allowed to affect end-user configuration\n(e.g. writing \"[diff] external=rm -fr .\" in the file has no effect)\nat all, right?\n\nAnd we should describe what we can write in these in-tree files\nseparately and make it clear that they are _different_ from the\nconfiguration variables.\n\n"},{"id":"397401","messageId":"63dce1f7-3dbd-1540-c6e5-547962555f57@gmail.com","threadId":"53425","inReplyTo":"xmqqk11n2xf3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 01/10] unpack-trees: avoid array out-of-bounds error","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-05-08T12:19:35Z","receivedAt":"2020-05-08T12:19:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/7/2020 6:27 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n>> diff --git a/unpack-trees.c b/unpack-trees.c\n>> index 9a3ccd9d083..4f880f2da90 100644\n>> --- a/unpack-trees.c\n>> +++ b/unpack-trees.c\n>> @@ -563,10 +563,11 @@ static int warn_conflicted_path(struct index_state *istate,\n>> \tadd_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n>> \n>> \t/* Find out how many higher stage entries at same path */\n>> -\twhile (++count < istate->cache_nr &&\n>> +\twhile (i + ++count < istate->cache_nr &&\n>> \t !strcmp(conflicting_path,\n>> \t\t istate->cache[i+count]->name))\n>> \t\t/* do nothing */;\n> \n> Eek. Yes, it is obvious that the original is wrong once you point\n> it out. But \"i + ++count\" looks like a line noise, and funny way\n> that lines are wrapped in the original does not help X-<.\n> \n> We may want to fix the style and the grammar while we are at it,\n> perhaps like the attached.\n> \n> In any case, thanks for a fix.\n> \n> unpack-trees.c | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 6bbf58d28e..c38938d96c 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -562,11 +562,11 @@ static int warn_conflicted_path(struct index_state *istate,\n> \n> \tadd_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n> \n> -\t/* Find out how many higher stage entries at same path */\n> -\twhile (++count < istate->cache_nr &&\n> -\t !strcmp(conflicting_path,\n> -\t\t istate->cache[i+count]->name))\n> -\t\t/* do nothing */;\n> +\t/* Find out how many higher stage entries are at same path */\n> +\twhile ((++count) + i < istate->cache_nr &&\n> +\t !strcmp(conflicting_path, istate->cache[count + i]->name))\n> +\t\t; /* do nothing */\n> +\n\nThis looks much better, thanks!\n\nAs I mentioned in the cover letter, this is worth taking on its own. Could\nyou queue the collaborative patch? I'll eject it from the next version of\nthis series. \n\nThanks,\n-Stolee\n"},{"id":"397408","messageId":"xmqqd07e1n1r.fsf@gitster.c.googlers.com","threadId":"53425","inReplyTo":"63dce1f7-3dbd-1540-c6e5-547962555f57@gmail.com","subject":"Re: [PATCH 01/10] unpack-trees: avoid array out-of-bounds error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-08T15:09:04Z","receivedAt":"2020-05-08T15:09:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> As I mentioned in the cover letter, this is worth taking on its own. Could\n> you queue the collaborative patch? I'll eject it from the next version of\n> this series. \n\nYes, I think this is worth taking independently.\n\nI do not think people would find the updated version \"much better\",\nbut it made it readable at least to me.\n\nThanks.\n"},{"id":"397412","messageId":"6d354901-9361-d8d1-539d-3b6c3edb2d9f@gmail.com","threadId":"53425","inReplyTo":"xmqqeerv2w01.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-05-08T15:40:49Z","receivedAt":"2020-05-08T15:40:56Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/7/2020 6:58 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> One of the difficulties of using the sparse-checkout feature is not\n>> knowing which directories are absolutely needed for working in a portion\n>> of the repository. Some of this can be documented in README files or\n>> included in a bootstrapping tool along with the repository. This is done\n>> in an ad-hoc way by every project that wants to use it.\n>>\n>> Let's make this process easier for users by creating a way to define a\n>> useful sparse-checkout definition inside the Git tree data. This has\n>> several benefits. In particular, the data is available to anyone who has\n>> a copy of the repository without needing a different data source.\n>> Second, the needs of the repository can change over time and Git can\n>> present a way to automatically update the working directory as these\n>> sparse-checkout definitions change over time.\n> \n> And two lines of development can merge them together?\n> \n> Any time a new \"feature\" pops up that would eventually affect how\n> \"git clone\" and \"git checkout\" work based on untrusted user data, we\n> need to make sure there is no negative security implications. \n> \n> If it only boils down to \"we have files that can record list of\n> leading directory names and without offering extra 'flexibility'\", I\n> guess there aren't all that much that a malicious sparse definition\n> can do and we would be safe, though.\n\nYes. I hope that we can be extremely careful with this feature.\nThe RFC status of this series implicitly includes the question\n\"Should we do this at all?\" I think the benefits outweigh the\nrisks, but we can minimize those risks with very careful design\nand implementation.\n\n>> To use this feature, add the \"--in-tree\" option when setting or adding\n>> directories to the sparse-checkout definition. For example:\n>>\n>> $ git sparse-checkout set --in-tree .sparse/base\n>> $ git sparse-checkout add --in-tree .sparse/extra\n>>\n>> These commands add values to the multi-valued config setting\n>> \"sparse.inTree\". When updating the sparse-checkout definition, these\n>> values describe paths in the repository to find the sparse-checkout\n>> data. After the commands listed earlier, we expect to see the following\n>> in .git/config.worktree:\n>>\n>> \t[sparse]\n>> \t\tintree = .sparse/base\n>> \t\tintree = .sparse/extra\n> \n> What does this say in human words? \"These two tracked files specify\n> which paths should be in the working tree\"? Spelling it out here\n> would help readers of this commit.\n\nYou got it. Sounds good.\n\n>> When applying the sparse-checkout definitions from this config, the\n>> blobs at HEAD:.sparse/base and HEAD:.sparse/extra are loaded. \n> \n> OK, so end-user edit to the working tree copy or what is added to\n> the index does not count and only the committed version gets used.\n> \n> That makes it simple---I was wondering how we would operate when\n> merging a branch with different contents in the .sparse/* files\n> until the conflicts are resolved.\n\nIt's worth testing this case so we can be sure what happens.\n\n>> In those\n>> files, the multi-valued config values \"sparse.dir\" are considered as\n>> the directories to construct a cone mode sparse-checkout file. The end\n>> result is as if these paths were provided to \"git sparse-checkout set\"\n>> in cone mode.\n> \n> OK.\n> \n>> For example, suppose .sparse/base had the following content:\n>>\n>> \t[sparse]\n>> \t\tdir = A\n>> \t\tdir = B/C\n>> \t\tdir = D/E/F\n>>\n>> and .sparse/extra had the following content:\n>>\n>> \t[sparse]\n>> \t\tdir = D\n>> \t\tdir = X\n>>\n>> Then, the output of \"git sparse-checkout list\" would be\n>>\n>> \tA\n>> \tB/C\n>> \tD\n>> \tX\n>>\n>> Note that since \"D\" contains \"D/E/F\", that directory replaces the\n>> position of \"D/E/F\" in the list.\n>>\n>> Since these are parsed using the config library, the parser is robust\n>> enough to understand comments and complicated string values.\n>>\n>> The key benefit to this approach is that it can be extended by defining\n>> new config values. In a later change, we will introduce \"sparse.inherit\"\n>> to point to another file in the tree. This will solve the problem of\n>> editing many files when core dependencies change.\n> \n> With only a multi-valued sparse.dir elements allowed in these\n> in-tree .sparse/* files, I guess there isn't much mischeaf a\n> malicious .sparse/* file can do. Can it try to [includeIf] some\n> paths external to the repository to cause a remote attacker to learn\n> about the paths on the local system, perhaps? \n\nI was unaware of includes in the config format [1]. While the behavior\nshould be safe because we are only pulling very specific data from the\nconfig, it would be best to see if we can disable includes when reading\nconfig from a blob.\n\n[1] https://git-scm.com/docs/git-config#_includes\n\n> \n>> diff --git a/Documentation/config/sparse.txt b/Documentation/config/sparse.txt\n>> new file mode 100644\n>> index 00000000000..c1fce87cd33\n>> --- /dev/null\n>> +++ b/Documentation/config/sparse.txt\n>> @@ -0,0 +1,15 @@\n>> +sparse.inTree::\n>> +\tThe `core.sparseCheckout` config option enables the `sparse-checkout`\n>> +\tfeature, but if there are any values for the multi-valued\n>> +\t`sparse.inTree` config option, then the sparse-checkout patterns are\n>> +\tdefined by parsing the files listed in these values. See\n>> +\tlinkgit:git-sparse-checkout[1] for more information.\n> \n> Does \"... but if ... then\" mean \"by parsing the files and ignoring\n> all other things that may otherwise define patterns\"?\n> \n>> +sparse.dir::\n>> +\tThis config setting is ignored if present in the repository config.\n>> +\tInstead, this multi-valued option is present in the files listed by\n>> +\t`sparse.inTree` and specifies the directories needed in the\n>> +\tworking directory. The union of all `sparse.dir` values across all\n>> +\t`sparse.inTree` files forms the input for `git sparse-checkout set`\n>> +\tin cone mode. See linkgit:git-sparse-checkout[1] for more\n>> +\tinformation.\n> \n> If this is *not* a config in the usual sense, we probably should not\n> include it in this document and in the \"git config --help\" output.\n> That will allow us to drop the first sentence.\n> \n> Those .sparse/* in-tree files are like .gitmodules in the sense that\n> they happen to use the same syntax so that the parser can be shared,\n> but they are not allowed to affect end-user configuration\n> (e.g. writing \"[diff] external=rm -fr .\" in the file has no effect)\n> at all, right?\n> \n> And we should describe what we can write in these in-tree files\n> separately and make it clear that they are _different_ from the\n> configuration variables.\n\nI can move the \"keys\" from the config documentation and into the\ngit-sparse-checkout.txt file.\n\nThanks,\n-Stolee\n"},{"id":"398264","messageId":"CABPp-BGnj=7BNK51yKC6J+sfa5ibvyeO+qD6_B7Bcm1zi6q_zA@mail.gmail.com","threadId":"53425","inReplyTo":"9a2cb7bb5ed7a5dc039d4b47bfee83c589252a45.1588857462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 05/10] sparse-checkout: automatically update in-tree definition","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T16:28:53Z","receivedAt":"2020-05-20T16:29:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nSorry for the long delay. I'm trying to read through the patch series\nbut may comment out of order...\n\nOn Thu, May 7, 2020 at 6:21 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> A benefit of having sparse-checkouts defined in the tree data is that\n> the sparse-checkout definition can change over time. To take advantage\n> of that data, a user could run \"git sparse-checkout reapply\" whenever\n> they thought the sparse-checkout definition was out of date. That is\n> likely too manual of a process, since a user would probably only\n> discover this after a failure to accomplish their goal, such as a build\n> failure.\n>\n> Prevent user frustration by automatically updating the sparse-checkout\n> definition after changing the index when an in-tree definition is\n> provided by config. This will happen during every index change,\n> including every step of a rebase, including conflict states.\n\n...every step of a rebase _that writes to the index_. (Currently,\nrebase writes to the index and working tree with every commit that is\napplied, though that's a waste of time and resources; if the patch\napplies cleanly it should be done in-memory and just continue on to\nthe next one. That avoids dirtying working tree files unnecessarily\ntoo. I'm hoping to start working on it again soon...)\n\n> Special care was needed around the --no-sparse-checkout option in \"git\n> read-tree\". This previously relied on changing the skip_sparse_checkout\n> option in \"struct unpack_trees_options\" to prevent applying the\n> skip-worktree bits. However, now that we make a second update to the\n> index focusing on the skip-worktree bits, this needs to be prevented in\n> another way. The simplest thing to do was disable the feature through\n> the core_apply_sparse_checkout global variable.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/git-sparse-checkout.txt | 7 ++++---\n> builtin/commit.c | 4 +++-\n> builtin/read-tree.c | 4 ++++\n> read-cache.c | 8 +++++---\n> sparse-checkout.c | 26 +++++++++++++++++++++++++-\n> sparse-checkout.h | 1 +\n> t/t1091-sparse-checkout-builtin.sh | 19 +++++++++++++++----\n> 7 files changed, 57 insertions(+), 12 deletions(-)\n>\n> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\n> index da9322c5e41..c1713ebb1d2 100644\n> --- a/Documentation/git-sparse-checkout.txt\n> +++ b/Documentation/git-sparse-checkout.txt\n> @@ -233,9 +233,10 @@ Use `git sparse-checkout set --in-tree ` to initialize the patterns\n> to those included in the file at ``. This will override any existing\n> patterns you have in your sparse-checkout file.\n>\n> -After switching between commits with different versions of this file, run\n> -`git sparse-checkout reapply` to adjust the sparse-checkout patterns to\n> -the new definition.\n> +As Git switches between commits, it will update the in-tree sparse-checkout\n> +definition according to the files available at the new commit. If any of\n> +the specified files do not exist at the new commit, then the sparse-checkout\n> +definition will not change.\n>\n>\n> SUBMODULES\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 7ba33a3bec4..0eab8d74469 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -36,6 +36,7 @@\n> #include \"help.h\"\n> #include \"commit-reach.h\"\n> #include \"commit-graph.h\"\n> +#include \"sparse-checkout.h\"\n>\n> static const char * const builtin_commit_usage[] = {\n> N_(\"git commit [] [--] ...\"),\n> @@ -222,7 +223,8 @@ static int commit_index_files(void)\n> case COMMIT_AS_IS:\n> break; /* nothing to do */\n> case COMMIT_NORMAL:\n> - err = commit_lock_file(&index_lock);\n> + err = commit_lock_file(&index_lock) ||\n> + update_in_tree_sparse_checkout();\n> break;\n> case COMMIT_PARTIAL:\n> err = commit_lock_file(&index_lock);\n\nI like the fact that you found one simple place to change to make sure\nall places call the necessary code. I'm trying to think out loud\nabout whether that catches everything and whether it has any ill\nside-effects. Not sure if there is one, but...if someone is doing a\nmerge, a rebase, or even a checkout, then:\n\n* unpack_trees() will apply sparsity patterns (but likely the old ones\nthat weren't updated yet, right?)\n* commit_index_files() will reapply the new sparsity patterns\n\nWhile I think this doesn't present any problem from a correctness\npoint of view, is there a performance issue here? Is there a case\nwhere this will cause a huge amount of unnecessary disk activity, e.g.\nwhen old rules include some big directory that later rules don't?\n\n> diff --git a/builtin/read-tree.c b/builtin/read-tree.c\n> index af7424b94c8..9ae81ffffa1 100644\n> --- a/builtin/read-tree.c\n> +++ b/builtin/read-tree.c\n> @@ -247,6 +247,10 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)\n> parse_tree(tree);\n> init_tree_desc(t+i, tree->buffer, tree->size);\n> }\n> +\n> + if (opts.skip_sparse_checkout)\n> + core_apply_sparse_checkout = 0;\n> +\n> if (unpack_trees(nr_trees, t, &opts))\n> return 128;\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index aa427c5c170..150e73feb0d 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -25,6 +25,7 @@\n> #include \"fsmonitor.h\"\n> #include \"thread-utils.h\"\n> #include \"progress.h\"\n> +#include \"sparse-checkout.h\"\n>\n> /* Mask for the name length in ce_flags in the on-disk index */\n>\n> @@ -3074,9 +3075,10 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n>\n> if (ret)\n> return ret;\n> - if (flags & COMMIT_LOCK)\n> - ret = commit_locked_index(lock);\n> - else\n> + if (flags & COMMIT_LOCK) {\n> + ret = commit_locked_index(lock) ||\n> + update_in_tree_sparse_checkout();\n> + } else\n> ret = close_lock_file_gently(lock);\n>\n> run_hook_le(NULL, \"post-index-change\",\n> diff --git a/sparse-checkout.c b/sparse-checkout.c\n> index d6c27ca19c4..6c58fda9722 100644\n> --- a/sparse-checkout.c\n> +++ b/sparse-checkout.c\n> @@ -92,9 +92,12 @@ int load_in_tree_pattern_list(struct repository *r,\n> * Exit silently, as this is likely the case where Git\n> * changed branches to a location where the inherit file\n> * does not exist. Do not update the sparse-checkout.\n> + *\n> + * Use -1 return to ensure populate_from_existing_patterns()\n> + * skips the sparse-checkout updates.\n> */\n> if (pos < 0)\n> - return 1;\n> + return -1;\n>\n> oid = &istate->cache[pos]->oid;\n> type = oid_object_info(r, oid, NULL);\n> @@ -145,6 +148,7 @@ int populate_sparse_checkout_patterns(struct pattern_list *pl)\n> return result;\n> }\n>\n> +static int updating_sparse_checkout = 0;\n> int update_working_directory(struct pattern_list *pl)\n> {\n> enum update_sparsity_result result;\n> @@ -152,6 +156,10 @@ int update_working_directory(struct pattern_list *pl)\n> struct lock_file lock_file = LOCK_INIT;\n> struct repository *r = the_repository;\n>\n> + if (updating_sparse_checkout)\n> + return 0;\n> + updating_sparse_checkout = 1;\n> +\n> memset(&o, 0, sizeof(o));\n> o.verbose_update = isatty(2);\n> o.update = 1;\n> @@ -180,9 +188,24 @@ int update_working_directory(struct pattern_list *pl)\n> else\n> rollback_lock_file(&lock_file);\n>\n> + updating_sparse_checkout = 0;\n> return result;\n> }\n>\n> +int update_in_tree_sparse_checkout(void)\n> +{\n> + const char *first_value;\n> +\n> + if (!core_apply_sparse_checkout)\n> + return 0;\n> +\n> + /* only update if doing so due to sparse.inTree. */\n> + if (!git_config_get_value(SPARSE_CHECKOUT_IN_TREE, &first_value) &&\n> + first_value)\n> + return update_working_directory(NULL);\n> + return 0;\n> +}\n> +\n> static char *escaped_pattern(char *pattern)\n> {\n> char *p = pattern;\n> @@ -273,6 +296,7 @@ int write_patterns_and_update(struct pattern_list *pl)\n> free(sparse_filename);\n> clear_pattern_list(pl);\n> update_working_directory(NULL);\n> + updating_sparse_checkout = 0;\n> return result;\n> }\n>\n> diff --git a/sparse-checkout.h b/sparse-checkout.h\n> index 993a5701a60..fb0ba48524a 100644\n> --- a/sparse-checkout.h\n> +++ b/sparse-checkout.h\n> @@ -13,6 +13,7 @@ char *get_sparse_checkout_filename(void);\n> int populate_sparse_checkout_patterns(struct pattern_list *pl);\n> void write_patterns_to_file(FILE *fp, struct pattern_list *pl);\n> int update_working_directory(struct pattern_list *pl);\n> +int update_in_tree_sparse_checkout(void);\n> int write_patterns(struct pattern_list *pl, int and_update);\n> int write_patterns_and_update(struct pattern_list *pl);\n> void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path);\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index 1040bf9c261..fdaafba5377 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -605,6 +605,7 @@ test_expect_success MINGW 'cone mode replaces backslashes with slashes' '\n> '\n>\n> test_expect_success 'basis of --in-tree' '\n> + git -C repo branch no-in-tree &&\n> git -C repo config auto.crlf false &&\n> cat >folder1 <<-\\EOF &&\n> [sparse]\n> @@ -690,18 +691,28 @@ test_expect_success '\"add\" with --in-tree' '\n> check_files repo a deep folder1\n> '\n>\n> -test_expect_success 'reapply after updating in-tree file' '\n> +test_expect_success 'automatically change after updating in-tree file' '\n> git -C repo sparse-checkout set --in-tree .sparse/sparse &&\n> check_files repo a &&\n> test_path_is_dir repo/.sparse &&\n> - echo \"\\tdir = folder1\" >>repo/.sparse/sparse &&\n> + printf \"\\tdir = folder1\\n\" >>repo/.sparse/sparse &&\n> git -C repo commit -a -m \"Update sparse file\" &&\n> - git -C repo sparse-checkout reapply &&\n> check_files repo a folder1 &&\n> test_path_is_dir repo/.sparse &&\n> git -C repo checkout HEAD~1 &&\n> - git -C repo sparse-checkout reapply &&\n> check_files repo a &&\n> + test_path_is_dir repo/.sparse &&\n> + git -C repo checkout - &&\n> + check_files repo a folder1 &&\n> + test_path_is_dir repo/.sparse\n> +'\n> +\n> +test_expect_success 'keep definition when in-tree file is missing' '\n> + git -C repo checkout no-in-tree &&\n> + check_files repo a folder1 &&\n> + test_path_is_missing repo/.sparse &&\n> + git -C repo checkout - &&\n> + check_files repo a folder1 &&\n> test_path_is_dir repo/.sparse\n> '\n>\n> --\n> gitgitgadget\n>\n"},{"id":"398265","messageId":"CABPp-BH=DQr3cFvAvWoCOna-RV8MsWy+FNO0x4GN1QVMsYimeg@mail.gmail.com","threadId":"53425","inReplyTo":"xmqqk11n2xf3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 01/10] unpack-trees: avoid array out-of-bounds error","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T16:32:08Z","receivedAt":"2020-05-20T16:32:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 7, 2020 at 3:29 PM Junio C Hamano wrote:\n>\n> \"Derrick Stolee via GitGitGadget\" writes:\n>\n> > From: Derrick Stolee \n> >\n> > The loop in warn_conflicted_path() that checks for the count of entries\n> > with the same path uses \"i+count\" for the array entry. However, the loop\n> > only verifies that the value of count is below the array size. Fix this\n> > by adding i to the condition.\n> >\n> > I hit this condition during a test of the in-tree sparse-checkout\n> > feature, so it is exercised by the end of the series.\n> >\n> > Signed-off-by: Derrick Stolee \n> > ---\n> > unpack-trees.c | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/unpack-trees.c b/unpack-trees.c\n> > index 9a3ccd9d083..4f880f2da90 100644\n> > --- a/unpack-trees.c\n> > +++ b/unpack-trees.c\n> > @@ -563,10 +563,11 @@ static int warn_conflicted_path(struct index_state *istate,\n> > add_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n> >\n> > /* Find out how many higher stage entries at same path */\n> > - while (++count < istate->cache_nr &&\n> > + while (i + ++count < istate->cache_nr &&\n> > !strcmp(conflicting_path,\n> > istate->cache[i+count]->name))\n> > /* do nothing */;\n>\n> Eek. Yes, it is obvious that the original is wrong once you point\n> it out. But \"i + ++count\" looks like a line noise, and funny way\n> that lines are wrapped in the original does not help X-<.\n\nEek, indeed. :-(\n\n> We may want to fix the style and the grammar while we are at it,\n> perhaps like the attached.\n>\n> In any case, thanks for a fix.\n>\n> unpack-trees.c | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 6bbf58d28e..c38938d96c 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -562,11 +562,11 @@ static int warn_conflicted_path(struct index_state *istate,\n>\n> add_rejected_path(o, WARNING_SPARSE_UNMERGED_FILE, conflicting_path);\n>\n> - /* Find out how many higher stage entries at same path */\n> - while (++count < istate->cache_nr &&\n> - !strcmp(conflicting_path,\n> - istate->cache[i+count]->name))\n> - /* do nothing */;\n> + /* Find out how many higher stage entries are at same path */\n> + while ((++count) + i < istate->cache_nr &&\n> + !strcmp(conflicting_path, istate->cache[count + i]->name))\n> + ; /* do nothing */\n> +\n> return count;\n> }\n\nThanks, both.\n"},{"id":"398267","messageId":"CABPp-BEkf0TVTt4=adJ9x70j814frL932vxyQCpm74AQiHWwGQ@mail.gmail.com","threadId":"53425","inReplyTo":"fcf948bda7aebcc5f88c17f5b308b2ce0cc285f5.1588857462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 06/10] sparse-checkout: use oidset to prevent repeat blobs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T16:40:46Z","receivedAt":"2020-05-20T16:41:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 7, 2020 at 6:21 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> As we parse the in-tree config files that store the sparse.dir values\n> used to create an in-tree sparse-checkout definition, we can easily\n> avoid parsing the same file multiple times by using an oidset on those\n> blobs. We only parse if the oid is new to the oidset.\n>\n> This is unlikely to have a major performance benefit right now, but will\n> be extremely important when we introduce the sparse.inherit options to\n> link multiple files in a directed graph. This oidset will prevent\n> infinite loops when cycles exist in that digraph, or exponential blowups\n> even in the case of a directed acyclic graph.\n\nI'm still not sure if I like the idea of having a mirror dependency\nstructure separate from (and duplicative of) the build code; I'm still\nmulling that over.\n\nIt's good that you've protected against infinite loops.\n\nIs there any reason to prefer swallowing infinite loops rather than\nwarning or flagging as an error? (I'm not sure, just thinking my\nquestions out loud.)\n\n> Signed-off-by: Derrick Stolee \n> ---\n> sparse-checkout.c | 24 +++++++++++++++++++-----\n> 1 file changed, 19 insertions(+), 5 deletions(-)\n>\n> diff --git a/sparse-checkout.c b/sparse-checkout.c\n> index 6c58fda9722..d01f4d7b525 100644\n> --- a/sparse-checkout.c\n> +++ b/sparse-checkout.c\n> @@ -9,6 +9,7 @@\n> #include \"string-list.h\"\n> #include \"unpack-trees.h\"\n> #include \"object-store.h\"\n> +#include \"oidset.h\"\n>\n> char *get_sparse_checkout_filename(void)\n> {\n> @@ -77,9 +78,12 @@ int load_in_tree_pattern_list(struct repository *r,\n> struct string_list *sl,\n> struct pattern_list *pl)\n> {\n> + int result = 0;\n> struct index_state *istate = r->index;\n> struct string_list_item *item;\n> struct strbuf path = STRBUF_INIT;\n> + struct oidset set;\n> + oidset_init(&set, 16);\n>\n> pl->use_cone_patterns = 1;\n>\n> @@ -96,24 +100,34 @@ int load_in_tree_pattern_list(struct repository *r,\n> * Use -1 return to ensure populate_from_existing_patterns()\n> * skips the sparse-checkout updates.\n> */\n> - if (pos < 0)\n> - return -1;\n> + if (pos < 0) {\n> + result = -1;\n> + goto cleanup;\n> + }\n>\n> oid = &istate->cache[pos]->oid;\n> +\n> + if (oidset_contains(&set, oid))\n> + continue;\n> +\n> + oidset_insert(&set, oid);\n> +\n> type = oid_object_info(r, oid, NULL);\n>\n> if (type != OBJ_BLOB) {\n> warning(_(\"expected a file at '%s'; not updating sparse-checkout\"),\n> oid_to_hex(oid));\n> - return 1;\n> + result = 1;\n> + goto cleanup;\n> }\n>\n> load_in_tree_from_blob(pl, oid);\n> }\n>\n> +cleanup:\n> strbuf_release(&path);\n> -\n> - return 0;\n> + oidset_clear(&set);\n> + return result;\n> }\n>\n> int populate_sparse_checkout_patterns(struct pattern_list *pl)\n> --\n> gitgitgadget\n>\n"},{"id":"398280","messageId":"CABPp-BEz+XE48_vmEa4jmH53YSHUoj5G-OY+UT1mv92irQVEkQ@mail.gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T17:38:30Z","receivedAt":"2020-05-20T17:38:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 7, 2020 at 6:20 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> This is based on ds/sparse-allow-empty-working-tree.\n>\n> BACKGROUND\n> ==========\n>\n> After discussing the future of sparse-checkout at the Git Contributor\n> Summit, Minh Tai caught me during the break to introduce a very interesting\n> idea: in-tree sparse-checkout definitions.\n>\n> Here is the context: I was talking about how the Office team is building a\n> tool to update the sparse-checkout definition dynamically as their build\n> definitions change. They have a very compartmentalized build system, and\n> certain \"projects\" depend on each other. To ensure that builds do not break\n> as dependencies change, the plan was to have this tool run as a hook when\n> HEAD changes. This is a bit expensive to do, and we were discussing how much\n> we need this to be automated versus a part of the build process.\n>\n> This kind of idea would need to be replicated for every team that wants to\n> use sparse-checkout in this way. That doesn't bode well for wide adoption of\n> the feature.\n>\n> IN-TREE SPARSE-CHECKOUT DEFINITIONS\n> ===================================\n>\n> Minh's idea was simple: have sparse-checkout files in the working directory\n> and use config to point to them. As these in-tree files update, we can\n> automatically update the sparse-checkout definition accordingly. Now, the\n> only thing to do would be to ensure that the sparse-checkout files are\n> updated when someone updates the build definitions. This requires some extra\n> build validation, but would not require special tools built on every client.\n\nI'm not sure how well having a parallel dependency structure will be\nreceived, but the idea of auto-updating does seem really nice. (I put\na hook in the build system that compares the timestamps on the files\nthat define dependency structures to the .git/info/sparse-checkout\nfile, and re-run `git sparse-checkout reapply`, which helps with\nupdating but is still delayed from the checkout/merge/rebase that\nbrought in new dependencies.)\n\n> As soon as I sat down to implement this idea, I discovered some\n> complications to that basic idea. Each of these sparse-checkout definitions\n> should fit a \"role\" on the team. To continue with the Office analogy,\n> suppose we had a definition for developers working on each of Word,\n> PowerPoint, and Excel. What happens when someone wants to port a feature\n> from Excel to Word? That user would want to take the union of the two\n> sparse-checkout definitions. But what does that mean when we are working\n> with arbitrary sparse-checkout files?\n>\n> This leads to the restriction that I built into this feature: we only care\n> about \"cone mode\" definitions. These rely on directory-based prefix matches\n> that are very fast. With this restriction, it is simple to understand what\n> the \"union\" operation should do: take all directories that would be included\n> by either.\n\nSeems like you're adopting my idea of pushing folks towards \"cone\nmode\" and limiting support for non-code modes[1], eh? ;-)\n\nI'm totally on board. :-)\n\n[1] https://lore.kernel.org/git/CABPp-BHKYR9QBcAG_pM6DniaeGS40X=ErKGGsvWa_KhogCZzEA@mail.gmail.com/\n\n> Once we restrict to cone mode, there is no reason to continue storing files\n> using the sparse-checkout file format. The list of patterns is larger than\n> the input to \"git sparse-checkout set\" or output from \"git sparse-checkout\n> list\" in cone mode, and more prone to user error. Thus, let's be simpler and\n> use a list of directories to specify the sparse-checkout definition.\n\nAhah, you did come around to using a new config file[2]. ;-)\n\nI like having simpler config files; sounds great.\n\n[2] https://lore.kernel.org/git/c919513a-f41f-2ce8-60ed-e0b0733c0c7f@gmail.com/\n (\"another config file\")\n\n> This leads to the second issue that complicates things. If a build\n> dependency changes to a core library, but there are N \"roles\" that depend on\n> that library, then the person changing that library also needs to change and\n> validate N sparse-checkout definitions! This does not scale well. So, let's\n> take a page from build dependencies and allow these in-tree sparse-checkout\n> definitions to depend on each other!\n>\n> The end result is a mechanism that should be flexible enough to future needs\n> but simple enough to build using existing Git features. The format re-uses\n> the config file format. This provides good re-use of code as well as being\n> something easy to extend in the future. In Patch 4, I tried to describe\n> several alternatives and why this is the best of those options. I'm open to\n> suggestions here.\n>\n> OUTLINE\n> =======\n>\n> * Patch 1 is a bugfix that should work on its own. I caught it in tests\n> after Patch 5.\n>\n>\n> * Patches 2-7 implement the feature.\n>\n>\n> * Patches 8-9 make the Git build system a bit more robust to missing\n> directories.\n>\n>\n> * Patch 10 adds a .sparse directory to Git as well as two in-tree\n> sparse-checkout definitions. One is for a bare-bones Linux developer and\n> the other adds the compat code for Windows on top.\n>\n>\n>\n> As mentioned in patch 10, this allows the following workflow for Git\n> contributors that want to \"eat our own dogfood\" with the partial clone and\n> sparse-checkout features:\n>\n> $ git clone --sparse --filter=blob:none https://github.com/git/git\n> $ cd git\n> $ git sparse-checkout set --in-tree .sparse/base.deps\n>\n> Then, we have a smaller working directory but can still build and test the\n> code.\n>\n> FUTURE DIRECTIONS\n> =================\n>\n> There are definitely ways to make this feature more seamless, or more useful\n> to us.\n>\n> * It would be nice to extend git clone to have a string value to\n> --sparse= and possibly have it multi-valued. This would initialize\n> the in-tree sparse-checkout definition immediately upon cloning.\n\nAlso in terms of future directions, I would like to have the option\nfor these to be passed to --filter instead of using --filter:none.\n\nAnd on top of that, it may also make sense for users to be able to\nfetch cones of history somehow. For example, if they started with\n`git clone --sparse=featureA ...` and later wanted to add the\n'featureB' cone, it'd be nice to be able to run some kind of `git\nfetch --sparse=featureB ...` command when they know they are on a good\nnetwork to download the history of objects that touched the featureB\ncone instead of relying on the normal lazy fetching of partial clones.\n\nThis kind of thing could be especially useful if, say, we were in a\npandemic and everyone was forced to work from home. But it'd also be\nuseful if you just had people travelling a lot and stuck in third\nworld places with low connectivity, like CalTrain. ;-)\n\n> * I think there are some ways we could reorganize the Git codebase to make\n> our \"most basic\" sparse-checkout file have even fewer files. Mostly, it\n> would require modifying the build to ask \"do these files exist on-disk?\n> if not, then disable this build option.\" This would also require adding\n> test dependencies that also disable some tests when those do not exist.\n>\n>\n> * Currently, if we have an in-tree sparse-checkout definition, then we\n> can't add more directories to it manually. If someone runs git\n> sparse-checkout set --in-tree followed by git sparse-checkout add\n> , then will disappear as it is overridden by the in-tree definition.\n> The only way I can think to get around this would be to add these\n> directories through Git config, since we were using the sparse-checkout\n> file as the storage of \"what directories do I care about?\" but it is no\n> longer the source of truth in this situation. I'm open to ideas here, and\n> it would be nice to have this interaction hardened before moving the\n> series out of RFC status.\n\nThis seems like a big problem to me. Actually, it sounds like two big problems.\n\n1) non-extensibility\n\nI suspect most will be happy enough with the in-tree definitions, but\nthere are always a few users that want to add to the pre-defined\nsparsity sets to get some additional directories. We could use\n.gitignore as an example; although people have shared ignore rules in\n.gitignore, we still allow user-defined additional ignore rules that\nare not meant to be shared. Perhaps we can allow users to use some\npre-defined additional file (similar to .git/info/exclude) or even\nuser-defined additional file (similar to core.excludesFile)?\n\nWe would still need to be careful to not allow the in-tree definitions\nto depend on files that are not in-tree, of course.\n\n(Even with my `sparsify` script which allowed things like `./sparsify\n--modules moduleA moduleB` and which walked build system dependencies\nto get the right directories, I still allowed additional directories\nto be specified within a .git/info/sparse-checkout-user file. Not\nsure I liked the location of the file (it's sometimes hard to find for\nusers of worktrees), or the basename of the file I randomly chose that\nday, but it came in handy for a few people.)\n\n2) Ugly, overly verbose (and even error-prone?) UI.\n\nIf `git sparse-checkout set --in-tree ` means that any\nsubsequent checkout will blow apart any user-ran `git sparse-checkout\nadd `, then we really need to give at least warning and maybe\neven error messages if people are using in-tree mode and use either\nset or add. Also, this feels like we're just about deprecating the\nusage of set and add without the --in-tree flag, which could lead to\nyears of questions of \"Why do I have to specify --in-tree every time I\nrun these commands?\" Can we avoid that somehow?\n\nMaybe we just take advantage of the huge backward compatibility\nwarning we put in place and require non-in-tree mode to specify an\nadditional flag? Maybe we just check the argument to set or add when\nin cone mode, and if it's a file (instead of a directory) then we\nautomatically assume --in-tree (or attempt to parse it first and throw\nan error if it isn't in the --in-tree format)? Maybe we pick a\ndifferent verb than add or set ('use') and guide people to it instead\nof `add` and `set`?\n\nAlso, as a side note, can people specify multiple in-tree files at\nonce (`git sparse-checkout set --in-tree `)?\n"},{"id":"398282","messageId":"CABPp-BH5p1VPXfMOyN_0SLnsFKkRU9R-ZpiAe4k5r=ZUbHeibQ@mail.gmail.com","threadId":"53425","inReplyTo":"6d354901-9361-d8d1-539d-3b6c3edb2d9f@gmail.com","subject":"Re: [PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T17:52:41Z","receivedAt":"2020-05-20T17:52:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 8, 2020 at 8:42 AM Derrick Stolee wrote:\n>\n> On 5/7/2020 6:58 PM, Junio C Hamano wrote:\n> > \"Derrick Stolee via GitGitGadget\" writes:\n> >\n> >> One of the difficulties of using the sparse-checkout feature is not\n> >> knowing which directories are absolutely needed for working in a portion\n> >> of the repository. Some of this can be documented in README files or\n> >> included in a bootstrapping tool along with the repository. This is done\n> >> in an ad-hoc way by every project that wants to use it.\n> >>\n> >> Let's make this process easier for users by creating a way to define a\n> >> useful sparse-checkout definition inside the Git tree data. This has\n> >> several benefits. In particular, the data is available to anyone who has\n> >> a copy of the repository without needing a different data source.\n> >> Second, the needs of the repository can change over time and Git can\n> >> present a way to automatically update the working directory as these\n> >> sparse-checkout definitions change over time.\n> >\n> > And two lines of development can merge them together?\n> >\n> > Any time a new \"feature\" pops up that would eventually affect how\n> > \"git clone\" and \"git checkout\" work based on untrusted user data, we\n> > need to make sure there is no negative security implications.\n> >\n> > If it only boils down to \"we have files that can record list of\n> > leading directory names and without offering extra 'flexibility'\", I\n> > guess there aren't all that much that a malicious sparse definition\n> > can do and we would be safe, though.\n>\n> Yes. I hope that we can be extremely careful with this feature.\n> The RFC status of this series implicitly includes the question\n> \"Should we do this at all?\" I think the benefits outweigh the\n> risks, but we can minimize those risks with very careful design\n> and implementation.\n>\n> >> To use this feature, add the \"--in-tree\" option when setting or adding\n> >> directories to the sparse-checkout definition. For example:\n> >>\n> >> $ git sparse-checkout set --in-tree .sparse/base\n> >> $ git sparse-checkout add --in-tree .sparse/extra\n> >>\n> >> These commands add values to the multi-valued config setting\n> >> \"sparse.inTree\". When updating the sparse-checkout definition, these\n> >> values describe paths in the repository to find the sparse-checkout\n> >> data. After the commands listed earlier, we expect to see the following\n> >> in .git/config.worktree:\n> >>\n> >> [sparse]\n> >> intree = .sparse/base\n> >> intree = .sparse/extra\n> >\n> > What does this say in human words? \"These two tracked files specify\n> > which paths should be in the working tree\"? Spelling it out here\n> > would help readers of this commit.\n>\n> You got it. Sounds good.\n>\n> >> When applying the sparse-checkout definitions from this config, the\n> >> blobs at HEAD:.sparse/base and HEAD:.sparse/extra are loaded.\n> >\n> > OK, so end-user edit to the working tree copy or what is added to\n> > the index does not count and only the committed version gets used.\n> >\n> > That makes it simple---I was wondering how we would operate when\n> > merging a branch with different contents in the .sparse/* files\n> > until the conflicts are resolved.\n>\n> It's worth testing this case so we can be sure what happens.\n\nDuring a merge or rebase or checkout -m, what happens if .sparse/extra\nhas the following working tree content:\n\n[sparse]\n dir = D\n dir = X\n<<<<<< HEAD\n dir = Y\n|||||| MERGE_BASE\n======\n inherit = .sparse/tools\n>>>>>> MERGE_HEAD\n inherit = .sparse/base\n\nand, of course, three different entries in the index?\n\nAlso, do we use the version of the --in-tree file from the latest\ncommit, from the index, or from the working tree? (This is a question\nnot only for merge and rebase, but also checkout with dirty changes\nand even checkout -m.) Which one \"wins\"?\n\nAnd what if the user updates and commits an ill-formed version of the\nfile -- is it equivalent to getting an empty cone with just the\ntoplevel directory, equivalent to getting a complete checkout of\neverything, or something else?\n"},{"id":"398283","messageId":"CABPp-BEz42zvT_Vsu2xxg9RnuhBZ2aF8b+KYEu-CW=bMGQOC=Q@mail.gmail.com","threadId":"53425","inReplyTo":"f55cd0fb3897db9ca22156347293ca830cdf018c.1588857462.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 07/10] sparse-checkout: define in-tree dependencies","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-20T18:10:20Z","receivedAt":"2020-05-20T18:10:34Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 7, 2020 at 6:22 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> As mentioned in the definition of the in-tree sparse-checkout\n> definitions, a large repository could have many different roles for the\n> users of that repository. That means that many different in-tree\n> sparse-checkout definitions need to exist. If a directory needs to be\n> added to many of these roles, then many of these files need to be\n> edited. This is too much work to scale properly.\n>\n> Instead, let's make these definitions easier to maintain by following\n> basic principles of build dependencies. Add simple links between these\n> in-tree files using the new \"sparse.inherit\" config key. This multi-\n> valued config setting specifies more files that are needed to define the\n> sparse-checkout for a role. By separating out the common directories\n> into a base file, the more specialized roles can focus their files on\n> the directories only needed by those roles and leave the common\n> directories to that base file.\n>\n> For example, suppose that .sparse/base has the following data:\n>\n> [sparse]\n> dir = A\n> dir = B/C\n> dir = D/E/F\n>\n> and .sparse/extra has the following data:\n>\n> [sparse]\n> dir = D\n> dir = X\n> inherit = .sparse/base\n>\n> Now, a user can run \"git sparse-checkout set --in-tree .sparse/extra\"\n> and the resulting directories will define their cone-mode\n> sparse-checkout patterns:\n>\n> A\n> B/C\n> D\n> X\n>\n> Thus, the resulting sparse-checkout definition is the union of the\n> definitions from .sparse/base and .sparse/extra, but the user only has\n> one value of sparse.inTree of \".sparse/extra\".\n>\n> It is simple to modify our existing logic to explore the directed graph\n> created by the sparse.inherit values. We simply need to append to the\n> string_list containing the list of in-tree files. Since we never repeat\n> parsing on the same blob oid, this will not lead to infinite loops or\n> exponential blowups in the parsing time.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/git-sparse-checkout.txt | 26 ++++++++++++++++++++++\n> sparse-checkout.c | 27 ++++++++++++++++++-----\n> sparse-checkout.h | 1 +\n> t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++\n> 4 files changed, 80 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\n> index c1713ebb1d2..941658e0011 100644\n> --- a/Documentation/git-sparse-checkout.txt\n> +++ b/Documentation/git-sparse-checkout.txt\n> @@ -238,6 +238,32 @@ definition according to the files available at the new commit. If any of\n> the specified files do not exist at the new commit, then the sparse-checkout\n> definition will not change.\n>\n> +In a very large repository, there may be need to have multiple of these\n> +in-tree sparse-checkout definitions to fit the roles of multiple types of\n> +users. As each definition grows and the number of user types grow, it can\n> +be difficult to manage updating all of the definitions when a common core\n> +is modified somehow. For this reason, the in-tree pattern sets can inherit\n> +the directories from other in-tree pattern sets. Use the `sparse.inherit`\n> +option to specify other files in the tree.\n> +\n> +For example, suppose the file listed earlier is at `.sparse/core`. Another\n> +file could be stored as `.sparse/extra` with contents\n> +\n> +----------------------------------\n> +[sparse]\n> + dir = X\n> + dir = Y/Z\n> + inherit = .sparse/core\n> +----------------------------------\n\nSo you apparently expect full path rather than a relative path. This\nexample does imply this, but it might be good to explicitly state it.\n\nWhat happens if the user specifies a non-existent path, or perhaps\nequivalently, didn't understand and specified the path using a\nrelative path specification? What if the user specifies a path that\ndoesn't exist in the commit, but happens to exist in someones's\nworking directory? (And maybe even did it intentionally as a way to\ncheat and add user-defined additional paths to include? Do we want to\nallow that, or do we want to enforce that extra includes use some kind\nof pre-defined path?) What if they specify some path that is invalid\nto record in a git commit (.git/mydeps or\n../../../../../../../otherdeps) but happens to exist on some machines?\n (Are there future security vulnerabilities of any sort going down\nthis path?)\n\n> +\n> +Then, if you run `git sparse-checkout set --in-tree .sparse/extra`, the\n> +sparse-checkout definition will include `X` and `Y/Z` from `.sparse/extra`\n> +as well as `A`, `B/C`, and `D/E/F` from `.sparse/core`. This is similar\n> +to specifying both `.sparse/core` and `.sparse/extra` in the `set`\n> +subcommand, but has a slight advantage. If the `.sparse/extra` file changes\n> +the set of `inherit` files, then your sparse-checkout definition will\n> +update accordingly as you switch between commits.\n> +\n>\n> SUBMODULES\n> ----------\n> diff --git a/sparse-checkout.c b/sparse-checkout.c\n> index d01f4d7b525..4edeab49a10 100644\n> --- a/sparse-checkout.c\n> +++ b/sparse-checkout.c\n> @@ -66,12 +66,29 @@ static int sparse_dir_cb(const char *var, const char *value, void *data)\n> return 0;\n> }\n>\n> +static int sparse_inherit_cb(const char *var, const char *value, void *data)\n> +{\n> + struct string_list *sl = (struct string_list *)data;\n> +\n> + if (!strcmp(var, SPARSE_CHECKOUT_INHERIT))\n> + string_list_append(sl, value);\n\nSo anyone can record lots of additional items within these files, and\nwe'll ignore them and only pay attention to certain fields. That\nprobably allows future versions of git to write new values and old\nversions of git to not choke on them. Does it lead to entrepreneurial\ndevelopers cramming random additional fields in these files that\npossibly conflict with fieldnames that we want to use in the future?\nDo we want to document what is allowed and what is not?\n\n> +\n> + return 0;\n> +}\n> +\n> static int load_in_tree_from_blob(struct pattern_list *pl,\n> - struct object_id *oid)\n> + struct object_id *oid,\n> + struct string_list *inherit)\n> {\n> - return git_config_from_blob_oid(sparse_dir_cb,\n> - SPARSE_CHECKOUT_DIR,\n> - oid, pl);\n> + if (git_config_from_blob_oid(sparse_dir_cb,\n> + SPARSE_CHECKOUT_DIR,\n> + oid, pl))\n> + return 1;\n> + if (git_config_from_blob_oid(sparse_inherit_cb,\n> + SPARSE_CHECKOUT_INHERIT,\n> + oid, inherit))\n> + return 1;\n> + return 0;\n> }\n>\n> int load_in_tree_pattern_list(struct repository *r,\n> @@ -121,7 +138,7 @@ int load_in_tree_pattern_list(struct repository *r,\n> goto cleanup;\n> }\n>\n> - load_in_tree_from_blob(pl, oid);\n> + load_in_tree_from_blob(pl, oid, sl);\n> }\n>\n> cleanup:\n> diff --git a/sparse-checkout.h b/sparse-checkout.h\n> index fb0ba48524a..8b766ea38fb 100644\n> --- a/sparse-checkout.h\n> +++ b/sparse-checkout.h\n> @@ -5,6 +5,7 @@\n> #include \"repository.h\"\n>\n> #define SPARSE_CHECKOUT_DIR \"sparse.dir\"\n> +#define SPARSE_CHECKOUT_INHERIT \"sparse.inherit\"\n> #define SPARSE_CHECKOUT_IN_TREE \"sparse.intree\"\n>\n> struct pattern_list;\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index fdaafba5377..b2389e5b5e6 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -716,4 +716,35 @@ test_expect_success 'keep definition when in-tree file is missing' '\n> test_path_is_dir repo/.sparse\n> '\n>\n> +test_expect_success 'inherit definition from other files' '\n> + cat >repo/.sparse/inherit <<-EOF &&\n> + [sparse]\n> + inherit = .sparse/sparse\n> + inherit = .sparse/deep\n> + inherit = .sparse/deeper1\n> + EOF\n> + git -C repo add .sparse &&\n> + git -C repo commit -m \"Add inherited file\" &&\n> + git -C repo sparse-checkout set --in-tree .sparse/inherit &&\n> + check_files repo a deep folder1 &&\n> + check_files repo/deep a deeper1 deeper2 &&\n> + test_path_is_dir repo/.sparse &&\n> + cat >repo/.sparse/sparse <<-EOF &&\n> + [sparse]\n> + dir = .sparse\n> + EOF\n> + git -C repo commit -a -m \"drop folder1 from sparse\" &&\n> + check_files repo a deep &&\n> + check_files repo/deep a deeper1 deeper2 &&\n> + test_path_is_dir repo/.sparse\n> +'\n> +\n> +test_expect_success 'inherit files can have cycles' '\n> + echo \"\\tinherit = .sparse/inherit\" >>repo/.sparse/sparse &&\n> + git -C repo commit -a -m \"create inherit cycle\" &&\n> + check_files repo a deep &&\n> + check_files repo/deep a deeper1 deeper2 &&\n> + test_path_is_dir repo/.sparse\n> +'\n> +\n> test_done\n> --\n> gitgitgadget\n>\n"},{"id":"398325","messageId":"CABPp-BGcjpJOht7ip_cPcHEtd3kx5fCwm=i19narWCEfCUwWAQ@mail.gmail.com","threadId":"53425","inReplyTo":"CABPp-BEkf0TVTt4=adJ9x70j814frL932vxyQCpm74AQiHWwGQ@mail.gmail.com","subject":"Re: [PATCH 06/10] sparse-checkout: use oidset to prevent repeat blobs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-21T03:49:20Z","receivedAt":"2020-05-21T03:49:35Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Replying to my own questions...\n\nOn Wed, May 20, 2020 at 9:40 AM Elijah Newren wrote:\n>\n> On Thu, May 7, 2020 at 6:21 AM Derrick Stolee via GitGitGadget\n> wrote:\n> >\n> > From: Derrick Stolee \n> >\n> > As we parse the in-tree config files that store the sparse.dir values\n> > used to create an in-tree sparse-checkout definition, we can easily\n> > avoid parsing the same file multiple times by using an oidset on those\n> > blobs. We only parse if the oid is new to the oidset.\n> >\n> > This is unlikely to have a major performance benefit right now, but will\n> > be extremely important when we introduce the sparse.inherit options to\n> > link multiple files in a directed graph. This oidset will prevent\n> > infinite loops when cycles exist in that digraph, or exponential blowups\n> > even in the case of a directed acyclic graph.\n>\n> I'm still not sure if I like the idea of having a mirror dependency\n> structure separate from (and duplicative of) the build code; I'm still\n> mulling that over.\n\nI mentioned this to a few other buildsystem folks at $DAYJOB. They\nwere strongly opposed to having more than one source of truth, but\ngenerating the git in-tree sparse values from the official build\nsystem files, with commit hooks and build system checks to make sure\nthey get updated seemed like it'd be fine or not concern them much.\n\n> It's good that you've protected against infinite loops.\n>\n> Is there any reason to prefer swallowing infinite loops rather than\n> warning or flagging as an error? (I'm not sure, just thinking my\n> questions out loud.)\n\nThe buildsystem folks also reminded me that we have cylic dependencies\nalready, and although it's absolutely ugly, it is somewhat forced on\nus by a combination of different external tools that we can't change.\nAs such, warnings or errors would be really annoying and we'd be one\nof the ones to want to turn them off. So drop that idea from me. :-)\n"},{"id":"398372","messageId":"b78b124a-81f2-933a-775d-1d8256e83e05@gmail.com","threadId":"53425","inReplyTo":"CABPp-BGcjpJOht7ip_cPcHEtd3kx5fCwm=i19narWCEfCUwWAQ@mail.gmail.com","subject":"Re: [PATCH 06/10] sparse-checkout: use oidset to prevent repeat blobs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-05-21T17:54:36Z","receivedAt":"2020-05-21T17:54:40Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/20/2020 11:49 PM, Elijah Newren wrote:\n> Replying to my own questions...\n> \n> On Wed, May 20, 2020 at 9:40 AM Elijah Newren wrote:\n>>\n>> On Thu, May 7, 2020 at 6:21 AM Derrick Stolee via GitGitGadget\n>> wrote:\n>>>\n>>> From: Derrick Stolee \n>>>\n>>> As we parse the in-tree config files that store the sparse.dir values\n>>> used to create an in-tree sparse-checkout definition, we can easily\n>>> avoid parsing the same file multiple times by using an oidset on those\n>>> blobs. We only parse if the oid is new to the oidset.\n>>>\n>>> This is unlikely to have a major performance benefit right now, but will\n>>> be extremely important when we introduce the sparse.inherit options to\n>>> link multiple files in a directed graph. This oidset will prevent\n>>> infinite loops when cycles exist in that digraph, or exponential blowups\n>>> even in the case of a directed acyclic graph.\n>>\n>> I'm still not sure if I like the idea of having a mirror dependency\n>> structure separate from (and duplicative of) the build code; I'm still\n>> mulling that over.\n> \n> I mentioned this to a few other buildsystem folks at $DAYJOB. They\n> were strongly opposed to having more than one source of truth, but\n> generating the git in-tree sparse values from the official build\n> system files, with commit hooks and build system checks to make sure\n> they get updated seemed like it'd be fine or not concern them much.\n\nThe intention is that these files are \"downstream\" from the build\nsystem. As the build changes, it would adjust the config files to\ntell Git what to do.\n\n>> It's good that you've protected against infinite loops.\n>>\n>> Is there any reason to prefer swallowing infinite loops rather than\n>> warning or flagging as an error? (I'm not sure, just thinking my\n>> questions out loud.)\n> \n> The buildsystem folks also reminded me that we have cylic dependencies\n> already, and although it's absolutely ugly, it is somewhat forced on\n> us by a combination of different external tools that we can't change.\n> As such, warnings or errors would be really annoying and we'd be one\n> of the ones to want to turn them off. So drop that idea from me. :-)\n\nThis is more common than one might think!\n\nThanks for taking a look,\n-Stolee\n"},{"id":"398938","messageId":"CABPp-BEOb2-7xPfkbf2GBNxgGYpMyWXeKAiYzz2Mc99a8RoxGQ@mail.gmail.com","threadId":"53425","inReplyTo":"CABPp-BEz42zvT_Vsu2xxg9RnuhBZ2aF8b+KYEu-CW=bMGQOC=Q@mail.gmail.com","subject":"Re: [PATCH 07/10] sparse-checkout: define in-tree dependencies","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-30T17:26:09Z","receivedAt":"2020-05-30T17:26:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Just thought of another angle here... (This feature you suggest\nsounds interesting, but I'm still mulling over in my head whether it's\npossible to make it work and how...)\n\nOn Wed, May 20, 2020 at 11:10 AM Elijah Newren wrote:\n>\n> On Thu, May 7, 2020 at 6:22 AM Derrick Stolee via GitGitGadget\n> wrote:\n\n> > --- a/Documentation/git-sparse-checkout.txt\n> > +++ b/Documentation/git-sparse-checkout.txt\n> > @@ -238,6 +238,32 @@ definition according to the files available at the new commit. If any of\n> > the specified files do not exist at the new commit, then the sparse-checkout\n> > definition will not change.\n> >\n> > +In a very large repository, there may be need to have multiple of these\n> > +in-tree sparse-checkout definitions to fit the roles of multiple types of\n> > +users. As each definition grows and the number of user types grow, it can\n> > +be difficult to manage updating all of the definitions when a common core\n> > +is modified somehow. For this reason, the in-tree pattern sets can inherit\n> > +the directories from other in-tree pattern sets. Use the `sparse.inherit`\n> > +option to specify other files in the tree.\n> > +\n> > +For example, suppose the file listed earlier is at `.sparse/core`. Another\n> > +file could be stored as `.sparse/extra` with contents\n> > +\n> > +----------------------------------\n> > +[sparse]\n> > + dir = X\n> > + dir = Y/Z\n> > + inherit = .sparse/core\n> > +----------------------------------\n>\n...\n> What happens if the user specifies a non-existent path, or perhaps\n> equivalently, didn't understand and specified the path using a\n> relative path specification? What if the user specifies a path that\n> doesn't exist in the commit, but happens to exist in someones's\n> working directory? (And maybe even did it intentionally as a way to\n> cheat and add user-defined additional paths to include? Do we want to\n> allow that, or do we want to enforce that extra includes use some kind\n> of pre-defined path?) What if they specify some path that is invalid\n> to record in a git commit (.git/mydeps or\n> ../../../../../../../otherdeps) but happens to exist on some machines?\n> (Are there future security vulnerabilities of any sort going down\n> this path?)\n\nAlthough the above considers non-existent paths, perhaps a more common\nway that a non-existent path specification arises is when people do\nadd paths that exist, but then a later merge or rebase or even 'git rm\n...' removes them. In such a case, the path used to be valid, it just\nisn't any more. Perhaps this suggests that the right fallback is to\nprint a warning and then just ignore that path specification?\n\n(Still have no clue how to handle merge conflicts, as discussed on\npatch 4, but maybe this at least addresses one case.)\n"},{"id":"400012","messageId":"CABPp-BEzz2MF72jZeJP6=9vJqcoJV15LN-O8umDDfWwQXLjZRA@mail.gmail.com","threadId":"53425","inReplyTo":"CABPp-BH5p1VPXfMOyN_0SLnsFKkRU9R-ZpiAe4k5r=ZUbHeibQ@mail.gmail.com","subject":"Re: [PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-06-17T23:07:01Z","receivedAt":"2020-06-17T23:07:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, May 20, 2020 at 10:52 AM Elijah Newren wrote:\n>\n> On Fri, May 8, 2020 at 8:42 AM Derrick Stolee wrote:\n> >\n> > On 5/7/2020 6:58 PM, Junio C Hamano wrote:\n> > > \"Derrick Stolee via GitGitGadget\" writes:\n> > >\n> > >> One of the difficulties of using the sparse-checkout feature is not\n> > >> knowing which directories are absolutely needed for working in a portion\n> > >> of the repository. Some of this can be documented in README files or\n> > >> included in a bootstrapping tool along with the repository. This is done\n> > >> in an ad-hoc way by every project that wants to use it.\n> > >>\n> > >> Let's make this process easier for users by creating a way to define a\n> > >> useful sparse-checkout definition inside the Git tree data. This has\n> > >> several benefits. In particular, the data is available to anyone who has\n> > >> a copy of the repository without needing a different data source.\n> > >> Second, the needs of the repository can change over time and Git can\n> > >> present a way to automatically update the working directory as these\n> > >> sparse-checkout definitions change over time.\n> > >\n> > > And two lines of development can merge them together?\n> > >\n> > > Any time a new \"feature\" pops up that would eventually affect how\n> > > \"git clone\" and \"git checkout\" work based on untrusted user data, we\n> > > need to make sure there is no negative security implications.\n> > >\n> > > If it only boils down to \"we have files that can record list of\n> > > leading directory names and without offering extra 'flexibility'\", I\n> > > guess there aren't all that much that a malicious sparse definition\n> > > can do and we would be safe, though.\n> >\n> > Yes. I hope that we can be extremely careful with this feature.\n> > The RFC status of this series implicitly includes the question\n> > \"Should we do this at all?\" I think the benefits outweigh the\n> > risks, but we can minimize those risks with very careful design\n> > and implementation.\n> >\n> > >> To use this feature, add the \"--in-tree\" option when setting or adding\n> > >> directories to the sparse-checkout definition. For example:\n> > >>\n> > >> $ git sparse-checkout set --in-tree .sparse/base\n> > >> $ git sparse-checkout add --in-tree .sparse/extra\n> > >>\n> > >> These commands add values to the multi-valued config setting\n> > >> \"sparse.inTree\". When updating the sparse-checkout definition, these\n> > >> values describe paths in the repository to find the sparse-checkout\n> > >> data. After the commands listed earlier, we expect to see the following\n> > >> in .git/config.worktree:\n> > >>\n> > >> [sparse]\n> > >> intree = .sparse/base\n> > >> intree = .sparse/extra\n> > >\n> > > What does this say in human words? \"These two tracked files specify\n> > > which paths should be in the working tree\"? Spelling it out here\n> > > would help readers of this commit.\n> >\n> > You got it. Sounds good.\n> >\n> > >> When applying the sparse-checkout definitions from this config, the\n> > >> blobs at HEAD:.sparse/base and HEAD:.sparse/extra are loaded.\n> > >\n> > > OK, so end-user edit to the working tree copy or what is added to\n> > > the index does not count and only the committed version gets used.\n> > >\n> > > That makes it simple---I was wondering how we would operate when\n> > > merging a branch with different contents in the .sparse/* files\n> > > until the conflicts are resolved.\n> >\n> > It's worth testing this case so we can be sure what happens.\n>\n> During a merge or rebase or checkout -m, what happens if .sparse/extra\n> has the following working tree content:\n>\n> [sparse]\n> dir = D\n> dir = X\n> <<<<<< HEAD\n> dir = Y\n> |||||| MERGE_BASE\n> ======\n> inherit = .sparse/tools\n> >>>>>> MERGE_HEAD\n> inherit = .sparse/base\n>\n> and, of course, three different entries in the index?\n>\n> Also, do we use the version of the --in-tree file from the latest\n> commit, from the index, or from the working tree? (This is a question\n> not only for merge and rebase, but also checkout with dirty changes\n> and even checkout -m.) Which one \"wins\"?\n>\n> And what if the user updates and commits an ill-formed version of the\n> file -- is it equivalent to getting an empty cone with just the\n> toplevel directory, equivalent to getting a complete checkout of\n> everything, or something else?\n\nSon pointed out that mercurial has a 'sparse' extension that has some\npossible ideas of things we could do here; see\nhttps://lore.kernel.org/git/CABPp-BGLBmWXrmPsTogyBFMgwYbHjN39oWbU=qDWroU1_fJaoQ@mail.gmail.com/\nfor some further discussion.\n"},{"id":"400013","messageId":"CABPp-BET86K_N5_W1pXMWeFccbUdxU8+vNvi+m-i6PX0MrXknw@mail.gmail.com","threadId":"53425","inReplyTo":"pull.627.git.1588857462.gitgitgadget@gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-06-17T23:14:18Z","receivedAt":"2020-06-17T23:14:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nAnother late addition...\n\nOn Thu, May 7, 2020 at 6:20 AM Derrick Stolee via GitGitGadget\n wrote:\n\n> IN-TREE SPARSE-CHECKOUT DEFINITIONS\n> ===================================\n>\n> Minh's idea was simple: have sparse-checkout files in the working directory\n> and use config to point to them. As these in-tree files update, we can\n> automatically update the sparse-checkout definition accordingly. Now, the\n> only thing to do would be to ensure that the sparse-checkout files are\n> updated when someone updates the build definitions. This requires some extra\n> build validation, but would not require special tools built on every client.\n\n\"In-tree\" still bugs me after a few weeks; the wording seems slightly\nawkward. I don't have a good suggestion, but I'm curious if there's a\nbetter term.\n\nBut I really came here to comment on another issue I think I glossed\nover the first time around. I'm curious if all module definition\nfiles have to exist in the working directory, as possibly suggested\nabove, or if we can allow them to just exist in the index. To give\nyou a flavor for what I mean, with my sparsify tool people can do\nthings like:\n ./sparsify --modules MODULE_A\nwhich provides MODULE_A and it's dependencies while removing all other\ndirectories. If MODULE_B, is not a dependency (direct or transitive)\nof MODULE_A, it will not exist in the working directory after this\nstep. Our equivalent of the \"in-tree\" definition of MODULE_B exists\n*in* the directory for MODULE_B, because it seems to make sense for\nus. I want people to be able to do\n ./sparsify --modules MODULE_B\nand have it correctly check out all the necessary files even though\nthe definition of MODULE_B wasn't even in the working directory at the\ntime the command ran. (The sparsify script knows to check the working\ndirectory first, then fall back to the index).\n"},{"id":"400017","messageId":"64d477b6-7bf2-fa0d-b9b4-821285af386e@gmail.com","threadId":"53425","inReplyTo":"CABPp-BET86K_N5_W1pXMWeFccbUdxU8+vNvi+m-i6PX0MrXknw@mail.gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-06-18T01:42:54Z","receivedAt":"2020-06-18T01:43:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/17/2020 7:14 PM, Elijah Newren wrote:\n> Hi,\n> \n> Another late addition...\n> \n> On Thu, May 7, 2020 at 6:20 AM Derrick Stolee via GitGitGadget\n> wrote:\n> \n>> IN-TREE SPARSE-CHECKOUT DEFINITIONS\n>> ===================================\n>>\n>> Minh's idea was simple: have sparse-checkout files in the working directory\n>> and use config to point to them. As these in-tree files update, we can\n>> automatically update the sparse-checkout definition accordingly. Now, the\n>> only thing to do would be to ensure that the sparse-checkout files are\n>> updated when someone updates the build definitions. This requires some extra\n>> build validation, but would not require special tools built on every client.\n> \n> \"In-tree\" still bugs me after a few weeks; the wording seems slightly\n> awkward. I don't have a good suggestion, but I'm curious if there's a\n> better term.\n\nI am open to suggestions. It reminds me of the two hardest problems\nin software engineering:\n\n\t1. concurrency\n\t2. naming things\n\t3. off-by-one errors\n\n> But I really came here to comment on another issue I think I glossed\n> over the first time around. I'm curious if all module definition\n> files have to exist in the working directory, as possibly suggested\n> above, or if we can allow them to just exist in the index. To give\n> you a flavor for what I mean, with my sparsify tool people can do\n> things like:\n> ./sparsify --modules MODULE_A\n> which provides MODULE_A and it's dependencies while removing all other\n> directories. If MODULE_B, is not a dependency (direct or transitive)\n> of MODULE_A, it will not exist in the working directory after this\n> step. Our equivalent of the \"in-tree\" definition of MODULE_B exists\n> *in* the directory for MODULE_B, because it seems to make sense for\n> us. I want people to be able to do\n> ./sparsify --modules MODULE_B\n> and have it correctly check out all the necessary files even though\n> the definition of MODULE_B wasn't even in the working directory at the\n> time the command ran. (The sparsify script knows to check the working\n> directory first, then fall back to the index).\n\nI think one tricky part of my RFC is that it _only_ looks at the\nindex. This allows us to read the contents even when the files are\nnot part of the current sparse-checkout definition.\n\nYou mentioned in another thread that it is a bit unwieldy for a user\nto rely on a committed (or staged?) file, so adding the ability to\ncheck the working directory first is interesting. I wonder how the\ntiming comes into play when changing HEAD to a new commit? Seems\ntricky, but solvable.\n\nThanks,\n-Stolee\n\n\n\n"},{"id":"400021","messageId":"CABPp-BFDcM+QSx_YSAAazkN9AoTPWVYV+ZpSQBPTihVM=gP-qw@mail.gmail.com","threadId":"53425","inReplyTo":"64d477b6-7bf2-fa0d-b9b4-821285af386e@gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-06-18T01:59:27Z","receivedAt":"2020-06-18T01:59:49Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jun 17, 2020 at 6:42 PM Derrick Stolee wrote:\n>\n> On 6/17/2020 7:14 PM, Elijah Newren wrote:\n> > Hi,\n> >\n> > Another late addition...\n> >\n> > On Thu, May 7, 2020 at 6:20 AM Derrick Stolee via GitGitGadget\n> > wrote:\n> >\n> >> IN-TREE SPARSE-CHECKOUT DEFINITIONS\n> >> ===================================\n> >>\n> >> Minh's idea was simple: have sparse-checkout files in the working directory\n> >> and use config to point to them. As these in-tree files update, we can\n> >> automatically update the sparse-checkout definition accordingly. Now, the\n> >> only thing to do would be to ensure that the sparse-checkout files are\n> >> updated when someone updates the build definitions. This requires some extra\n> >> build validation, but would not require special tools built on every client.\n> >\n> > \"In-tree\" still bugs me after a few weeks; the wording seems slightly\n> > awkward. I don't have a good suggestion, but I'm curious if there's a\n> > better term.\n>\n> I am open to suggestions. It reminds me of the two hardest problems\n> in software engineering:\n>\n> 1. concurrency\n> 2. naming things\n> 3. off-by-one errors\n\n:-)\n\n> > But I really came here to comment on another issue I think I glossed\n> > over the first time around. I'm curious if all module definition\n> > files have to exist in the working directory, as possibly suggested\n> > above, or if we can allow them to just exist in the index. To give\n> > you a flavor for what I mean, with my sparsify tool people can do\n> > things like:\n> > ./sparsify --modules MODULE_A\n> > which provides MODULE_A and it's dependencies while removing all other\n> > directories. If MODULE_B, is not a dependency (direct or transitive)\n> > of MODULE_A, it will not exist in the working directory after this\n> > step. Our equivalent of the \"in-tree\" definition of MODULE_B exists\n> > *in* the directory for MODULE_B, because it seems to make sense for\n> > us. I want people to be able to do\n> > ./sparsify --modules MODULE_B\n> > and have it correctly check out all the necessary files even though\n> > the definition of MODULE_B wasn't even in the working directory at the\n> > time the command ran. (The sparsify script knows to check the working\n> > directory first, then fall back to the index).\n>\n> I think one tricky part of my RFC is that it _only_ looks at the\n> index. This allows us to read the contents even when the files are\n> not part of the current sparse-checkout definition.\n>\n> You mentioned in another thread that it is a bit unwieldy for a user\n> to rely on a committed (or staged?) file, so adding the ability to\n> check the working directory first is interesting. I wonder how the\n> timing comes into play when changing HEAD to a new commit? Seems\n> tricky, but solvable.\n\nIsn't that essentially the same timing issue that comes into play if\nyou only look at the index, and are changing HEAD to a new commit?\n"},{"id":"400024","messageId":"2ee11a3c-9785-fa29-afa4-9eeac2248972@gmail.com","threadId":"53425","inReplyTo":"CABPp-BFDcM+QSx_YSAAazkN9AoTPWVYV+ZpSQBPTihVM=gP-qw@mail.gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-06-18T03:01:52Z","receivedAt":"2020-06-18T03:01:56Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/17/2020 9:59 PM, Elijah Newren wrote:\n> On Wed, Jun 17, 2020 at 6:42 PM Derrick Stolee wrote:\n>>\n>> On 6/17/2020 7:14 PM, Elijah Newren wrote:\n>> You mentioned in another thread that it is a bit unwieldy for a user\n>> to rely on a committed (or staged?) file, so adding the ability to\n>> check the working directory first is interesting. I wonder how the\n>> timing comes into play when changing HEAD to a new commit? Seems\n>> tricky, but solvable.\n> \n> Isn't that essentially the same timing issue that comes into play if\n> you only look at the index, and are changing HEAD to a new commit?\n\nIt adds to the complexity. We can inspect the new index for the\nin-tree definition and update the skip-worktree bits before actually\nchanging the working directory to match the new index. However, if\nwe trust the working directory copy over the index copy, then we\nneed to also decide if the working directory copy _would_ change\nin this move before using it.\n\nOf course, maybe I'm making it over-complicated. I still know so\nlittle about the index. I got into this feature due to a simple\npattern-matching problem that I could understand, but now I'm\ngetting lost in index states and dirty statuses. ;)\n\nThanks,\n-Stolee\n"},{"id":"400027","messageId":"CABPp-BHqPqEyf_US9GXB1bqC_ks0Xt5xLcDAwdKE=GhC6CAHZw@mail.gmail.com","threadId":"53425","inReplyTo":"2ee11a3c-9785-fa29-afa4-9eeac2248972@gmail.com","subject":"Re: [PATCH 00/10] [RFC] In-tree sparse-checkout definitions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-06-18T05:03:09Z","receivedAt":"2020-06-18T05:03:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jun 17, 2020 at 8:01 PM Derrick Stolee wrote:\n>\n> On 6/17/2020 9:59 PM, Elijah Newren wrote:\n> > On Wed, Jun 17, 2020 at 6:42 PM Derrick Stolee wrote:\n> >>\n> >> On 6/17/2020 7:14 PM, Elijah Newren wrote:\n> >> You mentioned in another thread that it is a bit unwieldy for a user\n> >> to rely on a committed (or staged?) file, so adding the ability to\n> >> check the working directory first is interesting. I wonder how the\n> >> timing comes into play when changing HEAD to a new commit? Seems\n> >> tricky, but solvable.\n> >\n> > Isn't that essentially the same timing issue that comes into play if\n> > you only look at the index, and are changing HEAD to a new commit?\n>\n> It adds to the complexity. We can inspect the new index for the\n> in-tree definition and update the skip-worktree bits before actually\n> changing the working directory to match the new index. However, if\n> we trust the working directory copy over the index copy, then we\n> need to also decide if the working directory copy _would_ change\n> in this move before using it.\n>\n> Of course, maybe I'm making it over-complicated. I still know so\n> little about the index. I got into this feature due to a simple\n> pattern-matching problem that I could understand, but now I'm\n> getting lost in index states and dirty statuses. ;)\n\nHonestly, I think your first cut for switching branches with this new\nfeature should just be:\n 1) Do a switch/checkout exactly the way it's done today already:\n 1a) Load the index and existing sparsity rules (from worktree then\nindex), according to what existed before the branch switch\n 1b) Do the appropriate 1-way or 2-way merge from unpack_trees() to\nmove to the right branch (as builtin/checkout.c already does)\n 2) *If* using in-tree sparsity feature, re-load the sparsity rules\n(from worktree then index) and run the equivalent of `sparse-checkout\nreapply`\n\nNot only does this avoid messing up any code for the normal case, I'm\npretty sure this gets the behavior the user expects in all the special\ncases. There is one big downside, and a little downside. The big\ndownside is that it'll have two progress meters instead of just one\n(much like sparse-checkout init && sparse-checkout set do today). The\nlittle downside is that this isn't optimal from a performance\nstandpoint, for two reasons:\n(a) some files may be updated in the working tree in step 1b despite\nthe fact that they'll be removed in step 2.\n(b) step 2 may just be an expensive no-op, in fact I suspect it will\nbe most the time\n\nThe reason I think the performance isn't a big deal is because:\nC) the fact that (b) is a problem means (a) is not often an issue --\nthe sparsity patterns are usually the same.\nD) Even if the sparsity patterns differ, it's often the case that\nfiles within the tree won't change (people tend to only modify a few\nfiles per commit, after all, so even switching branches only updates a\nsubset of all files).\nE) Even if the sparsity patterns differ and files differ and have to\nbe updated, it's still proportional at most to the number of files\nselected by the sparsity patterns, so it won't feel excessive or slow\nto the user.\nF) `sparse-checkout reapply` is pretty cheap, especially if called as\na function instead of as a subprocess\n\n\nIn my opinion, other than the unfortunate dual progress meters, the\nonly place where this new feature gets hairy is that `git\nsparse-checkout reapply` (or its libified variant) now needs to know\nhow to \"read your new form of sparsity rules\". That sounds easy at\nfirst but in my opinion it is a bit of a mess. The user could have\nwritten garbage to the file and I don't know what to do with garbage,\nbut I think we have to do something sane. (Give up and checkout\neverything? Give up and checkout nothing but the toplevel directory?\nIgnore each line we don't understand and treat the rest as stuff that\nneeds to be checked out? Can that last one even be done with the .ini\nreader?) I don't think saying \"well, tough luck the user should know\nbetter\" works because we already know that merges or rebases or\ncheckout -m could have written garbage to those files (in the form of\nconflict markers), and users will say that *we* were the ones that\nfilled the file with garbage. So we have to handle it intelligently\nin some fashion.\n\nThoughts?\n"},{"id":"400036","messageId":"20200618081820.GA97642@C02YX140LVDN.corpad.adbkng.com","threadId":"53425","inReplyTo":"CABPp-BEzz2MF72jZeJP6=9vJqcoJV15LN-O8umDDfWwQXLjZRA@mail.gmail.com","subject":"Re: [PATCH 04/10] sparse-checkout: allow in-tree definitions","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2020-06-18T08:18:20Z","receivedAt":"2020-06-18T08:18:37Z","isPatch":true,"sender":{"key":"sluongng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26684313?v=4"},"body":"Hi,\n\nOn Wed, Jun 17, 2020 at 04:07:01PM -0700, Elijah Newren wrote:\n> \n> Son pointed out that mercurial has a 'sparse' extension that has some\n> possible ideas of things we could do here; see\n> https://lore.kernel.org/git/CABPp-BGLBmWXrmPsTogyBFMgwYbHjN39oWbU=qDWroU1_fJaoQ@mail.gmail.com/\n> for some further discussion.\n\nI just want to note that you can find the latest version of FB's 'sparse'\nextension here[1] and the tests for 'profile' feature could be found here[2].\n\nAnother relevant source of reading could be Google's Narrow extension for\nMercurial[3].\n\nCheers,\nSon Luong.\n\n[1]: https://github.com/facebookexperimental/eden/blob/master/eden/scm/edenscm/hgext/sparse.py\n[2]: https://github.com/facebookexperimental/eden/blob/master/eden/scm/tests/test-sparse-profiles.t\n[3]: https://bitbucket.org/Google/narrowhg/src/cb51d673e9c5820fc3da86a67f7e74b789820b4f/tests/test-merge.t#lines-63\n"}]}