{"thread":{"id":"44180","subject":"[PATCH 0/3] i-t-a entries in git-status, and git-commit","startedAt":"2016-09-28T11:44:14Z","lastAt":"2016-10-25T09:35:25Z","messageCount":21,"participants":["Nguyễn Thái Ngọc Duy","Duy Nguyen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"302802","messageId":"20160928114348.1470-1-pclouds@gmail.com","threadId":"44180","inReplyTo":null,"subject":"[PATCH 0/3] i-t-a entries in git-status, and git-commit","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-28T11:43:45Z","receivedAt":"2016-09-28T11:44:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Let's see if I can get the first commit graduated before it turns two\nyears old. Six months to go! This series is about some oddities of\nintent-to-add entries (aka \"git add -N\"):\n\n1) An ita entry in git-status shows that it's added in the index\n (compared to worktree) as a file change, not new file addition. And\n it shows the entry added to HEAD as an empty version. This is due\n to an implementation detail that i-t-a entries are registered in\n the index as empty blobs with a special flag.\n\n2) When you do a \"git commit\" with no changes whatsoever, but you\n happen to have some ita entries registered, git-commit does not\n recognize the resulting commit would be empty and should be caught,\n unless --allow-empty is given. This has been reported several times.\n\n3) Same symptom as 2) but with initial commit (diff code is not used\n for detecting empty commits this time). If you only have ita\n entries in the index by the time you make an initial commit, you'll\n create an empty commit even if --allow-empty is not specified.\n\n1) and 2) are fixed by changing the position of ita entries in diff\ncode. ita entries should be seen as a new file when compared between\nworktree and HEAD, and no change when compared between index and HEAD.\n\nSome previous commit made this move globally and was reverted because\nit could have dangerous unseen side effects, especially in merge code.\nNow we are moving slowly towards that (first patch), this time we try\nto handle case by case (e.g. index_differs_from in the second patch).\n\n3) could be handled pretty easily once you know the problem. This is\nthe third patch.\n\nNguyễn Thái Ngọc Duy (3):\n Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"\n diff-lib.c: enable --shift-ita in index_differs_from()\n commit: don't be fooled by ita entries when creating initial commit\n\n Documentation/diff-options.txt | 7 +++++++\n builtin/commit.c | 11 ++++++++---\n cache.h | 1 +\n diff-lib.c | 13 +++++++++++++\n diff.c | 2 ++\n diff.h | 1 +\n read-cache.c | 10 ++++++++++\n sequencer.c | 5 +++--\n t/t2203-add-intent.sh | 41 +++++++++++++++++++++++++++++++++++++++--\n t/t7064-wtstatus-pv2.sh | 4 ++--\n wt-status.c | 7 ++++++-\n 11 files changed, 92 insertions(+), 10 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302803","messageId":"20160928114348.1470-2-pclouds@gmail.com","threadId":"44180","inReplyTo":"20160928114348.1470-1-pclouds@gmail.com","subject":"[PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-28T11:43:46Z","receivedAt":"2016-09-28T11:44:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The original commit d95d728aba06a34394d15466045cbdabdada58a2 was\nreverted in commit 78cc1a540ba127b13f2f3fd531777b57f3a9cd46 because we\nwere (and still are) not ready for a new world order. A lot more\ninvestigation must be done to see what is impacted. See the 78cc1a5 for\ndetails.\n\nThis patch takes a smaller and safer step. The new behavior is\ncontrolled by shift_ita flag. We can gradually move more diff users to\nthe new behavior after we are sure it's safe to do so. This flag is\nexposed to outside temporarily as \"--shift-ita\" for people who prefer\n\"git diff [--cached] --stat\" to \"git status\"\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/diff-options.txt | 7 +++++++\n diff-lib.c | 12 ++++++++++++\n diff.c | 2 ++\n diff.h | 1 +\n t/t2203-add-intent.sh | 20 ++++++++++++++++++--\n t/t7064-wtstatus-pv2.sh | 4 ++--\n wt-status.c | 7 ++++++-\n 7 files changed, 48 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7805a0c..e63285c 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -575,5 +575,12 @@ endif::git-format-patch[]\n --line-prefix=::\n \tPrepend an additional prefix to every line of output.\n \n+--shift-ita::\n+\tBy default entries added by \"git add -N\" appear as an existing\n+\tempty file in \"git diff\" and a new file in \"git diff --cached\".\n+\tThis option makes the entry appear as a new file in \"git diff\"\n+\tand non-existent in \"git diff --cached\". Experimental option,\n+\tcould be removed in future.\n+\n For more detailed explanation on these common options, see also\n linkgit:gitdiffcore[7].\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 3007c85..62d67c8 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -214,6 +214,11 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\t\t !is_null_oid(&ce->oid),\n \t\t\t\t\t ce->name, 0);\n \t\t\t\tcontinue;\n+\t\t\t} else if (revs->diffopt.shift_ita && ce_intent_to_add(ce)) {\n+\t\t\t\tdiff_addremove(&revs->diffopt, '+', ce->ce_mode,\n+\t\t\t\t\t EMPTY_BLOB_SHA1_BIN, 0,\n+\t\t\t\t\t ce->name, 0);\n+\t\t\t\tcontinue;\n \t\t\t}\n \n \t\t\tchanged = match_stat_with_submodule(&revs->diffopt, ce, &st,\n@@ -379,6 +384,13 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n \tstruct rev_info *revs = o->unpack_data;\n \tint match_missing, cached;\n \n+\t/* i-t-a entries do not actually exist in the index */\n+\tif (revs->diffopt.shift_ita && idx && ce_intent_to_add(idx)) {\n+\t\tidx = NULL;\n+\t\tif (!tree)\n+\t\t\treturn;\t/* nothing to diff.. */\n+\t}\n+\n \t/* if the entry is not checked out, don't examine work tree */\n \tcached = o->index_only ||\n \t\t(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));\ndiff --git a/diff.c b/diff.c\nindex c6da383..4178689 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3923,6 +3923,8 @@ int diff_opt_parse(struct diff_options *options,\n \t\treturn parse_submodule_opt(options, arg);\n \telse if (skip_prefix(arg, \"--ws-error-highlight=\", &arg))\n \t\treturn parse_ws_error_highlight(options, arg);\n+\telse if (!strcmp(arg, \"--shift-ita\"))\n+\t\toptions->shift_ita = 1;\n \n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\ndiff --git a/diff.h b/diff.h\nindex ec76a90..5dd4f9c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -146,6 +146,7 @@ struct diff_options {\n \tint dirstat_permille;\n \tint setup;\n \tint abbrev;\n+\tint shift_ita;\n /* white-space error highlighting */\n #define WSEH_NEW 1\n #define WSEH_CONTEXT 2\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 8f22c43..c6a4648 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -5,10 +5,24 @@ test_description='Intent to add'\n . ./test-lib.sh\n \n test_expect_success 'intent to add' '\n+\ttest_commit 1 &&\n+\tgit rm 1.t &&\n+\techo hello >1.t &&\n \techo hello >file &&\n \techo hello >elif &&\n \tgit add -N file &&\n-\tgit add elif\n+\tgit add elif &&\n+\tgit add -N 1.t\n+'\n+\n+test_expect_success 'git status' '\n+\tgit status --porcelain | grep -v actual >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tDA 1.t\n+\tA elif\n+\t A file\n+\tEOF\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'check result of \"add -N\"' '\n@@ -43,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' '\n \tgit add -N nitfol &&\n \tgit commit -m second &&\n \ttest $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&\n-\ttest $(git diff --name-only HEAD -- nitfol | wc -l) = 1\n+\ttest $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&\n+\ttest $(git diff --name-only --shift-ita HEAD -- nitfol | wc -l) = 0 &&\n+\ttest $(git diff --name-only --shift-ita -- nitfol | wc -l) = 1\n '\n \n test_expect_success 'can commit with an unrelated i-t-a entry in index' '\ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nindex 3012a4d..e319fa2 100755\n--- a/t/t7064-wtstatus-pv2.sh\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -246,8 +246,8 @@ test_expect_success 'verify --intent-to-add output' '\n \tgit add --intent-to-add intent1.add intent2.add &&\n \n \tcat >expect <<-EOF &&\n-\t1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent1.add\n-\t1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent2.add\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 intent1.add\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 intent2.add\n \tEOF\n \n \tgit status --porcelain=v2 >actual &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 9a14658..5f9b1cd 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -437,7 +437,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \n \t\tswitch (p->status) {\n \t\tcase DIFF_STATUS_ADDED:\n-\t\t\tdie(\"BUG: worktree status add???\");\n+\t\t\td->mode_worktree = p->two->mode;\n \t\t\tbreak;\n \n \t\tcase DIFF_STATUS_DELETED:\n@@ -547,6 +547,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, NULL);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \tDIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);\n+\trev.diffopt.shift_ita = 1;\n \tif (!s->show_untracked_files)\n \t\tDIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \tif (s->ignore_submodule_arg) {\n@@ -570,6 +571,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, &opt);\n \n \tDIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);\n+\trev.diffopt.shift_ita = 1;\n \tif (s->ignore_submodule_arg) {\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n \t} else {\n@@ -605,6 +607,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)\n \n \t\tif (!ce_path_match(ce, &s->pathspec, NULL))\n \t\t\tcontinue;\n+\t\tif (ce_intent_to_add(ce))\n+\t\t\tcontinue;\n \t\tit = string_list_insert(&s->change, ce->name);\n \t\td = it->util;\n \t\tif (!d) {\n@@ -911,6 +915,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \n \tinit_revisions(&rev, NULL);\n \tDIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);\n+\trev.diffopt.shift_ita = 1;\n \n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302804","messageId":"20160928114348.1470-3-pclouds@gmail.com","threadId":"44180","inReplyTo":"20160928114348.1470-1-pclouds@gmail.com","subject":"[PATCH 2/3] diff-lib.c: enable --shift-ita in index_differs_from()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-28T11:43:47Z","receivedAt":"2016-09-28T11:44:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This function is basically \"git diff --cached HEAD\", It has three\ncallers:\n\n - One in builtin/commit.c, which uses it to determine if the index is\n different from HEAD and go ahead making a new commit.\n\n - Two in sequencer.c, which use it to see if the index is dirty.\n\nIn the first case, if ita entries are present, index_differs_from() may\nreport \"dirty\". However at tree creation phase, ita entries are dropped\nand the result tree may look exactly the same as HEAD (assuming that\nnothing else is changed in index). This is what we need index_differs_from()\nfor, to catch new empty commits. Enabling shift_ita in index_differs_from()\nfixes this.\n\nIn the second case, the presence of ita entries are enough to say the\nindex is dirty and not continue on. Make an explicit check for that\nbefore comparing index against HEAD (whether --shift-ita is present is\nirrelevant)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n cache.h | 1 +\n diff-lib.c | 1 +\n read-cache.c | 10 ++++++++++\n sequencer.c | 5 +++--\n t/t2203-add-intent.sh | 11 +++++++++++\n 5 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d0494c8..1ddd515 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -561,6 +561,7 @@ extern int do_read_index(struct index_state *istate, const char *path,\n extern int read_index_from(struct index_state *, const char *path);\n extern int is_index_unborn(struct index_state *);\n extern int read_index_unmerged(struct index_state *);\n+extern int has_ita_entries(struct index_state *);\n #define COMMIT_LOCK\t\t(1 << 0)\n #define CLOSE_LOCK\t\t(1 << 1)\n extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 62d67c8..ea55ee2 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -545,6 +545,7 @@ int index_differs_from(const char *def, int diff_flags)\n \tDIFF_OPT_SET(&rev.diffopt, QUICK);\n \tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \trev.diffopt.flags |= diff_flags;\n+\trev.diffopt.shift_ita = 1;\n \trun_diff_index(&rev, 1);\n \tif (rev.pending.alloc)\n \t\tfree(rev.pending.objects);\ndiff --git a/read-cache.c b/read-cache.c\nindex 31eddec..f6a5f61 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1674,6 +1674,16 @@ int is_index_unborn(struct index_state *istate)\n \treturn (!istate->cache_nr && !istate->timestamp.sec);\n }\n \n+int has_ita_entries(struct index_state *istate)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\tif (ce_intent_to_add(istate->cache[i]))\n+\t\t\treturn 1;\n+\treturn 0;\n+}\n+\n int discard_index(struct index_state *istate)\n {\n \tint i;\ndiff --git a/sequencer.c b/sequencer.c\nindex eec8a60..10cded0 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -469,7 +469,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\tunborn = get_sha1(\"HEAD\", head);\n \t\tif (unborn)\n \t\t\thashcpy(head, EMPTY_TREE_SHA1_BIN);\n-\t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0))\n+\t\tif (has_ita_entries(&the_index) ||\n+\t\t index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0))\n \t\t\treturn error_dirty_index(opts);\n \t}\n \tdiscard_cache();\n@@ -1064,7 +1065,7 @@ static int sequencer_continue(struct replay_opts *opts)\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n-\tif (index_differs_from(\"HEAD\", 0))\n+\tif (has_ita_entries(&the_index) || index_differs_from(\"HEAD\", 0))\n \t\treturn error_dirty_index(opts);\n \ttodo_list = todo_list->next;\n \treturn pick_commits(todo_list, opts);\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex c6a4648..aa06415 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -129,5 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '\n \t)\n '\n \n+test_expect_success 'commit: ita entries ignored in empty commit check' '\n+\tgit init empty-subsequent-commit &&\n+\t(\n+\t\tcd empty-subsequent-commit &&\n+\t\ttest_commit one &&\n+\t\t: >two &&\n+\t\tgit add -N two &&\n+\t\ttest_must_fail git commit -m nothing-new-here\n+\t)\n+'\n+\n test_done\n \n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302805","messageId":"20160928114348.1470-4-pclouds@gmail.com","threadId":"44180","inReplyTo":"20160928114348.1470-1-pclouds@gmail.com","subject":"[PATCH 3/3] commit: don't be fooled by ita entries when creating initial commit","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-28T11:43:48Z","receivedAt":"2016-09-28T11:44:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"ita entries are dropped at tree creation phase. If the entire index\nconsists of just ita entries, the result would be a a commit with no\nentries, which should be caught unless --allow-empty is specified.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/commit.c | 11 ++++++++---\n t/t2203-add-intent.sh | 10 ++++++++++\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex bb9f79b..56b24cb 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tif (amend)\n \t\t\tparent = \"HEAD^1\";\n \n-\t\tif (get_sha1(parent, sha1))\n-\t\t\tcommitable = !!active_nr;\n-\t\telse {\n+\t\tif (get_sha1(parent, sha1)) {\n+\t\t\tint i, ita_nr = 0;\n+\n+\t\t\tfor (i = 0; i < active_nr; i++)\n+\t\t\t\tif (ce_intent_to_add(active_cache[i]))\n+\t\t\t\t\tita_nr++;\n+\t\t\tcommitable = active_nr - ita_nr > 0;\n+\t\t} else {\n \t\t\t/*\n \t\t\t * Unless the user did explicitly request a submodule\n \t\t\t * ignore mode by passing a command line option we do\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex aa06415..65314fc 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -129,6 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '\n \t)\n '\n \n+test_expect_success 'commit: ita entries ignored in empty intial commit check' '\n+\tgit init empty-intial-commit &&\n+\t(\n+\t\tcd empty-intial-commit &&\n+\t\t: >one &&\n+\t\tgit add -N one &&\n+\t\ttest_must_fail git commit -m nothing-new-here\n+\t)\n+'\n+\n test_expect_success 'commit: ita entries ignored in empty commit check' '\n \tgit init empty-subsequent-commit &&\n \t(\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302806","messageId":"CACsJy8C_+MeS2LrCejNQFEoHsCPxsSvC_K3mZ6H=_+_ULO291w@mail.gmail.com","threadId":"44180","inReplyTo":"20160928114348.1470-1-pclouds@gmail.com","subject":"Re: [PATCH 0/3] i-t-a entries in git-status, and git-commit","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-28T11:51:44Z","receivedAt":"2016-09-28T11:52:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Sep 28, 2016 at 6:43 PM, Nguyễn Thái Ngọc Duy wrote:\n> 1) and 2) are fixed by changing the position of ita entries in diff\n> code. ita entries should be seen as a new file when compared between\n> worktree and HEAD\n\nBig typo. \"between worktree and index\".\n-- \nDuy\n"},{"id":"302826","messageId":"xmqq8tubkgg5.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"20160928114348.1470-3-pclouds@gmail.com","subject":"Re: [PATCH 2/3] diff-lib.c: enable --shift-ita in index_differs_from()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T18:49:30Z","receivedAt":"2016-09-28T18:49:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This function is basically \"git diff --cached HEAD\", It has three\n> callers:\n>\n> - One in builtin/commit.c, which uses it to determine if the index is\n> different from HEAD and go ahead making a new commit.\n>\n> - Two in sequencer.c, which use it to see if the index is dirty.\n>\n> In the first case, if ita entries are present, index_differs_from() may\n> report \"dirty\". However at tree creation phase, ita entries are dropped\n> and the result tree may look exactly the same as HEAD (assuming that\n> nothing else is changed in index). This is what we need index_differs_from()\n> for, to catch new empty commits. Enabling shift_ita in index_differs_from()\n> fixes this.\n>\n> In the second case, the presence of ita entries are enough to say the\n> index is dirty and not continue on. Make an explicit check for that\n> before comparing index against HEAD (whether --shift-ita is present is\n> irrelevant)\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nThere are three callers to index_differs_from(), which asks \"is the\nindex different from the HEAD\". Because you want to change the\nbehaviour of the function for one of these callers while not\nexposing its undesirable behaviour for the other two callers, you\nguard the call to it with another call to a new helper function,\nwhich needs to scan the entire index one more time.\n\nIt somehow sounds like backwards to me.\n\nIOW, I wonder if it makes more sense to add a new interface to tell\nthe index_differs_from() function \"I want you to use shift-ita\nsemantics\" bit, and pass that when calling it from builtin/commit.c\nwhile not toggling that bit on when the other two callers call it,\nwithout introducing the has_ita_entries() helper function.\n\nBy the way, I think \"shift\" is a bit unclear name for the diffopt\nfield. The log message of [1/3] is totally unclear (it claims\n\"smaller and safer\" without explaining what it exactly does and why\nthat is safer); the documentation update in it is slightly better in\nthat it lets intelligent readers to guess that the option is to\ndeclare that ita entries do not yet exist in the index (hence, \"git\ndiff\" would say \"that's a new file\", while \"git diff --cached\" says\nnothing about it). From that observation, I think a descriptive\nphrase that is suitable for its name than \"shift\" needs to be found\nin a short explanation of what it does: \"treat ita as missing in the\nindex\", e.g. \"rev.diffopt.ita_is_missing = 1\", perhaps?\n\nOther than these small implementation details, I think I like the\ndirection these two patches are taking us (I haven't checked 3/3\nyet).\n\nThanks.\n\n"},{"id":"302829","messageId":"xmqqzimrj03j.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"20160928114348.1470-2-pclouds@gmail.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T19:28:00Z","receivedAt":"2016-09-28T19:28:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The original commit d95d728aba06a34394d15466045cbdabdada58a2 was\n> reverted in commit 78cc1a540ba127b13f2f3fd531777b57f3a9cd46 because we\n> were (and still are) not ready for a new world order. A lot more\n> investigation must be done to see what is impacted. See the 78cc1a5 for\n> details.\n>\n> This patch takes a smaller and safer step. The new behavior is\n> controlled by shift_ita flag. We can gradually move more diff users to\n> the new behavior after we are sure it's safe to do so. This flag is\n> exposed to outside temporarily as \"--shift-ita\" for people who prefer\n> \"git diff [--cached] --stat\" to \"git status\"\n\nLet's stop advertising this as a resurrection of something else.\nThe original that was unconditional was simply broken.\n\nIt is very good to refer to it (and its reversion), when justifying\nwhy this version takes the particular approach to introduce a new\noptional behaviour that can be toggled on selectively, by explaining\nwhy doing this unconditionally was a broken idea that needed to be\nreverted later.\n\nBut you would need to explain what problem this patch attempts to\nsolve and how before even going into that. The above two paragraphs\nare backwards.\n\nAs I already said, --shift-ita is not quite descriptive and I think\nit should be renamed to something else, but I kept that in the\nfollowing attempt to rewrite:\n\n Subject: diff-lib: allow ita entries treated as \"not yet exist in index\"\n\n When comparing the index and the working tree to show which\n paths are new, and comparing the tree recorded in the HEAD and\n the index to see if committing the contents recorded in the\n index would result in an empty commit, we would want the former\n comparison to say \"these are new paths\" and the latter to say\n \"there is no change\" for paths that are marked as intent-to-add.\n\n We made a similar attempt at d95d728a (\"diff-lib.c: adjust\n position of i-t-a entries in diff\", 2015-03-16), which redefined\n the semantics of these two comparison modes globally, which was\n a disastor and had to be reverted at 78cc1a54 (\"Revert\n \"diff-lib.c: adjust position of i-t-a entries in diff\"\",\n 2015-06-23). To make sure we do not repeat the same mistake,\n introduce a new internal diffopt option so that this different\n semantics can be asked for only by callers that ask it, while\n making sure other unaudited callers will get the same comparison\n result. This internal option is also exposed temporarily as\n \"--shift-ita\" to help experiment.\n\nAfter reading the three patches through, however, I do not think we\nuse the command line option anywhere. I'm inclined to say that we\nshouldn't add it at all. Or at least do so in a separate follow-up\npatch \"now we have an internal mechanism, let's expose it anyway\" at\nthe end. Which means that the last sentence in my attempted rewrite\nshould go.\n\nThe patch to diff-lib.c machinery looks good.\n\nThanks.\n"},{"id":"302836","messageId":"xmqqh98zix1k.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"xmqqzimrj03j.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T20:33:59Z","receivedAt":"2016-09-28T20:34:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> As I already said, --shift-ita is not quite descriptive and I think\n> it should be renamed to something else, but I kept that in the\n> following attempt to rewrite:\n> ...\n\nPlease do not use that verbatim; it was full of typo and grammo.\n\n> After reading the three patches through, however, I do not think we\n> use the command line option anywhere. I'm inclined to say that we\n> shouldn't add it at all. Or at least do so in a separate follow-up\n> patch \"now we have an internal mechanism, let's expose it anyway\" at\n> the end. Which means that the last sentence in my attempted rewrite\n> should go.\n>\n> The patch to diff-lib.c machinery looks good.\n>\n> Thanks.\n"},{"id":"303080","messageId":"CACsJy8D28iq3r3O_uzjyyJT--KQunAySRgUthF3FMrb1VM6XKw@mail.gmail.com","threadId":"44180","inReplyTo":"xmqqzimrj03j.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-03T10:36:28Z","receivedAt":"2016-10-03T10:37:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 29, 2016 at 2:28 AM, Junio C Hamano wrote:\n> After reading the three patches through, however, I do not think we\n> use the command line option anywhere. I'm inclined to say that we\n> shouldn't add it at all. Or at least do so in a separate follow-up\n> patch \"now we have an internal mechanism, let's expose it anyway\" at\n> the end. Which means that the last sentence in my attempted rewrite\n> should go.\n\nWe don't use it internally _yet_. I need to go through all the\nexternal diff code and see --shift-ita should be there. The end goal\nis still changing the default behavior and getting rid of --shift-ita,\nafter making sure we don't break stuff. I do use it though because\n\"git diff\" is more often run in my workflow than \"git status\".\n\n> As I already said, --shift-ita is not quite descriptive and I think\n> it should be renamed to something else, but I kept that in the\n> following attempt to rewrite:\n\nIt's meant to be a temporary thing (which could last a year or three,\ndepending on how fast I scan through the code base) so I didn't give\nmuch thought on naming.\n\nUmm... after a couple of minutes, I still couldn't think of any\nbetter. The one-line summary of this change is \"correct the position\nof intent-to-add entries in diff\", or as you put it more precisely\n(with a bit paraphrasing), \"make ita entries not exist in index\". I\ndon't see any good way to shorten that to one or two words.\n--ita-not-in-index good enough? Or maybe --[no-]ita-visible-in-index.\n-- \nDuy\n"},{"id":"303252","messageId":"xmqqd1jgw0nx.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"CACsJy8D28iq3r3O_uzjyyJT--KQunAySRgUthF3FMrb1VM6XKw@mail.gmail.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T16:15:30Z","receivedAt":"2016-10-04T16:15:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> We don't use it internally _yet_. I need to go through all the\n> external diff code and see --shift-ita should be there. The end goal\n> is still changing the default behavior and getting rid of --shift-ita,\n\nI do not agree with that endgame, and quite honestly I do not want\nto waste time reviewing such a series.\n"},{"id":"303341","messageId":"CACsJy8D7c8Z_ugasn_scf391+C6GxJp1CYwHY4ndvVtLiJzxnQ@mail.gmail.com","threadId":"44180","inReplyTo":"xmqqd1jgw0nx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-05T09:43:13Z","receivedAt":"2016-10-05T09:43:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> We don't use it internally _yet_. I need to go through all the\n>> external diff code and see --shift-ita should be there. The end goal\n>> is still changing the default behavior and getting rid of --shift-ita,\n>\n> I do not agree with that endgame, and quite honestly I do not want\n> to waste time reviewing such a series.\n\nI do not believe current \"diff\" behavior wrt. i-t-a entries is right\neither. There's no point in pursuing this series then. Feel free to\nrevert 3f6d56d (commit: ignore intent-to-add entries instead of\nrefusing - 2012-02-07) and bring back the old i-t-a behavior. All the\nproblems would go away.\n-- \nDuy\n"},{"id":"303516","messageId":"xmqqwphljnlj.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"CACsJy8D7c8Z_ugasn_scf391+C6GxJp1CYwHY4ndvVtLiJzxnQ@mail.gmail.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-06T19:15:20Z","receivedAt":"2016-10-06T19:15:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano wrote:\n>> Duy Nguyen writes:\n>>\n>>> We don't use it internally _yet_. I need to go through all the\n>>> external diff code and see --shift-ita should be there. The end goal\n>>> is still changing the default behavior and getting rid of --shift-ita,\n>>\n>> I do not agree with that endgame, and quite honestly I do not want\n>> to waste time reviewing such a series.\n\nI definitely shouldn't have said that, especially \"waste\". Many\nissues around i-t-a and diff make my head hurt when I think about\nthem [*1*], but not wanting to spend time that gets my\nhead hurt and not wanting to waste time are totally different\nthings. My apologies.\n\nI missed something curious in your statement above, i.e. \"external\ndiff\". I thought we have pretty much got rid of all the invocation\nof \"git diff\" via the run_command() interface and we do not need the\ncommand line option (we only need the options->shift_ita so that\ncallers like \"git status\" can seletively ask for it when making\ninternal calls), and that is why I didn't want to see it.\n\n> I do not believe current \"diff\" behavior wrt. i-t-a entries is right\n> either. There's no point in pursuing this series then. Feel free to\n> revert 3f6d56d (commit: ignore intent-to-add entries instead of\n> refusing - 2012-02-07) and bring back the old i-t-a behavior. All the\n> problems would go away.\n\nIt is way too late to revert 3f6d56d. Even though I think it was\noverall a mistake to treat i-t-a as \"not exist\" while committing,\npeople are now used to the behaviour with that change, and we need\nto make our progress within the constraint of the real world.\n\n\n[Footnote]\n\nHere is one of the things around i-t-a and diff. If you make \"git\ndiff\" (between the index and the working tree) report \"new\" file, it\nwould imply that \"git apply\" run without \"--index\" should create an\nita entry in the index for symmetry, wouldn't it? That by itself\ncan be seen as an improvement (we no longer would have to say that\n\"git apply patchfile && git commit -a\" that is run in a clean state\nwill forget new files the patchfile creates), but it also means we\nnow need a repository in order to run \"git apply\" (without \"--index\"),\nwhich is a problem, as \"git apply\" is often used as a better \"patch\".\n\n\"git apply --cached\" may also become \"interesting\". A patch that\nwould apply cleanly to HEAD should apply cleanly if you did this:\n\n $ git read-tree HEAD\n $ git apply --cached wrote:\n> Duy Nguyen writes:\n>\n>> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano wrote:\n>>> Duy Nguyen writes:\n>>>\n>>>> We don't use it internally _yet_. I need to go through all the\n>>>> external diff code and see --shift-ita should be there. The end goal\n>>>> is still changing the default behavior and getting rid of --shift-ita,\n>>>\n>>> I do not agree with that endgame, and quite honestly I do not want\n>>> to waste time reviewing such a series.\n>\n> I definitely shouldn't have said that, especially \"waste\". Many\n> issues around i-t-a and diff make my head hurt when I think about\n> them [*1*], but not wanting to spend time that gets my\n> head hurt and not wanting to waste time are totally different\n> things. My apologies.\n\nNo problem. I do appreciate a straight shoot down though. Many of my\ntopics have been going on for months (ones not in 'pu') and seeing it\nrejected near the end is worse than stopping working on them early.\n\n> I missed something curious in your statement above, i.e. \"external\n> diff\". I thought we have pretty much got rid of all the invocation\n> of \"git diff\" via the run_command() interface and we do not need the\n> command line option (we only need the options->shift_ita so that\n> callers like \"git status\" can seletively ask for it when making\n> internal calls), and that is why I didn't want to see it.\n\nI don't know if we have had any external diff calls in our shell-based\ncommands. I don't read them often. Regardless, people do use \"git\ndiff\" and it should show the \"right thing\" (I know it's subjective).\nOr at least be consistent with both git-commit and git-status.\n\n> [Footnote]\n>\n> Here is one of the things around i-t-a and diff. If you make \"git\n> diff\" (between the index and the working tree) report \"new\" file, it\n> would imply that \"git apply\" run without \"--index\" should create an\n\nOff topic. This reminds me of an old patch about apply and ita [1] but\nthat one is not the same here\n\n> ita entry in the index for symmetry, wouldn't it? That by itself\n> can be seen as an improvement (we no longer would have to say that\n> \"git apply patchfile && git commit -a\" that is run in a clean state\n> will forget new files the patchfile creates), but it also means we\n> now need a repository in order to run \"git apply\" (without \"--index\"),\n> which is a problem, as \"git apply\" is often used as a better \"patch\".\n\nWe could detect \"no repo available\" and ignore the index, I guess.\n\n> \"git apply --cached\" may also become \"interesting\". A patch that\n> would apply cleanly to HEAD should apply cleanly if you did this:\n>\n> $ git read-tree HEAD\n> $ git apply --cached \n> no matter what the working tree state is. Should a patch that\n> creates a \"new\" file add contents to the index, or just an i-t-a\n> entry? I could argue it both ways, but either is quite satisfactory\n> and makes my head hurt.\n\n--cached tells you to put new contents in the index. I-ta entries,\nbeing a reminder to add stuff, don't really fit in here because you\nwant to add contents _now_, i think. After a successful \"git apply\n--cached\", a \"git commit\" should contain exactly what the applied\npatch has. If new files are i-t-a entries instead, then the new commit\nwould not be the same as the patch.\n\n[1] https://public-inbox.org/git/1451181092-26054-4-git-send-email-pclouds@gmail.com/\n-- \nDuy\n"},{"id":"303818","messageId":"xmqqh98jby5d.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"CACsJy8DiGoaKZZ1je=3L3y4odVHB7wLvvKs9pccjiN=-UeqeVw@mail.gmail.com","subject":"Re: [PATCH 1/3] Resurrect \"diff-lib.c: adjust position of i-t-a entries in diff\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-10T23:08:14Z","receivedAt":"2016-10-10T23:08:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Off topic. This reminds me of an old patch about apply and ita [1] but\n> that one is not the same here\n\n... Yeah, and re-reading that one, I think that sort-of makes\nsense. I am hesitant to take it out of context, though. I wonder\nhow it would interact with that broken mode of \"git apply\" where it\ncan take patches to the same path number of times...\n\n> [1] https://public-inbox.org/git/1451181092-26054-4-git-send-email-pclouds@gmail.com/\n\n>> ita entry in the index for symmetry, wouldn't it? That by itself\n>> can be seen as an improvement (we no longer would have to say that\n>> \"git apply patchfile && git commit -a\" that is run in a clean state\n>> will forget new files the patchfile creates), but it also means ...\n\nAnother downside is that \"git reset --hard\" after \"git apply\" will\nsuddenly start removing them. Perhaps that can be seen a feature?\nI dunno.\n\n"},{"id":"304765","messageId":"20161024104222.31128-2-pclouds@gmail.com","threadId":"44180","inReplyTo":"20161024104222.31128-1-pclouds@gmail.com","subject":"[PATCH 1/4] diff-lib: allow ita entries treated as \"not yet exist in index\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-24T10:42:19Z","receivedAt":"2016-10-24T10:42:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When comparing the index and the working tree to show which paths are\nnew, and comparing the tree recorded in the HEAD and the index to see if\ncommitting the contents recorded in the index would result in an empty\ncommit, we would want the former comparison to say \"these are new paths\"\nand the latter to say \"there is no change\" for paths that are marked as\nintent-to-add.\n\nWe made a similar attempt at d95d728a (\"diff-lib.c: adjust position of\ni-t-a entries in diff\", 2015-03-16), which redefined the semantics of\nthese two comparison modes globally, which was a disaster and had to be\nreverted at 78cc1a54 (\"Revert \"diff-lib.c: adjust position of i-t-a\nentries in diff\"\", 2015-06-23).\n\nTo make sure we do not repeat the same mistake, introduce a new internal\ndiffopt option so that this different semantics can be asked for only by\ncallers that ask it, while making sure other unaudited callers will get\nthe same comparison result.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n diff-lib.c | 14 ++++++++++++++\n diff.h | 1 +\n t/t2203-add-intent.sh | 16 +++++++++++++++-\n t/t7064-wtstatus-pv2.sh | 4 ++--\n wt-status.c | 7 ++++++-\n 5 files changed, 38 insertions(+), 4 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 3007c85..27f1228 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -214,6 +214,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\t\t !is_null_oid(&ce->oid),\n \t\t\t\t\t ce->name, 0);\n \t\t\t\tcontinue;\n+\t\t\t} else if (revs->diffopt.ita_invisible_in_index &&\n+\t\t\t\t ce_intent_to_add(ce)) {\n+\t\t\t\tdiff_addremove(&revs->diffopt, '+', ce->ce_mode,\n+\t\t\t\t\t EMPTY_BLOB_SHA1_BIN, 0,\n+\t\t\t\t\t ce->name, 0);\n+\t\t\t\tcontinue;\n \t\t\t}\n \n \t\t\tchanged = match_stat_with_submodule(&revs->diffopt, ce, &st,\n@@ -379,6 +385,14 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n \tstruct rev_info *revs = o->unpack_data;\n \tint match_missing, cached;\n \n+\t/* i-t-a entries do not actually exist in the index */\n+\tif (revs->diffopt.ita_invisible_in_index &&\n+\t idx && ce_intent_to_add(idx)) {\n+\t\tidx = NULL;\n+\t\tif (!tree)\n+\t\t\treturn;\t/* nothing to diff.. */\n+\t}\n+\n \t/* if the entry is not checked out, don't examine work tree */\n \tcached = o->index_only ||\n \t\t(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));\ndiff --git a/diff.h b/diff.h\nindex ec76a90..68a6618 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -146,6 +146,7 @@ struct diff_options {\n \tint dirstat_permille;\n \tint setup;\n \tint abbrev;\n+\tint ita_invisible_in_index;\n /* white-space error highlighting */\n #define WSEH_NEW 1\n #define WSEH_CONTEXT 2\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 8f22c43..2276e4e 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -5,10 +5,24 @@ test_description='Intent to add'\n . ./test-lib.sh\n \n test_expect_success 'intent to add' '\n+\ttest_commit 1 &&\n+\tgit rm 1.t &&\n+\techo hello >1.t &&\n \techo hello >file &&\n \techo hello >elif &&\n \tgit add -N file &&\n-\tgit add elif\n+\tgit add elif &&\n+\tgit add -N 1.t\n+'\n+\n+test_expect_success 'git status' '\n+\tgit status --porcelain | grep -v actual >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tDA 1.t\n+\tA elif\n+\t A file\n+\tEOF\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'check result of \"add -N\"' '\ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nindex 3012a4d..e319fa2 100755\n--- a/t/t7064-wtstatus-pv2.sh\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -246,8 +246,8 @@ test_expect_success 'verify --intent-to-add output' '\n \tgit add --intent-to-add intent1.add intent2.add &&\n \n \tcat >expect <<-EOF &&\n-\t1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent1.add\n-\t1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent2.add\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 intent1.add\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 intent2.add\n \tEOF\n \n \tgit status --porcelain=v2 >actual &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 9a14658..05a7dcb 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -437,7 +437,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \n \t\tswitch (p->status) {\n \t\tcase DIFF_STATUS_ADDED:\n-\t\t\tdie(\"BUG: worktree status add???\");\n+\t\t\td->mode_worktree = p->two->mode;\n \t\t\tbreak;\n \n \t\tcase DIFF_STATUS_DELETED:\n@@ -547,6 +547,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, NULL);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \tDIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);\n+\trev.diffopt.ita_invisible_in_index = 1;\n \tif (!s->show_untracked_files)\n \t\tDIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \tif (s->ignore_submodule_arg) {\n@@ -570,6 +571,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, &opt);\n \n \tDIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);\n+\trev.diffopt.ita_invisible_in_index = 1;\n \tif (s->ignore_submodule_arg) {\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n \t} else {\n@@ -605,6 +607,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)\n \n \t\tif (!ce_path_match(ce, &s->pathspec, NULL))\n \t\t\tcontinue;\n+\t\tif (ce_intent_to_add(ce))\n+\t\t\tcontinue;\n \t\tit = string_list_insert(&s->change, ce->name);\n \t\td = it->util;\n \t\tif (!d) {\n@@ -911,6 +915,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \n \tinit_revisions(&rev, NULL);\n \tDIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);\n+\trev.diffopt.ita_invisible_in_index = 1;\n \n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"304766","messageId":"20161024104222.31128-1-pclouds@gmail.com","threadId":"44180","inReplyTo":"20160928114348.1470-1-pclouds@gmail.com","subject":"[PATCH 0/4] nd/ita-empty-commit update","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-24T10:42:18Z","receivedAt":"2016-10-24T10:42:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This version splits the old 1/3 into two, with better description in\n1/4. The index_differs_from() also takes a flag to set/clear this new\nflag instead of relying on has_ita_entries like the old 2/3.\n\nThe name ita-invisible-in-index is not perfect but I could not think\nof any better. Another name could be diff-cached-ignores-ita, but\nthat's just half of what it does. The other half is diff-files-includes-ita...\n\nNguyễn Thái Ngọc Duy (4):\n Subject: diff-lib: allow ita entries treated as \"not yet exist in index\"\n diff: add --ita-[in]visible-in-index\n commit: fix empty commit creation when there's no changes but ita entries\n commit: don't be fooled by ita entries when creating initial commit\n\n Documentation/diff-options.txt | 8 ++++++++\n builtin/commit.c | 13 +++++++++----\n diff-lib.c | 18 +++++++++++++++++-\n diff.c | 4 ++++\n diff.h | 3 ++-\n sequencer.c | 4 ++--\n t/t2203-add-intent.sh | 41 +++++++++++++++++++++++++++++++++++++++--\n t/t7064-wtstatus-pv2.sh | 4 ++--\n wt-status.c | 7 ++++++-\n 9 files changed, 89 insertions(+), 13 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"304767","messageId":"20161024104222.31128-3-pclouds@gmail.com","threadId":"44180","inReplyTo":"20161024104222.31128-1-pclouds@gmail.com","subject":"[PATCH 2/4] diff: add --ita-[in]visible-in-index","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-24T10:42:20Z","receivedAt":"2016-10-24T10:42:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The option --ita-invisible-in-index exposes the \"ita_invisible_in_index\"\ndiff flag to outside to allow easier experimentation with this new mode.\nThe \"plan\" is to make --ita-invisible-in-index default to keep consistent\nbehavior with 'status' and 'commit', but a bunch other commands like\n'apply', 'merge', 'reset'.... need to be taken into consideration as well.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/diff-options.txt | 8 ++++++++\n diff.c | 4 ++++\n t/t2203-add-intent.sh | 4 +++-\n 3 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7805a0c..0fdd53f 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -575,5 +575,13 @@ endif::git-format-patch[]\n --line-prefix=::\n \tPrepend an additional prefix to every line of output.\n \n+--ita-invisible-in-index::\n+\tBy default entries added by \"git add -N\" appear as an existing\n+\tempty file in \"git diff\" and a new file in \"git diff --cached\".\n+\tThis option makes the entry appear as a new file in \"git diff\"\n+\tand non-existent in \"git diff --cached\". This option could be\n+\treverted with `--ita-visible-in-index`. Both options are\n+\texperimental and could be removed in future.\n+\n For more detailed explanation on these common options, see also\n linkgit:gitdiffcore[7].\ndiff --git a/diff.c b/diff.c\nindex c6da383..e8e73f8 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3923,6 +3923,10 @@ int diff_opt_parse(struct diff_options *options,\n \t\treturn parse_submodule_opt(options, arg);\n \telse if (skip_prefix(arg, \"--ws-error-highlight=\", &arg))\n \t\treturn parse_ws_error_highlight(options, arg);\n+\telse if (!strcmp(arg, \"--ita-invisible-in-index\"))\n+\t\toptions->ita_invisible_in_index = 1;\n+\telse if (!strcmp(arg, \"--ita-visible-in-index\"))\n+\t\toptions->ita_invisible_in_index = 0;\n \n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 2276e4e..0e54f63 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -57,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' '\n \tgit add -N nitfol &&\n \tgit commit -m second &&\n \ttest $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&\n-\ttest $(git diff --name-only HEAD -- nitfol | wc -l) = 1\n+\ttest $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&\n+\ttest $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 0 &&\n+\ttest $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1\n '\n \n test_expect_success 'can commit with an unrelated i-t-a entry in index' '\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"304768","messageId":"20161024104222.31128-4-pclouds@gmail.com","threadId":"44180","inReplyTo":"20161024104222.31128-1-pclouds@gmail.com","subject":"[PATCH 3/4] commit: fix empty commit creation when there's no changes but ita entries","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-24T10:42:21Z","receivedAt":"2016-10-24T10:42:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If i-t-a entries are present and there is no change between the index\nand HEAD i-t-a entries, index_differs_from() still returns \"dirty, new\nentries\" (aka, the resulting commit is not empty), but cache-tree will\nskip i-t-a entries and produce the exact same tree of current\ncommit.\n\nindex_differs_from() is supposed to catch this so we can abort\ngit-commit (unless --no-empty is specified). Update it to optionally\nignore i-t-a entries when doing a diff between the index and HEAD so\nthat it would return \"no change\" in this case and abort commit.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/commit.c | 2 +-\n diff-lib.c | 4 +++-\n diff.h | 2 +-\n sequencer.c | 4 ++--\n t/t2203-add-intent.sh | 11 +++++++++++\n 5 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex bb9f79b..fe8694d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -910,7 +910,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tif (ignore_submodule_arg &&\n \t\t\t !strcmp(ignore_submodule_arg, \"all\"))\n \t\t\t\tdiff_flags |= DIFF_OPT_IGNORE_SUBMODULES;\n-\t\t\tcommitable = index_differs_from(parent, diff_flags);\n+\t\t\tcommitable = index_differs_from(parent, diff_flags, 1);\n \t\t}\n \t}\n \tstrbuf_release(&committer_ident);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 27f1228..5244746 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -535,7 +535,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \treturn 0;\n }\n \n-int index_differs_from(const char *def, int diff_flags)\n+int index_differs_from(const char *def, int diff_flags,\n+\t\t int ita_invisible_in_index)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n@@ -547,6 +548,7 @@ int index_differs_from(const char *def, int diff_flags)\n \tDIFF_OPT_SET(&rev.diffopt, QUICK);\n \tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \trev.diffopt.flags |= diff_flags;\n+\trev.diffopt.ita_invisible_in_index = ita_invisible_in_index;\n \trun_diff_index(&rev, 1);\n \tif (rev.pending.alloc)\n \t\tfree(rev.pending.objects);\ndiff --git a/diff.h b/diff.h\nindex 68a6618..b171172 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -356,7 +356,7 @@ extern int diff_result_code(struct diff_options *, int);\n \n extern void diff_no_index(struct rev_info *, int, const char **);\n \n-extern int index_differs_from(const char *def, int diff_flags);\n+extern int index_differs_from(const char *def, int diff_flags, int ita_invisible_in_index);\n \n /*\n * Fill the contents of the filespec \"df\", respecting any textconv defined by\ndiff --git a/sequencer.c b/sequencer.c\nindex eec8a60..b082635 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -469,7 +469,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\tunborn = get_sha1(\"HEAD\", head);\n \t\tif (unborn)\n \t\t\thashcpy(head, EMPTY_TREE_SHA1_BIN);\n-\t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0))\n+\t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0, 0))\n \t\t\treturn error_dirty_index(opts);\n \t}\n \tdiscard_cache();\n@@ -1064,7 +1064,7 @@ static int sequencer_continue(struct replay_opts *opts)\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n-\tif (index_differs_from(\"HEAD\", 0))\n+\tif (index_differs_from(\"HEAD\", 0, 0))\n \t\treturn error_dirty_index(opts);\n \ttodo_list = todo_list->next;\n \treturn pick_commits(todo_list, opts);\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 0e54f63..8652a96 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -129,5 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '\n \t)\n '\n \n+test_expect_success 'commit: ita entries ignored in empty commit check' '\n+\tgit init empty-subsequent-commit &&\n+\t(\n+\t\tcd empty-subsequent-commit &&\n+\t\ttest_commit one &&\n+\t\t: >two &&\n+\t\tgit add -N two &&\n+\t\ttest_must_fail git commit -m nothing-new-here\n+\t)\n+'\n+\n test_done\n \n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"304769","messageId":"20161024104222.31128-5-pclouds@gmail.com","threadId":"44180","inReplyTo":"20161024104222.31128-1-pclouds@gmail.com","subject":"[PATCH 4/4] commit: don't be fooled by ita entries when creating initial commit","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-24T10:42:22Z","receivedAt":"2016-10-24T10:42:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"ita entries are dropped at tree generation phase. If the entire index\nconsists of just ita entries, the result would be a a commit with no\nentries, which should be caught unless --allow-empty is specified. The\ntest \"!!active_nr\" is not sufficient to catch this.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/commit.c | 11 ++++++++---\n t/t2203-add-intent.sh | 10 ++++++++++\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fe8694d..42732ba 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tif (amend)\n \t\t\tparent = \"HEAD^1\";\n \n-\t\tif (get_sha1(parent, sha1))\n-\t\t\tcommitable = !!active_nr;\n-\t\telse {\n+\t\tif (get_sha1(parent, sha1)) {\n+\t\t\tint i, ita_nr = 0;\n+\n+\t\t\tfor (i = 0; i < active_nr; i++)\n+\t\t\t\tif (ce_intent_to_add(active_cache[i]))\n+\t\t\t\t\tita_nr++;\n+\t\t\tcommitable = active_nr - ita_nr > 0;\n+\t\t} else {\n \t\t\t/*\n \t\t\t * Unless the user did explicitly request a submodule\n \t\t\t * ignore mode by passing a command line option we do\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 8652a96..84a9028 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -129,6 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '\n \t)\n '\n \n+test_expect_success 'commit: ita entries ignored in empty intial commit check' '\n+\tgit init empty-intial-commit &&\n+\t(\n+\t\tcd empty-intial-commit &&\n+\t\t: >one &&\n+\t\tgit add -N one &&\n+\t\ttest_must_fail git commit -m nothing-new-here\n+\t)\n+'\n+\n test_expect_success 'commit: ita entries ignored in empty commit check' '\n \tgit init empty-subsequent-commit &&\n \t(\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"304781","messageId":"xmqqeg357hou.fsf@gitster.mtv.corp.google.com","threadId":"44180","inReplyTo":"20161024104222.31128-1-pclouds@gmail.com","subject":"Re: [PATCH 0/4] nd/ita-empty-commit update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-24T17:58:09Z","receivedAt":"2016-10-24T17:58:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The index_differs_from() also takes a flag to set/clear this new\n> flag instead of relying on has_ita_entries like the old 2/3.\n\nI think that probably is a good move.\n\n> The name ita-invisible-in-index is not perfect but I could not think\n> of any better. Another name could be diff-cached-ignores-ita, but\n> that's just half of what it does. The other half is diff-files-includes-ita...\n\nI can't either, and it is one of the reasons why I am reluctant.\nNot being able to be named with a short-and-sweet name often is a\nsign that the thing to be named is conceptually not well thought\nout.\n\nBut as we need to give it some name to the flat to ease\nexperimenting, let's take that name as-is.\n"},{"id":"304823","messageId":"CACsJy8DJqomLNB=BXSfKn4s5pmtsZffrjDJe8JspYx1MiDNsZA@mail.gmail.com","threadId":"44180","inReplyTo":"xmqqeg357hou.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/4] nd/ita-empty-commit update","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-25T09:34:50Z","receivedAt":"2016-10-25T09:35:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 25, 2016 at 12:58 AM, Junio C Hamano wrote:\n>> The name ita-invisible-in-index is not perfect but I could not think\n>> of any better. Another name could be diff-cached-ignores-ita, but\n>> that's just half of what it does. The other half is diff-files-includes-ita...\n>\n> I can't either, and it is one of the reasons why I am reluctant.\n> Not being able to be named with a short-and-sweet name often is a\n> sign that the thing to be named is conceptually not well thought\n> out.\n\nIt's implementation detail leak, and probably why naming it for\n\"normal\" people is so hard. Whatever the name is must somehow imply\n\"so these i-t-a markers actually live in the index and considered real\nindex entries, associated to empty blob, most of the time...\"\n\n> But as we need to give it some name to the flat to ease\n> experimenting, let's take that name as-is.\n-- \nDuy\n"}]}