[PATCH] file_exists(): dangling symlinks do exist

[PATCH] file_exists(): dangling symlinks do exist

[Junio C Hamano]

This function is used to see if a path given by the user does exist
on the filesystem.  A symbolic link that does not point anywhere does
exist but running stat() on it would yield an error, and it incorrectly
said it does not exist.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 dir.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index 01790ab..87e5dec 100644
--- a/dir.c
+++ b/dir.c
@@ -688,11 +688,10 @@ int read_directory(struct dir_struct *dir, const char *path, const char *base, i
 	return dir->nr;
 }
 
-int
-file_exists(const char *f)
+int file_exists(const char *f)
 {
-  struct stat sb;
-  return stat(f, &sb) == 0;
+	struct stat sb;
+	return lstat(f, &sb) == 0;
 }
 
 /*
-- 
1.5.3.5.1815.g9445b

[PATCH] Export three helper functions from ls-files

[Junio C Hamano]

This exports three helper functions from ls-files.

 * pathspec_match() checks if a given path matches a set of pathspecs
   and optionally records which pathspec was used.  This function used
   to be called "match()" but renamed to be a bit less vague.

 * report_path_error() takes a set of pathspecs and the record
   pathspec_match() above leaves, and gives error message.  This
   was split out of the main function of ls-files.

 * overlay_tree_on_cache() takes a tree-ish (typically "HEAD")
   and overlays it on the current in-core index.  By iterating
   over the resulting index, the caller can find out the paths
   in either the index or the HEAD.  This function used to be
   called "overlay_tree()" but renamed to be a bit more
   descriptive.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-ls-files.c |   98 +++++++++++++++++++++++++++------------------------
 cache.h            |    6 +++
 2 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index e0b856f..be485bb 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -38,28 +38,28 @@ static const char *tag_modified = "";
 
 
 /*
- * Match a pathspec against a filename. The first "len" characters
+ * Match a pathspec against a filename. The first "skiplen" characters
  * are the common prefix
  */
