Merge pull request #775 from arrbee/fix-index-filemodes

Make index add/append support core.filemode flag

include/git2/index.h

@@ -90,6 +90,14 @@ typedef struct git_index_entry_unmerged {
 	char *path;
 } git_index_entry_unmerged;
 
+/** Capabilities of system that affect index actions. */
+enum {
+	GIT_INDEXCAP_IGNORE_CASE = 1,
+	GIT_INDEXCAP_NO_FILEMODE = 2,
+	GIT_INDEXCAP_NO_SYMLINKS = 4,
+	GIT_INDEXCAP_FROM_OWNER  = ~0u
+};
+
 /**
  * Create a new bare Git index object as a memory representation
  * of the Git index file in 'index_path', without a repository
@@ -127,6 +135,27 @@ GIT_EXTERN(void) git_index_clear(git_index *index);
 GIT_EXTERN(void) git_index_free(git_index *index);
 
 /**
+ * Read index capabilities flags.
+ *
+ * @param index An existing index object
+ * @return A combination of GIT_INDEXCAP values
+ */
+GIT_EXTERN(unsigned int) git_index_caps(const git_index *index);
+
+/**
+ * Set index capabilities flags.
+ *
+ * If you pass `GIT_INDEXCAP_FROM_OWNER` for the caps, then the
+ * capabilities will be read from the config of the owner object,
+ * looking at `core.ignorecase`, `core.filemode`, `core.symlinks`.
+ *
+ * @param index An existing index object
+ * @param caps A combination of GIT_INDEXCAP values
+ * @return 0 on success, -1 on failure
+ */
+GIT_EXTERN(int) git_index_set_caps(git_index *index, unsigned int caps);
+
+/**
  * Update the contents of an existing index object in memory
  * by reading from the hard disk.
  *

src/diff.c

@@ -456,10 +456,10 @@ static int maybe_modified(
 	if (!diff_path_matches_pathspec(diff, oitem->path))
 		return 0;
 
-	/* on platforms with no symlinks, promote plain files to symlinks */
+	/* on platforms with no symlinks, preserve mode of existing symlinks */
 	if (S_ISLNK(omode) && S_ISREG(nmode) &&
 		!(diff->diffcaps & GIT_DIFFCAPS_HAS_SYMLINKS))
