Commit c16556aaddffc1d663c6403747d793adc0819e0a

Patrick Steinhardt 2017-11-12T10:31:48

indexer: introduce options struct to `git_indexer_new` We strive to keep an options structure to many functions to be able to extend options in the future without breaking the API. `git_indexer_new` doesn't have one right now, but we want to be able to add an option for enabling strict packfile verification. Add a new `git_indexer_options` structure and adjust callers to use that.

diff --git a/examples/network/index-pack.c b/examples/network/index-pack.c
index 314f211..5962f90 100644
--- a/examples/network/index-pack.c
+++ b/examples/network/index-pack.c
@@ -46,7 +46,7 @@ int index_pack(git_repository *repo, int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	if (git_indexer_new(&idx, ".", 0, NULL, NULL, NULL) < 0) {
+	if (git_indexer_new(&idx, ".", 0, NULL, NULL) < 0) {
 		puts("bad idx");
 		return -1;
 	}
diff --git a/include/git2/indexer.h b/include/git2/indexer.h
index d2d315e..53a59fc 100644
--- a/include/git2/indexer.h
+++ b/include/git2/indexer.h
@@ -15,6 +15,30 @@ GIT_BEGIN_DECL
 
 typedef struct git_indexer git_indexer;
 
+typedef struct git_indexer_options {
+	unsigned int version;
+
+	/** progress_cb function to call with progress information */
+	git_transfer_progress_cb progress_cb;
+	/** progress_cb_payload payload for the progress callback */
+	void *progress_cb_payload;
+} git_indexer_options;
+
+#define GIT_INDEXER_OPTIONS_VERSION 1
+#define GIT_INDEXER_OPTIONS_INIT { GIT_INDEXER_OPTIONS_VERSION }
+
+/**
+ * Initializes a `git_indexer_options` with default values. Equivalent to
+ * creating an instance with GIT_INDEXER_OPTIONS_INIT.
+ *
+ * @param opts the `git_indexer_options` struct to initialize.
+ * @param version Version of struct; pass `GIT_INDEXER_OPTIONS_VERSION`
+ * @return Zero on success; -1 on failure.
+ */
+GIT_EXTERN(int) git_indexer_init_options(
+	git_indexer_options *opts,
+	unsigned int version);
+
 /**
  * Create a new indexer instance
  *
@@ -24,16 +48,15 @@ typedef struct git_indexer git_indexer;
  * @param odb object database from which to read base objects when
  * fixing thin packs. Pass NULL if no thin pack is expected (an error
  * will be returned if there are bases missing)
- * @param progress_cb function to call with progress information
- * @param progress_cb_payload payload for the progress callback
+ * @param opts Optional structure containing additional options. See
+ * `git_indexer_options` above.
  */
 GIT_EXTERN(int) git_indexer_new(
 		git_indexer **out,
 		const char *path,
 		unsigned int mode,
 		git_odb *odb,
-		git_transfer_progress_cb progress_cb,
-		void *progress_cb_payload);
+		git_indexer_options *opts);
 
 /**
  * Add data to the indexer
diff --git a/src/indexer.c b/src/indexer.c
index 1a59f03..fc9a3bb 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -113,24 +113,34 @@ static int objects_cmp(const void *a, const void *b)
 	return git_oid__cmp(&entrya->oid, &entryb->oid);
 }
 
+int git_indexer_init_options(git_indexer_options *opts, unsigned int version)
+{
+	GIT_INIT_STRUCTURE_FROM_TEMPLATE(
+		opts, version, git_indexer_options, GIT_INDEXER_OPTIONS_INIT);
+	return 0;
+}
+
 int git_indexer_new(
 		git_indexer **out,
 		const char *prefix,
 		unsigned int mode,
 		git_odb *odb,
-		git_transfer_progress_cb progress_cb,
-		void *progress_payload)
+		git_indexer_options *in_opts)
 {
+	git_indexer_options opts = GIT_INDEXER_OPTIONS_INIT;
 	git_indexer *idx;
 	git_buf path = GIT_BUF_INIT, tmp_path = GIT_BUF_INIT;
 	static const char suff[] = "/pack";
 	int error, fd = -1;
 
+	if (in_opts)
+		memcpy(&opts, in_opts, sizeof(opts));
+
 	idx = git__calloc(1, sizeof(git_indexer));
 	GITERR_CHECK_ALLOC(idx);
 	idx->odb = odb;
-	idx->progress_cb = progress_cb;
-	idx->progress_payload = progress_payload;
+	idx->progress_cb = opts.progress_cb;
+	idx->progress_payload = opts.progress_cb_payload;
 	idx->mode = mode ? mode : GIT_PACK_FILE_MODE;
 	git_hash_ctx_init(&idx->hash_ctx);
 	git_hash_ctx_init(&idx->trailer);
diff --git a/src/odb_pack.c b/src/odb_pack.c
index 2c30480..4107db6 100644
--- a/src/odb_pack.c
+++ b/src/odb_pack.c
@@ -519,6 +519,7 @@ static int pack_backend__writepack(struct git_odb_writepack **out,
 	git_transfer_progress_cb progress_cb,
 	void *progress_payload)
 {
+	git_indexer_options opts;
 	struct pack_backend *backend;
 	struct pack_writepack *writepack;
 
@@ -526,13 +527,16 @@ static int pack_backend__writepack(struct git_odb_writepack **out,
 
 	*out = NULL;
 
+	opts.progress_cb = progress_cb;
+	opts.progress_cb_payload = progress_payload;
+
 	backend = (struct pack_backend *)_backend;
 
 	writepack = git__calloc(1, sizeof(struct pack_writepack));
 	GITERR_CHECK_ALLOC(writepack);
 
 	if (git_indexer_new(&writepack->indexer,
-		backend->pack_folder, 0, odb, progress_cb, progress_payload) < 0) {
+		backend->pack_folder, 0, odb, &opts) < 0) {
 		git__free(writepack);
 		return -1;
 	}
diff --git a/src/pack-objects.c b/src/pack-objects.c
index c12f25e..2b786df 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -1388,6 +1388,7 @@ int git_packbuilder_write(
 	git_transfer_progress_cb progress_cb,
 	void *progress_cb_payload)
 {
+	git_indexer_options opts = GIT_INDEXER_OPTIONS_INIT;
 	git_indexer *indexer;
 	git_transfer_progress stats;
 	struct pack_write_context ctx;
@@ -1395,8 +1396,11 @@ int git_packbuilder_write(
 
 	PREPARE_PACK;
 
+	opts.progress_cb = progress_cb;
+	opts.progress_cb_payload = progress_cb_payload;
+
 	if (git_indexer_new(
-		&indexer, path, mode, pb->odb, progress_cb, progress_cb_payload) < 0)
+		&indexer, path, mode, pb->odb, &opts) < 0)
 		return -1;
 
 	if (!git_repository__cvar(&t, pb->repo, GIT_CVAR_FSYNCOBJECTFILES) && t)
diff --git a/tests/pack/indexer.c b/tests/pack/indexer.c
index 4533998..754b307 100644
--- a/tests/pack/indexer.c
+++ b/tests/pack/indexer.c
@@ -82,7 +82,7 @@ void test_pack_indexer__out_of_order(void)
 	git_indexer *idx = 0;
 	git_transfer_progress stats = { 0 };
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	cl_git_pass(git_indexer_append(
 		idx, out_of_order_pack, out_of_order_pack_len, &stats));
 	cl_git_pass(git_indexer_commit(idx, &stats));
@@ -99,7 +99,7 @@ void test_pack_indexer__missing_trailer(void)
 	git_indexer *idx = 0;
 	git_transfer_progress stats = { 0 };
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	cl_git_pass(git_indexer_append(
 		idx, missing_trailer_pack, missing_trailer_pack_len, &stats));
 	cl_git_fail(git_indexer_commit(idx, &stats));
@@ -115,7 +115,7 @@ void test_pack_indexer__leaky(void)
 	git_indexer *idx = 0;
 	git_transfer_progress stats = { 0 };
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	cl_git_pass(git_indexer_append(
 		idx, leaky_pack, leaky_pack_len, &stats));
 	cl_git_fail(git_indexer_commit(idx, &stats));
@@ -142,7 +142,7 @@ void test_pack_indexer__fix_thin(void)
 	git_oid_fromstr(&should_id, "e68fe8129b546b101aee9510c5328e7f21ca1d18");
 	cl_assert_equal_oid(&should_id, &id);
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, odb, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, odb, NULL));
 	cl_git_pass(git_indexer_append(idx, thin_pack, thin_pack_len, &stats));
 	cl_git_pass(git_indexer_commit(idx, &stats));
 
@@ -175,7 +175,7 @@ void test_pack_indexer__fix_thin(void)
 
 		cl_git_pass(p_stat(name, &st));
 
-		cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+		cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 		read = p_read(fd, buffer, sizeof(buffer));
 		cl_assert(read != -1);
 		p_close(fd);
@@ -208,7 +208,7 @@ void test_pack_indexer__corrupt_length(void)
 	git_oid_fromstr(&should_id, "e68fe8129b546b101aee9510c5328e7f21ca1d18");
 	cl_assert_equal_oid(&should_id, &id);
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, odb, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, odb, NULL));
 	cl_git_pass(git_indexer_append(
 		idx, corrupt_thin_pack, corrupt_thin_pack_len, &stats));
 	cl_git_fail(git_indexer_commit(idx, &stats));
@@ -252,7 +252,7 @@ void test_pack_indexer__no_tmp_files(void)
 	git_buf_dispose(&path);
 	cl_assert(git_buf_len(&first_tmp_file) == 0);
 
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	git_indexer_free(idx);
 
 	cl_git_pass(git_buf_sets(&path, clar_sandbox_path()));
diff --git a/tests/pack/packbuilder.c b/tests/pack/packbuilder.c
index 932cb38..bd2cebe 100644
--- a/tests/pack/packbuilder.c
+++ b/tests/pack/packbuilder.c
@@ -100,7 +100,7 @@ void test_pack_packbuilder__create_pack(void)
 
 	seed_packbuilder();
 
-	cl_git_pass(git_indexer_new(&_indexer, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&_indexer, ".", 0, NULL, NULL));
 	cl_git_pass(git_packbuilder_foreach(_packbuilder, feed_indexer, &stats));
 	cl_git_pass(git_indexer_commit(_indexer, &stats));
 
@@ -237,7 +237,7 @@ void test_pack_packbuilder__foreach(void)
 	git_indexer *idx;
 
 	seed_packbuilder();
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	cl_git_pass(git_packbuilder_foreach(_packbuilder, foreach_cb, idx));
 	cl_git_pass(git_indexer_commit(idx, &_stats));
 	git_indexer_free(idx);
@@ -255,7 +255,7 @@ void test_pack_packbuilder__foreach_with_cancel(void)
 	git_indexer *idx;
 
 	seed_packbuilder();
-	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL, NULL));
+	cl_git_pass(git_indexer_new(&idx, ".", 0, NULL, NULL));
 	cl_git_fail_with(
 		git_packbuilder_foreach(_packbuilder, foreach_cancel_cb, idx), -1111);
 	git_indexer_free(idx);