treebuilder: enhance attributes handling on insertion

3 files changed, 151 insertions, 5 deletions

diff --git a/include/git2/tree.h b/include/git2/tree.h
index 85407d7a..29aedacc 100644
--- a/include/git2/tree.h
+++ b/include/git2/tree.h
@@ -263,11 +263,17 @@ GIT_EXTERN(const git_tree_entry *) git_treebuilder_get(git_treebuilder *bld, con
  * The optional pointer `entry_out` can be used to retrieve a
  * pointer to the newly created/updated entry.
  *
+ * No attempt is being made to ensure that the provided oid points
+ * to an existing git object in the object database, nor that the
+ * attributes make sense regarding the type of the pointed at object.
+ *
  * @param entry_out Pointer to store the entry (optional)
  * @param bld Tree builder
  * @param filename Filename of the entry
  * @param id SHA1 oid of the entry
- * @param attributes Folder attributes of the entry
+ * @param attributes Folder attributes of the entry. This parameter must
+ *			be valued with one of the following entries: 0040000, 0100644,
+ *			0100755, 0120000 or 0160000.
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_treebuilder_insert(
diff --git a/src/tree.c b/src/tree.c
index 19250fe5..0eee9473 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -12,12 +12,16 @@
 #include "git2/object.h"
 
 #define DEFAULT_TREE_SIZE 16
-#define MAX_FILEMODE 0777777
 #define MAX_FILEMODE_BYTES 6
 
-static int valid_attributes(const int attributes)
+static bool valid_attributes(const int attributes)
 {
-	return attributes >= 0 && attributes <= MAX_FILEMODE;
+	return (attributes == 0040000	/* Directory */
+		|| attributes == 0100644 /* Non executable file */
+		|| attributes == 0100664 /* Non executable group writable file */
+		|| attributes == 0100755 /* Executable file */
+		|| attributes == 0120000 /* Symbolic link */
+		|| attributes == 0160000); /* Git link */
 }
 
 static int valid_entry_name(const char *filename)
@@ -513,6 +517,19 @@ static void sort_entries(git_treebuilder *bld)
 	git_vector_sort(&bld->entries);
 }
 
+GIT_INLINE(int) normalize_attributes(const int attributes)
+{
+	/* 100664 mode is an early design mistake. Tree entries may bear
+	 * this mode in some old git repositories, but it's now deprecated.
+	 * We silently normalize while inserting new entries in a tree 
+	 * being built.
+	 */
+	if (attributes == 0100664)
+		return 0100644;
+
+	return attributes;
+}
+
 int git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source)
 {
 	git_treebuilder *bld;
@@ -533,7 +550,10 @@ int git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source)
 		for (i = 0; i < source->entries.length; ++i) {
 			git_tree_entry *entry_src = source->entries.contents[i];
 
-			if (append_entry(bld, entry_src->filename, &entry_src->oid, entry_src->attr) < 0)
+			if (append_entry(
+				bld, entry_src->filename,
+				&entry_src->oid,
+				normalize_attributes(entry_src->attr)) < 0)
 				goto on_error;
 		}
 	}
