{"thread":{"id":"29340","subject":"[PATCH RFC] commit: allow to commit even if there are intent-to-add entries","startedAt":"2012-01-11T06:01:47Z","lastAt":"2012-01-12T03:05:48Z","messageCount":12,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Jonathan Nieder","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"182320","messageId":"1326261707-11484-1-git-send-email-pclouds@gmail.com","threadId":"29340","inReplyTo":null,"subject":"[PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-11T06:01:47Z","receivedAt":"2012-01-11T06:01:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This patch replaces the approach in 331fcb5 (git add --intent-to-add:\ndo not let an empty blob be committed by accident) regarding i-t-a\nentries: instead of forbidding i-t-a entries at commit time, we can\nsimply ignore them.\n\nWe already ignore CE_REMOVE entries while updating cache-tree. Putting\nCE_INTENT_TO_ADD ones in the same category should not cause any negative\neffects regarding cache-tree.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n On the few chances I have to use \"git add -N\" it does not fit well\n with \"git add -p; git diff --cached; git commit -m foo\" style. I\n think this may be a good thing to do.\n\n builtin/commit.c      |    2 +-\n builtin/write-tree.c  |    2 +-\n cache-tree.c          |   14 +++++---------\n t/t2203-add-intent.sh |   10 +++++++++-\n 4 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eba1377..767b78a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -871,7 +871,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tdiscard_cache();\n \tread_cache_from(index_file);\n \tif (update_main_cache_tree(0)) {\n-\t\terror(_(\"Error building trees\"));\n+\t\terror(_(\"Error building trees; the index is unmerged?\"));\n \t\treturn 0;\n \t}\n \ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex b223af4..68baa24 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -46,7 +46,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n \t\tdie(\"%s: error reading the index\", me);\n \t\tbreak;\n \tcase WRITE_TREE_UNMERGED_INDEX:\n-\t\tdie(\"%s: error building trees\", me);\n+\t\tdie(\"%s: error building trees; the index is unmerged?\", me);\n \t\tbreak;\n \tcase WRITE_TREE_PREFIX_ERROR:\n \t\tdie(\"%s: prefix %s not found\", me, prefix);\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 8de3959..47defd1 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -158,19 +158,15 @@ static int verify_cache(struct cache_entry **cache,\n \tfunny = 0;\n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n-\t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\n+\t\tif (ce_stage(ce)) {\n \t\t\tif (silent)\n \t\t\t\treturn -1;\n \t\t\tif (10 < ++funny) {\n \t\t\t\tfprintf(stderr, \"...\\n\");\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tif (ce_stage(ce))\n-\t\t\t\tfprintf(stderr, \"%s: unmerged (%s)\\n\",\n-\t\t\t\t\tce->name, sha1_to_hex(ce->sha1));\n-\t\t\telse\n-\t\t\t\tfprintf(stderr, \"%s: not added yet\\n\",\n-\t\t\t\t\tce->name);\n+\t\t\tfprintf(stderr, \"%s: unmerged (%s)\\n\",\n+\t\t\t\tce->name, sha1_to_hex(ce->sha1));\n \t\t}\n \t}\n \tif (funny)\n@@ -338,8 +334,8 @@ static int update_one(struct cache_tree *it,\n \t\t\t\tmode, sha1_to_hex(sha1), entlen+baselen, path);\n \t\t}\n \n-\t\tif (ce->ce_flags & CE_REMOVE)\n-\t\t\tcontinue; /* entry being removed */\n+\t\tif (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))\n+\t\t\tcontinue; /* entry being removed or just placeholder */\n \n \t\tstrbuf_grow(&buffer, entlen + 100);\n \t\tstrbuf_addf(&buffer, \"%o %.*s%c\", mode, entlen, path + baselen, '\\0');\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 2543529..65430e4 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -41,7 +41,15 @@ test_expect_success 'cannot commit with i-t-a entry' '\n \techo frotz >nitfol &&\n \tgit add rezrov &&\n \tgit add -N nitfol &&\n-\ttest_must_fail git commit -m initial\n+\tgit commit -m initial &&\n+\tgit ls-tree -r HEAD >actual &&\n+\tcat >expected <<EOF &&\n+100644 blob ce013625030ba8dba906f756967f9e9ca394464a\telif\n+100644 blob ce013625030ba8dba906f756967f9e9ca394464a\tfile\n+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4\trezrov\n+EOF\n+\ttest_cmp expected actual &&\n+\tgit reset HEAD^\n '\n \n test_expect_success 'can commit with an unrelated i-t-a entry in index' '\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"182326","messageId":"7vr4z67jbb.fsf@alter.siamese.dyndns.org","threadId":"29340","inReplyTo":"1326261707-11484-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T08:08:24Z","receivedAt":"2012-01-11T08:08:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This patch replaces the approach in 331fcb5 (git add --intent-to-add:\n> do not let an empty blob be committed by accident) regarding i-t-a\n> entries: instead of forbidding i-t-a entries at commit time, we can\n> simply ignore them.\n\nI have a mild suspicion that in earlier incarnation of the patch we used\nto let empty blobs committed, and then we used to instead not commit\nanything at all for such a path, and the real users were bitten by either\nof these approaches, forgetting to add the contents to the final commit.\n\nSo I am not sure if this is such a good idea.\n"},{"id":"182336","messageId":"1326275982-29866-1-git-send-email-pclouds@gmail.com","threadId":"29340","inReplyTo":"1326261707-11484-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-11T09:59:40Z","receivedAt":"2012-01-11T09:59:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2012/1/11 Junio C Hamano <gitster@pobox.com>:\n> I have a mild suspicion that in earlier incarnation of the patch we used\n> to let empty blobs committed, and then we used to instead not commit\n> anything at all for such a path, and the real users were bitten by either\n> of these approaches, forgetting to add the contents to the final commit.\n>\n> So I am not sure if this is such a good idea.\n\nI found your elaborate writing [1] about it. These are the\ninterpretations listed in that post:\n\n-- 8< --\nWhen running \"commit\" and \"status\" with files marked with \"intent to add\",\nI think there are three possible interpretations of what the user wants to\ndo.\n\n (1) I earlier said \"I'll decide the exact contents to be committed for\n     these paths and tell you by running 'git add' later.\" when I said\n     'git add -N'.  But I forgot to do so before running \"git commit\".\n     Thanks for catching this mistake and erroring out.\n\n (2) I said \"I'll decide the exact content to be committed ... later.\"\n     when I said 'git add -N'. I am running \"git commit\" now, but I still\n     don't know what the final contents for this path should be.  I\n     changed my mind, and I do not want to include the addition of these\n     paths in the commit I am making.  Please do not error out, but just\n     ignore the earlier 'add -N' for now.\n\n (3) I said \"I'll decide the exact content to be committed ... later.\"\n     when I said 'git add -N'. I am running \"git commit\" now, without\n     explicitly telling you with 'git add' about the final contents for\n     these paths.  Please take it as an implicit hint that I am happy with\n     the contents in the working tree and commit them as if I said 'git\n     add' on these paths, but do leave modifications to already tracked\n     paths that I haven't added with 'git add'.\n-- 8< --\n\nSo (1) may be the safe and sane interpretation and should be the\ndefault. But perhaps we should allow (2) also, for example with\n--skip-intent-to-add option? It's really frustrating to remove all\ni-t-a, commit (I don't do \"commit <path>\"), then add them back.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/170658\n\nNguyễn Thái Ngọc Duy (2):\n  cache-tree: update API to take abitrary flags\n  commit: add --skip-intent-to-add to allow commit with i-t-a entries\n    in index\n\n builtin/commit.c       |   10 +++++++---\n cache-tree.c           |   35 +++++++++++++++++------------------\n cache-tree.h           |    5 ++++-\n merge-recursive.c      |    2 +-\n t/t2203-add-intent.sh  |   17 +++++++++++++++++\n test-dump-cache-tree.c |    2 +-\n 6 files changed, 47 insertions(+), 24 deletions(-)\n\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"182337","messageId":"1326275982-29866-2-git-send-email-pclouds@gmail.com","threadId":"29340","inReplyTo":"1326261707-11484-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/2] cache-tree: update API to take abitrary flags","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-11T09:59:41Z","receivedAt":"2012-01-11T09:59:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"---\n builtin/commit.c       |    4 ++--\n cache-tree.c           |   27 ++++++++++++---------------\n cache-tree.h           |    4 +++-\n merge-recursive.c      |    2 +-\n test-dump-cache-tree.c |    2 +-\n 5 files changed, 19 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eba1377..bf42bb3 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n-\t\tupdate_main_cache_tree(1);\n+\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(_(\"unable to write new_index file\"));\n@@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tif (active_cache_changed) {\n-\t\t\tupdate_main_cache_tree(1);\n+\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n \t\t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\t    commit_locked_index(&index_lock))\n \t\t\t\tdie(_(\"unable to write new_index file\"));\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 8de3959..16355d6 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)\n }\n \n static int verify_cache(struct cache_entry **cache,\n-\t\t\tint entries, int silent)\n+\t\t\tint entries, int flags)\n {\n \tint i, funny;\n+\tint silent = flags & WRITE_TREE_SILENT;\n \n \t/* Verify that the tree is merged */\n \tfunny = 0;\n@@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it,\n \t\t      int entries,\n \t\t      const char *base,\n \t\t      int baselen,\n-\t\t      int missing_ok,\n-\t\t      int dryrun)\n+\t\t      int flags)\n {\n \tstruct strbuf buffer;\n+\tint missing_ok = flags & WRITE_TREE_MISSING_OK;\n+\tint dryrun = flags & WRITE_TREE_DRY_RUN;\n \tint i;\n \n \tif (0 <= it->entry_count && has_sha1_file(it->sha1))\n@@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it,\n \t\t\t\t    cache + i, entries - i,\n \t\t\t\t    path,\n \t\t\t\t    baselen + sublen + 1,\n-\t\t\t\t    missing_ok,\n-\t\t\t\t    dryrun);\n+\t\t\t\t    flags);\n \t\tif (subcnt < 0)\n \t\t\treturn subcnt;\n \t\ti += subcnt - 1;\n@@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it,\n int cache_tree_update(struct cache_tree *it,\n \t\t      struct cache_entry **cache,\n \t\t      int entries,\n-\t\t      int missing_ok,\n-\t\t      int dryrun,\n-\t\t      int silent)\n+\t\t      int flags)\n {\n \tint i;\n-\ti = verify_cache(cache, entries, silent);\n+\ti = verify_cache(cache, entries, flags);\n \tif (i)\n \t\treturn i;\n-\ti = update_one(it, cache, entries, \"\", 0, missing_ok, dryrun);\n+\ti = update_one(it, cache, entries, \"\", 0, flags);\n \tif (i < 0)\n \t\treturn i;\n \treturn 0;\n@@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)\n \n \twas_valid = cache_tree_fully_valid(active_cache_tree);\n \tif (!was_valid) {\n-\t\tint missing_ok = flags & WRITE_TREE_MISSING_OK;\n-\n \t\tif (cache_tree_update(active_cache_tree,\n \t\t\t\t      active_cache, active_nr,\n-\t\t\t\t      missing_ok, 0, 0) < 0)\n+\t\t\t\t      flags) < 0)\n \t\t\treturn WRITE_TREE_UNMERGED_INDEX;\n \t\tif (0 <= newfd) {\n \t\t\tif (!write_cache(newfd, active_cache, active_nr) &&\n@@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root,\n \treturn 0;\n }\n \n-int update_main_cache_tree (int silent)\n+int update_main_cache_tree (int flags)\n {\n \tif (!the_index.cache_tree)\n \t\tthe_index.cache_tree = cache_tree();\n \treturn cache_tree_update(the_index.cache_tree,\n-\t\t\t\t the_index.cache, the_index.cache_nr, 0, 0, silent);\n+\t\t\t\t the_index.cache, the_index.cache_nr, flags);\n }\ndiff --git a/cache-tree.h b/cache-tree.h\nindex 0ec0b2a..d8cb2e9 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root);\n struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);\n \n int cache_tree_fully_valid(struct cache_tree *);\n-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);\n+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int);\n \n int update_main_cache_tree(int);\n \n /* bitmasks to write_cache_as_tree flags */\n #define WRITE_TREE_MISSING_OK 1\n #define WRITE_TREE_IGNORE_CACHE_TREE 2\n+#define WRITE_TREE_DRY_RUN 4\n+#define WRITE_TREE_SILENT 8\n \n /* error return codes */\n #define WRITE_TREE_UNREADABLE_INDEX (-1)\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d83cd6c..6479a60 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n \n \tif (!cache_tree_fully_valid(active_cache_tree) &&\n \t    cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0, 0) < 0)\n+\t\t\t      active_cache, active_nr, 0) < 0)\n \t\tdie(\"error building trees\");\n \n \tresult = lookup_tree(active_cache_tree->sha1);\ndiff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c\nindex e6c2923..a6ffdf3 100644\n--- a/test-dump-cache-tree.c\n+++ b/test-dump-cache-tree.c\n@@ -59,6 +59,6 @@ int main(int ac, char **av)\n \tstruct cache_tree *another = cache_tree();\n \tif (read_cache() < 0)\n \t\tdie(\"unable to read index file\");\n-\tcache_tree_update(another, active_cache, active_nr, 0, 1, 0);\n+\tcache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN);\n \treturn dump_cache_tree(active_cache_tree, another, \"\");\n }\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"182338","messageId":"1326275982-29866-3-git-send-email-pclouds@gmail.com","threadId":"29340","inReplyTo":"1326261707-11484-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] commit: add --skip-intent-to-add to allow commit with i-t-a entries in index","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-11T09:59:42Z","receivedAt":"2012-01-11T09:59:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"---\n builtin/commit.c      |   10 +++++++---\n cache-tree.c          |    8 +++++---\n cache-tree.h          |    1 +\n t/t2203-add-intent.sh |   17 +++++++++++++++++\n 4 files changed, 30 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex bf42bb3..021206e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int no_post_rewrite, allow_empty_message;\n+static int cache_tree_flags, skip_intent_to_add;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg;\n static char *sign_commit;\n \n@@ -170,6 +171,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"no-post-rewrite\", &no_post_rewrite, \"bypass post-rewrite hook\"),\n \t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, \"mode\", \"show untracked files, optional modes: all, normal, no. (Default: all)\", PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n+\tOPT_BOOL(0, \"skip-intent-to-add\", &skip_intent_to_add, \"allow intent-to-add entries in index\"),\n \t/* end commit contents options */\n \n \t{ OPTION_BOOLEAN, 0, \"allow-empty\", &allow_empty, NULL,\n@@ -400,7 +402,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n-\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n+\t\tupdate_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(_(\"unable to write new_index file\"));\n@@ -421,7 +423,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tif (active_cache_changed) {\n-\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n+\t\t\tupdate_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);\n \t\t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\t    commit_locked_index(&index_lock))\n \t\t\t\tdie(_(\"unable to write new_index file\"));\n@@ -870,7 +872,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t */\n \tdiscard_cache();\n \tread_cache_from(index_file);\n-\tif (update_main_cache_tree(0)) {\n+\tif (update_main_cache_tree(cache_tree_flags)) {\n \t\terror(_(\"Error building trees\"));\n \t\treturn 0;\n \t}\n@@ -1088,6 +1090,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tcleanup_mode = CLEANUP_ALL;\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n+\tif (skip_intent_to_add)\n+\t\tcache_tree_flags = WRITE_TREE_INTENT_TO_ADD_OK;\n \n \thandle_untracked_files_arg(s);\n \ndiff --git a/cache-tree.c b/cache-tree.c\nindex 16355d6..b8593e0 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -159,7 +159,9 @@ static int verify_cache(struct cache_entry **cache,\n \tfunny = 0;\n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n-\t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\n+\t\tif (ce_stage(ce) ||\n+\t\t    ((flags & WRITE_TREE_INTENT_TO_ADD_OK) == 0 &&\n+\t\t     (ce->ce_flags & CE_INTENT_TO_ADD))) {\n \t\t\tif (silent)\n \t\t\t\treturn -1;\n \t\t\tif (10 < ++funny) {\n@@ -339,8 +341,8 @@ static int update_one(struct cache_tree *it,\n \t\t\t\tmode, sha1_to_hex(sha1), entlen+baselen, path);\n \t\t}\n \n-\t\tif (ce->ce_flags & CE_REMOVE)\n-\t\t\tcontinue; /* entry being removed */\n+\t\tif (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))\n+\t\t\tcontinue; /* entry being removed or placeholder */\n \n \t\tstrbuf_grow(&buffer, entlen + 100);\n \t\tstrbuf_addf(&buffer, \"%o %.*s%c\", mode, entlen, path + baselen, '\\0');\ndiff --git a/cache-tree.h b/cache-tree.h\nindex d8cb2e9..865733c 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -38,6 +38,7 @@ int update_main_cache_tree(int);\n #define WRITE_TREE_IGNORE_CACHE_TREE 2\n #define WRITE_TREE_DRY_RUN 4\n #define WRITE_TREE_SILENT 8\n+#define WRITE_TREE_INTENT_TO_ADD_OK 16\n \n /* error return codes */\n #define WRITE_TREE_UNREADABLE_INDEX (-1)\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 2543529..990c765 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -44,6 +44,23 @@ test_expect_success 'cannot commit with i-t-a entry' '\n \ttest_must_fail git commit -m initial\n '\n \n+test_expect_success 'can commit with i-t-a entry' '\n+\tgit reset --hard &&\n+\techo xyzzy >rezrov &&\n+\techo frotz >nitfol &&\n+\tgit add rezrov &&\n+\tgit add -N nitfol &&\n+\tgit commit --skip-intent-to-add -m initial &&\n+\tgit ls-tree -r HEAD >actual &&\n+\tcat >expected <<EOF &&\n+100644 blob ce013625030ba8dba906f756967f9e9ca394464a\telif\n+100644 blob ce013625030ba8dba906f756967f9e9ca394464a\tfile\n+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4\trezrov\n+EOF\n+\ttest_cmp expected actual &&\n+\tgit reset HEAD^\n+'\n+\n test_expect_success 'can commit with an unrelated i-t-a entry in index' '\n \tgit reset --hard &&\n \techo xyzzy >rezrov &&\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"182340","messageId":"20120111110222.GA32173@burratino","threadId":"29340","inReplyTo":"7vr4z67jbb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-11T11:02:22Z","receivedAt":"2012-01-11T11:02:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> I have a mild suspicion that in earlier incarnation of the patch we used\n> to let empty blobs committed, and then we used to instead not commit\n> anything at all for such a path, and the real users were bitten by either\n> of these approaches, forgetting to add the contents to the final commit.\n\nI remember the empty blob era. :)  I don't think I ever saw something\nlike this patch, though, and a quick search finds that the first\niteration of the bugfix to stop commiting empty blobs was the one that\nwas used:\n\n http://thread.gmane.org/gmane.comp.version-control.git/101881/focus=101894\n\nI suspect that at the time, introducing an intent-to-add flag (which\nwas always the right thing to do) provided enough momentum to avoid\nany worries about smaller details like whether to error out or skip\nthose entries on commit, which could always be changed later (today).\n\n> So I am not sure if this is such a good idea.\n\nMy first reaction was the same, but on reflection, I think this is a\ngood idea as long as the \"git status\" output in the editor says\nsomething appropriate.\n\nThe response Duy mentioned[1] to a report about the unenlightening\nmessage \"error building trees\" was also memorable:\n\n> When running \"commit\" and \"status\" with files marked with \"intent to add\",\n> I think there are three possible interpretations of what the user\n> wants to do.\n[ (1) thanks for stopping me, I had forgotten about that file;\n  (2) I changed my mind, please leave that file out; or (3) please\n  dwim and add the file ]\n\nI think (3) was a trick --- no one that does not use the \"-a\" option\nwould want that. :)\n\nAt the time, I did not understand what (2) meant.  Now I see why ---\nin interpretation (2), the user did not change her mind at all.  She\nsaid \"I will be adding this file at some point, so please keep track\nof it along with the others for the sake of commands like 'git diff'\nand 'git add -u', but that does not mean \"I will be adding this file\nat some point _before the next commit_\".\n\nSo at the time I thought (1) was the only sensible behavior but kept\nmy mouth shut; and now I see that (1) and (2) both fit into reasonable\nworkflows.\n\nHowever.  A person using \"git diff\" to review remaining changes and\n\"git add\" to mark files once they have reached their final form would\nbenefit even more from a switch for \"git commit\" to error out if _any_\nfiles in the worktree do not match the index.  So if we are to take\nthis workflow seriously, treating \"git add -N\" as a special case is\nnot helping.  What we currently provide for this workflow is a\nreminder in the status area of changes that were not marked with \"git\nadd\".\n\nI suspect no longer erroring out might feel eerie for a period for\npeople that were relying on \"git add -N\" as a reminder but that as\nlong as the \"git status\" output is sensible enough, Duy's proposed\nbehavior would end up seeming just as natural.\n\n(2) makes intent-to-add entries just like any other tracked index\nentry with some un-added content.  It is conceptually pleasant and\nfits well in all workflows I can imagine[2].\n\nHope that helps, and sorry for the ramble,\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/170651/focus=170658\n[2] Ok, that is a small lie.  In \"git stash\", a commit is used to save\nthe state of the index, so the user would want the presence of the\nintent-to-add entry to be stored somehow in the commit, and none of\n(1), (2), or (3) will make her happy.  Using \"git commit\" this way is\nnot going to work when there are unmerged entries, for example,\neither, so I think it is okay to ignore that problem here.\n"},{"id":"182381","messageId":"7vr4z654m3.fsf@alter.siamese.dyndns.org","threadId":"29340","inReplyTo":"20120111110222.GA32173@burratino","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T21:08:52Z","receivedAt":"2012-01-11T21:08:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> When running \"commit\" and \"status\" with files marked with \"intent to add\",\n>> I think there are three possible interpretations of what the user\n>> wants to do.\n> [ (1) thanks for stopping me, I had forgotten about that file;\n>   (2) I changed my mind, please leave that file out; or (3) please\n>   dwim and add the file ]\n>\n> I think (3) was a trick --- no one that does not use the \"-a\" option\n> would want that. :)\n\nI really wish it were the case, but I doubt it.\n\nPeople from other VCS background seem to still think that \"commit\" without\npaths should commit everything; after getting told that \"what you add to\nthe index is what you will commit\", I can easily see this commint: but but\nbut I told Git that I _want_ to add with -N! Why aren't you committing it?\n\n> At the time, I did not understand what (2) meant.  Now I see why ---\n> in interpretation (2), the user did not change her mind at all.\n\nYou are correct. \"I still cannot make up my mind\" is what is happening in\nthat situation.\n\nThe user explicitly said \"I cannot decide about this path right now\" when\nshe said \"add -N\". And we haven't heard from the user what should happen\nto the path. Now we have to make a commit so somebody needs to decide.\n\n> She\n> said \"I will be adding this file at some point, so please keep track\n> of it along with the others for the sake of commands like 'git diff'\n> and 'git add -u', but that does not mean \"I will be adding this file\n> at some point _before the next commit_\".\n\nCorrect. She only said \"I cannot decide right now\" when she said \"add -N\"\nand hasn't gave us any more hint as to what should happen now we have to\nmake a commit.\n\nIt is _wrong_ for us to unilaterally decide for the user that she does not\nwant the path in the commit. The last we heard from her was that she does\nnot know what should happen to the path.\n\n> (2) makes intent-to-add entries just like any other tracked index\n> entry with some un-added content.\n\nYou are comparing files edited in the working tree without the user\ntelling anything about them to Git (both tracked and untracked) and files\nthe user explicitly told Git that the user hasn't made up her mind\nabout. Why is it a good thing to make the latter behave \"just like any\nother\"?\n"},{"id":"182391","messageId":"7vmx9t6bs5.fsf@alter.siamese.dyndns.org","threadId":"29340","inReplyTo":"1326275982-29866-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] cache-tree: update API to take abitrary flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T23:48:42Z","receivedAt":"2012-01-11T23:48:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> ---\n\nThanks; this one looks very sensible regardless of what follows (or does\nnot follow).  Forgot to sign-off?\n"},{"id":"182392","messageId":"7vipkh6bg8.fsf@alter.siamese.dyndns.org","threadId":"29340","inReplyTo":"1326275982-29866-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/2] commit: add --skip-intent-to-add to allow commit with i-t-a entries in index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T23:55:51Z","receivedAt":"2012-01-11T23:55:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> ---\n>  builtin/commit.c      |   10 +++++++---\n>  cache-tree.c          |    8 +++++---\n>  cache-tree.h          |    1 +\n>  t/t2203-add-intent.sh |   17 +++++++++++++++++\n>  4 files changed, 30 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index bf42bb3..021206e 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n>  static int edit_flag = -1; /* unspecified */\n>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n>  static int no_post_rewrite, allow_empty_message;\n> +static int cache_tree_flags, skip_intent_to_add;\n>  static char *untracked_files_arg, *force_date, *ignore_submodule_arg;\n>  static char *sign_commit;\n>  \n> @@ -170,6 +171,7 @@ static struct option builtin_commit_options[] = {\n>  \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n>  \tOPT_BOOLEAN(0, \"no-post-rewrite\", &no_post_rewrite, \"bypass post-rewrite hook\"),\n>  \t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, \"mode\", \"show untracked files, optional modes: all, normal, no. (Default: all)\", PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n> +\tOPT_BOOL(0, \"skip-intent-to-add\", &skip_intent_to_add, \"allow intent-to-add entries in index\"),\n\nThis is more like \"ignore\", not \"allow\", from end user's point of view,\nno? The user earlier said \"I cannot decide what contents to put in the\ncommit yet for this path\", and normally we catch it and remind the user\nthat she needs to decide. This option gives her a quick way to say \"I\ndecide that I do not want to add this path at all to this commit I am\ncreating, so please ignore it in the meantime.\"\n\n> @@ -1088,6 +1090,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>  \t\tcleanup_mode = CLEANUP_ALL;\n>  \telse\n>  \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> +\tif (skip_intent_to_add)\n> +\t\tcache_tree_flags = WRITE_TREE_INTENT_TO_ADD_OK;\n\nThe name WRITE_TREE_INTENT_TO_ADD_OK says \"it is OK to call write-tree\nwith i-t-a entries in the index, please do not barf\", but I think \"when\nwriting a tree, ignore i-t-a entries\" would be a more appropriate way to\nsay the same thing, i.e. WRITE_TREE_IGNORE_INTENT_TO_ADD.\n\nOther than that, I do not see an issue in the implementation of the\npatch. It is a separate design level issue if we want to worsen\nproliferation of the options, though.\n\nThanks.\n"},{"id":"182396","messageId":"CACsJy8CqkCjTeUAyx7h0+PWXCPhRxDG1D5UX886dv=w-M=RHFQ@mail.gmail.com","threadId":"29340","inReplyTo":"7vmx9t6bs5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] cache-tree: update API to take abitrary flags","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-12T01:20:29Z","receivedAt":"2012-01-12T01:20:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2012/1/12 Junio C Hamano <gitster@pobox.com>:\n> Thanks; this one looks very sensible regardless of what follows (or does\n> not follow).  Forgot to sign-off?\n\nDeliberately to stop you from using it because I did not test it\ncarefully. It was created as material for the discussion only. Will\nresubmit later.\n-- \nDuy\n"},{"id":"182404","messageId":"CACsJy8CHOuZMP4hWp6Lb_TbdfwSgofSg_0tDZ4oDcD0veie2Dw@mail.gmail.com","threadId":"29340","inReplyTo":"7vr4z654m3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-12T02:53:22Z","receivedAt":"2012-01-12T02:53:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2012/1/12 Junio C Hamano <gitster@pobox.com>:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>>> When running \"commit\" and \"status\" with files marked with \"intent to add\",\n>>> I think there are three possible interpretations of what the user\n>>> wants to do.\n>> [ (1) thanks for stopping me, I had forgotten about that file;\n>>   (2) I changed my mind, please leave that file out; or (3) please\n>>   dwim and add the file ]\n>>\n>> I think (3) was a trick --- no one that does not use the \"-a\" option\n>> would want that. :)\n>\n> I really wish it were the case, but I doubt it.\n>\n> People from other VCS background seem to still think that \"commit\" without\n> paths should commit everything; after getting told that \"what you add to\n> the index is what you will commit\", I can easily see this commint: but but\n> but I told Git that I _want_ to add with -N! Why aren't you committing it?\n\nI see \"-N\" just as an indication, not really an \"add\" operation.\nPerhaps update-index is a better place for it.\n\n>> (2) makes intent-to-add entries just like any other tracked index\n>> entry with some un-added content.\n>\n> You are comparing files edited in the working tree without the user\n> telling anything about them to Git (both tracked and untracked) and files\n> the user explicitly told Git that the user hasn't made up her mind\n> about. Why is it a good thing to make the latter behave \"just like any\n> other\"?\n\nThe way I see this flag is \"include these files in my diff in addition\nto tracked files\", and therefore should not have any effects at commit\ntime. I might turn some of those extra files to tracked some time if I\nwant to commit.\n-- \nDuy\n"},{"id":"182410","messageId":"7vaa5t4o37.fsf@alter.siamese.dyndns.org","threadId":"29340","inReplyTo":"CACsJy8CHOuZMP4hWp6Lb_TbdfwSgofSg_0tDZ4oDcD0veie2Dw@mail.gmail.com","subject":"Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-12T03:05:48Z","receivedAt":"2012-01-12T03:05:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n>> I really wish it were the case, but I doubt it.\n>>\n>> People from other VCS background seem to still think that \"commit\" without\n>> paths should commit everything; after getting told that \"what you add to\n>> the index is what you will commit\", I can easily see this commint: but but\n>> but I told Git that I _want_ to add with -N! Why aren't you committing it?\n>\n> I see \"-N\" just as an indication, not really an \"add\" operation.\n\nBut the thing is, what _you_, who knows a bit more than these new people\nabout Git, see does not matter, because they would not listen to you.\n"}]}