diff options
author | Edward Thomson <ethomson@github.com> | 2016-02-28 09:34:11 -0500 |
---|---|---|
committer | Edward Thomson <ethomson@github.com> | 2016-03-17 11:06:00 -0400 |
commit | 6d8b2cdbee00f2c4e97796b52e05dd39bd655138 (patch) | |
tree | a00499e01bdfa09dc75f90c71055d1b872bd6dd5 | |
parent | 3f7d3df1ecc0ed7c31427aa37a8de62026c7edef (diff) | |
download | libgit2-6d8b2cdbee00f2c4e97796b52e05dd39bd655138.tar.gz |
merge driver: remove `check` callback
Since the `apply` callback can defer, the `check` callback is not
necessary. Removing the `check` callback further makes the `payload`
unnecessary along with the `cleanup` callback.
-rw-r--r-- | include/git2/sys/merge.h | 68 | ||||
-rw-r--r-- | src/merge.c | 37 | ||||
-rw-r--r-- | src/merge.h | 6 | ||||
-rw-r--r-- | src/merge_driver.c | 121 | ||||
-rw-r--r-- | src/merge_driver.h | 24 | ||||
-rw-r--r-- | tests/merge/driver.c | 131 |
6 files changed, 81 insertions, 306 deletions
diff --git a/include/git2/sys/merge.h b/include/git2/sys/merge.h index bc9908e36..031941042 100644 --- a/include/git2/sys/merge.h +++ b/include/git2/sys/merge.h @@ -83,41 +83,7 @@ typedef int (*git_merge_driver_init_fn)(git_merge_driver *self); typedef void (*git_merge_driver_shutdown_fn)(git_merge_driver *self); /** - * Callback to decide if a given conflict can be resolved with this merge - * driver. - * - * Specified as `driver.check`, this is an optional callback that checks - * if the given conflict can be resolved with this merge driver. - * - * It should return 0 if the merge driver should be applied (i.e. success), - * `GIT_PASSTHROUGH` if the driver is not available, which is the equivalent - * of an unregistered or nonexistent merge driver. In this case, the default - * (`text`) driver will be run. This is useful if you register a wildcard - * merge driver but are not interested in handling the requested file (and - * should just fallback). The driver can also return `GIT_EMERGECONFLICT` - * if the driver is not able to produce a merge result, and the file will - * remain conflicted. Any other errors will fail and return to the caller. - * - * The `name` will be set to the name of the driver as configured in the - * attributes. - * - * The `src` contains the data about the file to be merged. - * - * The `payload` will be a pointer to a reference payload for the driver. - * This will start as NULL, but `check` can assign to this pointer for - * later use by the `apply` callback. Note that the value should be heap - * allocated (not stack), so that it doesn't go away before the `apply` - * callback can use it. If a driver allocates and assigns a value to the - * `payload`, it will need a `cleanup` callback to free the payload. - */ -typedef int (*git_merge_driver_check_fn)( - git_merge_driver *self, - void **payload, - const char *name, - const git_merge_driver_source *src); - -/** - * Callback to actually perform the merge. + * Callback to perform the merge. * * Specified as `driver.apply`, this is the callback that actually does the * merge. If it can successfully perform a merge, it should populate @@ -129,32 +95,20 @@ typedef int (*git_merge_driver_check_fn)( * and the file will remain conflicted. Any other errors will fail and * return to the caller. * - * The `src` contains the data about the file to be merged. + * The `filter_name` contains the name of the filter that was invoked, as + * specified by the file's attributes. * - * The `payload` value will refer to any payload that was set by the - * `check` callback. It may be read from or written to as needed. + * The `src` contains the data about the file to be merged. */ typedef int (*git_merge_driver_apply_fn)( git_merge_driver *self, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src); /** - * Callback to clean up after merge has been performed. - * - * Specified as `driver.cleanup`, this is an optional callback invoked - * after the driver has been run. If the `check` or `apply` callbacks - * allocated a `payload` to keep per-source merge driver state, use this - * callback to free that payload and release resources as required. - */ -typedef void (*git_merge_driver_cleanup_fn)( - git_merge_driver *self, - void *payload); - -/** * Merge driver structure used to register custom merge drivers. * * To associate extra data with a driver, allocate extra data and put the @@ -172,22 +126,12 @@ struct git_merge_driver { git_merge_driver_shutdown_fn shutdown; /** - * Called to determine whether the merge driver should be invoked - * for a given file. If this function returns `GIT_PASSTHROUGH` - * then the `apply` function will not be invoked and the default - * (`text`) merge driver will instead be run. - */ - git_merge_driver_check_fn check; - - /** * Called to merge the contents of a conflict. If this function * returns `GIT_PASSTHROUGH` then the default (`text`) merge driver * will instead be invoked. If this function returns * `GIT_EMERGECONFLICT` then the file will remain conflicted. + */ git_merge_driver_apply_fn apply; - - /** Called when the system is done filtering for a file. */ - git_merge_driver_cleanup_fn cleanup; }; #define GIT_MERGE_DRIVER_VERSION 1 diff --git a/src/merge.c b/src/merge.c index 742330583..27c0cea95 100644 --- a/src/merge.c +++ b/src/merge.c @@ -838,10 +838,10 @@ static bool merge_conflict_can_resolve_contents( static int merge_conflict_invoke_driver( git_index_entry **out, + const char *name, git_merge_driver *driver, - void *data, git_merge_diff_list *diff_list, - git_merge_driver_source *source) + git_merge_driver_source *src) { git_index_entry *result; git_buf buf = GIT_BUF_INIT; @@ -853,10 +853,8 @@ static int merge_conflict_invoke_driver( *out = NULL; - if ((error = driver->apply(driver, &data, &path, &mode, &buf, source)) < 0) - goto done; - - if ((error = git_repository_odb(&odb, source->repo)) < 0 || + if ((error = driver->apply(driver, &path, &mode, &buf, name, src)) < 0 || + (error = git_repository_odb(&odb, src->repo)) < 0 || (error = git_odb_write(&oid, odb, buf.ptr, buf.size, GIT_OBJ_BLOB)) < 0) goto done; @@ -873,9 +871,6 @@ static int merge_conflict_invoke_driver( *out = result; done: - if (driver->cleanup) - driver->cleanup(driver, data); - git_buf_free(&buf); git_odb_free(odb); @@ -892,9 +887,10 @@ static int merge_conflict_resolve_contents( git_merge_driver_source source = {0}; git_merge_file_result result = {0}; git_merge_driver *driver; + git_merge_driver__builtin builtin = {{0}}; git_index_entry *merge_result; git_odb *odb = NULL; - void *data; + const char *name; int error = 0; assert(resolved && diff_list && conflict); @@ -916,23 +912,26 @@ static int merge_conflict_resolve_contents( if (file_opts->favor != GIT_MERGE_FILE_FAVOR_NORMAL) { /* if the user requested a particular type of resolution (via the - * favor flag) then let that override the gitattributes. + * favor flag) then let that override the gitattributes and use + * the builtin driver. */ - driver = &git_merge_driver__normal; - data = (void **)&file_opts->favor; + name = "text"; + builtin.base.apply = git_merge_driver__builtin_apply; + builtin.favor = file_opts->favor; + + driver = &builtin.base; } else { /* find the merge driver for this file */ - if ((error = git_merge_driver_for_source(&driver, &data, &source)) < 0) + if ((error = git_merge_driver_for_source(&name, &driver, &source)) < 0) goto done; } - error = merge_conflict_invoke_driver(&merge_result, - driver, data, diff_list, &source); + error = merge_conflict_invoke_driver(&merge_result, name, driver, + diff_list, &source); if (error == GIT_PASSTHROUGH) { - data = NULL; - error = merge_conflict_invoke_driver(&merge_result, - &git_merge_driver__text, data, diff_list, &source); + error = merge_conflict_invoke_driver(&merge_result, "text", + &git_merge_driver__text.base, diff_list, &source); } if (error < 0) { diff --git a/src/merge.h b/src/merge.h index 68290e7cf..f8cac161f 100644 --- a/src/merge.h +++ b/src/merge.h @@ -145,12 +145,6 @@ int git_merge_diff_list__find_renames(git_repository *repo, git_merge_diff_list void git_merge_diff_list__free(git_merge_diff_list *diff_list); -/* Merge driver configuration */ -int git_merge_driver_for_source( - git_merge_driver **driver_out, - void **data_out, - const git_merge_driver_source *src); - /* Merge metadata setup */ int git_merge__setup( diff --git a/src/merge_driver.c b/src/merge_driver.c index e5bb169ed..cc039dbb5 100644 --- a/src/merge_driver.c +++ b/src/merge_driver.c @@ -30,32 +30,29 @@ typedef struct { static struct merge_driver_registry merge_driver_registry; -static git_merge_file_favor_t merge_favor_normal = GIT_MERGE_FILE_FAVOR_NORMAL; -static git_merge_file_favor_t merge_favor_union = GIT_MERGE_FILE_FAVOR_UNION; - static void git_merge_driver_global_shutdown(void); -static int merge_driver_apply( +int git_merge_driver__builtin_apply( git_merge_driver *self, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src) { + git_merge_driver__builtin *driver = (git_merge_driver__builtin *)self; git_merge_file_options file_opts = GIT_MERGE_FILE_OPTIONS_INIT; - git_merge_file_favor_t *favor = (git_merge_file_favor_t *) *payload; git_merge_file_result result = {0}; int error; - GIT_UNUSED(self); + GIT_UNUSED(filter_name); if (src->file_opts) memcpy(&file_opts, src->file_opts, sizeof(git_merge_file_options)); - if (favor) - file_opts.favor = *favor; + if (driver->favor) + file_opts.favor = driver->favor; if ((error = git_merge_file_from_index(&result, src->repo, src->ancestor, src->ours, src->theirs, &file_opts)) < 0) @@ -87,47 +84,19 @@ done: return error; } -static int merge_driver_text_check( - git_merge_driver *self, - void **payload, - const char *name, - const git_merge_driver_source *src) -{ - GIT_UNUSED(self); - GIT_UNUSED(name); - GIT_UNUSED(src); - - *payload = &merge_favor_normal; - return 0; -} - -static int merge_driver_union_check( - git_merge_driver *self, - void **payload, - const char *name, - const git_merge_driver_source *src) -{ - GIT_UNUSED(self); - GIT_UNUSED(name); - GIT_UNUSED(src); - - *payload = &merge_favor_union; - return 0; -} - static int merge_driver_binary_apply( git_merge_driver *self, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src) { GIT_UNUSED(self); - GIT_UNUSED(payload); GIT_UNUSED(path_out); GIT_UNUSED(mode_out); GIT_UNUSED(merged_out); + GIT_UNUSED(filter_name); GIT_UNUSED(src); return GIT_EMERGECONFLICT; @@ -149,35 +118,30 @@ static int merge_driver_entry_search(const void *a, const void *b) return strcmp(name_a, entry_b->name); } -git_merge_driver git_merge_driver__normal = { - GIT_MERGE_DRIVER_VERSION, - NULL, - NULL, - NULL, - merge_driver_apply -}; - -git_merge_driver git_merge_driver__text = { - GIT_MERGE_DRIVER_VERSION, - NULL, - NULL, - merge_driver_text_check, - merge_driver_apply +git_merge_driver__builtin git_merge_driver__text = { + { + GIT_MERGE_DRIVER_VERSION, + NULL, + NULL, + git_merge_driver__builtin_apply, + }, + GIT_MERGE_FILE_FAVOR_NORMAL }; -git_merge_driver git_merge_driver__union = { - GIT_MERGE_DRIVER_VERSION, - NULL, - NULL, - merge_driver_union_check, - merge_driver_apply +git_merge_driver__builtin git_merge_driver__union = { + { + GIT_MERGE_DRIVER_VERSION, + NULL, + NULL, + git_merge_driver__builtin_apply, + }, + GIT_MERGE_FILE_FAVOR_UNION }; git_merge_driver git_merge_driver__binary = { GIT_MERGE_DRIVER_VERSION, NULL, NULL, - NULL, merge_driver_binary_apply }; @@ -209,9 +173,9 @@ int git_merge_driver_global_init(void) goto done; if ((error = merge_driver_registry_insert( - merge_driver_name__text, &git_merge_driver__text)) < 0 || + merge_driver_name__text, &git_merge_driver__text.base)) < 0 || (error = merge_driver_registry_insert( - merge_driver_name__union, &git_merge_driver__union)) < 0 || + merge_driver_name__union, &git_merge_driver__union.base)) < 0 || (error = merge_driver_registry_insert( merge_driver_name__binary, &git_merge_driver__binary)) < 0) @@ -333,7 +297,7 @@ git_merge_driver *git_merge_driver_lookup(const char *name) * to take a lock and look it up in the vector. */ if (name == merge_driver_name__text) - return &git_merge_driver__text; + return &git_merge_driver__text.base; else if (name == merge_driver_name__binary) return &git_merge_driver__binary; @@ -409,13 +373,11 @@ GIT_INLINE(git_merge_driver *) merge_driver_lookup_with_wildcard( } int git_merge_driver_for_source( + const char **name_out, git_merge_driver **driver_out, - void **data_out, const git_merge_driver_source *src) { const char *path, *driver_name; - git_merge_driver *driver; - void *data = NULL; int error = 0; path = git_merge_file__best_path( @@ -427,31 +389,8 @@ int git_merge_driver_for_source( &driver_name, src->repo, path, src->default_driver)) < 0) return error; - driver = merge_driver_lookup_with_wildcard(driver_name); - - if (driver && driver->check) { - error = driver->check(driver, &data, driver_name, src); - - if (error == GIT_PASSTHROUGH) - driver = &git_merge_driver__text; - else if (error == GIT_EMERGECONFLICT) - driver = &git_merge_driver__binary; - else - goto done; - } - - error = 0; - data = NULL; - - if (driver->check) - error = driver->check(driver, &data, driver_name, src); - - /* the text and binary drivers must succeed their check */ - assert(error == 0); - -done: - *driver_out = driver; - *data_out = data; + *name_out = driver_name; + *driver_out = merge_driver_lookup_with_wildcard(driver_name); return error; } diff --git a/src/merge_driver.h b/src/merge_driver.h index c0f75faf2..bde27502c 100644 --- a/src/merge_driver.h +++ b/src/merge_driver.h @@ -21,6 +21,11 @@ struct git_merge_driver_source { const git_index_entry *theirs; }; +typedef struct git_merge_driver__builtin { + git_merge_driver base; + git_merge_file_favor_t favor; +} git_merge_driver__builtin; + extern int git_merge_driver_global_init(void); extern int git_merge_driver_for_path( @@ -29,14 +34,25 @@ extern int git_merge_driver_for_path( git_repository *repo, const char *path); -/* Basic (normal) merge driver, takes favor type as the payload argument */ -extern git_merge_driver git_merge_driver__normal; +/* Merge driver configuration */ +extern int git_merge_driver_for_source( + const char **name_out, + git_merge_driver **driver_out, + const git_merge_driver_source *src); + +extern int git_merge_driver__builtin_apply( + git_merge_driver *self, + const char **path_out, + uint32_t *mode_out, + git_buf *merged_out, + const char *filter_name, + const git_merge_driver_source *src); /* Merge driver for text files, performs a standard three-way merge */ -extern git_merge_driver git_merge_driver__text; +extern git_merge_driver__builtin git_merge_driver__text; /* Merge driver for union-style merging */ -extern git_merge_driver git_merge_driver__union; +extern git_merge_driver__builtin git_merge_driver__union; /* Merge driver for unmergeable (binary) files: always produces conflicts */ extern git_merge_driver git_merge_driver__binary; diff --git a/tests/merge/driver.c b/tests/merge/driver.c index 26041eca7..670c2c482 100644 --- a/tests/merge/driver.c +++ b/tests/merge/driver.c @@ -63,27 +63,12 @@ static void test_driver_shutdown(git_merge_driver *s) self->shutdown = 1; } -static int test_driver_check( - git_merge_driver *s, - void **payload, - const char *name, - const git_merge_driver_source *src) -{ - GIT_UNUSED(s); - GIT_UNUSED(src); - - *payload = git__strdup(name); - GITERR_CHECK_ALLOC(*payload); - - return 0; -} - static int test_driver_apply( git_merge_driver *s, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src) { GIT_UNUSED(s); @@ -93,25 +78,15 @@ static int test_driver_apply( *mode_out = GIT_FILEMODE_BLOB; return git_buf_printf(merged_out, "This is the `%s` driver.\n", - (char *)*payload); + filter_name); } -static void test_driver_cleanup(git_merge_driver *s, void *payload) -{ - GIT_UNUSED(s); - - git__free(payload); -} - - static struct test_merge_driver test_driver_custom = { { GIT_MERGE_DRIVER_VERSION, test_driver_init, test_driver_shutdown, - test_driver_check, test_driver_apply, - test_driver_cleanup }, 0, 0, @@ -122,9 +97,7 @@ static struct test_merge_driver test_driver_wildcard = { GIT_MERGE_DRIVER_VERSION, test_driver_init, test_driver_shutdown, - test_driver_check, test_driver_apply, - test_driver_cleanup }, 0, 0, @@ -219,62 +192,19 @@ void test_merge_driver__shutdown_is_called(void) test_drivers_register(); } -static int defer_driver_check( - git_merge_driver *s, - void **payload, - const char *name, - const git_merge_driver_source *src) -{ - GIT_UNUSED(s); - GIT_UNUSED(payload); - GIT_UNUSED(name); - GIT_UNUSED(src); - - return GIT_PASSTHROUGH; -} - -static struct test_merge_driver test_driver_defer_check = { - { - GIT_MERGE_DRIVER_VERSION, - test_driver_init, - test_driver_shutdown, - defer_driver_check, - test_driver_apply, - test_driver_cleanup - }, - 0, - 0, -}; - -void test_merge_driver__check_can_defer(void) -{ - const git_index_entry *idx; - - cl_git_pass(git_merge_driver_register("defer", - &test_driver_defer_check.base)); - - set_gitattributes_to("defer"); - merge_branch(); - - cl_assert((idx = git_index_get_bypath(repo_index, "automergeable.txt", 0))); - cl_assert_equal_oid(&automergeable_id, &idx->id); - - git_merge_driver_unregister("defer"); -} - static int defer_driver_apply( git_merge_driver *s, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src) { GIT_UNUSED(s); - GIT_UNUSED(payload); GIT_UNUSED(path_out); GIT_UNUSED(mode_out); GIT_UNUSED(merged_out); + GIT_UNUSED(filter_name); GIT_UNUSED(src); return GIT_PASSTHROUGH; @@ -285,9 +215,7 @@ static struct test_merge_driver test_driver_defer_apply = { GIT_MERGE_DRIVER_VERSION, test_driver_init, test_driver_shutdown, - test_driver_check, defer_driver_apply, - test_driver_cleanup }, 0, 0, @@ -309,62 +237,19 @@ void test_merge_driver__apply_can_defer(void) git_merge_driver_unregister("defer"); } -static int conflict_driver_check( - git_merge_driver *s, - void **payload, - const char *name, - const git_merge_driver_source *src) -{ - GIT_UNUSED(s); - GIT_UNUSED(payload); - GIT_UNUSED(name); - GIT_UNUSED(src); - - return GIT_EMERGECONFLICT; -} - -static struct test_merge_driver test_driver_conflict_check = { - { - GIT_MERGE_DRIVER_VERSION, - test_driver_init, - test_driver_shutdown, - conflict_driver_check, - test_driver_apply, - test_driver_cleanup - }, - 0, - 0, -}; - -void test_merge_driver__check_can_conflict(void) -{ - const git_index_entry *ancestor, *ours, *theirs; - - cl_git_pass(git_merge_driver_register("conflict", - &test_driver_conflict_check.base)); - - set_gitattributes_to("conflict"); - merge_branch(); - - cl_git_pass(git_index_conflict_get(&ancestor, &ours, &theirs, - repo_index, "automergeable.txt")); - - git_merge_driver_unregister("conflict"); -} - static int conflict_driver_apply( git_merge_driver *s, - void **payload, const char **path_out, uint32_t *mode_out, git_buf *merged_out, + const char *filter_name, const git_merge_driver_source *src) { GIT_UNUSED(s); - GIT_UNUSED(payload); GIT_UNUSED(path_out); GIT_UNUSED(mode_out); GIT_UNUSED(merged_out); + GIT_UNUSED(filter_name); GIT_UNUSED(src); return GIT_EMERGECONFLICT; @@ -375,9 +260,7 @@ static struct test_merge_driver test_driver_conflict_apply = { GIT_MERGE_DRIVER_VERSION, test_driver_init, test_driver_shutdown, - test_driver_check, conflict_driver_apply, - test_driver_cleanup }, 0, 0, @@ -447,7 +330,7 @@ void test_merge_driver__mergedefault_deferring_falls_back_to_text(void) const git_index_entry *idx; cl_git_pass(git_merge_driver_register("defer", - &test_driver_defer_check.base)); + &test_driver_defer_apply.base)); cl_repo_set_string(repo, "merge.default", "defer"); merge_branch(); |