Merge pull request #3502 from libgit2/cmn/createblob-stream

Add ability to write a filtered blob through a stream

CHANGELOG.md

@@ -8,8 +8,17 @@ v0.24 + 1
 * `git_commit_create_buffer()` creates a commit and writes it into a
   user-provided buffer instead of writing it into the object db.
 
+* `git_blob_create_fromstream()` and
+  `git_blob_create_fromstream_commit()` allow you to create a blob by
+  writing into a stream. Useful when you do not know the final size or
+  want to copy the contents from another stream.
+
 ### API removals
 
+* `git_blob_create_fromchunks()` has been removed in favour of
+  `git_blob_create_fromstream()`.
+
+
 ### Breaking API changes
 
 v0.24

include/git2/blob.h

@@ -192,6 +192,49 @@ GIT_EXTERN(int) git_blob_create_fromchunks(
 	void *payload);
 
 /**
+ * Create a stream to write a new blob into the object db
+ *
+ * This function may need to buffer the data on disk and will in
+ * general not be the right choice if you know the size of the data
+ * to write. If you have data in memory, use
+ * `git_blob_create_frombuffer()`. If you do not, but know the size of
+ * the contents (and don't want/need to perform filtering), use
+ * `git_odb_open_wstream()`.
+ *
+ * Don't close this stream yourself but pass it to
+ * `git_blob_create_fromstream_commit()` to commit the write to the
+ * object db and get the object id.
+ *
+ * If the `hintpath` parameter is filled, it will be used to determine
+ * what git filters should be applied to the object before it is written
+ * to the object database.
+ *
+ * @param out the stream into which to write
+ * @param repo Repository where the blob will be written.
+ *        This repository can be bare or not.
+ * @param hintpath If not NULL, will be used to select data filters
+ *        to apply onto the content of the blob to be created.
+ * @return 0 or error code
+ */
+GIT_EXTERN(int) git_blob_create_fromstream(
+	git_writestream **out,
+	git_repository *repo,
+	const char *hintpath);
+
+/**
+ * Close the stream and write the blob to the object db
+ *
+ * The stream will be closed and freed.
+ *
+ * @param out the id of the new blob
+ * @param stream the stream to close
+ * @return 0 or an error code
+ */
+GIT_EXTERN(int) git_blob_create_fromstream_commit(
+	git_oid *out,
+	git_writestream *stream);
+
+/**
  * Write an in-memory buffer to the ODB as a blob
  *
  * @param id return the id of the written blob

src/blob.c

@@ -274,64 +274,96 @@ int git_blob_create_fromdisk(
 	return error;
 }
 
-#define BUFFER_SIZE 4096
+typedef struct {
+	git_writestream parent;
+	git_filebuf fbuf;
+	git_repository *repo;
+	char *hintpath;
+} blob_writestream;
+
+static int blob_writestream_close(git_writestream *_stream)
+{
+	blob_writestream *stream = (blob_writestream *) _stream;
 
-int git_blob_create_fromchunks(
-	git_oid *id,
-	git_repository *repo,
-	const char *hintpath,
-	int (*source_cb)(char *content, size_t max_length, void *payload),
-	void *payload)
+	git_filebuf_cleanup(&stream->fbuf);
+	return 0;
+}
+
+static void blob_writestream_free(git_writestream *_stream)
+{
+	blob_writestream *stream = (blob_writestream *) _stream;
+
+	git_filebuf_cleanup(&stream->fbuf);
+	git__free(stream->hintpath);
+	git__free(stream);
+}
+
+static int blob_writestream_write(git_writestream *_stream, const char *buffer, size_t len)
+{
+	blob_writestream *stream = (blob_writestream *) _stream;
+
+	return git_filebuf_write(&stream->fbuf, buffer, len);
+}
+
+int git_blob_create_fromstream(git_writestream **out, git_repository *repo, const char *hintpath)
 {
 	int error;
-	char *content = NULL;
-	git_filebuf file = GIT_FILEBUF_INIT;
 	git_buf path = GIT_BUF_INIT;
+	blob_writestream *stream;
 
-	assert(id && repo && source_cb);
+	assert(out && repo);
 
-	if ((error = git_buf_joinpath(
-			&path, git_repository_path(repo), GIT_OBJECTS_DIR "streamed")) < 0)
-		goto cleanup;
+	stream = git__calloc(1, sizeof(blob_writestream));
+	GITERR_CHECK_ALLOC(stream);
 
-	content = git__malloc(BUFFER_SIZE);
-	GITERR_CHECK_ALLOC(content);
+	if (hintpath) {
+		stream->hintpath = git__strdup(hintpath);
+		GITERR_CHECK_ALLOC(stream->hintpath);
+	}
 
-	if ((error = git_filebuf_open(
-			&file, git_buf_cstr(&path), GIT_FILEBUF_TEMPORARY, 0666)) < 0)
+	stream->repo = repo;
+	stream->parent.write = blob_writestream_write;
+	stream->parent.close = blob_writestream_close;
+	stream->parent.free  = blob_writestream_free;
+
+	if ((error = git_buf_joinpath(&path,
+				      git_repository_path(repo), GIT_OBJECTS_DIR "streamed")) < 0)
 		goto cleanup;
 
-	while (1) {
-		int read_bytes = source_cb(content, BUFFER_SIZE, payload);
+	if ((error = git_filebuf_open_withsize(&stream->fbuf, git_buf_cstr(&path), GIT_FILEBUF_TEMPORARY,
+					       0666, 2 * 1024 * 1024)) < 0)
+		goto cleanup;
 
-		if (!read_bytes)
-			break;
+	*out = (git_writestream *) stream;
 
-		if (read_bytes > BUFFER_SIZE) {
-			giterr_set(GITERR_OBJECT, "Invalid chunk size while creating blob");
-			error = GIT_EBUFS;
-		} else if (read_bytes < 0) {
-			error = giterr_set_after_callback(read_bytes);
-		} else {
-			error = git_filebuf_write(&file, content, read_bytes);
-		}
+cleanup:
+	if (error < 0)
+		blob_writestream_free((git_writestream *) stream);
 
-		if (error < 0)
-			goto cleanup;
-	}
+	git_buf_free(&path);
+	return error;
+}
 
-	if ((error = git_filebuf_flush(&file)) < 0)
+int git_blob_create_fromstream_commit(git_oid *out, git_writestream *_stream)
+{
+	int error;
+	blob_writestream *stream = (blob_writestream *) _stream;
+
+	/*
+	 * We can make this more officient by avoiding writing to
+	 * disk, but for now let's re-use the helper functions we
+	 * have.
+	 */
+	if ((error = git_filebuf_flush(&stream->fbuf)) < 0)
 		goto cleanup;
 