-		nmode = GIT_MODE_TYPE(omode) | (nmode & GIT_MODE_PERMS_MASK);
+		nmode = omode;
 
 	/* on platforms with no execmode, just preserve old mode */
 	if (!(diff->diffcaps & GIT_DIFFCAPS_TRUST_MODE_BITS) &&

src/index.c

@@ -15,6 +15,7 @@
 #include "hash.h"
 #include "git2/odb.h"
 #include "git2/blob.h"
+#include "git2/config.h"
 
 #define entry_size(type,len) ((offsetof(type, path) + (len) + 8) & ~7)
 #define short_entry_size(len) entry_size(struct entry_short, len)
@@ -124,11 +125,27 @@ static unsigned int index_create_mode(unsigned int mode)
 {
 	if (S_ISLNK(mode))
 		return S_IFLNK;
+
 	if (S_ISDIR(mode) || (mode & S_IFMT) == (S_IFLNK | S_IFDIR))
 		return (S_IFLNK | S_IFDIR);
+
 	return S_IFREG | ((mode & 0100) ? 0755 : 0644);
 }
 
+static unsigned int index_merge_mode(
+	git_index *index, git_index_entry *existing, unsigned int mode)
+{
+	if (index->no_symlinks && S_ISREG(mode) &&
+		existing && S_ISLNK(existing->mode))
+		return existing->mode;
+
+	if (index->distrust_filemode && S_ISREG(mode))
+		return (existing && S_ISREG(existing->mode)) ?
+			existing->mode : index_create_mode(0666);
+
+	return index_create_mode(mode);
+}
+
 int git_index_open(git_index **index_out, const char *index_path)
 {
 	git_index *index;
@@ -208,6 +225,45 @@ void git_index_clear(git_index *index)
 	index->tree = NULL;
 }
 
+int git_index_set_caps(git_index *index, unsigned int caps)
+{
+	assert(index);
+
+	if (caps == GIT_INDEXCAP_FROM_OWNER) {
+		git_config *cfg;
+		int val;
+
+		if (INDEX_OWNER(index) == NULL ||
+			git_repository_config__weakptr(&cfg, INDEX_OWNER(index)) < 0)
+		{
+			giterr_set(GITERR_INDEX,
+				"Cannot get repository config to set index caps");
+			return -1;
+		}
+
+		if (git_config_get_bool(&val, cfg, "core.ignorecase") == 0)
+			index->ignore_case = (val != 0);
+		if (git_config_get_bool(&val, cfg, "core.filemode") == 0)
+			index->distrust_filemode = (val == 0);
+		if (git_config_get_bool(&val, cfg, "core.symlinks") == 0)
+			index->no_symlinks = (val != 0);
+	}
+	else {
+		index->ignore_case = ((caps & GIT_INDEXCAP_IGNORE_CASE) != 0);
+		index->distrust_filemode = ((caps & GIT_INDEXCAP_NO_FILEMODE) != 0);
+		index->no_symlinks = ((caps & GIT_INDEXCAP_NO_SYMLINKS) != 0);
+	}
+
+	return 0;
+}
+
+unsigned int git_index_caps(const git_index *index)
+{
+	return ((index->ignore_case ? GIT_INDEXCAP_IGNORE_CASE : 0) |
+			(index->distrust_filemode ? GIT_INDEXCAP_NO_FILEMODE : 0) |
+			(index->no_symlinks ? GIT_INDEXCAP_NO_SYMLINKS : 0));
+}
+
 int git_index_read(git_index *index)
 {
 	int error, updated;
@@ -383,7 +439,7 @@ static int index_insert(git_index *index, git_index_entry *entry, int replace)
 {
 	size_t path_length;
 	int position;
-	git_index_entry **entry_array;
+	git_index_entry **existing = NULL;
 
 	assert(index && entry && entry->path != NULL);
 
@@ -397,28 +453,24 @@ static int index_insert(git_index *index, git_index_entry *entry, int replace)
 	else
 		entry->flags |= GIT_IDXENTRY_NAMEMASK;;
 
-	/*
-	 * replacing is not requested: just insert entry at the end;
-	 * the index is no longer sorted
-	 */
-	if (!replace)
-		return git_vector_insert(&index->entries, entry);
-
 	/* look if an entry with this path already exists */
-	position = git_index_find(index, entry->path);
+	if ((position = git_index_find(index, entry->path)) >= 0) {
+		existing = (git_index_entry **)&index->entries.contents[position];
+
+		/* update filemode to existing values if stat is not trusted */
+		entry->mode = index_merge_mode(index, *existing, entry->mode);
+	}
 
-	/*
-	 * if no entry exists add the entry at the end;
-	 * the index is no longer sorted
+	/* if replacing is not requested or no existing entry exists, just
+	 * insert entry at the end; the index is no longer sorted
 	 */
-	if (position == GIT_ENOTFOUND)
+	if (!replace || !existing)
 		return git_vector_insert(&index->entries, entry);
 
 	/* exists, replace it */
-	entry_array = (git_index_entry **) index->entries.contents;
-	git__free(entry_array[position]->path);
-	git__free(entry_array[position]);
-	entry_array[position] = entry;
+	git__free((*existing)->path);
+	git__free(*existing);
+	*existing = entry;
 
 	return 0;
 }
@@ -475,7 +527,7 @@ int git_index_add2(git_index *index, const git_index_entry *source_entry)
 
 int git_index_append2(git_index *index, const git_index_entry *source_entry)
 {
-	return index_add2(index, source_entry, 1);
+	return index_add2(index, source_entry, 0);
 }
 
 int git_index_remove(git_index *index, int position)

src/index.h

@@ -26,6 +26,11 @@ struct git_index {
 	git_vector entries;
 
 	unsigned int on_disk:1;
+
+	unsigned int ignore_case:1;
+	unsigned int distrust_filemode:1;
+	unsigned int no_symlinks:1;
+
 	git_tree_cache *tree;
 
 	git_vector unmerged;

src/repository.c

@@ -573,6 +573,9 @@ int git_repository_index__weakptr(git_index **out, git_repository *repo)
 			return -1;
 
 		GIT_REFCOUNT_OWN(repo->_index, repo);
+
+		if (git_index_set_caps(repo->_index, GIT_INDEXCAP_FROM_OWNER) < 0)
+			return -1;
 	}
 
 	*out = repo->_index;

tests-clar/clar_helpers.c

@@ -28,9 +28,10 @@ void cl_git_mkfile(const char *filename, const char *content)
 	cl_must_pass(p_close(fd));
 }
 
-void cl_git_write2file(const char *filename, const char *new_content, int flags)
+void cl_git_write2file(
+	const char *filename, const char *new_content, int flags, unsigned int mode)
 {
-	int fd = p_open(filename, flags, 0644);
+	int fd = p_open(filename, flags, mode);
 	cl_assert(fd >= 0);
 	if (!new_content)
 		new_content = "\n";
@@ -40,12 +41,12 @@ void cl_git_write2file(const char *filename, const char *new_content, int flags)
 
 void cl_git_append2file(const char *filename, const char *new_content)
 {
-	cl_git_write2file(filename, new_content, O_WRONLY | O_CREAT | O_APPEND);
+	cl_git_write2file(filename, new_content, O_WRONLY | O_CREAT | O_APPEND, 0644);
 }
 
 void cl_git_rewritefile(const char *filename, const char *new_content)
 {
-	cl_git_write2file(filename, new_content, O_WRONLY | O_CREAT | O_TRUNC);
+	cl_git_write2file(filename, new_content, O_WRONLY | O_CREAT | O_TRUNC, 0644);
 }
 
 #ifdef GIT_WIN32

tests-clar/clar_libgit2.h

@@ -38,7 +38,7 @@
 void cl_git_mkfile(const char *filename, const char *content);
 void cl_git_append2file(const char *filename, const char *new_content);
 void cl_git_rewritefile(const char *filename, const char *new_content);
-void cl_git_write2file(const char *filename, const char *new_content, int mode);
+void cl_git_write2file(const char *filename, const char *new_content, int flags, unsigned int mode);
 
 bool cl_toggle_filemode(const char *filename);
 bool cl_is_chmod_supported(void);

tests-clar/index/filemodes.c

+#include "clar_libgit2.h"
+#include "buffer.h"
+#include "posix.h"
+#include "index.h"
+
+static git_repository *g_repo = NULL;
+
+void test_index_filemodes__initialize(void)
+{
+	g_repo = cl_git_sandbox_init("filemodes");
+}
+
+void test_index_filemodes__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_index_filemodes__read(void)
+{
+	git_index *index;
+	unsigned int i;
+	static bool expected[6] = { 0, 1, 0, 1, 0, 1 };
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_assert_equal_i(6, git_index_entrycount(index));
+
+	for (i = 0; i < 6; ++i) {
+		git_index_entry *entry = git_index_get(index, i);
+		cl_assert(entry != NULL);
+		cl_assert(((entry->mode & 0100) ? 1 : 0) == expected[i]);
+	}
+
+	git_index_free(index);
+}
+
+static void replace_file_with_mode(
+	const char *filename, const char *backup, unsigned int create_mode)
+{
+	git_buf path = GIT_BUF_INIT, content = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_joinpath(&path, "filemodes", filename));
+	cl_git_pass(git_buf_printf(&content, "%s as %08u (%d)",
+		filename, create_mode, rand()));
+
+	cl_git_pass(p_rename(path.ptr, backup));
+	cl_git_write2file(
+		path.ptr, content.ptr, O_WRONLY|O_CREAT|O_TRUNC, create_mode);
+
+	git_buf_free(&path);
+	git_buf_free(&content);
+}
+
+static void add_and_check_mode(
+	git_index *index, const char *filename, unsigned int expect_mode)
+{
+	int pos;
+	git_index_entry *entry;
+
+	cl_git_pass(git_index_add(index, filename, 0));
+
+	pos = git_index_find(index, filename);
+	cl_assert(pos >= 0);
+
+	entry = git_index_get(index, pos);
+	cl_assert(entry->mode == expect_mode);
+}
+
+static void append_and_check_mode(
+	git_index *index, const char *filename, unsigned int expect_mode)
+{
+	unsigned int before, after;
+	git_index_entry *entry;
+
+	before = git_index_entrycount(index);
+
+	cl_git_pass(git_index_append(index, filename, 0));
+
+	after = git_index_entrycount(index);
+	cl_assert_equal_i(before + 1, after);
+
+	/* bypass git_index_get since that resorts the index */
+	entry = (git_index_entry *)git_vector_get(&index->entries, after - 1);
+
+	cl_assert_equal_s(entry->path, filename);
+	cl_assert(expect_mode == entry->mode);
+}
+
+void test_index_filemodes__untrusted(void)
+{
+	git_config *cfg;
+	git_index *index;
+	bool can_filemode = cl_is_chmod_supported();
+
+	cl_git_pass(git_repository_config(&cfg, g_repo));
+	cl_git_pass(git_config_set_bool(cfg, "core.filemode", false));
+	git_config_free(cfg);
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_assert((git_index_caps(index) & GIT_INDEXCAP_NO_FILEMODE) != 0);
+
+	/* 1 - add 0644 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.0", 0644);
+	add_and_check_mode(index, "exec_off", 0100644);
+
+	/* 2 - add 0644 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.0", 0644);
+	add_and_check_mode(index, "exec_on", 0100755);
+
+	/* 3 - add 0755 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.1", 0755);
+	add_and_check_mode(index, "exec_off", 0100644);
+
+	/* 4 - add 0755 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.1", 0755);
+	add_and_check_mode(index, "exec_on", 0100755);
+
+	/* 5 - append 0644 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.2", 0644);
+	append_and_check_mode(index, "exec_off", 0100644);
+
+	/* 6 - append 0644 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.2", 0644);
+	append_and_check_mode(index, "exec_on", 0100755);
+
+	/* 7 - append 0755 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.3", 0755);
+	append_and_check_mode(index, "exec_off", 0100644);
+
+	/* 8 - append 0755 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.3", 0755);
+	append_and_check_mode(index, "exec_on", 0100755);
+
+	/*  9 - add new 0644 -> expect 0644 */
+	cl_git_write2file("filemodes/new_off", "blah",
+		O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	add_and_check_mode(index, "new_off", 0100644);
+
+	/* this test won't give predictable results on a platform
+	 * that doesn't support filemodes correctly, so skip it.
+	 */
+	if (can_filemode) {
+		/* 10 - add 0755 -> expect 0755 */
+		cl_git_write2file("filemodes/new_on", "blah",
+			O_WRONLY | O_CREAT | O_TRUNC, 0755);
+		add_and_check_mode(index, "new_on", 0100755);
+	}
+
+	git_index_free(index);
+}
+
+void test_index_filemodes__trusted(void)
+{
+	git_config *cfg;
+	git_index *index;
+
+	/* Only run these tests on platforms where I can actually
+	 * chmod a file and get the stat results I expect!
+	 */
+	if (!cl_is_chmod_supported())
+		return;
+
+	cl_git_pass(git_repository_config(&cfg, g_repo));
+	cl_git_pass(git_config_set_bool(cfg, "core.filemode", true));
+	git_config_free(cfg);
+
+	cl_git_pass(git_repository_index(&index, g_repo));
+	cl_assert((git_index_caps(index) & GIT_INDEXCAP_NO_FILEMODE) == 0);
+
+	/* 1 - add 0644 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.0", 0644);
+	add_and_check_mode(index, "exec_off", 0100644);
+
+	/* 2 - add 0644 over existing 0755 -> expect 0644 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.0", 0644);
+	add_and_check_mode(index, "exec_on", 0100644);
+
+	/* 3 - add 0755 over existing 0644 -> expect 0755 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.1", 0755);
+	add_and_check_mode(index, "exec_off", 0100755);
+
+	/* 4 - add 0755 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.1", 0755);
+	add_and_check_mode(index, "exec_on", 0100755);
+
+	/* 5 - append 0644 over existing 0644 -> expect 0644 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.2", 0644);
+	append_and_check_mode(index, "exec_off", 0100644);
+
+	/* 6 - append 0644 over existing 0755 -> expect 0644 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.2", 0644);
+	append_and_check_mode(index, "exec_on", 0100644);
+
+	/* 7 - append 0755 over existing 0644 -> expect 0755 */
+	replace_file_with_mode("exec_off", "filemodes/exec_off.3", 0755);
+	append_and_check_mode(index, "exec_off", 0100755);
+
+	/* 8 - append 0755 over existing 0755 -> expect 0755 */
+	replace_file_with_mode("exec_on", "filemodes/exec_on.3", 0755);
+	append_and_check_mode(index, "exec_on", 0100755);
+
+	/*  9 - add new 0644 -> expect 0644 */
+	cl_git_write2file("filemodes/new_off", "blah",
+		O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	add_and_check_mode(index, "new_off", 0100644);
+
+	/* 10 - add 0755 -> expect 0755 */
+	cl_git_write2file("filemodes/new_on", "blah",
+		O_WRONLY | O_CREAT | O_TRUNC, 0755);
+	add_and_check_mode(index, "new_on", 0100755);
+
+	git_index_free(index);
+}