{"thread":{"id":"16487","subject":"What's cooking in git.git (Nov 2008, #06; Wed, 26)","startedAt":"2008-11-27T00:28:35Z","lastAt":"2008-12-13T05:51:57Z","messageCount":37,"participants":["Junio C Hamano","Johannes Schindelin","Shawn O. Pearce","Daniel Barkalow","Nguyen Thai Ngoc Duy","Sverre Rabbelier","Jeff King","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"96594","messageId":"7v7i6qc8r0.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":null,"subject":"What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-27T00:28:35Z","receivedAt":"2008-11-27T00:28:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking. Commits prefixed\nwith '-' are only in 'pu' while commits prefixed with '+' are\nin 'next'.\n\nThe topics list the commits in reverse chronological order. The topics\nmeant to be merged to the maintenance series have \"maint-\" in their names.\n\n----------------------------------------------------------------\n[New Topics]\n\n* cr/remote-update-v (Tue Nov 18 19:04:02 2008 +0800) 1 commit\n + git-remote: add verbose mode to git remote update\n\nShould be in 1.6.1-rc1.\n\n* rs/strbuf-expand (Sun Nov 23 00:16:59 2008 +0100) 6 commits\n + remove the unused files interpolate.c and interpolate.h\n + daemon: deglobalize variable 'directory'\n + daemon: inline fill_in_extra_table_entries()\n + daemon: use strbuf_expand() instead of interpolate()\n + merge-recursive: use strbuf_expand() instead of interpolate()\n + add strbuf_expand_dict_cb(), a helper for simple cases\n\nShould be in 1.6.1-rc1.\n\n* mv/fast-export (Sun Nov 23 12:55:54 2008 +0100) 2 commits\n + fast-export: use an unsorted string list for extra_refs\n + Add new testcase to show fast-export does not always exports all\n tags\n\nShould be in 1.6.1-rc1 and backmerged to 'maint'.\n\n* st/levenshtein (Thu Nov 20 14:27:27 2008 +0100) 2 commits\n + Document levenshtein.c\n + Fix deletion of last character in levenshtein distance\n\nShould be in 1.6.1-rc1.\n\n* js/mingw-rename-fix (Wed Nov 19 17:25:27 2008 +0100) 1 commit\n + compat/mingw.c: Teach mingw_rename() to replace read-only files\n\nShould be in 1.6.1-rc1 and backmerged to 'maint'.\n\n* mv/clone-strbuf (Fri Nov 21 01:45:01 2008 +0100) 3 commits\n + builtin_clone: use strbuf in cmd_clone()\n + builtin-clone: use strbuf in clone_local() and\n copy_or_link_directory()\n + builtin-clone: use strbuf in guess_dir_name()\n\nShould be in 1.6.1-rc1.\n\n* pw/maint-p4 (Wed Nov 26 13:52:15 2008 -0500) 1 commit\n - git-p4: fix keyword-expansion regex\n\nWaiting for Ack from git-p4 folks.\n\n* cc/bisect-skip (Sun Nov 23 22:02:49 2008 +0100) 1 commit\n - bisect: teach \"skip\" to accept special arguments like \"A..B\"\n\nShould be in 1.6.1-rc1.\n\n* cc/bisect-replace (Mon Nov 24 22:20:30 2008 +0100) 9 commits\n - bisect: add \"--no-replace\" option to bisect without using replace\n refs\n - rev-list: make it possible to disable replacing using \"--no-\n bisect-replace\"\n - bisect: use \"--bisect-replace\" options when checking merge bases\n - merge-base: add \"--bisect-replace\" option to use fixed up revs\n - commit: add \"bisect_replace_all\" prototype to \"commit.h\"\n - rev-list: add \"--bisect-replace\" to list revisions with fixed up\n history\n - Documentation: add \"git bisect replace\" documentation\n - bisect: add test cases for \"git bisect replace\"\n - bisect: add \"git bisect replace\" subcommand\n\nI really hate the idea of introducing a potentially much more useful\nreplacement of the existing graft mechanism and tie it very tightly to\nbisect, making it unusable from outside.\n\n (1) I do not think \"bisect replace\" workflow is a practical and usable\n one;\n\n (2) The underlying mechanism to express \"this object replaces that other\n object\" is much easier to work with than what the graft does which is\n \"the parents of this commit are these\", and idea to use the normal\n ref to point at them means this can potentially be used for\n transferring the graft information across repositories, which the\n current graft mechanism cannot do.\n\n (3) Because I like the aspect (2) of this series so much, it deeply\n disappoints and troubles me that this is implemented minimally near\n the surface, and that it is controlled by the \"bisect\" Porcelain\n alone, by explicitly passing command line arguments.\n\nI think a mechanism like this should be added to replace grafts, but it\nshould always be enabled for normal revision traversal operation, while\nalways disabled for object enumeration and transfer operation (iow, fsck,\nfetch and push should use the real ancestry information recorded in the\nunderlying objects, while rev-list, log, etc. should always use the\nreplaced objects). I have a suspicion that even cat-file could honor it.\n\n----------------------------------------------------------------\n[Graduated to \"master\"]\n\n* bc/maint-keep-pack (Thu Nov 13 14:11:46 2008 -0600) 1 commit\n + repack: only unpack-unreachable if we are deleting redundant packs\n\nThis makes \"repack -A -d\" without -d do the same thing as \"repack -a -d\",\nwhich makes sense. This does not have to go to 'maint', though.\n\n* jk/commit-v-strip (Wed Nov 12 03:23:37 2008 -0500) 4 commits\n + status: show \"-v\" diff even for initial commit\n + Merge branch 'jk/maint-commit-v-strip' into jk/commit-v-strip\n + wt-status: refactor initial commit printing\n + define empty tree sha1 as a macro\n\n----------------------------------------------------------------\n[Will merge to \"master\" soon]\n\n* lt/preload-lstat (Mon Nov 17 09:01:20 2008 -0800) 2 commits\n + Fix index preloading for racy dirty case\n + Add cache preload facility\n\n* ta/quiet-pull (Mon Nov 17 23:09:30 2008 +0100) 2 commits\n + Retain multiple -q/-v occurrences in git pull\n + Teach/Fix pull/fetch -q/-v options\n\n* nd/narrow (Tue Nov 18 06:33:16 2008 -0500) 10 commits\n + t2104: touch portability fix\n + grep: skip files outside sparse checkout area\n + checkout_entry(): CE_NO_CHECKOUT on checked out entries.\n + Prevent diff machinery from examining worktree outside sparse\n checkout\n + ls-files: Add tests for --sparse and friends\n + update-index: add --checkout/--no-checkout to update\n CE_NO_CHECKOUT bit\n + update-index: refactor mark_valid() in preparation for new options\n + ls-files: add options to support sparse checkout\n + Introduce CE_NO_CHECKOUT bit\n + Extend index to save more flags\n\n* ph/send-email (Tue Nov 11 00:54:02 2008 +0100) 4 commits\n + git send-email: ask less questions when --compose is used.\n + git send-email: add --annotate option\n + git send-email: interpret unknown files as revision lists\n + git send-email: make the message file name more specific.\n\n----------------------------------------------------------------\n[Actively Cooking]\n\n* cb/mergetool (Thu Nov 13 12:41:15 2008 +0000) 3 commits\n - [DONTMERGE] Add -k/--keep-going option to mergetool\n - Add -y/--no-prompt option to mergetool\n - Fix some tab/space inconsistencies in git-mergetool.sh\n\nJeff had good comments on the last one; the discussion needs concluded,\nand also waiting for comments from the original author (Ted).\n\n* ds/uintmax-config (Mon Nov 3 09:14:28 2008 -0900) 1 commit\n - autoconf: Enable threaded delta search when pthreads are supported\n\n* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits\n + blame: show \"previous\" information in --porcelain/--incremental\n format\n + git-blame: refactor code to emit \"porcelain format\" output\n\n----------------------------------------------------------------\n[On Hold]\n\n* jc/send-pack-tell-me-more (Thu Mar 20 00:44:11 2008 -0700) 1 commit\n - \"git push\": tellme-more protocol extension\n\nThis seems to have a deadlock during communication between the peers.\nSomeone needs to pick up this topic and resolve the deadlock before it can\ncontinue.\n\n* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit\n - diff: enable \"too large a rename\" warning when -M/-C is explicitly\n asked for\n\nThis would be the right thing to do for command line use,\nbut gitk will be hit due to tcl/tk's limitation, so I am holding\nthis back for now.\n\n* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits\n - git-am --forge: add Signed-off-by: line for the author\n - git-am: clean-up Signed-off-by: lines\n - stripspace: add --log-clean option to clean up signed-off-by:\n lines\n - stripspace: use parse_options()\n - Add \"git am -s\" test\n - git-am: refactor code to add signed-off-by line for the committer\n"},{"id":"96653","messageId":"alpine.DEB.1.00.0811272347010.30769@pacific.mpi-cbg.de","threadId":"16487","inReplyTo":"7v7i6qc8r0.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-27T22:49:40Z","receivedAt":"2008-11-27T22:49:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 26 Nov 2008, Junio C Hamano wrote:\n\n> ----------------------------------------------------------------\n> [Will merge to \"master\" soon]\n>\n> [...]\n> \n> * nd/narrow (Tue Nov 18 06:33:16 2008 -0500) 10 commits\n> + t2104: touch portability fix\n> + grep: skip files outside sparse checkout area\n> + checkout_entry(): CE_NO_CHECKOUT on checked out entries.\n> + Prevent diff machinery from examining worktree outside sparse\n> checkout\n> + ls-files: Add tests for --sparse and friends\n> + update-index: add --checkout/--no-checkout to update\n> CE_NO_CHECKOUT bit\n> + update-index: refactor mark_valid() in preparation for new options\n> + ls-files: add options to support sparse checkout\n> + Introduce CE_NO_CHECKOUT bit\n> + Extend index to save more flags\n\nI have a strong suspicion that the narrow stuff will make the worktree \nmess pale in comparison.\n\nNote that I do not have time to review this myself (which is not helped at \nall by it being no longer a trivial single patch, but a full 10 patches!), \nbut I really have a bad feeling about this. IMO it is substantially \nunder-reviewed.\n\nCiao,\nDscho\n"},{"id":"96667","messageId":"7vtz9s8uzu.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"alpine.DEB.1.00.0811272347010.30769@pacific.mpi-cbg.de","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-28T02:06:13Z","receivedAt":"2008-11-28T02:06:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> I have a strong suspicion that the narrow stuff will make the worktree \n> mess pale in comparison.\n>\n> Note that I do not have time to review this myself (which is not helped at \n> all by it being no longer a trivial single patch, but a full 10 patches!), \n> but I really have a bad feeling about this. IMO it is substantially \n> under-reviewed.\n\nWell, \"a bad feeling\" is not a convincing enough argument either, is it?\nWhat kind of bad interaction are you fearing?\n\nI thought the changes this first half of the topic implements were safe\nfor people who do not use this feature at all (which is the most important\nthing I care about very first), and also I thought they made sense.\n\nA bigger concern I actually have about this series is that the original\nauthor seems to have gone quiet. I would have expected a discussion on\nadding Porcelain level support after the series hit 'next' to begin, so\nthat the underlying feature can be made more accessible by the end users.\nAlso a new section to tutorial or a new addition to how-to series of\ndocuments that describe how to work inside narrowly checked out work tree,\nwhat the pitfalls are, etc., together with follow-up improvements of what\nis already in 'next', and end user questions and reports on issues should\nhave come, if this is ever being used by anybody by now, but none of that\nhas happened.\n"},{"id":"96681","messageId":"alpine.DEB.1.00.0811281225040.30769@pacific.mpi-cbg.de","threadId":"16487","inReplyTo":"7vtz9s8uzu.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-28T11:47:01Z","receivedAt":"2008-11-28T11:47:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 27 Nov 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > I have a strong suspicion that the narrow stuff will make the worktree \n> > mess pale in comparison.\n> >\n> > Note that I do not have time to review this myself (which is not \n> > helped at all by it being no longer a trivial single patch, but a full \n> > 10 patches!), but I really have a bad feeling about this. IMO it is \n> > substantially under-reviewed.\n> \n> Well, \"a bad feeling\" is not a convincing enough argument either, is it? \n> What kind of bad interaction are you fearing?\n\nI just remember the worktree stuff well enough. I had a bad gut feeling \nwhen it was proposed, and I had a bad impression of the (IMO way too \nintrusive) patch series implementing it. It was pretty buggy and affected \nGit in serious ways (in order to accomodate worktree, we broke operations \nin bare repositories at least once, for example).\n\n(So no, \"bad feeling\" is not convincing, but it is basically a primitive \npattern matching in experiences that have not been fully analyzed, but \nthat turned out to be bad enough.)\n\nI tried to fix it, but did not a very good job at it. In the meantime, I \nthink I know why: there is no elegant way to implement this that is \nperformant at the same time. (Just think of having git_dir be relative: \nthis is a necessity for the performance, but ugly to implement in the \npresence of worktree where it may _need_ to be absolute).\n\nTo me, the narrow patch series has all the looks of becoming the same type \nof nightmare:\n\n- it is intrusive,\n\n- it consists of a substantial number of patches (making bugs the opposite \n of shallow),\n\n- it is heavily under-reviewed,\n\n- it _needs_ a lot of changes to be accomodated, affecting common code \n paths, having all the potential to break existing workflows, and\n\n- there is as little interest in the feature from core Git developers as \n with worktree, literally guaranteeing that it will not, or only very \n slowly, and probably badly, get fixed if it breaks.\n\nAnd the worst part: I think that as with worktree, there has not been \nenough of kicking forth and back ideas how to design the beast, so I fully \nexpect a subtle breakage that would require a redesign (which will be \npainful, with existing users of the feature).\n\nMaybe I am crying \"wolf\", but I _do_ want to caution against risking too \nmuch, too fast, with that feature.\n\nIn other words, unless there is more interest in that feature, enough to \ngenerate a well-understood design before a good implementation, I'd rather \nsee this patch series dropped.\n\nCiao,\nDscho\n"},{"id":"96691","messageId":"20081128192033.GF23984@spearce.org","threadId":"16487","inReplyTo":"alpine.DEB.1.00.0811281225040.30769@pacific.mpi-cbg.de","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-28T19:20:33Z","receivedAt":"2008-11-28T19:20:33Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Thu, 27 Nov 2008, Junio C Hamano wrote:\n> > Johannes Schindelin writes:\n> > \n> > > I have a strong suspicion that the narrow stuff will make the worktree \n> > > mess pale in comparison.\n> > >\n> > > Note that I do not have time to review this myself (which is not \n> > > helped at all by it being no longer a trivial single patch, but a full \n> > > 10 patches!), but I really have a bad feeling about this. IMO it is \n> > > substantially under-reviewed.\n> > \n> > Well, \"a bad feeling\" is not a convincing enough argument either, is it? \n> > What kind of bad interaction are you fearing?\n...\n> And the worst part: I think that as with worktree, there has not been \n> enough of kicking forth and back ideas how to design the beast, so I fully \n> expect a subtle breakage that would require a redesign (which will be \n> painful, with existing users of the feature).\n> \n> Maybe I am crying \"wolf\", but I _do_ want to caution against risking too \n> much, too fast, with that feature.\n> \n> In other words, unless there is more interest in that feature, enough to \n> generate a well-understood design before a good implementation, I'd rather \n> see this patch series dropped.\n\nAck. I agree with every remark made by Dscho, and also want to cry \"wolf\".\n\nI haven't had time to read the patch series. Its big and intrusive\nand I just don't need the feature.\n\nBut I feel like if it were in fact merged I'll fall over some bug\nin it sometime soon and be forced to stop and debug it. Heck at\nthe least I'll have to go back to JGit's index code and implement\nthe new file format. That shouldn't cause git.git's development to\nstop, but I am whining (a little) about the file format change. ;-)\n\n-- \nShawn.\n"},{"id":"96703","messageId":"7voczz4cfb.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"20081128192033.GF23984@spearce.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-29T00:13:12Z","receivedAt":"2008-11-29T00:13:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Johannes Schindelin wrote:\n> ...\n>> In other words, unless there is more interest in that feature, enough to \n>> generate a well-understood design before a good implementation, I'd rather \n>> see this patch series dropped.\n>\n> Ack. I agree with every remark made by Dscho, and also want to cry \"wolf\".\n>\n> I haven't had time to read the patch series. Its big and intrusive\n> and I just don't need the feature.\n\nWell, \"me neither\". Although I personally think resisting changes until\nit becomes absolutely necessary is a good discipline, we also need to\nrecognise that there is a chicken-and-egg problem. When you have a\npotentially useful feature, unless people actually try using it in the\nfield, you won't discover the drawbacks in either the design nor the\nimplementation, let alone any improvements.\n\n> But I feel like if it were in fact merged I'll fall over some bug\n> in it sometime soon and be forced to stop and debug it.\n\nExactly. That is how you make progress.\n\nHaving said that, I am willing to carry it over in 'next' outside 'master'\nfor the 1.6.1 cycle, as three people who are most likely to be able to fix\nany potential issues are not using that feature.\n\n> Heck at\n> the least I'll have to go back to JGit's index code and implement\n> the new file format.\n\nI am sorry to dissapoint you but I am planning to use the first one in the\nseries, which is the one that adds extended index flag bits, for the fix\nto an unrelated feature.\n"},{"id":"96704","messageId":"7vk5an4cba.fsf_-_@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"7voczz4cfb.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git add --intent-to-add: fix removal of cached emptiness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-29T00:15:37Z","receivedAt":"2008-11-29T00:15:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This uses the extended index flag mechanism introduced earlier to mark\nthe entries added to the index via \"git add -N\" with CE_INTENT_TO_ADD.\n\nThe logic to detect an \"intent to add\" entry for the purpose of allowing\n\"git rm --cached $path\" is tightened to check not just for a staged empty\nblob, but with the CE_INTENT_TO_ADD bit. This protects an empty blob that\nwas explicitly added and then modified in the work tree from being dropped\nwith this sequence:\n\n\t$ >empty\n\t$ git add empty\n\t$ echo \"non empty\" >empty\n\t$ git rm --cached empty\n\nAn index an \"intent to add\" entry is blocked. This implies that you\ncannot \"git commit\" from such a state; however \"git commit -a\" still\nworks.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This applies on top of the result of merging commit 06aaaa0 (Extend\n index to save more flags, 2008-10-01) from nd/narrow topic into 'master'.\n\n builtin-rm.c | 11 ++++++-----\n builtin-write-tree.c | 2 +-\n cache-tree.c | 10 +++++++---\n cache.h | 3 ++-\n read-cache.c | 2 ++\n t/t3600-rm.sh | 4 ++--\n t/t3701-add-interactive.sh | 18 ++++++++++++++++++\n 7 files changed, 38 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex b7126e3..8debcec 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -73,14 +73,15 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\t}\n \t\tif (ce_match_stat(ce, &st, 0))\n \t\t\tlocal_changes = 1;\n-\t\tif (no_head\n-\t\t || get_tree_entry(head, name, sha1, &mode)\n-\t\t || ce->ce_mode != create_ce_mode(mode)\n-\t\t || hashcmp(ce->sha1, sha1))\n+\t\tif (no_head ||\n+\t\t ((get_tree_entry(head, name, sha1, &mode)\n+\t\t || ce->ce_mode != create_ce_mode(mode)\n+\t\t || hashcmp(ce->sha1, sha1)) &&\n+\t\t !(ce->ce_flags & CE_INTENT_TO_ADD)))\n \t\t\tstaged_changes = 1;\n \n \t\tif (local_changes && staged_changes &&\n-\t\t !(index_only && is_empty_blob_sha1(ce->sha1)))\n+\t\t !(index_only && (ce->ce_flags & CE_INTENT_TO_ADD)))\n \t\t\terrs = error(\"'%s' has staged content different \"\n \t\t\t\t \"from both the file and the HEAD\\n\"\n \t\t\t\t \"(use -f to force removal)\", name);\ndiff --git a/builtin-write-tree.c b/builtin-write-tree.c\nindex 52a3c01..9d64050 100644\n--- a/builtin-write-tree.c\n+++ b/builtin-write-tree.c\n@@ -42,7 +42,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; the index is unmerged?\", me);\n+\t\tdie(\"%s: error building trees\", 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 5f8ee87..3d8f218 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -155,13 +155,17 @@ 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)) {\n+\t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\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\tfprintf(stderr, \"%s: unmerged (%s)\\n\",\n-\t\t\t\tce->name, sha1_to_hex(ce->sha1));\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}\n \t}\n \tif (funny)\ndiff --git a/cache.h b/cache.h\nindex ef2e7f9..f15b3fc 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -176,10 +176,11 @@ struct cache_entry {\n /*\n * Extended on-disk flags\n */\n+#define CE_INTENT_TO_ADD 0x20000000\n /* CE_EXTENDED2 is for future extension */\n #define CE_EXTENDED2 0x80000000\n \n-#define CE_EXTENDED_FLAGS (0)\n+#define CE_EXTENDED_FLAGS (CE_INTENT_TO_ADD)\n \n /*\n * Safeguard to avoid saving wrong flags:\ndiff --git a/read-cache.c b/read-cache.c\nindex abc627b..fa30a0f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -546,6 +546,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tce->ce_flags = namelen;\n \tif (!intent_only)\n \t\tfill_stat_cache_info(ce, st);\n+\telse\n+\t\tce->ce_flags |= CE_INTENT_TO_ADD;\n \n \tif (trust_executable_bit && has_symlinks)\n \t\tce->ce_mode = create_ce_mode(st_mode);\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 5b4d6f7..b7d46e5 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -187,8 +187,8 @@ test_expect_success 'but with -f it should work.' '\n \ttest_must_fail git ls-files --error-unmatch baz\n '\n \n-test_expect_failure 'refuse to remove cached empty file with modifications' '\n-\ttouch empty &&\n+test_expect_success 'refuse to remove cached empty file with modifications' '\n+\t>empty &&\n \tgit add empty &&\n \techo content >empty &&\n \ttest_must_fail git rm --cached empty\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex e95663d..473ef85 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -133,6 +133,24 @@ test_expect_success 'real edit works' '\n \ttest_cmp expected output\n '\n \n+test_expect_success 'cannot 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+\ttest_must_fail git commit\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+\techo frotz >nitfol &&\n+\tgit add rezrov &&\n+\tgit add -N nitfol &&\n+\tgit commit -m partial rezrov\n+'\n+\n if test \"$(git config --bool core.filemode)\" = false\n then\n say 'skipping filemode tests (filesystem does not properly support modes)'\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96706","messageId":"alpine.LNX.1.00.0811281938250.19665@iabervon.org","threadId":"16487","inReplyTo":"7voczz4cfb.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-11-29T01:25:15Z","receivedAt":"2008-11-29T01:25:15Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 28 Nov 2008, Junio C Hamano wrote:\n\n> \"Shawn O. Pearce\" writes:\n> \n> > Johannes Schindelin wrote:\n> > ...\n> >> In other words, unless there is more interest in that feature, enough to \n> >> generate a well-understood design before a good implementation, I'd rather \n> >> see this patch series dropped.\n> >\n> > Ack. I agree with every remark made by Dscho, and also want to cry \"wolf\".\n> >\n> > I haven't had time to read the patch series. Its big and intrusive\n> > and I just don't need the feature.\n> \n> Well, \"me neither\". Although I personally think resisting changes until\n> it becomes absolutely necessary is a good discipline, we also need to\n> recognise that there is a chicken-and-egg problem. When you have a\n> potentially useful feature, unless people actually try using it in the\n> field, you won't discover the drawbacks in either the design nor the\n> implementation, let alone any improvements.\n\nI just looked over most of it (skipping the generic index extension \nportion). It looks to me like it's introducing an extra concept to avoid \nactually fixing maybe-bugs in the \"assume unchanged\" implementation \nwhen used with files that have been changed intentionally (with the user \nintending git to overlook this change). Sparse checkout is essentially a \nspecial case of this, where the user has changed the working directory \nradically (not populating it at all) and wants git to carry on as if this \nwas not the case (with a certain amount of porcelain code to cause this to \nhappen automatically).\n\nIf there's any need for this to be distinguished from \"assume unchanged\", \nI think it should be used with, not instead of, the CE_VALID bit; and it \ncould probably use some bit in the stat info section, since we don't need \nstat info if we know by assumption that the entry is valid.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"96707","messageId":"7vvdu72nq9.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"7vk5an4cba.fsf_-_@gitster.siamese.dyndns.org","subject":"[PATCH 1/3] builtin-rm.c: explain and clarify the \"local change\" logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-29T03:51:58Z","receivedAt":"2008-11-29T03:51:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the logic to check local modification a bit more in the comment,\nespecially because the existing comment that talks about \"git rm --cached\"\nwas placed in a part that was not about \"--cached\" at all.\n\nAlso clarify \"if .. else if ..\" structure.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is the first patch of a re-rolled sesries of the previous one, and\n again applies to the master plus the first commit from nd/narrow.\n\n builtin-rm.c | 53 ++++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 42 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex b7126e3..3d03da0 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -31,7 +31,8 @@ static void add_list(const char *name)\n \n static int check_local_mod(unsigned char *head, int index_only)\n {\n-\t/* items in list are already sorted in the cache order,\n+\t/*\n+\t * Items in list are already sorted in the cache order,\n \t * so we could do this a lot more efficiently by using\n \t * tree_desc based traversal if we wanted to, but I am\n \t * lazy, and who cares if removal of files is a tad\n@@ -71,25 +72,55 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\t\t */\n \t\t\tcontinue;\n \t\t}\n+\n+\t\t/*\n+\t\t * \"rm\" of a path that has changes need to be treated\n+\t\t * carefully not to allow losing local changes\n+\t\t * accidentally. A local change could be (1) file in\n+\t\t * work tree is different since the index; and/or (2)\n+\t\t * the user staged a content that is different from\n+\t\t * the current commit in the index.\n+\t\t *\n+\t\t * In such a case, you would need to --force the\n+\t\t * removal. However, \"rm --cached\" (remove only from\n+\t\t * the index) is safe if the index matches the file in\n+\t\t * the work tree or the HEAD commit, as it means that\n+\t\t * the content being removed is available elsewhere.\n+\t\t */\n+\n+\t\t/*\n+\t\t * Is the index different from the file in the work tree?\n+\t\t */\n \t\tif (ce_match_stat(ce, &st, 0))\n \t\t\tlocal_changes = 1;\n+\n+\t\t/*\n+\t\t * Is the index different from the HEAD commit? By\n+\t\t * definition, before the very initial commit,\n+\t\t * anything staged in the index is treated by the same\n+\t\t * way as changed from the HEAD.\n+\t\t */\n \t\tif (no_head\n \t\t || get_tree_entry(head, name, sha1, &mode)\n \t\t || ce->ce_mode != create_ce_mode(mode)\n \t\t || hashcmp(ce->sha1, sha1))\n \t\t\tstaged_changes = 1;\n \n-\t\tif (local_changes && staged_changes &&\n-\t\t !(index_only && is_empty_blob_sha1(ce->sha1)))\n-\t\t\terrs = error(\"'%s' has staged content different \"\n-\t\t\t\t \"from both the file and the HEAD\\n\"\n-\t\t\t\t \"(use -f to force removal)\", name);\n+\t\t/*\n+\t\t * If the index does not match the file in the work\n+\t\t * tree and if it does not match the HEAD commit\n+\t\t * either, (1) \"git rm\" without --cached definitely\n+\t\t * will lose information; (2) \"git rm --cached\" will\n+\t\t * lose information unless it is about removing an\n+\t\t * \"intent to add\" entry.\n+\t\t */\n+\t\tif (local_changes && staged_changes) {\n+\t\t\tif (!index_only || !is_empty_blob_sha1(ce->sha1))\n+\t\t\t\terrs = error(\"'%s' has staged content different \"\n+\t\t\t\t\t \"from both the file and the HEAD\\n\"\n+\t\t\t\t\t \"(use -f to force removal)\", name);\n+\t\t}\n \t\telse if (!index_only) {\n-\t\t\t/* It's not dangerous to \"git rm --cached\" a\n-\t\t\t * file if the index matches the file or the\n-\t\t\t * HEAD, since it means the deleted content is\n-\t\t\t * still available somewhere.\n-\t\t\t */\n \t\t\tif (staged_changes)\n \t\t\t\terrs = error(\"'%s' has changes staged in the index\\n\"\n \t\t\t\t\t \"(use --cached to keep the file, \"\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96708","messageId":"7vprkf2nki.fsf_-_@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"7vvdu72nq9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-29T03:55:25Z","receivedAt":"2008-11-29T03:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This uses the extended index flag mechanism introduced earlier to mark\nthe entries added to the index via \"git add -N\" with CE_INTENT_TO_ADD.\n\nThe logic to detect an \"intent to add\" entry for the purpose of allowing\n\"git rm --cached $path\" is tightened to check not just for a staged empty\nblob, but with the CE_INTENT_TO_ADD bit. This protects an empty blob that\nwas explicitly added and then modified in the work tree from being dropped\nwith this sequence:\n\n\t$ >empty\n\t$ git add empty\n\t$ echo \"non empty\" >empty\n\t$ git rm --cached empty\n\nSigned-off-by: Junio C Hamano \n---\n builtin-rm.c | 2 +-\n cache.h | 3 ++-\n read-cache.c | 2 ++\n t/t3600-rm.sh | 4 ++--\n 4 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 3d03da0..c11f455 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -115,7 +115,7 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\t * \"intent to add\" entry.\n \t\t */\n \t\tif (local_changes && staged_changes) {\n-\t\t\tif (!index_only || !is_empty_blob_sha1(ce->sha1))\n+\t\t\tif (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))\n \t\t\t\terrs = error(\"'%s' has staged content different \"\n \t\t\t\t\t \"from both the file and the HEAD\\n\"\n \t\t\t\t\t \"(use -f to force removal)\", name);\ndiff --git a/cache.h b/cache.h\nindex ef2e7f9..f15b3fc 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -176,10 +176,11 @@ struct cache_entry {\n /*\n * Extended on-disk flags\n */\n+#define CE_INTENT_TO_ADD 0x20000000\n /* CE_EXTENDED2 is for future extension */\n #define CE_EXTENDED2 0x80000000\n \n-#define CE_EXTENDED_FLAGS (0)\n+#define CE_EXTENDED_FLAGS (CE_INTENT_TO_ADD)\n \n /*\n * Safeguard to avoid saving wrong flags:\ndiff --git a/read-cache.c b/read-cache.c\nindex abc627b..fa30a0f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -546,6 +546,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tce->ce_flags = namelen;\n \tif (!intent_only)\n \t\tfill_stat_cache_info(ce, st);\n+\telse\n+\t\tce->ce_flags |= CE_INTENT_TO_ADD;\n \n \tif (trust_executable_bit && has_symlinks)\n \t\tce->ce_mode = create_ce_mode(st_mode);\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 5b4d6f7..b7d46e5 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -187,8 +187,8 @@ test_expect_success 'but with -f it should work.' '\n \ttest_must_fail git ls-files --error-unmatch baz\n '\n \n-test_expect_failure 'refuse to remove cached empty file with modifications' '\n-\ttouch empty &&\n+test_expect_success 'refuse to remove cached empty file with modifications' '\n+\t>empty &&\n \tgit add empty &&\n \techo content >empty &&\n \ttest_must_fail git rm --cached empty\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96709","messageId":"7vk5an2nil.fsf_-_@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"7vvdu72nq9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/3] git add --intent-to-add: do not let an empty blob committed by accident","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-29T03:56:34Z","receivedAt":"2008-11-29T03:56:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Writing a tree out of an index with an \"intent to add\" entry is blocked.\nThis implies that you cannot \"git commit\" from such a state; however \"git\ncommit -a\" still works.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-write-tree.c | 2 +-\n cache-tree.c | 10 +++++++---\n t/t3701-add-interactive.sh | 18 ++++++++++++++++++\n 3 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-write-tree.c b/builtin-write-tree.c\nindex 52a3c01..9d64050 100644\n--- a/builtin-write-tree.c\n+++ b/builtin-write-tree.c\n@@ -42,7 +42,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; the index is unmerged?\", me);\n+\t\tdie(\"%s: error building trees\", 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 5f8ee87..3d8f218 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -155,13 +155,17 @@ 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)) {\n+\t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\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\tfprintf(stderr, \"%s: unmerged (%s)\\n\",\n-\t\t\t\tce->name, sha1_to_hex(ce->sha1));\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}\n \t}\n \tif (funny)\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex e95663d..473ef85 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -133,6 +133,24 @@ test_expect_success 'real edit works' '\n \ttest_cmp expected output\n '\n \n+test_expect_success 'cannot 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+\ttest_must_fail git commit\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+\techo frotz >nitfol &&\n+\tgit add rezrov &&\n+\tgit add -N nitfol &&\n+\tgit commit -m partial rezrov\n+'\n+\n if test \"$(git config --bool core.filemode)\" = false\n then\n say 'skipping filemode tests (filesystem does not properly support modes)'\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96719","messageId":"fcaeb9bf0811290502j5db4056fo9b125aaa8b564314@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0811281938250.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-11-29T13:02:12Z","receivedAt":"2008-11-29T13:02:12Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 11/29/08, Daniel Barkalow wrote:\n> On Fri, 28 Nov 2008, Junio C Hamano wrote:\n>\n> > \"Shawn O. Pearce\" writes:\n> >\n> > > Johannes Schindelin wrote:\n> > > ...\n> > >> In other words, unless there is more interest in that feature, enough to\n> > >> generate a well-understood design before a good implementation, I'd rather\n> > >> see this patch series dropped.\n> > >\n> > > Ack. I agree with every remark made by Dscho, and also want to cry \"wolf\".\n> > >\n> > > I haven't had time to read the patch series. Its big and intrusive\n> > > and I just don't need the feature.\n> >\n> > Well, \"me neither\". Although I personally think resisting changes until\n> > it becomes absolutely necessary is a good discipline, we also need to\n> > recognise that there is a chicken-and-egg problem. When you have a\n> > potentially useful feature, unless people actually try using it in the\n> > field, you won't discover the drawbacks in either the design nor the\n> > implementation, let alone any improvements.\n>\n>\n> I just looked over most of it (skipping the generic index extension\n> portion). It looks to me like it's introducing an extra concept to avoid\n> actually fixing maybe-bugs in the \"assume unchanged\" implementation\n> when used with files that have been changed intentionally (with the user\n> intending git to overlook this change). Sparse checkout is essentially a\n> special case of this, where the user has changed the working directory\n> radically (not populating it at all) and wants git to carry on as if this\n> was not the case (with a certain amount of porcelain code to cause this to\n> happen automatically).\n\nI chose to use another bit because I did not want to change the\nbehaviour of CE_VALID bit: it assumes all files are present.\n\n> If there's any need for this to be distinguished from \"assume unchanged\",\n> I think it should be used with, not instead of, the CE_VALID bit; and it\n> could probably use some bit in the stat info section, since we don't need\n> stat info if we know by assumption that the entry is valid.\n\nInteresting. I'll think more about this.\n\n> -Daniel\n> *This .sig left intentionally blank*\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>\n\n\n-- \nDuy\n"},{"id":"96724","messageId":"bd6139dc0811290738qbd93ff6oa7aa854708009075@mail.gmail.com","threadId":"16487","inReplyTo":"7vprkf2nki.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-11-29T15:38:12Z","receivedAt":"2008-11-29T15:38:12Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Sat, Nov 29, 2008 at 04:55, Junio C Hamano wrote:\n> This uses the extended index flag mechanism introduced earlier to mark\n> the entries added to the index via \"git add -N\" with CE_INTENT_TO_ADD.\n\nIs 'intent' [0] used properly here? Should it not be 'intend' [1]?\n\n[0] http://en.wiktionary.org/wiki/intent\n[1] http://en.wiktionary.org/wiki/intend\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"96770","messageId":"fcaeb9bf0811300229v4e7bfbb7g9a0ac72dcddb4326@mail.gmail.com","threadId":"16487","inReplyTo":"fcaeb9bf0811290502j5db4056fo9b125aaa8b564314@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-11-30T10:29:08Z","receivedAt":"2008-11-30T10:29:08Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 11/29/08, Nguyen Thai Ngoc Duy wrote:\n> On 11/29/08, Daniel Barkalow wrote:\n> > If there's any need for this to be distinguished from \"assume unchanged\",\n> > I think it should be used with, not instead of, the CE_VALID bit; and it\n> > could probably use some bit in the stat info section, since we don't need\n> > stat info if we know by assumption that the entry is valid.\n>\n>\n> Interesting. I'll think more about this.\n>\n\nAs I said, CE_VALID implies all files are present. I could make\nCE_NO_CHECKOUT to be used with CE_VALID, but I would need to check all\nCE_VALID code path to make sure the behaviour remains if\nCE_NO_CHECKOUT is absent. It's just more intrusive.\n\nI have nothing against storing CE_NO_CHECKOUT in stat info except that\nit seems inappropriate/hidden place to do. ce_flags is more obvious\nchoice. I haven't looked closely to stat info code in read-cache.c\nthough.\n-- \nDuy\n"},{"id":"96792","messageId":"20081130191444.GC10981@coredump.intra.peff.net","threadId":"16487","inReplyTo":"7vk5an2nil.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] git add --intent-to-add: do not let an empty blob committed by accident","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-30T19:14:44Z","receivedAt":"2008-11-30T19:14:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 28, 2008 at 07:56:34PM -0800, Junio C Hamano wrote:\n\n> Subject: Re: [PATCH 3/3] git add --intent-to-add: do not let an empty blob\n>\tcommitted by accident\n\nMinor nit: grammatical error in the subject.\n\n> This implies that you cannot \"git commit\" from such a state; however \"git\n> commit -a\" still works.\n\nI was going to provide a test for \"git commit -a\" to squash in, but it\nlooks like the version in 'pu' already has one.\n\n> \tcase WRITE_TREE_UNMERGED_INDEX:\n> -\t\tdie(\"%s: error building trees; the index is unmerged?\", me);\n> +\t\tdie(\"%s: error building trees\", me);\n\nThis caught me by surprise while reading, but I assume the rationale is\n\"now there is a new, different reason not to be able to build the trees,\nso our guess is less likely to be correct\". I wonder if we can do better\nby actually passing out a more exact error value (though it looks like\nwe will already have said \"foo: not added yet\" by that point anyway, so\nmaybe it is just pointless to say more).\n\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n\nWhy in t3701? These don't have anything to do with interactive add, and\nthere is a already a t2203-add-intent.\n\n-Peff\n"},{"id":"96793","messageId":"20081130192101.GD10981@coredump.intra.peff.net","threadId":"16487","inReplyTo":"bd6139dc0811290738qbd93ff6oa7aa854708009075@mail.gmail.com","subject":"Re: [PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-30T19:21:01Z","receivedAt":"2008-11-30T19:21:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 29, 2008 at 04:38:12PM +0100, Sverre Rabbelier wrote:\n\n> On Sat, Nov 29, 2008 at 04:55, Junio C Hamano wrote:\n> > This uses the extended index flag mechanism introduced earlier to mark\n> > the entries added to the index via \"git add -N\" with CE_INTENT_TO_ADD.\n> \n> Is 'intent' [0] used properly here? Should it not be 'intend' [1]?\n> \n> [0] http://en.wiktionary.org/wiki/intent\n> [1] http://en.wiktionary.org/wiki/intend\n\nI think it's fine. The flags describe the entry, not the user (e.g.,\nCE_VALID). So the entry does not _intend_ to add anything, but rather\nthere exists _intent_ to add this entry (you might also say the entry is\n\"_intended_ to be added\", but that is getting a bit clunky).\n\n-Peff\n"},{"id":"96795","messageId":"alpine.LNX.1.00.0811301509070.19665@iabervon.org","threadId":"16487","inReplyTo":"fcaeb9bf0811300229v4e7bfbb7g9a0ac72dcddb4326@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-11-30T21:26:19Z","receivedAt":"2008-11-30T21:26:19Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 30 Nov 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On 11/29/08, Nguyen Thai Ngoc Duy wrote:\n> > On 11/29/08, Daniel Barkalow wrote:\n> > > If there's any need for this to be distinguished from \"assume unchanged\",\n> > > I think it should be used with, not instead of, the CE_VALID bit; and it\n> > > could probably use some bit in the stat info section, since we don't need\n> > > stat info if we know by assumption that the entry is valid.\n> >\n> >\n> > Interesting. I'll think more about this.\n> >\n> \n> As I said, CE_VALID implies all files are present.\n\nMy first question is whether this actually should be true. Going back to \nthe message for 5f73076c1a9b4b8dc94f77eac98eb558d25e33c0, it sounds like \nthe CE_VALID code is designed to be safe and sort of correct even if the \nfiles are not actually unchanged; I don't think it would be out-of-spec \nfor CE_VALID to (1) always produce output as if the working tree contained \nwhat the index contains, while (2) refusing to make any changes to working \ntree files that do not actually match the index. As it is now, (2) is \nexplicitly true, but (1) is left vague-- commands may fail entirely or \nproduce different output if CE_VALID is set in the index for a file that \nhas changes in the working tree, but not in any particular way.\n\nNow, it might be necessary for CE_NO_CHECKOUT to differ from CE_VALID in \nsome ways in (2): if a file is CE_NO_CHECKOUT and absent, code which would \nmodify it could probably just report sucess, while CE_VALID on a file \nwith changes should probably report failure. On the other hand, that could \njust as easily be at the porcelain layer, with the porcelain instructing \nthe plumbing to change the index without changing the working tree for \nthose files outside the sparse checkout, and the plumbing would report \nerrors if the porcelain did not do this.\n\n> I could make CE_NO_CHECKOUT to be used with CE_VALID, but I would need \n> to check all CE_VALID code path to make sure the behaviour remains if\n> CE_NO_CHECKOUT is absent. It's just more intrusive.\n\nI would expect all code that has a CE_VALID path to do something actually \nwrong if it took the non-CE_VALID code path on CE_NO_CHECKOUT and there \nwas no CE_NO_CHECKOUT code path. So I'd expect that your patch is \ninsufficient to the extent that CE_NO_CHECKOUT doesn't imply CE_VALID \n(since there is very little in the way of CE_NO_CHECKOUT-specific \nhandling in your patch).\n\nThe only case I can think of where NO_CHECKOUT is more like !VALID than \nVALID is with respect to whether we can report the content in the index by \nlooking in the filesystem instead of in the database; I don't think this \nis an intentional optimization anywhere, and I think it would be a likely \nsource of bugs if it were (e.g., it would have to know about files which \nare up-to-date with respect to stat info, but which have been \"smudged\" on \ndisk and therefore don't match byte-for-byte with the database). Actually, \nit might be most accurate to treat --no-checkout as being CE_VALID with a \nsmudge filter of \"rm\". If the combination of CE_VALID and on-disk \nconversion works (which is likely to be the common pattern for Windows \nusers, who need autocrlf and have a slow lstat(), and is therefore \nmaintained), surely this combination would work for CE_NO_CHECKOUT.\n\n> I have nothing against storing CE_NO_CHECKOUT in stat info except that\n> it seems inappropriate/hidden place to do. ce_flags is more obvious\n> choice. I haven't looked closely to stat info code in read-cache.c\n> though.\n\nIt should be pretty clean to check CE_VALID when reading an entry from \ndisk and remap bits from it to additional flags in memory. I wouldn't \nsuggest overlaying them in memory, but there's also no shortage of space \nfor flags in memory.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"96817","messageId":"7vhc5os0xw.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"20081130191444.GC10981@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] git add --intent-to-add: do not let an empty blob committed by accident","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-01T09:24:11Z","receivedAt":"2008-12-01T09:24:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Nov 28, 2008 at 07:56:34PM -0800, Junio C Hamano wrote:\n> ...\n>> \tcase WRITE_TREE_UNMERGED_INDEX:\n>> -\t\tdie(\"%s: error building trees; the index is unmerged?\", me);\n>> +\t\tdie(\"%s: error building trees\", me);\n>\n> This caught me by surprise while reading, but I assume the rationale is\n> \"now there is a new, different reason not to be able to build the trees,\n> so our guess is less likely to be correct\". I wonder if we can do better\n> by actually passing out a more exact error value (though it looks like\n> we will already have said \"foo: not added yet\" by that point anyway, so\n> maybe it is just pointless to say more).\n\nThe places that detect the \"unmerged\" (and then newly added \"intent-only\")\nentries already have issued error messages in the codepath that leads to\nthis error.\n\n>> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n>\n> Why in t3701?\n\nGood question. Brain fart, perhaps.\n"},{"id":"97270","messageId":"fcaeb9bf0812060926r2ee443bfl3adb3f2d1129e5b8@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0811301509070.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-06T17:26:06Z","receivedAt":"2008-12-06T17:26:06Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/1/08, Daniel Barkalow wrote:\n> On Sun, 30 Nov 2008, Nguyen Thai Ngoc Duy wrote:\n>\n> > On 11/29/08, Nguyen Thai Ngoc Duy wrote:\n> > > On 11/29/08, Daniel Barkalow wrote:\n> > > > If there's any need for this to be distinguished from \"assume unchanged\",\n> > > > I think it should be used with, not instead of, the CE_VALID bit; and it\n> > > > could probably use some bit in the stat info section, since we don't need\n> > > > stat info if we know by assumption that the entry is valid.\n> > >\n> > >\n> > > Interesting. I'll think more about this.\n> > >\n> >\n> > As I said, CE_VALID implies all files are present.\n>\n>\n> My first question is whether this actually should be true. Going back to\n> the message for 5f73076c1a9b4b8dc94f77eac98eb558d25e33c0, it sounds like\n> the CE_VALID code is designed to be safe and sort of correct even if the\n> files are not actually unchanged; I don't think it would be out-of-spec\n> for CE_VALID to (1) always produce output as if the working tree contained\n> what the index contains, while (2) refusing to make any changes to working\n> tree files that do not actually match the index. As it is now, (2) is\n> explicitly true, but (1) is left vague-- commands may fail entirely or\n> produce different output if CE_VALID is set in the index for a file that\n> has changes in the working tree, but not in any particular way.\n\n(1) is not always true. For example diff machinary may examine\nworktree files regardless CE_VALID, which is updated for\nCE_NO_CHECKOUT in d9f8fca (Prevent diff machinery from examining\nworktree outside sparse checkout)\n\n> Now, it might be necessary for CE_NO_CHECKOUT to differ from CE_VALID in\n> some ways in (2): if a file is CE_NO_CHECKOUT and absent, code which would\n> modify it could probably just report sucess, while CE_VALID on a file\n> with changes should probably report failure. On the other hand, that could\n> just as easily be at the porcelain layer, with the porcelain instructing\n> the plumbing to change the index without changing the working tree for\n> those files outside the sparse checkout, and the plumbing would report\n> errors if the porcelain did not do this.\n\nThat's right. Much of work in the last half of the series is on\nporcelain layer. \"git grep\" fix is the only porcelain that gets fixed\nin this series.\n\n> > I could make CE_NO_CHECKOUT to be used with CE_VALID, but I would need\n> > to check all CE_VALID code path to make sure the behaviour remains if\n> > CE_NO_CHECKOUT is absent. It's just more intrusive.\n>\n>\n> I would expect all code that has a CE_VALID path to do something actually\n> wrong if it took the non-CE_VALID code path on CE_NO_CHECKOUT and there\n> was no CE_NO_CHECKOUT code path. So I'd expect that your patch is\n> insufficient to the extent that CE_NO_CHECKOUT doesn't imply CE_VALID\n> (since there is very little in the way of CE_NO_CHECKOUT-specific\n> handling in your patch).\n\nI read the code again. CE_NO_CHECKOUT should follow CE_VALID code path\n(which was extended to CE_VALID_MASK to have both flags). That means\nCE_NO_CHECKOUT is treated as same as CE_VALID. The only difference\nhere is CE_VALID is set/unset by \"git update-index --really-refresh\"\nand core.ingorestat while CE_NO_CHECKOUT has its own way to set/unset.\n\nThere is not much work for CE_NO_CHECKOUT on plumbling level except\nsome fixes. The last half of the series, for porcelain level, you will\nsee more.\n\n> The only case I can think of where NO_CHECKOUT is more like !VALID than\n> VALID is with respect to whether we can report the content in the index by\n> looking in the filesystem instead of in the database; I don't think this\n> is an intentional optimization anywhere, and I think it would be a likely\n> source of bugs if it were (e.g., it would have to know about files which\n> are up-to-date with respect to stat info, but which have been \"smudged\" on\n> disk and therefore don't match byte-for-byte with the database).\n\nThere is worktree file reuse in diff code somewhere IIRC. Yes, this\nshould be checked.\n\n> Actually,\n> it might be most accurate to treat --no-checkout as being CE_VALID with a\n> smudge filter of \"rm\". If the combination of CE_VALID and on-disk\n> conversion works (which is likely to be the common pattern for Windows\n> users, who need autocrlf and have a slow lstat(), and is therefore\n> maintained), surely this combination would work for CE_NO_CHECKOUT.\n\nVery interesting.\n\n> > I have nothing against storing CE_NO_CHECKOUT in stat info except that\n> > it seems inappropriate/hidden place to do. ce_flags is more obvious\n> > choice. I haven't looked closely to stat info code in read-cache.c\n> > though.\n>\n>\n> It should be pretty clean to check CE_VALID when reading an entry from\n> disk and remap bits from it to additional flags in memory. I wouldn't\n> suggest overlaying them in memory, but there's also no shortage of space\n> for flags in memory.\n\nI see. Still I prefer the current approach, less headache to decide\nwhat bit to take from stat info ;-)\n-- \nDuy\n"},{"id":"97271","messageId":"alpine.LNX.1.00.0812061238260.19665@iabervon.org","threadId":"16487","inReplyTo":"fcaeb9bf0812060926r2ee443bfl3adb3f2d1129e5b8@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-12-06T18:39:18Z","receivedAt":"2008-12-06T18:39:18Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 7 Dec 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On 12/1/08, Daniel Barkalow wrote:\n> > On Sun, 30 Nov 2008, Nguyen Thai Ngoc Duy wrote:\n> >\n> > > On 11/29/08, Nguyen Thai Ngoc Duy wrote:\n> > > > On 11/29/08, Daniel Barkalow wrote:\n> > > > > If there's any need for this to be distinguished from \"assume unchanged\",\n> > > > > I think it should be used with, not instead of, the CE_VALID bit; and it\n> > > > > could probably use some bit in the stat info section, since we don't need\n> > > > > stat info if we know by assumption that the entry is valid.\n> > > >\n> > > >\n> > > > Interesting. I'll think more about this.\n> > > >\n> > >\n> > > As I said, CE_VALID implies all files are present.\n> >\n> >\n> > My first question is whether this actually should be true. Going back to\n> > the message for 5f73076c1a9b4b8dc94f77eac98eb558d25e33c0, it sounds like\n> > the CE_VALID code is designed to be safe and sort of correct even if the\n> > files are not actually unchanged; I don't think it would be out-of-spec\n> > for CE_VALID to (1) always produce output as if the working tree contained\n> > what the index contains, while (2) refusing to make any changes to working\n> > tree files that do not actually match the index. As it is now, (2) is\n> > explicitly true, but (1) is left vague-- commands may fail entirely or\n> > produce different output if CE_VALID is set in the index for a file that\n> > has changes in the working tree, but not in any particular way.\n> \n> (1) is not always true. For example diff machinary may examine\n> worktree files regardless CE_VALID, which is updated for\n> CE_NO_CHECKOUT in d9f8fca (Prevent diff machinery from examining\n> worktree outside sparse checkout)\n\nI know (1) is not always true in the current implementation. What I'm \ngetting at is to ask (a) whether our documented behavior ever violates (1) \nand (b) whether enforcing (1) would be an improvement.\n\nI suspect that enforcing (1) would be less surprising to users in the \nsituation where the assumption is false that worktree files match the \nindex when CE_VALID is set. As it is, the diff machinery does surprising \nthings in this situation, and I think we just say \"Nasal Monkey Territory\" \n(that is, we tell the user, \"if you don't want git to do surprising \nthings, don't get into this situation\"). If that is the case, we should be \nfine changing it to match your CE_NO_CHECKOUT behavior, and it wouldn't be \nworse for CE_VALID and might be better.\n\n> > Now, it might be necessary for CE_NO_CHECKOUT to differ from CE_VALID in\n> > some ways in (2): if a file is CE_NO_CHECKOUT and absent, code which would\n> > modify it could probably just report sucess, while CE_VALID on a file\n> > with changes should probably report failure. On the other hand, that could\n> > just as easily be at the porcelain layer, with the porcelain instructing\n> > the plumbing to change the index without changing the working tree for\n> > those files outside the sparse checkout, and the plumbing would report\n> > errors if the porcelain did not do this.\n> \n> That's right. Much of work in the last half of the series is on\n> porcelain layer. \"git grep\" fix is the only porcelain that gets fixed\n> in this series.\n\nI haven't gotten to reading the last half yet, so I'll avoid taking a \nspecific position on how it should work, and just make unsupported \nsuggestions.\n\n> > > I could make CE_NO_CHECKOUT to be used with CE_VALID, but I would need\n> > > to check all CE_VALID code path to make sure the behaviour remains if\n> > > CE_NO_CHECKOUT is absent. It's just more intrusive.\n> >\n> >\n> > I would expect all code that has a CE_VALID path to do something actually\n> > wrong if it took the non-CE_VALID code path on CE_NO_CHECKOUT and there\n> > was no CE_NO_CHECKOUT code path. So I'd expect that your patch is\n> > insufficient to the extent that CE_NO_CHECKOUT doesn't imply CE_VALID\n> > (since there is very little in the way of CE_NO_CHECKOUT-specific\n> > handling in your patch).\n> \n> I read the code again. CE_NO_CHECKOUT should follow CE_VALID code path\n> (which was extended to CE_VALID_MASK to have both flags). That means\n> CE_NO_CHECKOUT is treated as same as CE_VALID. The only difference\n> here is CE_VALID is set/unset by \"git update-index --really-refresh\"\n> and core.ingorestat while CE_NO_CHECKOUT has its own way to set/unset.\n\nI think, then, that it would be cleaner to have CE_VALID instead of \nCE_VALID_MASK, and have the things that care test CE_NO_CHECKOUT or \n!CE_NO_CHECKOUT. This wouldn't be all that different, except that it would \nmean that distinguishing them appears as a special case, which in turn \nmakes it easier to question whether they should differ.\n\n> There is not much work for CE_NO_CHECKOUT on plumbling level except\n> some fixes. The last half of the series, for porcelain level, you will\n> see more.\n\nFor the porcelain level, do we need the difference to be in the index? If \nthe porcelain knows the sparse checkout area and can instruct the plumbing \nappropriately, the information shouldn't need to be stored in the index \nunless it's ever important to remember whether an entry is CE_VALID due to \nhaving been outside the checkout when the index was written, even though \nthe checkout area now includes it. I don't have a good intuition as to \nwhat ought to happen if the user manually changes what's specified for \ncheckout without actually updating the index and working tree.\n\n> > The only case I can think of where NO_CHECKOUT is more like !VALID than\n> > VALID is with respect to whether we can report the content in the index by\n> > looking in the filesystem instead of in the database; I don't think this\n> > is an intentional optimization anywhere, and I think it would be a likely\n> > source of bugs if it were (e.g., it would have to know about files which\n> > are up-to-date with respect to stat info, but which have been \"smudged\" on\n> > disk and therefore don't match byte-for-byte with the database).\n> \n> There is worktree file reuse in diff code somewhere IIRC. Yes, this\n> should be checked.\n> \n> > Actually,\n> > it might be most accurate to treat --no-checkout as being CE_VALID with a\n> > smudge filter of \"rm\". If the combination of CE_VALID and on-disk\n> > conversion works (which is likely to be the common pattern for Windows\n> > users, who need autocrlf and have a slow lstat(), and is therefore\n> > maintained), surely this combination would work for CE_NO_CHECKOUT.\n> \n> Very interesting.\n> \n> > > I have nothing against storing CE_NO_CHECKOUT in stat info except that\n> > > it seems inappropriate/hidden place to do. ce_flags is more obvious\n> > > choice. I haven't looked closely to stat info code in read-cache.c\n> > > though.\n> >\n> >\n> > It should be pretty clean to check CE_VALID when reading an entry from\n> > disk and remap bits from it to additional flags in memory. I wouldn't\n> > suggest overlaying them in memory, but there's also no shortage of space\n> > for flags in memory.\n> \n> I see. Still I prefer the current approach, less headache to decide\n> what bit to take from stat info ;-)\n\nTrue. But it would be even easier if it didn't have to be marked in the \nindex at all.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"97292","messageId":"7vwsecejhc.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0811301509070.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-07T03:45:35Z","receivedAt":"2008-12-07T03:45:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n>> As I said, CE_VALID implies all files are present.\n>\n> My first question is whether this actually should be true.\n\nIf the user says \"Please pretend that I have never touched this file\",\nwhich is what \"assume unchanged\" is all about, I think we should not\nnotice if the user removes one of such files from the working tree, just\nlike we don't notice (rather, pretend not to notice) if the user modified\nit.\n\nI am inclined to think that we should rather treat that as a bug.\n"},{"id":"97302","messageId":"fcaeb9bf0812070427s64438216s41bf1294aa6398a3@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812061238260.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-07T12:27:00Z","receivedAt":"2008-12-07T12:27:00Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/7/08, Daniel Barkalow wrote:\n> > There is not much work for CE_NO_CHECKOUT on plumbling level except\n> > some fixes. The last half of the series, for porcelain level, you will\n> > see more.\n>\n>\n> For the porcelain level, do we need the difference to be in the index? If\n> the porcelain knows the sparse checkout area and can instruct the plumbing\n> appropriately, the information shouldn't need to be stored in the index\n\nThis was discussed since the beginning of this feature. I recall that\nthe index reflects worktree, and because we mark CE_NO_CHECKOUT on\nfile basis, it's best to save the information there, not separately.\nWe do save high level information to form the checkout area (sparse\npatterns) in the last half, but basically you should be able to live\nwithout that.\n\n> unless it's ever important to remember whether an entry is CE_VALID due to\n> having been outside the checkout when the index was written, even though\n> the checkout area now includes it. I don't have a good intuition as to\n> what ought to happen if the user manually changes what's specified for\n> checkout without actually updating the index and working tree.\n\nSo if a user changes worktree without updating index, they will have\nthe same results as they do now: files are shown as modified if they\ndon't have CE_NO_CHECKOUT set. If those files do, they are considered\n'orphaned' or staled and are recommended to be removed/updated to\navoid unexpected consequences (not availble this this first half\nseries because that belongs to \"git status\").\n-- \nDuy\n"},{"id":"97311","messageId":"alpine.LNX.1.00.0812071455020.19665@iabervon.org","threadId":"16487","inReplyTo":"fcaeb9bf0812070427s64438216s41bf1294aa6398a3@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-12-07T21:26:21Z","receivedAt":"2008-12-07T21:26:21Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 7 Dec 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On 12/7/08, Daniel Barkalow wrote:\n> > > There is not much work for CE_NO_CHECKOUT on plumbling level except\n> > > some fixes. The last half of the series, for porcelain level, you will\n> > > see more.\n> >\n> >\n> > For the porcelain level, do we need the difference to be in the index? If\n> > the porcelain knows the sparse checkout area and can instruct the plumbing\n> > appropriately, the information shouldn't need to be stored in the index\n> \n> This was discussed since the beginning of this feature. I recall that\n> the index reflects worktree, and because we mark CE_NO_CHECKOUT on\n> file basis, it's best to save the information there, not separately.\n> We do save high level information to form the checkout area (sparse\n> patterns) in the last half, but basically you should be able to live\n> without that.\n\nWe need to mark in the index the information that reflects the worktree.\n\nIf, however, we take CE_VALID to be the flag for \"ignore the worktree \nentirely at this path; act as it if contains what the index contains\" (and \nuse this to cause that aspect of no-checkout), and we then entirely ignore \nthe worktree, including not caring whether there are files there or not \n(except, of course, that in the transition from caring to not caring for \nno-checkout, we make the worktree empty, while in the case for \n\"stat-is-expensive\", we bring it into agreement with the index), then \nthere is no additional information that needs to be conveyed in the index.\n\n> > unless it's ever important to remember whether an entry is CE_VALID due to\n> > having been outside the checkout when the index was written, even though\n> > the checkout area now includes it. I don't have a good intuition as to\n> > what ought to happen if the user manually changes what's specified for\n> > checkout without actually updating the index and working tree.\n> \n> So if a user changes worktree without updating index, they will have\n> the same results as they do now: files are shown as modified if they\n> don't have CE_NO_CHECKOUT set. If those files do, they are considered\n> 'orphaned' or staled and are recommended to be removed/updated to\n> avoid unexpected consequences (not availble this this first half\n> series because that belongs to \"git status\").\n\nI was actually thinking that there would be a file for \"this is what the \nuser wants to have checked out\" (as opposed to the index, which must \ncontain \"this is what is checked out\"), and the porcelain would instruct \nthe plumbing as to what to do with the worktree (that the plumbing with \nthen ignore, due to the index bit) based on this information.\n\nThe index obviously can't contain the user's full instructions for what \nshould be checked out, because the user will want to say \"I don't care \nabout anything in Documentation/\" and have this apply to \nDocumentation/some-file-not-in-the-index, so that if this file is in the \nworktree, the user gets a warning.\n\nI think you're doing this with core.defaultsparse, although you seem to \nallow the index to diverge from this easily.\n\nThe question, then, is what happens when the index and core.defaultsparse \ndisagree, either because the porcelain supports causing it or because the \nuser has simply editting the config file or used plumbing to modify the \nindex. That is, (1) we have index entries that say that the worktree is \nignored, and the rules don't say they're outside the sparse checkout; do \nwe care whether we expect the worktree to be empty or match the index? \nAnd, (2) we have index entries that say we do care about them, but the \nrules say they're outside the sparse checkout; what happens with these?\n\nCase (1) is where we would need to know, in the index, whether we expect \nthe worktree to actually match the index (traditional CE_VALID) or whether \nwe expect the worktree to be empty (CE_NO_CHECKOUT), if our behavior \nshould actually differ. My vague feeling is that we don't want it to \ndiffer, and these paths are unexpectional \"interesting to the user, but \nthe worktree is ignored\" until reading a tree into the index again. (But \nnote that we will have to check on the worktree when reading into the \nindex if this changes the index from \"blobA, CE_VALID\" to \n\"blobA, !CE_VALID\", since the worktree could differ in a way that we don't \nwant to retain. And I think we want it to be an error to have the worktree \nbe something other than blobA or nothing before, but \"nothing\" is fine and \nwe just write it out. (This means that users of CE_VALID who remove files \nbehind git's back may lose their removal work; but this is a pretty \ntrivial danger).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"97342","messageId":"fcaeb9bf0812080451k6e213d0fo8d1da9bbac872649@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812071455020.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-08T12:51:00Z","receivedAt":"2008-12-08T12:51:00Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/8/08, Daniel Barkalow wrote:\n> > This was discussed since the beginning of this feature. I recall that\n> > the index reflects worktree, and because we mark CE_NO_CHECKOUT on\n> > file basis, it's best to save the information there, not separately.\n> > We do save high level information to form the checkout area (sparse\n> > patterns) in the last half, but basically you should be able to live\n> > without that.\n>\n>\n> We need to mark in the index the information that reflects the worktree.\n>\n> If, however, we take CE_VALID to be the flag for \"ignore the worktree\n> entirely at this path; act as it if contains what the index contains\" (and\n> use this to cause that aspect of no-checkout), and we then entirely ignore\n> the worktree, including not caring whether there are files there or not\n> (except, of course, that in the transition from caring to not caring for\n> no-checkout, we make the worktree empty, while in the case for\n> \"stat-is-expensive\", we bring it into agreement with the index), then\n> there is no additional information that needs to be conveyed in the index.\n\nThat's not enough. CE_VALID is \"ignore the worktree files\" while\nCE_NO_CHECKOUT is stricter: \"those files does (or should) not exist\".\nThe difference is\n\n - for \"git grep\", we ignore path with CE_NO_CHECKOUT (while using\ncache version for CE_VALID)\n - porcelain-level support to widen/narrow checkout area will need\nCE_NO_CHECKOUT, not CE_VALID\n\n>\n> > > unless it's ever important to remember whether an entry is CE_VALID due to\n> > > having been outside the checkout when the index was written, even though\n> > > the checkout area now includes it. I don't have a good intuition as to\n> > > what ought to happen if the user manually changes what's specified for\n> > > checkout without actually updating the index and working tree.\n> >\n> > So if a user changes worktree without updating index, they will have\n> > the same results as they do now: files are shown as modified if they\n> > don't have CE_NO_CHECKOUT set. If those files do, they are considered\n> > 'orphaned' or staled and are recommended to be removed/updated to\n> > avoid unexpected consequences (not availble this this first half\n> > series because that belongs to \"git status\").\n>\n>\n> I was actually thinking that there would be a file for \"this is what the\n> user wants to have checked out\" (as opposed to the index, which must\n> contain \"this is what is checked out\"), and the porcelain would instruct\n> the plumbing as to what to do with the worktree (that the plumbing with\n> then ignore, due to the index bit) based on this information.\n>\n> The index obviously can't contain the user's full instructions for what\n> should be checked out, because the user will want to say \"I don't care\n> about anything in Documentation/\" and have this apply to\n> Documentation/some-file-not-in-the-index, so that if this file is in the\n> worktree, the user gets a warning.\n>\n> I think you're doing this with core.defaultsparse, although you seem to\n> allow the index to diverge from this easily.\n\nYes they can.\n\n>\n> The question, then, is what happens when the index and core.defaultsparse\n> disagree, either because the porcelain supports causing it or because the\n> user has simply editting the config file or used plumbing to modify the\n> index. That is, (1) we have index entries that say that the worktree is\n> ignored, and the rules don't say they're outside the sparse checkout; do\n> we care whether we expect the worktree to be empty or match the index?\n> And, (2) we have index entries that say we do care about them, but the\n> rules say they're outside the sparse checkout; what happens with these?\n\nThe rule is CE_NO_CHECKOUT is king. core.defaultsparse only helps\nsetting CE_NO_CHECKOUT on new entries when they enter the index.\n\nSo if core.defaultsparse does not match what is in index, that's fine.\nYou may get more files with \"git merge\", \"git checkout\".. but\nalready-removed files should stay removed.\n\nIf you are not happy with core.defaultsparse, you can replace it with\nyour own tools by manipulating directly CE_NO_CHECKOUT using \"git\nupdate-index\" and \"git ls-files\". BTW the implementation has not had\nan easy way to replace core.defaultsparse. You have to modify\napply_narrow_spec(). But if someone really needs it, a hook can be\nadded.\n-- \nDuy\n"},{"id":"97379","messageId":"alpine.LNX.1.00.0812081223140.19665@iabervon.org","threadId":"16487","inReplyTo":"fcaeb9bf0812080451k6e213d0fo8d1da9bbac872649@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-12-08T19:41:37Z","receivedAt":"2008-12-08T19:41:37Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 8 Dec 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On 12/8/08, Daniel Barkalow wrote:\n> > > This was discussed since the beginning of this feature. I recall that\n> > > the index reflects worktree, and because we mark CE_NO_CHECKOUT on\n> > > file basis, it's best to save the information there, not separately.\n> > > We do save high level information to form the checkout area (sparse\n> > > patterns) in the last half, but basically you should be able to live\n> > > without that.\n> >\n> >\n> > We need to mark in the index the information that reflects the worktree.\n> >\n> > If, however, we take CE_VALID to be the flag for \"ignore the worktree\n> > entirely at this path; act as it if contains what the index contains\" (and\n> > use this to cause that aspect of no-checkout), and we then entirely ignore\n> > the worktree, including not caring whether there are files there or not\n> > (except, of course, that in the transition from caring to not caring for\n> > no-checkout, we make the worktree empty, while in the case for\n> > \"stat-is-expensive\", we bring it into agreement with the index), then\n> > there is no additional information that needs to be conveyed in the index.\n> \n> That's not enough. CE_VALID is \"ignore the worktree files\" while\n> CE_NO_CHECKOUT is stricter: \"those files does (or should) not exist\".\n> The difference is\n> \n> - for \"git grep\", we ignore path with CE_NO_CHECKOUT (while using\n> cache version for CE_VALID)\n\nIs this sufficient? I'd expect \"git grep\" to ignore paths that are outside \nthe checked-out region, even when searching an arbitrary tree, and even \nwhen those files aren't in the index at all (i.e., the current commit \ndoesn't have them). That is, I'd expect core.defaultsparse or the \nequivalent to limit the paths, normally giving this effect.\n\n> - porcelain-level support to widen/narrow checkout area will need\n> CE_NO_CHECKOUT, not CE_VALID\n\nThis isn't a meaningful difference between CE_NO_CHECKOUT and CE_VALID if \nthere aren't any other differences.\n\n> > > > unless it's ever important to remember whether an entry is CE_VALID due to\n> > > > having been outside the checkout when the index was written, even though\n> > > > the checkout area now includes it. I don't have a good intuition as to\n> > > > what ought to happen if the user manually changes what's specified for\n> > > > checkout without actually updating the index and working tree.\n> > >\n> > > So if a user changes worktree without updating index, they will have\n> > > the same results as they do now: files are shown as modified if they\n> > > don't have CE_NO_CHECKOUT set. If those files do, they are considered\n> > > 'orphaned' or staled and are recommended to be removed/updated to\n> > > avoid unexpected consequences (not availble this this first half\n> > > series because that belongs to \"git status\").\n> >\n> >\n> > I was actually thinking that there would be a file for \"this is what the\n> > user wants to have checked out\" (as opposed to the index, which must\n> > contain \"this is what is checked out\"), and the porcelain would instruct\n> > the plumbing as to what to do with the worktree (that the plumbing with\n> > then ignore, due to the index bit) based on this information.\n> >\n> > The index obviously can't contain the user's full instructions for what\n> > should be checked out, because the user will want to say \"I don't care\n> > about anything in Documentation/\" and have this apply to\n> > Documentation/some-file-not-in-the-index, so that if this file is in the\n> > worktree, the user gets a warning.\n> >\n> > I think you're doing this with core.defaultsparse, although you seem to\n> > allow the index to diverge from this easily.\n> \n> Yes they can.\n> \n> >\n> > The question, then, is what happens when the index and core.defaultsparse\n> > disagree, either because the porcelain supports causing it or because the\n> > user has simply editting the config file or used plumbing to modify the\n> > index. That is, (1) we have index entries that say that the worktree is\n> > ignored, and the rules don't say they're outside the sparse checkout; do\n> > we care whether we expect the worktree to be empty or match the index?\n> > And, (2) we have index entries that say we do care about them, but the\n> > rules say they're outside the sparse checkout; what happens with these?\n> \n> The rule is CE_NO_CHECKOUT is king. core.defaultsparse only helps\n> setting CE_NO_CHECKOUT on new entries when they enter the index.\n\nThis seems like a really bad idea to me. If you ask for a file that's \noutside your default area to be checked out, and then you switch branches \nand switch back, the file may or may not disappear (depending on whether \nthe branch you switched to temporarily had it or not). Likewise, if you \nremove files, and then switch branches and back, the files may or may not \nreappear.\n\nOf course, commands need to look at the index to determine what we've \nactually done with the worktree and index, but I think there should be \nsome other location that is responsible for keeping track of what the user \nhas asked for.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"97595","messageId":"fcaeb9bf0812110504u1acfb612he3edae1df3774045@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812081223140.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-11T13:04:32Z","receivedAt":"2008-12-11T13:04:32Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/9/08, Daniel Barkalow wrote:\n> > - for \"git grep\", we ignore path with CE_NO_CHECKOUT (while using\n> > cache version for CE_VALID)\n>\n>\n> Is this sufficient? I'd expect \"git grep\" to ignore paths that are outside\n> the checked-out region, even when searching an arbitrary tree, and even\n> when those files aren't in the index at all (i.e., the current commit\n> doesn't have them). That is, I'd expect core.defaultsparse or the\n> equivalent to limit the paths, normally giving this effect.\n\nThat's the point. CE_VALID does not define checkout area while\nCE_NO_CHECKOUT does. If an entry is CE_VALID, it is still in checkout\narea. But if it is CE_NO_CHECKOUT, \"git grep\" should ignore that path.\ncore.defaultsparse has nothing to do here.\n\n> > > The question, then, is what happens when the index and core.defaultsparse\n> > > disagree, either because the porcelain supports causing it or because the\n> > > user has simply editting the config file or used plumbing to modify the\n> > > index. That is, (1) we have index entries that say that the worktree is\n> > > ignored, and the rules don't say they're outside the sparse checkout; do\n> > > we care whether we expect the worktree to be empty or match the index?\n> > > And, (2) we have index entries that say we do care about them, but the\n> > > rules say they're outside the sparse checkout; what happens with these?\n> >\n> > The rule is CE_NO_CHECKOUT is king. core.defaultsparse only helps\n> > setting CE_NO_CHECKOUT on new entries when they enter the index.\n>\n>\n> This seems like a really bad idea to me. If you ask for a file that's\n> outside your default area to be checked out, and then you switch branches\n> and switch back, the file may or may not disappear (depending on whether\n> the branch you switched to temporarily had it or not). Likewise, if you\n> remove files, and then switch branches and back, the files may or may not\n> reappear.\n\nWell, if you set core.defaultsparse properly, those files should\nappear/disappear as you wish (and as of now if you define your\ncheckout area with \"git checkout --{include-,exclude-,}sparse\" then\ncore.defaultsparse should be updated accordingly). I don't say\ncore.defaultsparse is perfect.\n\nAnyway how do you suppose the tool to do in your case (checkout,\nswitch away then switch back)?\n-- \nDuy\n"},{"id":"97617","messageId":"alpine.LNX.1.00.0812111520490.19665@iabervon.org","threadId":"16487","inReplyTo":"fcaeb9bf0812110504u1acfb612he3edae1df3774045@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-12-11T20:30:45Z","receivedAt":"2008-12-11T20:30:45Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 11 Dec 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On 12/9/08, Daniel Barkalow wrote:\n> > > - for \"git grep\", we ignore path with CE_NO_CHECKOUT (while using\n> > > cache version for CE_VALID)\n> >\n> >\n> > Is this sufficient? I'd expect \"git grep\" to ignore paths that are outside\n> > the checked-out region, even when searching an arbitrary tree, and even\n> > when those files aren't in the index at all (i.e., the current commit\n> > doesn't have them). That is, I'd expect core.defaultsparse or the\n> > equivalent to limit the paths, normally giving this effect.\n> \n> That's the point. CE_VALID does not define checkout area while\n> CE_NO_CHECKOUT does. If an entry is CE_VALID, it is still in checkout\n> area. But if it is CE_NO_CHECKOUT, \"git grep\" should ignore that path.\n> core.defaultsparse has nothing to do here.\n\nMy point is that the index cannot tell git grep whether it should search a \npath if the path isn't in the index. If I do a narrow checkout of only \nDocumentation/, and I do \"git grep foo\", I won't see files that aren't in \nDocumentation/; if I do \"git grep foo origin/next\", I think I shouldn't \nsee files that aren't in Documentation/, and \"new-program.c\" isn't in my \nindex at all, marked as CE_NO_CHECKOUT or otherwise, so git grep can't \nfind out from the index whether that file is outside my area of interest. \nIt needs to be able to determine that \"only Documentation/ is in the \ncheckout area\" ignoring the details of the list of files in the working \ndirectory currently in or out of the area.\n\n> > > > The question, then, is what happens when the index and core.defaultsparse\n> > > > disagree, either because the porcelain supports causing it or because the\n> > > > user has simply editting the config file or used plumbing to modify the\n> > > > index. That is, (1) we have index entries that say that the worktree is\n> > > > ignored, and the rules don't say they're outside the sparse checkout; do\n> > > > we care whether we expect the worktree to be empty or match the index?\n> > > > And, (2) we have index entries that say we do care about them, but the\n> > > > rules say they're outside the sparse checkout; what happens with these?\n> > >\n> > > The rule is CE_NO_CHECKOUT is king. core.defaultsparse only helps\n> > > setting CE_NO_CHECKOUT on new entries when they enter the index.\n> >\n> >\n> > This seems like a really bad idea to me. If you ask for a file that's\n> > outside your default area to be checked out, and then you switch branches\n> > and switch back, the file may or may not disappear (depending on whether\n> > the branch you switched to temporarily had it or not). Likewise, if you\n> > remove files, and then switch branches and back, the files may or may not\n> > reappear.\n> \n> Well, if you set core.defaultsparse properly, those files should\n> appear/disappear as you wish (and as of now if you define your\n> checkout area with \"git checkout --{include-,exclude-,}sparse\" then\n> core.defaultsparse should be updated accordingly). I don't say\n> core.defaultsparse is perfect.\n\nRight, so in order to get reasonable behavior, the user must use \n--{include,exclude}-sparse. I think that this should be the *default* \nbehavior, and probably the *only porcelain-supported* behavior, because \notherwise it's confusing.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"97653","messageId":"7vy6ym9nm8.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812111520490.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-12T01:41:03Z","receivedAt":"2008-12-12T01:41:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n>> That's the point. CE_VALID does not define checkout area while\n>> CE_NO_CHECKOUT does. If an entry is CE_VALID, it is still in checkout\n>> area. But if it is CE_NO_CHECKOUT, \"git grep\" should ignore that path.\n>> core.defaultsparse has nothing to do here.\n>\n> My point is that the index cannot tell git grep whether it should search a \n> path if the path isn't in the index.\n\nLet's step back a bit. I think \"git grep\" that stays silent outside of\nthe checkout area when used to grep in the work tree or in the index is a\nmistake.\n\nThe problem \"sparse checkout\" attempts to address is not this:\n\n I ran \"git init && git add .\" in /usr/src by mistake. There is no\n reason for coreutils that is in /usr/src/coreutils and gnucash that is\n in /usr/src/gnucash to share the same development history nor their\n should be any ordering between commits in these two independent\n projects. I should have done N separate \"init & add\" independently at\n one level deeper in the directory hierarchy, but I am too lazy to\n filter branch the resulting mess now.\n\nAt least, it should not be that, at least to me.\n\n\"Sparse\" is \"I am not going to modify the files in these areas, and I know\nthey do not need to be present for my purposes (e.g. build), so I do not\nneed copies in the work tree.\" It still works on the whole tree structure\nrecorded in the commit, but gives you a way to work inside a sparsely\npopulated work tree, iow, without checking everything out.\n\nSo \"git grep -e frotz Documentation/\", whether you only check out\nDocumentation or the whole tree, should grep only in Documentation area,\nand \"git grep -e frotz\" should grep in the whole tree, even if you happen\nto have a sparse checkout. By definition, a sparse checkout has no\nmodifications outside the checkout area, so whenever grep wants to look\nfor strings outside the checkout area it should pretend as if the same\ncontent as what the index records is in the work tree. This is consistent\nwith the way how \"git diff\" in a sparsely checked out work tree should\nbehave.\n\nIf you understand that, it is clear what \"git grep -e frotz HEAD^\" should\ndo. No checkout area is involved.\n"},{"id":"97660","messageId":"alpine.LNX.1.00.0812112045120.19665@iabervon.org","threadId":"16487","inReplyTo":"7vy6ym9nm8.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-12-12T02:40:47Z","receivedAt":"2008-12-12T02:40:47Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 11 Dec 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> >> That's the point. CE_VALID does not define checkout area while\n> >> CE_NO_CHECKOUT does. If an entry is CE_VALID, it is still in checkout\n> >> area. But if it is CE_NO_CHECKOUT, \"git grep\" should ignore that path.\n> >> core.defaultsparse has nothing to do here.\n> >\n> > My point is that the index cannot tell git grep whether it should search a \n> > path if the path isn't in the index.\n> \n> Let's step back a bit. I think \"git grep\" that stays silent outside of\n> the checkout area when used to grep in the work tree or in the index is a\n> mistake.\n> \n> The problem \"sparse checkout\" attempts to address is not this:\n> \n> I ran \"git init && git add .\" in /usr/src by mistake. There is no\n> reason for coreutils that is in /usr/src/coreutils and gnucash that is\n> in /usr/src/gnucash to share the same development history nor their\n> should be any ordering between commits in these two independent\n> projects. I should have done N separate \"init & add\" independently at\n> one level deeper in the directory hierarchy, but I am too lazy to\n> filter branch the resulting mess now.\n> \n> At least, it should not be that, at least to me.\n> \n> \"Sparse\" is \"I am not going to modify the files in these areas, and I know\n> they do not need to be present for my purposes (e.g. build), so I do not\n> need copies in the work tree.\" It still works on the whole tree structure\n> recorded in the commit, but gives you a way to work inside a sparsely\n> populated work tree, iow, without checking everything out.\n\nThere's the meta question of: \"Do people who have declared that they \naren't going to modify or build with some files want their searches to \ntell them about those files?\"\n\nSay I'm the \"tr\" guy, and I care about the build system, library code, and \n\"tr.c\", and I run \"make tr\"; my sparse checkout doesn't include \"head.c\", \nand I totally ignore all the other stuff that's in coreutils. Maybe I want \n\"git grep\" to exclude the other stuff.\n\nI don't really have a firm position on whether \"git grep\" should ignore \n\"head.c\" or not, but I think it should be consistent between \"git grep\" \nand \"git grep origin/next\", and I think that, if origin/next has a new \n\"foot.c\" that isn't in the current branch to by marked as NO_CHECKOUT, it \nshould be skipped if \"tail.c\" (which is in my current branch) is skipped.\n\n> So \"git grep -e frotz Documentation/\", whether you only check out\n> Documentation or the whole tree, should grep only in Documentation area,\n> and \"git grep -e frotz\" should grep in the whole tree, even if you happen\n> to have a sparse checkout. By definition, a sparse checkout has no\n> modifications outside the checkout area, so whenever grep wants to look\n> for strings outside the checkout area it should pretend as if the same\n> content as what the index records is in the work tree. This is consistent\n> with the way how \"git diff\" in a sparsely checked out work tree should\n> behave.\n\n\"git diff\" is an ambiguous model for \"git grep\". It equally describes \nthe behavior of \"git diff\" to say that it treats files outside the \ncheckout area as matching the index or to say that it never lists files \noutside the checkout area. On the other hand, there is the question of \nwhether \"git diff branch1 branch2\" shows differences that are outside the \ncheckout area, and whether \"git log\" shows commits that only change things \noutside the checkout area, and \"git grep\" should match the behavior of \nthese.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"97680","messageId":"7vmyf29jd6.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812112045120.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-12T03:12:53Z","receivedAt":"2008-12-12T03:12:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> \"git diff\" is an ambiguous model for \"git grep\". It equally describes \n> the behavior of \"git diff\" to say that it treats files outside the \n> checkout area as matching the index or to say that it never lists files \n> outside the checkout area. On the other hand, there is the question of \n> whether \"git diff branch1 branch2\" shows differences that are outside the \n> checkout area, and whether \"git log\" shows commits that only change things \n> outside the checkout area, and \"git grep\" should match the behavior of \n> these.\n\nSure, but as \"sparse\" does not (again, \"it should not, at least to me\")\nchange the fact that git is about tracking the history of whole tree, not\njust a single file, nor just a subset of files, none of these operations\nshould be affected about what the checkout area is.\n"},{"id":"97682","messageId":"20081212033659.GB29663@coredump.intra.peff.net","threadId":"16487","inReplyTo":"7vmyf29jd6.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-12T03:36:59Z","receivedAt":"2008-12-12T03:36:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 11, 2008 at 07:12:53PM -0800, Junio C Hamano wrote:\n\n> Sure, but as \"sparse\" does not (again, \"it should not, at least to me\")\n> change the fact that git is about tracking the history of whole tree, not\n> just a single file, nor just a subset of files, none of these operations\n> should be affected about what the checkout area is.\n\nI agree with Junio here. If you want \"git grep foo HEAD^\" to ignore\ncertain files, then sparse _checkout_ is not the right feature. In that\ncase you want a sparse _repo_, which is not something I think anybody is\nseriously working on.\n\n-Peff\n"},{"id":"97706","messageId":"fcaeb9bf0812120808y656c0c6bx9d1c44ea00aeb7b9@mail.gmail.com","threadId":"16487","inReplyTo":"alpine.LNX.1.00.0812111520490.19665@iabervon.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-12T16:08:39Z","receivedAt":"2008-12-12T16:08:39Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/12/08, Daniel Barkalow wrote:\n> > Well, if you set core.defaultsparse properly, those files should\n> > appear/disappear as you wish (and as of now if you define your\n> > checkout area with \"git checkout --{include-,exclude-,}sparse\" then\n> > core.defaultsparse should be updated accordingly). I don't say\n> > core.defaultsparse is perfect.\n>\n>\n> Right, so in order to get reasonable behavior, the user must use\n> --{include,exclude}-sparse. I think that this should be the *default*\n> behavior, and probably the *only porcelain-supported* behavior, because\n> otherwise it's confusing.\n\nIt's pretty hard (or intrusive) to enforce such behaviour. How about\nshowing files that does not match core.defaultsparse in \"git status\"\nalong with instructions how to add them to core.defaultsparse? That\nway people can keep it consistent and less modification to current\ncode.\n-- \nDuy\n"},{"id":"97707","messageId":"fcaeb9bf0812120813m2949e36ar7905d5688b8f6ecb@mail.gmail.com","threadId":"16487","inReplyTo":"7vy6ym9nm8.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-12T16:13:27Z","receivedAt":"2008-12-12T16:13:27Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/12/08, Junio C Hamano wrote:\n> So \"git grep -e frotz Documentation/\", whether you only check out\n> Documentation or the whole tree, should grep only in Documentation area,\n> and \"git grep -e frotz\" should grep in the whole tree, even if you happen\n> to have a sparse checkout. By definition, a sparse checkout has no\n> modifications outside the checkout area, so whenever grep wants to look\n> for strings outside the checkout area it should pretend as if the same\n> content as what the index records is in the work tree. This is consistent\n> with the way how \"git diff\" in a sparsely checked out work tree should\n> behave.\n\nAssume someone is using sparse checkout with KDE git repository. They\nsparse-checkout kdeutils module and do \"git grep -e foo\". I would\nexpect that the command only searches in kdeutils only (and is the\ncurrent behavior).\n-- \nDuy\n"},{"id":"97710","messageId":"4942952E.1060706@viscovery.net","threadId":"16487","inReplyTo":"fcaeb9bf0812120813m2949e36ar7905d5688b8f6ecb@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-12-12T16:45:34Z","receivedAt":"2008-12-12T16:45:34Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Nguyen Thai Ngoc Duy schrieb:\n> On 12/12/08, Junio C Hamano wrote:\n>> So \"git grep -e frotz Documentation/\", whether you only check out\n>> Documentation or the whole tree, should grep only in Documentation area,\n>> and \"git grep -e frotz\" should grep in the whole tree, even if you happen\n>> to have a sparse checkout. By definition, a sparse checkout has no\n>> modifications outside the checkout area, so whenever grep wants to look\n>> for strings outside the checkout area it should pretend as if the same\n>> content as what the index records is in the work tree. This is consistent\n>> with the way how \"git diff\" in a sparsely checked out work tree should\n>> behave.\n> \n> Assume someone is using sparse checkout with KDE git repository. They\n> sparse-checkout kdeutils module and do \"git grep -e foo\". I would\n> expect that the command only searches in kdeutils only (and is the\n> current behavior).\n\nBut what if the same persion notices a #define in a kdeutils header file\nand want's to know whether it is unused in order to remove it:\n\n $ git grep FOO\n kdeutils/foo.h:#define FOO bar\n\nConclusion from this output: \"It's only defined, but not used anywhere.\"\nBut this conclusion is not necessarily correct because FOO could be used\noutside kdeutils.\n\nSo, no, \"git grep\" should disregard the checkout area.\n\n-- Hannes\n"},{"id":"97712","messageId":"fcaeb9bf0812120854k1c366327o9bc696184ea4f02e@mail.gmail.com","threadId":"16487","inReplyTo":"4942952E.1060706@viscovery.net","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-12-12T16:54:43Z","receivedAt":"2008-12-12T16:54:43Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/12/08, Johannes Sixt wrote:\n> Nguyen Thai Ngoc Duy schrieb:\n>\n> > On 12/12/08, Junio C Hamano wrote:\n> >> So \"git grep -e frotz Documentation/\", whether you only check out\n> >> Documentation or the whole tree, should grep only in Documentation area,\n> >> and \"git grep -e frotz\" should grep in the whole tree, even if you happen\n> >> to have a sparse checkout. By definition, a sparse checkout has no\n> >> modifications outside the checkout area, so whenever grep wants to look\n> >> for strings outside the checkout area it should pretend as if the same\n> >> content as what the index records is in the work tree. This is consistent\n> >> with the way how \"git diff\" in a sparsely checked out work tree should\n> >> behave.\n> >\n> > Assume someone is using sparse checkout with KDE git repository. They\n> > sparse-checkout kdeutils module and do \"git grep -e foo\". I would\n> > expect that the command only searches in kdeutils only (and is the\n> > current behavior).\n>\n>\n> But what if the same persion notices a #define in a kdeutils header file\n> and want's to know whether it is unused in order to remove it:\n>\n> $ git grep FOO\n> kdeutils/foo.h:#define FOO bar\n\n\"git grep --cached FOO\" ?\n\n> Conclusion from this output: \"It's only defined, but not used anywhere.\"\n> But this conclusion is not necessarily correct because FOO could be used\n> outside kdeutils.\n>\n> So, no, \"git grep\" should disregard the checkout area.\n>\n> -- Hannes\n>\n\n\n-- \nDuy\n"},{"id":"97781","messageId":"7vvdto7hcg.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"fcaeb9bf0812120854k1c366327o9bc696184ea4f02e@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-13T05:51:43Z","receivedAt":"2008-12-13T05:51:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nguyen Thai Ngoc Duy\" writes:\n\n> On 12/12/08, Johannes Sixt wrote:\n> ...\n>> But what if the same persion notices a #define in a kdeutils header file\n>> and want's to know whether it is unused in order to remove it:\n>>\n>> $ git grep FOO\n>> kdeutils/foo.h:#define FOO bar\n>\n> \"git grep --cached FOO\" ?\n\nThat should behave identically when the work tree does not have change\nsince the index, and by definition paths outside the checkout area in the\n\"sparse\" mode cannot have changes, so \"git grep FOO\" should behave the\nsame and should find it.\n"},{"id":"97782","messageId":"7vprjw7hc2.fsf@gitster.siamese.dyndns.org","threadId":"16487","inReplyTo":"fcaeb9bf0812120813m2949e36ar7905d5688b8f6ecb@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-13T05:51:57Z","receivedAt":"2008-12-13T05:51:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nguyen Thai Ngoc Duy\" writes:\n\n> On 12/12/08, Junio C Hamano wrote:\n>> So \"git grep -e frotz Documentation/\", whether you only check out\n>> Documentation or the whole tree, should grep only in Documentation area,\n>> and \"git grep -e frotz\" should grep in the whole tree, even if you happen\n>> to have a sparse checkout. By definition, a sparse checkout has no\n>> modifications outside the checkout area, so whenever grep wants to look\n>> for strings outside the checkout area it should pretend as if the same\n>> content as what the index records is in the work tree. This is consistent\n>> with the way how \"git diff\" in a sparsely checked out work tree should\n>> behave.\n>\n> Assume someone is using sparse checkout with KDE git repository. They\n> sparse-checkout kdeutils module and do \"git grep -e foo\". I would\n> expect that the command only searches in kdeutils only (and is the\n> current behavior).\n\nYes it is the \"current in next\" behaviour, and no that is not what you\nshould expect, and that is why I earlier said it is a mistake. The\nability to choose which part to leave out of the working tree should not\nchange the fact that git is about managing the history of the whole tree,\nnot an individual file nor a subset of files.\n\nI do not think it is unreasonable to have a mechanism to let the user\nlimit the area of the whole tree often used Porcelain commands look at.\nWe already have pathspec \"git grep -e foo kdeutils/\" mechanism that lets\nyou do such limiting. It is conceivable that some workflows _might_ find\nhaving the default pathspec convenient in end-user initiated operations,\nbut then it would be convenient whether the end-user uses the sparse\ncheckout to limit the area to kdeutils/ or has the whole checkout.\nAlthough I think it would be Okay to default the default pathspec match\nthe checkout area when the sparse checkout feature is in use, I think the\n\"checkout area\" and \"area of interest\" should be two independent concepts.\n\nI said \"_might_\" in the above because I do not think it is such a good\nidea to have _the_ default pathspec to begin with, though. It would\nprobably be more useful to allow people to use shorthands to pathspecs,\nand at that point you can use usual shell variables to do that already,\ne.g. instead of having to say \"git grep -e foo arch/x86 include/asm-i386\",\nyou would say \"git grep -e foo $i386\", after \"i386=arch/x86 include/asm-i386\",\nor something like that.\n"}]}