builtin-commit: fix partial-commit support

When making a partial-commit, we need to prepare two index
files, one to be used to write out the tree to be committed
(temporary index) and the other to be used as the index file
after the commit is made.

The temporary index needs to be initialized to HEAD and then all
the named paths on the command line need to be staged on top of
the index.  For this, running add_files_to_cache() that compares
what is in the index and the paths given from the command line
is not enough -- the comparison will miss the paths that the
user previously ran "git add" to the index since the HEAD
because the index reset to the HEAD would not know about them.
The index file needs to get the same modification done when
preparing the temporary index as described above.

This implementation mimics the behaviour of the scripted
version of git-commit.  It first runs overlay_tree_on_cache(),
which was stolen from ls-files with the earlier change, to get
the list of paths that the user can potentially mean, and then
uses pathspec_match() to find which ones the user meant.  This
list of paths is used to update both the temporary and the real
index file.

Additional fixes are:

 - read the index file after pre-commit hook returns, as the
   hook can modify it to affect the contents of the commit.

 - remove the temporary index file .git/next-index-* after
   commit is done or aborted.

 - run post-commit hook with the real index file to be used
   after the commit (previously it gave the temporary commit if
   a partial commit was made).

 - resurrect the safety mechanism to refuse partial commits
   during a merge to match the scripted version.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 202 insertions, 40 deletions

diff --git a/builtin-commit.c b/builtin-commit.c
index cd2f5ca..e779db8 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -7,6 +7,7 @@
 
 #include "cache.h"
 #include "cache-tree.h"
+#include "dir.h"
 #include "builtin.h"
 #include "diff.h"
 #include "diffcore.h"
