{"thread":{"id":"25752","subject":"[PATCH 00/10] Sparse checkout fixes and improvements","startedAt":"2010-11-15T10:36:40Z","lastAt":"2010-11-17T15:02:50Z","messageCount":34,"participants":["Nguyễn Thái Ngọc Duy","Jonathan Nieder","Thiago Farina","Miles Bader","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"155912","messageId":"1289817410-32470-1-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":null,"subject":"[PATCH 00/10] Sparse checkout fixes and improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:40Z","receivedAt":"2010-11-15T10:36:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The first part fixes the long standing t1011.7 and removes the\nwork around in excluded_from_list(). As a result, the\nEXC_FLAG_MUSTBEDIR fix for sparse checkout [1] is no longer needed.\n\nAnother plus is that because the index is now traversed tree-alike,\nwhen a directory is match, all its children does not need to be checked,\nwhich could speed things up a bit.\n\n[1] http://http://article.gmane.org/gmane.comp.version-control.git/160892\n\nNguyễn Thái Ngọc Duy (10):\n  add: do not rely on dtype being NULL behavior\n  unpack-trees: move all skip-worktree check back to unpack_trees()\n  unpack-trees: add function to update ce_flags based on sparse\n    patterns\n  unpack-trees: fix sparse checkout's \"unable to match directories\"\n    fault\n\n\nThe second part is more experimental although I think it's good\nchange:\n\n  unpack-trees: optimize full checkout case\n\nThe intention is that $GIT_DIR/info/sparse-checkout can be always-on.\nBut it should not impact performance when not really used.\n\n\n  templates: add info/sparse-checkout\n\nI should have done this long before. I did not notice it until recently.\n\n\n  checkout: add -S to update sparse checkout\n  checkout: add --full to fully populate working directory\n  git-checkout.txt: mention of sparse checkout\n\nThese form a friendlier interface to update sparse checkout. Users need\nnot to dig deep in git-read-tree.txt just to use sparse checkout.\n\"git checkout -S\" resembles \"cleartool edcs\", which makes sense to me.\nBoth edit a file (location \"unknown\" in clearcase case) and update\nworktree after that.\n\n\n  clean: support cleaning sparse checkout with -S\n\nSparse checkout does not prohibit you from checking out other parts of the\nindex. But you are pretty much left alone when doing so. This helps a bit.\nSupport \"git clean -S -e\" is possible with clear_ce_flags() from the first\npart of this series but I need to think a bit more.\n\n\n Documentation/git-checkout.txt       |   49 +++++++++\n Documentation/git-clean.txt          |    6 +-\n Documentation/git-read-tree.txt      |   18 +---\n builtin/add.c                        |    3 +-\n builtin/checkout.c                   |   59 +++++++++++-\n builtin/clean.c                      |   70 +++++++++++++\n cache.h                              |    1 +\n dir.c                                |    6 -\n t/t1011-read-tree-sparse-checkout.sh |   39 +++++++-\n t/t7301-clean-sparse.sh              |   92 +++++++++++++++++\n templates/info--sparse-checkout      |    4 +\n unpack-trees.c                       |  188 +++++++++++++++++++++++++++++++---\n 12 files changed, 490 insertions(+), 45 deletions(-)\n create mode 100755 t/t7301-clean-sparse.sh\n create mode 100644 templates/info--sparse-checkout\n\n-- \n1.7.3.2.210.g045198\n"},{"id":"155913","messageId":"1289817410-32470-2-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:41Z","receivedAt":"2010-11-15T10:36:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Commit c84de70 (excluded_1(): support exclude files in index -\n2009-08-20) added support for excluded() where dtype can be NULL. It\nwas designed specifically for index matching because there was no\nother way to extract dtype information from index. It did not support\nwildcard matching (for example, \"a*/\" pattern would fail to match).\n\nThe code was probably misread when commit 108da0d (git add: Add the\n\"--ignore-missing\" option for the dry run - 2010-07-10) was made\nbecause DT_UNKNOWN happens to be zero (NULL) too.\n\nDo not pass DT_UNKNOWN/NULL to excluded(), instead pass a pointer to a\nvariable that contains DT_UNKNOWN. The real dtype will be extracted\nfrom worktree by excluded(), as expected.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/add.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 56a4e0a..1a4672d 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -446,7 +446,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\tif (!seen[i] && pathspec[i][0]\n \t\t\t    && !file_exists(pathspec[i])) {\n \t\t\t\tif (ignore_missing) {\n-\t\t\t\t\tif (excluded(&dir, pathspec[i], DT_UNKNOWN))\n+\t\t\t\t\tint dtype = DT_UNKNOWN;\n+\t\t\t\t\tif (excluded(&dir, pathspec[i], &dtype))\n \t\t\t\t\t\tdir_add_ignored(&dir, pathspec[i], strlen(pathspec[i]));\n \t\t\t\t} else\n \t\t\t\t\tdie(\"pathspec '%s' did not match any files\",\n-- \n1.7.3.2.210.g045198\n"},{"id":"155914","messageId":"1289817410-32470-3-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 02/10] unpack-trees: move all skip-worktree check back to unpack_trees()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:42Z","receivedAt":"2010-11-15T10:36:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Earlier, the will_have_skip_worktree() checks are done in various places:\n\n1. in verify_* functions to prevent absent/uptodate checks outside\n   worktree\n2. in merged_entry for new index entries\n3. in apply_sparse_checkout() for all entries\n\nIn case all entries are added new ($GIT_DIR/index is missing) all the\nabove checks will be done for all entries, which in the worst case can\nbecome cache_nr*el->nr*3 fnmatch() calls. Quite expensive.\n\nBy moving all will_have_skip_worktree() checks to unpack_trees(), we\nnow have two places to run the checks:\n\n - one before calling traverse_trees(), where all current index\n   entries are checked.\n - one after traverse_trees(), where newly-added entries are checked.\n\nIn worst case, each entry will be checked once, or cache_nr*el->nr\nfnmatch() calls at most. The two places are a loop over all entries,\nwhich also makes optimization easier.\n\nCE_ADDED is used to mark new entries. The flag is only used by\nadd_to_index(), which should not be called while unpack_trees() is\nrunning.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n cache.h        |    1 +\n unpack-trees.c |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 63 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 33decd9..d87708a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -182,6 +182,7 @@ struct cache_entry {\n #define CE_WT_REMOVE (0x400000) /* remove in work directory */\n \n #define CE_UNPACKED  (0x1000000)\n+#define CE_NEW_SKIP_WORKTREE (0x2000000)\n \n /*\n  * Extended on-disk flags\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 803445a..9acd9be 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -258,7 +258,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n {\n \tint was_skip_worktree = ce_skip_worktree(ce);\n \n-\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n+\tif (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n \t\tce->ce_flags |= CE_SKIP_WORKTREE;\n \telse\n \t\tce->ce_flags &= ~CE_SKIP_WORKTREE;\n@@ -834,6 +834,49 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \treturn mask;\n }\n \n+static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n+{\n+\tint i;\n+\n+\tfor (i = 0;i < o->src_index->cache_nr;i++) {\n+\t\tstruct cache_entry *ce = o->src_index->cache[i];\n+\t\tce->ce_flags &= ~CE_ADDED;\n+\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n+\t\t\tce->ce_flags |= CE_NEW_SKIP_WORKTREE;\n+\t\telse\n+\t\t\tce->ce_flags &= ~CE_NEW_SKIP_WORKTREE;\n+\t}\n+}\n+\n+static int verify_absent(struct cache_entry *, enum unpack_trees_error_types, struct unpack_trees_options *);\n+static int set_new_skip_worktree_2(struct unpack_trees_options *o)\n+{\n+\tint i;\n+\n+\t/*\n+\t * CE_ADDED marks new index entries. These have not been processed\n+\t * by set_new_skip_worktree_1() so we do it here.\n+\t */\n+\tfor (i = 0;i < o->result.cache_nr;i++) {\n+\t\tstruct cache_entry *ce = o->result.cache[i];\n+\n+\t\tif (!(ce->ce_flags & CE_ADDED))\n+\t\t\tcontinue;\n+\n+\t\tce->ce_flags &= ~CE_ADDED;\n+\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n+\t\t\tce->ce_flags |= CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE;\n+\t\telse\n+\t\t\tce->ce_flags &= ~(CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n+\n+\t\t/* Left-over checks from merged_entry when old == NULL */\n+\t\tif (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n+\t\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n /*\n  * N-way merge \"len\" trees.  Returns 0 on success, -1 on failure to manipulate the\n  * resulting index, -2 on failure to reflect the changes to the work tree.\n@@ -862,6 +905,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\to->el = &el;\n \t}\n \n+\tif (!o->skip_sparse_checkout)\n+\t\tset_new_skip_worktree_1(o);\n+\n \tmemset(&o->result, 0, sizeof(o->result));\n \to->result.initialized = 1;\n \to->result.timestamp.sec = o->src_index->timestamp.sec;\n@@ -922,6 +968,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \n \tif (!o->skip_sparse_checkout) {\n \t\tint empty_worktree = 1;\n+\n+\t\tif (set_new_skip_worktree_2(o))\n+\t\t\tgoto return_failed;\n+\n \t\tfor (i = 0;i < o->result.cache_nr;i++) {\n \t\t\tstruct cache_entry *ce = o->result.cache[i];\n \n@@ -1017,7 +1067,7 @@ static int verify_uptodate_1(struct cache_entry *ce,\n static int verify_uptodate(struct cache_entry *ce,\n \t\t\t   struct unpack_trees_options *o)\n {\n-\tif (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n+\tif (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n \t\treturn 0;\n \treturn verify_uptodate_1(ce, o, ERROR_NOT_UPTODATE_FILE);\n }\n@@ -1204,7 +1254,7 @@ static int verify_absent(struct cache_entry *ce,\n \t\t\t enum unpack_trees_error_types error_type,\n \t\t\t struct unpack_trees_options *o)\n {\n-\tif (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n+\tif (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n \t\treturn 0;\n \treturn verify_absent_1(ce, error_type, o);\n }\n@@ -1226,10 +1276,15 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \tint update = CE_UPDATE;\n \n \tif (!old) {\n+\t\t/*\n+\t\t * Set CE_NEW_SKIP_WORKTREE on \"merge\" to make\n+\t\t * verify_absent() no-op. The true check will be done\n+\t\t * later on in unpack_trees().\n+\t\t */\n+\t\tmerge->ce_flags |= CE_NEW_SKIP_WORKTREE;\n \t\tif (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n \t\t\treturn -1;\n-\t\tif (!o->skip_sparse_checkout && will_have_skip_worktree(merge, o))\n-\t\t\tupdate |= CE_SKIP_WORKTREE;\n+\t\tupdate |= CE_ADDED;\n \t\tinvalidate_ce_path(merge, o);\n \t} else if (!(old->ce_flags & CE_CONFLICTED)) {\n \t\t/*\n@@ -1245,8 +1300,8 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\t} else {\n \t\t\tif (verify_uptodate(old, o))\n \t\t\t\treturn -1;\n-\t\t\tif (ce_skip_worktree(old))\n-\t\t\t\tupdate |= CE_SKIP_WORKTREE;\n+\t\t\t/* Migrate old bits over */\n+\t\t\tupdate |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n \t\t\tinvalidate_ce_path(old, o);\n \t\t}\n \t} else {\n-- \n1.7.3.2.210.g045198\n"},{"id":"155915","messageId":"1289817410-32470-4-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 03/10] unpack-trees: add function to update ce_flags based on sparse patterns","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:43Z","receivedAt":"2010-11-15T10:36:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The function will reconstruct directory structure from index and feed\nexcluded_from_list() with proper dtype.\n\nAnother advantage over the old will_have_skip_worktree() is that when\na directory is matched or not matched, we can go ahead and set\nce_flags for all of its children without calling excluded_from_list()\nagain. The old solution requires one excluded_from_list() call per\nindex entry.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n unpack-trees.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 88 insertions(+), 0 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9acd9be..6c266ef 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -834,6 +834,94 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \treturn mask;\n }\n \n+/*\n+ * traverse the index, find every entry that matches according to\n+ * o->el. Do \"ce_flags &= ~clear_mask\" on those entries. Return the\n+ * number of traversed entries.\n+ *\n+ * If select_mask is non-zero, only entries whose ce_flags has on of\n+ * those bits enabled are traversed.\n+ */\n+static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n+\t\t\t    int prefix_len, int match_all,\n+\t\t\t    int select_mask, int clear_mask,\n+\t\t\t    struct unpack_trees_options *o)\n+{\n+\tconst char *name, *slash;\n+\tstruct cache_entry *ce = cache[0];\n+\tconst char *prefix = ce->name;\n+\tint processed, original_nr = nr;\n+\n+\tif (prefix_len && !match_all) {\n+\t\tint dtype = DT_DIR;\n+\t\tstatic char path[PATH_MAX];\n+\t\tint pathlen = prefix_len - 1;\n+\t\tconst char *basename;\n+\n+\t\tmemcpy(path, prefix, pathlen);\n+\t\tpath[pathlen] = '\\0';\n+\t\tbasename = strrchr(path, '/');\n+\t\tbasename = basename ? basename+1 : path;\n+\t\tswitch (excluded_from_list(path, pathlen, basename, &dtype, o->el)) {\n+\t\tcase 0:\n+\t\t\twhile (nr && !strncmp(ce->name, prefix, prefix_len)) {\n+\t\t\t\tcache++;\n+\t\t\t\tce = *cache;\n+\t\t\t\tnr--;\n+\t\t\t}\n+\t\t\treturn original_nr - nr;\n+\t\tcase 1:\n+\t\t\tmatch_all = 1;\n+\t\t\tbreak;\n+\t\tdefault:\t/* case -1, undecided */\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\twhile (nr) {\n+\t\tif (select_mask && !(ce->ce_flags & select_mask)) {\n+\t\t\tcache++;\n+\t\t\tce = *cache;\n+\t\t\tnr--;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (prefix_len && strncmp(ce->name, prefix, prefix_len))\n+\t\t\tbreak;\n+\n+\t\tname = ce->name + prefix_len;\n+\t\tslash = strchr(name, '/');\n+\n+\t\t/* no slash, this is a file */\n+\t\tif (!slash) {\n+\t\t\tint dtype = ce_to_dtype(ce);\n+\t\t\tif (match_all ||\n+\t\t\t    excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, o->el) > 0)\n+\t\t\t\tce->ce_flags &= ~clear_mask;\n+\t\t\tcache++;\n+\t\t\tce = *cache;\n+\t\t\tnr--;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* has slash, this is a directory */\n+\t\tprocessed = clear_ce_flags_1(cache, nr,\n+\t\t\t\t\t     slash + 1 - ce->name, match_all,\n+\t\t\t\t\t     select_mask, clear_mask, o);\n+\t\tcache += processed;\n+\t\tce = *cache;\n+\t\tnr -= processed;\n+\t}\n+\treturn original_nr - nr;\n+}\n+\n+static int clear_ce_flags(struct cache_entry **cache, int nr,\n+\t\t\t    int select_mask, int clear_mask,\n+\t\t\t    struct unpack_trees_options *o)\n+{\n+\treturn clear_ce_flags_1(cache, nr, 0, 0, select_mask, clear_mask, o);\n+}\n+\n static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n {\n \tint i;\n-- \n1.7.3.2.210.g045198\n"},{"id":"155916","messageId":"1289817410-32470-5-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 04/10] unpack-trees: fix sparse checkout's \"unable to match directories\" fault","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:44Z","receivedAt":"2010-11-15T10:36:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"excluded_from_list() is designed to be work with a directory\nstructure, where there are files and directories. Index however is a\nlist of files, no directories, not really fit in excluded_from_list()\ndesign.\n\nCommit c84de70 (excluded_1(): support exclude files in index -\n2009-08-20) helped make it work somewhat. Although the lack of proper\ndtype means pattern \"blah\" won't match entries \"blah/...\" in the index.\n\nclear_ce_flags() is now used to match the index against sparse\npatterns. It does generate directories for excluded_from_list() so the\n\"blah\" case should now work.\n\nAlso revert c84de70 because it is no longer needed.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-read-tree.txt      |    7 -------\n dir.c                                |    6 ------\n t/t1011-read-tree-sparse-checkout.sh |   14 +++++++++++---\n unpack-trees.c                       |   33 ++++++++++++++++++---------------\n 4 files changed, 29 insertions(+), 31 deletions(-)\n\ndiff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt\nindex e88e9c2..634423a 100644\n--- a/Documentation/git-read-tree.txt\n+++ b/Documentation/git-read-tree.txt\n@@ -416,13 +416,6 @@ turn `core.sparseCheckout` on in order to have sparse checkout\n support.\n \n \n-BUGS\n-----\n-In order to match a directory with $GIT_DIR/info/sparse-checkout,\n-trailing slash must be used. The form without trailing slash, while\n-works with .gitignore, does not work with sparse checkout.\n-\n-\n SEE ALSO\n --------\n linkgit:git-write-tree[1]; linkgit:git-ls-files[1];\ndiff --git a/dir.c b/dir.c\nindex d1e5e5e..2b38f94 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -359,12 +359,6 @@ int excluded_from_list(const char *pathname,\n \t\t\tint to_exclude = x->to_exclude;\n \n \t\t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n-\t\t\t\tif (!dtype) {\n-\t\t\t\t\tif (!prefixcmp(pathname, exclude))\n-\t\t\t\t\t\treturn to_exclude;\n-\t\t\t\t\telse\n-\t\t\t\t\t\tcontinue;\n-\t\t\t\t}\n \t\t\t\tif (*dtype == DT_UNKNOWN)\n \t\t\t\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n \t\t\t\tif (*dtype != DT_DIR)\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 9a07de1..50f7dfe 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -91,12 +91,20 @@ test_expect_success 'match directories with trailing slash' '\n \ttest -f sub/added\n '\n \n-test_expect_failure 'match directories without trailing slash' '\n-\techo init.t >.git/info/sparse-checkout &&\n+test_expect_success 'match directories without trailing slash' '\n \techo sub >>.git/info/sparse-checkout &&\n \tgit read-tree -m -u HEAD &&\n \tgit ls-files -t >result &&\n-\ttest_cmp expected.swt result &&\n+\ttest_cmp expected.swt-noinit result &&\n+\ttest ! -f init.t &&\n+\ttest -f sub/added\n+'\n+\n+test_expect_success 'match directory pattern' '\n+\techo \"s?b\" >>.git/info/sparse-checkout &&\n+\tgit read-tree -m -u HEAD &&\n+\tgit ls-files -t >result &&\n+\ttest_cmp expected.swt-noinit result &&\n \ttest ! -f init.t &&\n \ttest -f sub/added\n '\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 6c266ef..f005454 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -245,15 +245,6 @@ static int check_updates(struct unpack_trees_options *o)\n static int verify_uptodate_sparse(struct cache_entry *ce, struct unpack_trees_options *o);\n static int verify_absent_sparse(struct cache_entry *ce, enum unpack_trees_error_types, struct unpack_trees_options *o);\n \n-static int will_have_skip_worktree(const struct cache_entry *ce, struct unpack_trees_options *o)\n-{\n-\tconst char *basename;\n-\n-\tbasename = strrchr(ce->name, '/');\n-\tbasename = basename ? basename+1 : ce->name;\n-\treturn excluded_from_list(ce->name, ce_namelen(ce), basename, NULL, o->el) <= 0;\n-}\n-\n static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n \tint was_skip_worktree = ce_skip_worktree(ce);\n@@ -926,14 +917,19 @@ static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n {\n \tint i;\n \n+\t/* Mark everything out (except staged entries) */\n \tfor (i = 0;i < o->src_index->cache_nr;i++) {\n \t\tstruct cache_entry *ce = o->src_index->cache[i];\n \t\tce->ce_flags &= ~CE_ADDED;\n-\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n+\t\tif (!ce_stage(ce))\n \t\t\tce->ce_flags |= CE_NEW_SKIP_WORKTREE;\n \t\telse\n \t\t\tce->ce_flags &= ~CE_NEW_SKIP_WORKTREE;\n \t}\n+\n+\t/* Then mark some of them in again according to o->el */\n+\tclear_ce_flags(o->src_index->cache, o->src_index->cache_nr,\n+\t\t       0, CE_NEW_SKIP_WORKTREE, o);\n }\n \n static int verify_absent(struct cache_entry *, enum unpack_trees_error_types, struct unpack_trees_options *);\n@@ -943,19 +939,26 @@ static int set_new_skip_worktree_2(struct unpack_trees_options *o)\n \n \t/*\n \t * CE_ADDED marks new index entries. These have not been processed\n-\t * by set_new_skip_worktree_1() so we do it here.\n+\t * by set_new_skip_worktree_1() so we do it here. Mark every new\n+\t * entries out.\n \t */\n \tfor (i = 0;i < o->result.cache_nr;i++) {\n \t\tstruct cache_entry *ce = o->result.cache[i];\n+\t\tif (ce->ce_flags & CE_ADDED)\n+\t\t\tce->ce_flags |= CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE;\n+\t}\n+\n+\t/* Then mark some of them in again */\n+\tclear_ce_flags(o->result.cache, o->result.cache_nr, CE_ADDED,\n+\t\t       CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE, o);\n+\n+\tfor (i = 0;i < o->result.cache_nr;i++) {\n+\t\tstruct cache_entry *ce = o->result.cache[i];\n \n \t\tif (!(ce->ce_flags & CE_ADDED))\n \t\t\tcontinue;\n \n \t\tce->ce_flags &= ~CE_ADDED;\n-\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n-\t\t\tce->ce_flags |= CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE;\n-\t\telse\n-\t\t\tce->ce_flags &= ~(CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n \n \t\t/* Left-over checks from merged_entry when old == NULL */\n \t\tif (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n-- \n1.7.3.2.210.g045198\n"},{"id":"155917","messageId":"1289817410-32470-6-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 05/10] unpack-trees: optimize full checkout case","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:45Z","receivedAt":"2010-11-15T10:36:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n unpack-trees.c |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex f005454..eb5d357 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -996,6 +996,16 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\to->el = &el;\n \t}\n \n+\tif (!o->skip_sparse_checkout &&\n+\t    o->el->nr == 1 &&\n+\t    !strcmp(o->el->excludes[0]->pattern, \"*\")) {\n+\t\tfor (i = 0; i < o->src_index->cache_nr; i++)\n+\t\t\tif (ce_skip_worktree(o->src_index->cache[i]))\n+\t\t\t\tbreak;\n+\t\tif (i == o->src_index->cache_nr)\n+\t\t\to->skip_sparse_checkout = 1;\n+\t}\n+\n \tif (!o->skip_sparse_checkout)\n \t\tset_new_skip_worktree_1(o);\n \n-- \n1.7.3.2.210.g045198\n"},{"id":"155918","messageId":"1289817410-32470-7-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 06/10] templates: add info/sparse-checkout","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:46Z","receivedAt":"2010-11-15T10:36:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n templates/info--sparse-checkout |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n create mode 100644 templates/info--sparse-checkout\n\ndiff --git a/templates/info--sparse-checkout b/templates/info--sparse-checkout\nnew file mode 100644\nindex 0000000..7426475\n--- /dev/null\n+++ b/templates/info--sparse-checkout\n@@ -0,0 +1,3 @@\n+# Specify what files are checked out in working directory\n+# Run \"git checkout\" after updating this file to update working directory\n+*\n-- \n1.7.3.2.210.g045198\n"},{"id":"155919","messageId":"1289817410-32470-8-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 07/10] checkout: add -S to update sparse checkout","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:47Z","receivedAt":"2010-11-15T10:36:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The traditional way of updating sparse checkout is to open\n$GIT_DIR/info/sparse-checkout manually, edit it, save, then run \"git\ncheckout\" or \"git read-tree -m -u HEAD\". That's not convenient.\n\n\"git checkout -S\" is introduced to combine those steps in one. It also\nprovides some additional checks:\n\n - warn users if core.sparseCheckout is off\n - refuse to update sparse checkout if there is no valid sparse\n   patterns (faulty sparse-checkout file for example)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-checkout.txt       |   16 +++++++++++++\n builtin/checkout.c                   |   42 +++++++++++++++++++++++++++++++++-\n t/t1011-read-tree-sparse-checkout.sh |   18 ++++++++++++++\n templates/info--sparse-checkout      |    1 +\n 4 files changed, 76 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 22d3611..478bfe7 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]\n 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...\n 'git checkout' --patch [<tree-ish>] [--] [<paths>...]\n+'git checkout' -S\n \n DESCRIPTION\n -----------\n@@ -176,6 +177,13 @@ the conflicted merge in the specified paths.\n This means that you can use `git checkout -p` to selectively discard\n edits from your current working tree.\n \n+-S::\n+--update-sparse-checkout::\n+\tAn editor is invoked to let you update your sparse checkout\n+\tpatterns. The updated patterns will be saved in\n+\t$GIT_DIR/info/sparse-checkout. The working directory is also\n+\tupdated. An empty file will abort the process.\n+\n <branch>::\n \tBranch to checkout; if it refers to a branch (i.e., a name that,\n \twhen prepended with \"refs/heads/\", is a valid ref), then that\n@@ -316,6 +324,14 @@ $ git add frotz\n ------------\n \n \n+ENVIRONMENT AND CONFIGURATION VARIABLES\n+---------------------------------------\n+The editor used to edit the commit log message will be chosen from the\n+GIT_EDITOR environment variable, the core.editor configuration variable, the\n+VISUAL environment variable, or the EDITOR environment variable (in that\n+order).  See linkgit:git-var[1] for details.\n+\n+\n Author\n ------\n Written by Linus Torvalds <torvalds@osdl.org>\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 9240faf..47847b3 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -675,6 +675,31 @@ static const char *unique_tracking_name(const char *name)\n \treturn NULL;\n }\n \n+static void edit_info_sparse_checkout()\n+{\n+\tchar *tmpfile = xstrdup(git_path(\"sparse-checkout\"));\n+\tstruct exclude_list el;\n+\tint i;\n+\n+\tcopy_file(tmpfile, git_path(\"info/sparse-checkout\"), 0666);\n+\n+\tif (launch_editor(tmpfile, NULL, NULL))\n+\t\texit(1);\n+\n+\tmemset(&el, 0, sizeof(el));\n+\tif (add_excludes_from_file_to_list(tmpfile, \"\", 0, NULL, &el, 0) < 0 ||\n+\t    el.nr == 0)\n+\t\tdie(\"No valid sparse patterns. Abort.\");\n+\tfor (i = 0; i < el.nr; i++)\n+\t\tfree(el.excludes[i]);\n+\tfree(el.excludes);\n+\n+\tif (rename(tmpfile, git_path(\"info/sparse-checkout\")) < 0)\n+\t\tdie_errno(\"Can't update %s\", git_path(\"info/sparse-checkout\"));\n+\n+\tfree(tmpfile);\n+}\n+\n int cmd_checkout(int argc, const char **argv, const char *prefix)\n {\n \tstruct checkout_opts opts;\n@@ -685,6 +710,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \tchar *conflict_style = NULL;\n \tint patch_mode = 0;\n \tint dwim_new_local_branch = 1;\n+\tint update_sparse_checkout = 0;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&opts.quiet),\n \t\tOPT_STRING('b', NULL, &opts.new_branch, \"branch\",\n@@ -704,6 +730,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"conflict\", &conflict_style, \"style\",\n \t\t\t   \"conflict style (merge or diff3)\"),\n \t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, \"select hunks interactively\"),\n+\t\tOPT_BOOLEAN('S', \"update-sparse-checkout\", &update_sparse_checkout,\n+\t\t\t    \"open up editor to edit $GIT_DIR/info/sparse-checkout\" ),\n \t\t{ OPTION_BOOLEAN, 0, \"guess\", &dwim_new_local_branch, NULL,\n \t\t  \"second guess 'git checkout no-such-branch'\",\n \t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n@@ -722,6 +750,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n+\tif (update_sparse_checkout && !core_apply_sparse_checkout)\n+\t\tdie(\"core.sparseCheckout needs to be turned on\");\n+\n \t/* we can assume from now on new_branch = !new_branch_force */\n \tif (opts.new_branch && opts.new_branch_force)\n \t\tdie(\"-B cannot be used with -b\");\n@@ -874,6 +905,9 @@ no_reference:\n \t\tif (!pathspec)\n \t\t\tdie(\"invalid path specification\");\n \n+\t\tif (update_sparse_checkout)\n+\t\t\tdie(\"git checkout: update paths is incompatible with updating sparse checkout.\");\n+\n \t\tif (patch_mode)\n \t\t\treturn interactive_checkout(new.name, pathspec, &opts);\n \n@@ -892,8 +926,11 @@ no_reference:\n \t\treturn checkout_paths(source_tree, pathspec, &opts);\n \t}\n \n-\tif (patch_mode)\n+\tif (patch_mode) {\n+\t\tif (update_sparse_checkout)\n+\t\t\tdie(\"git checkout: interactive checkout is incompatible with updating sparse checkout.\");\n \t\treturn interactive_checkout(new.name, NULL, &opts);\n+\t}\n \n \tif (opts.new_branch) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n@@ -915,5 +952,8 @@ no_reference:\n \tif (opts.writeout_stage)\n \t\tdie(\"--ours/--theirs is incompatible with switching branches.\");\n \n+\tif (update_sparse_checkout)\n+\t\tedit_info_sparse_checkout();\n+\n \treturn switch_branches(&opts, &new);\n }\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 50f7dfe..fe4716c 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -184,4 +184,22 @@ test_expect_success 'read-tree --reset removes outside worktree' '\n \ttest_cmp empty result\n '\n \n+test_expect_success 'git checkout -S fails to launch editor' '\n+\tGIT_EDITOR=/non-existent test_must_fail git checkout -S &&\n+\tgrep init.t .git/info/sparse-checkout\n+'\n+\n+test_expect_success 'git checkout -S' '\n+\tgit checkout -f top &&\n+\tcat >editor.sh <<\\EOF &&\n+#!/bin/sh\n+echo sub > \"$1\"\n+EOF\n+\tchmod 755 editor.sh &&\n+\tGIT_EDITOR=\"./editor.sh\" git checkout -S &&\n+\tgrep sub .git/info/sparse-checkout &&\n+\ttest -f sub/added &&\n+\ttest ! -f init.t\n+'\n+\n test_done\ndiff --git a/templates/info--sparse-checkout b/templates/info--sparse-checkout\nindex 7426475..588008b 100644\n--- a/templates/info--sparse-checkout\n+++ b/templates/info--sparse-checkout\n@@ -1,3 +1,4 @@\n # Specify what files are checked out in working directory\n # Run \"git checkout\" after updating this file to update working directory\n+# If this is opened by \"git checkout -S\", empty this file will abort it.\n *\n-- \n1.7.3.2.210.g045198\n"},{"id":"155920","messageId":"1289817410-32470-9-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 08/10] checkout: add --full to fully populate working directory","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:48Z","receivedAt":"2010-11-15T10:36:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-checkout.txt       |    6 +++++-\n builtin/checkout.c                   |   23 ++++++++++++++++++++---\n t/t1011-read-tree-sparse-checkout.sh |    7 +++++++\n 3 files changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 478bfe7..1d82063 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]\n 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...\n 'git checkout' --patch [<tree-ish>] [--] [<paths>...]\n-'git checkout' -S\n+'git checkout' [-S|--full]\n \n DESCRIPTION\n -----------\n@@ -184,6 +184,10 @@ edits from your current working tree.\n \t$GIT_DIR/info/sparse-checkout. The working directory is also\n \tupdated. An empty file will abort the process.\n \n+--full:\n+\tReset $GIT_DIR/info/sparse-checkout to include everything and\n+\tupdate working directory accordingly.\n+\n <branch>::\n \tBranch to checkout; if it refers to a branch (i.e., a name that,\n \twhen prepended with \"refs/heads/\", is a valid ref), then that\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 47847b3..f070930 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -710,7 +710,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \tchar *conflict_style = NULL;\n \tint patch_mode = 0;\n \tint dwim_new_local_branch = 1;\n-\tint update_sparse_checkout = 0;\n+\tint update_sparse_checkout = 0, full_checkout = 0;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&opts.quiet),\n \t\tOPT_STRING('b', NULL, &opts.new_branch, \"branch\",\n@@ -732,6 +732,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, \"select hunks interactively\"),\n \t\tOPT_BOOLEAN('S', \"update-sparse-checkout\", &update_sparse_checkout,\n \t\t\t    \"open up editor to edit $GIT_DIR/info/sparse-checkout\" ),\n+\t\tOPT_BOOLEAN(0, \"full\", &full_checkout,\n+\t\t\t    \"reset $GIT_DIR/info/sparse-checkout to checkout everything\" ),\n \t\t{ OPTION_BOOLEAN, 0, \"guess\", &dwim_new_local_branch, NULL,\n \t\t  \"second guess 'git checkout no-such-branch'\",\n \t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n@@ -750,6 +752,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n+\tif (full_checkout)\n+\t\tupdate_sparse_checkout = 1;\n+\n \tif (update_sparse_checkout && !core_apply_sparse_checkout)\n \t\tdie(\"core.sparseCheckout needs to be turned on\");\n \n@@ -952,8 +957,20 @@ no_reference:\n \tif (opts.writeout_stage)\n \t\tdie(\"--ours/--theirs is incompatible with switching branches.\");\n \n-\tif (update_sparse_checkout)\n-\t\tedit_info_sparse_checkout();\n+\tif (update_sparse_checkout) {\n+\t\tif (full_checkout) {\n+\t\t\tFILE *fp = fopen(git_path(\"info/sparse-checkout\"), \"w+\");\n+\t\t\tif (!fp)\n+\t\t\t\tdie_errno(\"Unable to reset %s\", git_path(\"info/sparse-checkout\"));\n+\t\t\tfprintf(fp, \"# Specify what files are checked out in working directory\\n\");\n+\t\t\tfprintf(fp, \"# Run \\\"git checkout\\\" after updating this file to update working directory\\n\");\n+\t\t\tfprintf(fp, \"# If this is opened by \\\"git checkout -S\\\", empty this file will abort it.\\n\");\n+\t\t\tfprintf(fp, \"*\\n\");\n+\t\t\tfclose(fp);\n+\t\t}\n+\t\telse\n+\t\t\tedit_info_sparse_checkout(full_checkout);\n+\t}\n \n \treturn switch_branches(&opts, &new);\n }\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex fe4716c..055e858 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -202,4 +202,11 @@ EOF\n \ttest ! -f init.t\n '\n \n+test_expect_success 'git checkout --full' '\n+\tgit checkout --full &&\n+\tgrep \"^\\*$\" .git/info/sparse-checkout &&\n+\ttest -f sub/added &&\n+\ttest -f init.t\n+'\n+\n test_done\n-- \n1.7.3.2.210.g045198\n"},{"id":"155921","messageId":"1289817410-32470-10-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 09/10] git-checkout.txt: mention of sparse checkout","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:49Z","receivedAt":"2010-11-15T10:36:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"All the boring technical stuff still resides in git-read-tree.txt. The\nintroduction here targets end users.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-checkout.txt  |   29 +++++++++++++++++++++++++++++\n Documentation/git-read-tree.txt |   11 +----------\n 2 files changed, 30 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 1d82063..b9a97c9 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -253,6 +253,35 @@ $ git log -g -2 HEAD\n ------------\n \n \n+Sparse checkout\n+---------------\n+\n+\"Sparse checkout\" allows to sparsely populate working directory\n+according to patterns defined in `$GIT_DIR/info/sparse-checkout`. The\n+syntax of this file is similar to linkgit:gitignore[5].  This feature\n+requires core.sparseCheckout to be turned on. `git checkout -S` should\n+be used when editing this file. It will update working directory\n+properly after editing. See linkgit:git-read-tree[1] for more\n+information.\n+\n+Normally sparse-checkout file contains only one rule, which indicates\n+that all files are checked out:\n+\n+----------------\n+*\n+----------------\n+\n+You can update the file to only contain files you want to be checked\n+out, or files _not_ to be checked out, using negate patterns. For\n+example, to remove file \"unwanted\":\n+\n+----------------\n+*\n+!unwanted\n+----------------\n+\n+To fully populate working directory again, use \"git checkout --full\".\n+\n EXAMPLES\n --------\n \ndiff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt\nindex 634423a..4b72c96 100644\n--- a/Documentation/git-read-tree.txt\n+++ b/Documentation/git-read-tree.txt\n@@ -390,16 +390,7 @@ Then it compares the new skip-worktree value with the previous one. If\n skip-worktree turns from unset to set, it will add the corresponding\n file back. If it turns from set to unset, that file will be removed.\n \n-While `$GIT_DIR/info/sparse-checkout` is usually used to specify what\n-files are in. You can also specify what files are _not_ in, using\n-negate patterns. For example, to remove file \"unwanted\":\n-\n-----------------\n-*\n-!unwanted\n-----------------\n-\n-Another tricky thing is fully repopulating working directory when you\n+A tricky thing is fully repopulating working directory when you\n no longer want sparse checkout. You cannot just disable \"sparse\n checkout\" because skip-worktree are still in the index and you working\n directory is still sparsely populated. You should re-populate working\n-- \n1.7.3.2.210.g045198\n"},{"id":"155922","messageId":"1289817410-32470-11-git-send-email-pclouds@gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 10/10] clean: support cleaning sparse checkout with -S","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-15T10:36:50Z","receivedAt":"2010-11-15T10:36:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Those files that are managed by git, but out of working directory due\nto sparse-checkout file are subjected to be cleaned by \"-S\". Files\nthat match the index exactly will be cleaned without \"-f\". Otherwise\n'-f' is needed.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-clean.txt |    6 ++-\n builtin/clean.c             |   70 ++++++++++++++++++++++++++++++++\n t/t7301-clean-sparse.sh     |   92 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 167 insertions(+), 1 deletions(-)\n create mode 100755 t/t7301-clean-sparse.sh\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex 60e38e6..e0c95b1 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...\n+'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X | -S] [--] <path>...\n \n DESCRIPTION\n -----------\n@@ -61,6 +61,10 @@ OPTIONS\n \tRemove only files ignored by git.  This may be useful to rebuild\n \teverything from scratch, but keep manually created files.\n \n+-S::\n+\tRemove files tracked by git but are outside of sparse checkout.\n+\tFiles that match the index exactly will be removed even when\n+\t'-f' is not given and clean.requireForce is no.\n \n Author\n ------\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex c8798f5..5827993 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -34,11 +34,71 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static int clean_sparse_checkout(const char *prefix, const char **pathspec,\n+\t\t\t\t int show_only, int quiet, int force)\n+{\n+\tstruct stat st;\n+\tint i, errors = 0;\n+\tunsigned char sha1[20];\n+\tconst char *qname;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\tfor (i = 0; i < the_index.cache_nr; i++) {\n+\t\tstruct cache_entry *ce = the_index.cache[i];\n+\n+\t\tif (!ce_skip_worktree(ce))\n+\t\t\tcontinue;\n+\t\tif (pathspec && !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))\n+\t\t\tcontinue;\n+\n+\t\tif (stat(ce->name, &st) < 0)\n+\t\t\tcontinue;\n+\t\tqname = quote_path_relative(ce->name, ce_namelen(ce), &buf, prefix);\n+\t\tif (index_path(sha1, ce->name, &st, 0) < 0) {\n+\t\t\twarning(\"failed to hash %s\", qname);\n+\t\t\terrors++;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!hashcmp(sha1, ce->sha1)) {\n+\t\t\tif (show_only) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", qname);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!quiet)\n+\t\t\t\tprintf(\"Removing %s\\n\", qname);\n+\t\t\tif (unlink(ce->name) != 0) {\n+\t\t\t\twarning(\"failed to remove %s\", qname);\n+\t\t\t\terrors++;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (force) {\n+\t\t\tif (show_only) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", qname);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!quiet)\n+\t\t\t\tprintf(\"Removing %s\\n\", qname);\n+\t\t\tif (unlink(ce->name) != 0) {\n+\t\t\t\twarning(\"failed to remove %s\", qname);\n+\t\t\t\terrors++;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (show_only)\n+\t\t\tprintf(\"Would not remove %s\\n\", qname);\n+\t}\n+\treturn errors != 0;\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tint show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, baselen = 0, config_set = 0, errors = 0;\n+\tint sparse = 0;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n \tstruct strbuf directory = STRBUF_INIT;\n \tstruct dir_struct dir;\n@@ -55,6 +115,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t\"remove whole directories\"),\n \t\t{ OPTION_CALLBACK, 'e', \"exclude\", &exclude_list, \"pattern\",\n \t\t  \"exclude <pattern>\", PARSE_OPT_NONEG, exclude_cb },\n+\t\tOPT_BOOLEAN('S', NULL, &sparse, \"remove tracked files outside sparse checkout\"),\n \t\tOPT_BOOLEAN('x', NULL, &ignored, \"remove ignored files, too\"),\n \t\tOPT_BOOLEAN('X', NULL, &ignored_only,\n \t\t\t\t\"remove only ignored files\"),\n@@ -70,6 +131,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, builtin_clean_usage,\n \t\t\t     0);\n \n+\tif (sparse) {\n+\t\tif (ignored || ignored_only)\n+\t\t\tdie(\"-S, -x and -X cannot be used together\");\n+\t\tif (exclude_list.nr)\n+\t\t\tdie(\"-S and -e cannot be used together (yet)\");\n+\t\tpathspec = get_pathspec(prefix, argv);\n+\t\treturn clean_sparse_checkout(prefix, pathspec, show_only, quiet, force);\n+\t}\n+\n \tmemset(&dir, 0, sizeof(dir));\n \tif (ignored_only)\n \t\tdir.flags |= DIR_SHOW_IGNORED;\ndiff --git a/t/t7301-clean-sparse.sh b/t/t7301-clean-sparse.sh\nnew file mode 100755\nindex 0000000..3ac0b0a\n--- /dev/null\n+++ b/t/t7301-clean-sparse.sh\n@@ -0,0 +1,92 @@\n+#!/bin/sh\n+\n+test_description='git clean -S basic tests'\n+\n+. ./test-lib.sh\n+\n+git config clean.requireForce yes\n+\n+test_expect_success 'setup' '\n+\tmkdir src &&\n+\ttouch file1 file2 &&\n+\ttouch src/file1 src/file2 &&\n+\tgit add . &&\n+\tgit update-index --skip-worktree file1 src/file1\n+'\n+\n+test_expect_success 'clean -x -S does not work' '\n+\ttest_must_fail git clean -x -S &&\n+\ttest_must_fail git clean -X -S\n+'\n+\n+test_expect_success 'clean -n -S' '\n+\tcat >expected <<\\EOF\n+Would remove file1\n+Would remove src/file1\n+EOF\n+\tgit clean -n -S >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'clean -n -S src' '\n+\tcat >expected <<\\EOF\n+Would remove src/file1\n+EOF\n+\tgit clean -n -S src >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success '[src] clean -n -S .' '\n+\t(\n+\tcd src\n+\tcat >expected <<\\EOF\n+Would remove file1\n+EOF\n+\tgit clean -n -S . >result &&\n+\ttest_cmp expected result\n+\t)\n+'\n+\n+test_expect_success '[src] clean -n -S ../file1' '\n+\t(\n+\tcd src\n+\tcat >expected <<\\EOF\n+Would remove ../file1\n+EOF\n+\tgit clean -n -S ../file1 >result &&\n+\ttest_cmp expected result\n+\t)\n+'\n+\n+test_expect_success 'clean -n -S with dirty worktree' '\n+\techo dirty >file1 &&\n+\tcat >expected <<\\EOF\n+Would not remove file1\n+Would remove src/file1\n+EOF\n+\tgit clean -n -S >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'clean -f -n -S with dirty worktree' '\n+\techo dirty >file1 &&\n+\tcat >expected <<\\EOF\n+Would remove file1\n+Would remove src/file1\n+EOF\n+\tgit clean -f -n -S >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'clean -S with dirty worktree' '\n+\tgit clean -S &&\n+\tgrep dirty file1 &&\n+\ttest ! -f src/file1\n+'\n+\n+test_expect_success 'clean -f -S with dirty worktree' '\n+\tgit clean -f -S &&\n+\ttest ! -f file1\n+'\n+\n+test_done\n-- \n1.7.3.2.210.g045198\n"},{"id":"155923","messageId":"20101115121415.GB14729@burratino","threadId":"25752","inReplyTo":"1289817410-32470-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T12:14:15Z","receivedAt":"2010-11-15T12:14:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> Commit c84de70 (excluded_1(): support exclude files in index -\n> 2009-08-20) added support for excluded() where dtype can be NULL. It\n> was designed specifically for index matching because there was no\n> other way to extract dtype information from index. It did not support\n> wildcard matching (for example, \"a*/\" pattern would fail to match).\n> \n> The code was probably misread when commit 108da0d (git add: Add the\n> \"--ignore-missing\" option for the dry run - 2010-07-10) was made\n> because DT_UNKNOWN happens to be zero (NULL) too.\n> \n> Do not pass DT_UNKNOWN/NULL to excluded(), instead pass a pointer to a\n> variable that contains DT_UNKNOWN. The real dtype will be extracted\n> from worktree by excluded(), as expected.\n\nCould you rephrase this in a way that contrasts current and desired\nbehavior?  Is it like this?\n\n\tThe \"git add --ignore-missing --dry-run\" codepath is\n\tinterpreting .gitignore incorrectly, unlike \"git add\".  For\n\texample:\n\n\t\t$ test -e foo || echo missing\n\t\tmissing\n\t\t$ echo foo/ >>.gitignore\n\t\t$ mkdir bar\n\t\t$ git add --ignore-missing --dry-run foo; echo $?\n\t\tThe following paths are ignored by one of your .gitignore files:\n\t\tfoo/\n\t\tUse -f if you really want to add them.\n\t\tfatal: no files added\n\t\t128\n\t\t$ git add --ignore-missing --dry-run bar/foo; echo $?\n\t\t0\n\n\tIn the original use case (preparing to add a submodule) the\n\tbehavior of the first command is correct, second incorrect.\n\tIf the entry to be added was a regular file, it would be the\n\tother way around.\n\n\tThe cause: the --ignore-missing code passes DT_UNKNOWN as the\n\tdtype_ptr argument to excluded() which happens to equal zero\n\t(NULL) and accidentally triggers the \"match pathspecs in index\n\tonly\" codepath (see c84de70, excluded_1(): support exclude\n\tfiles in index, 2009-08-20) that is unfortunately a bit\n\tprimitive.\n\n\tSurely what was really wanted is to check paths against the\n\tindex and work tree, defaulting to \"regular file\".\n\nWait --- that's not true.  In the \"git submodule add\" case, we really\nwant to default to (or even better, force) \"directory\".\n"},{"id":"155924","messageId":"AANLkTi=cvt2W1Eqj7KmtM+ng-BS7wRTM=+49gq5n+ghs@mail.gmail.com","threadId":"25752","inReplyTo":"1289817410-32470-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 02/10] unpack-trees: move all skip-worktree check back to unpack_trees()","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-11-15T12:34:31Z","receivedAt":"2010-11-15T12:34:31Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"2010/11/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n>  cache.h        |    1 +\n>  unpack-trees.c |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++-----\n>  2 files changed, 63 insertions(+), 7 deletions(-)\n>\n> diff --git a/cache.h b/cache.h\n> index 33decd9..d87708a 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -182,6 +182,7 @@ struct cache_entry {\n>  #define CE_WT_REMOVE (0x400000) /* remove in work directory */\n>\n>  #define CE_UNPACKED  (0x1000000)\n> +#define CE_NEW_SKIP_WORKTREE (0x2000000)\n>\nWould be good to remove the () around the hex here and else in this file?\n\n>  /*\n>  * Extended on-disk flags\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 803445a..9acd9be 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -258,7 +258,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n>  {\n>        int was_skip_worktree = ce_skip_worktree(ce);\n>\n> -       if (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n> +       if (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n>                ce->ce_flags |= CE_SKIP_WORKTREE;\n>        else\n>                ce->ce_flags &= ~CE_SKIP_WORKTREE;\n> @@ -834,6 +834,49 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n>        return mask;\n>  }\n>\n> +static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n> +{\n> +       int i;\n> +\n> +       for (i = 0;i < o->src_index->cache_nr;i++) {\nCould you add spaces after ; for readability, please? There is another\nfor below that needs this to.\n"},{"id":"155926","messageId":"20101115160135.GA16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 02/10] unpack-trees: move all skip-worktree check back to unpack_trees()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T16:01:35Z","receivedAt":"2010-11-15T16:01:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> Earlier, the will_have_skip_worktree() checks are done in various places:\n> \n> 1. in verify_* functions to prevent absent/uptodate checks outside\n>    worktree\n> 2. in merged_entry for new index entries\n> 3. in apply_sparse_checkout() for all entries\n> \n> In case all entries are added new ($GIT_DIR/index is missing) all the\n> above checks will be done for all entries, which in the worst case can\n> become cache_nr*el->nr*3 fnmatch() calls. Quite expensive.\n\nDoes this mean something like:\n\n\tHandling of sparse checkouts is slow.\n\n\t[timings]\n\n\tTo fix this, we will do such-and-such.  As a first step,\n\twe'll do such-and-such which does not change semantics\n\tand at least does not make it any slower.\n\n?\n\nIn other words, any commit message should make clear\n\n 1. The purpose.\n 2. The baseline of (sane or insane) behavior that is affected.\n 3. The intended resulting functionality.\n\nHow the patch works and why it succeeds are just optional extras\n(nice, certainly, but in 90% of cases it is obvious from the code once\nyou know (1), (2), and (3) anyway).\n\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -182,6 +182,7 @@ struct cache_entry {\n>  #define CE_WT_REMOVE (0x400000) /* remove in work directory */\n>  \n>  #define CE_UNPACKED  (0x1000000)\n> +#define CE_NEW_SKIP_WORKTREE (0x2000000)\n\nSemantics?\n\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -258,7 +258,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n>  {\n>  \tint was_skip_worktree = ce_skip_worktree(ce);\n>  \n> -\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n> +\tif (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n>  \t\tce->ce_flags |= CE_SKIP_WORKTREE;\n\nSo CE_NEW_SKIP_WORKTREE roughly means \"stage-0 entry outside the sparse checkout area\"?\n\n>  \telse\n>  \t\tce->ce_flags &= ~CE_SKIP_WORKTREE;\n\nIn particular, I guess the NEW part refers to the sparse checkout\narea, not the entry, since existing index entries with SKIP_WORKTREE\nbits need to keep that bit.\n\n> @@ -834,6 +834,49 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n>  \treturn mask;\n>  }\n>  \n> +static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n> +{\n\nWill comment on name once we get to the call site.\n\n> +\tint i;\n> +\n> +\tfor (i = 0;i < o->src_index->cache_nr;i++) {\n> +\t\tstruct cache_entry *ce = o->src_index->cache[i];\n> +\t\tce->ce_flags &= ~CE_ADDED;\n> +\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n> +\t\t\tce->ce_flags |= CE_NEW_SKIP_WORKTREE;\n> +\t\telse\n> +\t\t\tce->ce_flags &= ~CE_NEW_SKIP_WORKTREE;\n> +\t}\n> +}\n\nPopulating the CE_NEW_SKIP_WORKTREE flags based on the new\nworktree area.\n\n> +\n> +static int verify_absent(struct cache_entry *, enum unpack_trees_error_types, struct unpack_trees_options *);\n> +static int set_new_skip_worktree_2(struct unpack_trees_options *o)\n> +{\n> +\tint i;\n> +\n> +\t/*\n> +\t * CE_ADDED marks new index entries. These have not been processed\n> +\t * by set_new_skip_worktree_1() so we do it here.\n> +\t */\n\nProbably this comment belongs at the call site instead, to avoid some\nchasing.\n\n> +\tfor (i = 0;i < o->result.cache_nr;i++) {\n> +\t\tstruct cache_entry *ce = o->result.cache[i];\n> +\n> +\t\tif (!(ce->ce_flags & CE_ADDED))\n> +\t\t\tcontinue;\n> +\n> +\t\tce->ce_flags &= ~CE_ADDED;\n> +\t\tif (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n> +\t\t\tce->ce_flags |= CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE;\n> +\t\telse\n> +\t\t\tce->ce_flags &= ~(CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n\nCE_ADDED is private to the add_file_to_index() code path shared\nbetween add and rerere builtins.  rerere is libified and used e.g. by\ncherry-pick foo..bar.  Can it get us in trouble or do we have clear\nthe flags before using them here?  I think it would be worth a note in\napi-in-core-index.txt to warn future refactorers.\n\n> +\n> +\t\t/* Left-over checks from merged_entry when old == NULL */\n\nHuh?  (That is completely cryptic outside the context of the patch.)\n\n> +\t\tif (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n> +\t\t\treturn -1;\n> +\t}\n[...]\n> @@ -862,6 +905,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \t\t\to->el = &el;\n>  \t}\n>  \n> +\tif (!o->skip_sparse_checkout)\n> +\t\tset_new_skip_worktree_1(o);\n> +\n\nWhy is this not folded into the above if ()?\n\nThis populates the NEW_SKIP_WORKTREE (= future SKIP_WORKTREE?) bit\nin the index that represents the preimage for a reset or merge.\n\nPerhaps call it:\n\n\t\tset_new_skip_worktree(o->src_index, 0);\n\nand mark that function __attribute__((always_inline)) if the\noptimizer doesn't want to cooperate?\n\nOr:\n\n\t\tset_src_skip_worktree(o);\n\n>  \tmemset(&o->result, 0, sizeof(o->result));\n>  \to->result.initialized = 1;\n>  \to->result.timestamp.sec = o->src_index->timestamp.sec;\n> @@ -922,6 +968,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \n>  \tif (!o->skip_sparse_checkout) {\n>  \t\tint empty_worktree = 1;\n> +\n> +\t\tif (set_new_skip_worktree_2(o))\n> +\t\t\tgoto return_failed;\n> +\n\nTrivial part of the merge is over.  So now we can check the new\nindex entries against the sparse worktree patterns.  They were added in\nthat trivial part using add_entry() and will have the CE_ADDED flag.\n\nSo this might be:\n\n\t\tset_new_skip_worktree(&o->result, CE_ADDED);\n\nor\n\n\t\tset_result_skip_worktree(o);\n\nThe CE_ADDED flag was set in merged_entry, called by the merge_fn_t\nimplementations.  If there were an api-traverse-trees.txt explaining\nthe API, it would be worth a note there; for now it should suffice\nto add a note to future merge_fn_t implementors in the commit\nmessage (\"each unpack-trees merge function arranges for\nCE_SKIP_WORKTREE and CE_SKIP_NEW_WORKTREE to be propagated and for the\nCE_ADDED flag to be set on entries of new paths\").\n\n>  \t\tfor (i = 0;i < o->result.cache_nr;i++) {\n>  \t\t\tstruct cache_entry *ce = o->result.cache[i];\n>  \n> @@ -1017,7 +1067,7 @@ static int verify_uptodate_1(struct cache_entry *ce,\n>  static int verify_uptodate(struct cache_entry *ce,\n>  \t\t\t   struct unpack_trees_options *o)\n>  {\n> -\tif (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n> +\tif (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n[...]\n> @@ -1204,7 +1254,7 @@ static int verify_absent(struct cache_entry *ce,\n>  \t\t\t enum unpack_trees_error_types error_type,\n>  \t\t\t struct unpack_trees_options *o)\n>  {\n> -\tif (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n> +\tif (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n\nThe point, I hope.  Currently we alternate between finding entries to\nremove and deciding whether if lie in the worktree.  After this patch,\nwhether they lie in the worktree is precomputed.\n\n> @@ -1226,10 +1276,15 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n>  \tint update = CE_UPDATE;\n>  \n>  \tif (!old) {\n> +\t\t/*\n> +\t\t * Set CE_NEW_SKIP_WORKTREE on \"merge\" to make\n> +\t\t * verify_absent() no-op. The true check will be done\n> +\t\t * later on in unpack_trees().\n> +\t\t */\n> +\t\tmerge->ce_flags |= CE_NEW_SKIP_WORKTREE;\n\nMm?  Since verify_absent() is a no-op, why call it?  This looks like a\nplaceholder for code that calls verify_absent later, when we know if\nit lies in the worktree.\n\n>  \t\tif (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n>  \t\t\treturn -1;\n> -\t\tif (!o->skip_sparse_checkout && will_have_skip_worktree(merge, o))\n> -\t\t\tupdate |= CE_SKIP_WORKTREE;\n> +\t\tupdate |= CE_ADDED;\n\nMore of the main act.  Currently we alternate between finding entries\nto add and deciding if they are in the worktree.  After the patch,\nthey are piled up for addition first, then checked against the\nworktree in one batch.\n\n>  \t\tinvalidate_ce_path(merge, o);\n>  \t} else if (!(old->ce_flags & CE_CONFLICTED)) {\n>  \t\t/*\n> @@ -1245,8 +1300,8 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n>  \t\t} else {\n>  \t\t\tif (verify_uptodate(old, o))\n>  \t\t\t\treturn -1;\n> -\t\t\tif (ce_skip_worktree(old))\n> -\t\t\t\tupdate |= CE_SKIP_WORKTREE;\n> +\t\t\t/* Migrate old bits over */\n> +\t\t\tupdate |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n\nThanks, this looks promising.\nJonathan\n"},{"id":"155933","messageId":"20101115183040.GD16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 03/10] unpack-trees: add function to update ce_flags based on sparse patterns","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T18:30:40Z","receivedAt":"2010-11-15T18:30:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> The function will reconstruct directory structure from index and feed\n> excluded_from_list() with proper dtype.\n\nMight be worth squashing with a patch introducing a caller; otherwise it's\nhard to get a sense of what these functions do.\n"},{"id":"155939","messageId":"20101115191041.GE16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 04/10] unpack-trees: fix sparse checkout's \"unable to match directories\" fault","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T19:10:41Z","receivedAt":"2010-11-15T19:10:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> excluded_from_list() is designed to be work with a directory\n> structure, where there are files and directories. Index however is a\n                                                   ^\n                                                   The\n> list of files, no directories, not really fit in excluded_from_list()\n                 ^with no separate entries for directories, so it does\n> design.\n[...]\n> clear_ce_flags() is now used to match the index against sparse\n> patterns. It does generate directories for excluded_from_list()\n\nSounds exciting.  I can't believe I missed that, but I guess what\nwas needed is a simple example caller.  Let's see how this patch\nworks in that vein.\n\n> --- a/Documentation/git-read-tree.txt\n> +++ b/Documentation/git-read-tree.txt\n> @@ -416,13 +416,6 @@ turn `core.sparseCheckout` on in order to have sparse checkout\n>  support.\n>  \n>  \n> -BUGS\n> -----\n> -In order to match a directory with $GIT_DIR/info/sparse-checkout,\n> -trailing slash must be used. The form without trailing slash, while\n> -works with .gitignore, does not work with sparse checkout.\n\nNice.\n\n[...]\n> diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\n> index 9a07de1..50f7dfe 100755\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -91,12 +91,20 @@ test_expect_success 'match directories with trailing slash' '\n>  \ttest -f sub/added\n>  '\n>  \n> -test_expect_failure 'match directories without trailing slash' '\n> +test_expect_success 'match directories without trailing slash' '\n\nAlso nice.\n\n> -\techo init.t >.git/info/sparse-checkout &&\n>  \techo sub >>.git/info/sparse-checkout &&\n>  \tgit read-tree -m -u HEAD &&\n>  \tgit ls-files -t >result &&\n> -\ttest_cmp expected.swt result &&\n> +\ttest_cmp expected.swt-noinit result &&\n> +\ttest ! -f init.t &&\n> +\ttest -f sub/added\n\nMaybe the new\n\n\ttest_path_is_missing\n\nand\n\n\ttest_path_is_file\n\nwould be a good fit here, for better behavior when the path is a directory in\nthe former case and better debugging output for failures in the latter.\n\n> +'\n> +\n> +test_expect_success 'match directory pattern' '\n> +\techo \"s?b\" >>.git/info/sparse-checkout &&\n> +\tgit read-tree -m -u HEAD &&\n> +\tgit ls-files -t >result &&\n> +\ttest_cmp expected.swt-noinit result &&\n>  \ttest ! -f init.t &&\n>  \ttest -f sub/added\n>  '\n\nA pattern change that changes the list of matched paths would be more\nimpressive.  (Though I understand, this version already works for\ndemonstrating progress.)\n\n[...]\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -359,12 +359,6 @@ int excluded_from_list(const char *pathname,\n>  \t\t\tint to_exclude = x->to_exclude;\n>  \n>  \t\t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n> -\t\t\t\tif (!dtype) {\n> -\t\t\t\t\tif (!prefixcmp(pathname, exclude))\n> -\t\t\t\t\t\treturn to_exclude;\n> -\t\t\t\t\telse\n> -\t\t\t\t\t\tcontinue;\n> -\t\t\t\t}\n\nRemoving the no longer used !dtype codepath.\n\nThis hunk could be a separate patch (simplifying the diff and\ndecoupling this series from patch 1).\n\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n[...]\n> @@ -926,14 +917,19 @@ static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n>  {\n>  \tint i;\n>  \n> +\t/* Mark everything out (except staged entries) */\n>  \tfor (i = 0;i < o->src_index->cache_nr;i++) {\n>  \t\tstruct cache_entry *ce = o->src_index->cache[i];\n>  \t\tce->ce_flags &= ~CE_ADDED;\n> +\t\tif (!ce_stage(ce))\n\nNot a fan of this new definition of \"staged\".  The following might be\nclearer:\n\n\t/*\n\t * The WILL_BE_SKIP_WORKTREE bit is initialized in two stages:\n\t *\n\t * 1. Pretend the checkout is so narrow it contains no files.\n\t *    Only unresolved entries participating in a merge would\n\t *    get written to the worktree.\n\t *\n\t * 2. Add back entries according to .git/info/sparse-checkout\n\t */\n\n[...]\n> @@ -943,19 +939,26 @@ static int set_new_skip_worktree_2(struct unpack_trees_options *o)\n>  \n>  \t/*\n>  \t * CE_ADDED marks new index entries. These have not been processed\n> -\t * by set_new_skip_worktree_1() so we do it here.\n> +\t * by set_new_skip_worktree_1() so we do it here. Mark every new\n> +\t * entries out.\n>  \t */\n\nNit: \"every\" wants a singular noun.  But even better would be to move\nthe description of effect to the call site.\n\nThe description of strategy can stay here so it doesn't go out of date\nwhen the code changes:\n\n\t/*\n\t * Just like set_new_skip_worktree_1.\n\t * unpack-trees never writes conflicted entries to the worktree,\n\t * so there is no need to worry about conflicted files.\n\t *\n\t * 1. Set WILL_BE_SKIP_WORKTREE for all new entries.\n\t * 2. Clear it again for paths within the sparse checkout.\n\t */\n\nIs it possible to make the two code paths share code without\nsacrificing speed?\n"},{"id":"155944","messageId":"20101115201945.GF16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 03/10] unpack-trees: add function to update ce_flags based on sparse patterns","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T20:19:45Z","receivedAt":"2010-11-15T20:19:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> The function will reconstruct directory structure from index and feed\n> excluded_from_list() with proper dtype.\n\nOkay, so I guess what this means is:\n\n Matching index entries against an excludes file currently has two\n problems.\n\n First, there's no function to do it.  Code paths (like sparse checkout)\n that wanted to try it would iterate over index entries and for each\n index entry pass that path to excluded_from_list().  But that is not\n how excluded_from_list() works; one is supposed to feed in each\n ancester of a path before a given path to find out if it was excluded\n because of some parent or grandparent matching a\n\n\tbigsubdirectory/\n\n pattern despite the path not matching any .gitignore pattern directly.\n\n Second, it's inefficient.  The excludes mechanism is supposed to let\n us block off vast swaths of the filesystem as uninteresting; separately\n checking every index entry doesn't fit that model.\n\n Introduce a new function to take care of both these problems.  This\n traverses the index in depth-first order (well, that's what order the\n index is in) to mark un-excluded entries.\n\n Maybe some day the in-core index format will be restructured to make\n this sort of operation easier.  Or maybe we will want to try some\n binary search based thing.  The interface is simple enough to allow\n all those things.  Example:\n\n\tclear_ce_flags(the_index.cache, the_index.cache_nr,\n\t\t\tCE_CANDIDATE, CE_CLEARME, exclude_list);\n\n would clear the CE_CLEARME flag on all index entries with\n CE_CANDIDATE flag and not matched by exclude_list.\n\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -834,6 +834,94 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n>  \treturn mask;\n>  }\n>  \n> +/*\n> + * traverse the index, find every entry that matches according to\n> + * o->el. Do \"ce_flags &= ~clear_mask\" on those entries. Return the\n> + * number of traversed entries.\n> + *\n> + * If select_mask is non-zero, only entries whose ce_flags has on of\n> + * those bits enabled are traversed.\n> + */\n> +static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n> +\t\t\t    int prefix_len, int match_all,\n> +\t\t\t    int select_mask, int clear_mask,\n> +\t\t\t    struct unpack_trees_options *o)\n\no is only used for its excludes list.  Why not cut out the middle man\nand take o->el as argument?\n\nState (struct-worthy?): \n\tcache     \tpointer to an index entry\n\tprefix_len\tan offset into its path\n\tmatch_all \ta boolean telling whether we have some skipping to do\n\nThe current path (called the prefix), including trailing '/', is\n\n\tcache[0]->name[0] .. cache[0]->name[prefix_len - 1]\n\n> +{\n> +\tconst char *name, *slash;\n> +\tstruct cache_entry *ce = cache[0];\n> +\tconst char *prefix = ce->name;\n> +\tint processed, original_nr = nr;\n> +\n> +\tif (prefix_len && !match_all) {\n\nTop level gets special handling.  Let's skip to it (see [*] for where we\nwere).\n\n> +\twhile (nr) {\n\nFor each cache entry...\n\nMight be nice to phrase this as a for loop.\n\n\tconst char *prefix = cache[0]->name;\n\tstruct cache_entry ** const cache_end = cache + nr;\n\tstruct cache_entry **cep;\n\tfor (cep = cache; cep != cache_end; ) {\n\t\tstruct cache_entry *ce = *cep;\n\n> +\t\tif (select_mask && !(ce->ce_flags & select_mask)) {\n> +\t\t\tcache++;\n> +\t\t\tce = *cache;\n> +\t\t\tnr--;\n> +\t\t\tcontinue;\n> +\t\t}\n\nSome index entries are disqualified.\n\n\t\tif (select_mask)\n\t\t\tif (!(ce->ce_cflags & select_mask)) {\n\t\t\t\tcep++;\n\t\t\t\tcontinue;\n\t\t\t}\n\n> +\t\tif (prefix_len && strncmp(ce->name, prefix, prefix_len))\n> +\t\t\tbreak;\n\nEntries not under prefix are handled by the caller.\n\n\t\tif (prefix_len)\n\t\t\tif (strncmp(ce->name, prefix, prefix_len))\n\t\t\t\treturn cep - cache;\t/* number processed */\n\n> +\t\tname = ce->name + prefix_len;\n> +\t\tslash = strchr(name, '/');\n> +\n> +\t\t/* no slash, this is a file */\n> +\t\tif (!slash) {\n\nLeaf!\n\n\t\t\t/* File or directory?  Check if it's a submodule. */\n> +\t\t\tint dtype = ce_to_dtype(ce);\n> +\t\t\tif (match_all ||\n> +\t\t\t    excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, o->el) > 0)\n> +\t\t\t\tce->ce_flags &= ~clear_mask;\n> +\t\t\tcache++;\n> +\t\t\tce = *cache;\n> +\t\t\tnr--;\n> +\t\t\tcontinue;\n> +\t\t}\n\n\t\t\tif (excluded_block ||\n\t\t\t    excluded_from_list(...\t\t/* Excluded? */\n\t\t\t\tce->ce_flags &= ~clear_mask;\n\t\t\tcep++;\n\t\t\tcontinue;\n\t\t}\n\nPutting the loop body in its own function could allow some nice depth\nreduction.\n\n> +\n> +\t\t/* has slash, this is a directory */\n> +\t\tprocessed = clear_ce_flags_1(cache, nr,\n> +\t\t\t\t\t     slash + 1 - ce->name, match_all,\n> +\t\t\t\t\t     select_mask, clear_mask, o);\n> +\t\tcache += processed;\n> +\t\tce = *cache;\n> +\t\tnr -= processed;\n> +\t}\n> +\treturn original_nr - nr;\n\n\t\t/* Non-leaf.  Recurse. */\n\t\tcep += clear_ce_ ...\n\t}\n\nHow about the other case, mentioned before (see [*] above)?\n\n\tassert(prefix_len);\t/* Not toplevel but a subdirectory. */\n\tassert(!match_all);\t/* Not wholesale excluded. */\n\n> +\t\tint dtype = DT_DIR;\n> +\t\tstatic char path[PATH_MAX];\n> +\t\tint pathlen = prefix_len - 1;\n> +\t\tconst char *basename;\n> +\n> +\t\tmemcpy(path, prefix, pathlen);\n> +\t\tpath[pathlen] = '\\0';\n> +\t\tbasename = strrchr(path, '/');\n\nUse memrchr?\n\n> +\t\tbasename = basename ? basename+1 : path;\n> +\t\tswitch (excluded_from_list(path, pathlen, basename, &dtype, o->el)) {\n> +\t\tcase 0:\n> +\t\t\twhile (nr && !strncmp(ce->name, prefix, prefix_len)) {\n> +\t\t\t\tcache++;\n> +\t\t\t\tce = *cache;\n> +\t\t\t\tnr--;\n> +\t\t\t}\n> +\t\t\treturn original_nr - nr;\n\n\t\tcase 0:\t/* Included.  Great, no flag to clear. */\n\t\t\tfor (; cep != cache_end; cep++)\n\t\t\t\tif (strncmp((*cep)->name, prefix, prefix_len))\n\t\t\t\t\tbreak;\n\t\t\treturn cep - cache;\n\n> +\t\tcase 1:\n> +\t\t\tmatch_all = 1;\n> +\t\t\tbreak;\n\n\t\tcase 1:\t/* Excluded.  Great, clear them wholesale. */\n\t\t\tmatch_all = 1;\n\t\t\tcontinue;\n\n> +\t\tdefault:\t/* case -1, undecided */\n> +\t\t\tbreak;\n> +\t\t}\n\n\t\tcase -1:\t/* Undecided.\n\t\t\tcontinue;\n\t\t}\n> +\t}\n> +\treturn original_nr - nr;\n> +}\n\n\t}\n\t/* Ran off the end. */\n\treturn cep - cache;\n }\n\nThat last bit could even longjmp() if expecting deeply nested\ndirectory hierarchies.  (Sorry, joking, you should not do such an\nawful thing unless timing it proves it useful.)\n\nThat was clean, thanks.\n\nHope some of my musings are useful anyway.\n"},{"id":"155947","messageId":"20101115204158.GG16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 05/10] unpack-trees: optimize full checkout case","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T20:41:58Z","receivedAt":"2010-11-15T20:41:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n\nSo, what does it do?  What workflow was not optimized (yes, I know full\ncheckout means pattern \"*\", but the reader might not), why ought it to\nbe optimized, and what are the side-effects, if any?\n\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -996,6 +996,16 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \t\t\to->el = &el;\n>  \t}\n>  \n> +\tif (!o->skip_sparse_checkout &&\n> +\t    o->el->nr == 1 &&\n> +\t    !strcmp(o->el->excludes[0]->pattern, \"*\")) {\n> +\t\tfor (i = 0; i < o->src_index->cache_nr; i++)\n> +\t\t\tif (ce_skip_worktree(o->src_index->cache[i]))\n> +\t\t\t\tbreak;\n> +\t\tif (i == o->src_index->cache_nr)\n> +\t\t\to->skip_sparse_checkout = 1;\n\nMillinit:\n\n\tif (!o->skip_sparse_checkout &&\n\t\t\tcheckout_is_whole_tree(o) &&\n\t\t\tindex_uses_flag(o->src_index, CE_SKIP_WORKTREE))\n\t\to->skip_sparse_checkout = 1;\n\nFunctions for the two conditions (especially the second) would make\nthis clearer, I think.\n\nSensible.\n"},{"id":"155949","messageId":"20101115211636.GH16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-8-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 07/10] checkout: add -S to update sparse checkout","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T21:16:36Z","receivedAt":"2010-11-15T21:16:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> --- a/Documentation/git-checkout.txt\n> +++ b/Documentation/git-checkout.txt\n> @@ -12,6 +12,7 @@ SYNOPSIS\n>  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]\n>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...\n>  'git checkout' --patch [<tree-ish>] [--] [<paths>...]\n> +'git checkout' -S\n>  \n>  DESCRIPTION\n>  -----------\n> @@ -176,6 +177,13 @@ the conflicted merge in the specified paths.\n>  This means that you can use `git checkout -p` to selectively discard\n>  edits from your current working tree.\n>  \n> +-S::\n> +--update-sparse-checkout::\n> +\tAn editor is invoked to let you update your sparse checkout\n> +\tpatterns. The updated patterns will be saved in\n> +\t$GIT_DIR/info/sparse-checkout. The working directory is also\n> +\tupdated. An empty file will abort the process.\n\nWording nit: this doesn't make the worktree more up-to-date.  How\nabout:\n\n --edit-sparse-checkout\n --define-work-area\n --narrow-worktree\n\nHmph.\n\n--edit-sparse-checkout seems best for consistency of the choices I can\nthink of.\n\n[...]\n> @@ -316,6 +324,14 @@ $ git add frotz\n>  ------------\n>  \n>  \n> +ENVIRONMENT AND CONFIGURATION VARIABLES\n> +---------------------------------------\n> +The editor used to edit the commit log message will be chosen from the\n                                 ^\n  patterns defining what subset of the tracked tree to work on\n\n> +GIT_EDITOR environment variable, the core.editor configuration variable, the\n\nMight be simpler to say:\n\n CONFIGURATION\n -------------\n --edit-sparse-checkout uses the configured editor.  See git-var(1) for\n details.\n\n[...]\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -675,6 +675,31 @@ static const char *unique_tracking_name(const char *name)\n>  \treturn NULL;\n>  }\n>  \n> +static void edit_info_sparse_checkout()\n> +{\n> +\tchar *tmpfile = xstrdup(git_path(\"sparse-checkout\"));\n\ngit_pathdup()?\n\n> +\tstruct exclude_list el;\n> +\tint i;\n> +\n> +\tcopy_file(tmpfile, git_path(\"info/sparse-checkout\"), 0666);\n> +\n> +\tif (launch_editor(tmpfile, NULL, NULL))\n> +\t\texit(1);\n> +\n> +\tmemset(&el, 0, sizeof(el));\n> +\tif (add_excludes_from_file_to_list(tmpfile, \"\", 0, NULL, &el, 0) < 0 ||\n> +\t    el.nr == 0)\n> +\t\tdie(\"No valid sparse patterns. Abort.\");\n\nPerhaps the error message should mention the temp file.\n\n> +\tfor (i = 0; i < el.nr; i++)\n> +\t\tfree(el.excludes[i]);\n> +\tfree(el.excludes);\n\nNeat.\n\n> +\n> +\tif (rename(tmpfile, git_path(\"info/sparse-checkout\")) < 0)\n> +\t\tdie_errno(\"Can't update %s\", git_path(\"info/sparse-checkout\"));\n\nWouldn't git_path() clobber errno?  Probably worth keeping a temporary\nwith the result from git_path before the rename.\n\n> +\n> +\tfree(tmpfile);\n> +}\n> +\n>  int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct checkout_opts opts;\n> @@ -685,6 +710,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \tchar *conflict_style = NULL;\n>  \tint patch_mode = 0;\n>  \tint dwim_new_local_branch = 1;\n> +\tint update_sparse_checkout = 0;\n>  \tstruct option options[] = {\n>  \t\tOPT__QUIET(&opts.quiet),\n>  \t\tOPT_STRING('b', NULL, &opts.new_branch, \"branch\",\n> @@ -704,6 +730,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_STRING(0, \"conflict\", &conflict_style, \"style\",\n>  \t\t\t   \"conflict style (merge or diff3)\"),\n>  \t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, \"select hunks interactively\"),\n> +\t\tOPT_BOOLEAN('S', \"update-sparse-checkout\", &update_sparse_checkout,\n> +\t\t\t    \"open up editor to edit $GIT_DIR/info/sparse-checkout\" ),\n>  \t\t{ OPTION_BOOLEAN, 0, \"guess\", &dwim_new_local_branch, NULL,\n>  \t\t  \"second guess 'git checkout no-such-branch'\",\n>  \t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n> @@ -722,6 +750,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n>  \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n>  \n> +\tif (update_sparse_checkout && !core_apply_sparse_checkout)\n> +\t\tdie(\"core.sparseCheckout needs to be turned on\");\n\n--%s requires core.sparsecheckout to be turned out\n\n> +\n>  \t/* we can assume from now on new_branch = !new_branch_force */\n>  \tif (opts.new_branch && opts.new_branch_force)\n>  \t\tdie(\"-B cannot be used with -b\");\n> @@ -874,6 +905,9 @@ no_reference:\n>  \t\tif (!pathspec)\n>  \t\t\tdie(\"invalid path specification\");\n>  \n> +\t\tif (update_sparse_checkout)\n> +\t\t\tdie(\"git checkout: update paths is incompatible with updating sparse checkout.\");\n> +\n\n\"updating paths is incompatible\", I think.\n\nMaybe an area for future work. :)\n\n> @@ -892,8 +926,11 @@ no_reference:\n>  \t\treturn checkout_paths(source_tree, pathspec, &opts);\n>  \t}\n>  \n> -\tif (patch_mode)\n> +\tif (patch_mode) {\n> +\t\tif (update_sparse_checkout)\n> +\t\t\tdie(\"git checkout: interactive checkout is incompatible with updating sparse checkout.\");\n\nHmm, I'd put\n\n\tif (patch_mode && update_sparse_checkout)\n\t\tdie(...\n\nearlier as part of option validation.\n\n[...]\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -184,4 +184,22 @@ test_expect_success 'read-tree --reset removes outside worktree' '\n>  \ttest_cmp empty result\n>  '\n>  \n> +test_expect_success 'git checkout -S fails to launch editor' '\n> +\tGIT_EDITOR=/non-existent test_must_fail git checkout -S &&\n\nExporting envvars via a function is non-portable.  The usual workaround:\n\n\t(\n\t\tGIT_EDITOR=... &&\n\t\texport  GIT_EDITOR &&\n\t\ttest_must_fail ...\n\t) &&\n\nMaybe it's time to introduce\n\n\teval_must_fail GIT_EDITOR=/non-existent git checkout -S\n\n?\n\n[...]\n> +test_expect_success 'git checkout -S' '\n> +\tgit checkout -f top &&\n> +\tcat >editor.sh <<\\EOF &&\n> +#!/bin/sh\n> +echo sub > \"$1\"\n> +EOF\n\nStyle: the git test scripts usually omit the space after >.\n\n> --- a/templates/info--sparse-checkout\n> +++ b/templates/info--sparse-checkout\n> @@ -1,3 +1,4 @@\n>  # Specify what files are checked out in working directory\n>  # Run \"git checkout\" after updating this file to update working directory\n> +# If this is opened by \"git checkout -S\", empty this file will abort it.\n>  *\n\nUsage nits: s/if/when/; s/opened/edited/; s/will/to/; s/it//.\n\n # When editing with \"git checkout -S\", empty this file to abort.\n"},{"id":"155950","messageId":"20101115212311.GI16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-9-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 08/10] checkout: add --full to fully populate working directory","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T21:23:11Z","receivedAt":"2010-11-15T21:23:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n> --- a/Documentation/git-checkout.txt\n> +++ b/Documentation/git-checkout.txt\n> @@ -12,7 +12,7 @@ SYNOPSIS\n>  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]\n>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...\n>  'git checkout' --patch [<tree-ish>] [--] [<paths>...]\n> -'git checkout' -S\n> +'git checkout' [-S|--full]\n\nBrackets mean \"optional\".\n\n> @@ -184,6 +184,10 @@ edits from your current working tree.\n>  \t$GIT_DIR/info/sparse-checkout. The working directory is also\n>  \tupdated. An empty file will abort the process.\n>  \n> +--full:\n> +\tReset $GIT_DIR/info/sparse-checkout to include everything and\n> +\tupdate working directory accordingly.\n\nMaybe say something about \"widen\" and \"whole project\"?\n\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -952,8 +957,20 @@ no_reference:\n>  \tif (opts.writeout_stage)\n>  \t\tdie(\"--ours/--theirs is incompatible with switching branches.\");\n>  \n> -\tif (update_sparse_checkout)\n> -\t\tedit_info_sparse_checkout();\n> +\tif (update_sparse_checkout) {\n> +\t\tif (full_checkout) {\n> +\t\t\tFILE *fp = fopen(git_path(\"info/sparse-checkout\"), \"w+\");\n\nWhat should --full --edit-worktree-shape-or-whatever-that-was-called do?\n\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -202,4 +202,11 @@ EOF\n>  \ttest ! -f init.t\n>  '\n>  \n> +test_expect_success 'git checkout --full' '\n> +\tgit checkout --full &&\n> +\tgrep \"^\\*$\" .git/info/sparse-checkout &&\n> +\ttest -f sub/added &&\n> +\ttest -f init.t\n\nProbably worth checking that that's the only pattern in the file so\nshort-circuiting happens.\n"},{"id":"155951","messageId":"20101115213059.GJ16385@burratino","threadId":"25752","inReplyTo":"1289817410-32470-11-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 10/10] clean: support cleaning sparse checkout with -S","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-15T21:30:59Z","receivedAt":"2010-11-15T21:30:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyễn Thái Ngọc Duy wrote:\n\n>                                                              Files\n> that match the index exactly will be cleaned without \"-f\".\n\nHmm, that's new.  Seems fine, though; a person using -S would be\nforwarned.\n\n> --- a/Documentation/git-clean.txt\n> +++ b/Documentation/git-clean.txt\n> @@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n>  SYNOPSIS\n>  --------\n>  [verse]\n> -'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...\n> +'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X | -S] [--] <path>...\n\nSo -S and -x don't combine?\n\n>  \n>  DESCRIPTION\n>  -----------\n> @@ -61,6 +61,10 @@ OPTIONS\n>  \tRemove only files ignored by git.  This may be useful to rebuild\n>  \teverything from scratch, but keep manually created files.\n>  \n> +-S::\n> +\tRemove files tracked by git but are outside of sparse checkout.\n> +\tFiles that match the index exactly will be removed even when\n> +\t'-f' is not given and clean.requireForce is no.\n\nDoes \"no\" mean \"yes\" here?\n\nProbably worth mentioning that -S does not apply here.\n\nWould it be worth adding a -s/--clean-full-worktree option to\ncomplete the analogy to -X?\n\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n\nNot reading the code or tests because I'm trusting that I wouldn't\nneed to use this command. :)\n\nThanks, sparse checkout is looking sane now.\n"},{"id":"155952","messageId":"87bp5qmez1.fsf@catnip.gol.com","threadId":"25752","inReplyTo":"20101115211636.GH16385@burratino","subject":"Re: [PATCH 07/10] checkout: add -S to update sparse checkout","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-11-15T21:52:50Z","receivedAt":"2010-11-15T21:52:50Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n>> +-S::\n>> +--update-sparse-checkout::\n>> +\tAn editor is invoked to let you update your sparse checkout\n>> +\tpatterns. The updated patterns will be saved in\n>> +\t$GIT_DIR/info/sparse-checkout. The working directory is also\n>> +\tupdated. An empty file will abort the process.\n>\n> Wording nit: this doesn't make the worktree more up-to-date.  How\n> about:\n>\n>  --edit-sparse-checkout\n>  --define-work-area\n>  --narrow-worktree\n>\n> Hmph.\n>\n> --edit-sparse-checkout seems best for consistency of the choices I can\n> think of.\n\n\"--change-sparse-checkout\"?\n\nOnna-account of \"edit\" sounding like you're actually somehow editting\nthe checkout itself...  OTOH, since it invokes the editor... hmmm\n\nBTW, wouldn't it be more convenient to allow specifying patterns\ndirectly via the command line?  I'd think in many (maybe the majority\nof) cases people will really only want one entry, and having to edit a\nfile to specify it seems vaguely annoying...\n\n-Miles\n\n-- \n.Numeric stability is probably not all that important when you're guessing.\n"},{"id":"155964","messageId":"AANLkTikkArsu=NLJWQcP61uf3yrQmdxRQtB+3AmC9tHQ@mail.gmail.com","threadId":"25752","inReplyTo":"20101115121415.GB14729@burratino","subject":"Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:18:04Z","receivedAt":"2010-11-16T02:18:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Nov 15, 2010 at 7:14 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Nguyễn Thái Ngọc Duy wrote:\n>\n>> Commit c84de70 (excluded_1(): support exclude files in index -\n>> 2009-08-20) added support for excluded() where dtype can be NULL. It\n>> was designed specifically for index matching because there was no\n>> other way to extract dtype information from index. It did not support\n>> wildcard matching (for example, \"a*/\" pattern would fail to match).\n>>\n>> The code was probably misread when commit 108da0d (git add: Add the\n>> \"--ignore-missing\" option for the dry run - 2010-07-10) was made\n>> because DT_UNKNOWN happens to be zero (NULL) too.\n>>\n>> Do not pass DT_UNKNOWN/NULL to excluded(), instead pass a pointer to a\n>> variable that contains DT_UNKNOWN. The real dtype will be extracted\n>> from worktree by excluded(), as expected.\n>\n> Could you rephrase this in a way that contrasts current and desired\n> behavior?  Is it like this?\n>\n>        The \"git add --ignore-missing --dry-run\" codepath is\n>        interpreting .gitignore incorrectly, unlike \"git add\".  For\n>        example:\n>\n>                $ test -e foo || echo missing\n>                missing\n>                $ echo foo/ >>.gitignore\n>                $ mkdir bar\n>                $ git add --ignore-missing --dry-run foo; echo $?\n>                The following paths are ignored by one of your .gitignore files:\n>                foo/\n>                Use -f if you really want to add them.\n>                fatal: no files added\n>                128\n>                $ git add --ignore-missing --dry-run bar/foo; echo $?\n>                0\n>\n>        In the original use case (preparing to add a submodule) the\n>        behavior of the first command is correct, second incorrect.\n>        If the entry to be added was a regular file, it would be the\n>        other way around.\n>\n>        The cause: the --ignore-missing code passes DT_UNKNOWN as the\n>        dtype_ptr argument to excluded() which happens to equal zero\n>        (NULL) and accidentally triggers the \"match pathspecs in index\n>        only\" codepath (see c84de70, excluded_1(): support exclude\n>        files in index, 2009-08-20) that is unfortunately a bit\n>        primitive.\n>\n>        Surely what was really wanted is to check paths against the\n>        index and work tree, defaulting to \"regular file\".\n>\n> Wait --- that's not true.  In the \"git submodule add\" case, we really\n> want to default to (or even better, force) \"directory\".\n\nHmm.. get_index_dtype() would return DT_DIR if the submodule exists in\nindex. If it does not it must be a directory in worktree, right?\nCall flow: excluded_from_list() -> get_dtype() -> get_index_dtype()\n-- \nDuy\n"},{"id":"155965","messageId":"AANLkTimO2fZ13BEL51GVTOChkzAD+jLm5HWAuc-+Rutc@mail.gmail.com","threadId":"25752","inReplyTo":"AANLkTi=cvt2W1Eqj7KmtM+ng-BS7wRTM=+49gq5n+ghs@mail.gmail.com","subject":"Re: [PATCH 02/10] unpack-trees: move all skip-worktree check back to unpack_trees()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:19:59Z","receivedAt":"2010-11-16T02:19:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/15 Thiago Farina <tfransosi@gmail.com>:\n> 2010/11/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n>>  cache.h        |    1 +\n>>  unpack-trees.c |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++-----\n>>  2 files changed, 63 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/cache.h b/cache.h\n>> index 33decd9..d87708a 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -182,6 +182,7 @@ struct cache_entry {\n>>  #define CE_WT_REMOVE (0x400000) /* remove in work directory */\n>>\n>>  #define CE_UNPACKED  (0x1000000)\n>> +#define CE_NEW_SKIP_WORKTREE (0x2000000)\n>>\n> Would be good to remove the () around the hex here and else in this file?\n\nThat was the tradition of CE_*. I don't know why () is there. Will do\na preparation patch to remove all () first.\n\n>\n>>  /*\n>>  * Extended on-disk flags\n>> diff --git a/unpack-trees.c b/unpack-trees.c\n>> index 803445a..9acd9be 100644\n>> --- a/unpack-trees.c\n>> +++ b/unpack-trees.c\n>> @@ -258,7 +258,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n>>  {\n>>        int was_skip_worktree = ce_skip_worktree(ce);\n>>\n>> -       if (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n>> +       if (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n>>                ce->ce_flags |= CE_SKIP_WORKTREE;\n>>        else\n>>                ce->ce_flags &= ~CE_SKIP_WORKTREE;\n>> @@ -834,6 +834,49 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n>>        return mask;\n>>  }\n>>\n>> +static void set_new_skip_worktree_1(struct unpack_trees_options *o)\n>> +{\n>> +       int i;\n>> +\n>> +       for (i = 0;i < o->src_index->cache_nr;i++) {\n> Could you add spaces after ; for readability, please? There is another\n> for below that needs this to.\n>\n\nYes. I kept reminding myself of doing that, then forgot.\n-- \nDuy\n"},{"id":"155966","messageId":"AANLkTinpPyjd_Pr=XR9SkmQj1mPodQvG0OCeaxU8NjEp@mail.gmail.com","threadId":"25752","inReplyTo":"20101115160135.GA16385@burratino","subject":"Re: [PATCH 02/10] unpack-trees: move all skip-worktree check back to unpack_trees()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:39:47Z","receivedAt":"2010-11-16T02:39:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/15 Jonathan Nieder <jrnieder@gmail.com>:\n> Nguyễn Thái Ngọc Duy wrote:\n>\n>> Earlier, the will_have_skip_worktree() checks are done in various places:\n>>\n>> 1. in verify_* functions to prevent absent/uptodate checks outside\n>>    worktree\n>> 2. in merged_entry for new index entries\n>> 3. in apply_sparse_checkout() for all entries\n>>\n>> In case all entries are added new ($GIT_DIR/index is missing) all the\n>> above checks will be done for all entries, which in the worst case can\n>> become cache_nr*el->nr*3 fnmatch() calls. Quite expensive.\n>\n> Does this mean something like:\n>\n>        Handling of sparse checkouts is slow.\n>\n>        [timings]\n>\n>        To fix this, we will do such-and-such.  As a first step,\n>        we'll do such-and-such which does not change semantics\n>        and at least does not make it any slower.\n>\n> ?\n>\n> In other words, any commit message should make clear\n>\n>  1. The purpose.\n>  2. The baseline of (sane or insane) behavior that is affected.\n>  3. The intended resulting functionality.\n>\n> How the patch works and why it succeeds are just optional extras\n> (nice, certainly, but in 90% of cases it is obvious from the code once\n> you know (1), (2), and (3) anyway).\n\nAddressing the slowness is a nice side effect. My main concern is to\ncollect all skip-worktree checks in a central place so that I can do\ntree-like traversing.\n\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -182,6 +182,7 @@ struct cache_entry {\n>>  #define CE_WT_REMOVE (0x400000) /* remove in work directory */\n>>\n>>  #define CE_UNPACKED  (0x1000000)\n>> +#define CE_NEW_SKIP_WORKTREE (0x2000000)\n>\n> Semantics?\n\nYep, missed this in the commit message. While unpack_trees() is\nrunning, this bit will be set in entries that are outside worktree\narea, according to the (possibly updated) sparse-checkout file.\nCompare this bit with CE_SKIP_WORKTREE, we would know this entry\nshould be removed/added due to sparse-checkout updates.\n\n>> --- a/unpack-trees.c\n>> +++ b/unpack-trees.c\n>> @@ -258,7 +258,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n>>  {\n>>       int was_skip_worktree = ce_skip_worktree(ce);\n>>\n>> -     if (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n>> +     if (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n>>               ce->ce_flags |= CE_SKIP_WORKTREE;\n>\n> So CE_NEW_SKIP_WORKTREE roughly means \"stage-0 entry outside the sparse checkout area\"?\n\nYes. Conflicted entries must always stay in worktree regardless\nsparse-checkout patterns.\n\n>\n>>       else\n>>               ce->ce_flags &= ~CE_SKIP_WORKTREE;\n>\n> In particular, I guess the NEW part refers to the sparse checkout\n> area, not the entry, since existing index entries with SKIP_WORKTREE\n> bits need to keep that bit.\n\nThe NEW part still refers to entry. It's basically\nce_flags[CE_SKIP_WORKTREE] = ce_flags[CE_NEW_SKIP_WORKTREE]. The\nformer bit is kept in file, the later is in memory only.\n\n>> +\n>> +static int verify_absent(struct cache_entry *, enum unpack_trees_error_types, struct unpack_trees_options *);\n>> +static int set_new_skip_worktree_2(struct unpack_trees_options *o)\n>> +{\n>> +     int i;\n>> +\n>> +     /*\n>> +      * CE_ADDED marks new index entries. These have not been processed\n>> +      * by set_new_skip_worktree_1() so we do it here.\n>> +      */\n>\n> Probably this comment belongs at the call site instead, to avoid some\n> chasing.\n\nThere is a comment in the callsite, though it does not directly say\n\"set_new_skip_worktree_2\". Will fix.\n\n>> +     for (i = 0;i < o->result.cache_nr;i++) {\n>> +             struct cache_entry *ce = o->result.cache[i];\n>> +\n>> +             if (!(ce->ce_flags & CE_ADDED))\n>> +                     continue;\n>> +\n>> +             ce->ce_flags &= ~CE_ADDED;\n>> +             if (!ce_stage(ce) && will_have_skip_worktree(ce, o))\n>> +                     ce->ce_flags |= CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE;\n>> +             else\n>> +                     ce->ce_flags &= ~(CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n>\n> CE_ADDED is private to the add_file_to_index() code path shared\n> between add and rerere builtins.  rerere is libified and used e.g. by\n> cherry-pick foo..bar.  Can it get us in trouble or do we have clear\n> the flags before using them here?  I think it would be worth a note in\n> api-in-core-index.txt to warn future refactorers.\n\nIf it's too much trouble, I can use a new bit. Though the number of\navailable bits is decreasing.\n\n>> +\n>> +             /* Left-over checks from merged_entry when old == NULL */\n>\n> Huh?  (That is completely cryptic outside the context of the patch.)\n\nWill elaborate.\n\n>> @@ -862,6 +905,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>>                       o->el = &el;\n>>       }\n>>\n>> +     if (!o->skip_sparse_checkout)\n>> +             set_new_skip_worktree_1(o);\n>> +\n>\n> Why is this not folded into the above if ()?\n>\n> This populates the NEW_SKIP_WORKTREE (= future SKIP_WORKTREE?) bit\n> in the index that represents the preimage for a reset or merge.\n>\n> Perhaps call it:\n>\n>                set_new_skip_worktree(o->src_index, 0);\n>\n> and mark that function __attribute__((always_inline)) if the\n> optimizer doesn't want to cooperate?\n\nIt's setting future skip-worktree on existing entries index, contrast\nwith the _2() function, which is setting future skip-worktree on _new_\nentries in the index. How do you name those functions? Wait..\n\n>>       memset(&o->result, 0, sizeof(o->result));\n>>       o->result.initialized = 1;\n>>       o->result.timestamp.sec = o->src_index->timestamp.sec;\n>> @@ -922,6 +968,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>>\n>>       if (!o->skip_sparse_checkout) {\n>>               int empty_worktree = 1;\n>> +\n>> +             if (set_new_skip_worktree_2(o))\n>> +                     goto return_failed;\n>> +\n>\n> Trivial part of the merge is over.  So now we can check the new\n> index entries against the sparse worktree patterns.  They were added in\n> that trivial part using add_entry() and will have the CE_ADDED flag.\n>\n> So this might be:\n>\n>                set_new_skip_worktree(&o->result, CE_ADDED);\n>\n> or\n>\n>                set_result_skip_worktree(o);\n>\n\nOK I like the former over the latter.\n\n> The CE_ADDED flag was set in merged_entry, called by the merge_fn_t\n> implementations.  If there were an api-traverse-trees.txt explaining\n> the API, it would be worth a note there; for now it should suffice\n> to add a note to future merge_fn_t implementors in the commit\n> message (\"each unpack-trees merge function arranges for\n> CE_SKIP_WORKTREE and CE_SKIP_NEW_WORKTREE to be propagated and for the\n> CE_ADDED flag to be set on entries of new paths\").\n\nOK\n\n>> @@ -1017,7 +1067,7 @@ static int verify_uptodate_1(struct cache_entry *ce,\n>>  static int verify_uptodate(struct cache_entry *ce,\n>>                          struct unpack_trees_options *o)\n>>  {\n>> -     if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n>> +     if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n> [...]\n>> @@ -1204,7 +1254,7 @@ static int verify_absent(struct cache_entry *ce,\n>>                        enum unpack_trees_error_types error_type,\n>>                        struct unpack_trees_options *o)\n>>  {\n>> -     if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))\n>> +     if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n>\n> The point, I hope.  Currently we alternate between finding entries to\n> remove and deciding whether if lie in the worktree.  After this patch,\n> whether they lie in the worktree is precomputed.\n\nYep.\n\n>> @@ -1226,10 +1276,15 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n>>       int update = CE_UPDATE;\n>>\n>>       if (!old) {\n>> +             /*\n>> +              * Set CE_NEW_SKIP_WORKTREE on \"merge\" to make\n>> +              * verify_absent() no-op. The true check will be done\n>> +              * later on in unpack_trees().\n>> +              */\n>> +             merge->ce_flags |= CE_NEW_SKIP_WORKTREE;\n>\n> Mm?  Since verify_absent() is a no-op, why call it?  This looks like a\n> placeholder for code that calls verify_absent later, when we know if\n> it lies in the worktree.\n\nIt is no-op only when sparse checkout is enabled. In such cases we\ndon't know (yet) whether we will add those files in worktree. In\nnormal cases, everything must be added in worktree, so verify_absent()\nis real.\n\n>> @@ -1245,8 +1300,8 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n>>               } else {\n>>                       if (verify_uptodate(old, o))\n>>                               return -1;\n>> -                     if (ce_skip_worktree(old))\n>> -                             update |= CE_SKIP_WORKTREE;\n>> +                     /* Migrate old bits over */\n>> +                     update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n>\n> Thanks, this looks promising.\n\nThanks for looking at it. These stuff can be error-prone.\n-- \nDuy\n"},{"id":"155967","messageId":"20101116024252.GA29358@burratino","threadId":"25752","inReplyTo":"AANLkTikkArsu=NLJWQcP61uf3yrQmdxRQtB+3AmC9tHQ@mail.gmail.com","subject":"Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-16T02:42:52Z","receivedAt":"2010-11-16T02:42:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyen Thai Ngoc Duy wrote:\n> On Mon, Nov 15, 2010 at 7:14 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>>        Surely what was really wanted is to check paths against the\n>>        index and work tree, defaulting to \"regular file\".\n>>\n>> Wait --- that's not true.  In the \"git submodule add\" case, we really\n>> want to default to (or even better, force) \"directory\".\n>\n> Hmm.. get_index_dtype() would return DT_DIR if the submodule exists in\n> index. If it does not it must be a directory in worktree, right?\n> Call flow: excluded_from_list() -> get_dtype() -> get_index_dtype()\n\nbased on\n\n\tgit ls-files --error-unmatch \"$path\" >/dev/null 2>&1 &&\n\tdie \"'$path' already exists in the index\"\n\n\tif test -z \"$force\" && ! git add --dry-run --ignore-missing \"$path\" > /dev/null 2>&1\n\tthen\n\t\techo >&2 \"The following path is ignored by one of your .gitignore files:\" &&\n\t\techo >&2 $path &&\n\t\techo >&2 \"Use -f if you really want to add it.\"\n\t\texit 1\n\tfi\n\n\t# perhaps the path exists and is already a git repo, else clone it\n\tif test -e \"$path\"\n\nI'd say no, the usual case is that the potential submodule does not\nexist in the index or worktree, which is why that call site uses\n--ignore-missing.\n"},{"id":"155968","messageId":"AANLkTimozeFehA2SHcwsPYxs-Q2k_DFa2DE-i_m--Zq8@mail.gmail.com","threadId":"25752","inReplyTo":"20101115191041.GE16385@burratino","subject":"Re: [PATCH 04/10] unpack-trees: fix sparse checkout's \"unable to match directories\" fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:43:57Z","receivedAt":"2010-11-16T02:43:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/16 Jonathan Nieder <jrnieder@gmail.com>:\n> Is it possible to make the two code paths share code without\n> sacrificing speed?\n\nI think so. I'll merge the two functions in set_new_skip_worktree() as\nyou suggested in the previous mail.\n-- \nDuy\n"},{"id":"155969","messageId":"AANLkTin8SbeFkcOQXyGBEmwk7K3LKfV2N5GhfDx6hGnw@mail.gmail.com","threadId":"25752","inReplyTo":"20101115212311.GI16385@burratino","subject":"Re: [PATCH 08/10] checkout: add --full to fully populate working directory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:50:18Z","receivedAt":"2010-11-16T02:50:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/16 Jonathan Nieder <jrnieder@gmail.com>:\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -952,8 +957,20 @@ no_reference:\n>>       if (opts.writeout_stage)\n>>               die(\"--ours/--theirs is incompatible with switching branches.\");\n>>\n>> -     if (update_sparse_checkout)\n>> -             edit_info_sparse_checkout();\n>> +     if (update_sparse_checkout) {\n>> +             if (full_checkout) {\n>> +                     FILE *fp = fopen(git_path(\"info/sparse-checkout\"), \"w+\");\n>\n> What should --full --edit-worktree-shape-or-whatever-that-was-called do?\n\nShould die().\n-- \nDuy\n"},{"id":"155970","messageId":"AANLkTimrm2ArRn9Ym271iFEv2bstPPD0WPuP0=Majf0K@mail.gmail.com","threadId":"25752","inReplyTo":"20101115213059.GJ16385@burratino","subject":"Re: [PATCH 10/10] clean: support cleaning sparse checkout with -S","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T02:53:47Z","receivedAt":"2010-11-16T02:53:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/16 Jonathan Nieder <jrnieder@gmail.com>:\n> Nguyễn Thái Ngọc Duy wrote:\n>\n>>                                                              Files\n>> that match the index exactly will be cleaned without \"-f\".\n>\n> Hmm, that's new.  Seems fine, though; a person using -S would be\n> forwarned.\n\nIt's safe, which is why I let it do that without '-f'. One can always\ncheckout those entries again (provided that they know what entries to\ncheckout).\n\nHow do you want them to be warned?\n\n>> --- a/Documentation/git-clean.txt\n>> +++ b/Documentation/git-clean.txt\n>> @@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n>>  SYNOPSIS\n>>  --------\n>>  [verse]\n>> -'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...\n>> +'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X | -S] [--] <path>...\n>\n> So -S and -x don't combine?\n\nNo. Technical difficulties. Wait, nothing prevents me from doing that.\nYes combination is possible.\n\n>>\n>>  DESCRIPTION\n>>  -----------\n>> @@ -61,6 +61,10 @@ OPTIONS\n>>       Remove only files ignored by git.  This may be useful to rebuild\n>>       everything from scratch, but keep manually created files.\n>>\n>> +-S::\n>> +     Remove files tracked by git but are outside of sparse checkout.\n>> +     Files that match the index exactly will be removed even when\n>> +     '-f' is not given and clean.requireForce is no.\n>\n> Does \"no\" mean \"yes\" here?\n\nProbably, need to look at the that config variable again :(\n-- \nDuy\n"},{"id":"155971","messageId":"20101116030728.GB29358@burratino","threadId":"25752","inReplyTo":"AANLkTimrm2ArRn9Ym271iFEv2bstPPD0WPuP0=Majf0K@mail.gmail.com","subject":"Re: [PATCH 10/10] clean: support cleaning sparse checkout with -S","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-16T03:07:28Z","receivedAt":"2010-11-16T03:07:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nguyen Thai Ngoc Duy wrote:\n> 2010/11/16 Jonathan Nieder <jrnieder@gmail.com>:\n\n>> Hmm, that's new.  Seems fine, though; a person using -S would be\n>> forwarned.\n>\n> It's safe, which is why I let it do that without '-f'. One can always\n> checkout those entries again (provided that they know what entries to\n> checkout).\n>\n> How do you want them to be warned?\n\nManual and -h output, ideally.\n\n>\t\tOPT_BOOLEAN('S', NULL, &sparse, \"remove tracked files outside sparse checkout\"),\n\nMaybe something to the effect of\n\n\t\tOPT_SET_INT('S', NULL, &sparse,\n\t\t\t\"prune worktree to sparse pattern (even without -f)\", 1),\n\nProbably someone will chime in with the perfect wording that fits\nin the 80-column \"git clean -h\" output format. :)\n"},{"id":"155972","messageId":"AANLkTim3LMhQiO0RmPTW3dLNQa0FxkyX3m7Jjd_rPN7C@mail.gmail.com","threadId":"25752","inReplyTo":"20101115211636.GH16385@burratino","subject":"Re: [PATCH 07/10] checkout: add -S to update sparse checkout","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T03:08:27Z","receivedAt":"2010-11-16T03:08:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/16 Jonathan Nieder <jrnieder@gmail.com>:\n>>  DESCRIPTION\n>>  -----------\n>> @@ -176,6 +177,13 @@ the conflicted merge in the specified paths.\n>>  This means that you can use `git checkout -p` to selectively discard\n>>  edits from your current working tree.\n>>\n>> +-S::\n>> +--update-sparse-checkout::\n>> +     An editor is invoked to let you update your sparse checkout\n>> +     patterns. The updated patterns will be saved in\n>> +     $GIT_DIR/info/sparse-checkout. The working directory is also\n>> +     updated. An empty file will abort the process.\n>\n> Wording nit: this doesn't make the worktree more up-to-date.  How\n> about:\n>\n>  --edit-sparse-checkout\n>  --define-work-area\n>  --narrow-worktree\n>\n> Hmph.\n>\n> --edit-sparse-checkout seems best for consistency of the choices I can\n> think of.\n\nYep. --edit-sparse-checkout.\n\n>> @@ -316,6 +324,14 @@ $ git add frotz\n>>  ------------\n>>\n>>\n>> +ENVIRONMENT AND CONFIGURATION VARIABLES\n>> +---------------------------------------\n>> +The editor used to edit the commit log message will be chosen from the\n>                                 ^\n>  patterns defining what subset of the tracked tree to work on\n>\n>> +GIT_EDITOR environment variable, the core.editor configuration variable, the\n>\n> Might be simpler to say:\n\nThat was copied from git-commit.txt (I think). Maybe that file should\nbe updated too.\n\n> [...]\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -675,6 +675,31 @@ static const char *unique_tracking_name(const char *name)\n>>       return NULL;\n>>  }\n>>\n>> +static void edit_info_sparse_checkout()\n>> +{\n>> +     char *tmpfile = xstrdup(git_path(\"sparse-checkout\"));\n>\n> git_pathdup()?\n\nYup.\n\n>\n>> +     for (i = 0; i < el.nr; i++)\n>> +             free(el.excludes[i]);\n>> +     free(el.excludes);\n>\n> Neat.\n\nThis code is actually duplicated in unpack_trees(). It's time to\ncreate free_exclude() function.\n\n>> +\n>> +     if (rename(tmpfile, git_path(\"info/sparse-checkout\")) < 0)\n>> +             die_errno(\"Can't update %s\", git_path(\"info/sparse-checkout\"));\n>\n> Wouldn't git_path() clobber errno?  Probably worth keeping a temporary\n> with the result from git_path before the rename.\n\nOK\n\n>> --- a/t/t1011-read-tree-sparse-checkout.sh\n>> +++ b/t/t1011-read-tree-sparse-checkout.sh\n>> @@ -184,4 +184,22 @@ test_expect_success 'read-tree --reset removes outside worktree' '\n>>       test_cmp empty result\n>>  '\n>>\n>> +test_expect_success 'git checkout -S fails to launch editor' '\n>> +     GIT_EDITOR=/non-existent test_must_fail git checkout -S &&\n>\n> Exporting envvars via a function is non-portable.  The usual workaround:\n>\n>        (\n>                GIT_EDITOR=... &&\n>                export  GIT_EDITOR &&\n>                test_must_fail ...\n>        ) &&\n>\n> Maybe it's time to introduce\n>\n>        eval_must_fail GIT_EDITOR=/non-existent git checkout -S\n>\n> ?\n\nI'll go with the former.\n-- \nDuy\n"},{"id":"156010","messageId":"7v39r1hz92.fsf@alter.siamese.dyndns.org","threadId":"25752","inReplyTo":"AANLkTikkArsu=NLJWQcP61uf3yrQmdxRQtB+3AmC9tHQ@mail.gmail.com","subject":"Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-16T18:58:17Z","receivedAt":"2010-11-16T18:58:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> Hmm.. get_index_dtype() would return DT_DIR if the submodule exists in\n> index. If it does not it must be a directory in worktree, right?\n> Call flow: excluded_from_list() -> get_dtype() -> get_index_dtype()\n\nBy the way, your c84de70 (excluded_1(): support exclude files in index,\n2009-08-20) looks broken.  What happens when the exclude pattern wants a\ndirectory but specifies it with a wildcard?\n\nThe safest and sanest first step is to just change DT_UNKNOWN (which\nclearly is a wrong thing to pass) to NULL to keep the current behaviour,\nbut it probably makes more sense to do the second step at the same time to\npass a pointer to an int that holds DT_UNKNOWN to avoid triggering that\nbroken codepath.\n"},{"id":"156033","messageId":"AANLkTi=DUXS8i9oE+2spm3_g=F137AOfKi0rj3Tu=+AG@mail.gmail.com","threadId":"25752","inReplyTo":"7v39r1hz92.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-17T06:38:14Z","receivedAt":"2010-11-17T06:38:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 17, 2010 at 1:58 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n>\n>> Hmm.. get_index_dtype() would return DT_DIR if the submodule exists in\n>> index. If it does not it must be a directory in worktree, right?\n>> Call flow: excluded_from_list() -> get_dtype() -> get_index_dtype()\n>\n> By the way, your c84de70 (excluded_1(): support exclude files in index,\n> 2009-08-20) looks broken.  What happens when the exclude pattern wants a\n> directory but specifies it with a wildcard?\n\nIt fails. I realized that just this week unfortunately. That commit is\nreverted in this series.\n\n> The safest and sanest first step is to just change DT_UNKNOWN (which\n> clearly is a wrong thing to pass) to NULL to keep the current behaviour,\n> but it probably makes more sense to do the second step at the same time to\n> pass a pointer to an int that holds DT_UNKNOWN to avoid triggering that\n> broken codepath.\n\nYes.\n-- \nDuy\n"},{"id":"156051","messageId":"AANLkTim=beQhCvXmV6NvDiNHAYC+SBgvw36FzSthXHN2@mail.gmail.com","threadId":"25752","inReplyTo":"87bp5qmez1.fsf@catnip.gol.com","subject":"Re: [PATCH 07/10] checkout: add -S to update sparse checkout","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-17T15:02:50Z","receivedAt":"2010-11-17T15:02:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/11/16 Miles Bader <miles@gnu.org>:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>> +-S::\n>>> +--update-sparse-checkout::\n>>> +    An editor is invoked to let you update your sparse checkout\n>>> +    patterns. The updated patterns will be saved in\n>>> +    $GIT_DIR/info/sparse-checkout. The working directory is also\n>>> +    updated. An empty file will abort the process.\n>>\n>> Wording nit: this doesn't make the worktree more up-to-date.  How\n>> about:\n>>\n>>  --edit-sparse-checkout\n>>  --define-work-area\n>>  --narrow-worktree\n>>\n>> Hmph.\n>>\n>> --edit-sparse-checkout seems best for consistency of the choices I can\n>> think of.\n>\n> \"--change-sparse-checkout\"?\n>\n> Onna-account of \"edit\" sounding like you're actually somehow editting\n> the checkout itself...  OTOH, since it invokes the editor... hmmm\n>\n> BTW, wouldn't it be more convenient to allow specifying patterns\n> directly via the command line?  I'd think in many (maybe the majority\n> of) cases people will really only want one entry, and having to edit a\n> file to specify it seems vaguely annoying...\n\nA command line option to append patterns only sounds good to me. But\nit would clutter up sparse-checkout file over time and may decrease\nperformance. The same option can also be reused for git-clone. Do you\nsuggest any name? I'm bad at naming.\n\nI don't think I can make an option to remove patterns though.\n-- \nDuy\n"}]}