From 9a12a6256efa7da4b4245d0f2b7df6f3b84edabd Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 18 Sep 2012 15:13:07 -0700 Subject: [PATCH 1/4] New take on iterating over diff content Allow diff deltas to be accessed by index and make patch generation explicit with hunk and line access by index as well. --- include/git2/diff.h | 243 ++++++++++++++++++-------------------------- 1 file changed, 100 insertions(+), 143 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index 05825c50d..2a5cdacc0 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -98,9 +98,6 @@ enum { GIT_DIFF_FILE_FREE_PATH = (1 << 1), GIT_DIFF_FILE_BINARY = (1 << 2), GIT_DIFF_FILE_NOT_BINARY = (1 << 3), - GIT_DIFF_FILE_FREE_DATA = (1 << 4), - GIT_DIFF_FILE_UNMAP_DATA = (1 << 5), - GIT_DIFF_FILE_NO_DATA = (1 << 6), }; /** @@ -220,9 +217,13 @@ typedef int (*git_diff_data_fn)( size_t content_len); /** - * The diff iterator object is used to scan a diff list. + * The diff patch is used to store all the text diffs for a delta. + * + * You can easily loop over the content of patches and get information about + * them. */ -typedef struct git_diff_iterator git_diff_iterator; +typedef struct git_diff_patch git_diff_patch; + /** @name Diff List Generator Functions * @@ -349,7 +350,7 @@ GIT_EXTERN(int) git_diff_merge( /**@{*/ /** - * Iterate over a diff list issuing callbacks. + * Loop over all deltas in a diff list issuing callbacks. * * This will iterate through all of the files described in a diff. You * should provide a file callback to learn about each file. @@ -380,137 +381,6 @@ GIT_EXTERN(int) git_diff_foreach( git_diff_hunk_fn hunk_cb, git_diff_data_fn line_cb); -/** - * Create a diff iterator object that can be used to traverse a diff. - * - * This iterator can be used instead of `git_diff_foreach` in situations - * where callback functions are awkward to use. Because of the way that - * diffs are calculated internally, using an iterator will use somewhat - * more memory than `git_diff_foreach` would. - * - * @param iterator Output parameter of newly created iterator. - * @param diff Diff over which you wish to iterate. - * @return 0 on success, < 0 on error - */ -GIT_EXTERN(int) git_diff_iterator_new( - git_diff_iterator **iterator, - git_diff_list *diff); - -/** - * Release the iterator object. - * - * Call this when you are done using the iterator. - * - * @param iterator The diff iterator to be freed. - */ -GIT_EXTERN(void) git_diff_iterator_free(git_diff_iterator *iterator); - -/** - * Return progress value for traversing the diff. - * - * This returns a value between 0.0 and 1.0 that represents the progress - * through the diff iterator. The value is monotonically increasing and - * will advance gradually as you progress through the iteration. - * - * @param iterator The diff iterator - * @return Value between 0.0 and 1.0 - */ -GIT_EXTERN(float) git_diff_iterator_progress(git_diff_iterator *iterator); - -/** - * Return the number of hunks in the current file - * - * This will return the number of diff hunks in the current file. If the - * diff has not been performed yet, this may result in loading the file and - * performing the diff. - * - * @param iterator The iterator object - * @return The number of hunks in the current file or <0 on loading failure - */ -GIT_EXTERN(int) git_diff_iterator_num_hunks_in_file(git_diff_iterator *iterator); - -/** - * Return the number of lines in the hunk currently being examined. - * - * This will return the number of lines in the current hunk. If the diff - * has not been performed yet, this may result in loading the file and - * performing the diff. - * - * @param iterator The iterator object - * @return The number of lines in the current hunk (context, added, and - * removed all added together) or <0 on loading failure - */ -GIT_EXTERN(int) git_diff_iterator_num_lines_in_hunk(git_diff_iterator *iterator); - -/** - * Return the delta information for the next file in the diff. - * - * This will return a pointer to the next git_diff_delta` to be processed or - * NULL if the iterator is at the end of the diff, then advance. This - * returns the value `GIT_ITEROVER` after processing the last file. - * - * @param delta Output parameter for the next delta object - * @param iterator The iterator object - * @return 0 on success, GIT_ITEROVER when done, other value < 0 on error - */ -GIT_EXTERN(int) git_diff_iterator_next_file( - git_diff_delta **delta, - git_diff_iterator *iterator); - -/** - * Return the hunk information for the next hunk in the current file. - * - * It is recommended that you not call this if the file is a binary - * file, but it is allowed to do so. - * - * The `header` text output will contain the standard hunk header that - * would appear in diff output. The header string will be NUL terminated. - * - * WARNING! Call this function for the first time on a file is when the - * actual text diff will be computed (it cannot be computed incrementally) - * so the first call for a new file is expensive (at least in relative - * terms - in reality, it is still pretty darn fast). - * - * @param range Output pointer to range of lines covered by the hunk; - * This range object is owned by the library and should not be freed. - * @param header Output pointer to the text of the hunk header - * This string is owned by the library and should not be freed. - * @param header_len Output pointer to store the length of the header text - * @param iterator The iterator object - * @return 0 on success, GIT_ITEROVER when done with current file, other - * value < 0 on error - */ -GIT_EXTERN(int) git_diff_iterator_next_hunk( - git_diff_range **range, - const char **header, - size_t *header_len, - git_diff_iterator *iterator); - -/** - * Return the next line of the current hunk of diffs. - * - * The `line_origin` output will tell you what type of line this is - * (e.g. was it added or removed or is it just context for the diff). - * - * The `content` will be a pointer to the file data that goes in the - * line. IT WILL NOT BE NUL TERMINATED. You have to use the `content_len` - * value and only process that many bytes of data from the content string. - * - * @param line_origin Output pointer to store a GIT_DIFF_LINE value for this - * next chunk of data. The value is a single character, not a buffer. - * @param content Output pointer to store the content of the diff; this - * string is owned by the library and should not be freed. - * @param content_len Output pointer to store the length of the content. - * @param iterator The iterator object - * @return 0 on success, GIT_ITEROVER when done with current line, other - * value < 0 on error - */ -GIT_EXTERN(int) git_diff_iterator_next_line( - char *line_origin, /**< GIT_DIFF_LINE_... value from above */ - const char **content, - size_t *content_len, - git_diff_iterator *iterator); - /** * Iterate over a diff generating text output like "git diff --name-status". * @@ -552,17 +422,104 @@ GIT_EXTERN(int) git_diff_print_patch( /** * Query how many diff records are there in a diff list. * - * You can optionally pass in a `git_delta_t` value if you want a count - * of just entries that match that delta type, or pass -1 for all delta - * records. + * @param diff A git_diff_list generated by one of the above functions + * @return Count of number of deltas in the list + */ +GIT_EXTERN(size_t) git_diff_entrycount(git_diff_list *diff); + +/** + * Query how many diff deltas are there in a diff list filtered by type. + * + * This works just like `git_diff_entrycount()` with an extra parameter + * that is a `git_delta_t` and returns just the count of how many deltas + * match that particular type. * * @param diff A git_diff_list generated by one of the above functions - * @param delta_t A git_delta_t value to filter the count, or -1 for all records + * @param type A git_delta_t value to filter the count * @return Count of number of deltas matching delta_t type */ -GIT_EXTERN(int) git_diff_entrycount( +GIT_EXTERN(size_t) git_diff_entrycount_of_type( git_diff_list *diff, - int delta_t); + git_delta_t type); + +/** + * Return the diff delta and patch for an entry in the diff list. + * + * The `git_diff_patch` is a newly created object contains the text diffs + * for the delta. You have to call `git_diff_patch_free()` when you are + * done with it. You can use the patch object to loop over all the hunks + * and lines in the diff of the one delta. + * + * For a binary file, no `git_diff_patch` will be created, the output will + * be set to NULL, and the `binary` flag will be set true in the + * `git_diff_delta` structure. + * + * The `git_diff_delta` pointer points to internal data and you do not have + * to release it when you are done with it. It will go away when the + * `git_diff_list` and `git_diff_patch` go away. + * + * It is okay to pass NULL for either of the output parameters; if you pass + * NULL for the `git_diff_patch`, then the text diff will not be calculated. + * + * @param patch Output parameter for the delta patch object + * @param delta Output parameter for the delta object + * @param diff Diff list object + * @param idx Index into diff list + * @return 0 on success, other value < 0 on error + */ +GIT_EXTERN(int) git_diff_get_patch( + git_diff_patch **patch, + const git_diff_delta **delta, + git_diff_list *diff, + size_t idx); + +/** + * Free a git_diff_patch object. + */ +GIT_EXTERN(void) git_diff_patch_free( + git_diff_patch *patch); + +/** + * Get the delta associated with a patch + */ +GIT_EXTERN(void) git_diff_patch_get_delta( + const git_diff_delta **delta, + git_diff_patch *patch); + +/** + * Get the number of hunks in a patch + */ +GIT_EXTERN(size_t) git_diff_patch_hunks( + git_diff_patch *patch); + +/** + * Get the information about a hunk in a patch + */ +GIT_EXTERN(int) git_diff_patch_get_hunk( + const git_diff_range **range, + const char **header, + size_t *header_len, + size_t *lines_in_hunk, + git_diff_patch *patch, + size_t hunk); + +/** + * Get the number of lines in a hunk + */ +GIT_EXTERN(size_t) git_diff_patch_lines_in_hunk( + git_diff_patch *patch, + size_t hunk); + +/** + * Get a line in a hunk of a patch + */ +GIT_EXTERN(int) git_diff_patch_get_line_in_hunk( + char *line_origin, + const char **content, + size_t *content_len, + git_diff_patch *patch, + size_t hunk, + size_t line_of_hunk); /**@}*/ From 5f69a31f7d706aa5788ad9937391577a66e3c77d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 24 Sep 2012 20:52:34 -0700 Subject: [PATCH 2/4] Initial implementation of new diff patch API Replacing the `git_iterator` object, this creates a simple API for accessing the "patch" for any file pair in a diff list and then gives indexed access to the hunks in the patch and the lines in the hunk. This is the initial implementation of this revised API - it is still broken, but at least builds cleanly. --- include/git2/diff.h | 62 +- src/diff.c | 46 +- src/diff.h | 32 +- src/diff_output.c | 1102 +++++++++++++++----------------- src/diff_output.h | 86 +++ src/submodule.c | 7 +- tests-clar/diff/diff_helpers.c | 95 +-- tests-clar/diff/diffiter.c | 275 ++++++-- tests-clar/diff/tree.c | 73 +-- tests-clar/diff/workdir.c | 69 +- 10 files changed, 994 insertions(+), 853 deletions(-) create mode 100644 src/diff_output.h diff --git a/include/git2/diff.h b/include/git2/diff.h index 2a5cdacc0..d216c1303 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -98,6 +98,9 @@ enum { GIT_DIFF_FILE_FREE_PATH = (1 << 1), GIT_DIFF_FILE_BINARY = (1 << 2), GIT_DIFF_FILE_NOT_BINARY = (1 << 3), + GIT_DIFF_FILE_FREE_DATA = (1 << 4), + GIT_DIFF_FILE_UNMAP_DATA = (1 << 5), + GIT_DIFF_FILE_NO_DATA = (1 << 6), }; /** @@ -425,7 +428,7 @@ GIT_EXTERN(int) git_diff_print_patch( * @param diff A git_diff_list generated by one of the above functions * @return Count of number of deltas in the list */ -GIT_EXTERN(size_t) git_diff_entrycount(git_diff_list *diff); +GIT_EXTERN(size_t) git_diff_num_deltas(git_diff_list *diff); /** * Query how many diff deltas are there in a diff list filtered by type. @@ -438,7 +441,7 @@ GIT_EXTERN(size_t) git_diff_entrycount(git_diff_list *diff); * @param type A git_delta_t value to filter the count * @return Count of number of deltas matching delta_t type */ -GIT_EXTERN(size_t) git_diff_entrycount_of_type( +GIT_EXTERN(size_t) git_diff_num_deltas_of_type( git_diff_list *diff, git_delta_t type); @@ -469,7 +472,7 @@ GIT_EXTERN(size_t) git_diff_entrycount_of_type( */ GIT_EXTERN(int) git_diff_get_patch( git_diff_patch **patch, - const git_diff_delta **delta, + git_diff_delta **delta, git_diff_list *diff, size_t idx); @@ -482,43 +485,76 @@ GIT_EXTERN(void) git_diff_patch_free( /** * Get the delta associated with a patch */ -GIT_EXTERN(void) git_diff_patch_get_delta( - const git_diff_delta **delta, +GIT_EXTERN(const git_diff_delta *) git_diff_patch_delta( git_diff_patch *patch); /** * Get the number of hunks in a patch */ -GIT_EXTERN(size_t) git_diff_patch_hunks( +GIT_EXTERN(size_t) git_diff_patch_num_hunks( git_diff_patch *patch); /** * Get the information about a hunk in a patch + * + * Given a patch and a hunk index into the patch, this returns detailed + * information about that hunk. Any of the output pointers can be passed + * as NULL if you don't care about that particular piece of information. + * + * @param range Output pointer to git_diff_range of hunk + * @param header Output pointer to header string for hunk. Unlike the + * content pointer for each line, this will be NUL-terminated + * @param header_len Output value of characters in header string + * @param lines_in_hunk Output count of total lines in this hunk + * @param patch Input pointer to patch object + * @param hunk_idx Input index of hunk to get information about + * @return 0 on success, GIT_ENOTFOUND if hunk_idx out of range, <0 on error */ GIT_EXTERN(int) git_diff_patch_get_hunk( - const git_diff_range **range, + git_diff_range **range, const char **header, size_t *header_len, size_t *lines_in_hunk, git_diff_patch *patch, - size_t hunk); + size_t hunk_idx); /** - * Get the number of lines in a hunk + * Get the number of lines in a hunk. + * + * @param patch The git_diff_patch object + * @param hunk_idx Index of the hunk + * @return Number of lines in hunk or -1 if invalid hunk index */ -GIT_EXTERN(size_t) git_diff_patch_lines_in_hunk( +GIT_EXTERN(int) git_diff_patch_num_lines_in_hunk( git_diff_patch *patch, - size_t hunk); + size_t hunk_idx); /** - * Get a line in a hunk of a patch + * Get data about a line in a hunk of a patch. + * + * Given a patch, a hunk index, and a line index in the hunk, this + * will return a lot of details about that line. If you pass a hunk + * index larger than the number of hunks or a line index larger than + * the number of lines in the hunk, this will return -1. + * + * @param line_origin A GIT_DIFF_LINE constant from above + * @param content Pointer to content of diff line, not NUL-terminated + * @param content_len Number of characters in content + * @param old_lineno Line number in old file or -1 if line is added + * @param new_lineno Line number in new file or -1 if line is deleted + * @param patch The patch to look in + * @param hunk_idx The index of the hunk + * @param line_of_index The index of the line in the hunk + * @return 0 on success, <0 on failure */ GIT_EXTERN(int) git_diff_patch_get_line_in_hunk( char *line_origin, const char **content, size_t *content_len, + int *old_lineno, + int *new_lineno, git_diff_patch *patch, - size_t hunk, + size_t hunk_idx, size_t line_of_hunk); /**@}*/ diff --git a/src/diff.c b/src/diff.c index 499b95b44..7c8e2a9bb 100644 --- a/src/diff.c +++ b/src/diff.c @@ -5,8 +5,6 @@ * a Linking Exception. For full terms see the included COPYING file. */ #include "common.h" -#include "git2/diff.h" -#include "git2/oid.h" #include "diff.h" #include "fileops.h" #include "config.h" @@ -268,9 +266,17 @@ static int diff_delta__from_two( delta->old_file.mode = old_mode; delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID; - git_oid_cpy(&delta->new_file.oid, new_oid ? new_oid : &new_entry->oid); + git_oid_cpy(&delta->new_file.oid, &new_entry->oid); delta->new_file.size = new_entry->file_size; delta->new_file.mode = new_mode; + + if (new_oid) { + if ((diff->opts.flags & GIT_DIFF_REVERSE) != 0) + git_oid_cpy(&delta->old_file.oid, new_oid); + else + git_oid_cpy(&delta->new_file.oid, new_oid); + } + if (new_oid || !git_oid_iszero(&new_entry->oid)) delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID; @@ -425,6 +431,11 @@ void git_diff_list_free(git_diff_list *diff) GIT_REFCOUNT_DEC(diff, diff_list_free); } +void git_diff_list_addref(git_diff_list *diff) +{ + GIT_REFCOUNT_INC(diff); +} + static int oid_for_workdir_item( git_repository *repo, const git_index_entry *item, @@ -519,17 +530,17 @@ static int maybe_modified( omode == nmode) status = GIT_DELTA_UNMODIFIED; - /* if modes match and we have an unknown OID and a workdir iterator, - * then check deeper for matching + /* if we have an unknown OID and a workdir iterator, then check some + * circumstances that can accelerate things or need special handling */ - else if (omode == nmode && - git_oid_iszero(&nitem->oid) && - new_iter->type == GIT_ITERATOR_WORKDIR) + else if (git_oid_iszero(&nitem->oid) && + new_iter->type == GIT_ITERATOR_WORKDIR) { /* TODO: add check against index file st_mtime to avoid racy-git */ - /* if they files look exactly alike, then we'll assume the same */ - if (oitem->file_size == nitem->file_size && + /* if the stat data looks exactly alike, then assume the same */ + if (omode == nmode && + oitem->file_size == nitem->file_size && (!(diff->diffcaps & GIT_DIFFCAPS_TRUST_CTIME) || (oitem->ctime.seconds == nitem->ctime.seconds)) && oitem->mtime.seconds == nitem->mtime.seconds && @@ -554,16 +565,15 @@ static int maybe_modified( status = GIT_DELTA_UNMODIFIED; } } + } - /* TODO: check git attributes so we will not have to read the file - * in if it is marked binary. - */ - - else if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0) + /* if we got here and decided that the files are modified, but we + * haven't calculated the OID of the new item, then calculate it now + */ + if (status == GIT_DELTA_MODIFIED && git_oid_iszero(&nitem->oid)) { + if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0) return -1; - - else if (git_oid_cmp(&oitem->oid, &noid) == 0 && - omode == nmode) + else if (omode == nmode && git_oid_equal(&oitem->oid, &noid)) status = GIT_DELTA_UNMODIFIED; /* store calculated oid so we don't have to recalc later */ diff --git a/src/diff.h b/src/diff.h index ea38a678f..862c33c1b 100644 --- a/src/diff.h +++ b/src/diff.h @@ -7,6 +7,9 @@ #ifndef INCLUDE_diff_h__ #define INCLUDE_diff_h__ +#include "git2/diff.h" +#include "git2/oid.h" + #include #include "vector.h" #include "buffer.h" @@ -25,14 +28,17 @@ enum { GIT_DIFFCAPS_USE_DEV = (1 << 4), /* use st_dev? */ }; -#define MAX_DIFF_FILESIZE 0x20000000 +typedef struct { + git_refcount rc; + git_diff_delta delta; +} git_diff_delta_refcounted; struct git_diff_list { git_refcount rc; git_repository *repo; git_diff_options opts; git_vector pathspec; - git_vector deltas; /* vector of git_diff_file_delta */ + git_vector deltas; /* vector of git_diff_delta_refcounted */ git_pool pool; git_iterator_type_t old_src; git_iterator_type_t new_src; @@ -42,27 +48,7 @@ struct git_diff_list { extern void git_diff__cleanup_modes( uint32_t diffcaps, uint32_t *omode, uint32_t *nmode); -/** - * Return the maximum possible number of files in the diff. - * - * NOTE: This number has to be treated as an upper bound on the number of - * files that have changed if the diff is with the working directory. - * - * Why?! For efficiency, we defer loading the file contents as long as - * possible, so if a file has been "touched" in the working directory and - * then reverted to the original content, it may get stored in the diff list - * as MODIFIED along with a flag that the status should be reconfirmed when - * it is actually loaded into memory. When that load happens, it could get - * flipped to UNMODIFIED. If unmodified files are being skipped, then the - * iterator will skip that file and this number may be too high. - * - * This behavior is true of `git_diff_foreach` as well, but the only - * implication there is that the `progress` value would not advance evenly. - * - * @param iterator The iterator object - * @return The maximum number of files to be iterated over - */ -int git_diff_iterator__max_files(git_diff_iterator *iterator); +extern void git_diff_list_addref(git_diff_list *diff); #endif diff --git a/src/diff_output.c b/src/diff_output.c index 58a1a3567..b84b0c2a4 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -5,58 +5,13 @@ * a Linking Exception. For full terms see the included COPYING file. */ #include "common.h" -#include "git2/diff.h" #include "git2/attr.h" -#include "git2/blob.h" #include "git2/oid.h" -#include "xdiff/xdiff.h" +#include "diff_output.h" #include -#include "diff.h" -#include "map.h" #include "fileops.h" #include "filter.h" -/* - * A diff_delta_context represents all of the information that goes into - * processing the diff of an observed file change. In the case of the - * git_diff_foreach() call it is an emphemeral structure that is filled - * in to execute each diff. In the case of a git_diff_iterator, it holds - * most of the information for the diff in progress. - * - * As each delta is processed, it goes through 3 phases: prep, load, exec. - * - * - In the prep phase, we just set the delta and quickly check the file - * attributes to see if it should be treated as binary. - * - In the load phase, we actually load the file content into memory. - * At this point, if we had deferred calculating OIDs, we might have to - * correct the delta to be UNMODIFIED. - * - In the exec phase, we actually run the diff and execute the callbacks. - * For foreach, this is just a pass-through to the user's callbacks. For - * iterators, we record the hunks and data spans into memory. - */ -typedef struct { - git_repository *repo; - git_diff_options *opts; - xdemitconf_t xdiff_config; - xpparam_t xdiff_params; - git_diff_delta *delta; - uint32_t prepped : 1; - uint32_t loaded : 1; - uint32_t diffable : 1; - uint32_t diffed : 1; - git_iterator_type_t old_src; - git_iterator_type_t new_src; - git_blob *old_blob; - git_blob *new_blob; - git_map old_data; - git_map new_data; - void *cb_data; - git_diff_hunk_fn per_hunk; - git_diff_data_fn per_line; - int cb_error; - git_diff_range range; -} diff_delta_context; - static int read_next_int(const char **str, int *value) { const char *scan = *str; @@ -96,55 +51,28 @@ static int parse_hunk_header(git_diff_range *range, const char *header) return 0; } -static int format_hunk_header(char *header, size_t len, git_diff_range *range) +static bool diff_delta_should_skip( + diff_context *ctxt, git_diff_delta *delta) { - if (range->old_lines != 1) { - if (range->new_lines != 1) - return p_snprintf( - header, len, "@@ -%d,%d +%d,%d @@", - range->old_start, range->old_lines, - range->new_start, range->new_lines); - else - return p_snprintf( - header, len, "@@ -%d,%d +%d @@", - range->old_start, range->old_lines, range->new_start); - } else { - if (range->new_lines != 1) - return p_snprintf( - header, len, "@@ -%d +%d,%d @@", - range->old_start, range->new_start, range->new_lines); - else - return p_snprintf( - header, len, "@@ -%d +%d @@", - range->old_start, range->new_start); - } -} + uint32_t flags = ctxt->opts ? ctxt->opts->flags : 0; -static bool diff_delta_is_ambiguous(git_diff_delta *delta) -{ - return (git_oid_iszero(&delta->new_file.oid) && - (delta->new_file.flags & GIT_DIFF_FILE_VALID_OID) == 0 && - delta->status == GIT_DELTA_MODIFIED); -} - -static bool diff_delta_should_skip(git_diff_options *opts, git_diff_delta *delta) -{ if (delta->status == GIT_DELTA_UNMODIFIED && - (opts->flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0) + (flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0) return true; if (delta->status == GIT_DELTA_IGNORED && - (opts->flags & GIT_DIFF_INCLUDE_IGNORED) == 0) + (flags & GIT_DIFF_INCLUDE_IGNORED) == 0) return true; if (delta->status == GIT_DELTA_UNTRACKED && - (opts->flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) + (flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) return true; return false; } -#define BINARY_DIFF_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY) +#define KNOWN_BINARY_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY) +#define NOT_BINARY_FLAGS (GIT_DIFF_FILE_NOT_BINARY|GIT_DIFF_FILE_NO_DATA) static int update_file_is_binary_by_attr( git_repository *repo, git_diff_file *file) @@ -173,8 +101,6 @@ static void update_delta_is_binary(git_diff_delta *delta) (delta->new_file.flags & GIT_DIFF_FILE_BINARY) != 0) delta->binary = 1; -#define NOT_BINARY_FLAGS (GIT_DIFF_FILE_NOT_BINARY|GIT_DIFF_FILE_NO_DATA) - else if ((delta->old_file.flags & NOT_BINARY_FLAGS) != 0 && (delta->new_file.flags & NOT_BINARY_FLAGS) != 0) delta->binary = 0; @@ -182,10 +108,11 @@ static void update_delta_is_binary(git_diff_delta *delta) /* otherwise leave delta->binary value untouched */ } -static int diff_delta_is_binary_by_attr(diff_delta_context *ctxt) +static int diff_delta_is_binary_by_attr( + diff_context *ctxt, git_diff_patch *patch) { int error = 0, mirror_new; - git_diff_delta *delta = ctxt->delta; + git_diff_delta *delta = patch->delta; delta->binary = -1; @@ -200,7 +127,7 @@ static int diff_delta_is_binary_by_attr(diff_delta_context *ctxt) } /* check if user is forcing us to text diff these files */ - if (ctxt->opts->flags & GIT_DIFF_FORCE_TEXT) { + if (ctxt->opts && (ctxt->opts->flags & GIT_DIFF_FORCE_TEXT) != 0) { delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->binary = 0; @@ -214,7 +141,7 @@ static int diff_delta_is_binary_by_attr(diff_delta_context *ctxt) mirror_new = (delta->new_file.path == delta->old_file.path || strcmp(delta->new_file.path, delta->old_file.path) == 0); if (mirror_new) - delta->new_file.flags |= (delta->old_file.flags & BINARY_DIFF_FLAGS); + delta->new_file.flags |= (delta->old_file.flags & KNOWN_BINARY_FLAGS); else error = update_file_is_binary_by_attr(ctxt->repo, &delta->new_file); @@ -224,11 +151,13 @@ static int diff_delta_is_binary_by_attr(diff_delta_context *ctxt) } static int diff_delta_is_binary_by_content( - diff_delta_context *ctxt, git_diff_file *file, git_map *map) + diff_context *ctxt, git_diff_delta *delta, git_diff_file *file, git_map *map) { git_buf search; - if ((file->flags & BINARY_DIFF_FLAGS) == 0) { + GIT_UNUSED(ctxt); + + if ((file->flags & KNOWN_BINARY_FLAGS) == 0) { search.ptr = map->data; search.size = min(map->len, 4000); @@ -238,17 +167,17 @@ static int diff_delta_is_binary_by_content( file->flags |= GIT_DIFF_FILE_NOT_BINARY; } - update_delta_is_binary(ctxt->delta); + update_delta_is_binary(delta); return 0; } static int diff_delta_is_binary_by_size( - diff_delta_context *ctxt, git_diff_file *file) + diff_context *ctxt, git_diff_delta *delta, git_diff_file *file) { git_off_t threshold = MAX_DIFF_FILESIZE; - if ((file->flags & BINARY_DIFF_FLAGS) != 0) + if ((file->flags & KNOWN_BINARY_FLAGS) != 0) return 0; if (ctxt && ctxt->opts) { @@ -262,7 +191,7 @@ static int diff_delta_is_binary_by_size( if (file->size > threshold) file->flags |= GIT_DIFF_FILE_BINARY; - update_delta_is_binary(ctxt->delta); + update_delta_is_binary(delta); return 0; } @@ -289,8 +218,10 @@ static void setup_xdiff_options( param->flags |= XDF_IGNORE_WHITESPACE_AT_EOL; } + static int get_blob_content( - diff_delta_context *ctxt, + diff_context *ctxt, + git_diff_delta *delta, git_diff_file *file, git_map *map, git_blob **blob) @@ -318,9 +249,9 @@ static int get_blob_content( } /* if blob is too large to diff, mark as binary */ - if ((error = diff_delta_is_binary_by_size(ctxt, file)) < 0) + if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0) return error; - if (ctxt->delta->binary == 1) + if (delta->binary == 1) return 0; if (odb_obj != NULL) { @@ -336,11 +267,12 @@ static int get_blob_content( map->data = (void *)git_blob_rawcontent(*blob); map->len = git_blob_rawsize(*blob); - return diff_delta_is_binary_by_content(ctxt, file, map); + return diff_delta_is_binary_by_content(ctxt, delta, file, map); } static int get_workdir_content( - diff_delta_context *ctxt, + diff_context *ctxt, + git_diff_delta *delta, git_diff_file *file, git_map *map) { @@ -386,8 +318,8 @@ static int get_workdir_content( if (!file->size) file->size = git_futils_filesize(fd); - if ((error = diff_delta_is_binary_by_size(ctxt, file)) < 0 || - ctxt->delta->binary == 1) + if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0 || + delta->binary == 1) goto close_and_cleanup; error = git_filters_load( @@ -428,7 +360,7 @@ close_and_cleanup: } if (!error) - error = diff_delta_is_binary_by_content(ctxt, file, map); + error = diff_delta_is_binary_by_content(ctxt, delta, file, map); cleanup: git_buf_free(&path); @@ -454,78 +386,101 @@ static void release_content(git_diff_file *file, git_map *map, git_blob *blob) } } -static void diff_delta_init_context( - diff_delta_context *ctxt, - git_repository *repo, + +static void diff_context_init( + diff_context *ctxt, + git_diff_list *diff, + git_repository *repo, git_diff_options *opts, - git_iterator_type_t old_src, - git_iterator_type_t new_src) + void *data, + git_diff_file_fn file_cb, + git_diff_hunk_fn hunk_cb, + git_diff_data_fn data_cb) { - memset(ctxt, 0, sizeof(diff_delta_context)); + memset(ctxt, 0, sizeof(diff_context)); ctxt->repo = repo; + ctxt->diff = diff; ctxt->opts = opts; - ctxt->old_src = old_src; - ctxt->new_src = new_src; + ctxt->file_cb = file_cb; + ctxt->hunk_cb = hunk_cb; + ctxt->data_cb = data_cb; + ctxt->cb_data = data; + ctxt->cb_error = 0; - setup_xdiff_options(opts, &ctxt->xdiff_config, &ctxt->xdiff_params); + setup_xdiff_options(ctxt->opts, &ctxt->xdiff_config, &ctxt->xdiff_params); } -static void diff_delta_init_context_from_diff_list( - diff_delta_context *ctxt, - git_diff_list *diff) +static bool diff_delta_file_callback( + diff_context *ctxt, git_diff_delta *delta, size_t idx) { - diff_delta_init_context( - ctxt, diff->repo, &diff->opts, diff->old_src, diff->new_src); -} + float progress; -static void diff_delta_unload(diff_delta_context *ctxt) -{ - ctxt->diffed = 0; - - if (ctxt->loaded) { - release_content(&ctxt->delta->old_file, &ctxt->old_data, ctxt->old_blob); - release_content(&ctxt->delta->new_file, &ctxt->new_data, ctxt->new_blob); - ctxt->loaded = 0; - } - - ctxt->delta = NULL; - ctxt->prepped = 0; -} - -static int diff_delta_prep(diff_delta_context *ctxt) -{ - int error; - - if (ctxt->prepped || !ctxt->delta) + if (!ctxt->file_cb) return 0; - error = diff_delta_is_binary_by_attr(ctxt); + progress = (float)idx / ctxt->diff->deltas.length; - ctxt->prepped = !error; + if (ctxt->file_cb(ctxt->cb_data, delta, progress) != 0) + return GIT_EUSER; - return error; + return 0; } -static int diff_delta_load(diff_delta_context *ctxt) +static void diff_patch_init( + diff_context *ctxt, git_diff_patch *patch) +{ + memset(patch, 0, sizeof(*patch)); + + patch->diff = ctxt->diff; + patch->ctxt = ctxt; + + if (patch->diff) { + patch->old_src = patch->diff->old_src; + patch->new_src = patch->diff->new_src; + } else { + patch->old_src = patch->new_src = GIT_ITERATOR_TREE; + } +} + +static git_diff_patch *diff_patch_alloc( + diff_context *ctxt, git_diff_delta *delta) +{ + git_diff_patch *patch = git__malloc(sizeof(git_diff_patch)); + if (!patch) + return NULL; + + diff_patch_init(ctxt, patch); + + git_diff_list_addref(patch->diff); + + GIT_REFCOUNT_INC(&patch); + + patch->delta = delta; + patch->flags = GIT_DIFF_PATCH_ALLOCATED; + + return patch; +} + +static int diff_patch_load( + diff_context *ctxt, git_diff_patch *patch) { int error = 0; - git_diff_delta *delta = ctxt->delta; + git_diff_delta *delta = patch->delta; bool check_if_unmodified = false; - if (ctxt->loaded || !ctxt->delta) + if ((patch->flags & GIT_DIFF_PATCH_LOADED) != 0) return 0; - if (!ctxt->prepped && (error = diff_delta_prep(ctxt)) < 0) - goto cleanup; + error = diff_delta_is_binary_by_attr(ctxt, patch); - ctxt->old_data.data = ""; - ctxt->old_data.len = 0; - ctxt->old_blob = NULL; + patch->old_data.data = ""; + patch->old_data.len = 0; + patch->old_blob = NULL; - ctxt->new_data.data = ""; - ctxt->new_data.len = 0; - ctxt->new_blob = NULL; + patch->new_data.data = ""; + patch->new_data.len = 0; + patch->new_blob = NULL; if (delta->binary == 1) goto cleanup; @@ -557,36 +512,38 @@ static int diff_delta_load(diff_delta_context *ctxt) */ if ((delta->old_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 && - ctxt->old_src == GIT_ITERATOR_WORKDIR) { + patch->old_src == GIT_ITERATOR_WORKDIR) { if ((error = get_workdir_content( - ctxt, &delta->old_file, &ctxt->old_data)) < 0) + ctxt, delta, &delta->old_file, &patch->old_data)) < 0) goto cleanup; if (delta->binary == 1) goto cleanup; } if ((delta->new_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 && - ctxt->new_src == GIT_ITERATOR_WORKDIR) { + patch->new_src == GIT_ITERATOR_WORKDIR) { if ((error = get_workdir_content( - ctxt, &delta->new_file, &ctxt->new_data)) < 0) + ctxt, delta, &delta->new_file, &patch->new_data)) < 0) goto cleanup; if (delta->binary == 1) goto cleanup; } if ((delta->old_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 && - ctxt->old_src != GIT_ITERATOR_WORKDIR) { + patch->old_src != GIT_ITERATOR_WORKDIR) { if ((error = get_blob_content( - ctxt, &delta->old_file, &ctxt->old_data, &ctxt->old_blob)) < 0) + ctxt, delta, &delta->old_file, + &patch->old_data, &patch->old_blob)) < 0) goto cleanup; if (delta->binary == 1) goto cleanup; } if ((delta->new_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 && - ctxt->new_src != GIT_ITERATOR_WORKDIR) { + patch->new_src != GIT_ITERATOR_WORKDIR) { if ((error = get_blob_content( - ctxt, &delta->new_file, &ctxt->new_data, &ctxt->new_blob)) < 0) + ctxt, delta, &delta->new_file, + &patch->new_data, &patch->new_blob)) < 0) goto cleanup; if (delta->binary == 1) goto cleanup; @@ -606,33 +563,34 @@ static int diff_delta_load(diff_delta_context *ctxt) } cleanup: - /* last change to update binary flag */ if (delta->binary == -1) update_delta_is_binary(delta); - ctxt->loaded = !error; + if (!error) { + patch->flags |= GIT_DIFF_PATCH_LOADED; - /* flag if we would want to diff the contents of these files */ - if (ctxt->loaded) - ctxt->diffable = - (delta->binary != 1 && - delta->status != GIT_DELTA_UNMODIFIED && - (ctxt->old_data.len || ctxt->new_data.len) && - git_oid_cmp(&delta->old_file.oid, &delta->new_file.oid)); + if (delta->binary != 1 && + delta->status != GIT_DELTA_UNMODIFIED && + (patch->old_data.len || patch->new_data.len) && + !git_oid_equal(&delta->old_file.oid, &delta->new_file.oid)) + patch->flags |= GIT_DIFF_PATCH_DIFFABLE; + } return error; } -static int diff_delta_cb(void *priv, mmbuffer_t *bufs, int len) +static int diff_patch_cb(void *priv, mmbuffer_t *bufs, int len) { - diff_delta_context *ctxt = priv; + git_diff_patch *patch = priv; + diff_context *ctxt = patch->ctxt; if (len == 1) { - if ((ctxt->cb_error = parse_hunk_header(&ctxt->range, bufs[0].ptr)) < 0) + ctxt->cb_error = parse_hunk_header(&ctxt->cb_range, bufs[0].ptr); + if (ctxt->cb_error < 0) return ctxt->cb_error; - if (ctxt->per_hunk != NULL && - ctxt->per_hunk(ctxt->cb_data, ctxt->delta, &ctxt->range, + if (ctxt->hunk_cb != NULL && + ctxt->hunk_cb(ctxt->cb_data, patch->delta, &ctxt->cb_range, bufs[0].ptr, bufs[0].size)) ctxt->cb_error = GIT_EUSER; } @@ -644,9 +602,9 @@ static int diff_delta_cb(void *priv, mmbuffer_t *bufs, int len) (*bufs[0].ptr == '-') ? GIT_DIFF_LINE_DELETION : GIT_DIFF_LINE_CONTEXT; - if (ctxt->per_line != NULL && - ctxt->per_line(ctxt->cb_data, ctxt->delta, &ctxt->range, origin, - bufs[1].ptr, bufs[1].size)) + if (ctxt->data_cb != NULL && + ctxt->data_cb(ctxt->cb_data, patch->delta, &ctxt->cb_range, + origin, bufs[1].ptr, bufs[1].size)) ctxt->cb_error = GIT_EUSER; } @@ -661,105 +619,268 @@ static int diff_delta_cb(void *priv, mmbuffer_t *bufs, int len) (*bufs[0].ptr == '-') ? GIT_DIFF_LINE_ADD_EOFNL : GIT_DIFF_LINE_CONTEXT; - if (ctxt->per_line != NULL && - ctxt->per_line(ctxt->cb_data, ctxt->delta, &ctxt->range, origin, - bufs[2].ptr, bufs[2].size)) + if (ctxt->data_cb != NULL && + ctxt->data_cb(ctxt->cb_data, patch->delta, &ctxt->cb_range, + origin, bufs[2].ptr, bufs[2].size)) ctxt->cb_error = GIT_EUSER; } return ctxt->cb_error; } -static int diff_delta_exec( - diff_delta_context *ctxt, - void *cb_data, - git_diff_hunk_fn per_hunk, - git_diff_data_fn per_line) +static int diff_patch_generate( + diff_context *ctxt, git_diff_patch *patch) { int error = 0; xdemitcb_t xdiff_callback; mmfile_t old_xdiff_data, new_xdiff_data; - if (ctxt->diffed || !ctxt->delta) + if ((patch->flags & GIT_DIFF_PATCH_DIFFED) != 0) return 0; - if (!ctxt->loaded && (error = diff_delta_load(ctxt)) < 0) - goto cleanup; + if ((patch->flags & GIT_DIFF_PATCH_LOADED) == 0) + if ((error = diff_patch_load(ctxt, patch)) < 0) + return error; - if (!ctxt->diffable) + if ((patch->flags & GIT_DIFF_PATCH_DIFFABLE) == 0) return 0; - ctxt->cb_data = cb_data; - ctxt->per_hunk = per_hunk; - ctxt->per_line = per_line; - ctxt->cb_error = 0; + if (ctxt) + patch->ctxt = ctxt; memset(&xdiff_callback, 0, sizeof(xdiff_callback)); - xdiff_callback.outf = diff_delta_cb; - xdiff_callback.priv = ctxt; + xdiff_callback.outf = diff_patch_cb; + xdiff_callback.priv = patch; - old_xdiff_data.ptr = ctxt->old_data.data; - old_xdiff_data.size = ctxt->old_data.len; - new_xdiff_data.ptr = ctxt->new_data.data; - new_xdiff_data.size = ctxt->new_data.len; + old_xdiff_data.ptr = patch->old_data.data; + old_xdiff_data.size = patch->old_data.len; + new_xdiff_data.ptr = patch->new_data.data; + new_xdiff_data.size = patch->new_data.len; xdl_diff(&old_xdiff_data, &new_xdiff_data, &ctxt->xdiff_params, &ctxt->xdiff_config, &xdiff_callback); error = ctxt->cb_error; -cleanup: - ctxt->diffed = !error; + if (!error) + patch->flags |= GIT_DIFF_PATCH_DIFFED; return error; } +static void diff_patch_unload(git_diff_patch *patch) +{ + if ((patch->flags & GIT_DIFF_PATCH_DIFFED) != 0) { + patch->flags = (patch->flags & ~GIT_DIFF_PATCH_DIFFED); + + patch->hunks_size = 0; + patch->lines_size = 0; + } + + if ((patch->flags & GIT_DIFF_PATCH_LOADED) != 0) { + patch->flags = (patch->flags & ~GIT_DIFF_PATCH_LOADED); + + release_content( + &patch->delta->old_file, &patch->old_data, patch->old_blob); + release_content( + &patch->delta->new_file, &patch->new_data, patch->new_blob); + } +} + +static void diff_patch_free(git_diff_patch *patch) +{ + diff_patch_unload(patch); + + git__free(patch->lines); + patch->lines = NULL; + patch->lines_asize = 0; + + git__free(patch->hunks); + patch->hunks = NULL; + patch->hunks_asize = 0; + + if (!(patch->flags & GIT_DIFF_PATCH_ALLOCATED)) + return; + + patch->flags = 0; + + git_diff_list_free(patch->diff); /* decrements refcount */ + + git__free(patch); +} + +#define MAX_HUNK_STEP 128 +#define MIN_HUNK_STEP 8 +#define MAX_LINE_STEP 256 +#define MIN_LINE_STEP 8 + +static int diff_patch_hunk_cb( + void *cb_data, + git_diff_delta *delta, + git_diff_range *range, + const char *header, + size_t header_len) +{ + git_diff_patch *patch = cb_data; + diff_patch_hunk *hunk; + + GIT_UNUSED(delta); + + if (patch->hunks_size >= patch->hunks_asize) { + size_t new_size; + diff_patch_hunk *new_hunks; + + if (patch->hunks_asize > MAX_HUNK_STEP) + new_size = patch->hunks_asize + MAX_HUNK_STEP; + else + new_size = patch->hunks_asize * 2; + if (new_size < MIN_HUNK_STEP) + new_size = MIN_HUNK_STEP; + + new_hunks = git__realloc( + patch->hunks, new_size * sizeof(diff_patch_hunk)); + if (!new_hunks) + return -1; + + patch->hunks = new_hunks; + patch->hunks_asize = new_size; + } + + hunk = &patch->hunks[patch->hunks_size++]; + + memcpy(&hunk->range, range, sizeof(hunk->range)); + + assert(header_len + 1 < sizeof(hunk->header)); + memcpy(&hunk->header, header, header_len); + hunk->header[header_len] = '\0'; + hunk->header_len = header_len; + + hunk->line_start = patch->lines_size; + hunk->line_count = 0; + + return 0; +} + +static int diff_patch_line_cb( + void *cb_data, + git_diff_delta *delta, + git_diff_range *range, + char line_origin, + const char *content, + size_t content_len) +{ + git_diff_patch *patch = cb_data; + diff_patch_hunk *hunk; + diff_patch_line *last, *line; + + GIT_UNUSED(delta); + GIT_UNUSED(range); + + assert(patch->hunks_size > 0); + assert(patch->hunks != NULL); + + hunk = &patch->hunks[patch->hunks_size - 1]; + + if (patch->lines_size >= patch->lines_asize) { + size_t new_size; + diff_patch_line *new_lines; + + if (patch->lines_asize > MAX_LINE_STEP) + new_size = patch->lines_asize + MAX_LINE_STEP; + else + new_size = patch->lines_asize * 2; + if (new_size < MIN_LINE_STEP) + new_size = MIN_LINE_STEP; + + new_lines = git__realloc( + patch->lines, new_size * sizeof(diff_patch_line)); + if (!new_lines) + return -1; + + patch->lines = new_lines; + patch->lines_asize = new_size; + } + + last = (patch->lines_size > 0) ? + &patch->lines[patch->lines_size - 1] : NULL; + line = &patch->lines[patch->lines_size++]; + + line->ptr = content; + line->len = content_len; + line->origin = line_origin; + + /* do some bookkeeping so we can provide old/new line numbers */ + + for (line->lines = 0; content_len > 0; --content_len) { + if (*content++ == '\n') + ++line->lines; + } + + if (!last) { + line->oldno = hunk->range.old_start; + line->newno = hunk->range.new_start; + } else { + switch (last->origin) { + case GIT_DIFF_LINE_ADDITION: + line->oldno = last->oldno; + line->newno = last->newno + last->lines; + break; + case GIT_DIFF_LINE_DELETION: + line->oldno = last->oldno + last->lines; + line->newno = last->newno; + break; + default: + line->oldno = last->oldno + last->lines; + line->newno = last->newno + last->lines; + break; + } + } + + return 0; +} + + int git_diff_foreach( git_diff_list *diff, - void *data, + void *cb_data, git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, - git_diff_data_fn line_cb) + git_diff_data_fn data_cb) { - int error = 0; - diff_delta_context ctxt; + int error; + diff_context ctxt; size_t idx; + git_diff_patch patch; - diff_delta_init_context_from_diff_list(&ctxt, diff); + diff_context_init( + &ctxt, diff, diff->repo, &diff->opts, + cb_data, file_cb, hunk_cb, data_cb); - git_vector_foreach(&diff->deltas, idx, ctxt.delta) { - if (diff_delta_is_ambiguous(ctxt.delta)) - if ((error = diff_delta_load(&ctxt)) < 0) - goto cleanup; + diff_patch_init(&ctxt, &patch); - if (diff_delta_should_skip(ctxt.opts, ctxt.delta)) + git_vector_foreach(&diff->deltas, idx, patch.delta) { + + /* check flags against patch status */ + if (diff_delta_should_skip(&ctxt, patch.delta)) continue; - if ((error = diff_delta_load(&ctxt)) < 0) - goto cleanup; + if (!(error = diff_patch_load(&ctxt, &patch)) && + !(error = diff_delta_file_callback(&ctxt, patch.delta, idx))) + error = diff_patch_generate(&ctxt, &patch); - if (file_cb != NULL && - file_cb(data, ctxt.delta, (float)idx / diff->deltas.length) != 0) - { - error = GIT_EUSER; - goto cleanup; - } - - error = diff_delta_exec(&ctxt, data, hunk_cb, line_cb); - -cleanup: - diff_delta_unload(&ctxt); + diff_patch_unload(&patch); if (error < 0) break; } if (error == GIT_EUSER) - giterr_clear(); + giterr_clear(); /* don't let error message leak */ return error; } + typedef struct { git_diff_list *diff; git_diff_data_fn print_cb; @@ -851,7 +972,6 @@ int git_diff_print_compact( return error; } - static int print_oid_range(diff_print_info *pi, git_diff_delta *delta) { char start_oid[8], end_oid[8]; @@ -1024,30 +1144,6 @@ int git_diff_print_patch( return error; } -int git_diff_entrycount(git_diff_list *diff, int delta_t) -{ - int count = 0; - unsigned int i; - git_diff_delta *delta; - - assert(diff); - - git_vector_foreach(&diff->deltas, i, delta) { - if (diff_delta_should_skip(&diff->opts, delta)) - continue; - - if (delta_t < 0 || delta->status == (git_delta_t)delta_t) - count++; - } - - /* It is possible that this has overcounted the number of diffs because - * there may be entries that are marked as MODIFIED due to differences - * in stat() output that will turn out to be the same once we calculate - * the actual SHA of the data on disk. - */ - - return count; -} static void set_data_from_blob( git_blob *blob, git_map *map, git_diff_file *file) @@ -1056,6 +1152,7 @@ static void set_data_from_blob( map->data = (char *)git_blob_rawcontent(blob); file->size = map->len = git_blob_rawsize(blob); git_oid_cpy(&file->oid, git_object_id((const git_object *)blob)); + file->mode = 0644; } else { map->data = ""; file->size = map->len = 0; @@ -1070,429 +1167,250 @@ int git_diff_blobs( void *cb_data, git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, - git_diff_data_fn line_cb) + git_diff_data_fn data_cb) { int error; - diff_delta_context ctxt; - git_diff_delta delta; - git_blob *new, *old; git_repository *repo; - - new = new_blob; - old = old_blob; + diff_context ctxt; + git_diff_delta delta; + git_diff_patch patch; if (options && (options->flags & GIT_DIFF_REVERSE)) { - git_blob *swap = old; - old = new; - new = swap; + git_blob *swap = old_blob; + old_blob = new_blob; + new_blob = swap; } - if (new) - repo = git_object_owner((git_object *)new); - else if (old) - repo = git_object_owner((git_object *)old); + if (new_blob) + repo = git_object_owner((git_object *)new_blob); + else if (old_blob) + repo = git_object_owner((git_object *)old_blob); else repo = NULL; - diff_delta_init_context( - &ctxt, repo, options, GIT_ITERATOR_TREE, GIT_ITERATOR_TREE); + diff_context_init( + &ctxt, NULL, repo, options, + cb_data, file_cb, hunk_cb, data_cb); - /* populate a "fake" delta record */ + diff_patch_init(&ctxt, &patch); + + /* create a fake delta record and simulate diff_patch_load */ memset(&delta, 0, sizeof(delta)); + delta.binary = -1; - set_data_from_blob(old, &ctxt.old_data, &delta.old_file); - set_data_from_blob(new, &ctxt.new_data, &delta.new_file); + set_data_from_blob(old_blob, &patch.old_data, &delta.old_file); + set_data_from_blob(new_blob, &patch.new_data, &delta.new_file); - delta.status = new ? - (old ? GIT_DELTA_MODIFIED : GIT_DELTA_ADDED) : - (old ? GIT_DELTA_DELETED : GIT_DELTA_UNTRACKED); + delta.status = new_blob ? + (old_blob ? GIT_DELTA_MODIFIED : GIT_DELTA_ADDED) : + (old_blob ? GIT_DELTA_DELETED : GIT_DELTA_UNTRACKED); if (git_oid_cmp(&delta.new_file.oid, &delta.old_file.oid) == 0) delta.status = GIT_DELTA_UNMODIFIED; - ctxt.delta = δ + patch.delta = δ - if ((error = diff_delta_prep(&ctxt)) < 0) + if ((error = diff_delta_is_binary_by_content( + &ctxt, &delta, &delta.old_file, &patch.old_data)) < 0 || + (error = diff_delta_is_binary_by_content( + &ctxt, &delta, &delta.new_file, &patch.new_data)) < 0) goto cleanup; - if (delta.binary == -1) { - if ((error = diff_delta_is_binary_by_content( - &ctxt, &delta.old_file, &ctxt.old_data)) < 0 || - (error = diff_delta_is_binary_by_content( - &ctxt, &delta.new_file, &ctxt.new_data)) < 0) - goto cleanup; - } - - ctxt.loaded = 1; - ctxt.diffable = (delta.binary != 1 && delta.status != GIT_DELTA_UNMODIFIED); + patch.flags |= GIT_DIFF_PATCH_LOADED; + if (delta.binary != 1 && delta.status != GIT_DELTA_UNMODIFIED) + patch.flags |= GIT_DIFF_PATCH_DIFFABLE; /* do diffs */ - if (file_cb != NULL && file_cb(cb_data, &delta, 1)) { - error = GIT_EUSER; - goto cleanup; - } - - error = diff_delta_exec(&ctxt, cb_data, hunk_cb, line_cb); + if (!(error = diff_delta_file_callback(&ctxt, patch.delta, 1))) + error = diff_patch_generate(&ctxt, &patch); cleanup: + diff_patch_unload(&patch); + if (error == GIT_EUSER) giterr_clear(); - diff_delta_unload(&ctxt); - return error; } -typedef struct diffiter_line diffiter_line; -struct diffiter_line { - diffiter_line *next; - char origin; - const char *ptr; - size_t len; -}; -typedef struct diffiter_hunk diffiter_hunk; -struct diffiter_hunk { - diffiter_hunk *next; - git_diff_range range; - diffiter_line *line_head; - size_t line_count; -}; - -struct git_diff_iterator { - git_diff_list *diff; - diff_delta_context ctxt; - size_t file_index; - size_t next_index; - git_pool hunks; - size_t hunk_count; - diffiter_hunk *hunk_head; - diffiter_hunk *hunk_curr; - char hunk_header[128]; - git_pool lines; - diffiter_line *line_curr; -}; - -typedef struct { - git_diff_iterator *iter; - diffiter_hunk *last_hunk; - diffiter_line *last_line; -} diffiter_cb_info; - -static int diffiter_hunk_cb( - void *cb_data, - git_diff_delta *delta, - git_diff_range *range, - const char *header, - size_t header_len) +size_t git_diff_num_deltas(git_diff_list *diff) { - diffiter_cb_info *info = cb_data; - git_diff_iterator *iter = info->iter; - diffiter_hunk *hunk; - - GIT_UNUSED(delta); - GIT_UNUSED(header); - GIT_UNUSED(header_len); - - if ((hunk = git_pool_mallocz(&iter->hunks, 1)) == NULL) { - iter->ctxt.cb_error = -1; - return -1; - } - - if (info->last_hunk) - info->last_hunk->next = hunk; - info->last_hunk = hunk; - info->last_line = NULL; - - memcpy(&hunk->range, range, sizeof(hunk->range)); - - iter->hunk_count++; - - /* adding first hunk to list */ - if (iter->hunk_head == NULL) { - iter->hunk_head = hunk; - iter->hunk_curr = NULL; - } - - return 0; + assert(diff); + return (size_t)diff->deltas.length; } -static int diffiter_line_cb( - void *cb_data, - git_diff_delta *delta, - git_diff_range *range, - char line_origin, - const char *content, - size_t content_len) +size_t git_diff_num_deltas_of_type(git_diff_list *diff, git_delta_t type) { - diffiter_cb_info *info = cb_data; - git_diff_iterator *iter = info->iter; - diffiter_line *line; + size_t i, count = 0; + git_diff_delta *delta; - GIT_UNUSED(delta); - GIT_UNUSED(range); + assert(diff); - if ((line = git_pool_mallocz(&iter->lines, 1)) == NULL) { - iter->ctxt.cb_error = -1; - return -1; + git_vector_foreach(&diff->deltas, i, delta) { + count += (delta->status == type); } - if (info->last_line) - info->last_line->next = line; - info->last_line = line; - - line->origin = line_origin; - line->ptr = content; - line->len = content_len; - - info->last_hunk->line_count++; - - if (info->last_hunk->line_head == NULL) - info->last_hunk->line_head = line; - - return 0; + return count; } -static int diffiter_do_diff_file(git_diff_iterator *iter) +int git_diff_get_patch( + git_diff_patch **patch_ptr, + git_diff_delta **delta_ptr, + git_diff_list *diff, + size_t idx) { int error; - diffiter_cb_info info; + diff_context ctxt; + git_diff_delta *delta; + git_diff_patch *patch; - if (iter->ctxt.diffed || !iter->ctxt.delta) + if (patch_ptr) + *patch_ptr = NULL; + + delta = git_vector_get(&diff->deltas, idx); + if (!delta) { + if (delta_ptr) + *delta_ptr = NULL; + giterr_set(GITERR_INVALID, "Index out of range for delta in diff"); + return GIT_ENOTFOUND; + } + + if (delta_ptr) + *delta_ptr = delta; + + if (!patch_ptr && delta->binary != -1) return 0; - memset(&info, 0, sizeof(info)); - info.iter = iter; + diff_context_init( + &ctxt, diff, diff->repo, &diff->opts, + NULL, NULL, diff_patch_hunk_cb, diff_patch_line_cb); - error = diff_delta_exec( - &iter->ctxt, &info, diffiter_hunk_cb, diffiter_line_cb); + if (diff_delta_should_skip(&ctxt, delta)) + return 0; - if (error == GIT_EUSER) - error = iter->ctxt.cb_error; + patch = diff_patch_alloc(&ctxt, delta); + if (!patch) + return -1; - return error; -} + if (!(error = diff_patch_load(&ctxt, patch))) { + ctxt.cb_data = patch; -static void diffiter_do_unload_file(git_diff_iterator *iter) -{ - if (iter->ctxt.loaded) { - diff_delta_unload(&iter->ctxt); + error = diff_patch_generate(&ctxt, patch); - git_pool_clear(&iter->lines); - git_pool_clear(&iter->hunks); + if (error == GIT_EUSER) + error = ctxt.cb_error; } - iter->ctxt.delta = NULL; - iter->hunk_curr = iter->hunk_head = NULL; - iter->hunk_count = 0; - iter->line_curr = NULL; -} - -int git_diff_iterator_new( - git_diff_iterator **iterator_ptr, - git_diff_list *diff) -{ - git_diff_iterator *iter; - - assert(diff && iterator_ptr); - - *iterator_ptr = NULL; - - iter = git__malloc(sizeof(git_diff_iterator)); - GITERR_CHECK_ALLOC(iter); - - memset(iter, 0, sizeof(*iter)); - - iter->diff = diff; - GIT_REFCOUNT_INC(iter->diff); - - diff_delta_init_context_from_diff_list(&iter->ctxt, diff); - - if (git_pool_init(&iter->hunks, sizeof(diffiter_hunk), 0) < 0 || - git_pool_init(&iter->lines, sizeof(diffiter_line), 0) < 0) - goto fail; - - *iterator_ptr = iter; - - return 0; - -fail: - git_diff_iterator_free(iter); - - return -1; -} - -void git_diff_iterator_free(git_diff_iterator *iter) -{ - diffiter_do_unload_file(iter); - git_diff_list_free(iter->diff); /* decrement ref count */ - git__free(iter); -} - -float git_diff_iterator_progress(git_diff_iterator *iter) -{ - return (float)iter->next_index / (float)iter->diff->deltas.length; -} - -int git_diff_iterator__max_files(git_diff_iterator *iter) -{ - return (int)iter->diff->deltas.length; -} - -int git_diff_iterator_num_hunks_in_file(git_diff_iterator *iter) -{ - int error = diffiter_do_diff_file(iter); - return (error != 0) ? error : (int)iter->hunk_count; -} - -int git_diff_iterator_num_lines_in_hunk(git_diff_iterator *iter) -{ - int error = diffiter_do_diff_file(iter); if (error) - return error; - - if (iter->hunk_curr) - return (int)iter->hunk_curr->line_count; - if (iter->hunk_head) - return (int)iter->hunk_head->line_count; - return 0; -} - -int git_diff_iterator_next_file( - git_diff_delta **delta_ptr, - git_diff_iterator *iter) -{ - int error = 0; - - assert(iter); - - iter->file_index = iter->next_index; - - diffiter_do_unload_file(iter); - - while (!error) { - iter->ctxt.delta = git_vector_get(&iter->diff->deltas, iter->file_index); - if (!iter->ctxt.delta) { - error = GIT_ITEROVER; - break; - } - - if (diff_delta_is_ambiguous(iter->ctxt.delta) && - (error = diff_delta_load(&iter->ctxt)) < 0) - break; - - if (!diff_delta_should_skip(iter->ctxt.opts, iter->ctxt.delta)) - break; - - iter->file_index++; - } - - if (!error) { - iter->next_index = iter->file_index + 1; - - error = diff_delta_prep(&iter->ctxt); - } - - if (iter->ctxt.delta == NULL) { - iter->hunk_curr = iter->hunk_head = NULL; - iter->line_curr = NULL; - } - - if (delta_ptr != NULL) - *delta_ptr = !error ? iter->ctxt.delta : NULL; + git_diff_patch_free(patch); + else if (patch_ptr) + *patch_ptr = patch; return error; } -int git_diff_iterator_next_hunk( - git_diff_range **range_ptr, +void git_diff_patch_free(git_diff_patch *patch) +{ + if (patch) + GIT_REFCOUNT_DEC(patch, diff_patch_free); +} + +const git_diff_delta *git_diff_patch_delta(git_diff_patch *patch) +{ + assert(patch); + return patch->delta; +} + +size_t git_diff_patch_num_hunks(git_diff_patch *patch) +{ + assert(patch); + return patch->hunks_size; +} + +int git_diff_patch_get_hunk( + git_diff_range **range, const char **header, size_t *header_len, - git_diff_iterator *iter) + size_t *lines_in_hunk, + git_diff_patch *patch, + size_t hunk_idx) { - int error = diffiter_do_diff_file(iter); - git_diff_range *range; + diff_patch_hunk *hunk; - if (error) - return error; + assert(patch); - if (iter->hunk_curr == NULL) { - if (iter->hunk_head == NULL) - goto no_more_hunks; - iter->hunk_curr = iter->hunk_head; - } else { - if (iter->hunk_curr->next == NULL) - goto no_more_hunks; - iter->hunk_curr = iter->hunk_curr->next; + if (hunk_idx >= patch->hunks_size) { + if (range) *range = NULL; + if (header) *header = NULL; + if (header_len) *header_len = 0; + if (lines_in_hunk) *lines_in_hunk = 0; + return GIT_ENOTFOUND; } - range = &iter->hunk_curr->range; + hunk = &patch->hunks[hunk_idx]; - if (range_ptr) - *range_ptr = range; + if (range) *range = &hunk->range; + if (header) *header = hunk->header; + if (header_len) *header_len = hunk->header_len; + if (lines_in_hunk) *lines_in_hunk = hunk->line_count; - if (header) { - int out = format_hunk_header( - iter->hunk_header, sizeof(iter->hunk_header), range); - - /* TODO: append function name to header */ - - *(iter->hunk_header + out++) = '\n'; - - *header = iter->hunk_header; - - if (header_len) - *header_len = (size_t)out; - } - - iter->line_curr = iter->hunk_curr->line_head; - - return error; - -no_more_hunks: - if (range_ptr) *range_ptr = NULL; - if (header) *header = NULL; - if (header_len) *header_len = 0; - iter->line_curr = NULL; - - return GIT_ITEROVER; + return 0; } -int git_diff_iterator_next_line( - char *line_origin, /**< GIT_DIFF_LINE_... value from above */ - const char **content_ptr, +int git_diff_patch_num_lines_in_hunk( + git_diff_patch *patch, + size_t hunk_idx) +{ + assert(patch); + + if (hunk_idx >= patch->hunks_size) + return GIT_ENOTFOUND; + else + return patch->hunks[hunk_idx].line_count; +} + +int git_diff_patch_get_line_in_hunk( + char *line_origin, + const char **content, size_t *content_len, - git_diff_iterator *iter) + int *old_lineno, + int *new_lineno, + git_diff_patch *patch, + size_t hunk_idx, + size_t line_of_hunk) { - int error = diffiter_do_diff_file(iter); + diff_patch_hunk *hunk; + diff_patch_line *line; - if (error) - return error; + assert(patch); - /* if the user has not called next_hunk yet, call it implicitly (OK?) */ - if (iter->hunk_curr == NULL) { - error = git_diff_iterator_next_hunk(NULL, NULL, NULL, iter); - if (error) - return error; - } + if (hunk_idx >= patch->hunks_size) + goto notfound; + hunk = &patch->hunks[hunk_idx]; - if (iter->line_curr == NULL) { - if (line_origin) *line_origin = GIT_DIFF_LINE_CONTEXT; - if (content_ptr) *content_ptr = NULL; - if (content_len) *content_len = 0; - return GIT_ITEROVER; - } + if (line_of_hunk >= hunk->line_count) + goto notfound; - if (line_origin) - *line_origin = iter->line_curr->origin; - if (content_ptr) - *content_ptr = iter->line_curr->ptr; - if (content_len) - *content_len = iter->line_curr->len; + line = &patch->lines[hunk->line_start + line_of_hunk]; - iter->line_curr = iter->line_curr->next; + if (line_origin) *line_origin = line->origin; + if (content) *content = line->ptr; + if (content_len) *content_len = line->len; + if (old_lineno) *old_lineno = line->oldno; + if (new_lineno) *new_lineno = line->newno; - return error; + return 0; + +notfound: + if (line_origin) *line_origin = GIT_DIFF_LINE_CONTEXT; + if (content) *content = NULL; + if (content_len) *content_len = 0; + if (old_lineno) *old_lineno = -1; + if (new_lineno) *new_lineno = -1; + + return GIT_ENOTFOUND; } + diff --git a/src/diff_output.h b/src/diff_output.h new file mode 100644 index 000000000..f54639088 --- /dev/null +++ b/src/diff_output.h @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2009-2012 the libgit2 contributors + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_diff_output_h__ +#define INCLUDE_diff_output_h__ + +#include "git2/blob.h" +#include "diff.h" +#include "map.h" +#include "xdiff/xdiff.h" + +#define MAX_DIFF_FILESIZE 0x20000000 + +enum { + GIT_DIFF_PATCH_ALLOCATED = (1 << 0), + GIT_DIFF_PATCH_PREPPED = (1 << 1), + GIT_DIFF_PATCH_LOADED = (1 << 2), + GIT_DIFF_PATCH_DIFFABLE = (1 << 3), + GIT_DIFF_PATCH_DIFFED = (1 << 4), +}; + +/* context for performing diffs */ +typedef struct { + git_repository *repo; + git_diff_list *diff; + git_diff_options *opts; + git_diff_file_fn file_cb; + git_diff_hunk_fn hunk_cb; + git_diff_data_fn data_cb; + void *cb_data; + int cb_error; + git_diff_range cb_range; + xdemitconf_t xdiff_config; + xpparam_t xdiff_params; +} diff_context; + +/* cached information about a single span in a diff */ +typedef struct diff_patch_line diff_patch_line; +struct diff_patch_line { + const char *ptr; + size_t len; + int lines, oldno, newno; + char origin; +}; + +/* cached information about a hunk in a diff */ +typedef struct diff_patch_hunk diff_patch_hunk; +struct diff_patch_hunk { + git_diff_range range; + char header[128]; + size_t header_len; + size_t line_start; + size_t line_count; +}; + +struct git_diff_patch { + git_refcount rc; + git_diff_list *diff; /* for refcount purposes, maybe NULL for blob diffs */ + git_diff_delta *delta; + diff_context *ctxt; /* only valid while generating patch */ + git_iterator_type_t old_src; + git_iterator_type_t new_src; + git_blob *old_blob; + git_blob *new_blob; + git_map old_data; + git_map new_data; + uint32_t flags; + diff_patch_hunk *hunks; + size_t hunks_asize, hunks_size; + diff_patch_line *lines; + size_t lines_asize, lines_size; +}; + +/* context for performing diff on a single delta */ +typedef struct { + git_diff_patch *patch; + uint32_t prepped : 1; + uint32_t loaded : 1; + uint32_t diffable : 1; + uint32_t diffed : 1; +} diff_delta_context; + +#endif diff --git a/src/submodule.c b/src/submodule.c index 5ae38bccd..e55e12863 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1457,7 +1457,7 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm) error = git_diff_index_to_tree(sm_repo, &opt, sm_head, &diff); if (!error) { - if (git_diff_entrycount(diff, -1) > 0) + if (git_diff_num_deltas(diff) > 0) *status |= GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED; git_diff_list_free(diff); @@ -1474,12 +1474,13 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm) error = git_diff_workdir_to_index(sm_repo, &opt, &diff); if (!error) { - int untracked = git_diff_entrycount(diff, GIT_DELTA_UNTRACKED); + int untracked = + git_diff_num_deltas_of_type(diff, GIT_DELTA_UNTRACKED); if (untracked > 0) *status |= GIT_SUBMODULE_STATUS_WD_UNTRACKED; - if (git_diff_entrycount(diff, -1) - untracked > 0) + if ((git_diff_num_deltas(diff) - untracked) > 0) *status |= GIT_SUBMODULE_STATUS_WD_WD_MODIFIED; git_diff_list_free(diff); diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index 767b34392..1c0435975 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -112,64 +112,65 @@ int diff_foreach_via_iterator( git_diff_hunk_fn hunk_cb, git_diff_data_fn line_cb) { - int error; - git_diff_iterator *iter; - git_diff_delta *delta; + size_t d, num_d = git_diff_num_deltas(diff); - if ((error = git_diff_iterator_new(&iter, diff)) < 0) - return error; + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + size_t h, num_h; - while (!(error = git_diff_iterator_next_file(&delta, iter))) { - git_diff_range *range; - const char *hdr; - size_t hdr_len; - float progress = git_diff_iterator_progress(iter); + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); + cl_assert(delta && patch); /* call file_cb for this file */ - if (file_cb != NULL && file_cb(data, delta, progress) != 0) + if (file_cb != NULL && file_cb(data, delta, (float)d / num_d) != 0) { + git_diff_patch_free(patch); goto abort; - - if (!hunk_cb && !line_cb) - continue; - - while (!(error = git_diff_iterator_next_hunk( - &range, &hdr, &hdr_len, iter))) { - char origin; - const char *line; - size_t line_len; - - if (hunk_cb && hunk_cb(data, delta, range, hdr, hdr_len) != 0) - goto abort; - - if (!line_cb) - continue; - - while (!(error = git_diff_iterator_next_line( - &origin, &line, &line_len, iter))) { - - if (line_cb(data, delta, range, origin, line, line_len) != 0) - goto abort; - } - - if (error && error != GIT_ITEROVER) - goto done; } - if (error && error != GIT_ITEROVER) - goto done; + if (!hunk_cb && !line_cb) { + git_diff_patch_free(patch); + continue; + } + + num_h = git_diff_patch_num_hunks(patch); + + for (h = 0; h < num_h; h++) { + git_diff_range *range; + const char *hdr; + size_t hdr_len, l, num_l; + + cl_git_pass(git_diff_patch_get_hunk( + &range, &hdr, &hdr_len, &num_l, patch, h)); + + if (hunk_cb && hunk_cb(data, delta, range, hdr, hdr_len) != 0) { + git_diff_patch_free(patch); + goto abort; + } + + for (l = 0; l < num_l; ++l) { + char origin; + const char *line; + size_t line_len; + int old_lineno, new_lineno; + + cl_git_pass(git_diff_patch_get_line_in_hunk( + &origin, &line, &line_len, &old_lineno, &new_lineno, + patch, h, l)); + + if (line_cb(data, delta, range, origin, line, line_len) != 0) { + git_diff_patch_free(patch); + goto abort; + } + } + } + + git_diff_patch_free(patch); } -done: - git_diff_iterator_free(iter); - - if (error == GIT_ITEROVER) - error = 0; - - return error; + return 0; abort: - git_diff_iterator_free(iter); giterr_clear(); - return GIT_EUSER; } diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 23071e48b..9e33d91e1 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -14,11 +14,16 @@ void test_diff_diffiter__create(void) { git_repository *repo = cl_git_sandbox_init("attr"); git_diff_list *diff; - git_diff_iterator *iter; + size_t d, num_d; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - git_diff_iterator_free(iter); + + num_d = git_diff_num_deltas(diff); + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); + } + git_diff_list_free(diff); } @@ -26,24 +31,22 @@ void test_diff_diffiter__iterate_files(void) { git_repository *repo = cl_git_sandbox_init("attr"); git_diff_list *diff; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, count = 0; + size_t d, num_d; + int count = 0; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_d = git_diff_num_deltas(diff); + cl_assert_equal_i(6, num_d); + + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; } - - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(6, count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -51,24 +54,22 @@ void test_diff_diffiter__iterate_files_2(void) { git_repository *repo = cl_git_sandbox_init("status"); git_diff_list *diff; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, count = 0; + size_t d, num_d; + int count = 0; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_d = git_diff_num_deltas(diff); + cl_assert_equal_i(8, num_d); + + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; } - - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(8, count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -77,12 +78,8 @@ void test_diff_diffiter__iterate_files_and_hunks(void) git_repository *repo = cl_git_sandbox_init("status"); git_diff_options opts = {0}; git_diff_list *diff = NULL; - git_diff_iterator *iter; - git_diff_delta *delta; - git_diff_range *range; - const char *header; - size_t header_len; - int error, file_count = 0, hunk_count = 0; + size_t d, num_d; + int file_count = 0, hunk_count = 0; opts.context_lines = 3; opts.interhunk_lines = 1; @@ -90,28 +87,42 @@ void test_diff_diffiter__iterate_files_and_hunks(void) cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); + + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + size_t h, num_h; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); cl_assert(delta); + cl_assert(patch); file_count++; - while ((error = git_diff_iterator_next_hunk( - &range, &header, &header_len, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_h = git_diff_patch_num_hunks(patch); + + for (h = 0; h < num_h; h++) { + git_diff_range *range; + const char *header; + size_t header_len, num_l; + + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); + cl_assert(range); + cl_assert(header); + hunk_count++; } + + git_diff_patch_free(patch); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(13, file_count); cl_assert_equal_i(8, hunk_count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -120,45 +131,42 @@ void test_diff_diffiter__max_size_threshold(void) git_repository *repo = cl_git_sandbox_init("status"); git_diff_options opts = {0}; git_diff_list *diff = NULL; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, file_count = 0, binary_count = 0, hunk_count = 0; + int file_count = 0, binary_count = 0, hunk_count = 0; + size_t d, num_d; opts.context_lines = 3; opts.interhunk_lines = 1; opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); cl_assert(delta); + cl_assert(patch); file_count++; - - hunk_count += git_diff_iterator_num_hunks_in_file(iter); + hunk_count += git_diff_patch_num_hunks(patch); assert(delta->binary == 0 || delta->binary == 1); - binary_count += delta->binary; - } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); + git_diff_patch_free(patch); + } cl_assert_equal_i(13, file_count); cl_assert_equal_i(0, binary_count); cl_assert_equal_i(8, hunk_count); - git_diff_iterator_free(iter); git_diff_list_free(diff); /* try again with low file size threshold */ - file_count = 0; - binary_count = 0; - hunk_count = 0; + file_count = binary_count = hunk_count = 0; opts.context_lines = 3; opts.interhunk_lines = 1; @@ -166,36 +174,169 @@ void test_diff_diffiter__max_size_threshold(void) opts.max_size = 50; /* treat anything over 50 bytes as binary! */ cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); - cl_assert(delta); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); file_count++; - - hunk_count += git_diff_iterator_num_hunks_in_file(iter); + hunk_count += git_diff_patch_num_hunks(patch); assert(delta->binary == 0 || delta->binary == 1); - binary_count += delta->binary; + + git_diff_patch_free(patch); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); - cl_assert_equal_i(13, file_count); - /* Three files are over the 50 byte threshold: * - staged_changes_file_deleted * - staged_changes_modified_file * - staged_new_file_modified_file */ cl_assert_equal_i(3, binary_count); - cl_assert_equal_i(5, hunk_count); - git_diff_iterator_free(iter); + git_diff_list_free(diff); +} + + +void test_diff_diffiter__iterate_all(void) +{ + git_repository *repo = cl_git_sandbox_init("status"); + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp = {0}; + size_t d, num_d; + + opts.context_lines = 3; + opts.interhunk_lines = 1; + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + + cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); + + num_d = git_diff_num_deltas(diff); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + size_t h, num_h; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); + cl_assert(patch && delta); + exp.files++; + + num_h = git_diff_patch_num_hunks(patch); + for (h = 0; h < num_h; h++) { + git_diff_range *range; + const char *header; + size_t header_len, l, num_l; + + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); + cl_assert(range && header); + exp.hunks++; + + for (l = 0; l < num_l; ++l) { + char origin; + const char *content; + size_t content_len; + + cl_git_pass(git_diff_patch_get_line_in_hunk( + &origin, &content, &content_len, NULL, NULL, patch, h, l)); + cl_assert(content); + exp.lines++; + } + } + + git_diff_patch_free(patch); + } + + cl_assert_equal_i(13, exp.files); + cl_assert_equal_i(8, exp.hunks); + cl_assert_equal_i(13, exp.lines); + + git_diff_list_free(diff); +} + +static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp) +{ + size_t h, num_h = git_diff_patch_num_hunks(patch); + + exp->files++; + exp->hunks += num_h; + + /* let's iterate in reverse, just because we can! */ + for (h = 1; h <= num_h; ++h) + exp->lines += git_diff_patch_num_lines_in_hunk(patch, num_h - h); +} + +#define PATCH_CACHE 5 + +void test_diff_diffiter__iterate_randomly_while_saving_state(void) +{ + git_repository *repo = cl_git_sandbox_init("status"); + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp = {0}; + git_diff_patch *patches[PATCH_CACHE]; + size_t p, d, num_d; + + memset(patches, 0, sizeof(patches)); + + opts.context_lines = 3; + opts.interhunk_lines = 1; + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + + cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); + + num_d = git_diff_num_deltas(diff); + + /* To make sure that references counts work for diff and patch objects, + * this generates patches and randomly caches them. Only when the patch + * is removed from the cache are hunks and lines counted. At the end, + * there are still patches in the cache, so free the diff and try to + * process remaining patches after the diff is freed. + */ + + srand(121212); + p = rand() % PATCH_CACHE; + + for (d = 0; d < num_d; ++d) { + /* take old patch */ + git_diff_patch *patch = patches[p]; + patches[p] = NULL; + + /* cache new patch */ + cl_git_pass(git_diff_get_patch(&patches[p], NULL, diff, d)); + cl_assert(patches[p] != NULL); + + /* process old patch if non-NULL */ + if (patch != NULL) { + iterate_over_patch(patch, &exp); + git_diff_patch_free(patch); + } + + p = rand() % PATCH_CACHE; + } + + /* free diff list now - refcounts should keep things safe */ git_diff_list_free(diff); + /* process remaining unprocessed patches */ + for (p = 0; p < PATCH_CACHE; p++) { + git_diff_patch *patch = patches[p]; + + if (patch != NULL) { + iterate_over_patch(patch, &exp); + git_diff_patch_free(patch); + } + } + + /* hopefully it all still added up right */ + cl_assert_equal_i(13, exp.files); + cl_assert_equal_i(8, exp.hunks); + cl_assert_equal_i(13, exp.lines); } diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index f5e72cadc..78ab093ed 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -264,10 +264,12 @@ void test_diff_tree__larger_hunks(void) git_tree *a, *b; git_diff_options opts = {0}; git_diff_list *diff = NULL; - git_diff_iterator *iter = NULL; + size_t d, num_d, h, num_h, l, num_l, header_len, line_len; git_diff_delta *delta; - diff_expects exp; - int error, num_files = 0; + git_diff_patch *patch; + git_diff_range *range; + const char *header, *line; + char origin; g_repo = cl_git_sandbox_init("diff"); @@ -277,61 +279,38 @@ void test_diff_tree__larger_hunks(void) opts.context_lines = 1; opts.interhunk_lines = 0; - memset(&exp, 0, sizeof(exp)); - cl_git_pass(git_diff_tree_to_tree(g_repo, &opts, a, b, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - /* this should be exact */ - cl_assert(git_diff_iterator_progress(iter) == 0.0f); + num_d = git_diff_num_deltas(diff); + for (d = 0; d < num_d; ++d) { + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); + cl_assert(patch && delta); - /* You wouldn't actually structure an iterator loop this way, but - * I have here for testing purposes of the return value - */ - while (!(error = git_diff_iterator_next_file(&delta, iter))) { - git_diff_range *range; - const char *header; - size_t header_len; - int actual_hunks = 0, num_hunks; - float expected_progress; + num_h = git_diff_patch_num_hunks(patch); + for (h = 0; h < num_h; h++) { + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); - num_files++; - - expected_progress = (float)num_files / 2.0f; - cl_assert(expected_progress == git_diff_iterator_progress(iter)); - - num_hunks = git_diff_iterator_num_hunks_in_file(iter); - - while (!(error = git_diff_iterator_next_hunk( - &range, &header, &header_len, iter))) - { - int actual_lines = 0; - int num_lines = git_diff_iterator_num_lines_in_hunk(iter); - char origin; - const char *line; - size_t line_len; - - while (!(error = git_diff_iterator_next_line( - &origin, &line, &line_len, iter))) - { - actual_lines++; + for (l = 0; l < num_l; ++l) { + cl_git_pass(git_diff_patch_get_line_in_hunk( + &origin, &line, &line_len, NULL, NULL, patch, h, l)); + cl_assert(line); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(actual_lines, num_lines); - - actual_hunks++; + cl_git_fail(git_diff_patch_get_line_in_hunk( + &origin, &line, &line_len, NULL, NULL, patch, h, num_l)); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(actual_hunks, num_hunks); + cl_git_fail(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, num_h)); + + git_diff_patch_free(patch); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(2, num_files); - cl_assert(git_diff_iterator_progress(iter) == 1.0f); + cl_git_fail(git_diff_get_patch(&patch, &delta, diff, num_d)); + + cl_assert_equal_i(2, num_d); - git_diff_iterator_free(iter); git_diff_list_free(diff); diff = NULL; diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 40a888544..612e44303 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -678,7 +678,7 @@ void test_diff_workdir__larger_hunks(void) const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10"; git_tree *a, *b; git_diff_options opts = {0}; - int i, error; + size_t i, d, num_d, h, num_h, l, num_l, header_len, line_len; g_repo = cl_git_sandbox_init("diff"); @@ -690,9 +690,10 @@ void test_diff_workdir__larger_hunks(void) for (i = 0; i <= 2; ++i) { git_diff_list *diff = NULL; - git_diff_iterator *iter = NULL; - git_diff_delta *delta; - int num_files = 0; + git_diff_patch *patch; + git_diff_range *range; + const char *header, *line; + char origin; /* okay, this is a bit silly, but oh well */ switch (i) { @@ -707,54 +708,36 @@ void test_diff_workdir__larger_hunks(void) break; } - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); + cl_assert_equal_i(2, (int)num_d); - cl_assert(git_diff_iterator_progress(iter) == 0.0f); + for (d = 0; d < num_d; ++d) { + cl_git_pass(git_diff_get_patch(&patch, NULL, diff, d)); + cl_assert(patch); - while (!(error = git_diff_iterator_next_file(&delta, iter))) { - git_diff_range *range; - const char *header; - size_t header_len; - int actual_hunks = 0, num_hunks; - float expected_progress; + num_h = git_diff_patch_num_hunks(patch); + for (h = 0; h < num_h; h++) { + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); - num_files++; - - expected_progress = (float)num_files / 2.0f; - cl_assert(expected_progress == git_diff_iterator_progress(iter)); - - num_hunks = git_diff_iterator_num_hunks_in_file(iter); - - while (!(error = git_diff_iterator_next_hunk( - &range, &header, &header_len, iter))) - { - int actual_lines = 0; - int num_lines = git_diff_iterator_num_lines_in_hunk(iter); - char origin; - const char *line; - size_t line_len; - - while (!(error = git_diff_iterator_next_line( - &origin, &line, &line_len, iter))) - { - actual_lines++; + for (l = 0; l < num_l; ++l) { + cl_git_pass(git_diff_patch_get_line_in_hunk( + &origin, &line, &line_len, NULL, NULL, patch, h, l)); + cl_assert(line); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(actual_lines, num_lines); - - actual_hunks++; + /* confirm fail after the last item */ + cl_git_fail(git_diff_patch_get_line_in_hunk( + &origin, &line, &line_len, NULL, NULL, patch, h, num_l)); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(actual_hunks, num_hunks); + /* confirm fail after the last item */ + cl_git_fail(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, num_h)); + + git_diff_patch_free(patch); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert_equal_i(2, num_files); - cl_assert(git_diff_iterator_progress(iter) == 1.0f); - - git_diff_iterator_free(iter); git_diff_list_free(diff); } From 642863086575a61b3ed0bbbe909f4f07d87ff9db Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 25 Sep 2012 10:48:50 -0700 Subject: [PATCH 3/4] Fix bugs in new diff patch code This fixes all the bugs in the new diff patch code. The only really interesting one is that when we merge two diffs, we now have to actually exclude diff delta records that are not supposed to be tracked, as opposed to before where they could be included because they would be skipped silently by `git_diff_foreach()`. Other than that, there are just minor errors. --- include/git2/diff.h | 20 ++++++-- src/diff.c | 30 ++++++++++++ src/diff.h | 3 ++ src/diff_output.c | 87 ++++++++++++++++------------------ tests-clar/diff/diff_helpers.c | 8 +++- tests-clar/diff/diffiter.c | 12 +++-- 6 files changed, 105 insertions(+), 55 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index d216c1303..154264546 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -400,6 +400,20 @@ GIT_EXTERN(int) git_diff_print_compact( void *cb_data, git_diff_data_fn print_cb); +/** + * Look up the single character abbreviation for a delta status code. + * + * When you call `git_diff_print_compact` it prints single letter codes into + * the output such as 'A' for added, 'D' for deleted, 'M' for modified, etc. + * It is sometimes convenient to convert a git_delta_t value into these + * letters for your own purposes. This function does just that. By the + * way, unmodified will return a space (i.e. ' '). + * + * @param delta_t The git_delta_t value to look up + * @return The single character label for that code + */ +GIT_EXTERN(char) git_diff_status_char(git_delta_t status); + /** * Iterate over a diff generating text output like "git diff". * @@ -453,9 +467,9 @@ GIT_EXTERN(size_t) git_diff_num_deltas_of_type( * done with it. You can use the patch object to loop over all the hunks * and lines in the diff of the one delta. * - * For a binary file, no `git_diff_patch` will be created, the output will - * be set to NULL, and the `binary` flag will be set true in the - * `git_diff_delta` structure. + * For an unchanged file or a binary file, no `git_diff_patch` will be + * created, the output will be set to NULL, and the `binary` flag will be + * set true in the `git_diff_delta` structure. * * The `git_diff_delta` pointer points to internal data and you do not have * to release it when you are done with it. It will go away when the diff --git a/src/diff.c b/src/diff.c index 7c8e2a9bb..7ee919b5a 100644 --- a/src/diff.c +++ b/src/diff.c @@ -807,6 +807,28 @@ on_error: return -1; } + +bool git_diff_delta__should_skip( + git_diff_options *opts, git_diff_delta *delta) +{ + uint32_t flags = opts ? opts->flags : 0; + + if (delta->status == GIT_DELTA_UNMODIFIED && + (flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0) + return true; + + if (delta->status == GIT_DELTA_IGNORED && + (flags & GIT_DIFF_INCLUDE_IGNORED) == 0) + return true; + + if (delta->status == GIT_DELTA_UNTRACKED && + (flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) + return true; + + return false; +} + + int git_diff_merge( git_diff_list *onto, const git_diff_list *from) @@ -843,6 +865,14 @@ int git_diff_merge( j++; } + /* the ignore rules for the target may not match the source + * or the result of a merged delta could be skippable... + */ + if (git_diff_delta__should_skip(&onto->opts, delta)) { + git__free(delta); + continue; + } + if ((error = !delta ? -1 : git_vector_insert(&onto_new, delta)) < 0) break; } diff --git a/src/diff.h b/src/diff.h index 862c33c1b..64fe00928 100644 --- a/src/diff.h +++ b/src/diff.h @@ -50,5 +50,8 @@ extern void git_diff__cleanup_modes( extern void git_diff_list_addref(git_diff_list *diff); +extern bool git_diff_delta__should_skip( + git_diff_options *opts, git_diff_delta *delta); + #endif diff --git a/src/diff_output.c b/src/diff_output.c index b84b0c2a4..141859742 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -51,26 +51,6 @@ static int parse_hunk_header(git_diff_range *range, const char *header) return 0; } -static bool diff_delta_should_skip( - diff_context *ctxt, git_diff_delta *delta) -{ - uint32_t flags = ctxt->opts ? ctxt->opts->flags : 0; - - if (delta->status == GIT_DELTA_UNMODIFIED && - (flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0) - return true; - - if (delta->status == GIT_DELTA_IGNORED && - (flags & GIT_DIFF_INCLUDE_IGNORED) == 0) - return true; - - if (delta->status == GIT_DELTA_UNTRACKED && - (flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) - return true; - - return false; -} - #define KNOWN_BINARY_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY) #define NOT_BINARY_FLAGS (GIT_DIFF_FILE_NOT_BINARY|GIT_DIFF_FILE_NO_DATA) @@ -411,7 +391,7 @@ static void diff_context_init( setup_xdiff_options(ctxt->opts, &ctxt->xdiff_config, &ctxt->xdiff_params); } -static bool diff_delta_file_callback( +static int diff_delta_file_callback( diff_context *ctxt, git_diff_delta *delta, size_t idx) { float progress; @@ -419,18 +399,18 @@ static bool diff_delta_file_callback( if (!ctxt->file_cb) return 0; - progress = (float)idx / ctxt->diff->deltas.length; + progress = ctxt->diff ? ((float)idx / ctxt->diff->deltas.length) : 1.0f; if (ctxt->file_cb(ctxt->cb_data, delta, progress) != 0) - return GIT_EUSER; + ctxt->cb_error = GIT_EUSER; - return 0; + return ctxt->cb_error; } static void diff_patch_init( diff_context *ctxt, git_diff_patch *patch) { - memset(patch, 0, sizeof(*patch)); + memset(patch, 0, sizeof(git_diff_patch)); patch->diff = ctxt->diff; patch->ctxt = ctxt; @@ -454,7 +434,7 @@ static git_diff_patch *diff_patch_alloc( git_diff_list_addref(patch->diff); - GIT_REFCOUNT_INC(&patch); + GIT_REFCOUNT_INC(patch); patch->delta = delta; patch->flags = GIT_DIFF_PATCH_ALLOCATED; @@ -836,6 +816,8 @@ static int diff_patch_line_cb( } } + hunk->line_count++; + return 0; } @@ -847,7 +829,7 @@ int git_diff_foreach( git_diff_hunk_fn hunk_cb, git_diff_data_fn data_cb) { - int error; + int error = 0; diff_context ctxt; size_t idx; git_diff_patch patch; @@ -861,14 +843,20 @@ int git_diff_foreach( git_vector_foreach(&diff->deltas, idx, patch.delta) { /* check flags against patch status */ - if (diff_delta_should_skip(&ctxt, patch.delta)) + if (git_diff_delta__should_skip(ctxt.opts, patch.delta)) continue; - if (!(error = diff_patch_load(&ctxt, &patch)) && - !(error = diff_delta_file_callback(&ctxt, patch.delta, idx))) - error = diff_patch_generate(&ctxt, &patch); + if (!(error = diff_patch_load(&ctxt, &patch))) { - diff_patch_unload(&patch); + /* invoke file callback */ + error = diff_delta_file_callback(&ctxt, patch.delta, idx); + + /* generate diffs and invoke hunk and line callbacks */ + if (!error) + error = diff_patch_generate(&ctxt, &patch); + + diff_patch_unload(&patch); + } if (error < 0) break; @@ -899,25 +887,32 @@ static char pick_suffix(int mode) return ' '; } +char git_diff_status_char(git_delta_t status) +{ + char code; + + switch (status) { + case GIT_DELTA_ADDED: code = 'A'; break; + case GIT_DELTA_DELETED: code = 'D'; break; + case GIT_DELTA_MODIFIED: code = 'M'; break; + case GIT_DELTA_RENAMED: code = 'R'; break; + case GIT_DELTA_COPIED: code = 'C'; break; + case GIT_DELTA_IGNORED: code = 'I'; break; + case GIT_DELTA_UNTRACKED: code = '?'; break; + default: code = ' '; break; + } + + return code; +} + static int print_compact(void *data, git_diff_delta *delta, float progress) { diff_print_info *pi = data; - char code, old_suffix, new_suffix; + char old_suffix, new_suffix, code = git_diff_status_char(delta->status); GIT_UNUSED(progress); - switch (delta->status) { - case GIT_DELTA_ADDED: code = 'A'; break; - case GIT_DELTA_DELETED: code = 'D'; break; - case GIT_DELTA_MODIFIED: code = 'M'; break; - case GIT_DELTA_RENAMED: code = 'R'; break; - case GIT_DELTA_COPIED: code = 'C'; break; - case GIT_DELTA_IGNORED: code = 'I'; break; - case GIT_DELTA_UNTRACKED: code = '?'; break; - default: code = 0; - } - - if (!code) + if (code == ' ') return 0; old_suffix = pick_suffix(delta->old_file.mode); @@ -1288,7 +1283,7 @@ int git_diff_get_patch( &ctxt, diff, diff->repo, &diff->opts, NULL, NULL, diff_patch_hunk_cb, diff_patch_line_cb); - if (diff_delta_should_skip(&ctxt, delta)) + if (git_diff_delta__should_skip(ctxt.opts, delta)) return 0; patch = diff_patch_alloc(&ctxt, delta); diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index 1c0435975..0c4721897 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -120,7 +120,7 @@ int diff_foreach_via_iterator( size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); - cl_assert(delta && patch); + cl_assert(delta); /* call file_cb for this file */ if (file_cb != NULL && file_cb(data, delta, (float)d / num_d) != 0) { @@ -128,6 +128,12 @@ int diff_foreach_via_iterator( goto abort; } + /* if there are no changes, then the patch will be NULL */ + if (!patch) { + cl_assert(delta->status == GIT_DELTA_UNMODIFIED || delta->binary == 1); + continue; + } + if (!hunk_cb && !line_cb) { git_diff_patch_free(patch); continue; diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 9e33d91e1..4273b16dd 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -256,21 +256,23 @@ void test_diff_diffiter__iterate_all(void) cl_assert_equal_i(13, exp.files); cl_assert_equal_i(8, exp.hunks); - cl_assert_equal_i(13, exp.lines); + cl_assert_equal_i(14, exp.lines); git_diff_list_free(diff); } static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp) { - size_t h, num_h = git_diff_patch_num_hunks(patch); + size_t h, num_h = git_diff_patch_num_hunks(patch), num_l; exp->files++; exp->hunks += num_h; /* let's iterate in reverse, just because we can! */ - for (h = 1; h <= num_h; ++h) - exp->lines += git_diff_patch_num_lines_in_hunk(patch, num_h - h); + for (h = 1, num_l = 0; h <= num_h; ++h) + num_l += git_diff_patch_num_lines_in_hunk(patch, num_h - h); + + exp->lines += num_l; } #define PATCH_CACHE 5 @@ -338,5 +340,5 @@ void test_diff_diffiter__iterate_randomly_while_saving_state(void) /* hopefully it all still added up right */ cl_assert_equal_i(13, exp.files); cl_assert_equal_i(8, exp.hunks); - cl_assert_equal_i(13, exp.lines); + cl_assert_equal_i(14, exp.lines); } From bae957b95d59a840df72a725b06f00635471cfd8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 25 Sep 2012 16:31:46 -0700 Subject: [PATCH 4/4] Add const to all shared pointers in diff API There are a lot of places where the diff API gives the user access to internal data structures and many of these were being exposed through non-const pointers. This replaces them all with const pointers for any object that the user can access but is still owned internally to the git_diff_list or git_diff_patch objects. This will probably break some bindings... Sorry! --- include/git2/diff.h | 18 +++++++++--------- src/checkout.c | 4 ++-- src/diff.c | 2 +- src/diff.h | 2 +- src/diff_output.c | 34 ++++++++++++++++++---------------- src/diff_output.h | 2 +- tests-clar/diff/diff_helpers.c | 14 +++++++------- tests-clar/diff/diff_helpers.h | 10 +++++----- tests-clar/diff/diffiter.c | 18 +++++++++--------- tests-clar/diff/index.c | 2 +- tests-clar/diff/patch.c | 4 ++-- tests-clar/diff/tree.c | 4 ++-- tests-clar/diff/workdir.c | 2 +- 13 files changed, 59 insertions(+), 57 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index 154264546..1c41a54ba 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -122,7 +122,7 @@ typedef enum { */ typedef struct { git_oid oid; - char *path; + const char *path; git_off_t size; unsigned int flags; uint16_t mode; @@ -154,7 +154,7 @@ typedef struct { */ typedef int (*git_diff_file_fn)( void *cb_data, - git_diff_delta *delta, + const git_diff_delta *delta, float progress); /** @@ -172,8 +172,8 @@ typedef struct { */ typedef int (*git_diff_hunk_fn)( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, const char *header, size_t header_len); @@ -213,8 +213,8 @@ enum { */ typedef int (*git_diff_data_fn)( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, /**< GIT_DIFF_LINE_... value from above */ const char *content, size_t content_len); @@ -486,7 +486,7 @@ GIT_EXTERN(size_t) git_diff_num_deltas_of_type( */ GIT_EXTERN(int) git_diff_get_patch( git_diff_patch **patch, - git_diff_delta **delta, + const git_diff_delta **delta, git_diff_list *diff, size_t idx); @@ -525,7 +525,7 @@ GIT_EXTERN(size_t) git_diff_patch_num_hunks( * @return 0 on success, GIT_ENOTFOUND if hunk_idx out of range, <0 on error */ GIT_EXTERN(int) git_diff_patch_get_hunk( - git_diff_range **range, + const git_diff_range **range, const char **header, size_t *header_len, size_t *lines_in_hunk, @@ -595,7 +595,7 @@ GIT_EXTERN(int) git_diff_patch_get_line_in_hunk( GIT_EXTERN(int) git_diff_blobs( git_blob *old_blob, git_blob *new_blob, - git_diff_options *options, + const git_diff_options *options, void *cb_data, git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, diff --git a/src/checkout.c b/src/checkout.c index 7cf9fe033..e429d2876 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -126,7 +126,7 @@ static int blob_content_to_link(git_blob *blob, const char *path, bool can_symli static int checkout_blob( git_repository *repo, - git_oid *blob_oid, + const git_oid *blob_oid, const char *path, mode_t filemode, bool can_symlink, @@ -150,7 +150,7 @@ static int checkout_blob( static int checkout_diff_fn( void *cb_data, - git_diff_delta *delta, + const git_diff_delta *delta, float progress) { struct checkout_diff_data *data; diff --git a/src/diff.c b/src/diff.c index 7ee919b5a..4dedbe831 100644 --- a/src/diff.c +++ b/src/diff.c @@ -809,7 +809,7 @@ on_error: bool git_diff_delta__should_skip( - git_diff_options *opts, git_diff_delta *delta) + const git_diff_options *opts, const git_diff_delta *delta) { uint32_t flags = opts ? opts->flags : 0; diff --git a/src/diff.h b/src/diff.h index 64fe00928..15745b2f3 100644 --- a/src/diff.h +++ b/src/diff.h @@ -51,7 +51,7 @@ extern void git_diff__cleanup_modes( extern void git_diff_list_addref(git_diff_list *diff); extern bool git_diff_delta__should_skip( - git_diff_options *opts, git_diff_delta *delta); + const git_diff_options *opts, const git_diff_delta *delta); #endif diff --git a/src/diff_output.c b/src/diff_output.c index 141859742..05bca4668 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -177,7 +177,7 @@ static int diff_delta_is_binary_by_size( } static void setup_xdiff_options( - git_diff_options *opts, xdemitconf_t *cfg, xpparam_t *param) + const git_diff_options *opts, xdemitconf_t *cfg, xpparam_t *param) { memset(cfg, 0, sizeof(xdemitconf_t)); memset(param, 0, sizeof(xpparam_t)); @@ -371,7 +371,7 @@ static void diff_context_init( diff_context *ctxt, git_diff_list *diff, git_repository *repo, - git_diff_options *opts, + const git_diff_options *opts, void *data, git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, @@ -696,8 +696,8 @@ static void diff_patch_free(git_diff_patch *patch) static int diff_patch_hunk_cb( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, const char *header, size_t header_len) { @@ -743,8 +743,8 @@ static int diff_patch_hunk_cb( static int diff_patch_line_cb( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, const char *content, size_t content_len) @@ -905,7 +905,8 @@ char git_diff_status_char(git_delta_t status) return code; } -static int print_compact(void *data, git_diff_delta *delta, float progress) +static int print_compact( + void *data, const git_diff_delta *delta, float progress) { diff_print_info *pi = data; char old_suffix, new_suffix, code = git_diff_status_char(delta->status); @@ -967,7 +968,7 @@ int git_diff_print_compact( return error; } -static int print_oid_range(diff_print_info *pi, git_diff_delta *delta) +static int print_oid_range(diff_print_info *pi, const git_diff_delta *delta) { char start_oid[8], end_oid[8]; @@ -997,7 +998,8 @@ static int print_oid_range(diff_print_info *pi, git_diff_delta *delta) return 0; } -static int print_patch_file(void *data, git_diff_delta *delta, float progress) +static int print_patch_file( + void *data, const git_diff_delta *delta, float progress) { diff_print_info *pi = data; const char *oldpfx = pi->diff->opts.old_prefix; @@ -1064,8 +1066,8 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) static int print_patch_hunk( void *data, - git_diff_delta *d, - git_diff_range *r, + const git_diff_delta *d, + const git_diff_range *r, const char *header, size_t header_len) { @@ -1087,8 +1089,8 @@ static int print_patch_hunk( static int print_patch_line( void *data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, /* GIT_DIFF_LINE value from above */ const char *content, size_t content_len) @@ -1158,7 +1160,7 @@ static void set_data_from_blob( int git_diff_blobs( git_blob *old_blob, git_blob *new_blob, - git_diff_options *options, + const git_diff_options *options, void *cb_data, git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, @@ -1253,7 +1255,7 @@ size_t git_diff_num_deltas_of_type(git_diff_list *diff, git_delta_t type) int git_diff_get_patch( git_diff_patch **patch_ptr, - git_diff_delta **delta_ptr, + const git_diff_delta **delta_ptr, git_diff_list *diff, size_t idx) { @@ -1326,7 +1328,7 @@ size_t git_diff_patch_num_hunks(git_diff_patch *patch) } int git_diff_patch_get_hunk( - git_diff_range **range, + const git_diff_range **range, const char **header, size_t *header_len, size_t *lines_in_hunk, diff --git a/src/diff_output.h b/src/diff_output.h index f54639088..5fed1d998 100644 --- a/src/diff_output.h +++ b/src/diff_output.h @@ -26,7 +26,7 @@ enum { typedef struct { git_repository *repo; git_diff_list *diff; - git_diff_options *opts; + const git_diff_options *opts; git_diff_file_fn file_cb; git_diff_hunk_fn hunk_cb; git_diff_data_fn data_cb; diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index 0c4721897..b4c68769e 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -23,7 +23,7 @@ git_tree *resolve_commit_oid_to_tree( int diff_file_fn( void *cb_data, - git_diff_delta *delta, + const git_diff_delta *delta, float progress) { diff_expects *e = cb_data; @@ -48,8 +48,8 @@ int diff_file_fn( int diff_hunk_fn( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, const char *header, size_t header_len) { @@ -67,8 +67,8 @@ int diff_hunk_fn( int diff_line_fn( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, const char *content, size_t content_len) @@ -116,7 +116,7 @@ int diff_foreach_via_iterator( for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -142,7 +142,7 @@ int diff_foreach_via_iterator( num_h = git_diff_patch_num_hunks(patch); for (h = 0; h < num_h; h++) { - git_diff_range *range; + const git_diff_range *range; const char *hdr; size_t hdr_len, l, num_l; diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index 79e140921..7984294a2 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -27,20 +27,20 @@ typedef struct { extern int diff_file_fn( void *cb_data, - git_diff_delta *delta, + const git_diff_delta *delta, float progress); extern int diff_hunk_fn( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, const char *header, size_t header_len); extern int diff_line_fn( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, const char *content, size_t content_len); diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 4273b16dd..78f97fc86 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -20,7 +20,7 @@ void test_diff_diffiter__create(void) num_d = git_diff_num_deltas(diff); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); } @@ -40,7 +40,7 @@ void test_diff_diffiter__iterate_files(void) cl_assert_equal_i(6, num_d); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; @@ -63,7 +63,7 @@ void test_diff_diffiter__iterate_files_2(void) cl_assert_equal_i(8, num_d); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; @@ -91,7 +91,7 @@ void test_diff_diffiter__iterate_files_and_hunks(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -104,7 +104,7 @@ void test_diff_diffiter__iterate_files_and_hunks(void) num_h = git_diff_patch_num_hunks(patch); for (h = 0; h < num_h; h++) { - git_diff_range *range; + const git_diff_range *range; const char *header; size_t header_len, num_l; @@ -143,7 +143,7 @@ void test_diff_diffiter__max_size_threshold(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); cl_assert(delta); @@ -178,7 +178,7 @@ void test_diff_diffiter__max_size_threshold(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -221,7 +221,7 @@ void test_diff_diffiter__iterate_all(void) num_d = git_diff_num_deltas(diff); for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -230,7 +230,7 @@ void test_diff_diffiter__iterate_all(void) num_h = git_diff_patch_num_hunks(patch); for (h = 0; h < num_h; h++) { - git_diff_range *range; + const git_diff_range *range; const char *header; size_t header_len, l, num_l; diff --git a/tests-clar/diff/index.c b/tests-clar/diff/index.c index 2c6e89c4a..7c4bddb90 100644 --- a/tests-clar/diff/index.c +++ b/tests-clar/diff/index.c @@ -93,7 +93,7 @@ void test_diff_index__0(void) static int diff_stop_after_2_files( void *cb_data, - git_diff_delta *delta, + const git_diff_delta *delta, float progress) { diff_expects *e = cb_data; diff --git a/tests-clar/diff/patch.c b/tests-clar/diff/patch.c index 05e748667..e8468386c 100644 --- a/tests-clar/diff/patch.c +++ b/tests-clar/diff/patch.c @@ -23,8 +23,8 @@ void test_diff_patch__cleanup(void) static int check_removal_cb( void *cb_data, - git_diff_delta *delta, - git_diff_range *range, + const git_diff_delta *delta, + const git_diff_range *range, char line_origin, const char *formatted_output, size_t output_len) diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index 78ab093ed..e62fa2690 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -265,9 +265,9 @@ void test_diff_tree__larger_hunks(void) git_diff_options opts = {0}; git_diff_list *diff = NULL; size_t d, num_d, h, num_h, l, num_l, header_len, line_len; - git_diff_delta *delta; + const git_diff_delta *delta; git_diff_patch *patch; - git_diff_range *range; + const git_diff_range *range; const char *header, *line; char origin; diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 612e44303..916113178 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -691,7 +691,7 @@ void test_diff_workdir__larger_hunks(void) for (i = 0; i <= 2; ++i) { git_diff_list *diff = NULL; git_diff_patch *patch; - git_diff_range *range; + const git_diff_range *range; const char *header, *line; char origin;