Add filter flag to prevent preloading workdir datarb/filter-options

This bumps the custom filter structure definition and adds flags
to the new version.

This also adds support for a flag that says that a custom filter
can support directly reading working directory file content instead
of having the libgit2 filter framework preread the file data.  This
allows a filter to skip loading the on-disk data entirely into
memory when going in the TO_ODB direction (provided that the filter
is applied first the chain of filters).

5 files changed, 205 insertions, 44 deletions

diff --git a/include/git2/filter.h b/include/git2/filter.h
index 5b3f40394..0cab13359 100644
--- a/include/git2/filter.h
+++ b/include/git2/filter.h
@@ -36,7 +36,17 @@ typedef enum {
 } git_filter_mode_t;
 
 /**
- * Filter option flags.
+ * Option flags to alter filter application rules.
+ *
+ * A combination of these flags can be passed in when a filter is applied
+ * to change the behavior of certain filters.  The supported values are:
+ *
+ * - GIT_FILTER_OPT_DEFAULT: no special behavior, the default
+ *
+ * - GIT_FILTER_OPT_ALLOW_UNSAFE: normally, the CRLF filter raises an
+ *   error if you attempt to add an invalid CRLF character combination to
+ *   the index with `core.safecrlf` set; this downgrades that to a warning
+ *   so actions like diff are still possible.
  */
 typedef enum {
 	GIT_FILTER_OPT_DEFAULT = 0u,
diff --git a/include/git2/sys/filter.h b/include/git2/sys/filter.h
index 60248271a..2e7f15441 100644
--- a/include/git2/sys/filter.h
+++ b/include/git2/sys/filter.h
@@ -139,6 +139,23 @@ GIT_EXTERN(uint32_t) git_filter_source_options(const git_filter_source *src);
  */
 
 /**
+ * Options to alter filter behavior.
+ *
+ * There are options that affect filter behavior that are intrinsic to the
+ * definition of the filter (as opposed to the `git_filter_opt_t` values
+ * that are specific to the individual application of the filter).
+ *
+ * - GIT_FILTER_DONT_PRELOAD_WORKDIR: if this filter is the first in a
+ *   filter chain, when applying with `GIT_FILTER_TO_ODB` don't load the
+ *   file contents from the working directory; instead the `apply`
+ *   callback will get an empty `git_buf` and the filter must process the
+ *   on-disk data by itself.
+ */
+typedef enum {
+	GIT_FILTER_DONT_PRELOAD_WORKDIR = (1u << 0),
+} git_filter_flag_t;
+
+/**
  * Initialize callback on filter
  *
  * Specified as `filter.initialize`, this is an optional callback invoked
@@ -148,6 +165,9 @@ GIT_EXTERN(uint32_t) git_filter_source_options(const git_filter_source *src);
  * before the first use of the filter, so you can defer expensive
  * initialization operations (in case libgit2 is being used in a way that
  * doesn't need the filter).
+ *
+ * @param filter The `git_filter` object that was registered.
+ * @return 0 on success, negative on failure (which deregisters the filter)
  */
 typedef int (*git_filter_init_fn)(git_filter *self);
 
@@ -237,6 +257,9 @@ typedef void (*git_filter_cleanup_fn)(
  *
  * The `initialize`, `shutdown`, `check`, `apply`, and `cleanup` callbacks
  * are all documented above with the respective function pointer typedefs.
+ *
+ * `flags` is a combination of `git_filter_flag_t` values that alter how
+ * the filter will be used.  See individual flag values for details.
  */
 struct git_filter {
 	unsigned int           version;
@@ -248,9 +271,13 @@ struct git_filter {
 	git_filter_check_fn    check;
 	git_filter_apply_fn    apply;
 	git_filter_cleanup_fn  cleanup;
+
+	/* added in VERSION 2 */
+
+	uint32_t               flags; /* git_filter_flag_t values */
 };
 
-#define GIT_FILTER_VERSION 1
+#define GIT_FILTER_VERSION 2
 
 /**
  * Register a filter under a given name with a given priority.
diff --git a/src/blob.c b/src/blob.c
index 30d5b705b..1bfd2b0eb 100644
--- a/src/blob.c
+++ b/src/blob.c
@@ -109,6 +109,7 @@ static int write_file_stream(
 static int write_file_filtered(
 	git_oid *id,
 	git_off_t *size,
+	git_repository *repo,
 	git_odb *odb,
 	const char *full_path,
 	git_filter_list *fl)
@@ -116,7 +117,7 @@ static int write_file_filtered(
 	int error;
 	git_buf tgt = GIT_BUF_INIT;
 
-	error = git_filter_list_apply_to_file(&tgt, fl, NULL, full_path);
+	error = git_filter_list_apply_to_file(&tgt, fl, repo, full_path);
 
 	/* Write the file to disk if it was properly filtered */
 	if (!error) {
@@ -209,7 +210,7 @@ int git_blob__create_from_paths(
 			error = write_file_stream(id, odb, content_path, size);
 		else {
 			/* We need to apply one or more filters */
-			error = write_file_filtered(id, &size, odb, content_path, fl);
+			error = write_file_filtered(id, &size, repo, odb, content_path, fl);
 
 			git_filter_list_free(fl);
 		}
diff --git a/src/filter.c b/src/filter.c
index b5a8bdd66..cee58389e 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -18,12 +18,12 @@
 #include "array.h"
 
 struct git_filter_source {
-	git_repository *repo;
-	const char     *path;
-	git_oid         oid;  /* zero if unknown (which is likely) */
+	git_repository *repo;     /* repository being filtered */
+	const char     *path;     /* relative path of file in repo */
+	git_oid         oid;      /* zero if unknown (which is likely) */
 	uint16_t        filemode; /* zero if unknown */
-	git_filter_mode_t mode;
-	uint32_t        options;
+	git_filter_mode_t mode;   /* direction in which filter is being applied */
+	uint32_t        options;  /* git_filter_opt_t flags for filter apply */
 };
 
 typedef struct {
@@ -224,11 +224,30 @@ static git_filter_def *filter_registry_lookup(size_t *pos, const char *name)
 	return fdef;
 }
 
+static void filter_registry_remove_at(size_t pos)
+{
+	git_filter_def *fdef = git_vector_get(&git__filter_registry->filters, pos);
+	if (!fdef)
+		return;
+
+	(void)git_vector_remove(&git__filter_registry->filters, pos);
+
+	if (fdef->initialized) {
+		if (fdef->filter && fdef->filter->shutdown)
+			fdef->filter->shutdown(fdef->filter);
+		fdef->initialized = false;
+	}
+
+	git__free(fdef->filter_name);
+	git__free(fdef->attrdata);
+	git__free(fdef);
+}
+
 int git_filter_register(
 	const char *name, git_filter *filter, int priority)
 {
 	git_filter_def *fdef;
-	size_t nattr = 0, nmatch = 0;
+	size_t pos, nattr = 0, nmatch = 0;
 	git_buf attrs = GIT_BUF_INIT;
 
 	assert(name && filter);
@@ -236,15 +255,29 @@ int git_filter_register(
 	if (filter_registry_initialize() < 0)
 		return -1;
 
-	if (!filter_registry_find(NULL, name)) {
-		giterr_set(
-			GITERR_FILTER, "Attempt to reregister existing filter '%s'", name);
-		return GIT_EEXISTS;
+	GITERR_CHECK_VERSION(filter, GIT_FILTER_VERSION, "git_filter");
+
+	if ((fdef = filter_registry_lookup(&pos, name)) != NULL) {
+		const char *msg = NULL;
+
+		if (fdef->filter->version == filter->version)
+			msg = "Attempt to reregister existing filter '%s'";
+		else if (fdef->filter->version > filter->version)
+			msg = "Attempt to register older version of existing filter '%s'";
+		/* else allow replacing with a newer version */
+
+		if (msg) {
+			giterr_set(GITERR_FILTER, msg, name);
+			return GIT_EEXISTS;
+		}
 	}
 
 	if (filter_def_scan_attrs(&attrs, &nattr, &nmatch, filter->attributes) < 0)
 		return -1;
 
+	if (fdef != NULL)
+		filter_registry_remove_at(pos);
+
 	fdef = git__calloc(
 		sizeof(git_filter_def) + 2 * nattr * sizeof(char *), 1);
 	GITERR_CHECK_ALLOC(fdef);
@@ -289,16 +322,7 @@ int git_filter_unregister(const char *name)
 		return GIT_ENOTFOUND;
 	}
 
-	(void)git_vector_remove(&git__filter_registry->filters, pos);
-
-	if (fdef->initialized && fdef->filter && fdef->filter->shutdown) {
-		fdef->filter->shutdown(fdef->filter);
-		fdef->initialized = false;
-	}
-
-	git__free(fdef->filter_name);
-	git__free(fdef->attrdata);
-	git__free(fdef);
+	filter_registry_remove_at(pos);
 
 	return 0;
 }
@@ -658,22 +682,34 @@ cleanup:
 
 int git_filter_list_apply_to_file(
 	git_buf *out,
-	git_filter_list *filters,
+	git_filter_list *fl,
 	git_repository *repo,
 	const char *path)
 {
 	int error;
 	const char *base = repo ? git_repository_workdir(repo) : NULL;
 	git_buf abspath = GIT_BUF_INIT, raw = GIT_BUF_INIT;
+	git_filter_entry *first;
 
-	if (!(error = git_path_join_unrooted(&abspath, path, base, NULL)) &&
-		!(error = git_futils_readbuffer(&raw, abspath.ptr)))
-	{
-		error = git_filter_list_apply_to_data(out, filters, &raw);
+	if ((error = git_path_join_unrooted(&abspath, path, base, NULL)) < 0)
+		return error;
 
-		git_buf_free(&raw);
+	/* skip file load if TO_ODB and first filter has DONT_PRELOAD_WORKDIR */
+	if (!fl ||
+		fl->source.mode != GIT_FILTER_TO_ODB ||
+		!git_array_size(fl->filters) ||
+		!(first = git_array_get(fl->filters, 0)) ||
+		first->filter->version < 2 ||
+		(first->filter->flags & GIT_FILTER_DONT_PRELOAD_WORKDIR) == 0)
+	{
+		if ((error = git_futils_readbuffer(&raw, abspath.ptr)) < 0)
+			goto done;
 	}
 
+	error = git_filter_list_apply_to_data(out, fl, &raw);
+
+done:
+	git_buf_free(&raw);
 	git_buf_free(&abspath);
 	return error;
 }
diff --git a/tests/filter/custom.c b/tests/filter/custom.c
index 0fd7c3744..f0ed9268d 100644
--- a/tests/filter/custom.c
+++ b/tests/filter/custom.c
@@ -54,7 +54,8 @@ void test_filter_custom__initialize(void)
 		"hero* bitflip reverse\n"
 		"herofile text\n"
 		"heroflip -reverse binary\n"
-		"*.bin binary\n");
+		"*.bin binary\n"
+		"bigfile filter=no-pre-read\n");
 }
 
 void test_filter_custom__cleanup(void)
@@ -167,23 +168,23 @@ static git_filter *create_reverse_filter(const char *attrs)
 static void register_custom_filters(void)
 {
 	static int filters_registered = 0;
+	if (filters_registered)
+		return;
 
-	if (!filters_registered) {
-		cl_git_pass(git_filter_register(
-			"bitflip", create_bitflip_filter(), BITFLIP_FILTER_PRIORITY));
+	filters_registered = 1;
 
-		cl_git_pass(git_filter_register(
-			"reverse", create_reverse_filter("+reverse"),
-			REVERSE_FILTER_PRIORITY));
+	cl_git_pass(git_filter_register(
+		"bitflip", create_bitflip_filter(), BITFLIP_FILTER_PRIORITY));
 
-		/* re-register reverse filter with standard filter=xyz priority */
-		cl_git_pass(git_filter_register(
-			"pre-reverse",
-			create_reverse_filter("+prereverse"),
-			GIT_FILTER_DRIVER_PRIORITY));
+	cl_git_pass(git_filter_register(
+		"reverse", create_reverse_filter("+reverse"),
+		REVERSE_FILTER_PRIORITY));
 
-		filters_registered = 1;
-	}
+	/* re-register reverse filter with standard filter=xyz priority */
+	cl_git_pass(git_filter_register(
+		"pre-reverse",
+		create_reverse_filter("+prereverse"),
+		GIT_FILTER_DRIVER_PRIORITY));
 }
 
 
@@ -329,10 +330,96 @@ void test_filter_custom__order_dependency(void)
 void test_filter_custom__filter_registry_failure_cases(void)
 {
 	git_filter fake = { GIT_FILTER_VERSION, 0 };
+	git_filter newer_version = { GIT_FILTER_VERSION + 1, 0 };
+	git_filter older_version = { GIT_FILTER_VERSION - 1, 0 };
 
+	/* can't replace a registered filter with the same version */
 	cl_assert_equal_i(GIT_EEXISTS, git_filter_register("bitflip", &fake, 0));
 
+	/* can't replace the builtin filters */
 	cl_git_fail(git_filter_unregister(GIT_FILTER_CRLF));
 	cl_git_fail(git_filter_unregister(GIT_FILTER_IDENT));
+
+	/* can't unregister a non-registered filter */
 	cl_assert_equal_i(GIT_ENOTFOUND, git_filter_unregister("not-a-filter"));
+
+	/* can't register a version that is too new */
+	cl_git_fail(git_filter_register("version-too-new", &newer_version, 0));
+
+	/* can replace old with new, but not new with old */
+	cl_git_pass(git_filter_register("test-replaceable", &older_version, 0));
+	cl_git_pass(git_filter_register("test-replaceable", &fake, 0));
+	cl_git_fail(git_filter_register("test-replaceable", &older_version, 0));
+	cl_git_pass(git_filter_unregister("test-replaceable"));
+}
+
+static char *no_preread_data = "not the disk data";
+
+static int no_preread_filter_apply(
+	git_filter     *self,
+	void          **payload,
+	git_buf        *to,
+	const git_buf  *from,
+	const git_filter_source *source)
+{
+	GIT_UNUSED(self); GIT_UNUSED(payload);
+
+	/* verify that attribute path match worked as expected */
+	cl_assert_equal_i(
+		0, git__strncmp("bigfile", git_filter_source_path(source), 4));
+
+	if (git_filter_source_mode(source) == GIT_FILTER_TO_ODB) {
+		/* here is where we would read the file ourselves;
+		 * instead return status data
+		 */
+		cl_assert_equal_sz(0, from->size);
+		cl_git_pass(git_buf_sets(to, no_preread_data));
+		return 0;
+	}
+
+	return GIT_PASSTHROUGH;
+}
+
+static void no_preread_filter_free(git_filter *f)
+{
+	git__free(f);
+}
+
+static git_filter *create_no_preread_filter(void)
+{
+	git_filter *filter = git__calloc(1, sizeof(git_filter));
+	cl_assert(filter);
+
+	filter->version = GIT_FILTER_VERSION;
+	filter->attributes = "filter=no-pre-read";
+	filter->shutdown = no_preread_filter_free;
+	filter->apply = no_preread_filter_apply;
+	filter->flags = GIT_FILTER_DONT_PRELOAD_WORKDIR;
+
+	return filter;
 }
+
+void test_filter_custom__filter_can_read_workdir_data_on_its_own(void)
+{
+	git_filter_list *fl;
+	git_buf out = { 0 };
+
+	cl_git_pass(git_filter_register(
+		"no-pre-read", create_no_preread_filter(), GIT_FILTER_DRIVER_PRIORITY));
+
+	cl_git_mkfile(
+		"empty_standard_repo/bigfile", "the actual data on disk\n");
+
+	cl_git_pass(git_filter_list_load(
+		&fl, g_repo, NULL, "bigfile", GIT_FILTER_TO_ODB, 0));
+
+	cl_git_pass(git_filter_list_apply_to_file(&out, fl, g_repo, "bigfile"));
+
+	cl_assert_equal_s(no_preread_data, out.ptr);
+
+	git_filter_list_free(fl);
+	git_buf_free(&out);
+
+	cl_git_pass(git_filter_unregister("no-pre-read"));
+}
+