{"thread":{"id":"21958","subject":"[PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'","startedAt":"2009-12-15T08:42:06Z","lastAt":"2009-12-19T10:56:35Z","messageCount":21,"participants":["Johannes Sixt","Johannes Schindelin","Junio C Hamano","Sverre Rabbelier","Jakub Narebski","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"129943","messageId":"4B274BDE.8000504@viscovery.net","threadId":"21958","inReplyTo":null,"subject":"[PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-12-15T08:42:06Z","receivedAt":"2009-12-15T08:42:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nThe intent of this particular call to 'git read-tree' was to fill an\nindex. But in fact, it only allocated an empty index. Later in the\nprogram, the index is filled anyway by calling read-tree with specific\ncommits, and considering that elsewhere the index is even removed (i.e.,\nit is not relied upon that the index file exists), this first call of\nread-tree is completely redundant.\n\nSigned-off-by: Johannes Sixt \n---\n Calling read-tree without arguments is not allowed according to the\n documentation. The next patch will enforce this.\n\n -- Hannes\n\n git-filter-branch.sh | 1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex cb9d202..195b5ef 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -259,7 +259,6 @@ test -s \"$tempdir\"/heads ||\n\n GIT_INDEX_FILE=\"$(pwd)/../index\"\n export GIT_INDEX_FILE\n-git read-tree || die \"Could not seed the index\"\n\n # map old->new commit ids for rewriting parents\n mkdir ../map || die \"Could not create map/ directory\"\n-- \n1.6.6.rc1.46.g1635\n"},{"id":"129944","messageId":"4B274C3A.4060808@viscovery.net","threadId":"21958","inReplyTo":"4B274BDE.8000504@viscovery.net","subject":"[PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-12-15T08:43:38Z","receivedAt":"2009-12-15T08:43:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nPreviously, it was possible to run read-tree without any arguments,\nwhereupon it purged the index!\n\nSigned-off-by: Johannes Sixt \n---\n Discovered by typing\n\n git ..daab02\n\n with help.autocorrect > 0 :-)\n\n -- Hannes\n\n builtin-read-tree.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 50413ca..31623b9 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -125,6 +125,9 @@ int cmd_read_tree(int argc, const char **argv,\n \t\tstage = opts.merge = 1;\n \t}\n\n+\tif (argc == 0)\n+\t\tusage_with_options(read_tree_usage, read_tree_options);\n+\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n\n-- \n1.6.6.rc1.46.g1635\n"},{"id":"129952","messageId":"alpine.DEB.1.00.0912151819010.4985@pacific.mpi-cbg.de","threadId":"21958","inReplyTo":"4B274BDE.8000504@viscovery.net","subject":"Re: [PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-12-15T17:19:16Z","receivedAt":"2009-12-15T17:19:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 15 Dec 2009, Johannes Sixt wrote:\n\n> From: Johannes Sixt \n> \n> The intent of this particular call to 'git read-tree' was to fill an\n> index. But in fact, it only allocated an empty index. Later in the\n> program, the index is filled anyway by calling read-tree with specific\n> commits, and considering that elsewhere the index is even removed (i.e.,\n> it is not relied upon that the index file exists), this first call of\n> read-tree is completely redundant.\n\nYes, indeed.\n\nCiao,\nDscho\n"},{"id":"129970","messageId":"7v1vivydlg.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"4B274BDE.8000504@viscovery.net","subject":"Re: [PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-16T00:19:55Z","receivedAt":"2009-12-16T00:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> From: Johannes Sixt \n>\n> The intent of this particular call to 'git read-tree' was to fill an\n> index. But in fact, it only allocated an empty index. Later in the\n> program, the index is filled anyway by calling read-tree with specific\n> commits, and considering that elsewhere the index is even removed (i.e.,\n> it is not relied upon that the index file exists), this first call of\n> read-tree is completely redundant.\n>\n> Signed-off-by: Johannes Sixt \n\nVery true. The only thing it would have done is to error out early when\nthe user mistakenly tried to run the command in a directory in which s/he\ndoes not have write access to, before running potentially expensive\ncommit listing with rev-list, but then the user would have failed to\ncreate the tempdir before that to begin with.\n\nWill queue, but it doesn't seem urgent to put it in 1.6.6 or 1.6.5.X\nmaint (i.e. nothing is broken without this patch).\n\n> Calling read-tree without arguments is not allowed according to the\n> documentation.\n\nI think saying \"is not allowed\" is going a bit too far. The documentation\nsimply does not list it as a _useful_ thing to do, that's all.\n\nBy the way, I notice that the command still insists on being run in a\nclean work tree with a clean index at the beginning, but isn't everything\ndone inside the tempdir (i.e. \".git-rewrite\" by default) these days,\nincluding the temporary work tree tree-filter creates with \"checkout-index\n-fqua\"? It is obviously not a topic of this patch, but we may want to\nstop doing that if we are not rewriting the current commit (which we will\nknow by the time we list the commits to be rewritten with rev-list before\nactually starting to rewrite).\n"},{"id":"130067","messageId":"4B2B5085.1050906@viscovery.net","threadId":"21958","inReplyTo":"4B274C3A.4060808@viscovery.net","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-12-18T09:51:01Z","receivedAt":"2009-12-18T09:51:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"You have queued 1/2 (filter-branch: remove an unnecessary use of\n'git read-tree') of this 2-patch series, but I haven't seen any comments\nabout this 2/2 nor is it queued. Did it fall through the cracks?\n\n--- 8< ---\nFrom: Johannes Sixt \nDate: Tue, 15 Dec 2009 09:21:53 +0100\nSubject: [PATCH] read-tree: at least one tree-ish argument is required\n\nPreviously, it was possible to run read-tree without any arguments,\nwhereupon it purged the index!\n\nSigned-off-by: Johannes Sixt \n---\n builtin-read-tree.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 50413ca..31623b9 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -125,6 +125,9 @@ int cmd_read_tree(int argc, const char **argv,\n \t\tstage = opts.merge = 1;\n \t}\n\n+\tif (argc == 0)\n+\t\tusage_with_options(read_tree_usage, read_tree_options);\n+\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n\n-- \n1.6.6.rc3.54.g0f72d\n"},{"id":"130082","messageId":"7v4onoywws.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"4B2B5085.1050906@viscovery.net","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-18T18:11:47Z","receivedAt":"2009-12-18T18:11:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> You have queued 1/2 (filter-branch: remove an unnecessary use of\n> 'git read-tree') of this 2-patch series, but I haven't seen any comments\n> about this 2/2 nor is it queued. Did it fall through the cracks?\n\nNo. I think saying \"is not allowed\" is going a bit too far [*1*]. The\ndocumentation does not list it as a commonly useful thing, that's all.\n\nWhen a user wants to have an empty index, and does not want to touch files\nunder $GIT_DIR with any non-git tools (e.g. \"rm -f $GIT_DIR/index) for\nsome religious reasons, \"read-tree\" without a tree would be one valid way\nto do so [*2*]. That is not documented (the synopsis section even hints\nthat read-tree wants to take at least one tree), and I think it is a\nproblem that the documentation does not match what the program actually\nallows.\n\nThe approach taken by [2/2] is to match the program to the documentation\nby forbidding what has been allowed and what people may have been relying\non being able to do,\n\nIt was obvious that the line removed by [1/2] was a no-op not only in the\nusual case but also in an error case (i.e. when the user does not have\nwrite access to the repository), as I wrote in my review of the patch.\nCompared to that, I do not think it is cut-and-dried clear that [2/2] is\nthe right kind of improvement. An alternative approach to document the\nzero-tree case would be a valid way to address the documentation mismatch\nproblem.\n\nWe need to make arguments like \"'read-tree' given by mistake to empty the\nindex is risky\", \"'read-tree' as 'rm -f $GIT_DIR/index' replacement has\nlittle value\", and \"'read-tree' as 'rm -f $GIT_DIR/index' replacement is\nthe best way to get an empty index\" to weigh pros and cons between two\npossible approaches before deciding which way to go, but in a pre-release\nfreeze, I wasn't in the mood to be one who is doing the arguments myself.\n\n\n[Footnote]\n\n*1* Didn't I say that somewhere?\n\n*2* I suspect \"rm --cached .\" might be another, but it would probably be\nmuch more expensive.\n"},{"id":"130087","messageId":"200912182004.25520.j.sixt@viscovery.net","threadId":"21958","inReplyTo":"7v4onoywws.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-12-18T19:04:25Z","receivedAt":"2009-12-18T19:04:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Freitag, 18. Dezember 2009, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> > You have queued 1/2 (filter-branch: remove an unnecessary use of\n> > 'git read-tree') of this 2-patch series, but I haven't seen any comments\n> > about this 2/2 nor is it queued. Did it fall through the cracks?\n>\n> No. I think saying \"is not allowed\" is going a bit too far [*1*].\n\nYes, you said that, but in response to the footnote in 1/2.\n\n> The \n> documentation does not list it as a commonly useful thing, that's all.\n\nIMO, not only is it not useful, but it is also dangerous - it erases the \nindex!\n\n> When a user wants to have an empty index, and does not want to touch files\n> under $GIT_DIR with any non-git tools (e.g. \"rm -f $GIT_DIR/index) for\n> some religious reasons, \"read-tree\" without a tree would be one valid way\n> to do so [*2*].\n> *2* I suspect \"rm --cached .\" might be another, but it would probably be\n> much more expensive.\n\nFor an operation like this, shouldn't we advocate this alternate instruction \n(which explicitly tells what is wanted) rather than the implicit and \nundocumented operation of parameter-less read-tree?\n\n> We need to make arguments like \"'read-tree' given by mistake to empty the\n> index is risky\", \"'read-tree' as 'rm -f $GIT_DIR/index' replacement has\n> little value\", and \"'read-tree' as 'rm -f $GIT_DIR/index' replacement is\n> the best way to get an empty index\" to weigh pros and cons between two\n> possible approaches before deciding which way to go, but in a pre-release\n> freeze, I wasn't in the mood to be one who is doing the arguments myself.\n\nSorry to drag you into this discussion, but I felt this change is maint-worthy \n(because the behavior is not only risky, but dangerous).\n\n-- Hannes\n"},{"id":"130089","messageId":"fabb9a1e0912181124nef40a2dxac1b93f9dcc19bb8@mail.gmail.com","threadId":"21958","inReplyTo":"200912182004.25520.j.sixt@viscovery.net","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-18T19:24:32Z","receivedAt":"2009-12-18T19:24:32Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 18, 2009 at 13:04, Johannes Sixt wrote:\n> Sorry to drag you into this discussion, but I felt this change is maint-worthy\n> (because the behavior is not only risky, but dangerous).\n\nI agree, shouldn't such a dangerous function at least require a flag?\n'git read-tree --empty' or something?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"130090","messageId":"7voclwxemf.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"200912182004.25520.j.sixt@viscovery.net","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-18T19:32:08Z","receivedAt":"2009-12-18T19:32:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Sorry to drag you into this discussion, but I felt this change is\n> maint-worthy (because the behavior is not only risky, but dangerous).\n>\n> -- Hannes\n\nIn short, that is where we differ, as I don't think it is dangerous at\nall in the \"maint-worthy\" sense. \n\n\"read-tree\" without -m is to \"populate the index from emptiness with given\ntrees\". Unless you are hit by the bug in the auto-correction (whose fix\nwas maint-worthy), nobody would say read-tree without parameter and expect\nthat it wouldn't touch the index.\n\nSure, it will empty the index, so it is dangerous in the same sense that\n\"reset --hard\" is dangerous because it will wipe all your local changes,\nor \"rm -rf it\" will remove everything underneath it. But these are all\nfeatures that are \"dangerous if you didn't mean to do so but wanted to do\nsomething else.\"\n\nRemoval of such features might turn out to be maint-worthy but \n\n Previously, it was possible to run read-tree without any arguments,\n whereupon it purged the index!\n\ndoesn't justify it well enough.\n"},{"id":"130091","messageId":"fabb9a1e0912181137t1265f86cs9e02019da136a0a@mail.gmail.com","threadId":"21958","inReplyTo":"7voclwxemf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-18T19:37:03Z","receivedAt":"2009-12-18T19:37:03Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 18, 2009 at 13:32, Junio C Hamano wrote:\n> Sure, it will empty the index, so it is dangerous in the same sense that\n> \"reset --hard\" is dangerous because it will wipe all your local changes,\n> or \"rm -rf it\" will remove everything underneath it.\n\nWith the difference that both 'reset --hard' and 'rm -rf' need a flag\nto do their destructive work? Although 'git reset' might be just as\ndestructive if you've been using 'git add -p' a lot or something,\nmhh...\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"130092","messageId":"7veimsvz8a.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"fabb9a1e0912181137t1265f86cs9e02019da136a0a@mail.gmail.com","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-18T19:49:57Z","receivedAt":"2009-12-18T19:49:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Fri, Dec 18, 2009 at 13:32, Junio C Hamano wrote:\n>> Sure, it will empty the index, so it is dangerous in the same sense that\n>> \"reset --hard\" is dangerous because it will wipe all your local changes,\n>> or \"rm -rf it\" will remove everything underneath it.\n>\n> With the difference that both 'reset --hard' and 'rm -rf' need a flag\n> to do their destructive work? Although 'git reset' might be just as\n> destructive if you've been using 'git add -p' a lot or something,\n> mhh...\n\nI'll grant you that at least \"rm -rf it\" names \"it\" that will be wiped\nvery explicitly. But just like the index and the work tree plus the index\nare the implicit targets to \"reset\" and \"reset --hard\" respectively, the\nindex is the implicit target to \"read-tree\".\n\nSo it may be \"dangerous\" in the sense that \"it would change things and if\nyou meant to do something else the end result would be different from what\nyou wanted to do\". In that sense, \"log\", \"cat-file\" and friends may be\ndanger-free commands and all others would be dangerous. You might type\n\"commit\" when you meant to say \"commit -a\" and record an incomplete state;\nit is \"dangerous\" in that sense.\n\nThese are part of their feature.\n"},{"id":"130093","messageId":"fabb9a1e0912181159u65ae9c6fg1481ab744575475f@mail.gmail.com","threadId":"21958","inReplyTo":"7veimsvz8a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-18T19:59:50Z","receivedAt":"2009-12-18T19:59:50Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 18, 2009 at 13:49, Junio C Hamano wrote:\n> You might type\n> \"commit\" when you meant to say \"commit -a\" and record an incomplete state;\n> it is \"dangerous\" in that sense.\n\nSpeaking of which, it has hit me multiple times that I craft out a\ncommit with 'git add -p' and then do \"git commit -am 'foo some bars'\"\nand lose all my hard work (because I'm used to typing 'git commit -am'\nfor temporary commits). I'd be happy if \"git commit -am\" learned to\nsecond-guess me when I already have something in the index.\n\n> These are part of their feature.\n\nFair enough, then perhaps it is time for \"core.nodataloss\" which\neither logs states to a seperate reflog (so that you can go back to\nthe state you were in before doing 'git read-tree') or interactively\ninforms the user that this will command will result in data loss\n(although that sounds a tad too much like Window's \"Are you sure?\"\ndialogs).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"130094","messageId":"7vy6l0ujkx.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"fabb9a1e0912181159u65ae9c6fg1481ab744575475f@mail.gmail.com","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-18T20:13:18Z","receivedAt":"2009-12-18T20:13:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Fri, Dec 18, 2009 at 13:49, Junio C Hamano wrote:\n>> You might type\n>> \"commit\" when you meant to say \"commit -a\" and record an incomplete state;\n>> it is \"dangerous\" in that sense.\n>\n> Speaking of which, it has hit me multiple times that I craft out a\n> commit with 'git add -p' and then do \"git commit -am 'foo some bars'\"\n> and lose all my hard work (because I'm used to typing 'git commit -am'\n> for temporary commits). I'd be happy if \"git commit -am\" learned to\n> second-guess me when I already have something in the index.\n\nSounds like \"commit.confirm = xxx\" configuration patches are coming? What\nother questionable operations we might want to let users configure git to\nask for confirmation?\n\n> Fair enough, then perhaps it is time for \"core.nodataloss\" which\n> either logs states to a seperate reflog (so that you can go back to\n> the state you were in before doing 'git read-tree') or interactively\n> informs the user that this will command will result in data loss\n> (although that sounds a tad too much like Window's \"Are you sure?\"\n> dialogs).\n\nI somehow suspect that you had your morning coffee yet ;-) Aren't we\ntalking about the index, and why are you bringing up the reflog?\n\nMore importantly, if you accept that there are non-interrogator commands\nin the git command set, I somehow suspect that you will soon realize that\n\"git config core.nodataloss true\" is equivalent to \"chmod a-w -R .\". It\nmight be useful mode of non-operation (read-only historical archive) but I\ndo not think it deserves a configuration of its own with checks in the\ncode all over the place that might possibly change any states of the\nrepository.\n\n\"git config user.novice true\" to increase the verbosity and degree of\nhand-holding is an entirely different matter, but if that is what you are\nadvocating, you shouldn't call it \"core.nodataloss\".\n"},{"id":"130095","messageId":"fabb9a1e0912181221i31c32a76n50581adfb2185037@mail.gmail.com","threadId":"21958","inReplyTo":"7vy6l0ujkx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-18T20:21:49Z","receivedAt":"2009-12-18T20:21:49Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 18, 2009 at 14:13, Junio C Hamano wrote:\n> Sounds like \"commit.confirm = xxx\" configuration patches are coming?\n\nNot a bad idea.\n\n> What\n> other questionable operations we might want to let users configure git to\n> ask for confirmation?\n\nI don't see any other of the above category, that is, the category of\noperations that contain 'git commit -a' with a non-empty index, and\n'git rm' on a modified file (which we already prevent). But see below.\n\n> I somehow suspect that you haven't had your morning coffee yet ;-)\n> Aren't we talking about the index, and why are you bringing up the reflog?\n\nIt was a topic hijack of sorts, we were talking about data loss. I\nbrought up the reflog since I think that was mentioned before when we\ndiscussed ways to prevent data loss (e.g., create a commit of the\ncurrent state when doing 'git reset --hard' and record it in such a\nreflog).\n\n> More importantly, if you accept that there are non-interrogator commands\n> in the git command set, I somehow suspect that you will soon realize that\n> \"git config core.nodataloss true\" is equivalent to \"chmod a-w -R .\".\n\nI don't think that's quite right, since you don't lose any data if you\ndo only additive commands (e.g., create new commits, etc).\n\n> might be useful mode of non-operation (read-only historical archive) but I\n> do not think it deserves a configuration of its own with checks in the\n> code all over the place that might possibly change any states of the\n> repository.\n\nThat is not quite what I intended with 'core.preventdataloss' (which I\nagree is a bad name for what I intend with it). I meant for a\nconfiguration option which insures that all non-interrogation commands\nmake sure that what they throw away is recoverable (for example\nthrough the reflog).\n\n> \"git config user.novice true\" to increase the verbosity and degree of\n> hand-holding is an entirely different matter, but if that is what you are\n> advocating, you shouldn't call it \"core.nodataloss\".\n\nI'm not sure I'd call it \"user.novice\", since I don't consider myself\na novice user, and I would definitely like to be able to recover data\nthat I accidentally deleted with 'git reset --hard'.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"130102","messageId":"200912182304.27656.j.sixt@viscovery.net","threadId":"21958","inReplyTo":"7veimsvz8a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-12-18T22:04:27Z","receivedAt":"2009-12-18T22:04:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Freitag, 18. Dezember 2009, Junio C Hamano wrote:\n> I'll grant you that at least \"rm -rf it\" names \"it\" that will be wiped\n> very explicitly. But just like the index and the work tree plus the index\n> are the implicit targets to \"reset\" and \"reset --hard\" respectively, the\n> index is the implicit target to \"read-tree\".\n>\n> [...] You might type\n> \"commit\" when you meant to say \"commit -a\" and record an incomplete state;\n> it is \"dangerous\" in that sense.\n>\n> These are part of their feature.\n\nReally? \"rm -rf\", \"reset --hard\", \"commit -a\": yes, RTFM. But \"read-tree\" (w/o \narguments): no. There is no such sign in the documentation. Since the \noperation of the latter is dubious at best, I'd rather change the program \nthan the documentation.\n\nHow about this commit message, then?\n\nSubject: [PATCH] read-tree: at least one tree-ish argument is required\n\nRunning read-tree without any arguments purges the index, but this is not\ndocumented. This behavior is dubious at best because contrary to many\nother commands, it does not use HEAD if nothing else is specified.\n\nIf one really wants to clear the index, this can be achieved with\n'git rm --cached .' or 'rm -f .git/index' in a more explicit way.\n\nSigned-off-by: Johannes Sixt \n"},{"id":"130103","messageId":"m3d42cc4i8.fsf@localhost.localdomain","threadId":"21958","inReplyTo":"200912182304.27656.j.sixt@viscovery.net","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-12-18T22:17:15Z","receivedAt":"2009-12-18T22:17:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Johannes Sixt writes:\n\n> Running read-tree without any arguments purges the index, but this is not\n> documented. This behavior is dubious at best because contrary to many\n> other commands, it does not use HEAD if nothing else is specified.\n> \n> If one really wants to clear the index, this can be achieved with\n> 'git rm --cached .' or 'rm -f .git/index' in a more explicit way.\n\nOne can (I think) also always use \"git read-tree \",\nwhere = 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"130107","messageId":"7vr5qrsv4g.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"m3d42cc4i8.fsf@localhost.localdomain","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-18T23:46:55Z","receivedAt":"2009-12-18T23:46:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Johannes Sixt writes:\n>\n>> Running read-tree without any arguments purges the index, but this is not\n>> documented. This behavior is dubious at best because contrary to many\n>> other commands, it does not use HEAD if nothing else is specified.\n>> \n>> If one really wants to clear the index, this can be achieved with\n>> 'git rm --cached .' or 'rm -f .git/index' in a more explicit way.\n>\n> One can (I think) also always use \"git read-tree \",\n> where = 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n\nCome on. If you genuinely believe that\n\n $ git read-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n\nis a better way to purge the index than\n\n $ git read-tree\n\nthen you need to get your head examined. No, read-tree does not default\nto examine HEAD and that will not change ;-).\n\nBesides, read-tree is a plumbing.\n\nCome back with a proof that there has never existed any script that uses\n\"read-tree\" without arguments to purge the index, and I'd immediately\naccept and apply the patch to retroactively forbid what the implementation\nhas allowed users to do for a long time. Otherwise, I won't be involved\nin discussing this before the next release is cut, as a change like this\nneeds a reasonable transition strategy as usual, and needs to happen after\nthe next release.\n"},{"id":"130118","messageId":"20091219122509.6117@nanako3.lavabit.com","threadId":"21958","inReplyTo":"7vr5qrsv4g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-19T03:25:09Z","receivedAt":"2009-12-19T03:25:09Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano \n\n> Come back with a proof that there has never existed any script that uses\n> \"read-tree\" without arguments to purge the index, and I'd immediately\n> accept and apply the patch to retroactively forbid what the implementation\n> has allowed users to do for a long time.\n\nFor what it's worth, I compiled the very first version of git\n\n commit e83c5163316f89bfbde7d9ab23ca2e25604af290\n Author: Linus Torvalds \n Date: Thu Apr 7 15:13:13 2005 -0700\n\n Initial revision of \"git\", the information manager from hell\n\nand its read-tree fails with \n\n read-tree: read-tree \n\nIs it a proof enough?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"130120","messageId":"7veimrmv3r.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"20091219122509.6117@nanako3.lavabit.com","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-19T04:43:52Z","receivedAt":"2009-12-19T04:43:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Quoting Junio C Hamano \n>\n>> Come back with a proof that there has never existed any script that uses\n>> \"read-tree\" without arguments to purge the index, and I'd immediately\n>> accept and apply the patch to retroactively forbid what the implementation\n>> has allowed users to do for a long time.\n>\n> For what it's worth, I compiled the very first version of git\n>\n> commit e83c5163316f89bfbde7d9ab23ca2e25604af290\n> Author: Linus Torvalds \n> Date: Thu Apr 7 15:13:13 2005 -0700\n>\n> Initial revision of \"git\", the information manager from hell\n>\n> and its read-tree fails with \n>\n> read-tree: read-tree \n>\n> Is it a proof enough?\n\nOk, that initial one was \"read a single tree to populate the index\".\n\nI consider it a fundamentally different program from \"read-tree\" as we\nknow now, which was introduced by d99082e (Make \"read-tree\" merge the\ntrees it reads by giving them consecutive states., 2005-04-15). Ever\nsince that \"multi-stage\" version, read-tree was \"starting from an empty\nindex, read these trees into stages #1, #2, ...\" And even that version\ncalled the program \"read-tree\", not \"git read-tree\". IOW, \"git read-tree\"\nnever had that \"no tree is an error\" restriction during its entire life.\n\nHaving said all that, I don't care that deeply either way myself.\n\nAs read-tree is a very basic and low-level Porcelain, if somebody were\nusing it to empty the index in an existing script, this change would\nappear as a regression and hopefully will be caught eventually, and\nupdating such a script can be made reasonably easy if we want to be\nhelpful (the error message can suggest running \"rm $GIT_DIR/index\", for\nexample).\n"},{"id":"130121","messageId":"7vfx77lg2s.fsf@alter.siamese.dyndns.org","threadId":"21958","inReplyTo":"7veimrmv3r.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-19T04:53:47Z","receivedAt":"2009-12-19T04:53:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> As read-tree is a very basic and low-level Porcelain, if somebody were\n> using it to empty the index in an existing script, this change would\n> appear as a regression and hopefully will be caught eventually, and\n> updating such a script can be made reasonably easy if we want to be\n> helpful (the error message can suggest running \"rm $GIT_DIR/index\", for\n> example).\n\nIOW, I would prefer to queue something like this in the upcoming version,\nand then later make it die(). I do not think anybody relies on it, but we\nhave been wrong before. If the warning doesn't trigger for anybody, that\nis also fine as well.\n\n builtin-read-tree.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 2a3a32c..311c489 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -111,6 +111,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \targc = parse_options(argc, argv, unused_prefix, read_tree_options,\n \t\t\t read_tree_usage, 0);\n \n+\tif (!argc)\n+\t\twarning(\"running read-tree without argument to empty the index is deprecated.\");\n+\n \tnewfd = hold_locked_index(&lock_file, 1);\n \n \tprefix_set = opts.prefix ? 1 : 0;\n"},{"id":"130131","messageId":"alpine.DEB.1.00.0912191155360.4985@pacific.mpi-cbg.de","threadId":"21958","inReplyTo":"7vfx77lg2s.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-12-19T10:56:35Z","receivedAt":"2009-12-19T10:56:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 18 Dec 2009, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > As read-tree is a very basic and low-level Porcelain, if somebody were \n> > using it to empty the index in an existing script, this change would \n> > appear as a regression and hopefully will be caught eventually, and \n> > updating such a script can be made reasonably easy if we want to be \n> > helpful (the error message can suggest running \"rm $GIT_DIR/index\", \n> > for example).\n> \n> IOW, I would prefer to queue something like this in the upcoming \n> version, and then later make it die(). I do not think anybody relies on \n> it, but we have been wrong before. If the warning doesn't trigger for \n> anybody, that is also fine as well.\n\nI fully expect it not to trigger for anybody (except maybe you, if you \nhave hidden a script somewhere that uses read-tree to empty the index), \nbecause at least for this developer, the standard way of emptying the \nindex is simply \"rm .git/index\".\n\nCiao,\nDscho\n"}]}