git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <chriscool@tuxfamily.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Stephan Beyer <s-beyer@gmx.net>,
	Daniel Barkalow <barkalow@iabervon.org>,
	Jakub Narebski <jnareb@gmail.com>,
	Paolo Bonzini <bonzini@gnu.org>,
	Johannes Sixt <j.sixt@viscovery.net>,
	Stephen Boyd <bebarino@gmail.com>
Subject: [PATCH v5 3/7] reset: use "unpack_trees()" directly instead of "git read-tree"
Date: Sat, 12 Dec 2009 05:32:54 +0100	[thread overview]
Message-ID: <20091212043259.3930.82570.chriscool@tuxfamily.org> (raw)
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>

From: Stephan Beyer <s-beyer@gmx.net>

This patch makes "reset_index_file()" call "unpack_trees()" directly
instead of forking and execing "git read-tree". So the code is more
efficient.

And it's also easier to see which unpack_tree() options will be used,
as we don't need to follow "git read-tree"'s command line parsing
which is quite complex.

As Daniel Barkalow found, there is a difference between this new
version and the old one. The old version gives an error for
"git reset --merge" with unmerged entries and the new version does
not. But this can be seen as a bug fix, because "--merge" was the
only "git reset" option with this behavior and this behavior was
not documented.

In fact there is still an error with unmerge entries if we reset
the unmerge entries to the same state as HEAD. So the bug is not
completely fixed.

The code comes from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c        |   41 ++++++++++++++++++++++++++++++-----------
 t/t7110-reset-merge.sh |    8 +++++---
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 14bdb03..ac38aaa 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,6 +18,8 @@
 #include "tree.h"
 #include "branch.h"
 #include "parse-options.h"
+#include "unpack-trees.h"
+#include "cache-tree.h"
 
 static const char * const git_reset_usage[] = {
 	"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -54,27 +56,44 @@ static inline int is_merge(void)
 
 static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
 {
-	int i = 0;
-	const char *args[6];
+	int nr = 1;
+	int newfd;
+	struct tree_desc desc[2];
+	struct unpack_trees_options opts;
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 
-	args[i++] = "read-tree";
+	memset(&opts, 0, sizeof(opts));
+	opts.head_idx = 1;
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+	opts.fn = oneway_merge;
+	opts.merge = 1;
 	if (!quiet)
-		args[i++] = "-v";
+		opts.verbose_update = 1;
 	switch (reset_type) {
 	case MERGE:
-		args[i++] = "-u";
-		args[i++] = "-m";
+		opts.update = 1;
 		break;
 	case HARD:
-		args[i++] = "-u";
+		opts.update = 1;
 		/* fallthrough */
 	default:
-		args[i++] = "--reset";
+		opts.reset = 1;
 	}
-	args[i++] = sha1_to_hex(sha1);
-	args[i] = NULL;
 
-	return run_command_v_opt(args, RUN_GIT_CMD);
+	newfd = hold_locked_index(lock, 1);
+
+	read_cache_unmerged();
+
+	if (!fill_tree_descriptor(desc + nr - 1, sha1))
+		return error("Failed to find tree of %s.", sha1_to_hex(sha1));
+	if (unpack_trees(nr, desc, &opts))
+		return -1;
+	if (write_cache(newfd, active_cache, active_nr) ||
+	    commit_locked_index(lock))
+		return error("Could not write new index file.");
+
+	return 0;
 }
 
 static void print_new_head_line(struct commit *commit)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 8190da1..6afaf73 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -79,10 +79,12 @@ test_expect_success 'setup 2 different branches' '
      git commit -a -m "change in branch2"
 '
 
-test_expect_success '"reset --merge HEAD^" fails with pending merge' '
+test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
      test_must_fail git merge branch1 &&
-     test_must_fail git reset --merge HEAD^ &&
-     git reset --hard HEAD
+     git reset --merge HEAD^ &&
+     test -z "$(git diff --cached)" &&
+     test -n "$(git diff)" &&
+     git reset --hard HEAD@{1}
 '
 
 test_expect_success '"reset --merge HEAD" fails with pending merge' '
-- 
1.6.6.rc1.8.gd33ec

  parent reply	other threads:[~2009-12-12  4:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-12  4:32 [PATCH v5 0/7] "git reset --merge" related improvements Christian Couder
2009-12-12  4:32 ` [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir Christian Couder
2009-12-12 21:45   ` Junio C Hamano
2009-12-15 19:41     ` Christian Couder
2009-12-15 20:17       ` Junio C Hamano
2009-12-15 20:25         ` Junio C Hamano
2009-12-16  6:36         ` Christian Couder
2009-12-15 20:20       ` Junio C Hamano
2009-12-12  4:32 ` [PATCH v5 2/7] reset: add a few tests for "git reset --merge" Christian Couder
2009-12-12 23:30   ` Junio C Hamano
2009-12-12  4:32 ` Christian Couder [this message]
2009-12-12 21:46   ` [PATCH v5 3/7] reset: use "unpack_trees()" directly instead of "git read-tree" Junio C Hamano
2009-12-12  4:32 ` [PATCH v5 4/7] reset: add option "--keep" to "git reset" Christian Couder
2009-12-12 21:46   ` Junio C Hamano
2009-12-30  5:47     ` Christian Couder
2009-12-12  4:32 ` [PATCH v5 5/7] reset: add test cases for "--keep" option Christian Couder
2009-12-12  4:32 ` [PATCH v5 6/7] Documentation: reset: describe new " Christian Couder
2009-12-12  4:32 ` [PATCH v5 7/7] Documentation: reset: add some tables to describe the different options Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091212043259.3930.82570.chriscool@tuxfamily.org \
    --to=chriscool@tuxfamily.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=barkalow@iabervon.org \
    --cc=bebarino@gmail.com \
    --cc=bonzini@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=jnareb@gmail.com \
    --cc=s-beyer@gmx.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).