{"thread":{"id":"25064","subject":"[PATCH] read-tree: abort if no trees are given","startedAt":"2010-09-10T10:06:04Z","lastAt":"2010-09-10T15:36:45Z","messageCount":5,"participants":["Jan Krüger","Johannes Sixt","Sverre Rabbelier","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"150430","messageId":"20100910120604.50aa1e94@jk.gs","threadId":"25064","inReplyTo":null,"subject":"[PATCH] read-tree: abort if no trees are given","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2010-09-10T10:06:04Z","receivedAt":"2010-09-10T10:06:04Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"Currently, read-tree silently accepts an invocation without any\ntree-ishs given and simply clobbers the index in that case. This\ncontradicts the usage synopsis and it's also probably not what anyone\nwould want to happen. So, instead, abort with a fatal error.\n\nSigned-off-by: Jan Krüger \n---\nSomeone in #git got confused by this.\n\n builtin/read-tree.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex 9ad1e66..67eb08e 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -166,6 +166,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\tdie(\"failed to unpack tree object %s\", arg);\n \t\tstage++;\n \t}\n+\tif (nr_trees == 0)\n+\t\tdie(\"no trees specified to read\");\n \tif (1 < opts.index_only + opts.update)\n \t\tdie(\"-u and -i at the same time makes no sense\");\n \tif ((opts.update||opts.index_only) && !opts.merge)\n-- \n1.7.2.3.392.g02377.dirty\n"},{"id":"150432","messageId":"4C8A168F.1030502@viscovery.net","threadId":"25064","inReplyTo":"20100910120604.50aa1e94@jk.gs","subject":"Re: [PATCH] read-tree: abort if no trees are given","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-09-10T11:29:19Z","receivedAt":"2010-09-10T11:29:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 9/10/2010 12:06, schrieb Jan Krüger:\n> Currently, read-tree silently accepts an invocation without any\n> tree-ishs given and simply clobbers the index in that case. This\n> contradicts the usage synopsis and it's also probably not what anyone\n> would want to happen. So, instead, abort with a fatal error.\n\nSee\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/135280/focus=135407\n\nand the discussion that ensued; perhaps intersting is\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/135280/focus=135462\n\n(where Junio suggest to make this a warning for now).\n\n-- Hannes\n"},{"id":"150434","messageId":"20100910152859.778636d4@jk.gs","threadId":"25064","inReplyTo":"4C8A168F.1030502@viscovery.net","subject":"[PATCH] read-tree: deprecate syntax without tree-ish args","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2010-09-10T13:28:59Z","receivedAt":"2010-09-10T13:28:59Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"Currently, read-tree can be run without tree-ish arguments, in which\ncase it will empty the index. Since this behavior is undocumented and\nperhaps a bit too invasive to be the \"default\" action for read-tree,\ndeprecate it in favor of a new --empty option that does the same thing.\n\nSigned-off-by: Jan Krüger \n---\nOn Fri, 10 Sep 2010 13:29:19 +0200, Johannes Sixt wrote:\n\n> See\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/135280/focus=135407\n> \n> and the discussion that ensued; perhaps intersting is\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/135280/focus=135462\n> \n> (where Junio suggest to make this a warning for now).\n\nThanks for the pointer. I don't really agree that undocumented\nbehaviour (that has never even been sighted in the wild, no less) needs\nto be protected quite this much, but then again I would be much less\nvisibly responsible for the consequences than Junio, wouldn't I? :)\n\nThis patch effectively combines Sverre's and Junio's suggestions from\nthe old discussion; it deprecates the current behavior with a warning\nmessage and also introduces --empty as a replacement. [Cc'ing both]\n\nThe alternative would be to simply match up the documentation with the\nway read-tree currently works. I prefer my approach due to the\nreasoning in the commit message.\n\n Documentation/git-read-tree.txt | 6 +++++-\n builtin/read-tree.c | 10 +++++++++-\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt\nindex 2e78da4..e88e9c2 100644\n--- a/Documentation/git-read-tree.txt\n+++ b/Documentation/git-read-tree.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git read-tree' [[-m [--trivial] [--aggressive] | --reset | --prefix=]\n \t\t[-u [--exclude-per-directory=] | -i]]\n \t\t[--index-output=] [--no-sparse-checkout]\n-\t\t [ []]\n+\t\t(--empty | [ []])\n \n \n DESCRIPTION\n@@ -114,6 +114,10 @@ OPTIONS\n \tDisable sparse checkout support even if `core.sparseCheckout`\n \tis true.\n \n+--empty::\n+\tInstead of reading tree object(s) into the index, just empty\n+\tit.\n+\n ::\n \tThe id of the tree object(s) to be read/merged.\n \ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex 9ad1e66..eb1e3e7 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -16,6 +16,7 @@\n #include \"resolve-undo.h\"\n \n static int nr_trees;\n+static int read_empty;\n static struct tree *trees[MAX_UNPACK_TREES];\n \n static int list_tree(unsigned char *sha1)\n@@ -32,7 +33,7 @@ static int list_tree(unsigned char *sha1)\n }\n \n static const char * const read_tree_usage[] = {\n-\t\"git read-tree [[-m [--trivial] [--aggressive] | --reset | --prefix=] [-u [--exclude-per-directory=] | -i]] [--no-sparse-checkout] [--index-output=] [ []]\",\n+\t\"git read-tree [[-m [--trivial] [--aggressive] | --reset | --prefix=] [-u [--exclude-per-directory=] | -i]] [--no-sparse-checkout] [--index-output=] (--empty | [ []])\",\n \tNULL\n };\n \n@@ -106,6 +107,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t{ OPTION_CALLBACK, 0, \"index-output\", NULL, \"FILE\",\n \t\t \"write resulting index to \",\n \t\t PARSE_OPT_NONEG, index_output_cb },\n+\t\tOPT_SET_INT(0, \"empty\", &read_empty,\n+\t\t\t \"only empty the index\", 1),\n \t\tOPT__VERBOSE(&opts.verbose_update),\n \t\tOPT_GROUP(\"Merging\"),\n \t\tOPT_SET_INT('m', NULL, &opts.merge,\n@@ -166,6 +169,11 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\tdie(\"failed to unpack tree object %s\", arg);\n \t\tstage++;\n \t}\n+\tif (nr_trees == 0 && !read_empty)\n+\t\twarning(\"read-tree: emptying the index with no arguments is deprecated; use --empty\");\n+\telse if (nr_trees > 0 && read_empty)\n+\t\tdie(\"passing trees as arguments contradicts --empty\");\n+\n \tif (1 < opts.index_only + opts.update)\n \t\tdie(\"-u and -i at the same time makes no sense\");\n \tif ((opts.update||opts.index_only) && !opts.merge)\n-- \n1.7.2.3.392.g02377.dirty\n"},{"id":"150435","messageId":"AANLkTimiT7eV2XHMEjbp6cOqTTQ6OH8vrbMoDCLxneJC@mail.gmail.com","threadId":"25064","inReplyTo":"20100910152859.778636d4@jk.gs","subject":"Re: [PATCH] read-tree: deprecate syntax without tree-ish args","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-09-10T13:53:28Z","receivedAt":"2010-09-10T13:53:28Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Sep 10, 2010 at 08:28, Jan Krüger wrote:\n> Currently, read-tree can be run without tree-ish arguments, in which\n> case it will empty the index. Since this behavior is undocumented and\n> perhaps a bit too invasive to be the \"default\" action for read-tree,\n> deprecate it in favor of a new --empty option that does the same thing.\n\nFWIW, I still like it.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"150442","messageId":"7vhbhxip9u.fsf@alter.siamese.dyndns.org","threadId":"25064","inReplyTo":"20100910152859.778636d4@jk.gs","subject":"Re: [PATCH] read-tree: deprecate syntax without tree-ish args","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-10T15:36:45Z","receivedAt":"2010-09-10T15:36:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Krüger writes:\n\n> Currently, read-tree can be run without tree-ish arguments, in which\n> case it will empty the index. Since this behavior is undocumented and\n> perhaps a bit too invasive to be the \"default\" action for read-tree,\n> deprecate it in favor of a new --empty option that does the same thing.\n>\n> Signed-off-by: Jan Krüger \n\nSounds sensible; will queue. Thanks.\n"}]}