{"thread":{"id":"20637","subject":"[PATCH] read-tree: Fix regression with creation of a new index file.","startedAt":"2009-08-17T15:35:44Z","lastAt":"2009-08-18T03:37:11Z","messageCount":3,"participants":["Alexandre Julliard","Johannes Schindelin","Stephen Boyd"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"120905","messageId":"877hx25u7j.fsf@wine.dyndns.org","threadId":"20637","inReplyTo":null,"subject":"[PATCH] read-tree: Fix regression with creation of a new index file.","fromName":"Alexandre Julliard","fromEmail":"julliard@winehq.org","sentAt":"2009-08-17T15:35:44Z","receivedAt":"2009-08-17T15:35:44Z","isPatch":true,"sender":{"key":"julliard@winehq.org","avatar":null},"body":"Reading the index into an empty file has been broken by\n5a56da58060e50980fab0f4c38203a25440d1530, since it causes the existing\nindex to always be loaded first, and dies if it's an empty file:\n\n$ GIT_INDEX_FILE=`mktemp` git read-tree master\nfatal: index file smaller than expected\n\nIt breaks for instance committing from git.el. This patch reverts to the\nprevious behavior of only loading the index when merging it.\n\nSigned-off-by: Alexandre Julliard <julliard@winehq.org>\n---\n builtin-read-tree.c            |   10 ++++++----\n t/t1009-read-tree-new-index.sh |   25 +++++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 4 deletions(-)\n create mode 100755 t/t1009-read-tree-new-index.sh\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 9c2d634..14c836b 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -113,13 +113,15 @@ 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 (read_cache_unmerged() && (opts.prefix || opts.merge))\n-\t\tdie(\"You need to resolve your current index first\");\n-\n \tprefix_set = opts.prefix ? 1 : 0;\n \tif (1 < opts.merge + opts.reset + prefix_set)\n \t\tdie(\"Which one? -m, --reset, or --prefix?\");\n-\tstage = opts.merge = (opts.reset || opts.merge || prefix_set);\n+\n+\tif (opts.reset || opts.merge || opts.prefix) {\n+\t\tif (read_cache_unmerged() && (opts.prefix || opts.merge))\n+\t\t\tdie(\"You need to resolve your current index first\");\n+\t\tstage = opts.merge = 1;\n+\t}\n \n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\ndiff --git a/t/t1009-read-tree-new-index.sh b/t/t1009-read-tree-new-index.sh\nnew file mode 100755\nindex 0000000..59b3aa4\n--- /dev/null\n+++ b/t/t1009-read-tree-new-index.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+\n+test_description='test read-tree into a fresh index file'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo one >a &&\n+\tgit add a &&\n+\tgit commit -m initial\n+'\n+\n+test_expect_success 'non-existent index file' '\n+\trm -f new-index &&\n+\tGIT_INDEX_FILE=new-index git read-tree master\n+'\n+\n+test_expect_success 'empty index file' '\n+\trm -f new-index &&\n+\t> new-index &&\n+\tGIT_INDEX_FILE=new-index git read-tree master\n+'\n+\n+test_done\n+\n-- \n1.6.4.181.g3f2ea.dirty\n\n-- \nAlexandre Julliard\njulliard@winehq.org\n"},{"id":"121006","messageId":"alpine.DEB.1.00.0908180018020.8306@pacific.mpi-cbg.de","threadId":"20637","inReplyTo":"877hx25u7j.fsf@wine.dyndns.org","subject":"Re: [PATCH] read-tree: Fix regression with creation of a new index file.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-17T22:19:47Z","receivedAt":"2009-08-17T22:19:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Aug 2009, Alexandre Julliard wrote:\n\n> diff --git a/builtin-read-tree.c b/builtin-read-tree.c\n> index 9c2d634..14c836b 100644\n> --- a/builtin-read-tree.c\n> +++ b/builtin-read-tree.c\n> @@ -113,13 +113,15 @@ 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 (read_cache_unmerged() && (opts.prefix || opts.merge))\n> -\t\tdie(\"You need to resolve your current index first\");\n> -\n>  \tprefix_set = opts.prefix ? 1 : 0;\n>  \tif (1 < opts.merge + opts.reset + prefix_set)\n>  \t\tdie(\"Which one? -m, --reset, or --prefix?\");\n> -\tstage = opts.merge = (opts.reset || opts.merge || prefix_set);\n> +\n> +\tif (opts.reset || opts.merge || opts.prefix) {\n> +\t\tif (read_cache_unmerged() && (opts.prefix || opts.merge))\n> +\t\t\tdie(\"You need to resolve your current index first\");\n> +\t\tstage = opts.merge = 1;\n> +\t}\n\nActually, this should be enough:\n\n-- snipsnap --\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 9c2d634..d649c56 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -113,14 +113,14 @@ 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 (read_cache_unmerged() && (opts.prefix || opts.merge))\n-\t\tdie(\"You need to resolve your current index first\");\n-\n \tprefix_set = opts.prefix ? 1 : 0;\n \tif (1 < opts.merge + opts.reset + prefix_set)\n \t\tdie(\"Which one? -m, --reset, or --prefix?\");\n \tstage = opts.merge = (opts.reset || opts.merge || prefix_set);\n \n+\tif (opts.merge && (read_cache_unmerged() && !prefix_set && !opts.reset))\n+\t\tdie(\"You need to resolve your current index first\");\n+\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \n-- \n1.6.4.313.g3d9e3\n"},{"id":"121049","messageId":"4A8A21E7.7070001@gmail.com","threadId":"20637","inReplyTo":"alpine.DEB.1.00.0908180018020.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH] read-tree: Fix regression with creation of a new index file.","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-08-18T03:37:11Z","receivedAt":"2009-08-18T03:37:11Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Johannes Schindelin wrote:\n> diff --git a/builtin-read-tree.c b/builtin-read-tree.c\n> index 9c2d634..d649c56 100644\n> --- a/builtin-read-tree.c\n> +++ b/builtin-read-tree.c\n> @@ -113,14 +113,14 @@ 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 (read_cache_unmerged() && (opts.prefix || opts.merge))\n> -\t\tdie(\"You need to resolve your current index first\");\n> -\n>  \tprefix_set = opts.prefix ? 1 : 0;\n>  \tif (1 < opts.merge + opts.reset + prefix_set)\n>  \t\tdie(\"Which one? -m, --reset, or --prefix?\");\n>  \tstage = opts.merge = (opts.reset || opts.merge || prefix_set);\n>  \n> +\tif (opts.merge && (read_cache_unmerged() && !prefix_set && !opts.reset))\n>   \n\nThis looks more compact but I think the !prefix_set check is wrong.\n\nYes, we want to do read_cache_unmerged() if we're doing some sort of\nmerging operation. But we want to die() when either -m or --prefix is\nused. Therefore, die() if we're not doing a --reset. So we might as well\njust check that case and nothing else.\n\nThe original patch from Alexandre is correct, but if you want to avoid\nextra nesting I suppose you could do something like the patch below.\n\nThanks.\n\n---\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 9c2d634..c6d5b49 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -113,14 +113,14 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n        argc = parse_options(argc, argv, unused_prefix, read_tree_options,\n                             read_tree_usage, 0);\n \n-       if (read_cache_unmerged() && (opts.prefix || opts.merge))\n-               die(\"You need to resolve your current index first\");\n-\n        prefix_set = opts.prefix ? 1 : 0;\n        if (1 < opts.merge + opts.reset + prefix_set)\n                die(\"Which one? -m, --reset, or --prefix?\");\n        stage = opts.merge = (opts.reset || opts.merge || prefix_set);\n \n+       if (opts.merge && read_cache_unmerged() && !opts.reset)\n+               die(\"You need to resolve your current index first\");\n"}]}