summaryrefslogtreecommitdiff
path: root/diff.c
AgeCommit message (Collapse)AuthorFilesLines
2021-02-25Merge branch 'jc/diffcore-rotate'Libravatar Junio C Hamano1-0/+21
"git {diff,log} --{skip,rotate}-to=<path>" allows the user to discard diff output for early paths or move them to the end of the output. * jc/diffcore-rotate: diff: --{rotate,skip}-to=<path>
2021-02-16diff: --{rotate,skip}-to=<path>Libravatar Junio C Hamano1-0/+21
In the implementation of "git difftool", there is a case where the user wants to start viewing the diffs at a specific path and continue on to the rest, optionally wrapping around to the beginning. Since it is somewhat cumbersome to implement such a feature as a post-processing step of "git diff" output, let's support it internally with two new options. - "git diff --rotate-to=C", when the resulting patch would show paths A B C D E without the option, would "rotate" the paths to shows patch to C D E A B instead. It is an error when there is no patch for C is shown. - "git diff --skip-to=C" would instead "skip" the paths before C, and shows patch to C D E. Again, it is an error when there is no patch for C is shown. - "git log [-p]" also accepts these two options, but it is not an error if there is no change to the specified path. Instead, the set of output paths are rotated or skipped to the specified path or the first path that sorts after the specified path. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-11diff: plug memory leak from regcomp() on {log,diff} -ILibravatar Ævar Arnfjörð Bjarmason1-0/+12
Fix a memory leak in 296d4a94e7 (diff: add -I<regex> that ignores matching changes, 2020-10-20) by freeing the memory it allocates in the newly introduced diff_free(). See the previous commit for details on that. This memory leak was intentionally introduced in 296d4a94e7, see the discussion on a previous iteration of it in https://lore.kernel.org/git/xmqqeelycajx.fsf@gitster.c.googlers.com/ At that time freeing the memory was somewhat tedious, but since it isn't anymore with the newly introduced diff_free() let's use it. Let's retain the pattern for diff_free_file() and add a diff_free_ignore_regex(), even though (unlike "diff_free_file") we don't need to call it elsewhere. I think this'll make for more readable code than gradually accumulating a giant diff_free() function, sharing "int i" across unrelated code etc. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-11diff: add an API for deferred freeingLibravatar Ævar Arnfjörð Bjarmason1-4/+16
Add a diff_free() function to free anything we may have allocated in the "diff_options" struct, and the ability to make calling it a noop by setting "no_free" in "diff_options". This is required because when e.g. "git diff" is run we'll allocate things in that struct, use the diff machinery once, and then exit. But if we run e.g. "git log -p" we're going to re-use what we allocated across multiple diff_flush() calls, and only want to free things at the end. We've thus ended up with features like the recently added "diff -I"[1] where we'll leak memory. As it turns out it could have simply used the pattern established in 6ea57703f6 (log: prepare log/log-tree to reuse the diffopt.close_file attribute, 2016-06-22). Manually adding more such flags to things log_tree_commit() every time we need to allocate something would be tedious. Let's instead move that fclose() code it to a new diff_free(), in anticipation of freeing more things in that function in follow-up commits. Some functions such as log_tree_commit() need an idiom of optionally retaining a previous "no_free", as they may either free the memory themselves, or their caller may do so. I'm keeping that idiom in log_show_early() for good measure, even though I don't think it's currently called in this manner. It also gets passed an existing "struct rev_info", so future callers may want to set the "no_free" flag. This change is a bit hard to read because while the freeing pattern we're introducing isn't unusual, the "file" member is a special snowflake. We usually don't want to fclose() it. This is because "file" is usually stdout, in which case we don't want to fclose() it. We only want to opt-in to closing it when we e.g. open a file on the filesystem. Thus the opt-in "close_file" flag. So the API in general just needs a "no_free" flag to defer freeing, but the "file" member still needs its "close_file" flag. This is made more confusing because while refactoring this code we could replace some "close_file=0" with "no_free=1", whereas others need to set both flags. This is because there were some cases where an existing "close_file=0" meant "let's defer deallocation", and others where it meant "we don't want to close this file handle at all". 1. 296d4a94e7 (diff: add -I<regex> that ignores matching changes, 2020-10-20) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-25Merge branch 'sj/untracked-files-in-submodule-directory-is-not-dirty'Libravatar Junio C Hamano1-0/+3
"git diff" showed a submodule working tree with untracked cruft as "Submodule commit <objectname>-dirty", but a natural expectation is that the "-dirty" indicator would align with "git describe --dirty", which does not consider having untracked files in the working tree as source of dirtiness. The inconsistency has been fixed. * sj/untracked-files-in-submodule-directory-is-not-dirty: diff: do not show submodule with untracked files as "-dirty"
2020-12-18Merge branch 'jc/diff-I-status-fix'Libravatar Junio C Hamano1-1/+2
"git diff -I<pattern> -exit-code" should exit with 0 status when all the changes match the ignored pattern, but it didn't. * jc/diff-I-status-fix: diff: correct interaction between --exit-code and -I<pattern>
2020-12-16diff: correct interaction between --exit-code and -I<pattern>Libravatar Junio C Hamano1-1/+2
Just like "git diff -w --exit-code" should exit with 0 when ignoring whitespace differences results in no changes shown, if ignoring certain changes with "git diff -I<pattern> --exit-code" result in an empty patch, we should exit with 0. The test suite did not cover the interaction between "--exit-code" and "-w"; add one while adding a new test for "--exit-code" + "-I". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-08diff: do not show submodule with untracked files as "-dirty"Libravatar Sangeeta Jain1-0/+3
Git diff reports a submodule directory as -dirty even when there are only untracked files in the submodule directory. This is inconsistent with what `git describe --dirty` says when run in the submodule directory in that state. Make `--ignore-submodules=untracked` the default for `git diff` when there is no configuration variable or command line option, so that the command would not give '-dirty' suffix to a submodule whose working tree has untracked files, to make it consistent with `git describe --dirty` that is run in the submodule working tree. And also make `--ignore-submodules=none` the default for `git status` so that the user doesn't end up deleting a submodule that has uncommitted (untracked) files. Signed-off-by: Sangeeta Jain <sangunb09@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-21Merge branch 'en/strmap'Libravatar Junio C Hamano1-2/+2
A specialization of hashmap that uses a string as key has been introduced. Hopefully it will see wider use over time. * en/strmap: shortlog: use strset from strmap.h Use new HASHMAP_INIT macro to simplify hashmap initialization strmap: take advantage of FLEXPTR_ALLOC_STR when relevant strmap: enable allocations to come from a mem_pool strmap: add a strset sub-type strmap: split create_entry() out of strmap_put() strmap: add functions facilitating use as a string->int map strmap: enable faster clearing and reusing of strmaps strmap: add more utility functions strmap: new utility functions hashmap: provide deallocation function names hashmap: introduce a new hashmap_partial_clear() hashmap: allow re-use after hashmap_free() hashmap: adjust spacing to fix argument alignment hashmap: add usage documentation explaining hashmap_free[_entries]()
2020-11-21Merge branch 'jk/diff-release-filespec-fix'Libravatar Junio C Hamano1-0/+3
Running "git diff" while allowing external diff in a state with unmerged paths used to segfault, which has been corrected. * jk/diff-release-filespec-fix: t7800: simplify difftool test diff: allow passing NULL to diff_free_filespec_data()
2020-11-06diff: allow passing NULL to diff_free_filespec_data()Libravatar Jinoh Kang1-0/+3
Commit 3aef54e8b8 ("diff: munmap() file contents before running external diff") introduced calls to diff_free_filespec_data in run_external_diff, which may pass NULL pointers. Fix this and prevent any such bugs in the future by making `diff_free_filespec_data(NULL)` a no-op. Fixes: 3aef54e8b8 ("diff: munmap() file contents before running external diff") Signed-off-by: Jinoh Kang <luke1337@theori.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-02Merge branch 'mk/diff-ignore-regex'Libravatar Junio C Hamano1-0/+23
"git diff" family of commands learned the "-I<regex>" option to ignore hunks whose changed lines all match the given pattern. * mk/diff-ignore-regex: diff: add -I<regex> that ignores matching changes merge-base, xdiff: zero out xpparam_t structures
2020-11-02hashmap: provide deallocation function namesLibravatar Elijah Newren1-2/+2
hashmap_free(), hashmap_free_entries(), and hashmap_free_() have existed for a while, but aren't necessarily the clearest names, especially with hashmap_partial_clear() being added to the mix and lazy-initialization now being supported. Peff suggested we adopt the following names[1]: - hashmap_clear() - remove all entries and de-allocate any hashmap-specific data, but be ready for reuse - hashmap_clear_and_free() - ditto, but free the entries themselves - hashmap_partial_clear() - remove all entries but don't deallocate table - hashmap_partial_clear_and_free() - ditto, but free the entries This patch provides the new names and converts all existing callers over to the new naming scheme. [1] https://lore.kernel.org/git/20201030125059.GA3277724@coredump.intra.peff.net/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20diff: add -I<regex> that ignores matching changesLibravatar Michał Kępień1-0/+23
Add a new diff option that enables ignoring changes whose all lines (changed, removed, and added) match a given regular expression. This is similar to the -I/--ignore-matching-lines option in standalone diff utilities and can be used e.g. to ignore changes which only affect code comments or to look for unrelated changes in commits containing a large number of automatically applied modifications (e.g. a tree-wide string replacement). The difference between -G/-S and the new -I option is that the latter filters output on a per-change basis. Use the 'ignore' field of xdchange_t for marking a change as ignored or not. Since the same field is used by --ignore-blank-lines, identical hunk emitting rules apply for --ignore-blank-lines and -I. These two options can also be used together in the same git invocation (they are complementary to each other). Rename xdl_mark_ignorable() to xdl_mark_ignorable_lines(), to indicate that it is logically a "sibling" of xdl_mark_ignorable_regex() rather than its "parent". Signed-off-by: Michał Kępień <michal@isc.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-24diff: fix modified lines stats with --stat and --numstatLibravatar Thomas Guyot-Sionnest1-5/+7
Only skip diffstats when both oids are valid and identical. This check was causing both false-positives (files included in diffstats with no actual changes (0 lines modified) and false-negatives (showing 0 lines modified in stats when files had actually changed). Also replaced same_contents with may_differ to avoid confusion. Signed-off-by: Thomas Guyot-Sionnest <tguyot@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18Merge branch 'jc/quote-path-cleanup'Libravatar Junio C Hamano1-4/+4
"git status --short" quoted a path with SP in it when tracked, but not those that are untracked, ignored or unmerged. They are all shown quoted consistently. * jc/quote-path-cleanup: quote: turn 'nodq' parameter into a set of flags quote: rename misnamed sq_lookup[] to cq_lookup[] wt-status: consistently quote paths in "status --short" output quote_path: code clarification quote_path: optionally allow quoting a path with SP in it quote_path: give flags parameter to quote_path() quote_path: rename quote_path_relative() to quote_path()
2020-09-10quote: turn 'nodq' parameter into a set of flagsLibravatar Junio C Hamano1-4/+4
quote_c_style() and its friend quote_two_c_style() both take an optional "please omit the double quotes around the quoted body" parameter. Turn it into a flag word, assign one bit out of it, and call it CQUOTE_NODQ bit. No behaviour change intended. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-09Merge branch 'ss/submodule-summary-in-c'Libravatar Junio C Hamano1-1/+1
Yet another subcommand of "git submodule" is getting rewritten in C. * ss/submodule-summary-in-c: submodule: port submodule subcommand 'summary' from shell to C t7421: introduce a test script for verifying 'summary' output submodule: rename helper functions to avoid ambiguity submodule: remove extra line feeds between callback struct and macro
2020-09-03Merge branch 'mr/diff-hide-stat-wo-textual-change'Libravatar Junio C Hamano1-7/+31
"git diff --stat -w" showed 0-line changes for paths whose changes were only whitespaces, which was not intuitive. We now omit such paths from the stat output. * mr/diff-hide-stat-wo-textual-change: diff: teach --stat to ignore uninteresting modifications
2020-08-31Merge branch 'dd/diff-customize-index-line-abbrev'Libravatar Junio C Hamano1-1/+4
The output from the "diff" family of the commands had abbreviated object names of blobs involved in the patch, but its length was not affected by the --abbrev option. Now it is. * dd/diff-customize-index-line-abbrev: diff: index-line: respect --abbrev in object's name t4013: improve diff-post-processor logic
2020-08-24Merge branch 'rs/patch-id-with-incomplete-line'Libravatar Junio C Hamano1-0/+2
The patch-id computation did not ignore the "incomplete last line" marker like whitespaces. * rs/patch-id-with-incomplete-line: patch-id: ignore newline at end of file in diff_flush_patch_id()
2020-08-21diff: index-line: respect --abbrev in object's nameLibravatar Đoàn Trần Công Danh1-1/+4
A handful of Git's commands respect `--abbrev' for customizing length of abbreviation of object names. For diff-family, Git supports 2 different options for 2 different purposes, `--full-index' for showing diff-patch object's name in full, and `--abbrev' to customize the length of object names in diff-raw and diff-tree header lines, without any options to customise the length of object names in diff-patch format. When working with diff-patch format, we only have two options, either full index, or default abbrev length. Although, that behaviour is documented, it doesn't stop users from trying to use `--abbrev' with the hope of customising diff-patch's objects' name's abbreviation. Let's allow the blob object names shown on the "index" line to be abbreviated to arbitrary length given via the "--abbrev" option. To preserve backward compatibility with old script that specify both `--full-index' and `--abbrev', always show full object id if `--full-index' is specified. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-19diff: teach --stat to ignore uninteresting modificationsLibravatar Matthew Rogers1-7/+31
When options such as --ignore-space-change are in use, files with modifications can have no interesting textual changes worth showing. In such cases, "git diff --stat" shows 0 lines of additions and deletions. Teach "git diff --stat" not to show such a path in its output, which would be more natural. However, we don't want to prevent the display of all files that have 0 effective diffs since they could be the result of a rename, permission change, or other similar operation that may still be of interest so we special case additions and deletions as they are always interesting. Signed-off-by: Matthew Rogers <mattr94@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-18patch-id: ignore newline at end of file in diff_flush_patch_id()Libravatar René Scharfe1-0/+2
Whitespace is ignored when calculating patch IDs. This is done by removing all whitespace from diff lines before hashing them, including a newline at the end of a file. If that newline is missing, however, diff reports that fact in a separate line containing "\ No newline at end of file\n", and this marker is hashed like a context line. This goes against our goal of making patch IDs independent of whitespace. Use the same heuristic that 2485eab55cc (git-patch-id: do not trip over "no newline" markers, 2011-02-17) added to git patch-id instead and skip diff lines that start with a backslash and a space and are longer than twelve characters. Reported-by: Tilman Vogel <tilman.vogel@web.de> Initial-test-by: Tilman Vogel <tilman.vogel@web.de> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-12submodule: rename helper functions to avoid ambiguityLibravatar Shourya Shukla1-1/+1
The helper functions: show_submodule_summary(), prepare_submodule_summary() and print_submodule_summary() are used by the builtin_diff() function in diff.c to generate a summary of submodules in the context of a diff. Functions with similar names are to be introduced in the upcoming port of submodule's summary subcommand. So, rename the helper functions to '*_diff_submodule_summary()' to avoid ambiguity. Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30strvec: rename struct fieldsLibravatar Jeff King1-1/+1
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert more callers away from argv_array nameLibravatar Jeff King1-14/+14
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts remaining files from the first half of the alphabet, to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add '[abcdefghjkl]*'". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: rename files from argv-array to strvecLibravatar Jeff King1-1/+1
This requires updating #include lines across the code-base, but that's all fairly mechanical, and was done with: git ls-files '*.c' '*.h' | xargs perl -i -pe 's/argv-array.h/strvec.h/' Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-17Merge branch 'jk/diff-memuse-optim-with-stat-unmatch'Libravatar Junio C Hamano1-1/+4
Reduce memory usage during "diff --quiet" in a worktree with too many stat-unmatched paths. * jk/diff-memuse-optim-with-stat-unmatch: diff: discard blob data from stat-unmatched pairs
2020-06-02diff: discard blob data from stat-unmatched pairsLibravatar Jeff King1-1/+4
When performing a tree-level diff against the working tree, we may find that our index stat information is dirty, so we queue a filepair to be examined later. If the actual content hasn't changed, we call this a stat-unmatch; the stat information was out of date, but there's no actual diff. Normally diffcore_std() would detect and remove these identical filepairs via diffcore_skip_stat_unmatch(). However, when "--quiet" is used, we want to stop the diff as soon as we see any changes, so we check for stat-unmatches immediately in diff_change(). That check may require us to actually load the file contents into the pair of diff_filespecs. If we find that the pair isn't a stat-unmatch, then no big deal; we'd likely load the contents later anyway to generate a patch, do rename detection, etc, so we want to hold on to it. But if it is a stat-unmatch, then we have no more use for that data; the whole point is that we're going discard the pair. However, we never free the allocated diff_filespec data. In most cases, keeping that data isn't a problem. We don't expect a lot of stat-unmatch entries, and since we're using --quiet, we'd quit as soon as we saw such a real change anyway. However, there are extreme cases where it makes a big difference: 1. We'd generally mmap() the working tree half of the pair. And since the OS may limit the total number of maps, we can run afoul of this in large repositories. E.g.: $ cd linux $ git ls-files | wc -l 67959 $ sysctl vm.max_map_count vm.max_map_count = 65530 $ git ls-files | xargs touch ;# everything is stat-dirty! $ git diff --quiet fatal: mmap failed: Cannot allocate memory It should be unusual to have so many files stat-dirty, but it's possible if you've just run a script like "sed -i" or similar. After this patch, the above correctly exits with code 0. 2. Even if you don't hit mmap limits, the index half of the pair will have been pulled from the object database into heap memory. Again in a clone of linux.git, running: $ git ls-files | head -n 10000 | xargs touch $ git diff --quiet peaks at 145MB heap before this patch, and 94MB after. This patch solves the problem by freeing any diff_filespec data we picked up during the "--quiet" stat-unmatch check in diff_changes. Nobody is going to need that data later, so there's no point holding on to it. There are a few things to note: - we could skip queueing the pair entirely, which could in theory save a little work. But there's not much to save, as we need a diff_filepair to feed to diff_filespec_check_stat_unmatch() anyway. And since we cache the result of the stat-unmatch checks, a later call to diffcore_skip_stat_unmatch() call will quickly skip over them. The diffcore code also counts up the number of stat-unmatched pairs as it removes them. It's doubtful any callers would care about that in combination with --quiet, but we'd have to reimplement the logic here to be on the safe side. So it's not really worth the trouble. - I didn't write a test, because we always produce the correct output unless we run up against system mmap limits, which are both unportable and expensive to test against. Measuring peak heap would be interesting, but our perf suite isn't yet capable of that. - note that diff without "--quiet" does not suffer from the same problem. In diffcore_skip_stat_unmatch(), we detect the stat-unmatch entries and drop them immediately, so we're not carrying their data around. - you _can_ still trigger the mmap limit problem if you truly have that many files with actual changes. But it's rather unlikely. The stat-unmatch check avoids loading the file contents if the sizes don't match, so you'd need a pretty trivial change in every single file. Likewise, inexact rename detection might load the data for many files all at once. But you'd need not just 64k changes, but that many deletions and additions. The most likely candidate is perhaps break-detection, which would load the data for all pairs and keep it around for the content-level diff. But again, you'd need 64k actually changed files in the first place. So it's still possible to trigger this case, but it seems like "I accidentally made all my files stat-dirty" is the most likely case in the real world. Reported-by: Jan Christoph Uhde <Jan@UhdeJc.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-24diff: add config option relativeLibravatar Laurent Arnoud1-3/+8
The `diff.relative` boolean option set to `true` shows only changes in the current directory/value specified by the `path` argument of the `relative` option and shows pathnames relative to the aforementioned directory. Teach `--no-relative` to override earlier `--relative` Add for git-format-patch(1) options documentation `--relative` and `--no-relative` Signed-off-by: Laurent Arnoud <laurent@spkdev.net> Acked-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-28Merge branch 'jt/avoid-prefetch-when-able-in-diff'Libravatar Junio C Hamano1-50/+107
"git diff" in a partial clone learned to avoid lazy loading blob objects in more casese when they are not needed. * jt/avoid-prefetch-when-able-in-diff: diff: restrict when prefetching occurs diff: refactor object read diff: make diff_populate_filespec_options struct promisor-remote: accept 0 as oid_nr in function
2020-04-07diff: restrict when prefetching occursLibravatar Jonathan Tan1-22/+51
Commit 7fbbcb21b1 ("diff: batch fetching of missing blobs", 2019-04-08) optimized "diff" by prefetching blobs in a partial clone, but there are some cases wherein blobs do not need to be prefetched. In these cases, any command that uses the diff machinery will unnecessarily fetch blobs. diffcore_std() may read blobs when it calls the following functions: (1) diffcore_skip_stat_unmatch() (controlled by the config variable diff.autorefreshindex) (2) diffcore_break() and diffcore_merge_broken() (for break-rewrite detection) (3) diffcore_rename() (for rename detection) (4) diffcore_pickaxe() (for detecting addition/deletion of specified string) Instead of always prefetching blobs, teach diffcore_skip_stat_unmatch(), diffcore_break(), and diffcore_rename() to prefetch blobs upon the first read of a missing object. This covers (1), (2), and (3): to cover the rest, teach diffcore_std() to prefetch if the output type is one that includes blob data (and hence blob data will be required later anyway), or if it knows that (4) will be run. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07diff: refactor object readLibravatar Jonathan Tan1-8/+21
Refactor the object reads in diff_populate_filespec() to have the first object read not be in an if/else branch, because in a future patch, a retry will be added to that first object read. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07diff: make diff_populate_filespec_options structLibravatar Jonathan Tan1-19/+35
The behavior of diff_populate_filespec() currently can be customized through a bitflag, but a subsequent patch requires it to support a non-boolean option. Replace the bitflag with an options struct. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-02promisor-remote: accept 0 as oid_nr in functionLibravatar Jonathan Tan1-6/+5
There are 3 callers to promisor_remote_get_direct() that first check if the number of objects to be fetched is equal to 0. Fold that check into promisor_remote_get_direct(), and in doing so, be explicit as to what promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success, immediately). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16convert: provide additional metadata to filtersLibravatar brian m. carlson1-1/+4
Now that we have the codebase wired up to pass any additional metadata to filters, let's collect the additional metadata that we'd like to pass. The two main places we pass this metadata are checkouts and archives. In these two situations, reading HEAD isn't a valid option, since HEAD isn't updated for checkouts until after the working tree is written and archives can accept an arbitrary tree. In other situations, HEAD will usually reflect the refname of the branch in current use. We pass a smaller amount of data in other cases, such as git cat-file, where we can really only logically know about the blob. This commit updates only the parts of the checkout code where we don't use unpack_trees. That function and callers of it will be handled in a future commit. In the archive code, we leak a small amount of memory, since nothing we pass in the archiver argument structure is freed. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16convert: permit passing additional metadata to filter processesLibravatar brian m. carlson1-1/+1
There are a variety of situations where a filter process can make use of some additional metadata. For example, some people find the ident filter too limiting and would like to include the commit or the branch in their smudged files. This information isn't available during checkout as HEAD hasn't been updated at that point, and it wouldn't be available in archives either. Let's add a way to pass this metadata down to the filter. We pass the blob we're operating on, the treeish (preferring the commit over the tree if one exists), and the ref we're operating on. Note that we won't pass this information in all cases, such as when renormalizing or when we're performing diffs, since it doesn't make sense in those cases. The data we currently get from the filter process looks like the following: command=smudge pathname=git.c 0000 With this change, we'll get data more like this: command=smudge pathname=git.c refname=refs/tags/v2.25.1 treeish=c522f061d551c9bb8684a7c3859b2ece4499b56b blob=7be7ad34bd053884ec48923706e70c81719a8660 0000 There are a couple things to note about this approach. For operations like checkout, treeish will always be a commit, since we cannot check out individual trees, but for other operations, like archive, we can end up operating on only a particular tree, so we'll provide only a tree as the treeish. Similar comments apply for refname, since there are a variety of cases in which we won't have a ref. This commit wires up the code to print this information, but doesn't pass any of it at this point. In a future commit, we'll have various code paths pass the actual useful data down. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-14Merge branch 'mt/use-passed-repo-more-in-funcs'Libravatar Junio C Hamano1-1/+1
Some codepaths were given a repository instance as a parameter to work in the repository, but passed the_repository instance to its callees, which has been cleaned up (somewhat). * mt/use-passed-repo-more-in-funcs: sha1-file: allow check_object_signature() to handle any repo sha1-file: pass git_hash_algo to hash_object_file() sha1-file: pass git_hash_algo to write_object_file_prepare() streaming: allow open_istream() to handle any repo pack-check: use given repo's hash_algo at verify_packfile() cache-tree: use given repo's hash_algo at verify_one() diff: make diff_populate_filespec() honor its repo argument
2020-01-31diff: move diff.wsErrorHighlight to "basic" configLibravatar Jeff King1-8/+8
We parse diff.wsErrorHighlight in git_diff_ui_config(), meaning that it doesn't take effect for plumbing commands, only for porcelains like git-diff itself. This is mildly annoying as it means scripts like add--interactive, which produce a user-visible diff with color, don't respect the option. We could teach that script to parse the config and pass it along as --ws-error-highlight to the diff plumbing. But there's a simpler solution. It should be reasonably safe for plumbing to respect this option, as it only kicks in when color is otherwise enabled. And anybody parsing colorized output must already deal with the fact that color.diff.* may change the exact output they see; those options have been part of git_diff_basic_config() since its inception in 9a1805a872 (add a "basic" diff config callback, 2008-01-04). So we can just move it to the "basic" config, which fixes add--interactive, along with any other script in the same boat, with a very low risk of hurting any plumbing users. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-31diff: make diff_populate_filespec() honor its repo argumentLibravatar Matheus Tavares1-1/+1
diff_populate_filespec() takes a struct repository argument but it doesn't get passed down to read_object_file(). Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05Merge branch 'js/builtin-add-i'Libravatar Junio C Hamano1-23/+16
The beginning of rewriting "git add -i" in C. * js/builtin-add-i: built-in add -i: implement the `help` command built-in add -i: use color in the main loop built-in add -i: support `?` (prompt help) built-in add -i: show unique prefixes of the commands built-in add -i: implement the main loop built-in add -i: color the header in the `status` command built-in add -i: implement the `status` command diff: export diffstat interface Start to implement a built-in version of `git add --interactive`
2019-11-14diff: export diffstat interfaceLibravatar Daniel Ferreira1-23/+16
Make the diffstat interface (namely, the diffstat_t struct and compute_diffstat) no longer be internal to diff.c and allow it to be used by other parts of git. This is helpful for code that may want to easily extract information from files using the diff machinery, while flushing it differently from how the show_* functions used by diff_flush() do it. One example is the builtin implementation of git-add--interactive's status. Signed-off-by: Daniel Ferreira <bnmvco@gmail.com> Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Fix spelling errors in code commentsLibravatar Elijah Newren1-2/+2
Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-15Merge branch 'ew/hashmap'Libravatar Junio C Hamano1-13/+18
Code clean-up of the hashmap API, both users and implementation. * ew/hashmap: hashmap_entry: remove first member requirement from docs hashmap: remove type arg from hashmap_{get,put,remove}_entry OFFSETOF_VAR macro to simplify hashmap iterators hashmap: introduce hashmap_free_entries hashmap: hashmap_{put,remove} return hashmap_entry * hashmap: use *_entry APIs for iteration hashmap_cmp_fn takes hashmap_entry params hashmap_get{,_from_hash} return "struct hashmap_entry *" hashmap: use *_entry APIs to wrap container_of hashmap_get_next returns "struct hashmap_entry *" introduce container_of macro hashmap_put takes "struct hashmap_entry *" hashmap_remove takes "const struct hashmap_entry *" hashmap_get takes "const struct hashmap_entry *" hashmap_add takes "struct hashmap_entry *" hashmap_get_next takes "const struct hashmap_entry *" hashmap_entry_init takes "struct hashmap_entry *" packfile: use hashmap_entry in delta_base_cache_entry coccicheck: detect hashmap_entry.hash assignment diff: use hashmap_entry_init on moved_entry.ent
2019-10-11Merge branch 'bc/object-id-part17'Libravatar Junio C Hamano1-23/+23
Preparation for SHA-256 upgrade continues. * bc/object-id-part17: (26 commits) midx: switch to using the_hash_algo builtin/show-index: replace sha1_to_hex rerere: replace sha1_to_hex builtin/receive-pack: replace sha1_to_hex builtin/index-pack: replace sha1_to_hex packfile: replace sha1_to_hex wt-status: convert struct wt_status to object_id cache: remove null_sha1 builtin/worktree: switch null_sha1 to null_oid builtin/repack: write object IDs of the proper length pack-write: use hash_to_hex when writing checksums sequencer: convert to use the_hash_algo bisect: switch to using the_hash_algo sha1-lookup: switch hard-coded constants to the_hash_algo config: use the_hash_algo in abbrev comparison combine-diff: replace GIT_SHA1_HEXSZ with the_hash_algo bundle: switch to use the_hash_algo connected: switch GIT_SHA1_HEXSZ to the_hash_algo show-index: switch hard-coded constants to the_hash_algo blame: remove needless comparison with GIT_SHA1_HEXSZ ...
2019-10-07hashmap: remove type arg from hashmap_{get,put,remove}_entryLibravatar Eric Wong1-4/+2
Since these macros already take a `keyvar' pointer of a known type, we can rely on OFFSETOF_VAR to get the correct offset without relying on non-portable `__typeof__' and `offsetof'. Argument order is also rearranged, so `keyvar' and `member' are sequential as they are used as: `keyvar->member' Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07OFFSETOF_VAR macro to simplify hashmap iteratorsLibravatar Eric Wong1-3/+2
While we cannot rely on a `__typeof__' operator being portable to use with `offsetof'; we can calculate the pointer offset using an existing pointer and the address of a member using pointer arithmetic for compilers without `__typeof__'. This allows us to simplify usage of hashmap iterator macros by not having to specify a type when a pointer of that type is already given. In the future, list iterator macros (e.g. list_for_each_entry) may also be implemented using OFFSETOF_VAR to save hackers the trouble of using container_of/list_entry macros and without relying on non-portable `__typeof__'. v3: use `__typeof__' to avoid clang warnings Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: introduce hashmap_free_entriesLibravatar Eric Wong1-2/+4
`hashmap_free_entries' behaves like `container_of' and passes the offset of the hashmap_entry struct to the internal `hashmap_free_' function, allowing the function to free any struct pointer regardless of where the hashmap_entry field is located. `hashmap_free' no longer takes any arguments aside from the hashmap itself. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_cmp_fn takes hashmap_entry paramsLibravatar Eric Wong1-5/+7
Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>