-static int match(const char **spec, char *ps_matched,
-		 const char *filename, int len)
+int pathspec_match(const char **spec, char *ps_matched,
+		   const char *filename, int skiplen)
 {
 	const char *m;
 
 	while ((m = *spec++) != NULL) {
-		int matchlen = strlen(m + len);
+		int matchlen = strlen(m + skiplen);
 
 		if (!matchlen)
 			goto matched;
-		if (!strncmp(m + len, filename + len, matchlen)) {
-			if (m[len + matchlen - 1] == '/')
+		if (!strncmp(m + skiplen, filename + skiplen, matchlen)) {
+			if (m[skiplen + matchlen - 1] == '/')
 				goto matched;
-			switch (filename[len + matchlen]) {
+			switch (filename[skiplen + matchlen]) {
 			case '/': case '\0':
 				goto matched;
 			}
 		}
-		if (!fnmatch(m + len, filename + len, 0))
+		if (!fnmatch(m + skiplen, filename + skiplen, 0))
 			goto matched;
 		if (ps_matched)
 			ps_matched++;
@@ -80,7 +80,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 	if (len >= ent->len)
 		die("git-ls-files: internal error - directory entry not superset of prefix");
 
-	if (pathspec && !match(pathspec, ps_matched, ent->name, len))
+	if (pathspec && !pathspec_match(pathspec, ps_matched, ent->name, len))
 		return;
 
 	fputs(tag, stdout);
@@ -185,7 +185,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	if (len >= ce_namelen(ce))
 		die("git-ls-files: internal error - cache entry not superset of prefix");
 
-	if (pathspec && !match(pathspec, ps_matched, ce->name, len))
+	if (pathspec && !pathspec_match(pathspec, ps_matched, ce->name, len))
 		return;
 
 	if (tag && *tag && show_valid_bit &&
@@ -331,7 +331,7 @@ static const char *verify_pathspec(const char *prefix)
  * that were given from the command line.  We are not
  * going to write this index out.
  */
-static void overlay_tree(const char *tree_name, const char *prefix)
+void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 {
 	struct tree *tree;
 	unsigned char sha1[20];
@@ -384,6 +384,42 @@ static void overlay_tree(const char *tree_name, const char *prefix)
 	}
 }
 
+int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset)
+{
+	/*
+	 * Make sure all pathspec matched; otherwise it is an error.
+	 */
+	int num, errors = 0;
+	for (num = 0; pathspec[num]; num++) {
+		int other, found_dup;
+
+		if (ps_matched[num])
+			continue;
+		/*
+		 * The caller might have fed identical pathspec
+		 * twice.  Do not barf on such a mistake.
+		 */
+		for (found_dup = other = 0;
+		     !found_dup && pathspec[other];
+		     other++) {
+			if (other == num || !ps_matched[other])
+				continue;
+			if (!strcmp(pathspec[other], pathspec[num]))
+				/*
+				 * Ok, we have a match already.
+				 */
+				found_dup = 1;
+		}
+		if (found_dup)
+			continue;
+
+		error("pathspec '%s' did not match any file(s) known to git.",
+		      pathspec[num] + prefix_offset);
+		errors++;
+	}
+	return errors;
+}
+
 static const char ls_files_usage[] =
 	"git-ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
 	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
@@ -563,47 +599,17 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		 */
 		if (show_stage || show_unmerged)
 			die("ls-files --with-tree is incompatible with -s or -u");
-		overlay_tree(with_tree, prefix);
+		overlay_tree_on_cache(with_tree, prefix);
 	}
 	show_files(&dir, prefix);
 
 	if (ps_matched) {
-		/* We need to make sure all pathspec matched otherwise
-		 * it is an error.
-		 */
-		int num, errors = 0;
-		for (num = 0; pathspec[num]; num++) {
-			int other, found_dup;
-
-			if (ps_matched[num])
-				continue;
-			/*
-			 * The caller might have fed identical pathspec
-			 * twice.  Do not barf on such a mistake.
-			 */
-			for (found_dup = other = 0;
-			     !found_dup && pathspec[other];
-			     other++) {
-				if (other == num || !ps_matched[other])
-					continue;
-				if (!strcmp(pathspec[other], pathspec[num]))
-					/*
-					 * Ok, we have a match already.
-					 */
-					found_dup = 1;
-			}
-			if (found_dup)
-				continue;
-
-			error("pathspec '%s' did not match any file(s) known to git.",
-			      pathspec[num] + prefix_offset);
-			errors++;
-		}
-
-		if (errors)
+		int bad;
+		bad = report_path_error(ps_matched, pathspec, prefix_offset);
+		if (bad)
 			fprintf(stderr, "Did you forget to 'git add'?\n");
 
-		return errors ? 1 : 0;
+		return bad ? 1 : 0;
 	}
 
 	return 0;
diff --git a/cache.h b/cache.h
index f0a25c7..a84f343 100644
--- a/cache.h
+++ b/cache.h
@@ -603,4 +603,10 @@ extern int diff_auto_refresh_index;
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
 
+
+/* ls-files */
+int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
+int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset);
+void overlay_tree_on_cache(const char *tree_name, const char *prefix);
+
 #endif /* CACHE_H */
-- 
1.5.3.5.1815.g9445b

[PATCH] Fix add_files_to_cache() to take pathspec, not user specified list of files

[Junio C Hamano]

This separates the logic to limit the extent of change to the
index by where you are (controlled by "prefix") and what you
specify from the command line (controlled by "pathspec").

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-add.c |    8 +++++---
 cache.h       |    4 +++-
 commit.h      |    1 -
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 45b14e8..16f8557 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -111,12 +111,12 @@ static void update_callback(struct diff_queue_struct *q,
 	}
 }
 
-void add_files_to_cache(int verbose, const char *prefix, const char **files)
+void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)
 {
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = get_pathspec(prefix, files);
+	rev.prune_data = pathspec;
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &verbose;
@@ -198,9 +198,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	newfd = hold_locked_index(&lock_file, 1);
 
 	if (take_worktree_changes) {
+		const char **pathspec;
 		if (read_cache() < 0)
 			die("index file corrupt");
-		add_files_to_cache(verbose, prefix, argv);
+		pathspec = get_pathspec(prefix, argv);
+		add_files_to_cache(verbose, prefix, pathspec);
 		goto finish;
 	}
 
diff --git a/cache.h b/cache.h
index a84f343..24e7b72 100644
--- a/cache.h
+++ b/cache.h
@@ -597,13 +597,15 @@ extern void trace_argv_printf(const char **argv, int count, const char *format,
 extern int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst);
 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
 
+/* add */
+void add_files_to_cache(int verbose, const char *prefix, const char **pathspec);
+
 /* diff.c */
 extern int diff_auto_refresh_index;
 
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
 
-
 /* ls-files */
 int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
 int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset);
