diff options
author | Russell Belfer <rb@github.com> | 2014-12-30 11:10:10 -0800 |
---|---|---|
committer | Russell Belfer <rb@github.com> | 2014-12-30 11:14:17 -0800 |
commit | e5209da35f5089d292a2c4cd525e0d52a81dffc5 (patch) | |
tree | 1a2d38f6a3000cf2a57aa80caefd7518a2dbcbd8 | |
parent | a3ef70bb405a05c16fb533d828da2510ff751224 (diff) | |
download | libgit2-e5209da35f5089d292a2c4cd525e0d52a81dffc5.tar.gz |
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).
-rw-r--r-- | include/git2/filter.h | 12 | ||||
-rw-r--r-- | include/git2/sys/filter.h | 29 | ||||
-rw-r--r-- | src/blob.c | 5 | ||||
-rw-r--r-- | src/filter.c | 88 | ||||
-rw-r--r-- | tests/filter/custom.c | 115 |
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")); +} + |