@@ -561,6 +581,8 @@ int git_treebuilder_insert(
 	if (!valid_attributes(attributes))
 		return tree_error("Failed to insert entry. Invalid attributes");
 
+	attributes = normalize_attributes(attributes);
+
 	if (!valid_entry_name(filename))
 		return tree_error("Failed to insert entry. Invalid name for a tree entry");
 
diff --git a/tests-clar/object/tree/attributes.c b/tests-clar/object/tree/attributes.c
new file mode 100644
index 00000000..ed88e748
--- /dev/null
+++ b/tests-clar/object/tree/attributes.c
@@ -0,0 +1,118 @@
+#include "clar_libgit2.h"
+#include "tree.h"
+
+static const char *blob_oid = "3d0970ec547fc41ef8a5882dde99c6adce65b021";
+static const char *tree_oid  = "1b05fdaa881ee45b48cbaa5e9b037d667a47745e";
+
+#define GROUP_WRITABLE_FILE 0100664
+#define REGULAR_FILE 0100644
+
+void test_object_tree_attributes__ensure_correctness_of_attributes_on_insertion(void)
+{
+	git_treebuilder *builder;
+	git_oid oid;
+
+	cl_git_pass(git_oid_fromstr(&oid, blob_oid));
+
+	cl_git_pass(git_treebuilder_create(&builder, NULL));
+
+	cl_git_fail(git_treebuilder_insert(NULL, builder, "one.txt", &oid, 0777777));
+	cl_git_fail(git_treebuilder_insert(NULL, builder, "one.txt", &oid, 0100666));
+	cl_git_fail(git_treebuilder_insert(NULL, builder, "one.txt", &oid, 0000001));
+
+	git_treebuilder_free(builder);
+}
+
+void test_object_tree_attributes__group_writable_tree_entries_created_with_an_antique_git_version_can_still_be_accessed(void)
+{
+	git_repository *repo;
+	git_oid tid;
+	git_tree *tree;
+	const git_tree_entry *entry;
+
+	cl_git_pass(git_repository_open(&repo, cl_fixture("deprecated-mode.git")));
+
+	cl_git_pass(git_oid_fromstr(&tid, tree_oid));
+	cl_git_pass(git_tree_lookup(&tree, repo, &tid));
+
+	entry = git_tree_entry_byname(tree, "old_mode.txt");
+	cl_assert_equal_i(
+		GROUP_WRITABLE_FILE,
+		git_tree_entry_attributes(entry));
+
+	git_tree_free(tree);
+	git_repository_free(repo);
+}
+
+void test_object_tree_attributes__normalize_attributes_when_inserting_in_a_new_tree(void)
+{
+	git_repository *repo;
+	git_treebuilder *builder;
+	git_oid bid, tid;
+	git_tree *tree;
+	const git_tree_entry *entry;
+
+	repo = cl_git_sandbox_init("deprecated-mode.git");
+
+	cl_git_pass(git_oid_fromstr(&bid, blob_oid));
+
+	cl_git_pass(git_treebuilder_create(&builder, NULL));
+
+	cl_git_pass(git_treebuilder_insert(
+		&entry,
+		builder,
+		"normalized.txt",
+		&bid,
+		GROUP_WRITABLE_FILE));
+
+	cl_assert_equal_i(
+		REGULAR_FILE,
+		git_tree_entry_attributes(entry));
+	
+	cl_git_pass(git_treebuilder_write(&tid, repo, builder));
+	git_treebuilder_free(builder);
+
+	cl_git_pass(git_tree_lookup(&tree, repo, &tid));
+
+	entry = git_tree_entry_byname(tree, "normalized.txt");
+	cl_assert_equal_i(
+		REGULAR_FILE,
+		git_tree_entry_attributes(entry));
+
+	git_tree_free(tree);
+	cl_git_sandbox_cleanup();
+}
+
+void test_object_tree_attributes__normalize_attributes_when_creating_a_tree_from_an_existing_one(void)
+{
+	git_repository *repo;
+	git_treebuilder *builder;
+	git_oid tid, tid2;
+	git_tree *tree;
+	const git_tree_entry *entry;
+
+	repo = cl_git_sandbox_init("deprecated-mode.git");
+
+	cl_git_pass(git_oid_fromstr(&tid, tree_oid));
+	cl_git_pass(git_tree_lookup(&tree, repo, &tid));
+
+	cl_git_pass(git_treebuilder_create(&builder, tree));
+	
+	entry = git_treebuilder_get(builder, "old_mode.txt");
+	cl_assert_equal_i(
+		REGULAR_FILE,
+		git_tree_entry_attributes(entry));
+
+	cl_git_pass(git_treebuilder_write(&tid2, repo, builder));
+	git_treebuilder_free(builder);
+	git_tree_free(tree);
+
+	cl_git_pass(git_tree_lookup(&tree, repo, &tid2));
+	entry = git_tree_entry_byname(tree, "old_mode.txt");
+	cl_assert_equal_i(
+		REGULAR_FILE,
+		git_tree_entry_attributes(entry));
+
+	git_tree_free(tree);
+	cl_git_sandbox_cleanup();
+}