{"thread":{"id":"45632","subject":"[PATCH 0/3] fix working directory file issues while using sparse-checkout","startedAt":"2017-04-07T19:24:11Z","lastAt":"2017-04-17T12:10:07Z","messageCount":16,"participants":["Kevin Willford","Stefan Beller","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"316371","messageId":"20170407192357.948-1-kewillf@microsoft.com","threadId":"45632","inReplyTo":null,"subject":"[PATCH 0/3] fix working directory file issues while using sparse-checkout","fromName":"Kevin Willford","fromEmail":"kcwillford@gmail.com","sentAt":"2017-04-07T19:23:54Z","receivedAt":"2017-04-07T19:24:11Z","isPatch":true,"sender":{"key":"kcwillford@gmail.com","avatar":null},"body":"While using the sparse-checkout feature there are scenarios where \nthe working directory should and should not be updated. This patch\nseries addresses issues found in reset, apply, and merge conflicts.\n\nKevin Willford (3):\n merge-recursive.c: conflict using sparse should update file\n apply.c: do not checkout file when skip-worktree bit set\n reset.c: update files when using sparse to avoid data loss.\n\n apply.c | 18 +++++++++++++\n builtin/reset.c | 34 +++++++++++++++++++++++\n merge-recursive.c | 8 ++++++\n t/t7114-reset-sparse-checkout.sh | 58 ++++++++++++++++++++++++++++++++++++++++\n t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++\n 5 files changed, 145 insertions(+)\n create mode 100755 t/t7114-reset-sparse-checkout.sh\n create mode 100755 t/t7614-merge-sparse-checkout.sh\n\n-- \n2.12.2.windows.2.1.g7df5db8d31\n\n"},{"id":"316372","messageId":"20170407192357.948-2-kewillf@microsoft.com","threadId":"45632","inReplyTo":"20170407192357.948-1-kewillf@microsoft.com","subject":"[PATCH 1/3] merge-recursive.c: conflict using sparse should update file","fromName":"Kevin Willford","fromEmail":"kcwillford@gmail.com","sentAt":"2017-04-07T19:23:55Z","receivedAt":"2017-04-07T19:24:17Z","isPatch":true,"sender":{"key":"kcwillford@gmail.com","avatar":null},"body":"Update the file when there is a conflict with a modify/delete scenario\nwhen using the sparse-checkout feature since the file might not be on disk\nbecause the skip-worktree bit is on and the user will need the file and\ncontent to determine how to resolve the conflict.\n\nSigned-off-by: Kevin Willford \n---\n merge-recursive.c | 8 ++++++++\n t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 35 insertions(+)\n create mode 100755 t/t7614-merge-sparse-checkout.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex b7ff1ada3c..54fce93dae 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1103,6 +1103,14 @@ static int handle_change_delete(struct merge_options *o,\n \t\t\t \"and %s in %s. Version %s of %s left in tree.\"),\n \t\t\t change, path, o->branch2, change_past,\n \t\t\t o->branch1, o->branch1, path);\n+\t\t\t/*\n+\t\t\t * In a sparse checkout the file may not exist. Sadly,\n+\t\t\t * the CE_SKIP_WORKTREE flag is not preserved in the\n+\t\t\t * case of conflicts, therefore we do the next best\n+\t\t\t * thing: verify that the file exists.\n+\t\t\t */\n+\t\t\tif (!file_exists(path))\n+\t\t\t\tret = update_file(o, 0, a_oid, a_mode, path);\n \t\t} else {\n \t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n \t\t\t \"and %s in %s. Version %s of %s left in tree at %s.\"),\ndiff --git a/t/t7614-merge-sparse-checkout.sh b/t/t7614-merge-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..6922f0244f\n--- /dev/null\n+++ b/t/t7614-merge-sparse-checkout.sh\n@@ -0,0 +1,27 @@\n+#!/bin/sh\n+\n+test_description='merge can handle sparse-checkout'\n+\n+. ./test-lib.sh\n+\n+# merges with conflicts\n+\n+test_expect_success 'setup' '\n+\ttest_commit a &&\n+\ttest_commit file &&\n+\tgit checkout -b delete-file &&\n+\tgit rm file.t &&\n+\ttest_tick &&\n+\tgit commit -m \"remove file\" &&\n+\tgit checkout master &&\n+\ttest_commit modify file.t changed\n+'\n+\n+test_expect_success 'merge conflict deleted file and modified' '\n+\techo \"/a\" >.git/info/sparse-checkout &&\n+\ttest_config core.sparsecheckout true &&\n+\ttest_must_fail git merge delete-file &&\n+\ttest_path_is_file file.t\n+'\n+\n+test_done\n-- \n2.12.2.windows.2.1.g7df5db8d31\n\n"},{"id":"316373","messageId":"20170407192357.948-3-kewillf@microsoft.com","threadId":"45632","inReplyTo":"20170407192357.948-1-kewillf@microsoft.com","subject":"[PATCH 2/3] apply.c: do not checkout file when skip-worktree bit set","fromName":"Kevin Willford","fromEmail":"kcwillford@gmail.com","sentAt":"2017-04-07T19:23:56Z","receivedAt":"2017-04-07T19:24:18Z","isPatch":true,"sender":{"key":"kcwillford@gmail.com","avatar":null},"body":"When using the sparse-checkout feature git should not write to\nthe working directory for files with the skip-worktree bit on.\nWith the skip-worktree bit on the file may or may not be in\nthe working directory and if it is not we don't want or need to\ncreate it by calling checkout_entry.\n\nThere are two callers of checkout_target. Both of which check\nthat the file does not exist before calling checkout_target.\nload_current which make a call to lstat right before calling checkout_target\nand check_preimage which will only run checkout_taret it stat_ret\nis less than zero. It sets stat_ret to zero and only if\n!stat->cached will it lstat the file and set stat_ret to\nsomething other than zero.\n\nThis patch checks if skip-worktree bit is on in checkout_target\nand just returns so that the entry doesn't not end up in the\nworking directory. This is so that apply will not create a file\nin the working directory, then update the index but not keep the\nworking directory up to date with the changes that happened in the index.\n\nSigned-off-by: Kevin Willford \n---\n apply.c | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/apply.c b/apply.c\nindex 0e2caeab9c..0da5d0b7c9 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -3327,6 +3327,24 @@ static int checkout_target(struct index_state *istate,\n {\n \tstruct checkout costate = CHECKOUT_INIT;\n \n+\t/*\n+\t * Do not checkout the entry if the skipworktree bit is set\n+\t *\n+\t * Both callers of this method (check_preimage and load_current)\n+\t * check for the existance of the file before calling this\n+\t * method so we know that the file doesn't exist at this point\n+\t * and we don't need to perform that check again here.\n+\t * We just need to check the skip-worktree and return.\n+\t *\n+\t * This is to prevent git from creating a file in the\n+\t * working directory that has the skip-worktree bit on,\n+\t * then updating the index from the patch and not keeping\n+\t * the working directory version up to date with what it\n+\t * changed the index version to be.\n+\t */\n+\tif (ce_skip_worktree(ce))\n+\t\treturn 0;\n+\n \tcostate.refresh_cache = 1;\n \tcostate.istate = istate;\n \tif (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))\n-- \n2.12.2.windows.2.1.g7df5db8d31\n\n"},{"id":"316374","messageId":"20170407192357.948-4-kewillf@microsoft.com","threadId":"45632","inReplyTo":"20170407192357.948-1-kewillf@microsoft.com","subject":"[PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Kevin Willford","fromEmail":"kcwillford@gmail.com","sentAt":"2017-04-07T19:23:57Z","receivedAt":"2017-04-07T19:24:26Z","isPatch":true,"sender":{"key":"kcwillford@gmail.com","avatar":null},"body":"When using the sparse checkout feature the git reset command will add\nentries to the index that will have the skip-worktree bit off but will\nleave the working directory empty. File data is lost because the index\nversion of the files has been changed but there is nothing that is in the\nworking directory. This will cause the next status call to show either\ndeleted for files modified or deleting or nothing for files added.\nThe added files should be shown as untracked and modified files should\nbe shown as modified.\n\nTo fix this when the reset is running if there is not a file in the\nworking directory and if it will be missing with the new index entry\nor was not missing in the previous version, we create the previous index\nversion of the file in the working directory so that status will report\ncorrectly and the files will be availble for the user to deal with.\n\nSigned-off-by: Kevin Willford \n---\n builtin/reset.c | 34 +++++++++++++++++++++++\n t/t7114-reset-sparse-checkout.sh | 58 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 92 insertions(+)\n create mode 100755 t/t7114-reset-sparse-checkout.sh\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ab915bfcb..8ba97999f4 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,6 +21,7 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"dir.h\"\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n@@ -117,12 +118,45 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n \tint i;\n+\tint pos;\n \tint intent_to_add = *(int *)data;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *one = q->queue[i]->one;\n+\t\tstruct diff_filespec *two = q->queue[i]->two;\n \t\tint is_missing = !(one->mode && !is_null_oid(&one->oid));\n+\t\tint was_missing = !two->mode && is_null_oid(&two->oid);\n \t\tstruct cache_entry *ce;\n+\t\tstruct cache_entry *ceBefore;\n+\t\tstruct checkout state = CHECKOUT_INIT;\n+\n+\t\t/*\n+\t\t * When using the sparse-checkout feature the cache entries that are\n+\t\t * added here will not have the skip-worktree bit set.\n+\t\t * Without this code there is data that is lost because the files that\n+\t\t * would normally be in the working directory are not there and show as\n+\t\t * deleted for the next status or in the case of added files just disappear.\n+\t\t * We need to create the previous version of the files in the working\n+\t\t * directory so that they will have the right content and the next\n+\t\t * status call will show modified or untracked files correctly.\n+\t\t */\n+\t\tif (core_apply_sparse_checkout && !file_exists(two->path))\n+\t\t{\n+\t\t\tpos = cache_name_pos(two->path, strlen(two->path));\n+\t\t\tif ((pos >= 0 && ce_skip_worktree(active_cache[pos])) && (is_missing || !was_missing))\n+\t\t\t{\n+\t\t\t\tstate.force = 1;\n+\t\t\t\tstate.refresh_cache = 1;\n+\t\t\t\tstate.istate = &the_index;\n+\t\t\t\tceBefore = make_cache_entry(two->mode, two->oid.hash, two->path,\n+\t\t\t\t\t0, 0);\n+\t\t\t\tif (!ceBefore)\n+\t\t\t\t\tdie(_(\"make_cache_entry failed for path '%s'\"),\n+\t\t\t\t\t\ttwo->path);\n+\n+\t\t\t\tcheckout_entry(ceBefore, &state, NULL);\n+\t\t\t}\n+\t\t}\n \n \t\tif (is_missing && !intent_to_add) {\n \t\t\tremove_file_from_cache(one->path);\ndiff --git a/t/t7114-reset-sparse-checkout.sh b/t/t7114-reset-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..8dd88fd46d\n--- /dev/null\n+++ b/t/t7114-reset-sparse-checkout.sh\n@@ -0,0 +1,58 @@\n+#!/bin/sh\n+\n+test_description='reset when using a sparse-checkout'\n+\n+. ./test-lib.sh\n+\n+# reset using a sparse-checkout file\n+\n+test_expect_success 'setup' '\n+\ttest_tick &&\n+\techo \"checkout file\" >c &&\n+\techo \"modify file\" >m &&\n+\techo \"delete file\" >d &&\n+\tgit add . &&\n+\tgit commit -m \"initial commit\" &&\n+\techo \"added file\" >a &&\n+\techo \"modification of a file\" >m &&\n+\tgit rm d &&\n+\tgit add . &&\n+\tgit commit -m \"second commit\" &&\n+\tgit checkout -b endCommit\n+'\n+\n+test_expect_success 'reset when there is a sparse-checkout' '\n+\techo \"/c\" >.git/info/sparse-checkout &&\n+\ttest_config core.sparsecheckout true &&\n+\tgit checkout -b resetBranch &&\n+\ttest_path_is_missing m &&\n+\ttest_path_is_missing a &&\n+\ttest_path_is_missing d &&\n+\tgit reset HEAD~1 &&\n+\ttest \"checkout file\" = \"$(cat c)\" &&\n+\ttest \"modification of a file\" = \"$(cat m)\" &&\n+\ttest \"added file\" = \"$(cat a)\" &&\n+\ttest_path_is_missing d\n+'\n+\n+test_expect_success 'reset after deleting file without skip-worktree bit' '\n+\tgit checkout -f endCommit &&\n+\tgit clean -xdf &&\n+\techo \"/c\n+/m\" >.git/info/sparse-checkout &&\n+\ttest_config core.sparsecheckout true &&\n+\tgit checkout -b resetAfterDelete &&\n+\ttest_path_is_file m &&\n+\ttest_path_is_missing a &&\n+\ttest_path_is_missing d &&\n+\trm -f m &&\n+\tgit reset HEAD~1 &&\n+\ttest \"checkout file\" = \"$(cat c)\" &&\n+\ttest \"added file\" = \"$(cat a)\" &&\n+\ttest_path_is_missing m &&\n+\ttest_path_is_missing d\n+'\n+\n+\n+\n+test_done\n-- \n2.12.2.windows.2.1.g7df5db8d31\n\n"},{"id":"316375","messageId":"CAGZ79kZg+QpQ73i2=boGTDAhdtfkrbA8ijNEf2pmanNhiiLwmA@mail.gmail.com","threadId":"45632","inReplyTo":"20170407192357.948-1-kewillf@microsoft.com","subject":"Re: [PATCH 0/3] fix working directory file issues while using sparse-checkout","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-07T19:27:01Z","receivedAt":"2017-04-07T19:27:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"+ Duy, for sparse checkout\n\nOn Fri, Apr 7, 2017 at 12:23 PM, Kevin Willford wrote:\n> While using the sparse-checkout feature there are scenarios where\n> the working directory should and should not be updated. This patch\n> series addresses issues found in reset, apply, and merge conflicts.\n>\n> Kevin Willford (3):\n> merge-recursive.c: conflict using sparse should update file\n> apply.c: do not checkout file when skip-worktree bit set\n> reset.c: update files when using sparse to avoid data loss.\n>\n> apply.c | 18 +++++++++++++\n> builtin/reset.c | 34 +++++++++++++++++++++++\n> merge-recursive.c | 8 ++++++\n> t/t7114-reset-sparse-checkout.sh | 58 ++++++++++++++++++++++++++++++++++++++++\n> t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++\n> 5 files changed, 145 insertions(+)\n> create mode 100755 t/t7114-reset-sparse-checkout.sh\n> create mode 100755 t/t7614-merge-sparse-checkout.sh\n>\n> --\n> 2.12.2.windows.2.1.g7df5db8d31\n>\n"},{"id":"316376","messageId":"CAGZ79kYKVwCLjvo0_qpwHvnd0XVzKM0DL-xPsMKqh8=Y01=ROw@mail.gmail.com","threadId":"45632","inReplyTo":"CAGZ79kZg+QpQ73i2=boGTDAhdtfkrbA8ijNEf2pmanNhiiLwmA@mail.gmail.com","subject":"Re: [PATCH 0/3] fix working directory file issues while using sparse-checkout","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-07T19:27:20Z","receivedAt":"2017-04-07T19:27:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This time for real.\n\nOn Fri, Apr 7, 2017 at 12:27 PM, Stefan Beller wrote:\n> + Duy, for sparse checkout\n>\n> On Fri, Apr 7, 2017 at 12:23 PM, Kevin Willford wrote:\n>> While using the sparse-checkout feature there are scenarios where\n>> the working directory should and should not be updated. This patch\n>> series addresses issues found in reset, apply, and merge conflicts.\n>>\n>> Kevin Willford (3):\n>> merge-recursive.c: conflict using sparse should update file\n>> apply.c: do not checkout file when skip-worktree bit set\n>> reset.c: update files when using sparse to avoid data loss.\n>>\n>> apply.c | 18 +++++++++++++\n>> builtin/reset.c | 34 +++++++++++++++++++++++\n>> merge-recursive.c | 8 ++++++\n>> t/t7114-reset-sparse-checkout.sh | 58 ++++++++++++++++++++++++++++++++++++++++\n>> t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++\n>> 5 files changed, 145 insertions(+)\n>> create mode 100755 t/t7114-reset-sparse-checkout.sh\n>> create mode 100755 t/t7614-merge-sparse-checkout.sh\n>>\n>> --\n>> 2.12.2.windows.2.1.g7df5db8d31\n>>\n"},{"id":"316383","messageId":"CAGZ79kb30KXzvy7evSKQmtb8Qeo0OLH3-nfPn9eRsoTjcjFR=Q@mail.gmail.com","threadId":"45632","inReplyTo":"20170407192357.948-3-kewillf@microsoft.com","subject":"Re: [PATCH 2/3] apply.c: do not checkout file when skip-worktree bit set","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-07T22:28:53Z","receivedAt":"2017-04-07T22:29:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 7, 2017 at 12:23 PM, Kevin Willford wrote:\n> When using the sparse-checkout feature git should not write to\n> the working directory for files with the skip-worktree bit on.\n> With the skip-worktree bit on the file may or may not be in\n> the working directory and if it is not we don't want or need to\n> create it by calling checkout_entry.\n>\n> There are two callers of checkout_target. Both of which check\n> that the file does not exist before calling checkout_target.\n> load_current which make a call to lstat right before calling checkout_target\n> and check_preimage which will only run checkout_taret it stat_ret\n> is less than zero. It sets stat_ret to zero and only if\n> !stat->cached will it lstat the file and set stat_ret to\n> something other than zero.\n>\n> This patch checks if skip-worktree bit is on in checkout_target\n> and just returns so that the entry doesn't not end up in the\n> working directory. This is so that apply will not create a file\n> in the working directory, then update the index but not keep the\n> working directory up to date with the changes that happened in the index.\n>\n> Signed-off-by: Kevin Willford \n> ---\n> apply.c | 18 ++++++++++++++++++\n> 1 file changed, 18 insertions(+)\n>\n> diff --git a/apply.c b/apply.c\n> index 0e2caeab9c..0da5d0b7c9 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -3327,6 +3327,24 @@ static int checkout_target(struct index_state *istate,\n> {\n> struct checkout costate = CHECKOUT_INIT;\n>\n> + /*\n> + * Do not checkout the entry if the skipworktree bit is set\n> + *\n> + * Both callers of this method (check_preimage and load_current)\n> + * check for the existance of the file before calling this\n> + * method so we know that the file doesn't exist at this point\n> + * and we don't need to perform that check again here.\n> + * We just need to check the skip-worktree and return.\n> + *\n> + * This is to prevent git from creating a file in the\n> + * working directory that has the skip-worktree bit on,\n> + * then updating the index from the patch and not keeping\n> + * the working directory version up to date with what it\n> + * changed the index version to be.\n> + */\n\nI have the impression that this information is better in the\ncommit message rather than in the code itself, i.e. we can\ndrop this comment here?\n\nThanks,\nStefan\n\n> + if (ce_skip_worktree(ce))\n> + return 0;\n> +\n> costate.refresh_cache = 1;\n> costate.istate = istate;\n> if (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))\n> --\n> 2.12.2.windows.2.1.g7df5db8d31\n>\n"},{"id":"316384","messageId":"CAGZ79kYJvLQxTcL2-hLS+MOoWQoshxpeLzNoNXbH6zQqGbbCdw@mail.gmail.com","threadId":"45632","inReplyTo":"20170407192357.948-4-kewillf@microsoft.com","subject":"Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-07T22:41:42Z","receivedAt":"2017-04-07T22:41:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 7, 2017 at 12:23 PM, Kevin Willford wrote:\n> When using the sparse checkout feature the git reset command will add\n> entries to the index that will have the skip-worktree bit off but will\n> leave the working directory empty. File data is lost because the index\n> version of the files has been changed but there is nothing that is in the\n> working directory. This will cause the next status call to show either\n> deleted for files modified or deleting or nothing for files added.\n> The added files should be shown as untracked and modified files should\n> be shown as modified.\n>\n> To fix this when the reset is running if there is not a file in the\n> working directory and if it will be missing with the new index entry\n> or was not missing in the previous version, we create the previous index\n> version of the file in the working directory so that status will report\n> correctly and the files will be availble for the user to deal with.\n>\n> Signed-off-by: Kevin Willford \n> ---\n> builtin/reset.c | 34 +++++++++++++++++++++++\n> t/t7114-reset-sparse-checkout.sh | 58 ++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 92 insertions(+)\n> create mode 100755 t/t7114-reset-sparse-checkout.sh\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 8ab915bfcb..8ba97999f4 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -21,6 +21,7 @@\n> #include \"parse-options.h\"\n> #include \"unpack-trees.h\"\n> #include \"cache-tree.h\"\n> +#include \"dir.h\"\n>\n> static const char * const git_reset_usage[] = {\n> N_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n> @@ -117,12 +118,45 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n> struct diff_options *opt, void *data)\n> {\n> int i;\n> + int pos;\n\nYou could declare this inside the for loop.\n\n> int intent_to_add = *(int *)data;\n>\n> for (i = 0; i < q->nr; i++) {\n> struct diff_filespec *one = q->queue[i]->one;\n> + struct diff_filespec *two = q->queue[i]->two;\n> int is_missing = !(one->mode && !is_null_oid(&one->oid));\n> + int was_missing = !two->mode && is_null_oid(&two->oid);\n> struct cache_entry *ce;\n> + struct cache_entry *ceBefore;\n> + struct checkout state = CHECKOUT_INIT;\n> +\n> + /*\n> + * When using the sparse-checkout feature the cache entries that are\n> + * added here will not have the skip-worktree bit set.\n> + * Without this code there is data that is lost because the files that\n> + * would normally be in the working directory are not there and show as\n> + * deleted for the next status or in the case of added files just disappear.\n> + * We need to create the previous version of the files in the working\n> + * directory so that they will have the right content and the next\n> + * status call will show modified or untracked files correctly.\n> + */\n\nThis comment also belongs to the commit message IMHO, as\nit describes the bug on a high level, and when it is in the code it wastes\nscreen real estate; commit messages however have low prices for\nscreen real estate. ;-)\n\n> + if (core_apply_sparse_checkout && !file_exists(two->path))\n> + {\n\nCoding style: Git prefers { at the end of the line:\n\n if (..) {\n ..\n\n> +test_expect_success 'reset after deleting file without skip-worktree bit' '\n> + git checkout -f endCommit &&\n> + git clean -xdf &&\n> + echo \"/c\n> +/m\" >.git/info/sparse-checkout &&\n\nhuh? Did your mailer wrap lines here or do you intend to have\na LF in the output?\n\nAssuming the latter, I think we prefer cat with here-doc for\nmulti line content, i.e.\n\n cat >.git/info/sparse-checkout <<-\\EOF\n /c\n /m\n EOF\n test_config ...\n ...\n\n\nThanks,\nStefan\n"},{"id":"316466","messageId":"20170410101123.GA19325@ash","threadId":"45632","inReplyTo":"20170407192357.948-3-kewillf@microsoft.com","subject":"Re: [PATCH 2/3] apply.c: do not checkout file when skip-worktree bit set","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-10T10:11:23Z","receivedAt":"2017-04-10T10:11:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Apr 07, 2017 at 12:23:56PM -0700, Kevin Willford wrote:\n> When using the sparse-checkout feature git should not write to\n> the working directory for files with the skip-worktree bit on.\n> With the skip-worktree bit on the file may or may not be in\n> the working directory and if it is not we don't want or need to\n> create it by calling checkout_entry.\n> \n> There are two callers of checkout_target. Both of which check\n> that the file does not exist before calling checkout_target.\n> load_current which make a call to lstat right before calling checkout_target\n> and check_preimage which will only run checkout_taret it stat_ret\n> is less than zero. It sets stat_ret to zero and only if\n> !stat->cached will it lstat the file and set stat_ret to\n> something other than zero.\n> \n> This patch checks if skip-worktree bit is on in checkout_target\n> and just returns so that the entry doesn't not end up in the\n> working directory. This is so that apply will not create a file\n> in the working directory, then update the index but not keep the\n> working directory up to date with the changes that happened in the index.\n\nNo. I think we should check the skip-worktree at the call sites\ninstead of just exit early here. What if the caller expects the file\nto exist if checkout_target returns zero? We just don't have enough\ninformation to decide what is the right thing to do in this function.\n\nThere are two call sites.\n\nIn load_current() we could skip the lstat() call before\ncheckout_target() too if skip-worktree is set. We also need to skip\nthe verify_index_match(). I'm not exactly sure if that code expects\nthe target file on disk afterwarsd though (apply.c is not really my\nthing).\n\nIn check_preimage() it looks easier because we could follow the\nstate->cached code for skip-worktree, I think.\n\n> \n> Signed-off-by: Kevin Willford \n> ---\n> apply.c | 18 ++++++++++++++++++\n> 1 file changed, 18 insertions(+)\n> \n> diff --git a/apply.c b/apply.c\n> index 0e2caeab9c..0da5d0b7c9 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -3327,6 +3327,24 @@ static int checkout_target(struct index_state *istate,\n> {\n> \tstruct checkout costate = CHECKOUT_INIT;\n> \n> +\t/*\n> +\t * Do not checkout the entry if the skipworktree bit is set\n> +\t *\n> +\t * Both callers of this method (check_preimage and load_current)\n> +\t * check for the existance of the file before calling this\n> +\t * method so we know that the file doesn't exist at this point\n> +\t * and we don't need to perform that check again here.\n> +\t * We just need to check the skip-worktree and return.\n> +\t *\n> +\t * This is to prevent git from creating a file in the\n> +\t * working directory that has the skip-worktree bit on,\n> +\t * then updating the index from the patch and not keeping\n> +\t * the working directory version up to date with what it\n> +\t * changed the index version to be.\n> +\t */\n> +\tif (ce_skip_worktree(ce))\n> +\t\treturn 0;\n> +\n> \tcostate.refresh_cache = 1;\n> \tcostate.istate = istate;\n> \tif (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))\n> -- \n> 2.12.2.windows.2.1.g7df5db8d31\n> \n"},{"id":"316469","messageId":"20170410102429.GB19325@ash","threadId":"45632","inReplyTo":"20170407192357.948-4-kewillf@microsoft.com","subject":"Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-10T10:24:29Z","receivedAt":"2017-04-10T10:24:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Apr 07, 2017 at 12:23:57PM -0700, Kevin Willford wrote:\n> When using the sparse checkout feature the git reset command will add\n\n\"git reset\" has three different modes. It would be good if you mention\nwhat mode is affected here. The tests are for --mixed only. I wonder\nif we need to do anything for --hard and --soft?\n\n--soft touches branch SHA-1 index only, not worktree, so probably not.\n\n--hard should be handled by unpack_trees(), I think.\n\nBut it would be good to cover these in the commit message as well to\nstop readers from wondering.\n\n> entries to the index that will have the skip-worktree bit off but will\n> leave the working directory empty. File data is lost because the index\n> version of the files has been changed but there is nothing that is in the\n> working directory. This will cause the next status call to show either\n> deleted for files modified or deleting or nothing for files added.\n> The added files should be shown as untracked and modified files should\n> be shown as modified.\n\nHmm.. reading --mixed documentation again (\"Resets the index but not\nworking tree\"), I think the current behavior is expected regardless of\nskip-worktree bits.\n\nPerhaps the problem is the loss of skip-worktree bits on entries added\nby update_index_from_diff()? If the bits are at the right place, then\nit should not matter if the same version exists on worktree or not and\n\"status\" or \"commit\" should work as expected, I think.\n\n--\nDuy\n"},{"id":"316471","messageId":"20170410103643.GC19325@ash","threadId":"45632","inReplyTo":"20170407192357.948-2-kewillf@microsoft.com","subject":"Re: [PATCH 1/3] merge-recursive.c: conflict using sparse should update file","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-10T10:36:43Z","receivedAt":"2017-04-10T10:36:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Apr 07, 2017 at 12:23:55PM -0700, Kevin Willford wrote:\n> Update the file when there is a conflict with a modify/delete scenario\n> when using the sparse-checkout feature since the file might not be on disk\n> because the skip-worktree bit is on and the user will need the file and\n> content to determine how to resolve the conflict.\n\nI'm a bit uncertain about this, but it's mostly because I'm not\nfamiliar with merge-recursive.c. I think the general principle is, if\na file is conflicted it must NOT have skip-worktree bit on and the\nworktree version must be restored at the same time the bit is removed.\n\nI think we should do that (restore the worktree version) when the\nskip-worktree bit is removed. What I'm not sure is when the bit is\nremoved in this code.\n\nI'm guessing that the bit is removed by unpack_trees() with the\nthreeway merge. Maybe we should restore the worktree version there at\nthat time. It does not matter what conflict type it is, just restore\nthe file (which should be what merge-recursive.c expects in normal\ncase) then merge-recursive.c can proceed to update, delete or whatever\nto present the conflict to the user.\n\n>\n> Signed-off-by: Kevin Willford \n> ---\n> merge-recursive.c | 8 ++++++++\n> t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++++++++++\n> 2 files changed, 35 insertions(+)\n> create mode 100755 t/t7614-merge-sparse-checkout.sh\n> \n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index b7ff1ada3c..54fce93dae 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1103,6 +1103,14 @@ static int handle_change_delete(struct merge_options *o,\n> \t\t\t \"and %s in %s. Version %s of %s left in tree.\"),\n> \t\t\t change, path, o->branch2, change_past,\n> \t\t\t o->branch1, o->branch1, path);\n> +\t\t\t/*\n> +\t\t\t * In a sparse checkout the file may not exist. Sadly,\n> +\t\t\t * the CE_SKIP_WORKTREE flag is not preserved in the\n> +\t\t\t * case of conflicts, therefore we do the next best\n> +\t\t\t * thing: verify that the file exists.\n> +\t\t\t */\n> +\t\t\tif (!file_exists(path))\n> +\t\t\t\tret = update_file(o, 0, a_oid, a_mode, path);\n> \t\t} else {\n> \t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> \t\t\t \"and %s in %s. Version %s of %s left in tree at %s.\"),\n> diff --git a/t/t7614-merge-sparse-checkout.sh b/t/t7614-merge-sparse-checkout.sh\n> new file mode 100755\n> index 0000000000..6922f0244f\n> --- /dev/null\n> +++ b/t/t7614-merge-sparse-checkout.sh\n> @@ -0,0 +1,27 @@\n> +#!/bin/sh\n> +\n> +test_description='merge can handle sparse-checkout'\n> +\n> +. ./test-lib.sh\n> +\n> +# merges with conflicts\n> +\n> +test_expect_success 'setup' '\n> +\ttest_commit a &&\n> +\ttest_commit file &&\n> +\tgit checkout -b delete-file &&\n> +\tgit rm file.t &&\n> +\ttest_tick &&\n> +\tgit commit -m \"remove file\" &&\n> +\tgit checkout master &&\n> +\ttest_commit modify file.t changed\n> +'\n> +\n> +test_expect_success 'merge conflict deleted file and modified' '\n> +\techo \"/a\" >.git/info/sparse-checkout &&\n> +\ttest_config core.sparsecheckout true &&\n> +\ttest_must_fail git merge delete-file &&\n> +\ttest_path_is_file file.t\n> +'\n> +\n> +test_done\n> -- \n> 2.12.2.windows.2.1.g7df5db8d31\n> \n"},{"id":"316653","messageId":"SN2PR03MB2333BCFD2E1A6A400706F94CB7000@SN2PR03MB2333.namprd03.prod.outlook.com","threadId":"45632","inReplyTo":"20170410102429.GB19325@ash","subject":"RE: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Kevin Willford","fromEmail":"kewillf@microsoft.com","sentAt":"2017-04-11T22:30:04Z","receivedAt":"2017-04-11T22:30:12Z","isPatch":true,"sender":{"key":"kewillf@microsoft.com","avatar":null},"body":"> -----Original Message-----\n> From: Duy Nguyen [mailto:pclouds@gmail.com]\n> Sent: Monday, April 10, 2017 4:24 AM\n> To: Kevin Willford \n> Cc: git@vger.kernel.org; gitster@pobox.com; peff@peff.net; Kevin Willford\n> \n> Subject: Re: [PATCH 3/3] reset.c: update files when using sparse to avoid\n> data loss.\n> \n> On Fri, Apr 07, 2017 at 12:23:57PM -0700, Kevin Willford wrote:\n> > When using the sparse checkout feature the git reset command will add\n> \n> \"git reset\" has three different modes. It would be good if you mention what\n> mode is affected here. The tests are for --mixed only. I wonder if we need to\n> do anything for --hard and --soft?\n> \n> --soft touches branch SHA-1 index only, not worktree, so probably not.\n> \n> --hard should be handled by unpack_trees(), I think.\n> \n> But it would be good to cover these in the commit message as well to stop\n> readers from wondering.\n\nSounds good.\n\n> \n> > entries to the index that will have the skip-worktree bit off but will\n> > leave the working directory empty. File data is lost because the\n> > index version of the files has been changed but there is nothing that\n> > is in the working directory. This will cause the next status call to\n> > show either deleted for files modified or deleting or nothing for files\n> added.\n> > The added files should be shown as untracked and modified files should\n> > be shown as modified.\n> \n> Hmm.. reading --mixed documentation again (\"Resets the index but not\n> working tree\"), I think the current behavior is expected regardless of skip-\n> worktree bits.\n> \n> Perhaps the problem is the loss of skip-worktree bits on entries added by\n> update_index_from_diff()? If the bits are at the right place, then it should\n> not matter if the same version exists on worktree or not and \"status\" or\n> \"commit\" should work as expected, I think.\n> \n> --\n> Duy\n\nThe loss of the skip-worktree bits is part of the problem if you are talking\nabout modified files. The other issue that I was having is when running a reset\nand there were files added in the commit that is being reset, there will not\nbe an entry in the index and not a file on disk so the data for that file is\ncompletely lost at that point. \"status\" also doesn't include anything about\nthis loss of data. On modified files status will at least have the file as deleted\nsince there is still an index entry but again the previous version of the file\nand it's data is lost.\n\nTo me this is totally unexpected behavior, for example if I am on a commit\nwhere there are only files that where added and run a reset HEAD~1 and\nthen a status, it will show a clean working directory. Regardless of \nskip-worktree bits the user needs to have the data in the working directory\nafter the reset or data is lost which is always bad.\n\nKevin\n\n"},{"id":"316686","messageId":"CACsJy8Dw8qfS6G71y3FoT9w7H=FABMx=5R=v8JcPCEHtv1vLcw@mail.gmail.com","threadId":"45632","inReplyTo":"SN2PR03MB2333BCFD2E1A6A400706F94CB7000@SN2PR03MB2333.namprd03.prod.outlook.com","subject":"Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-12T13:21:10Z","receivedAt":"2017-04-12T13:21:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 12, 2017 at 5:30 AM, Kevin Willford wrote:\n> The loss of the skip-worktree bits is part of the problem if you are talking\n> about modified files. The other issue that I was having is when running a reset\n> and there were files added in the commit that is being reset, there will not\n> be an entry in the index and not a file on disk so the data for that file is\n> completely lost at that point. \"status\" also doesn't include anything about\n> this loss of data. On modified files status will at least have the file as deleted\n> since there is still an index entry but again the previous version of the file\n> and it's data is lost.\n\nWell, we could have \"deleted\" index entries, those marked with\nCE_REMOVE. They are never written down to file though, so 'status'\nwon't benefit from that. Hopefully we can restore the file before the\nindex file is written down and we really lose skip-worktree bits?\n\n> To me this is totally unexpected behavior, for example if I am on a commit\n> where there are only files that where added and run a reset HEAD~1 and\n> then a status, it will show a clean working directory. Regardless of\n> skip-worktree bits the user needs to have the data in the working directory\n> after the reset or data is lost which is always bad.\n\nI agree we no longer have a place to save the skip-worktree bit, we\nhave to restore the state back as if skip-worktree bit does not exist.\nIt would be good if we could keep the logic in unpack_trees() though.\nFor example, if the file is present on disk even if skip-worktree bit\nis on, unpack_trees() would abort instead of silently overwriting it.\nThis is a difference between skip-worktree and assume-unchanged bits.\nIf you do explicit checkout_entry() you might have to add more checks\nto keep behavior consistent.\n-- \nDuy\n"},{"id":"316691","messageId":"SN2PR03MB2333B9A0A7A3888F0402594DB7030@SN2PR03MB2333.namprd03.prod.outlook.com","threadId":"45632","inReplyTo":"CACsJy8Dw8qfS6G71y3FoT9w7H=FABMx=5R=v8JcPCEHtv1vLcw@mail.gmail.com","subject":"RE: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Kevin Willford","fromEmail":"kewillf@microsoft.com","sentAt":"2017-04-12T15:37:43Z","receivedAt":"2017-04-12T15:37:52Z","isPatch":true,"sender":{"key":"kewillf@microsoft.com","avatar":null},"body":"\n> -----Original Message-----\n> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n> Behalf Of Duy Nguyen\n> Sent: Wednesday, April 12, 2017 7:21 AM\n> To: Kevin Willford \n> Cc: Kevin Willford ; git@vger.kernel.org;\n> gitster@pobox.com; peff@peff.net\n> Subject: Re: [PATCH 3/3] reset.c: update files when using sparse to avoid\n> data loss.\n> \n> On Wed, Apr 12, 2017 at 5:30 AM, Kevin Willford \n> wrote:\n> > The loss of the skip-worktree bits is part of the problem if you are\n> > talking about modified files. The other issue that I was having is\n> > when running a reset and there were files added in the commit that is\n> > being reset, there will not be an entry in the index and not a file on\n> > disk so the data for that file is completely lost at that point.\n> > \"status\" also doesn't include anything about this loss of data. On\n> > modified files status will at least have the file as deleted since\n> > there is still an index entry but again the previous version of the file and it's\n> data is lost.\n> \n> Well, we could have \"deleted\" index entries, those marked with\n> CE_REMOVE. They are never written down to file though, so 'status'\n> won't benefit from that. Hopefully we can restore the file before the index\n> file is written down and we really lose skip-worktree bits?\n\nBecause this is a reset --mixed it will never run through unpack_trees and \nThe entries are never marked with CE_REMOVE.\n\n> \n> > To me this is totally unexpected behavior, for example if I am on a\n> > commit where there are only files that where added and run a reset\n> > HEAD~1 and then a status, it will show a clean working directory.\n> > Regardless of skip-worktree bits the user needs to have the data in\n> > the working directory after the reset or data is lost which is always bad.\n> \n> I agree we no longer have a place to save the skip-worktree bit, we have to\n> restore the state back as if skip-worktree bit does not exist.\n> It would be good if we could keep the logic in unpack_trees() though.\n> For example, if the file is present on disk even if skip-worktree bit is on,\n> unpack_trees() would abort instead of silently overwriting it.\n> This is a difference between skip-worktree and assume-unchanged bits.\n> If you do explicit checkout_entry() you might have to add more checks to\n> keep behavior consistent.\n> --\n> Duy\n\nBecause this is a reset --mixed it will follow the code path calling read_from_tree\nand ends up calling update_index_from_diff in the format_callback of the diff,\nso unpack_trees() is never called in the --mixed case. This code change also only applies\nwhen the file does not exist and the skip-worktree bit is on and the updated \nindex entry either will be missing (covers the added scenario) or was not missing\nbefore (covers the modified scenario). If there is a better way to get the previous\nindex entry to disk than what I am doing, I am happy to implement it correctly.\n\nThanks,\nKevin\n"},{"id":"316906","messageId":"CACsJy8CbxGPeWHgR8s-ix_wxPODvisw6eaLnEUV4LDmoeXt5BQ@mail.gmail.com","threadId":"45632","inReplyTo":"SN2PR03MB2333B9A0A7A3888F0402594DB7030@SN2PR03MB2333.namprd03.prod.outlook.com","subject":"Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-16T04:25:30Z","receivedAt":"2017-04-16T04:26:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 12, 2017 at 10:37 PM, Kevin Willford wrote:\n>\n>> -----Original Message-----\n>> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n>> Behalf Of Duy Nguyen\n>> Sent: Wednesday, April 12, 2017 7:21 AM\n>> To: Kevin Willford \n>> Cc: Kevin Willford ; git@vger.kernel.org;\n>> gitster@pobox.com; peff@peff.net\n>> Subject: Re: [PATCH 3/3] reset.c: update files when using sparse to avoid\n>> data loss.\n>>\n>> On Wed, Apr 12, 2017 at 5:30 AM, Kevin Willford \n>> wrote:\n>> > The loss of the skip-worktree bits is part of the problem if you are\n>> > talking about modified files. The other issue that I was having is\n>> > when running a reset and there were files added in the commit that is\n>> > being reset, there will not be an entry in the index and not a file on\n>> > disk so the data for that file is completely lost at that point.\n>> > \"status\" also doesn't include anything about this loss of data. On\n>> > modified files status will at least have the file as deleted since\n>> > there is still an index entry but again the previous version of the file and it's\n>> data is lost.\n>>\n>> Well, we could have \"deleted\" index entries, those marked with\n>> CE_REMOVE. They are never written down to file though, so 'status'\n>> won't benefit from that. Hopefully we can restore the file before the index\n>> file is written down and we really lose skip-worktree bits?\n>\n> Because this is a reset --mixed it will never run through unpack_trees and\n> The entries are never marked with CE_REMOVE.\n\nI know. But in my view, it should. All updates from a tree object to\nthe index should happen through unpack_trees().\n\n>> > To me this is totally unexpected behavior, for example if I am on a\n>> > commit where there are only files that where added and run a reset\n>> > HEAD~1 and then a status, it will show a clean working directory.\n>> > Regardless of skip-worktree bits the user needs to have the data in\n>> > the working directory after the reset or data is lost which is always bad.\n>>\n>> I agree we no longer have a place to save the skip-worktree bit, we have to\n>> restore the state back as if skip-worktree bit does not exist.\n>> It would be good if we could keep the logic in unpack_trees() though.\n>> For example, if the file is present on disk even if skip-worktree bit is on,\n>> unpack_trees() would abort instead of silently overwriting it.\n>> This is a difference between skip-worktree and assume-unchanged bits.\n>> If you do explicit checkout_entry() you might have to add more checks to\n>> keep behavior consistent.\n>> --\n>> Duy\n>\n> Because this is a reset --mixed it will follow the code path calling read_from_tree\n> and ends up calling update_index_from_diff in the format_callback of the diff,\n> so unpack_trees() is never called in the --mixed case. This code change also only applies\n> when the file does not exist and the skip-worktree bit is on and the updated\n> index entry either will be missing (covers the added scenario) or was not missing\n> before (covers the modified scenario). If there is a better way to get the previous\n> index entry to disk than what I am doing, I am happy to implement it correctly.\n\nI think it's ok to just look at the diff (from update_index_from_diff)\nand restore the on-disk version for now. I'd like to make --mixed use\nunpack_trees() too but I haven't studied this code long enough to see\nwhy it went with \"diff\" instead of \"read-tree\" (which translates\ndirectly to unpack_trees). Maybe there is some subtle reason for that.\nThough it looks like it was more convenient to do \"diff\" in the\ngit-reset.sh version, and that got translated literally to C when the\ncommand was rewritten.\n-- \nDuy\n"},{"id":"317004","messageId":"CACsJy8Bb_maecm2ywHuyMXhORXZBMxMzmtdX_YgCq_DVfwy+Dw@mail.gmail.com","threadId":"45632","inReplyTo":"CACsJy8CbxGPeWHgR8s-ix_wxPODvisw6eaLnEUV4LDmoeXt5BQ@mail.gmail.com","subject":"Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-17T12:09:26Z","receivedAt":"2017-04-17T12:10:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Apr 16, 2017 at 11:25 AM, Duy Nguyen wrote:\n>> Because this is a reset --mixed it will never run through unpack_trees and\n>> The entries are never marked with CE_REMOVE.\n>\n> I know. But in my view, it should. All updates from a tree object to\n> the index should happen through unpack_trees().\n\nJust fyi. My view is wrong. We need to handle a diff here, not through\nunpack_trees() because \"git reset --mixed\" support partial reset, see\n2ce633b928 (git-reset [--mixed] [--] ... - 2006-12-14).\nWe might be able to make unpack_trees() leave certain paths(pec)\nuntouched, but I don't think it is worth it. In other words, your\noriginal patch is the way to go.\n\nPS. I briefly wondered if \"git checkout -- \" had the\nsame problem. I think not, because while --mixed does not touch\nworktree, checkout does, so it should restore on-disk versions if\nneeded. The read_tree_some() call in checkout_paths() should respect\nsparse patterns and add skip-worktree bits back if needed though, but\nI don't think it does that.\n-- \nDuy\n"}]}