# [PATCH] read-tree: Fix regression with creation of a new index file. 3 messages from 2009-08-17 to 2009-08-18. Participants: Alexandre Julliard, Johannes Schindelin, Stephen Boyd. Thread: https://gitlist.dev/t/20637 ## Alexandre Julliard, 2009-08-17 15:35 Subject: [PATCH] read-tree: Fix regression with creation of a new index file. Message-ID: <877hx25u7j.fsf@wine.dyndns.org> URL: https://gitlist.dev/e/877hx25u7j.fsf%40wine.dyndns.org ``` Reading the index into an empty file has been broken by 5a56da58060e50980fab0f4c38203a25440d1530, since it causes the existing index to always be loaded first, and dies if it's an empty file: $ GIT_INDEX_FILE=`mktemp` git read-tree master fatal: index file smaller than expected It breaks for instance committing from git.el. This patch reverts to the previous behavior of only loading the index when merging it. Signed-off-by: Alexandre Julliard --- builtin-read-tree.c | 10 ++++++---- t/t1009-read-tree-new-index.sh | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) create mode 100755 t/t1009-read-tree-new-index.sh diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 9c2d634..14c836b 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -113,13 +113,15 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) argc = parse_options(argc, argv, unused_prefix, read_tree_options, read_tree_usage, 0); - if (read_cache_unmerged() && (opts.prefix || opts.merge)) - die("You need to resolve your current index first"); - prefix_set = opts.prefix ? 1 : 0; if (1 < opts.merge + opts.reset + prefix_set) die("Which one? -m, --reset, or --prefix?"); - stage = opts.merge = (opts.reset || opts.merge || prefix_set); + + if (opts.reset || opts.merge || opts.prefix) { + if (read_cache_unmerged() && (opts.prefix || opts.merge)) + die("You need to resolve your current index first"); + stage = opts.merge = 1; + } for (i = 0; i < argc; i++) { const char *arg = argv[i]; diff --git a/t/t1009-read-tree-new-index.sh b/t/t1009-read-tree-new-index.sh new file mode 100755 index 0000000..59b3aa4 --- /dev/null +++ b/t/t1009-read-tree-new-index.sh @@ -0,0 +1,25 @@ +#!/bin/sh + +test_description='test read-tree into a fresh index file' + +. ./test-lib.sh + +test_expect_success setup ' + echo one >a && + git add a && + git commit -m initial +' + +test_expect_success 'non-existent index file' ' + rm -f new-index && + GIT_INDEX_FILE=new-index git read-tree master +' + +test_expect_success 'empty index file' ' + rm -f new-index && + > new-index && + GIT_INDEX_FILE=new-index git read-tree master +' + +test_done + -- 1.6.4.181.g3f2ea.dirty -- Alexandre Julliard julliard@winehq.org ``` ## Johannes Schindelin, 2009-08-17 22:19 Subject: Re: [PATCH] read-tree: Fix regression with creation of a new index file. Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0908180018020.8306%40pacific.mpi-cbg.de In-Reply-To: <877hx25u7j.fsf@wine.dyndns.org> ``` Hi, On Mon, 17 Aug 2009, Alexandre Julliard wrote: > diff --git a/builtin-read-tree.c b/builtin-read-tree.c > index 9c2d634..14c836b 100644 > --- a/builtin-read-tree.c > +++ b/builtin-read-tree.c > @@ -113,13 +113,15 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) > argc = parse_options(argc, argv, unused_prefix, read_tree_options, > read_tree_usage, 0); > > - if (read_cache_unmerged() && (opts.prefix || opts.merge)) > - die("You need to resolve your current index first"); > - > prefix_set = opts.prefix ? 1 : 0; > if (1 < opts.merge + opts.reset + prefix_set) > die("Which one? -m, --reset, or --prefix?"); > - stage = opts.merge = (opts.reset || opts.merge || prefix_set); > + > + if (opts.reset || opts.merge || opts.prefix) { > + if (read_cache_unmerged() && (opts.prefix || opts.merge)) > + die("You need to resolve your current index first"); > + stage = opts.merge = 1; > + } Actually, this should be enough: -- snipsnap -- diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 9c2d634..d649c56 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -113,14 +113,14 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) argc = parse_options(argc, argv, unused_prefix, read_tree_options, read_tree_usage, 0); - if (read_cache_unmerged() && (opts.prefix || opts.merge)) - die("You need to resolve your current index first"); - prefix_set = opts.prefix ? 1 : 0; if (1 < opts.merge + opts.reset + prefix_set) die("Which one? -m, --reset, or --prefix?"); stage = opts.merge = (opts.reset || opts.merge || prefix_set); + if (opts.merge && (read_cache_unmerged() && !prefix_set && !opts.reset)) + die("You need to resolve your current index first"); + for (i = 0; i < argc; i++) { const char *arg = argv[i]; -- 1.6.4.313.g3d9e3 ``` ## Stephen Boyd, 2009-08-18 03:37 Subject: Re: [PATCH] read-tree: Fix regression with creation of a new index file. Message-ID: <4A8A21E7.7070001@gmail.com> URL: https://gitlist.dev/e/4A8A21E7.7070001%40gmail.com In-Reply-To: ``` Johannes Schindelin wrote: > diff --git a/builtin-read-tree.c b/builtin-read-tree.c > index 9c2d634..d649c56 100644 > --- a/builtin-read-tree.c > +++ b/builtin-read-tree.c > @@ -113,14 +113,14 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) > argc = parse_options(argc, argv, unused_prefix, read_tree_options, > read_tree_usage, 0); > > - if (read_cache_unmerged() && (opts.prefix || opts.merge)) > - die("You need to resolve your current index first"); > - > prefix_set = opts.prefix ? 1 : 0; > if (1 < opts.merge + opts.reset + prefix_set) > die("Which one? -m, --reset, or --prefix?"); > stage = opts.merge = (opts.reset || opts.merge || prefix_set); > > + if (opts.merge && (read_cache_unmerged() && !prefix_set && !opts.reset)) > This looks more compact but I think the !prefix_set check is wrong. Yes, we want to do read_cache_unmerged() if we're doing some sort of merging operation. But we want to die() when either -m or --prefix is used. Therefore, die() if we're not doing a --reset. So we might as well just check that case and nothing else. The original patch from Alexandre is correct, but if you want to avoid extra nesting I suppose you could do something like the patch below. Thanks. --- diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 9c2d634..c6d5b49 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -113,14 +113,14 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) argc = parse_options(argc, argv, unused_prefix, read_tree_options, read_tree_usage, 0); - if (read_cache_unmerged() && (opts.prefix || opts.merge)) - die("You need to resolve your current index first"); - prefix_set = opts.prefix ? 1 : 0; if (1 < opts.merge + opts.reset + prefix_set) die("Which one? -m, --reset, or --prefix?"); stage = opts.merge = (opts.reset || opts.merge || prefix_set); + if (opts.merge && read_cache_unmerged() && !opts.reset) + die("You need to resolve your current index first"); ```