-	error = git_blob__create_from_paths(
-		id, NULL, repo, file.path_lock, hintpath, 0, hintpath != NULL);
+	error = git_blob__create_from_paths(out, NULL, stream->repo, stream->fbuf.path_lock,
+					    stream->hintpath, 0, !!stream->hintpath);
 
 cleanup:
-	git_buf_free(&path);
-	git_filebuf_cleanup(&file);
-	git__free(content);
-
+	blob_writestream_free(_stream);
 	return error;
+
 }
 
 int git_blob_is_binary(const git_blob *blob)

src/filebuf.c

@@ -273,6 +273,11 @@ cleanup:
 
 int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode)
 {
+	return git_filebuf_open_withsize(file, path, flags, mode, WRITE_BUFFER_SIZE);
+}
+
+int git_filebuf_open_withsize(git_filebuf *file, const char *path, int flags, mode_t mode, size_t size)
+{
 	int compression, error = -1;
 	size_t path_len, alloc_len;
 
@@ -286,7 +291,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode
 	if (flags & GIT_FILEBUF_DO_NOT_BUFFER)
 		file->do_not_buffer = true;
 
-	file->buf_size = WRITE_BUFFER_SIZE;
+	file->buf_size = size;
 	file->buf_pos = 0;
 	file->fd = -1;
 	file->last_error = BUFERR_OK;

src/filebuf.h

@@ -79,6 +79,7 @@ int git_filebuf_reserve(git_filebuf *file, void **buff, size_t len);
 int git_filebuf_printf(git_filebuf *file, const char *format, ...) GIT_FORMAT_PRINTF(2, 3);
 
 int git_filebuf_open(git_filebuf *lock, const char *path, int flags, mode_t mode);
+int git_filebuf_open_withsize(git_filebuf *file, const char *path, int flags, mode_t mode, size_t size);
 int git_filebuf_commit(git_filebuf *lock);
 int git_filebuf_commit_at(git_filebuf *lock, const char *path);
 void git_filebuf_cleanup(git_filebuf *lock);

tests/object/blob/fromchunks.c → tests/object/blob/fromstream.c

@@ -7,12 +7,12 @@
 static git_repository *repo;
 static char textual_content[] = "libgit2\n\r\n\0";
 
-void test_object_blob_fromchunks__initialize(void)
+void test_object_blob_fromstream__initialize(void)
 {
 	repo = cl_git_sandbox_init("testrepo.git");
 }
 
-void test_object_blob_fromchunks__cleanup(void)
+void test_object_blob_fromstream__cleanup(void)
 {
 	cl_git_sandbox_cleanup();
 }
@@ -33,52 +33,29 @@ static int text_chunked_source_cb(char *content, size_t max_length, void *payloa
 	return (int)strlen(textual_content);
 }
 
-void test_object_blob_fromchunks__can_create_a_blob_from_a_in_memory_chunk_provider(void)
+void test_object_blob_fromstream__multiple_write(void)
 {
-	git_oid expected_oid, oid;
+	git_oid expected_id, id;
 	git_object *blob;
-	int howmany = 7;
+	git_writestream *stream;
+	int i, howmany = 6;
 
-	cl_git_pass(git_oid_fromstr(&expected_oid, "321cbdf08803c744082332332838df6bd160f8f9"));
+	cl_git_pass(git_oid_fromstr(&expected_id, "321cbdf08803c744082332332838df6bd160f8f9"));
 
-	cl_git_fail_with(
-		git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY),
-		GIT_ENOTFOUND);
+	cl_git_fail_with(GIT_ENOTFOUND,
+			 git_object_lookup(&blob, repo, &expected_id, GIT_OBJ_ANY));
 
-	cl_git_pass(git_blob_create_fromchunks(&oid, repo, NULL, text_chunked_source_cb, &howmany));
+	cl_git_pass(git_blob_create_fromstream(&stream, repo, NULL));
 
-	cl_git_pass(git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY));
-	cl_assert(git_oid_cmp(&expected_oid, git_object_id(blob)) == 0);
+	for (i = 0; i < howmany; i++)
+		cl_git_pass(stream->write(stream, textual_content, strlen(textual_content)));
 
