Merge pull request libgit2#5815 from libgit2/ethomson/treebuilder_write

tree: deprecate `git_treebuilder_write_with_buffer`

Author: ethomson

.github/workflows/main.yml

@@ -123,6 +123,7 @@ jobs:
             SKIP_SSH_TESTS: true
             SKIP_NEGOTIATE_TESTS: true
             ASAN_SYMBOLIZER_PATH: /usr/bin/llvm-symbolizer-10
+            UBSAN_OPTIONS: print_stacktrace=1
           os: ubuntu-latest
         - # Focal, Clang 10, OpenSSL, UndefinedBehaviorSanitizer
           container:
@@ -135,6 +136,7 @@ jobs:
             SKIP_SSH_TESTS: true
             SKIP_NEGOTIATE_TESTS: true
             ASAN_SYMBOLIZER_PATH: /usr/bin/llvm-symbolizer-10
+            UBSAN_OPTIONS: print_stacktrace=1
           os: ubuntu-latest
         - # Focal, Clang 10, OpenSSL, ThreadSanitizer
           container:
@@ -147,6 +149,7 @@ jobs:
             SKIP_SSH_TESTS: true
             SKIP_NEGOTIATE_TESTS: true
             ASAN_SYMBOLIZER_PATH: /usr/bin/llvm-symbolizer-10
+            UBSAN_OPTIONS: print_stacktrace=1
             TSAN_OPTIONS: suppressions=/home/libgit2/source/script/thread-sanitizer.supp second_deadlock_stack=1
           os: ubuntu-latest
         - # macOS
@@ -242,6 +245,7 @@ jobs:
               -e SKIP_NEGOTIATE_TESTS \
               -e SKIP_SSH_TESTS \
               -e TSAN_OPTIONS \
+              -e UBSAN_OPTIONS \
               ${{ env.docker-registry-container-sha }} \
               /bin/bash -c "mkdir build && cd build && ../ci/build.sh && ../ci/test.sh"
         else

include/git2/deprecated.h

