From 943700ecbbc2be4ef49c5c31d8e5c49353fd3d84 Mon Sep 17 00:00:00 2001 From: yorah Date: Fri, 18 Jan 2013 16:37:13 +0100 Subject: [PATCH 1/2] Return the matched pathspec pattern in `git_pathspec_match_path` Instead of returning directly the pattern as the return value, I used an out parameter, because the function also tests if the passed pathspecs vector is empty. If yes, it considers that the path "matches", but in that case there is no matched pattern per se. --- src/checkout.c | 2 +- src/diff.c | 6 ++++-- src/index.c | 3 ++- src/pathspec.c | 17 ++++++++++++++--- src/pathspec.h | 12 ++++++++++-- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 40f5732ed..0ce283beb 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -224,7 +224,7 @@ static int checkout_action_wd_only( if (!git_pathspec_match_path( pathspec, wd->path, (data->strategy & GIT_CHECKOUT_DISABLE_PATHSPEC_MATCH) != 0, - git_iterator_ignore_case(workdir))) + git_iterator_ignore_case(workdir), NULL)) return 0; /* check if item is tracked in the index but not in the checkout diff */ diff --git a/src/diff.c b/src/diff.c index 4b60935f0..3e8028551 100644 --- a/src/diff.c +++ b/src/diff.c @@ -47,6 +47,7 @@ static int diff_delta__from_one( const git_index_entry *entry) { git_diff_delta *delta; + const char *matched_pathspec; if (status == GIT_DELTA_IGNORED && (diff->opts.flags & GIT_DIFF_INCLUDE_IGNORED) == 0) @@ -59,7 +60,7 @@ static int diff_delta__from_one( if (!git_pathspec_match_path( &diff->pathspec, entry->path, (diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0, - (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0)) + (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0, &matched_pathspec)) return 0; delta = diff_delta__alloc(diff, status, entry->path); @@ -419,13 +420,14 @@ static int maybe_modified( unsigned int omode = oitem->mode; unsigned int nmode = nitem->mode; bool new_is_workdir = (new_iter->type == GIT_ITERATOR_TYPE_WORKDIR); + const char *matched_pathspec; GIT_UNUSED(old_iter); if (!git_pathspec_match_path( &diff->pathspec, oitem->path, (diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0, - (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0)) + (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0, &matched_pathspec)) return 0; /* on platforms with no symlinks, preserve mode of existing symlinks */ diff --git a/src/index.c b/src/index.c index 1e00dd3fa..f5bf954a8 100644 --- a/src/index.c +++ b/src/index.c @@ -1710,7 +1710,8 @@ int git_index_read_tree_match( goto cleanup; while (entry != NULL) { - if (git_pathspec_match_path(&pathspec, entry->path, false, false) && + if (git_pathspec_match_path( + &pathspec, entry->path, false, false, NULL) && (error = git_index_add(index, entry)) < 0) goto cleanup; diff --git a/src/pathspec.c b/src/pathspec.c index 2bde3ba5f..732180248 100644 --- a/src/pathspec.c +++ b/src/pathspec.c @@ -106,14 +106,21 @@ void git_pathspec_free(git_vector *vspec) /* match a path against the vectorized pathspec */ bool git_pathspec_match_path( - git_vector *vspec, const char *path, bool disable_fnmatch, bool casefold) + git_vector *vspec, + const char *path, + bool disable_fnmatch, + bool casefold, + const char **matched_pathspec) { - unsigned int i; + size_t i; git_attr_fnmatch *match; int fnmatch_flags = 0; int (*use_strcmp)(const char *, const char *); int (*use_strncmp)(const char *, const char *, size_t); + if (matched_pathspec) + *matched_pathspec = NULL; + if (!vspec || !vspec->length) return true; @@ -143,8 +150,12 @@ bool git_pathspec_match_path( path[match->length] == '/') result = 0; - if (result == 0) + if (result == 0) { + if (matched_pathspec) + *matched_pathspec = match->pattern; + return (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) ? false : true; + } } return false; diff --git a/src/pathspec.h b/src/pathspec.h index dde63c7d0..c44561520 100644 --- a/src/pathspec.h +++ b/src/pathspec.h @@ -25,8 +25,16 @@ extern int git_pathspec_init( /* free data from the pathspec vector */ extern void git_pathspec_free(git_vector *vspec); -/* match a path against the vectorized pathspec */ +/* + * Match a path against the vectorized pathspec. + * The matched pathspec is passed back into the `matched_pathspec` parameter, + * unless it is passed as NULL by the caller. + */ extern bool git_pathspec_match_path( - git_vector *vspec, const char *path, bool disable_fnmatch, bool casefold); + git_vector *vspec, + const char *path, + bool disable_fnmatch, + bool casefold, + const char **matched_pathspec); #endif From 0d64ba48372f95ade17a27f1b11620638ca52d61 Mon Sep 17 00:00:00 2001 From: yorah Date: Fri, 25 Jan 2013 17:35:46 +0100 Subject: [PATCH 2/2] diff: add a notify callback to `git_diff__from_iterators` The callback will be called for each file, just before the `git_delta_t` gets inserted into the diff list. When the callback: - returns < 0, the diff process will be aborted - returns > 0, the delta will not be inserted into the diff list, but the diff process continues - returns 0, the delta is inserted into the diff list, and the diff process continues --- include/git2/diff.h | 92 ++++++++++++------- src/diff.c | 44 +++++++-- tests-clar/diff/diff_helpers.h | 5 + tests-clar/diff/workdir.c | 163 +++++++++++++++++++++++++++++++++ 4 files changed, 260 insertions(+), 44 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index 81c41df04..3a88902ad 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -122,40 +122,6 @@ typedef enum { GIT_DIFF_IGNORE_FILEMODE = (1 << 17), } git_diff_option_t; -/** - * Structure describing options about how the diff should be executed. - * - * Setting all values of the structure to zero will yield the default - * values. Similarly, passing NULL for the options structure will - * give the defaults. The default values are marked below. - * - * - `flags` is a combination of the `git_diff_option_t` values above - * - `context_lines` is the number of unchanged lines that define the - * boundary of a hunk (and to display before and after) - * - `interhunk_lines` is the maximum number of unchanged lines between - * hunk boundaries before the hunks will be merged into a one. - * - `old_prefix` is the virtual "directory" to prefix to old file names - * in hunk headers (default "a") - * - `new_prefix` is the virtual "directory" to prefix to new file names - * in hunk headers (default "b") - * - `pathspec` is an array of paths / fnmatch patterns to constrain diff - * - `max_size` is a file size (in bytes) above which a blob will be marked - * as binary automatically; pass a negative value to disable. - */ -typedef struct { - unsigned int version; /**< version for the struct */ - uint32_t flags; /**< defaults to GIT_DIFF_NORMAL */ - uint16_t context_lines; /**< defaults to 3 */ - uint16_t interhunk_lines; /**< defaults to 0 */ - const char *old_prefix; /**< defaults to "a" */ - const char *new_prefix; /**< defaults to "b" */ - git_strarray pathspec; /**< defaults to include all paths */ - git_off_t max_size; /**< defaults to 512MB */ -} git_diff_options; - -#define GIT_DIFF_OPTIONS_VERSION 1 -#define GIT_DIFF_OPTIONS_INIT {GIT_DIFF_OPTIONS_VERSION} - /** * The diff list object that contains all individual file deltas. * @@ -265,6 +231,64 @@ typedef struct { int binary; } git_diff_delta; +/** + * Diff notification callback function. + * + * The callback will be called for each file, just before the `git_delta_t` + * gets inserted into the diff list. + * + * When the callback: + * - returns < 0, the diff process will be aborted. + * - returns > 0, the delta will not be inserted into the diff list, but the + * diff process continues. + * - returns 0, the delta is inserted into the diff list, and the diff process + * continues. + */ +typedef int (*git_diff_notify_cb)( + const git_diff_list *diff_so_far, + const git_diff_delta *delta_to_add, + const char *matched_pathspec, + void *payload); + +/** + * Structure describing options about how the diff should be executed. + * + * Setting all values of the structure to zero will yield the default + * values. Similarly, passing NULL for the options structure will + * give the defaults. The default values are marked below. + * + * - `flags` is a combination of the `git_diff_option_t` values above + * - `context_lines` is the number of unchanged lines that define the + * boundary of a hunk (and to display before and after) + * - `interhunk_lines` is the maximum number of unchanged lines between + * hunk boundaries before the hunks will be merged into a one. + * - `old_prefix` is the virtual "directory" to prefix to old file names + * in hunk headers (default "a") + * - `new_prefix` is the virtual "directory" to prefix to new file names + * in hunk headers (default "b") + * - `pathspec` is an array of paths / fnmatch patterns to constrain diff + * - `max_size` is a file size (in bytes) above which a blob will be marked + * as binary automatically; pass a negative value to disable. + * - `notify_cb` is an optional callback function, notifying the consumer of + * which files are being examined as the diff is generated + * - `notify_payload` is the payload data to pass to the `notify_cb` function + */ +typedef struct { + unsigned int version; /**< version for the struct */ + uint32_t flags; /**< defaults to GIT_DIFF_NORMAL */ + uint16_t context_lines; /**< defaults to 3 */ + uint16_t interhunk_lines; /**< defaults to 0 */ + const char *old_prefix; /**< defaults to "a" */ + const char *new_prefix; /**< defaults to "b" */ + git_strarray pathspec; /**< defaults to include all paths */ + git_off_t max_size; /**< defaults to 512MB */ + git_diff_notify_cb notify_cb; + void *notify_payload; +} git_diff_options; + +#define GIT_DIFF_OPTIONS_VERSION 1 +#define GIT_DIFF_OPTIONS_INIT {GIT_DIFF_OPTIONS_VERSION} + /** * When iterating over a diff, callback that will be made per file. * diff --git a/src/diff.c b/src/diff.c index 3e8028551..d9bc32a37 100644 --- a/src/diff.c +++ b/src/diff.c @@ -41,6 +41,18 @@ static git_diff_delta *diff_delta__alloc( return delta; } +static int diff_notify( + const git_diff_list *diff, + const git_diff_delta *delta, + const char *matched_pathspec) +{ + if (!diff->opts.notify_cb) + return 0; + + return diff->opts.notify_cb( + diff, delta, matched_pathspec, diff->opts.notify_payload); +} + static int diff_delta__from_one( git_diff_list *diff, git_delta_t status, @@ -48,6 +60,7 @@ static int diff_delta__from_one( { git_diff_delta *delta; const char *matched_pathspec; + int notify_res; if (status == GIT_DELTA_IGNORED && (diff->opts.flags & GIT_DIFF_INCLUDE_IGNORED) == 0) @@ -85,12 +98,16 @@ static int diff_delta__from_one( !git_oid_iszero(&delta->new_file.oid)) delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID; - if (git_vector_insert(&diff->deltas, delta) < 0) { + notify_res = diff_notify(diff, delta, matched_pathspec); + + if (notify_res) + git__free(delta); + else if (git_vector_insert(&diff->deltas, delta) < 0) { git__free(delta); return -1; } - return 0; + return notify_res < 0 ? GIT_EUSER : 0; } static int diff_delta__from_two( @@ -100,9 +117,11 @@ static int diff_delta__from_two( uint32_t old_mode, const git_index_entry *new_entry, uint32_t new_mode, - git_oid *new_oid) + git_oid *new_oid, + const char *matched_pathspec) { git_diff_delta *delta; + int notify_res; if (status == GIT_DELTA_UNMODIFIED && (diff->opts.flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0) @@ -139,12 +158,16 @@ static int diff_delta__from_two( if (new_oid || !git_oid_iszero(&new_entry->oid)) delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID; - if (git_vector_insert(&diff->deltas, delta) < 0) { + notify_res = diff_notify(diff, delta, matched_pathspec); + + if (notify_res) + git__free(delta); + else if (git_vector_insert(&diff->deltas, delta) < 0) { git__free(delta); return -1; } - return 0; + return notify_res < 0 ? GIT_EUSER : 0; } static git_diff_delta *diff_delta__last_for_item( @@ -528,7 +551,7 @@ static int maybe_modified( } return diff_delta__from_two( - diff, status, oitem, omode, nitem, nmode, use_noid); + diff, status, oitem, omode, nitem, nmode, use_noid, matched_pathspec); } static bool entry_is_prefixed( @@ -749,10 +772,11 @@ int git_diff__from_iterators( else { assert(oitem && nitem && cmp == 0); - if (maybe_modified(old_iter, oitem, new_iter, nitem, diff) < 0 || - git_iterator_advance(old_iter, &oitem) < 0 || - git_iterator_advance(new_iter, &nitem) < 0) - goto fail; + if (maybe_modified( + old_iter, oitem, new_iter, nitem, diff) < 0 || + git_iterator_advance(old_iter, &oitem) < 0 || + git_iterator_advance(new_iter, &nitem) < 0) + goto fail; } } diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index 49c265285..a43847b79 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -20,6 +20,11 @@ typedef struct { int line_dels; } diff_expects; +typedef struct { + const char *path; + const char *matched_pathspec; +} notify_expected; + extern int diff_file_cb( const git_diff_delta *delta, float progress, diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 21da63954..7e8915c4b 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -307,6 +307,169 @@ void test_diff_workdir__to_index_with_pathspec(void) git_diff_list_free(diff); } +static int assert_called_notifications( + const git_diff_list *diff_so_far, + const git_diff_delta *delta_to_add, + const char *matched_pathspec, + void *payload) +{ + bool found = false; + notify_expected *exp = (notify_expected*)payload; + notify_expected *e;; + + GIT_UNUSED(diff_so_far); + + for (e = exp; e->path != NULL; e++) { + if (strcmp(e->path, delta_to_add->new_file.path)) + continue; + + cl_assert_equal_s(e->matched_pathspec, matched_pathspec); + + found = true; + break; + } + + cl_assert(found); + return 0; +} + +void test_diff_workdir__to_index_notify(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + diff_expects exp; + + char *searched_pathspecs_solo[] = { + "*_deleted", + }; + notify_expected expected_matched_pathspecs_solo[] = { + { "file_deleted", "*_deleted" }, + { "staged_changes_file_deleted", "*_deleted" }, + { NULL, NULL } + }; + + char *searched_pathspecs_multiple[] = { + "staged_changes_cant_find_me", + "subdir/modified_cant_find_me", + "subdir/*", + "staged*" + }; + notify_expected expected_matched_pathspecs_multiple[] = { + { "staged_changes_file_deleted", "staged*" }, + { "staged_changes_modified_file", "staged*" }, + { "staged_delete_modified_file", "staged*" }, + { "staged_new_file_deleted_file", "staged*" }, + { "staged_new_file_modified_file", "staged*" }, + { "subdir/deleted_file", "subdir/*" }, + { "subdir/modified_file", "subdir/*" }, + { "subdir/new_file", "subdir/*" }, + { NULL, NULL } + }; + + g_repo = cl_git_sandbox_init("status"); + + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + opts.notify_cb = assert_called_notifications; + opts.pathspec.strings = searched_pathspecs_solo; + opts.pathspec.count = 1; + + opts.notify_payload = &expected_matched_pathspecs_solo; + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + cl_git_pass(git_diff_foreach(diff, diff_file_cb, NULL, NULL, &exp)); + + cl_assert_equal_i(2, exp.files); + + git_diff_list_free(diff); + + opts.pathspec.strings = searched_pathspecs_multiple; + opts.pathspec.count = 4; + opts.notify_payload = &expected_matched_pathspecs_multiple; + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + cl_git_pass(git_diff_foreach(diff, diff_file_cb, NULL, NULL, &exp)); + + cl_assert_equal_i(8, exp.files); + + git_diff_list_free(diff); +} + +static int abort_diff( + const git_diff_list *diff_so_far, + const git_diff_delta *delta_to_add, + const char *matched_pathspec, + void *payload) +{ + GIT_UNUSED(diff_so_far); + GIT_UNUSED(delta_to_add); + GIT_UNUSED(matched_pathspec); + GIT_UNUSED(payload); + + return -42; +} + +void test_diff_workdir__to_index_notify_can_be_aborted_by_callback(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + char *pathspec = NULL; + + g_repo = cl_git_sandbox_init("status"); + + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + opts.notify_cb = abort_diff; + opts.pathspec.strings = &pathspec; + opts.pathspec.count = 1; + + pathspec = "file_deleted"; + cl_git_fail(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + + pathspec = "staged_changes_modified_file"; + cl_git_fail(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); +} + +static int filter_all( + const git_diff_list *diff_so_far, + const git_diff_delta *delta_to_add, + const char *matched_pathspec, + void *payload) +{ + GIT_UNUSED(diff_so_far); + GIT_UNUSED(delta_to_add); + GIT_UNUSED(matched_pathspec); + GIT_UNUSED(payload); + + return 42; +} + +void test_diff_workdir__to_index_notify_can_be_used_as_filtering_function(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + char *pathspec = NULL; + diff_expects exp; + + g_repo = cl_git_sandbox_init("status"); + + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + opts.notify_cb = filter_all; + opts.pathspec.strings = &pathspec; + opts.pathspec.count = 1; + + pathspec = "*_deleted"; + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + cl_git_pass(git_diff_foreach(diff, diff_file_cb, NULL, NULL, &exp)); + + cl_assert_equal_i(0, exp.files); + + git_diff_list_free(diff); +} + + void test_diff_workdir__filemode_changes(void) { git_config *cfg;