-	git_object_free(blob);
-}
-
-void test_object_blob_fromchunks__doesnot_overwrite_an_already_existing_object(void)
-{
-	git_buf path = GIT_BUF_INIT;
-	git_buf content = GIT_BUF_INIT;
-	git_oid expected_oid, oid;
-	int howmany = 7;
-
-	cl_git_pass(git_oid_fromstr(&expected_oid, "321cbdf08803c744082332332838df6bd160f8f9"));
-
-	cl_git_pass(git_blob_create_fromchunks(&oid, repo, NULL, text_chunked_source_cb, &howmany));
-
-	/* Let's replace the content of the blob file storage with something else... */
-	cl_git_pass(git_buf_joinpath(&path, git_repository_path(repo), "objects/32/1cbdf08803c744082332332838df6bd160f8f9"));
-	cl_git_pass(p_unlink(git_buf_cstr(&path)));
-	cl_git_mkfile(git_buf_cstr(&path), "boom");
-
-	/* ...request a creation of the same blob... */
-	howmany = 7;
-	cl_git_pass(git_blob_create_fromchunks(&oid, repo, NULL, text_chunked_source_cb, &howmany));
+	cl_git_pass(git_blob_create_fromstream_commit(&id, stream));
+	cl_assert_equal_oid(&expected_id, &id);
 
-	/* ...and ensure the content of the faked blob file hasn't been altered */
-	cl_git_pass(git_futils_readbuffer(&content, git_buf_cstr(&path)));
-	cl_assert(!git__strcmp("boom", git_buf_cstr(&content)));
+	cl_git_pass(git_object_lookup(&blob, repo, &expected_id, GIT_OBJ_BLOB));
 
-	git_buf_free(&path);
-	git_buf_free(&content);
+	git_object_free(blob);
 }
 
 #define GITATTR "* text=auto\n" \