@@ -119,6 +119,33 @@ GIT_EXTERN(int) git_blob_filtered_content(
 
 /**@}*/
 
+/** @name Deprecated Tree Functions
+ *
+ * These functions are retained for backward compatibility.  The
+ * newer versions of these functions and values should be preferred
+ * in all new code.
+ *
+ * There is no plan to remove these backward compatibility values at
+ * this time.
+ */
+/**@{*/
+
+/**
+ * Write the contents of the tree builder as a tree object.
+ * This is an alias of `git_treebuilder_write` and is preserved
+ * for backward compatibility.
+ *
+ * This function is deprecated, but there is no plan to remove this
+ * function at this time.
+ *
+ * @deprecated Use git_treebuilder_write
+ * @see git_treebuilder_write
+ */
+GIT_EXTERN(int) git_treebuilder_write_with_buffer(
+	git_oid *oid, git_treebuilder *bld, git_buf *tree);
+
+/**@}*/
+
 /** @name Deprecated Buffer Functions
  *
  * These functions and enumeration values are retained for backward

include/git2/tree.h

@@ -378,20 +378,6 @@ GIT_EXTERN(int) git_treebuilder_filter(
 GIT_EXTERN(int) git_treebuilder_write(
 	git_oid *id, git_treebuilder *bld);
 
-/**
- * Write the contents of the tree builder as a tree object
- * using a shared git_buf.
- *
- * @see git_treebuilder_write
- *
- * @param oid Pointer to store the OID of the newly written tree
- * @param bld Tree builder to write
- * @param tree Shared buffer for writing the tree. Will be grown as necessary.
- * @return 0 or an error code
- */
-GIT_EXTERN(int) git_treebuilder_write_with_buffer(
-	git_oid *oid, git_treebuilder *bld, git_buf *tree);
-
 /** Callback for the tree traversal method */
 typedef int GIT_CALLBACK(git_treewalk_cb)(
 	const char *root, const git_tree_entry *entry, void *payload);

src/tree.c

@@ -492,6 +492,56 @@ static int check_entry(git_repository *repo, const char *filename, const git_oid
 	return 0;
 }
 
+static int git_treebuilder__write_with_buffer(
+	git_oid *oid,
+	git_treebuilder *bld,
+	git_buf *buf)
+{
+	int error = 0;
+	size_t i, entrycount;
+	git_odb *odb;
+	git_tree_entry *entry;
+	git_vector entries = GIT_VECTOR_INIT;
+
+	git_buf_clear(buf);
+
+	entrycount = git_strmap_size(bld->map);
+	if ((error = git_vector_init(&entries, entrycount, entry_sort_cmp)) < 0)
+		goto out;
+
+	if (buf->asize == 0 &&
+	    (error = git_buf_grow(buf, entrycount * 72)) < 0)
+		goto out;
+
+	git_strmap_foreach_value(bld->map, entry, {
+		if ((error = git_vector_insert(&entries, entry)) < 0)
+			goto out;
+	});
+
+	git_vector_sort(&entries);
+
+	for (i = 0; i < entries.length && !error; ++i) {
+		entry = git_vector_get(&entries, i);
+
+		git_buf_printf(buf, "%o ", entry->attr);
+		git_buf_put(buf, entry->filename, entry->filename_len + 1);
+		git_buf_put(buf, (char *)entry->oid->id, GIT_OID_RAWSZ);
+
+		if (git_buf_oom(buf)) {
+			error = -1;
+			goto out;
+		}
+	}
+
+	if ((error = git_repository_odb__weakptr(&odb, bld->repo)) == 0)
+		error = git_odb_write(oid, odb, buf->ptr, buf->size, GIT_OBJECT_TREE);
+
+out:
+	git_vector_free(&entries);
+
+	return error;
+}
+
 static int append_entry(
 	git_treebuilder *bld,
 	const char *filename,
@@ -610,7 +660,7 @@ static int write_tree(
 		}
 	}
 
-	if (git_treebuilder_write_with_buffer(oid, bld, shared_buf) < 0)
+	if (git_treebuilder__write_with_buffer(oid, bld, shared_buf) < 0)
 		goto on_error;
 
 	git_treebuilder_free(bld);
@@ -785,63 +835,10 @@ int git_treebuilder_remove(git_treebuilder *bld, const char *filename)
 
 int git_treebuilder_write(git_oid *oid, git_treebuilder *bld)
 {
-	int error;
-	git_buf buffer = GIT_BUF_INIT;
-
-	error = git_treebuilder_write_with_buffer(oid, bld, &buffer);
-
-	git_buf_dispose(&buffer);
-	return error;
-}
-
-int git_treebuilder_write_with_buffer(git_oid *oid, git_treebuilder *bld, git_buf *tree)
-{
-	int error = 0;
-	size_t i, entrycount;
-	git_odb *odb;
-	git_tree_entry *entry;
-	git_vector entries = GIT_VECTOR_INIT;
-
+	GIT_ASSERT_ARG(oid);
 	GIT_ASSERT_ARG(bld);
-	GIT_ASSERT_ARG(tree);
-
-	git_buf_clear(tree);
-
-	entrycount = git_strmap_size(bld->map);
-	if ((error = git_vector_init(&entries, entrycount, entry_sort_cmp)) < 0)
-		goto out;
-
-	if (tree->asize == 0 &&
-	    (error = git_buf_grow(tree, entrycount * 72)) < 0)
-		goto out;
-
-	git_strmap_foreach_value(bld->map, entry, {
-		if ((error = git_vector_insert(&entries, entry)) < 0)
-			goto out;
-	});
-
-	git_vector_sort(&entries);
 
-	for (i = 0; i < entries.length && !error; ++i) {
-		entry = git_vector_get(&entries, i);
-
-		git_buf_printf(tree, "%o ", entry->attr);
-		git_buf_put(tree, entry->filename, entry->filename_len + 1);
-		git_buf_put(tree, (char *)entry->oid->id, GIT_OID_RAWSZ);
-
-		if (git_buf_oom(tree)) {
-			error = -1;
-			goto out;
-		}
-	}
-
-	if ((error = git_repository_odb__weakptr(&odb, bld->repo)) == 0)
-		error = git_odb_write(oid, odb, tree->ptr, tree->size, GIT_OBJECT_TREE);
-
-out:
-	git_vector_free(&entries);
-
-	return error;
+	return git_treebuilder__write_with_buffer(oid, bld, &bld->write_cache);
 }
 
 int git_treebuilder_filter(
@@ -882,6 +879,7 @@ void git_treebuilder_free(git_treebuilder *bld)
 	if (bld == NULL)
 		return;
 
+	git_buf_dispose(&bld->write_cache);
 	git_treebuilder_clear(bld);
 	git_strmap_free(bld->map);
 	git__free(bld);
@@ -1306,3 +1304,16 @@ int git_tree_create_updated(git_oid *out, git_repository *repo, git_tree *baseli
 	git_vector_free(&entries);
 	return error;
 }
+
+/* Deprecated Functions */
+
+#ifndef GIT_DEPRECATE_HARD
+
+int git_treebuilder_write_with_buffer(git_oid *oid, git_treebuilder *bld, git_buf *buf)
+{
+	GIT_UNUSED(buf);
+
+	return git_treebuilder_write(oid, bld);
+}
+
+#endif

src/tree.h

@@ -32,6 +32,7 @@ struct git_tree {
 struct git_treebuilder {
 	git_repository *repo;
 	git_strmap *map;
+	git_buf write_cache;
 };
 
 GIT_INLINE(bool) git_tree_entry__is_tree(const struct git_tree_entry *e)