diff --git a/commit.h b/commit.h
index 13b5372..e22aa77 100644
--- a/commit.h
+++ b/commit.h
@@ -132,7 +132,6 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 int in_merge_bases(struct commit *, struct commit **, int);
 
 extern int interactive_add(void);
-extern void add_files_to_cache(int verbose, const char *prefix, const char **files);
 extern int rerere(void);
 
 #endif /* COMMIT_H */
-- 
1.5.3.5.1815.g9445b

[PATCH] builtin-commit: fix partial-commit support

[Junio C Hamano]

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.

Additionally:

 - 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>
---
 builtin-commit.c |  245 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 205 insertions(+), 40 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 3e7d281..e487bc0 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,182 @@ 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)
+{
+	struct dir_struct dir;
+	int i;
+	char *m;
+
+	for (i = 0; pattern[i]; i++)
+		;
+	m = xcalloc(1, i);
+
+	memset(&dir, 0, sizeof(dir));
+	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;
+		if (excluded(&dir, ce->name))
+			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_NORMAL;
 		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 [NEEDSWORK]
+	 * (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, but keep the addition;
+	 * (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
+	 * (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 +269,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)
@@ -437,7 +581,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;
 }
@@ -527,23 +671,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));
 
@@ -592,20 +749,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", commit_editmsg))
+	}
+	if (run_hook(index_file, "commit-msg", 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,
@@ -620,21 +784,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);
 
-- 
1.5.3.5.1815.g9445b

Re: [PATCH] builtin-commit: fix partial-commit support

[Junio C Hamano]

The above four are to be applied on top of kh/commit series,
(without the WIP I sent out last night) on top of Dscho's
"Replace "runstatus" with "status" in the tests" patch.

With these, all tests in t/ finally passes for me ;-)

But I am not a heavy user of partial commits nor commits from
subdirectories, so there may still be breakages.

Re: [PATCH] builtin-commit: fix partial-commit support

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> ...
> Additionally:

   - 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.

>  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_NORMAL;

This should be COMMIT_AS_IS, to match the scripted version.

> +	/*
> +	 * A partial commit.
> +	 *
> +	 * (0) find the set of affected paths [NEEDSWORK]

This [NEEDSWORK] has been resolved.

> +	 * (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, but keep the addition;

This ", but keep the addition" is no longer necessary; the set
of paths discovered in step (0) _will_ include the added paths
that are named by the user in **files.

> +	 * (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 the commit using the false index", of course.

> +	 * (A) if all goes well, commit the real index;
> +	 * (B) on failure, rollback the real index;
> +	 * In either case, rollback the false index.
> +	 */

Re: [PATCH] builtin-commit: fix partial-commit support

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> diff --git a/builtin-commit.c b/builtin-commit.c
> index 3e7d281..e487bc0 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c

There was another...

> +static int list_paths(struct path_list *list, const char *with_tree,
> +		      const char *prefix, const char **pattern)
> +{
> +	struct dir_struct dir;
> +	int i;
> +	char *m;
> +
> +	for (i = 0; pattern[i]; i++)
> +		;
> +	m = xcalloc(1, i);
> +
> +	memset(&dir, 0, sizeof(dir));
> +	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;
> +		if (excluded(&dir, ce->name))
> +			continue;
> +		path_list_insert(ce->name, list);
> +	}

This "excluded", and the whole "struct dir" business is
unneeded, as we are walking on the index and finding the set of
paths the user cares about.

Re: [PATCH] builtin-commit: fix partial-commit support

[Kristian Høgsberg]

On Sun, 2007-11-18 at 02:21 -0800, Junio C Hamano wrote:
> 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.

Oh boy, this turns out to be much more involved than what I thought.
Thanks for fixing up this part of builtin-commit.

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

Hmm, I see the left-over next-index-* files too, but I though the lock
file would be cleaned up automatically by the atexit handler?

cheers,
Kristian