@@ -19,6 +20,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "path-list.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git-commit [options] [--] <filepattern>...",
@@ -28,7 +30,13 @@ static const char * const builtin_commit_usage[] = {
 static unsigned char head_sha1[20], merge_head_sha1[20];
 static char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
-static struct lock_file lock_file;
+static struct lock_file index_lock; /* real index */
+static struct lock_file false_lock; /* used only for partial commits */
+static enum {
+	COMMIT_AS_IS = 1,
+	COMMIT_NORMAL,
+	COMMIT_PARTIAL,
+} commit_style;
 
 static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
@@ -78,41 +86,179 @@ static struct option builtin_commit_options[] = {
 	OPT_END()
 };
 
+static void rollback_index_files(void)
+{
+	switch (commit_style) {
+	case COMMIT_AS_IS:
+		break; /* nothing to do */
+	case COMMIT_NORMAL:
+		rollback_lock_file(&index_lock);
+		break;
+	case COMMIT_PARTIAL:
+		rollback_lock_file(&index_lock);
+		rollback_lock_file(&false_lock);
+		break;
+	}
+}
+
+static void commit_index_files(void)
+{
+	switch (commit_style) {
+	case COMMIT_AS_IS:
+		break; /* nothing to do */
+	case COMMIT_NORMAL:
+		commit_lock_file(&index_lock);
+		break;
+	case COMMIT_PARTIAL:
+		commit_lock_file(&index_lock);
+		rollback_lock_file(&false_lock);
+		break;
+	}
+}
+
+/*
+ * Take a union of paths in the index and the named tree (typically, "HEAD"),
+ * and return the paths that match the given pattern in list.
+ */
+static int list_paths(struct path_list *list, const char *with_tree,
+		      const char *prefix, const char **pattern)
+{
+	int i;
+	char *m;
+
+	for (i = 0; pattern[i]; i++)
+		;
+	m = xcalloc(1, i);
+
+	if (with_tree)
+		overlay_tree_on_cache(with_tree, prefix);
+
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+		if (ce->ce_flags & htons(CE_UPDATE))
+			continue;
+		if (!pathspec_match(pattern, m, ce->name, 0))
+			continue;
+		path_list_insert(ce->name, list);
+	}
+
+	return report_path_error(m, pattern, prefix ? strlen(prefix) : 0);
+}
+
+static void add_remove_files(struct path_list *list)
+{
+	int i;
+	for (i = 0; i < list->nr; i++) {
+		struct path_list_item *p = &(list->items[i]);
+		if (file_exists(p->path))
+			add_file_to_cache(p->path, 0);
+		else
+			remove_file_from_cache(p->path);
+	}
+}
+
 static char *prepare_index(const char **files, const char *prefix)
 {
 	int fd;
 	struct tree *tree;
-	struct lock_file *next_index_lock;
+	struct path_list partial;
+	const char **pathspec = NULL;
 
 	if (interactive) {
 		interactive_add();
+		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}
 
-	fd = hold_locked_index(&lock_file, 1);
 	if (read_cache() < 0)
 		die("index file corrupt");
 
-	if (all || also) {
-		add_files_to_cache(verbose, also ? prefix : NULL, files);
+	if (*files)
+		pathspec = get_pathspec(prefix, files);
+
+	/*
+	 * Non partial, non as-is commit.
+	 *
+	 * (1) get the real index;
+	 * (2) update the_index as necessary;
+	 * (3) write the_index out to the real index (still locked);
+	 * (4) return the name of the locked index file.
+	 *
+	 * The caller should run hooks on the locked real index, and
+	 * (A) if all goes well, commit the real index;
+	 * (B) on failure, rollback the real index.
+	 */
+	if (all || (also && pathspec && *pathspec)) {
+		int fd = hold_locked_index(&index_lock, 1);
+		add_files_to_cache(0, also ? prefix : NULL, pathspec);
 		refresh_cache(REFRESH_QUIET);
 		if (write_cache(fd, active_cache, active_nr) || close(fd))
 			die("unable to write new_index file");
-		return lock_file.filename;
+		commit_style = COMMIT_NORMAL;
+		return index_lock.filename;
 	}
 
-	if (*files == NULL) {
-		/* Commit index as-is. */
-		rollback_lock_file(&lock_file);
+	/*
+	 * As-is commit.
+	 *
+	 * (1) return the name of the real index file.
+	 *
+	 * The caller should run hooks on the real index, and run
+	 * hooks on the real index, and create commit from the_index.
+	 * We still need to refresh the index here.
+	 */
+	if (!pathspec || !*pathspec) {
+		fd = hold_locked_index(&index_lock, 1);
+		refresh_cache(REFRESH_QUIET);
+		if (write_cache(fd, active_cache, active_nr) ||
+		    close(fd) || commit_locked_index(&index_lock))
+			die("unable to write new_index file");
+		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}
 
-	/* update the user index file */
-	add_files_to_cache(verbose, prefix, files);
+	/*
+	 * A partial commit.
+	 *
+	 * (0) find the set of affected paths;
+	 * (1) get lock on the real index file;
+	 * (2) update the_index with the given paths;
+	 * (3) write the_index out to the real index (still locked);
+	 * (4) get lock on the false index file;
+	 * (5) reset the_index from HEAD;
+	 * (6) update the_index the same way as (2);
+	 * (7) write the_index out to the false index file;
+	 * (8) return the name of the false index file (still locked);
+	 *
+	 * The caller should run hooks on the locked false index, and
+	 * create commit from it.  Then
+	 * (A) if all goes well, commit the real index;
+	 * (B) on failure, rollback the real index;
+	 * In either case, rollback the false index.
+	 */
+	commit_style = COMMIT_PARTIAL;
+
+	if (file_exists(git_path("MERGE_HEAD")))
+		die("cannot do a partial commit during a merge.");
+
+	memset(&partial, 0, sizeof(partial));
+	partial.strdup_paths = 1;
+	if (list_paths(&partial, initial_commit ? NULL : "HEAD", prefix, pathspec))
+		exit(1);
+
+	discard_cache();
+	if (read_cache() < 0)
+		die("cannot read the index");
+
+	fd = hold_locked_index(&index_lock, 1);
+	add_remove_files(&partial);
 	refresh_cache(REFRESH_QUIET);
 	if (write_cache(fd, active_cache, active_nr) || close(fd))
 		die("unable to write new_index file");
 
+	fd = hold_lock_file_for_update(&false_lock,
+				       git_path("next-index-%d", getpid()), 1);
+	discard_cache();
 	if (!initial_commit) {
 		tree = parse_tree_indirect(head_sha1);
 		if (!tree)
@@ -120,17 +266,12 @@ static char *prepare_index(const char **files, const char *prefix)
 		if (read_tree(tree, 0, NULL))
 			die("failed to read HEAD tree object");
 	}
-
-	/* Use a lock file to garbage collect the temporary index file. */
-	next_index_lock = xmalloc(sizeof(*next_index_lock));
-	fd = hold_lock_file_for_update(next_index_lock,
-				       git_path("next-index-%d", getpid()), 1);
-	add_files_to_cache(verbose, prefix, files);
+	add_remove_files(&partial);
 	refresh_cache(REFRESH_QUIET);
-	if (write_cache(fd, active_cache, active_nr) || close(fd))
-		die("unable to write new_index file");
 
-	return next_index_lock->filename;
+	if (write_cache(fd, active_cache, active_nr) || close(fd))
+		die("unable to write temporary index file");
+	return false_lock.filename;
 }
 
 static int run_status(FILE *fp, const char *index_file, const char *prefix)
@@ -441,7 +582,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	commitable = run_status(stdout, index_file, prefix);
 
-	rollback_lock_file(&lock_file);
+	rollback_index_files();
 
 	return commitable ? 0 : 1;
 }
@@ -531,23 +672,36 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	index_file = prepare_index(argv, prefix);
 
-	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
-		exit(1);
+	if (!no_verify && run_hook(index_file, "pre-commit", NULL)) {
+		rollback_index_files();
+		return 1;
+	}
 
 	if (!prepare_log_message(index_file, prefix) && !in_merge) {
 		run_status(stdout, index_file, prefix);
+		rollback_index_files();
 		unlink(commit_editmsg);
 		return 1;
 	}
 
-	strbuf_init(&sb, 0);
-
-	/* Start building up the commit header */
+	/*
+	 * Re-read the index as pre-commit hook could have updated it,
+	 * and write it out as a tree.
+	 */
+	discard_cache();
 	read_cache_from(index_file);
-	active_cache_tree = cache_tree();
+	if (!active_cache_tree)
+		active_cache_tree = cache_tree();
 	if (cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0) < 0)
