From 425a28e0a4edfc39585cec6b0b6368c0ad9dbf7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2016 17:42:19 +0700 Subject: [PATCH 1/4] diff-lib: allow ita entries treated as "not yet exist in index" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When comparing the index and the working tree to show which paths are new, and comparing the tree recorded in the HEAD and the index to see if committing the contents recorded in the index would result in an empty commit, we would want the former comparison to say "these are new paths" and the latter to say "there is no change" for paths that are marked as intent-to-add. We made a similar attempt at d95d728a ("diff-lib.c: adjust position of i-t-a entries in diff", 2015-03-16), which redefined the semantics of these two comparison modes globally, which was a disaster and had to be reverted at 78cc1a54 ("Revert "diff-lib.c: adjust position of i-t-a entries in diff"", 2015-06-23). To make sure we do not repeat the same mistake, introduce a new internal diffopt option so that this different semantics can be asked for only by callers that ask it, while making sure other unaudited callers will get the same comparison result. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff-lib.c | 14 ++++++++++++++ diff.h | 1 + t/t2203-add-intent.sh | 16 +++++++++++++++- t/t7064-wtstatus-pv2.sh | 4 ++-- wt-status.c | 7 ++++++- 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 3007c8524c..27f12282b3 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -214,6 +214,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option) !is_null_oid(&ce->oid), ce->name, 0); continue; + } else if (revs->diffopt.ita_invisible_in_index && + ce_intent_to_add(ce)) { + diff_addremove(&revs->diffopt, '+', ce->ce_mode, + EMPTY_BLOB_SHA1_BIN, 0, + ce->name, 0); + continue; } changed = match_stat_with_submodule(&revs->diffopt, ce, &st, @@ -379,6 +385,14 @@ static void do_oneway_diff(struct unpack_trees_options *o, struct rev_info *revs = o->unpack_data; int match_missing, cached; + /* i-t-a entries do not actually exist in the index */ + if (revs->diffopt.ita_invisible_in_index && + idx && ce_intent_to_add(idx)) { + idx = NULL; + if (!tree) + return; /* nothing to diff.. */ + } + /* if the entry is not checked out, don't examine work tree */ cached = o->index_only || (idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx))); diff --git a/diff.h b/diff.h index 25ae60d5ff..ce3535e9ad 100644 --- a/diff.h +++ b/diff.h @@ -146,6 +146,7 @@ struct diff_options { int dirstat_permille; int setup; int abbrev; + int ita_invisible_in_index; /* white-space error highlighting */ #define WSEH_NEW 1 #define WSEH_CONTEXT 2 diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 8f22c43e24..2276e4e6b6 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -5,10 +5,24 @@ test_description='Intent to add' . ./test-lib.sh test_expect_success 'intent to add' ' + test_commit 1 && + git rm 1.t && + echo hello >1.t && echo hello >file && echo hello >elif && git add -N file && - git add elif + git add elif && + git add -N 1.t +' + +test_expect_success 'git status' ' + git status --porcelain | grep -v actual >actual && + cat >expect <<-\EOF && + DA 1.t + A elif + A file + EOF + test_cmp expect actual ' test_expect_success 'check result of "add -N"' ' diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh index 3012a4d7c0..e319fa2e84 100755 --- a/t/t7064-wtstatus-pv2.sh +++ b/t/t7064-wtstatus-pv2.sh @@ -246,8 +246,8 @@ test_expect_success 'verify --intent-to-add output' ' git add --intent-to-add intent1.add intent2.add && cat >expect <<-EOF && - 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent1.add - 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent2.add + 1 .A N... 000000 000000 100644 $_z40 $_z40 intent1.add + 1 .A N... 000000 000000 100644 $_z40 $_z40 intent2.add EOF git status --porcelain=v2 >actual && diff --git a/wt-status.c b/wt-status.c index 9628c1d5d7..5b0965ef27 100644 --- a/wt-status.c +++ b/wt-status.c @@ -437,7 +437,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q, switch (p->status) { case DIFF_STATUS_ADDED: - die("BUG: worktree status add???"); + d->mode_worktree = p->two->mode; break; case DIFF_STATUS_DELETED: @@ -547,6 +547,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s) setup_revisions(0, NULL, &rev, NULL); rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK; DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES); + rev.diffopt.ita_invisible_in_index = 1; if (!s->show_untracked_files) DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES); if (s->ignore_submodule_arg) { @@ -570,6 +571,7 @@ static void wt_status_collect_changes_index(struct wt_status *s) setup_revisions(0, NULL, &rev, &opt); DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG); + rev.diffopt.ita_invisible_in_index = 1; if (s->ignore_submodule_arg) { handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg); } else { @@ -605,6 +607,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s) if (!ce_path_match(ce, &s->pathspec, NULL)) continue; + if (ce_intent_to_add(ce)) + continue; it = string_list_insert(&s->change, ce->name); d = it->util; if (!d) { @@ -911,6 +915,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s) init_revisions(&rev, NULL); DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV); + rev.diffopt.ita_invisible_in_index = 1; memset(&opt, 0, sizeof(opt)); opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference; From b42b45191950a4ac39f6f5ae042c15ad114da79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2016 17:42:20 +0700 Subject: [PATCH 2/4] diff: add --ita-[in]visible-in-index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The option --ita-invisible-in-index exposes the "ita_invisible_in_index" diff flag to outside to allow easier experimentation with this new mode. The "plan" is to make --ita-invisible-in-index default to keep consistent behavior with 'status' and 'commit', but a bunch other commands like 'apply', 'merge', 'reset'.... need to be taken into consideration as well. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/diff-options.txt | 8 ++++++++ diff.c | 4 ++++ t/t2203-add-intent.sh | 4 +++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 2d77a19626..b249f6a2e7 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -570,5 +570,13 @@ endif::git-format-patch[] --line-prefix=:: Prepend an additional prefix to every line of output. +--ita-invisible-in-index:: + By default entries added by "git add -N" appear as an existing + empty file in "git diff" and a new file in "git diff --cached". + This option makes the entry appear as a new file in "git diff" + and non-existent in "git diff --cached". This option could be + reverted with `--ita-visible-in-index`. Both options are + experimental and could be removed in future. + For more detailed explanation on these common options, see also linkgit:gitdiffcore[7]. diff --git a/diff.c b/diff.c index a178ed39bc..297e0340e9 100644 --- a/diff.c +++ b/diff.c @@ -3951,6 +3951,10 @@ int diff_opt_parse(struct diff_options *options, return parse_submodule_opt(options, arg); else if (skip_prefix(arg, "--ws-error-highlight=", &arg)) return parse_ws_error_highlight(options, arg); + else if (!strcmp(arg, "--ita-invisible-in-index")) + options->ita_invisible_in_index = 1; + else if (!strcmp(arg, "--ita-visible-in-index")) + options->ita_invisible_in_index = 0; /* misc options */ else if (!strcmp(arg, "-z")) diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 2276e4e6b6..0e54f63cf7 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -57,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' ' git add -N nitfol && git commit -m second && test $(git ls-tree HEAD -- nitfol | wc -l) = 0 && - test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 + test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 && + test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 0 && + test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1 ' test_expect_success 'can commit with an unrelated i-t-a entry in index' ' From 018ec3c8203ad1aee683840a580cfba7914419b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2016 17:42:21 +0700 Subject: [PATCH 3/4] commit: fix empty commit creation when there's no changes but ita entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If i-t-a entries are present and there is no change between the index and HEAD i-t-a entries, index_differs_from() still returns "dirty, new entries" (aka, the resulting commit is not empty), but cache-tree will skip i-t-a entries and produce the exact same tree of current commit. index_differs_from() is supposed to catch this so we can abort git-commit (unless --no-empty is specified). Update it to optionally ignore i-t-a entries when doing a diff between the index and HEAD so that it would return "no change" in this case and abort commit. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- diff-lib.c | 4 +++- diff.h | 2 +- sequencer.c | 4 ++-- t/t2203-add-intent.sh | 11 +++++++++++ 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1cba3b75c8..400d2a54e6 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -910,7 +910,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (ignore_submodule_arg && !strcmp(ignore_submodule_arg, "all")) diff_flags |= DIFF_OPT_IGNORE_SUBMODULES; - commitable = index_differs_from(parent, diff_flags); + commitable = index_differs_from(parent, diff_flags, 1); } } strbuf_release(&committer_ident); diff --git a/diff-lib.c b/diff-lib.c index 27f12282b3..52447466b5 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -535,7 +535,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt) return 0; } -int index_differs_from(const char *def, int diff_flags) +int index_differs_from(const char *def, int diff_flags, + int ita_invisible_in_index) { struct rev_info rev; struct setup_revision_opt opt; @@ -547,6 +548,7 @@ int index_differs_from(const char *def, int diff_flags) DIFF_OPT_SET(&rev.diffopt, QUICK); DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); rev.diffopt.flags |= diff_flags; + rev.diffopt.ita_invisible_in_index = ita_invisible_in_index; run_diff_index(&rev, 1); if (rev.pending.alloc) free(rev.pending.objects); diff --git a/diff.h b/diff.h index ce3535e9ad..94422e1e93 100644 --- a/diff.h +++ b/diff.h @@ -357,7 +357,7 @@ extern int diff_result_code(struct diff_options *, int); extern void diff_no_index(struct rev_info *, int, const char **); -extern int index_differs_from(const char *def, int diff_flags); +extern int index_differs_from(const char *def, int diff_flags, int ita_invisible_in_index); /* * Fill the contents of the filespec "df", respecting any textconv defined by diff --git a/sequencer.c b/sequencer.c index eec8a60d6b..b0826353e3 100644 --- a/sequencer.c +++ b/sequencer.c @@ -469,7 +469,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) unborn = get_sha1("HEAD", head); if (unborn) hashcpy(head, EMPTY_TREE_SHA1_BIN); - if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0)) + if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0, 0)) return error_dirty_index(opts); } discard_cache(); @@ -1064,7 +1064,7 @@ static int sequencer_continue(struct replay_opts *opts) if (ret) return ret; } - if (index_differs_from("HEAD", 0)) + if (index_differs_from("HEAD", 0, 0)) return error_dirty_index(opts); todo_list = todo_list->next; return pick_commits(todo_list, opts); diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 0e54f63cf7..8652a96d86 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -129,5 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' ' ) ' +test_expect_success 'commit: ita entries ignored in empty commit check' ' + git init empty-subsequent-commit && + ( + cd empty-subsequent-commit && + test_commit one && + : >two && + git add -N two && + test_must_fail git commit -m nothing-new-here + ) +' + test_done From 2c49f7ffb3063fdccccf2a038ab2eee66e7395e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2016 17:42:22 +0700 Subject: [PATCH 4/4] commit: don't be fooled by ita entries when creating initial commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ita entries are dropped at tree generation phase. If the entire index consists of just ita entries, the result would be a a commit with no entries, which should be caught unless --allow-empty is specified. The test "!!active_nr" is not sufficient to catch this. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/commit.c | 11 ++++++++--- t/t2203-add-intent.sh | 10 ++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 400d2a54e6..1e74bc19f6 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (amend) parent = "HEAD^1"; - if (get_sha1(parent, sha1)) - commitable = !!active_nr; - else { + if (get_sha1(parent, sha1)) { + int i, ita_nr = 0; + + for (i = 0; i < active_nr; i++) + if (ce_intent_to_add(active_cache[i])) + ita_nr++; + commitable = active_nr - ita_nr > 0; + } else { /* * Unless the user did explicitly request a submodule * ignore mode by passing a command line option we do diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 8652a96d86..84a9028c43 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -129,6 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' ' ) ' +test_expect_success 'commit: ita entries ignored in empty intial commit check' ' + git init empty-intial-commit && + ( + cd empty-intial-commit && + : >one && + git add -N one && + test_must_fail git commit -m nothing-new-here + ) +' + test_expect_success 'commit: ita entries ignored in empty commit check' ' git init empty-subsequent-commit && (