aboutsummaryrefslogtreecommitdiff
path: root/builtin/log.c
AgeCommit message (Collapse)Author
2024-12-18pager: stop using `the_repository`Patrick Steinhardt
Stop using `the_repository` in the "pager" subsystem by passing in a repository when setting up the pager and when configuring it. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-18progress: stop using `the_repository`Patrick Steinhardt
Stop using `the_repository` in the "progress" subsystem by passing in a repository when initializing `struct progress`. Furthermore, store a pointer to the repository in that struct so that we can pass it to the trace2 API when logging information. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-13log: --remerge-diff needs to keep around commit parentsJohannes Schindelin
To show a remerge diff, the merge needs to be recreated. For that to work, the merge base(s) need to be found, which means that the commits' parents have to be traversed until common ancestors are found (if any). However, one optimization that hails all the way back to cb115748ec0d (Some more memory leak avoidance, 2006-06-17) is to release the commit's list of parents immediately after showing it _and to set that parent list to `NULL`_. This can break the merge base computation. This problem is most obvious when traversing the commits in reverse: In that instance, if a parent of a merge commit has been shown as part of the `git log` command, by the time the merge commit's diff needs to be computed, that parent commit's list of parent commits will have been set to `NULL` and as a result no merge base will be found (even if one should be found). Traversing commits in reverse is far from the only circumstance in which this problem occurs, though. There are many avenues to traversing at least one commit in the revision walk that will later be part of a merge base computation, for example when not even walking any revisions in `git show <merge1> <merge2>` where `<merge1>` is part of the commit graph between the parents of `<merge2>`. Another way to force a scenario where a commit is traversed before it has to be traversed again as part of a merge base computation is to start with two revisions (where the first one is reachable from the second but not in a first-parent ancestry) and show the commit log with `--topo-order` and `--first-parent`. Let's fix this by special-casing the `remerge_diff` mode, similar to what we did with reflogs in f35650dff6a4 (log: do not free parents when walking reflog, 2017-07-07). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt
Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-23Merge branch 'jc/pass-repo-to-builtins'Junio C Hamano
The convention to calling into built-in command implementation has been updated to pass the repository, if known, together with the prefix value. * jc/pass-repo-to-builtins: add: pass in repo variable instead of global the_repository builtin: remove USE_THE_REPOSITORY for those without the_repository builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h builtin: add a repository parameter for builtin functions
2024-09-16Merge branch 'jc/range-diff-lazy-setup'Junio C Hamano
Code clean-up. * jc/range-diff-lazy-setup: remerge-diff: clean up temporary objdir at a central place remerge-diff: lazily prepare temporary objdir on demand
2024-09-13builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.hJohn Cai
Instead of including USE_THE_REPOSITORY_VARIABLE by default on every builtin, remove it from builtin.h and add it to all the builtins that include builtin.h (by definition, that means all builtins/*.c). Also, remove the include statement for repository.h since it gets brought in through builtin.h. The next step will be to migrate each builtin from having to use the_repository. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-13builtin: add a repository parameter for builtin functionsJohn Cai
In order to reduce the usage of the global the_repository, add a parameter to builtin functions that will get passed a repository variable. This commit uses UNUSED on most of the builtin functions, as subsequent commits will modify the actual builtins to pass the repository parameter down. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-14builtin/log: fix leak when showing converted blob contentsPatrick Steinhardt
In `show_blob_object()`, we proactively call `textconv_object()`. In case we have a textconv driver for this blob we will end up showing the converted contents, otherwise we'll show the un-converted contents of it instead. When the object has been converted we never free the buffer containing the converted contents. Fix this to plug this memory leak. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-14builtin/format-patch: fix various trivial memory leaksPatrick Steinhardt
There are various memory leaks hit by git-format-patch(1). Basically all of them are trivial, except that un-setting `diffopt.no_free` requires us to unset the `diffopt.file` because we manually close it already. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-09remerge-diff: clean up temporary objdir at a central placeJunio C Hamano
After running a diff between two things, or a series of diffs while walking the history, the diff computation is concluded by a call to diff_result_code() to extract the exit status of the diff machinery. The function can work on "struct diffopt", but all the callers historically and currently pass "struct diffopt" that is embedded in the "struct rev_info" that is used to hold the remerge_diff bit and the remerge_objdir variable that points at the temporary object directory in use. Redefine diff_result_code() to take the whole "struct rev_info" to give it an access to these members related to remerge-diff, so that it can get rid of the temporary object directory for any and all callers that used the feature. We can lose the equivalent code to do so from the code paths for individual commands, diff-tree, diff, and log. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-09remerge-diff: lazily prepare temporary objdir on demandJunio C Hamano
It is error prone for each caller that sets revs.remerge_diff bit to be responsible for preparing a temporary object directory and rotate it into the list of alternate object stores, making it the primary object store. Instead, remove the code to set up and arrange the temporary object directory from the current callers and implement it in the code that runs remerge-diff logic. The code to undo the futzing of the list of alternate object store is still spread across the callers, but we will deal with it in future steps. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-01builtin/log: fix leaking branch name when creating cover lettersPatrick Steinhardt
When calling `make_cover_letter()` without a branch name, we try to derive the branch name by calling `find_branch_name()`. But while this function returns an allocated string, we never free the result and thus have a memory leak. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08Merge branch 'ps/leakfixes-more'Junio C Hamano
More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
2024-07-02Merge branch 'ps/use-the-repository'Junio C Hamano
A CPP macro USE_THE_REPOSITORY_VARIABLE is introduced to help transition the codebase to rely less on the availability of the singleton the_repository instance. * ps/use-the-repository: hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE` t/helper: remove dependency on `the_repository` in "proc-receive" t/helper: fix segfault in "oid-array" command without repository t/helper: use correct object hash in partial-clone helper compat/fsmonitor: fix socket path in networked SHA256 repos replace-object: use hash algorithm from passed-in repository protocol-caps: use hash algorithm from passed-in repository oidset: pass hash algorithm when parsing file http-fetch: don't crash when parsing packfile without a repo hash-ll: merge with "hash.h" refs: avoid include cycle with "repository.h" global: introduce `USE_THE_REPOSITORY_VARIABLE` macro hash: require hash algorithm in `empty_tree_oid_hex()` hash: require hash algorithm in `is_empty_{blob,tree}_oid()` hash: make `is_null_oid()` independent of `the_repository` hash: convert `oidcmp()` and `oideq()` to compare whole hash global: ensure that object IDs are always padded hash: require hash algorithm in `oidread()` and `oidclr()` hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()` hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
2024-06-20Merge branch 'rj/format-patch-auto-cover-with-interdiff'Junio C Hamano
"git format-patch --interdiff" for multi-patch series learned to turn on cover letters automatically (unless told never to enable cover letter with "--no-cover-letter" and such). * rj/format-patch-auto-cover-with-interdiff: format-patch: assume --cover-letter for diff in multi-patch series t4014: cleanups in a few tests
2024-06-14global: introduce `USE_THE_REPOSITORY_VARIABLE` macroPatrick Steinhardt
Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-14hash: require hash algorithm in `oidread()` and `oidclr()`Patrick Steinhardt
Both `oidread()` and `oidclr()` use `the_repository` to derive the hash function that shall be used. Require callers to pass in the hash algorithm to get rid of this implicit dependency. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11builtin/log: fix leaking commit list in git-cherry(1)Patrick Steinhardt
We're storing the list of commits that git-cherry(1) is about to print into a temporary list. This list is never getting free'd and thus leaks. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11object-name: free leaking object contextsPatrick Steinhardt
While it is documented in `struct object_context::path` that this variable needs to be released by the caller, this fact is rather easy to miss given that we do not ever provide a function to release the object context. And of course, while some callers dutifully release the path, many others don't. Introduce a new `object_context_release()` function that releases the path. Convert callsites that used to free the path to use that new function and add missing calls to callsites that were leaking memory. Refactor those callsites as required to have a single return path, only. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11parse-options: fix leaks for users of OPT_FILENAMEPatrick Steinhardt
The `OPT_FILENAME()` option will, if set, put an allocated string into the user-provided variable. Consequently, that variable thus needs to be free'd by the caller of `parse_options()`. Some callsites don't though and thus leak memory. Fix those. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07format-patch: assume --cover-letter for diff in multi-patch seriesRubén Justo
When we deal with a multi-patch series in git-format-patch(1), if we see `--interdiff` or `--range-diff` but no `--cover-letter`, we return with an error, saying: fatal: --range-diff requires --cover-letter or single patch or: fatal: --interdiff requires --cover-letter or single patch This makes sense because the cover-letter is where we place the diff from the previous version. However, considering that `format-patch` generates a multi-patch as needed, let's adopt a similar "cover as necessary" approach when using `--interdiff` or `--range-diff`. Therefore, relax the requirement for an explicit `--cover-letter` in a multi-patch series when the user says `--iterdiff` or `--range-diff`. Still, if only to return the error, respect "format.coverLetter=no" and `--no-cover-letter`. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07global: improve const correctness when assigning string constantsPatrick Steinhardt
We're about to enable `-Wwrite-strings`, which changes the type of string constants to `const char[]`. Fix various sites where we assign such constants to non-const variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-06Merge branch 'ps/leakfixes'Junio C Hamano
Leakfixes. * ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
2024-06-03Merge branch 'ps/leakfixes' into ps/leakfixes-moreJunio C Hamano
* ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
2024-05-29Merge branch 'ps/leakfixes' into ps/no-writable-stringsJunio C Hamano
* ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
2024-05-28Merge branch 'jc/format-patch-more-aggressive-range-diff'Junio C Hamano
The default "creation-factor" used by "git format-patch" has been raised to make it more aggressively find matching commits. * jc/format-patch-more-aggressive-range-diff: format-patch: run range-diff with larger creation-factor
2024-05-27config: clarify memory ownership in `git_config_string()`Patrick Steinhardt
The out parameter of `git_config_string()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27builtin/log: stop using globals for format configPatrick Steinhardt
This commit does the exact same as the preceding commit, only for the format configuration instead of the log configuration. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27builtin/log: stop using globals for log configPatrick Steinhardt
We're using global variables to store the log configuration. Many of these can be set both via the command line and via the config, and depending on how they are being set, they may contain allocated strings. This leads to hard-to-track memory ownership and memory leaks. Refactor the code to instead use a `struct log_config` that is being allocated on the stack. This allows us to more clearly scope the variables, track memory ownership and ultimately release the memory. This also prepares us for a change to `git_config_string()`, which will be adapted to have a `char **` out parameter instead of `const char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt
The out parameter of `git_config_pathname()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt
Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06format-patch: run range-diff with larger creation-factorJunio C Hamano
We see too often that a range-diff added to format-patch output shows too many "unmatched" patches. This is because the default value for creation-factor is set to a relatively low value. It may be justified for other uses (like you have a yet-to-be-sent new iteration of your series, and compare it against the 'seen' branch that has an older iteration, probably with the '--left-only' option, to pick out only your patches while ignoring the others) of "range-diff" command, but when the command is run as part of the format-patch, the user _knows_ and expects that the patches in the old and the new iterations roughly correspond to each other, so we can and should use a much higher default. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-30Merge branch 'jc/format-patch-rfc-more'Junio C Hamano
The "--rfc" option of "git format-patch" learned to take an optional string value to be used in place of "RFC" to tweak the "[PATCH]" on the subject header. * jc/format-patch-rfc-more: format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)] format-patch: allow --rfc to optionally take a value, like --rfc=WIP
2024-04-30Merge branch 'ds/format-patch-rfc-and-k'Junio C Hamano
The "-k" and "--rfc" options of "format-patch" will now error out when used together, as one tells us not to add anything to the title of the commit, and the other one tells us to add "RFC" in addition to "PATCH". * ds/format-patch-rfc-and-k: format-patch: ensure that --rfc and -k are mutually exclusive
2024-04-23format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]Junio C Hamano
In the previous step, the "--rfc" option of "format-patch" learned to take an optional string value to prepend to the subject prefix, so that --rfc=WIP can give "[WIP PATCH]". There may be cases in which the extra string wants to come after the subject prefix. Extend the mechanism to allow "--rfc=-(WIP)" [*] to signal that the extra string is to be appended instead of getting prepended, resulting in "[PATCH (WIP)]". In the documentation, discourage (ab)using "--rfc=-RFC" to say "[PATCH RFC]" just to be different, when "[RFC PATCH]" is the norm. [Footnote] * The syntax takes inspiration from Perl's open syntax that opens pipes "open fh, '|-', 'cmd'", where the dash signals "the other stuff comes here". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-23format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano
With the "--rfc" option, we can tweak the "[PATCH]" (or whatever string specified with the "--subject-prefix" option, instead of "PATCH") that we prefix the title of the commit with into "[RFC PATCH]", but some projects may want "[rfc PATCH]". Adding a new option, e.g., "--rfc-lowercase", to support such need every time somebody wants to use different strings would lead to insanity of accumulating unbounded number of such options. Allow an optional value specified for the option, so that users can use "--rfc=rfc" (think of "--rfc" without value as a short-hand for "--rfc=RFC") if they wanted to. This can of course be (ab)used to make the prefix "[WIP PATCH]" by passing "--rfc=WIP". Passing an empty string, i.e., "--rfc=", is the same as "--no-rfc" to override an option given earlier on the same command line. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-19format-patch: ensure that --rfc and -k are mutually exclusiveDragan Simic
Fix a bug that allows the "--rfc" and "-k" options to be specified together when "git format-patch" is executed, which was introduced in the commit e0d7db7423a9 ("format-patch: --rfc honors what --subject-prefix sets"). Add a couple of additional tests to t4014, to cover additional cases of the mutual exclusivity between different "git format-patch" options. Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-01Merge branch 'jk/pretty-subject-cleanup'Junio C Hamano
Code clean-up in the "git log" machinery that implements custom log message formatting. * jk/pretty-subject-cleanup: format-patch: fix leak of empty header string format-patch: simplify after-subject MIME header handling format-patch: return an allocated string from log_write_email_headers() log: do not set up extra_headers for non-email formats pretty: drop print_email_subject flag pretty: split oneline and email subject printing shortlog: stop setting pp.print_email_subject
2024-03-19format-patch: return an allocated string from log_write_email_headers()Jeff King
When pretty-printing a commit in the email format, we have to fill in the "after subject" field of the pretty_print_context with any extra headers the user provided (e.g., from "--to" or "--cc" options) plus any special MIME headers. We return an out-pointer that sometimes points to a newly heap-allocated string and sometimes not. To avoid leaking, we store the allocated version in a buffer with static lifetime, which is ugly. Worse, as we extend the header feature, we'll end up having to repeat this ugly pattern. Instead, let's have our out-pointer pass ownership back to the caller, and duplicate the string when necessary. This does mean one extra allocation per commit when you use extra headers, but in the context of format-patch which is showing diffs, I don't think that's even measurable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19pretty: drop print_email_subject flagJeff King
With one exception, the print_email_subject flag is set if and only if the commit format is email based: - in make_cover_letter() we set it along with CMIT_FMT_EMAIL explicitly - in show_log(), we set it if cmit_fmt_is_mail() is true. That covers format-patch as well as "git log --format=email" (or mboxrd). The one exception is "rev-list --format=email", which somewhat nonsensically prints the author and date as email headers, but no subject, like: $ git rev-list --format=email HEAD commit 64fc4c2cdd4db2645eaabb47aa4bac820b03cdba From: Jeff King <peff@peff.net> Date: Tue, 19 Mar 2024 19:39:26 -0400 this is the subject this is the body It's doubtful that this is a useful format at all (the "commit" lines replace the "From" lines that would make it work as an actual mbox). But I think that printing the subject as a header (like this patch does) is the least surprising thing to do. So let's drop this field, making the code a little simpler and easier to reason about. Note that we do need to set the "rev" field of the pretty_print_context in rev-list, since that is used to check for subject_prefix, etc. It's not possible to set those fields via rev-list, so we'll always just print "Subject: ". But unless we pass in our rev_info, fmt_output_email_subject() would segfault trying to figure it out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19pretty: split oneline and email subject printingJeff King
The pp_title_line() function is used for two formats: the oneline format and the subject line of the email format. But most of the logic in the function does not make any sense for oneline; it is about special formatting of email headers. Lumping the two formats together made sense long ago in 4234a76167 (Extend --pretty=oneline to cover the first paragraph, 2007-06-11), when there was a lot of manual logic to paste lines together. But later, 88c44735ab (pretty: factor out format_subject(), 2008-12-27) pulled that logic into its own function. We can implement the oneline format by just calling that one function. This makes the intention of the code much more clear, as we know we only need to worry about those extra email options when dealing with actual email. While the intent here is cleanup, it is possible to trigger these cases in practice by running format-patch with an explicit --oneline option. But if you did, the results are basically nonsense. For example, with the preserve_subject flag: $ printf "%s\n" one two three | git commit --allow-empty -F - $ git format-patch -1 --stdout -k | grep ^Subject Subject: =?UTF-8?q?one=0Atwo=0Athree?= $ git format-patch -1 --stdout -k --oneline --no-signature 2af7fbe one two three Or with extra headers: $ git format-patch -1 --stdout --cc=me --oneline --no-signature 2af7fbe one two three Cc: me So I'd actually consider this to be an improvement, though you are probably crazy to use other formats with format-patch in the first place (arguably it should forbid non-email formats entirely, but that's a bigger change). As a bonus, it eliminates some pointless extra allocations for the oneline output. The email code, since it has to deal with wrapping, formats into an extra auxiliary buffer. The speedup is tiny, though like "rev-list --no-abbrev --format=oneline" seems to improve by a consistent 1-2% for me. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(repo_get_merge_bases_many): pass on "missing commits" errorsJohannes Schindelin
The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases_many()` function is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next stop: `repo_get_merge_bases_dirty()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(repo_get_merge_bases): pass on "missing commits" errorsJohannes Schindelin
The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases()` function (which is also surfaced via the `repo_get_merge_bases()` macro) is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next step: adjust the callers of `get_octopus_merge_bases()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-28commit-reach(repo_in_merge_bases_many): report missing commitsJohannes Schindelin
Some functions in Git's source code follow the convention that returning a negative value indicates a fatal error, e.g. repository corruption. Let's use this convention in `repo_in_merge_bases()` to report when one of the specified commits is missing (i.e. when `repo_parse_commit()` reports an error). Also adjust the callers of `repo_in_merge_bases()` to handle such negative return values. Note: As of this patch, errors are returned only if any of the specified merge heads is missing. Over the course of the next patches, missing commits will also be reported by the `paint_down_to_common()` function, which is called by `repo_in_merge_bases_many()`, and those errors will be properly propagated back to the caller at that stage. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-08Merge branch 'en/header-cleanup' into maint-2.43Junio C Hamano
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2024-01-08Merge branch 'en/header-cleanup'Junio C Hamano
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2023-12-26treewide: remove unnecessary includes in source filesElijah Newren
Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-20Merge branch 'jk/implicit-true'Junio C Hamano
Some codepaths did not correctly parse configuration variables specified with valueless "true", which has been corrected. * jk/implicit-true: fsck: handle NULL value when parsing message config trailer: handle NULL value when parsing trailer-specific config submodule: handle NULL value when parsing submodule.*.branch help: handle NULL value for alias.* config trace2: handle NULL values in tr2_sysenv config callback setup: handle NULL value when parsing extensions config: handle NULL value when parsing non-bools
2023-12-09config: handle NULL value when parsing non-boolsJeff King
When the config parser sees an "implicit" bool like: [core] someVariable it passes NULL to the config callback. Any callback code which expects a string must check for NULL. This usually happens via helpers like git_config_string(), etc, but some custom code forgets to do so and will segfault. These are all fairly vanilla cases where the solution is just the usual pattern of: if (!value) return config_error_nonbool(var); though note that in a few cases we have to split initializers like: int some_var = initializer(); into: int some_var; if (!value) return config_error_nonbool(var); some_var = initializer(); There are still some broken instances after this patch, which I'll address on their own in individual patches after this one. Reported-by: Carlos Andrés Ramírez Cataño <antaigroupltda@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>