summaryrefslogtreecommitdiff
path: root/builtin/log.c
AgeCommit message (Collapse)AuthorFilesLines
2021-12-01log: load decorations with --simplify-by-decorationLibravatar Jeff King1-2/+3
It's possible to specify --simplify-by-decoration but not --decorate. In this case we do respect the simplification, but we don't actually show any decorations. However, it works by lazy-loading the decorations when needed; this is discussed in more detail in 0cc7380d88 (log-tree: call load_ref_decorations() in get_name_decoration(), 2019-09-08). This works for basic cases, but will fail to respect any --decorate-refs option (or its variants). Those are handled only when cmd_log_init() loads the ref decorations up front, which is only when --decorate is specified explicitly (or as of the previous commit, when the userformat asks for %d or similar). We can solve this by making sure to load the decorations if we're going to simplify using them but they're not otherwise going to be displayed. The new test shows a simple case that fails without this patch. Note that we expect two commits in the output: the one we asked for by --decorate-refs, and the initial commit. The latter is just a quirk of how --simplify-by-decoration works. Arguably it may be a bug, but it's unrelated to this patch (which is just about the loading of the decorations; you get the same behavior before this patch with an explicit --decorate). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01log: handle --decorate-refs with userformat "%d"Libravatar Jeff King1-2/+16
In order to show ref decorations, we first have to load them. If you run: git log --decorate then git-log will recognize the option and load them up front via cmd_log_init(). Likewise if log.decorate is set. If you don't say --decorate explicitly, but do mention "%d" or "%D" in the output format, like so: git log --format=%d then this also works, because we lazy-load the ref decorations. This has been true since 3b3d443feb (add '%d' pretty format specifier to show decoration, 2008-09-04), though the lazy-load was later moved into log-tree.c. But there's one problem: that lazy-load just uses the defaults; it doesn't take into account any --decorate-refs options (or its exclude variant, or their config). So this does not work: git log --decorate-refs=whatever --format=%d It will decorate using all refs, not just the specified ones. This has been true since --decorate-refs was added in 65516f586b (log: add option to choose which refs to decorate, 2017-11-21). Adding further confusion is that it _may_ work because of the auto-decoration feature. If that's in use (and it often is, as it's the default), then if the output is going to stdout, we do enable decorations early (and so load them up front, respecting the extra options). But otherwise we do not. So: git log --decorate-refs=whatever --format=%d >some-file would typically behave differently than it does when the output goes to the pager or terminal! The solution is simple: we should recognize in cmd_log_init() that we're going to show decorations, and make sure we load them there. We already check userformat_find_requirements(), so we can couple this with our existing code there. There are two new tests. The first shows off the actual fix. The second makes sure that our fix doesn't cause us to stomp on an existing --decorate option (see the new comment in the code, as well). Reported-by: Josh Rampersad <josh.rampersad@voiceflow.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-05revision: separate walk and unsorted flagsLibravatar Patrick Steinhardt1-1/+1
The `--no-walk` flag supports two modes: either it sorts the revisions given as input input or it doesn't. This is reflected in a single `no_walk` flag, which reflects one of the three states "walk", "don't walk but without sorting" and "don't walk but with sorting". Split up the flag into two separate bits, one indicating whether we should walk or not and one indicating whether the input should be sorted or not. This will allow us to more easily introduce a new flag `--unsorted-input`, which only impacts the sorting bit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-28Merge branch 'jk/log-decorate-optim'Libravatar Junio C Hamano1-0/+3
Optimize "git log" for cases where we wasted cycles to load ref decoration data that may not be needed. * jk/log-decorate-optim: load_ref_decorations(): fix decoration with tags add_ref_decoration(): rename s/type/deco_type/ load_ref_decorations(): avoid parsing non-tag objects object.h: add lookup_object_by_type() function object.h: expand docstring for lookup_unknown_object() log: avoid loading decorations for userformats that don't need it pretty.h: update and expand docstring for userformat_find_requirements()
2021-06-28log: avoid loading decorations for userformats that don't need itLibravatar Jeff King1-0/+3
If no --decorate option is given, we default to auto-decoration. And when that kicks in, cmd_log_init_finish() will unconditionally load the decoration refs. However, if we are using a user-format that does not include "%d" or "%D", we won't show the decorations at all, so we don't need to load them. We can detect this case and auto-disable them by adding a new field to our userformat_want helper. We can do this even when the user explicitly asked for --decorate, because it can't affect the output at all. This patch consistently reduces the time to run "git log -1 --format=%H" on my git.git clone (with ~2k refs) from 34ms to 7ms. On a much more extreme real-world repository (with ~220k refs), it goes from 2.5s to 4ms. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-14*: fix typos which duplicate a wordLibravatar Andrei Rybak1-2/+1
Fix typos in documentation, code comments, and RelNotes which repeat various words. In trivial cases, just delete the duplicated word and rewrap text, if needed. Reword the affected sentence in Documentation/RelNotes/1.8.4.txt for it to make sense. Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-30Merge branch 'so/log-diff-merge'Libravatar Junio C Hamano1-0/+2
"git log" learned "--diff-merges=<style>" option, with an associated configuration variable log.diffMerges. * so/log-diff-merge: doc/diff-options: document new --diff-merges features diff-merges: introduce log.diffMerges config variable diff-merges: adapt -m to enable default diff format diff-merges: refactor set_diff_merges() diff-merges: introduce --diff-merges=on
2021-04-16diff-merges: introduce log.diffMerges config variableLibravatar Sergey Organov1-0/+2
New log.diffMerges configuration variable sets the format that --diff-merges=on will be using. The default is "separate". t4013: add the following tests for log.diffMerges config: * Test that wrong values are denied. * Test that the value of log.diffMerges properly affects both --diff-merges=on and -m. t9902: fix completion tests for log.d* to match log.diffMerges. Added documentation for log.diffMerges. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-02Merge branch 'zh/format-patch-fractional-reroll-count'Libravatar Junio C Hamano1-9/+16
"git format-patch -v<n>" learned to allow a reroll count that is not an integer. * zh/format-patch-fractional-reroll-count: format-patch: allow a non-integral version numbers
2021-03-23format-patch: allow a non-integral version numbersLibravatar ZheNing Hu1-9/+16
The `-v<n>` option of `format-patch` can give nothing but an integral iteration number to patches in a series.  Some people, however, prefer to mark a new iteration with only a small fixup with a non integral iteration number (e.g. an "oops, that was wrong" fix-up patch for v4 iteration may be labeled as "v4.1"). Allow `format-patch` to take such a non-integral iteration number. `<n>` can be any string, such as '3.1' or '4rev2'. In the case where it is a non-integral value, the "Range-diff" and "Interdiff" headers will not include the previous version. Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-20tree.h API: simplify read_tree_recursive() signatureLibravatar Ævar Arnfjörð Bjarmason1-4/+4
Simplify the signature of read_tree_recursive() to omit the "base", "baselen" and "stage" arguments. No callers of it use these parameters for anything anymore. The last function to call read_tree_recursive() with a non-"" path was read_tree_recursive() itself, but that was changed in ffd31f661d5 (Reimplement read_tree_recursive() using tree_entry_interesting(), 2011-03-25). The last user of the "stage" parameter went away in the last commit, and even that use was mere boilerplate. So let's remove those and rename the read_tree_recursive() function to just read_tree(). We had another read_tree() function that I've refactored away in preceding commits, since all in-tree users read trees recursively with a callback we can change the name to signify that this is the norm. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-22Merge branch 'ab/diff-deferred-free'Libravatar Junio C Hamano1-11/+12
A small memleak in "diff -I<regexp>" has been corrected. * ab/diff-deferred-free: diff: plug memory leak from regcomp() on {log,diff} -I diff: add an API for deferred freeing
2021-02-17Merge branch 'js/range-diff-one-side-only'Libravatar Junio C Hamano1-2/+8
The "git range-diff" command learned "--(left|right)-only" option to show only one side of the compared range. * js/range-diff-one-side-only: range-diff: offer --left-only/--right-only options range-diff: move the diffopt initialization down one layer range-diff: combine all options in a single data structure range-diff: simplify code spawning `git log` range-diff: libify the read_patches() function again range-diff: avoid leaking memory in two error code paths
2021-02-17Merge branch 'js/range-diff-wo-dotdot'Libravatar Junio C Hamano1-1/+1
There are other ways than ".." for a single token to denote a "commit range", namely "<rev>^!" and "<rev>^-<n>", but "git range-diff" did not understand them. * js/range-diff-wo-dotdot: range-diff(docs): explain how to specify commit ranges range-diff/format-patch: handle commit ranges other than A..B range-diff/format-patch: refactor check for commit range
2021-02-11diff: add an API for deferred freeingLibravatar Ævar Arnfjörð Bjarmason1-11/+12
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-02-06range-diff: combine all options in a single data structureLibravatar Johannes Schindelin1-2/+8
This will make it easier to implement the `--left-only` and `--right-only` options. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-05Merge branch 'so/log-diff-merge'Libravatar Junio C Hamano1-15/+7
"git log" learned a new "--diff-merges=<how>" option. * so/log-diff-merge: (32 commits) t4013: add tests for --diff-merges=first-parent doc/git-show: include --diff-merges description doc/rev-list-options: document --first-parent changes merges format doc/diff-generate-patch: mention new --diff-merges option doc/git-log: describe new --diff-merges options diff-merges: add '--diff-merges=1' as synonym for 'first-parent' diff-merges: add old mnemonic counterparts to --diff-merges diff-merges: let new options enable diff without -p diff-merges: do not imply -p for new options diff-merges: implement new values for --diff-merges diff-merges: make -m/-c/--cc explicitly mutually exclusive diff-merges: refactor opt settings into separate functions diff-merges: get rid of now empty diff_merges_init_revs() diff-merges: group diff-merge flags next to each other inside 'rev_info' diff-merges: split 'ignore_merges' field diff-merges: fix -m to properly override -c/--cc t4013: add tests for -m failing to override -c/--cc t4013: support test_expect_failure through ':failure' magic diff-merges: revise revs->diff flag handling diff-merges: handle imply -p on -c/--cc logic for log.c ...
2021-01-27range-diff/format-patch: refactor check for commit rangeLibravatar Johannes Schindelin1-1/+1
Currently, when called with exactly two arguments, `git range-diff` tests for a literal `..` in each of the two. Likewise, the argument provided via `--range-diff` to `git format-patch` is checked in the same manner. However, `<commit>^!` is a perfectly valid commit range, equivalent to `<commit>^..<commit>` according to the `SPECIFYING RANGES` section of gitrevisions[7]. In preparation for allowing more sophisticated ways to specify commit ranges, let's refactor the check into its own function. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-25Merge branch 'ab/mailmap'Libravatar Junio C Hamano1-1/+1
Clean-up docs, codepaths and tests around mailmap. * ab/mailmap: (22 commits) shortlog: remove unused(?) "repo-abbrev" feature mailmap doc + tests: document and test for case-insensitivity mailmap tests: add tests for empty "<>" syntax mailmap tests: add tests for whitespace syntax mailmap tests: add a test for comment syntax mailmap doc + tests: add better examples & test them tests: refactor a few tests to use "test_commit --append" test-lib functions: add an --append option to test_commit test-lib functions: add --author support to test_commit test-lib functions: document arguments to test_commit test-lib functions: expand "test_commit" comment template mailmap: test for silent exiting on missing file/blob mailmap tests: get rid of overly complex blame fuzzing mailmap tests: add a test for "not a blob" error mailmap tests: remove redundant entry in test mailmap tests: improve --stdin tests mailmap tests: modernize syntax & test idioms mailmap tests: use our preferred whitespace syntax mailmap doc: start by mentioning the comment syntax check-mailmap doc: note config options ...
2021-01-12shortlog: remove unused(?) "repo-abbrev" featureLibravatar Ævar Arnfjörð Bjarmason1-1/+1
Remove support for the magical "repo-abbrev" comment in .mailmap files. This was added to .mailmap parsing in [1], as a generalized feature of the git-shortlog Perl script added earlier in [2]. There was no documentation or tests for this feature, and I don't think it's used in practice anymore. What it did was to allow you to specify a single string to be search-replaced with "/.../" in the .mailmap file. E.g. for linux.git's current .mailmap: git archive --remote=git@gitlab.com:linux-kernel/linux.git \ HEAD -- .mailmap | grep -a repo-abbrev # repo-abbrev: /pub/scm/linux/kernel/git/ Then when running e.g.: git shortlog --merges --author=Linus -1 v5.10-rc7..v5.10 | grep Merge We'd emit (the [...] is mine): Merge tag [...]git://git.kernel.org/.../tip/tip But will now emit: Merge tag [...]git.kernel.org/pub/scm/linux/kernel/git/tip/tip I think at this point this is just a historical artifact we can get rid of. It was initially meant for Linus's own use when we integrated the Perl script[2], but since then it seems he's stopped using it. Digging through Linus's release announcements on the LKML[3] the last release I can find that made use of this output is Linux 2.6.25-rc6 back in March 2008[4]. Later on Linus started using --no-merges[5], and nowadays seems to prefer some custom not-quite-shortlog format of merges from lieutenants[6]. You will still see it on linux.git if you run "git shortlog" manually yourself with --merges, with this removed you can still get the same output with: git log --pretty=fuller v5.10-rc7..v5.10 | sed 's!/pub/scm/linux/kernel/git/!/.../!g' | git shortlog Arguably we should do the same for the search-replacing of "[PATCH]" at the beginning with "". That seems to be another relic of a bygone era when linux.git patches would have their E-Mail subject lines applied as-is by "git am" or whatever. But we documented that feature in "git-shortlog(1)", and it seems more widely applicable than something purely kernel-specific. 1. 7595e2ee6ef (git-shortlog: make common repository prefix configurable with .mailmap, 2006-11-25) 2. fa375c7f1b6 (Add git-shortlog perl script, 2005-06-04) 3. https://lore.kernel.org/lkml/ 4. https://lore.kernel.org/lkml/alpine.LFD.1.00.0803161651350.3020@woody.linux-foundation.org/ 5. https://lore.kernel.org/lkml/BANLkTinrbh7Xi27an3uY7pDWrNKhJRYmEA@mail.gmail.com/ 6. https://lore.kernel.org/lkml/CAHk-=wg1+kf1AVzXA-RQX0zjM6t9J2Kay9xyuNqcFHWV-y5ZYw@mail.gmail.com/ Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-06builtin/*: update usage formatLibravatar ZheNing Hu1-4/+4
According to the guidelines in parse-options.h, we should not end in a full stop or start with a capital letter. Fix old error and usage messages to match this expectation. Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21diff-merges: handle imply -p on -c/--cc logic for log.cLibravatar Sergey Organov1-4/+0
Move logic that handles implying -p on -c/--cc from log_setup_revisions_tweak() to diff_merges_setup_revs(), where it belongs. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21diff-merges: rename diff_merges_default_to_enable() to match semanticsLibravatar Sergey Organov1-2/+2
Rename diff_merges_default_to_enable() to diff_merges_default_to_first_parent() to match its semantics. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21diff-merges: move checks for first_parent_only out of the moduleLibravatar Sergey Organov1-2/+6
The checks for first_parent_only don't in fact belong to this module, as the primary purpose of this flag is history traversal limiting, so get it out of this module and rename the diff_merges_first_parent_defaults_to_enable() to diff_merges_default_to_enable() to match new semantics. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21diff-merges: rename all functions to have common prefixLibravatar Sergey Organov1-2/+2
Use the same "diff_merges" prefix for all the diff merges function names. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21revision: move diff merges functions to its own diff-merges.cLibravatar Sergey Organov1-0/+1
Create separate diff-merges.c and diff-merges.h files, and move all the code related to handling of diff merges there. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-21revision: provide implementation for diff merges tweaksLibravatar Sergey Organov1-11/+2
Use these implementations from show_setup_revisions_tweak() and log_setup_revisions_tweak() in builtin/log.c. This completes moving of management of diff merges parameters to a single place, where we can finally observe them simultaneously. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-08Merge branch 'ma/grep-init-default'Libravatar Junio C Hamano1-1/+0
Code clean-up. * ma/grep-init-default: MyFirstObjectWalk: drop `init_walken_defaults()` grep: copy struct in one fell swoop grep: use designated initializers for `grep_defaults` grep: don't set up a "default" repo for grep
2020-11-21Merge branch 'jc/format-patch-name-max'Libravatar Junio C Hamano1-6/+14
The maximum length of output filenames "git format-patch" creates has become configurable (used to be capped at 64). * jc/format-patch-name-max: format-patch: make output filename configurable
2020-11-21grep: use designated initializers for `grep_defaults`Libravatar Martin Ågren1-1/+0
In 15fabd1bbd ("builtin/grep.c: make configuration callback more reusable", 2012-10-09), we learned to fill a `static struct grep_opt grep_defaults` which we can use as a blueprint for other such structs. At the time, we didn't consider designated initializers to be widely useable, but these days, we do. (See, e.g., cbc0f81d96 ("strbuf: use designated initializers in STRBUF_INIT", 2017-07-10).) Use designated initializers to let the compiler set up the struct and so that we don't need to remember to call `init_grep_defaults()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-21grep: don't set up a "default" repo for grepLibravatar Martin Ågren1-1/+1
`init_grep_defaults()` fills a `static struct grep_opt grep_defaults`. This struct is then used by `grep_init()` as a blueprint for other such structs. Notably, `grep_init()` takes a `struct repo *` and assigns it into the target struct. As a result, it is unnecessary for us to take a `struct repo *` in `init_grep_defaults()` as well. We assign it into the default struct and never look at it again. And in light of how we return early if we have already set up the default struct, it's not just unnecessary, but is also a bit confusing: If we are called twice and with different repos, is it a bug or a feature that we ignore the second repo? Drop the repo parameter for `init_grep_defaults()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-18Merge branch 'pb/blame-funcname-range-userdiff'Libravatar Junio C Hamano1-2/+2
"git blame -L :funcname -- path" did not work well for a path for which a userdiff driver is defined. * pb/blame-funcname-range-userdiff: blame: simplify 'setup_blame_bloom_data' interface blame: simplify 'setup_scoreboard' interface blame: enable funcname blaming with userdiff driver line-log: mention both modes in 'blame' and 'log' short help doc: add more pointers to gitattributes(5) for userdiff blame-options.txt: also mention 'funcname' in '-L' description doc: line-range: improve formatting doc: log, gitk: move '-L' description to 'line-range-options.txt'
2020-11-18Merge branch 'jk/format-patch-output'Libravatar Junio C Hamano1-15/+22
"git format-patch --output=there" did not work as expected and instead crashed. The option is now supported. * jk/format-patch-output: format-patch: support --output option format-patch: tie file-opening logic to output_directory format-patch: refactor output selection
2020-11-18Merge branch 'jc/line-log-takes-no-pathspec'Libravatar Junio C Hamano1-0/+3
"git log -L<range>:<path>" is documented to take no pathspec, but this was not enforced by the command line option parser, which has been corrected. * jc/line-log-takes-no-pathspec: log: diagnose -L used with pathspec as an error
2020-11-09format-patch: make output filename configurableLibravatar Junio C Hamano1-6/+14
For the past 15 years, we've used the hardcoded 64 as the length limit of the filename of the output from the "git format-patch" command. Since the value is shorter than the 80-column terminal, it could grow without line wrapping a bit. At the same time, since the value is longer than half of the 80-column terminal, we could fit two or more of them in "ls" output on such a terminal if we allowed to lower it. Introduce a new command line option --filename-max-length=<n> and a new configuration variable format.filenameMaxLength to override the hardcoded default. While we are at it, remove a check that the name of output directory does not exceed PATH_MAX---this check is pointless in that by the time control reaches the function, the caller would already have done an equivalent of "mkdir -p", so if the system does not like an overly long directory name, the control wouldn't have reached here, and otherwise, we know that the system allowed the output directory to exist. In the worst case, we will get an error when we try to open the output file and handle the error correctly anyway. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-04format-patch: support --output optionLibravatar Jeff King1-2/+9
We've never intended to support diff's --output option in format-patch. And until baa4adc66a (parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN, 2019-01-27), it was impossible to trigger. We first parse the format-patch options before handing the remainder off to setup_revisions(). Before that commit, we'd accept "--output=foo" as an abbreviation for "--output-directory=foo". But afterwards, we don't check abbreviations, and --output gets passed to the diff code. This results in nonsense behavior and bugs. The diff code will have opened a filehandle at rev.diffopt.file, but we'll overwrite that with our own handles that we open for each individual patch file. So the --output file will always just be empty. But worse, the diff code also sets rev.diffopt.close_file, so log_tree_commit() will close the filehandle itself. And then the main loop in cmd_format_patch() will try to close it again, resulting in a double-free. The simplest solution would be to just disallow --output with format-patch, as nobody ever intended it to work. However, we have accidentally documented it (because format-patch includes diff-options). And it does work with "git log", which writes the whole output to the specified file. It's easy enough to make that work for format-patch, too: it's really the same as --stdout, but pointed at a specific file. We can detect the use of the --output option by the "close_file" flag (note that we can't use rev.diffopt.file, since the diff setup will otherwise set it to stdout). So we just need to unset that flag, but don't have to do anything else. Our situation is otherwise exactly like --stdout (note that we don't fclose() the file, but nor does the stdout case; exiting the program takes care of that for us). Reported-by: Johannes Postler <johannes.postler@txture.io> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-04format-patch: tie file-opening logic to output_directoryLibravatar Jeff King1-6/+6
In format-patch we're either outputting to stdout or to individual files in an output directory (which may be just "./"). Our logic for whether to open a new file for each patch is checked with "!use_stdout", but it is equally correct to check for a non-NULL output_directory. The distinction will matter when we add a new single-stream output in a future patch, when only one of the three methods will want individual files. Let's swap the logic here in preparation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-04format-patch: refactor output selectionLibravatar Jeff King1-9/+9
The --stdout and --output-directory options are mutually exclusive, but it's hard to tell from reading the code. We have three separate conditionals that check for use_stdout, and it's only after we've set up the output_directory fully that we check whether the user also specified --stdout. Instead, let's check the exclusion explicitly first, then have a single conditional that handles stdout versus an output directory. This is slightly easier to follow now, and also will keep things sane when we add another output mode in a future patch. We'll add a few tests as well, covering the mutual exclusion and the fact that we are not confused by a configured output directory. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-04log: diagnose -L used with pathspec as an errorLibravatar Junio C Hamano1-0/+3
The -L option is documented to accept no pathspec, but the command line option parser has allowed the combination without checking so far. Ensure that there is no pathspec when the -L option is in effect to fix this. Incidentally, this change fixes another bug in the command line option parser, which has allowed the -L option used together with the --follow option. Because the latter requires exactly one path given, but the former takes no pathspec, they become mutually incompatible automatically. Because the -L option follows renames on its own, there is no reason to give --follow at the same time. The new tests say they may fail with "-L and --follow being incompatible" instead of "-L and pathspec being incompatible". Currently the expected failure can come only from the latter, but this is to futureproof them, in case we decide to add code to explicititly die on -L and --follow used together. Heled-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-01line-log: mention both modes in 'blame' and 'log' short helpLibravatar Philippe Blain1-2/+2
'git blame -h' and 'git log -h' both show '-L <n,m>' and describe this option as "Process only line range n,m, counting from 1". No hint is given that a function name regex can also be used. Use <range> instead, and expand the description of the option to mention both modes. Remove "counting from 1" as it's uneeded; it's uncommon to refer to the first line of a file as "line 0". Also, for 'git log', improve the wording to better reflect the long help. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20Documentation: stylistically normalize references to Signed-off-by:Libravatar Bradley M. Kuhn1-1/+1
Ted reported an old typo in the git-commit.txt and merge-options.txt. Namely, the phrase "Signed-off-by line" was used without either a definite nor indefinite article. Upon examination, it seems that the documentation (including items in Documentation/, but also option help strings) have been quite inconsistent on usage when referring to `Signed-off-by`. First, very few places used a definite or indefinite article with the phrase "Signed-off-by line", but that was the initial typo that led to this investigation. So, normalize using either an indefinite or definite article consistently. The original phrasing, in Commit 3f971fc425b (Documentation updates, 2005-08-14), is "Add Signed-off-by line". Commit 6f855371a53 (Add --signoff, --check, and long option-names. 2005-12-09) switched to using "Add `Signed-off-by:` line", but didn't normalize the former commit to match. Later commits seem to have cut and pasted from one or the other, which is likely how the usage became so inconsistent. Junio stated on the git mailing list in <xmqqy2k1dfoh.fsf@gitster.c.googlers.com> a preference to leave off the colon. Thus, prefer `Signed-off-by` (with backticks) for the documentation files and Signed-off-by (without backticks) for option help strings. Additionally, Junio argued that "trailer" is now the standard term to refer to `Signed-off-by`, saying that "becomes plenty clear that we are not talking about any random line in the log message". As such, prefer "trailer" over "line" anywhere the former word fits. However, leave alone those few places in documentation that use Signed-off-by to refer to the process (rather than the specific trailer), or in places where mail headers are generally discussed in comparison with Signed-off-by. Reported-by: "Theodore Y. Ts'o" <tytso@mit.edu> Signed-off-by: Bradley M. Kuhn <bkuhn@sfconservancy.org> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-05Merge branch 'jk/format-auto-base-when-able'Libravatar Junio C Hamano1-25/+105
"git format-patch" learns to take "whenAble" as a possible value for the format.useAutoBase configuration variable to become no-op when the automatically computed base does not make sense. * jk/format-auto-base-when-able: format-patch: teach format.useAutoBase "whenAble" option
2020-10-04Merge branch 'jk/shortlog-group-by-trailer'Libravatar Junio C Hamano1-0/+1
"git shortlog" has been taught to group commits by the contents of the trailer lines, like "Reviewed-by:", "Coauthored-by:", etc. * jk/shortlog-group-by-trailer: shortlog: allow multiple groups to be specified shortlog: parse trailer idents shortlog: rename parse_stdin_ident() shortlog: de-duplicate trailer values shortlog: match commit trailers with --group trailer: add interface for iterating over commit trailers shortlog: add grouping option shortlog: change "author" variables to "ident"
2020-10-01format-patch: teach format.useAutoBase "whenAble" optionLibravatar Jacob Keller1-25/+105
The format.useAutoBase configuration option exists to allow users to enable '--base=auto' for format-patch by default. This can sometimes lead to poor workflow, due to unexpected failures when attempting to format an ancient patch: $ git format-patch -1 <an old commit> fatal: base commit shouldn't be in revision list This can be very confusing, as it is not necessarily immediately obvious that the user requested a --base (since this was in the configuration, not on the command line). We do want --base=auto to fail when it cannot provide a suitable base, as it would be equally confusing if a formatted patch did not include the base information when it was requested. Teach format.useAutoBase a new mode, "whenAble". This mode will cause format-patch to attempt to include a base commit when it can. However, if no valid base commit can be found, then format-patch will continue formatting the patch without a base commit. In order to avoid making yet another branch name unusable with --base, do not teach --base=whenAble or --base=whenable. Instead, refactor the base_commit option to use a callback, and rely on the global configuration variable auto_base. This does mean that a user cannot request this optional base commit generation from the command line. However, this is likely not too valuable. If the user requests base information manually, they will be immediately informed of the failure to acquire a suitable base commit. This allows the user to make an informed choice about whether to continue the format. Add tests to cover the new mode of operation for --base. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: allow multiple groups to be specifiedLibravatar Jeff King1-0/+1
Now that shortlog supports reading from trailers, it can be useful to combine counts from multiple trailers, or between trailers and authors. This can be done manually by post-processing the output from multiple runs, but it's non-trivial to make sure that each name/commit pair is counted only once. This patch teaches shortlog to accept multiple --group options on the command line, and pull data from all of them. That makes it possible to run: git shortlog -ns --group=author --group=trailer:co-authored-by to get a shortlog that counts authors and co-authors equally. The implementation is mostly straightforward. The "group" enum becomes a bitfield, and the trailer key becomes a list. I didn't bother implementing the multi-group semantics for reading from stdin. It would be possible to do, but the existing matching code makes it awkward, and I doubt anybody cares. The duplicate suppression we used for trailers now covers authors and committers as well (though in non-trailer single-group mode we can skip the hash insertion and lookup, since we only see one value per commit). There is one subtlety: we now care about the case when no group bit is set (in which case we default to showing the author). The caller in builtin/log.c needs to be adapted to ask explicitly for authors, rather than relying on shortlog_init(). It would be possible with some gymnastics to make this keep working as-is, but it's not worth it for a single caller. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-22Merge branch 'es/format-patch-interdiff-cleanup'Libravatar Junio C Hamano1-9/+13
"format-patch --range-diff=<prev> <origin>..HEAD" has been taught not to ignore <origin> when <prev> is a single version. * es/format-patch-interdiff-cleanup: format-patch: use 'origin' as start of current-series-range when known diff-lib: tighten show_interdiff()'s interface diff: move show_interdiff() from its own file to diff-lib
2020-09-09Merge branch 'jt/interpret-branch-name-fallback'Libravatar Junio C Hamano1-1/+1
"git status" has trouble showing where it came from by interpreting reflog entries that recordcertain events, e.g. "checkout @{u}", and gives a hard/fatal error. Even though it inherently is impossible to give a correct answer because the reflog entries lose some information (e.g. "@{u}" does not record what branch the user was on hence which branch 'the upstream' needs to be computed, and even if the record were available, the relationship between branches may have changed), at least hide the error to allow "status" show its output. * jt/interpret-branch-name-fallback: wt-status: tolerate dangling marks refs: move dwim_ref() to header file sha1-name: replace unsigned int with option struct
2020-09-08format-patch: use 'origin' as start of current-series-range when knownLibravatar Eric Sunshine1-7/+11
When formatting a patch series over `origin..HEAD`, one would expect that range to be used as the current-series-range when computing a range-diff between the previous and current versions of a patch series. However, infer_range_diff_ranges() ignores `origin..HEAD` when --range-diff=<prev> specifies a single revision rather than a range, and instead unexpectedly computes the current-series-range based upon <prev>. Address this anomaly by unconditionally using `origin..HEAD` as the current-series-range regardless of <prev> as long as `origin` is known, and only fall back to basing current-series-range on <prev> when `origin` is not known. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-08diff-lib: tighten show_interdiff()'s interfaceLibravatar Eric Sunshine1-1/+2
To compute and show an interdiff, show_interdiff() needs only the two OID's to compare and a diffopts, yet it expects callers to supply an entire rev_info. The demand for rev_info is not only overkill, but also places unnecessary burden on potential future callers which might not otherwise have a rev_info at hand. Address this by tightening its signature to require only the items it needs instead of a full rev_info. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-08diff: move show_interdiff() from its own file to diff-libLibravatar Eric Sunshine1-1/+0
show_interdiff() is a relatively small function and not likely to grow larger or more complicated. Rather than dedicating an entire source file to it, relocate it to diff-lib.c which houses other "take two things and compare them" functions meant to be re-used but not so low-level as to reside in the core diff implementation. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>