+			      active_cache, active_nr, 0, 0) < 0) {
+		rollback_index_files();
 		die("Error building trees");
+	}
+
+	/*
+	 * The commit object
+	 */
+	strbuf_init(&sb, 0);
 	strbuf_addf(&sb, "tree %s\n",
 		    sha1_to_hex(active_cache_tree->sha1));
 
@@ -596,20 +750,27 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	header_len = sb.len;
 	if (!no_edit)
 		launch_editor(git_path(commit_editmsg), &sb);
-	else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0)
+	else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
+		rollback_index_files();
 		die("could not read commit message\n");
-	if (run_hook(index_file, "commit-msg", git_path(commit_editmsg)))
+	}
+	if (run_hook(index_file, "commit-msg", git_path(commit_editmsg))) {
+		rollback_index_files();
 		exit(1);
+	}
 	stripspace(&sb, 1);
-	if (sb.len < header_len ||
-	    message_is_empty(&sb, header_len))
+	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
+		rollback_index_files();
 		die("* no commit message?  aborting commit.");
+	}
 	strbuf_addch(&sb, '\0');
 	if (is_encoding_utf8(git_commit_encoding) && !is_utf8(sb.buf))
 		fprintf(stderr, commit_utf8_warn);
 
-	if (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1))
+	if (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1)) {
+		rollback_index_files();
 		die("failed to write commit object");
+	}
 
 	ref_lock = lock_any_ref_for_update("HEAD",
 					   initial_commit ? NULL : head_sha1,
@@ -624,21 +785,22 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
 	strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
 
-	if (!ref_lock)
+	if (!ref_lock) {
+		rollback_index_files();
 		die("cannot lock HEAD ref");
-	if (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0)
+	}
+	if (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0) {
+		rollback_index_files();
 		die("cannot update HEAD ref");
+	}
 
 	unlink(git_path("MERGE_HEAD"));
 	unlink(git_path("MERGE_MSG"));
 
-	if (lock_file.filename[0] && commit_locked_index(&lock_file))
-		die("failed to write new index");
+	commit_index_files();
 
 	rerere();
-
-	run_hook(index_file, "post-commit", NULL);
-
+	run_hook(get_index_file(), "post-commit", NULL);
 	if (!quiet)
 		print_summary(prefix, commit_sha1);