@@ -100,16 +77,23 @@ static void write_attributes(git_repository *repo)
 
 static void assert_named_chunked_blob(const char *expected_sha, const char *fake_name)
 {
-	git_oid expected_oid, oid;
-	int howmany = 7;
+	git_oid expected_id, id;
+	git_writestream *stream;
+	int i, howmany = 6;
+
+	cl_git_pass(git_oid_fromstr(&expected_id, expected_sha));
+
+	cl_git_pass(git_blob_create_fromstream(&stream, repo, fake_name));
+
+	for (i = 0; i < howmany; i++)
+		cl_git_pass(stream->write(stream, textual_content, strlen(textual_content)));
 
-	cl_git_pass(git_oid_fromstr(&expected_oid, expected_sha));
+	cl_git_pass(git_blob_create_fromstream_commit(&id, stream));
 
-	cl_git_pass(git_blob_create_fromchunks(&oid, repo, fake_name, text_chunked_source_cb, &howmany));
-	cl_assert(git_oid_cmp(&expected_oid, &oid) == 0);
+	cl_assert_equal_oid(&expected_id, &id);
 }
 
-void test_object_blob_fromchunks__creating_a_blob_from_chunks_honors_the_attributes_directives(void)
+void test_object_blob_fromstream__creating_a_blob_from_chunks_honors_the_attributes_directives(void)
 {
 	write_attributes(repo);
 
@@ -117,40 +101,3 @@ void test_object_blob_fromchunks__creating_a_blob_from_chunks_honors_the_attribu
 	assert_named_chunked_blob("e9671e138a780833cb689753570fd10a55be84fb", "dummy.txt");
 	assert_named_chunked_blob("e9671e138a780833cb689753570fd10a55be84fb", "dummy.dunno");
 }
-
-static int failing_chunked_source_cb(
-	char *content, size_t max_length, void *payload)
-{
-	int *count = (int *)payload;
-
-	GIT_UNUSED(max_length);
-
-	(*count)--;
-	if (*count == 0)
-		return -1234;
-
-	strcpy(content, textual_content);
-	return (int)strlen(textual_content);
-}
-
-void test_object_blob_fromchunks__can_stop_with_error(void)
-{
-	git_oid expected_oid, oid;
-	git_object *blob;
-	int howmany = 7;
-
-	cl_git_pass(git_oid_fromstr(
-		&expected_oid, "321cbdf08803c744082332332838df6bd160f8f9"));
-
-	cl_git_fail_with(
-		git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY),
-		GIT_ENOTFOUND);
-
-	cl_git_fail_with(git_blob_create_fromchunks(
-		&oid, repo, NULL, failing_chunked_source_cb, &howmany), -1234);
-
-	cl_git_fail_with(
-		git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY),
-		GIT_ENOTFOUND);
-}
-