From 04133f5bc4f3dc7c847f4ba50e02486bcc117d94 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Tue, 12 Aug 2025 06:44:35 +0000 Subject: [PATCH 01/16] send-email: add ability to send a copy of sent emails to an IMAP folder Some email providers like Apple iCloud Mail do not support sending a copy of sent emails to the "Sent" folder if SMTP server is used. As a workaround, various email clients like Thunderbird which rely on SMTP, use IMAP to send a copy of sent emails to the "Sent" folder. Something similar can be done if sending emails via `git send-email`, by using the `git imap-send` command to send a copy of the sent email to an IMAP folder specified by the user. Add this functionality to `git send-email` by introducing a new configuration variable `sendemail.imapfolder` and command line option `--imap-folder` which specifies the IMAP folder to send a copy of the sent emails to. If specified, a copy of the sent emails will be sent by piping the emails to `git imap-send` command, after all emails are sent via SMTP and the SMTP server has been closed. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/config/sendemail.adoc | 1 + Documentation/git-send-email.adoc | 12 +++++++++++ git-send-email.perl | 31 ++++++++++++++++++++++++++++- imap-send.c | 26 ++++++++++++++++-------- 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/Documentation/config/sendemail.adoc b/Documentation/config/sendemail.adoc index 47223346579727..dd2dbc87a06f17 100644 --- a/Documentation/config/sendemail.adoc +++ b/Documentation/config/sendemail.adoc @@ -88,6 +88,7 @@ sendemail.smtpServer:: sendemail.smtpServerPort:: sendemail.smtpServerOption:: sendemail.smtpUser:: +sendemail.imapSentFolder:: sendemail.thread:: sendemail.transferEncoding:: sendemail.validate:: diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index 5335502d68fc7b..d1c41a0dbd4c3f 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -299,6 +299,18 @@ must be used for each option. commands and replies will be printed. Useful to debug TLS connection and authentication problems. +--imap-sent-folder=:: + Some email providers (e.g. iCloud) do not send a copy of the emails sent + using SMTP to the `Sent` folder or similar in your mailbox. Use this option + to use `git imap-send` to send a copy of the emails to the folder specified + using this option. You can run `git imap-send --list` to get a list of + valid folder names, including the correct name of the `Sent` folder in + your mailbox. You can also use this option to send emails to a dedicated + IMAP folder of your choice. ++ +This feature requires setting up `git imap-send`. See linkgit:git-imap-send[1] +for instructions. + --batch-size=:: Some email servers (e.g. 'smtp.163.com') limit the number of emails to be sent per session (connection) and this will lead to a failure when diff --git a/git-send-email.perl b/git-send-email.perl index 437f8ac46a85dd..b3cc237baac2c1 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -73,6 +73,8 @@ sub usage { --no-smtp-auth * Disable SMTP authentication. Shorthand for `--smtp-auth=none` --smtp-debug <0|1> * Disable, enable Net::SMTP debug. + --imap-sent-folder * IMAP folder where a copy of the emails should be sent. + Make sure `git imap-send` is set up to use this feature. --batch-size * send max message per connection. --relogin-delay * delay seconds between two successive login. @@ -200,7 +202,7 @@ sub format_2822_time { # Variables we fill in automatically, or via prompting: my (@to,@cc,@xh,$envelope_sender, - $initial_in_reply_to,$reply_to,$initial_subject,@files, + $initial_in_reply_to,$reply_to,$initial_subject,@files,@imap_copy, $author,$sender,$smtp_authpass,$annotate,$compose,$time); # Things we either get from config, *or* are overridden on the # command-line. @@ -277,6 +279,7 @@ sub do_edit { my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path); my ($batch_size, $relogin_delay); my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth); +my ($imap_sent_folder); my ($confirm); my (@suppress_cc); my ($auto_8bit_encoding); @@ -322,6 +325,7 @@ sub do_edit { "smtpauth" => \$smtp_auth, "smtpbatchsize" => \$batch_size, "smtprelogindelay" => \$relogin_delay, + "imapsentfolder" => \$imap_sent_folder, "to" => \@config_to, "tocmd" => \$to_cmd, "cc" => \@config_cc, @@ -527,6 +531,7 @@ sub config_regexp { "smtp-domain:s" => \$smtp_domain, "smtp-auth=s" => \$smtp_auth, "no-smtp-auth" => sub {$smtp_auth = 'none'}, + "imap-sent-folder=s" => \$imap_sent_folder, "annotate!" => \$annotate, "compose" => \$compose, "quiet" => \$quiet, @@ -1829,6 +1834,17 @@ sub send_message { print "\n"; } + if ($imap_sent_folder && !$dry_run) { + my $imap_header = $header; + if (@initial_bcc) { + # Bcc is not a part of $header, so we add it here. + # This is only for the IMAP copy, not for the actual email + # sent to the recipients. + $imap_header .= "Bcc: " . join(", ", @initial_bcc) . "\n"; + } + push @imap_copy, "From git-send-email\n$imap_header\n$message"; + } + return 1; } @@ -2223,6 +2239,19 @@ sub cleanup_compose_files { $smtp->quit if $smtp; +if ($imap_sent_folder && @imap_copy && !$dry_run) { + my $imap_input = join("\n", @imap_copy); + eval { + print "\nStarting git imap-send...\n"; + my ($fh, $ctx) = Git::command_input_pipe(['imap-send', '-f', $imap_sent_folder]); + print $fh $imap_input; + Git::command_close_pipe($fh, $ctx); + 1; + } or do { + warn "Warning: failed to send messages to IMAP folder $imap_sent_folder: $@"; + }; +} + sub apply_transfer_encoding { my $message = shift; my $from = shift; diff --git a/imap-send.c b/imap-send.c index f5a656ac71dc2d..44de0c5a77e75e 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1441,14 +1441,24 @@ static int count_messages(struct strbuf *all_msgs) while (1) { if (starts_with(p, "From ")) { - p = strstr(p+5, "\nFrom: "); - if (!p) break; - p = strstr(p+7, "\nDate: "); - if (!p) break; - p = strstr(p+7, "\nSubject: "); - if (!p) break; - p += 10; - count++; + if (starts_with(p, "From git-send-email")) { + p = strstr(p+5, "\nFrom: "); + if (!p) break; + p += 7; + p = strstr(p, "\nTo: "); + if (!p) break; + p += 5; + count++; + } else { + p = strstr(p+5, "\nFrom: "); + if (!p) break; + p = strstr(p+7, "\nDate: "); + if (!p) break; + p = strstr(p+7, "\nSubject: "); + if (!p) break; + p += 10; + count++; + } } p = strstr(p+5, "\nFrom "); if (!p) From f33b2207da792b45354e9af8948745a169f75651 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Tue, 12 Aug 2025 06:44:36 +0000 Subject: [PATCH 02/16] send-email: enable copying emails to an IMAP folder without actually sending them `git imap-send` was built on the idea of copying emails to an IMAP folder like drafts, and sending them later using an email client. Currently the only way to do it is by piping output of `git format-patch` to IMAP send. Add another way to do it by using `git send-email` with the `--use-imap-only` or `sendmail.useImapOnly` option. This allows users to use the advanced features of `git send-email` like tweaking Cc: list programmatically, compose the cover letter, etc. and then send the well formatted emails to an IMAP folder using `git imap-send`. While at it, use `` instead of '' for --smtp-encryption ssl in help section of `git send-email`. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/config/sendemail.adoc | 1 + Documentation/git-send-email.adoc | 14 ++++++++++++++ git-send-email.perl | 9 ++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Documentation/config/sendemail.adoc b/Documentation/config/sendemail.adoc index dd2dbc87a06f17..90164c734d2660 100644 --- a/Documentation/config/sendemail.adoc +++ b/Documentation/config/sendemail.adoc @@ -89,6 +89,7 @@ sendemail.smtpServerPort:: sendemail.smtpServerOption:: sendemail.smtpUser:: sendemail.imapSentFolder:: +sendemail.useImapOnly:: sendemail.thread:: sendemail.transferEncoding:: sendemail.validate:: diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index d1c41a0dbd4c3f..a385f865fb3d13 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -311,6 +311,20 @@ must be used for each option. This feature requires setting up `git imap-send`. See linkgit:git-imap-send[1] for instructions. +--use-imap-only:: +--no-use-imap-only:: + If this is set, all emails will only be copied to the IMAP folder specified + with `--imap-sent-folder` or `sendemail.imapSentFolder` and will not be sent + to the recipients. Useful if you just want to create a draft of the emails + and use another email client to send them. + If disabled with `--no-use-imap-only`, the emails will be sent like usual. + Disabled by default, but the `sendemail.useImapOnly` configuration + variable can be used to enable it. + ++ +This feature requires setting up `git imap-send`. See linkgit:git-imap-send[1] +for instructions. + --batch-size=:: Some email servers (e.g. 'smtp.163.com') limit the number of emails to be sent per session (connection) and this will lead to a failure when diff --git a/git-send-email.perl b/git-send-email.perl index b3cc237baac2c1..96504e7be121f6 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -62,7 +62,7 @@ sub usage { --smtp-user * Username for SMTP-AUTH. --smtp-pass * Password for SMTP-AUTH; not necessary. --smtp-encryption * tls or ssl; anything else disables. - --smtp-ssl * Deprecated. Use '--smtp-encryption ssl'. + --smtp-ssl * Deprecated. Use `--smtp-encryption ssl`. --smtp-ssl-cert-path * Path to ca-certificates (either directory or file). Pass an empty string to disable certificate verification. @@ -75,6 +75,8 @@ sub usage { --smtp-debug <0|1> * Disable, enable Net::SMTP debug. --imap-sent-folder * IMAP folder where a copy of the emails should be sent. Make sure `git imap-send` is set up to use this feature. + --[no-]use-imap-only * Only copy emails to the IMAP folder specified by + `--imap-sent-folder` instead of actually sending them. --batch-size * send max message per connection. --relogin-delay * delay seconds between two successive login. @@ -296,6 +298,7 @@ sub do_edit { my $target_xfer_encoding = 'auto'; my $forbid_sendmail_variables = 1; my $outlook_id_fix = 'auto'; +my $use_imap_only = 0; my %config_bool_settings = ( "thread" => \$thread, @@ -312,6 +315,7 @@ sub do_edit { "forbidsendmailvariables" => \$forbid_sendmail_variables, "mailmap" => \$mailmap, "outlookidfix" => \$outlook_id_fix, + "useimaponly" => \$use_imap_only, ); my %config_settings = ( @@ -532,6 +536,7 @@ sub config_regexp { "smtp-auth=s" => \$smtp_auth, "no-smtp-auth" => sub {$smtp_auth = 'none'}, "imap-sent-folder=s" => \$imap_sent_folder, + "use-imap-only!" => \$use_imap_only, "annotate!" => \$annotate, "compose" => \$compose, "quiet" => \$quiet, @@ -1683,6 +1688,8 @@ sub send_message { if ($dry_run) { # We don't want to send the email. + } elsif ($use_imap_only) { + die __("The destination IMAP folder is not properly defined.") if !defined $imap_sent_folder; } elsif (defined $sendmail_cmd || file_name_is_absolute($smtp_server)) { my $pid = open my $sm, '|-'; defined $pid or die $!; From 8655908b9eb00a47332d53bf73d9d7fb6cd1d569 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 14 Aug 2025 08:09:17 -0700 Subject: [PATCH 03/16] abbrev: allow extending beyond 32 chars to disambiguate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When you have two or more objects with object names that share more than 32 letters in an SHA-1 repository, find_unique_abbrev() fails to show disambiguation. To see how many leading letters of a given full object name is sufficiently unambiguous, the algorithm starts from a initial length, guessed based on the estimated number of objects in the repository, and see if another object that shares the prefix, and keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ, which is counted as the number of bytes, since 5b20ace6 (sha1_name: unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that change, it extended up to GIT_SHA1_HEXSZ, which meant to stop at the end of hexadecimal SHA-1 object name. Because the hexadecimal object name passed to the function is NUL-terminated, and this fact is used to correctly terminate the loop that scans for the first difference earlier in the function, use it to make sure we do not increment the .cur_len member beyond the end of the string. Noticed-by: Jon Forrest Helped-by: René Scharfe Signed-off-by: Junio C Hamano --- object-name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/object-name.c b/object-name.c index 11aa0e6afc565e..4cd1d387784b11 100644 --- a/object-name.c +++ b/object-name.c @@ -704,7 +704,7 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data) while (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i)) i++; - if (i < GIT_MAX_RAWSZ && i >= mad->cur_len) + if (mad->hex[i] && i >= mad->cur_len) mad->cur_len = i + 1; return 0; From ab60c693a2f473c9952ad8ce31e94df5f76431b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 18 Aug 2025 13:13:09 +0200 Subject: [PATCH 04/16] line-log: fix assertion error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When line-level log is invoked with more than one disjoint line range in the same file, and one of the commits happens to change that file such that: - the last line of a line range R(n) immediately preceeds the first line modified or added by a hunk H, and - subtracting the number of lines added by hunk H from the start and end of the subsequent line range R(n+1) would result in a range overlapping with line range R(n), then git aborts with an assertion error, because those overlapping line ranges violate the invariants: $ git log --oneline -p 73e4e2f (HEAD -> master) Add lines 6 7 8 9 10 diff --git a/file b/file index 572d5d9..00935f1 100644 --- a/file +++ b/file @@ -3,3 +3,8 @@ Line 2 Line 3 Line 4 Line 5 +Line 6 +Line 7 +Line 8 +Line 9 +Line 10 66e3561 Add lines 1 2 3 4 5 diff --git a/file b/file new file mode 100644 index 0000000..572d5d9 --- /dev/null +++ b/file @@ -0,0 +1,5 @@ +Line 1 +Line 2 +Line 3 +Line 4 +Line 5 $ git log --oneline -L3,5:file -L7,8:file git: line-log.c:73: range_set_append: Assertion `rs->nr == 0 || rs->ranges[rs->nr-1].end <= a' failed. Aborted (core dumped) The line-log machinery encodes line and diff ranges internally as [start, end) pairs, i.e. include 'start' but exclude 'end', and line numbering starts at 0 (as opposed to the -LX,Y option, where it starts at 1, IOW the parameter -L3,5 is represented internally as { start = 2, end = 5 }). The reason for this assertion error and some related issues is that there are a couple of places where 'end' is mistakenly considered to be part of the range: - When a commit modifies an interesting path, the line-log machinery first checks which diff range (i.e. hunk) modify any line ranges. This is done in diff_ranges_filter_touched(), where the outer loop iterates over the diff ranges, and in each iteration the inner loop advances the line ranges supposedly until the current line range ends at or after the current diff range starts, and then the current diff and line ranges are checked for overlap. For HEAD in the above example the first line range [2, 5) ends just before the diff range [5, 10) starts, so the inner loop should advance, and then the second line range [6, 8) and the diff range should be checked for overlap. Unfortunately, the condition of the inner loop mistakenly considers 'end' as part of the line range, and, seeing the diff range starting at 5 and the line range ending at 5, it doesn't skip the first range. Consequently, the diff range and the first line range are checked for overlap, and after that the outer loop runs out of diff ranges, and then the processing goes on in the false belief that this commit didn't touch any of the interesting line ranges. The line-log machinery later shifts the line ranges to account for any added/removed lines in the diff ranges preceeding each line range. This leaves the first line range intact, but attempts to shift the second line range [6, 8) by 5 lines towards the beginning of the file, resulting in [1, 3), triggering the assertion error, because the two overlapping line ranges violate the invariants. Fix that loop condition in diff_ranges_filter_touched() to not treat 'end' as part of the line range. - With the above fix the assertion error is gone... but, alas, we now get stuck in an endless loop! This happens in range_set_difference(), where a couple of nested loops iterate over the line and diff ranges, and a condition is supposed to break the middle loop when the current line range ends before the current diff range, so processing could continue with the next line range. For HEAD in the above example the first line range [2, 5) ends just before the diff range [5, 10) starts, so this condition should trigger and break the middle loop. Unfortunately, just like in the case of the assertion error, this conditions mistakenly considers 'end' as part of the line range, and, seeing the line range ending at 5 and the diff range starting at 5, it doesn't break the loop, which will then go on and on. Fix this condition in range_set_difference() to not treat 'end' as part of the line range. - With the above fix the endless loop is gone... but, alas, the output is now wrong, as it shows both line ranges for HEAD, even though the first line range is not modified by that commit: $ git log --oneline -L3,5:file -L7,8:file 73e4e2f (HEAD -> master) Add lines 6 7 8 9 10 diff --git a/file b/file --- a/file +++ b/file @@ -3,3 +3,3 @@ Line 3 Line 4 Line 5 @@ -6,0 +7,2 @@ +Line 7 +Line 8 66e3561 Add lines 1 2 3 4 5 diff --git a/file b/file --- /dev/null +++ b/file @@ -0,0 +3,3 @@ +Line 3 +Line 4 +Line 5 In dump_diff_hacky_one() a couple of nested loops are responsible for finding and printing the modified line ranges: the big outer loop iterates over all line ranges, and the first inner loop skips over the diff ranges that end before the start of the current line range. This is followed by a condition checking whether the current diff range starts after the end of the current line range, which, when fulfilled, continues and advances the outer loop to the next line range. For HEAD in the above example the first line range [2, 5) ends just before the diff range [5, 10), so this condition should trigger, and the outer loop should advance to the second line range. Unfortunately, just like in the previous cases, this condition mistakenly considers 'end' as part of the line range, and, seeing the first line range ending at 5 and the diff range starting at 5, it doesn't continue to advance the outher loop, but goes on to show the (unmodified) first line range. Fix this condition to not treat 'end' as part of the line range, just like in the previous cases. After all this the command in the above example finally finishes and produces the right output: $ git log --oneline -L3,5:file -L7,8:file 73e4e2f (HEAD -> master) Add lines 6 7 8 9 10 diff --git a/file b/file --- a/file +++ b/file @@ -6,0 +7,2 @@ +Line 7 +Line 8 66e3561 Add lines 1 2 3 4 5 diff --git a/file b/file --- /dev/null +++ b/file @@ -0,0 +3,3 @@ +Line 3 +Line 4 +Line 5 Add a canned test similar to the above example, with the line ranges adjusted to the test repository's history. Reported-by: Evgeni Chasnovski Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- line-log.c | 6 +- t/t4211-line-log.sh | 2 + t/t4211/sha1/expect.no-assertion-error | 90 ++++++++++++++++++++++++ t/t4211/sha256/expect.no-assertion-error | 90 ++++++++++++++++++++++++ 4 files changed, 185 insertions(+), 3 deletions(-) create mode 100644 t/t4211/sha1/expect.no-assertion-error create mode 100644 t/t4211/sha256/expect.no-assertion-error diff --git a/line-log.c b/line-log.c index 628e3fe3ae18dc..9cb8ea1484ae84 100644 --- a/line-log.c +++ b/line-log.c @@ -201,7 +201,7 @@ static void range_set_difference(struct range_set *out, * b: ------| */ j++; - if (j >= b->nr || end < b->ranges[j].start) { + if (j >= b->nr || end <= b->ranges[j].start) { /* * b exhausted, or * a: ----| @@ -408,7 +408,7 @@ static void diff_ranges_filter_touched(struct diff_ranges *out, assert(out->target.nr == 0); for (i = 0; i < diff->target.nr; i++) { - while (diff->target.ranges[i].start > rs->ranges[j].end) { + while (diff->target.ranges[i].start >= rs->ranges[j].end) { j++; if (j == rs->nr) return; @@ -941,7 +941,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang while (j < diff->target.nr && diff->target.ranges[j].end < t_start) j++; - if (j == diff->target.nr || diff->target.ranges[j].start > t_end) + if (j == diff->target.nr || diff->target.ranges[j].start >= t_end) continue; /* Scan ahead to determine the last diff that falls in this range */ diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 950451cf6a66e6..0a7c3ca42f80fa 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -78,6 +78,8 @@ canned_test "-L :main:a.c -L 4,18:a.c simple" multiple-overlapping canned_test "-L 4:a.c -L 8,12:a.c simple" multiple-superset canned_test "-L 8,12:a.c -L 4:a.c simple" multiple-superset +canned_test "-L 10,16:b.c -L 18,26:b.c main" no-assertion-error + test_bad_opts "-L" "switch.*requires a value" test_bad_opts "-L b.c" "argument not .start,end:file" test_bad_opts "-L 1:" "argument not .start,end:file" diff --git a/t/t4211/sha1/expect.no-assertion-error b/t/t4211/sha1/expect.no-assertion-error new file mode 100644 index 00000000000000..994c37db1efec4 --- /dev/null +++ b/t/t4211/sha1/expect.no-assertion-error @@ -0,0 +1,90 @@ +commit 0d8dcfc6b968e06a27d5215bad1fdde3de9d6235 +Author: Thomas Rast +Date: Thu Feb 28 10:50:24 2013 +0100 + + move within the file + +diff --git a/b.c b/b.c +--- a/b.c ++++ b/b.c +@@ -25,0 +18,9 @@ ++long f(long x) ++{ ++ int s = 0; ++ while (x) { ++ x /= 2; ++ s++; ++ } ++ return s; ++} + +commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83 +Author: Thomas Rast +Date: Thu Feb 28 10:48:43 2013 +0100 + + change back to complete line + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -18,5 +18,7 @@ + int main () + { + printf("%ld\n", f(15)); + return 0; +-} +\ No newline at end of file ++} ++ ++/* incomplete lines are bad! */ + +commit 100b61a6f2f720f812620a9d10afb3a960ccb73c +Author: Thomas Rast +Date: Thu Feb 28 10:48:10 2013 +0100 + + change to an incomplete line at end + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -18,5 +18,5 @@ + int main () + { + printf("%ld\n", f(15)); + return 0; +-} ++} +\ No newline at end of file + +commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2 +Author: Thomas Rast +Date: Thu Feb 28 10:45:16 2013 +0100 + + touch both functions + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -17,5 +17,5 @@ + int main () + { +- printf("%d\n", f(15)); ++ printf("%ld\n", f(15)); + return 0; + } + +commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a +Author: Thomas Rast +Date: Thu Feb 28 10:44:48 2013 +0100 + + initial + +diff --git a/a.c b/a.c +--- /dev/null ++++ b/a.c +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} diff --git a/t/t4211/sha256/expect.no-assertion-error b/t/t4211/sha256/expect.no-assertion-error new file mode 100644 index 00000000000000..36ed12aa9cb8b9 --- /dev/null +++ b/t/t4211/sha256/expect.no-assertion-error @@ -0,0 +1,90 @@ +commit eb871b8aa9aff323e484723039c9a92ab0266e060bc0ef2afb08fadda25c5ace +Author: Thomas Rast +Date: Thu Feb 28 10:50:24 2013 +0100 + + move within the file + +diff --git a/b.c b/b.c +--- a/b.c ++++ b/b.c +@@ -25,0 +18,9 @@ ++long f(long x) ++{ ++ int s = 0; ++ while (x) { ++ x /= 2; ++ s++; ++ } ++ return s; ++} + +commit 5526ed05c2476b56af8b7be499e8f78bd50f490740733a9ca7e1f55878fa90a9 +Author: Thomas Rast +Date: Thu Feb 28 10:48:43 2013 +0100 + + change back to complete line + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -18,5 +18,7 @@ + int main () + { + printf("%ld\n", f(15)); + return 0; +-} +\ No newline at end of file ++} ++ ++/* incomplete lines are bad! */ + +commit 29f32ac3141c48b22803e5c4127b719917b67d0f8ca8c5248bebfa2a19f7da10 +Author: Thomas Rast +Date: Thu Feb 28 10:48:10 2013 +0100 + + change to an incomplete line at end + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -18,5 +18,5 @@ + int main () + { + printf("%ld\n", f(15)); + return 0; +-} ++} +\ No newline at end of file + +commit ccf97b9878189c40a981da50b15713bb80a35755326320ec80900caf22ced46f +Author: Thomas Rast +Date: Thu Feb 28 10:45:16 2013 +0100 + + touch both functions + +diff --git a/a.c b/a.c +--- a/a.c ++++ b/a.c +@@ -17,5 +17,5 @@ + int main () + { +- printf("%d\n", f(15)); ++ printf("%ld\n", f(15)); + return 0; + } + +commit 1dd7e9b2b1699324b53b341e728653b913bc192a14dfea168c5b51f2b3d03592 +Author: Thomas Rast +Date: Thu Feb 28 10:44:48 2013 +0100 + + initial + +diff --git a/a.c b/a.c +--- /dev/null ++++ b/a.c +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} From e3106998ff84c9e1b548b72e0b034ca837d6c06b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 18 Aug 2025 13:13:10 +0200 Subject: [PATCH 05/16] line-log: show all line ranges touched by the same diff range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When line-level log is invoked with more than one disjoint line range in the same file, and one of the commits happens to change that file such that one diff range modifies more than one line range, then changes to all modified line ranges should be shown, but only the changes in the first modified line range are: $ git log --oneline -p 80ca903 (HEAD -> master) Initial diff --git a/file b/file new file mode 100644 index 0000000..00935f1 --- /dev/null +++ b/file @@ -0,0 +1,10 @@ +Line 1 +Line 2 +Line 3 +Line 4 +Line 5 +Line 6 +Line 7 +Line 8 +Line 9 +Line 10 $ git log --oneline -L1,2:file -L4,5:file -L7,8:file 80ca903 (HEAD -> master) Initial diff --git a/file b/file --- /dev/null +++ b/file @@ -0,0 +1,2 @@ +Line 1 +Line 2 The line-log-specific diff printer is already clever enough to handle the case when one line range covers multiple diff ranges, but the possibility of one diff range touching multiple disjoint line ranges was apparently overlooked. Add the necessary condition to dump_diff_hacky_one() to handle this case as well, and show all modified line ranges: $ git log --oneline -L1,2:file -L4,5:file -L7,8:file 0f9a5b4 (HEAD -> master) Initial diff --git a/file b/file --- /dev/null +++ b/file @@ -0,0 +1,2 @@ +Line 1 +Line 2 @@ -0,0 +4,2 @@ +Line 4 +Line 5 @@ -0,0 +7,2 @@ +Line 7 +Line 8 This bug was already present in the initial line-log implementation added in 2da1d1f6f (Implement line-history search (git log -L), 2013-03-28). Interestingly, that commit already contained a canned test case covering a similar scenario: "-L '/long f/',/^}/:a.c -L /main/,/^}/:a.c simple" This test case looks for two line ranges in the same file, and both trace back disjointly to the test repository's inital commit, therefore changes to both line ranges should have been shown for the initial commit, but only changes for the first line range are shown. So this test case should have failed from the very beginning, but it never did, because, unfortunately, the canned expected result is incorrect, as it doesn't include changes for the second line range. A similar test with a similarly incorrect canned expected result was added later in 209618860c (log -L: fix overlapping input ranges, 2013-04-05). Correct these two canned expected results to contain the changes for the second line range for the initial commit as well. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- line-log.c | 9 +++++++++ t/t4211/sha1/expect.multiple | 6 ++++++ t/t4211/sha1/expect.two-ranges | 6 ++++++ t/t4211/sha256/expect.multiple | 6 ++++++ t/t4211/sha256/expect.two-ranges | 6 ++++++ 5 files changed, 33 insertions(+) diff --git a/line-log.c b/line-log.c index 9cb8ea1484ae84..9b75b4ac39a1eb 100644 --- a/line-log.c +++ b/line-log.c @@ -939,6 +939,15 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang long t_cur = t_start; unsigned int j_last; + /* + * If a diff range touches multiple line ranges, then all + * those line ranges should be shown, so take a step back if + * the current line range is still in the previous diff range + * (even if only partially). + */ + if (j > 0 && diff->target.ranges[j-1].end > t_start) + j--; + while (j < diff->target.nr && diff->target.ranges[j].end < t_start) j++; if (j == diff->target.nr || diff->target.ranges[j].start >= t_end) diff --git a/t/t4211/sha1/expect.multiple b/t/t4211/sha1/expect.multiple index 76ad5b598cb8dd..1eee8a7801055d 100644 --- a/t/t4211/sha1/expect.multiple +++ b/t/t4211/sha1/expect.multiple @@ -102,3 +102,9 @@ diff --git a/a.c b/a.c + s++; + } +} +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} diff --git a/t/t4211/sha1/expect.two-ranges b/t/t4211/sha1/expect.two-ranges index 6109aa0dcee759..c5164f3be3a287 100644 --- a/t/t4211/sha1/expect.two-ranges +++ b/t/t4211/sha1/expect.two-ranges @@ -100,3 +100,9 @@ diff --git a/a.c b/a.c + s++; + } +} +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} diff --git a/t/t4211/sha256/expect.multiple b/t/t4211/sha256/expect.multiple index ca00409b9a39b2..dbd987b74a451f 100644 --- a/t/t4211/sha256/expect.multiple +++ b/t/t4211/sha256/expect.multiple @@ -102,3 +102,9 @@ diff --git a/a.c b/a.c + s++; + } +} +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} diff --git a/t/t4211/sha256/expect.two-ranges b/t/t4211/sha256/expect.two-ranges index af57c8b9978383..6a94d3b9cba2dd 100644 --- a/t/t4211/sha256/expect.two-ranges +++ b/t/t4211/sha256/expect.two-ranges @@ -100,3 +100,9 @@ diff --git a/a.c b/a.c + s++; + } +} +@@ -0,0 +16,5 @@ ++int main () ++{ ++ printf("%d\n", f(15)); ++ return 0; ++} From ac7096723b6a42c177ffb2fbe1c4f4c5dc59e752 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 21 Aug 2025 08:06:52 -0700 Subject: [PATCH 06/16] config: document includeIf conditions consistently When 399b1984 (config: include file if remote URL matches a glob, 2022-01-18) added the 'hasconfig:remote.*.url:' condition to be used in the "includeIf..path" configuration, the keyword was added with an extra colon in the documentation. The section that documents these condition begins with this preamble: The condition starts with a keyword followed by a colon and some data whose format and meaning depends on the keyword. Supported keywords are: which makes it clear that the colon that comes between the condition keyword (e.g. "gitdir") and the parameter (aka "some data") is not a part of the keyword. Lose the extra colon. Also rewrite description of all keywords to clarify that "some data" does not directly follow "keyword", and the colon is not a part of keyword. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 8c0b3ed8075214..ee1f80cad6191f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -114,8 +114,7 @@ whose format and meaning depends on the keyword. Supported keywords are: `gitdir`:: - - The data that follows the keyword `gitdir:` is used as a glob + The data that follows the keyword `gitdir` and a colon is used as a glob pattern. If the location of the .git directory matches the pattern, the include condition is met. + @@ -148,7 +147,7 @@ refer to linkgit:gitignore[5] for details. For convenience: case-insensitively (e.g. on case-insensitive file systems) `onbranch`:: - The data that follows the keyword `onbranch:` is taken to be a + The data that follows the keyword `onbranch` and a colon is taken to be a pattern with standard globbing wildcards and two additional ones, `**/` and `/**`, that can match multiple path components. If we are in a worktree where the name of the branch that is @@ -161,8 +160,8 @@ all branches that begin with `foo/`. This is useful if your branches are organized hierarchically and you would like to apply a configuration to all the branches in that hierarchy. -`hasconfig:remote.*.url:`:: - The data that follows this keyword is taken to +`hasconfig:remote.*.url`:: + The data that follows this keyword and a colon is taken to be a pattern with standard globbing wildcards and two additional ones, `**/` and `/**`, that can match multiple components. The first time this keyword is seen, the rest of From fdae4114a696014b6bf28ad9b1bc076bd8d7eec8 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 26 Aug 2025 14:35:26 +0100 Subject: [PATCH 07/16] breaking-changes: deprecate support for core.commentString=auto When "core.commentString" is set to "auto" then "git commit" will automatically select the comment character ensuring that it is not the first character on any of the lines in the commit message. This was introduced by commit 84c9dc2c5a2 (commit: allow core.commentChar=auto for character auto selection, 2014-05-17). The motivation seems to be to avoid commenting out lines from the existing message when amending a commit that was created with a message from a file. Unfortunately this feature does not work with: * commit message templates that contain comments. * prepare-commit-msg hooks that introduce comments. * "git commit --cleanup=strip --edit -F " which means that it is incompatible with - the "fixup" and "squash" commands of "git rebase -i" as the comments added by those commands are then treated as part of the commit message. - the conflict comments added to the commit message by "git cherry-pick", "git rebase" etc. as these comments are then treated as part of the commit message. It is also ignored by "git notes" when amending a note. The issues with comments coming from a template, hook or file are a consequence of the design of this feature and are therefore hard to fix. As the costs of this feature outweigh the benefits, deprecate it and remove it in Git 3.0. If someone comes up with some patches that fix all the issues in a maintainable way then I'd be happy to see this change reverted. The next commits will add a warning and some advice for users on how they can update their config settings. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- Documentation/BreakingChanges.adoc | 5 +++++ Documentation/config/core.adoc | 20 ++++++++++++++++++-- builtin/commit.c | 4 ++++ environment.c | 10 ++++++++-- environment.h | 2 ++ t/t3404-rebase-interactive.sh | 2 +- t/t3418-rebase-continue.sh | 2 +- t/t7502-commit-porcelain.sh | 4 ++-- 8 files changed, 41 insertions(+), 8 deletions(-) diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc index f8d2eba061c82a..344ce5006031ce 100644 --- a/Documentation/BreakingChanges.adoc +++ b/Documentation/BreakingChanges.adoc @@ -239,6 +239,11 @@ These features will be removed. + The command will be removed. +* Support for `core.commentString=auto` has been deprecated and will + be removed in Git 3.0. ++ +cf. + == Superseded features that will not be deprecated Some features have gained newer replacements that aim to improve the design in diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc index 9fde1ab63a70ea..7133f00c38bdfa 100644 --- a/Documentation/config/core.adoc +++ b/Documentation/config/core.adoc @@ -531,9 +531,25 @@ core.commentString:: commented, and removes them after the editor returns (default '#'). + -If set to "auto", `git-commit` would select a character that is not +ifndef::with-breaking-changes[] +If set to "auto", `git-commit` will select a character that is not the beginning character of any line in existing commit messages. -+ +Support for this value is deprecated and will be removed in Git 3.0 +due to the following limitations: ++ +-- +* It is incompatible with adding comments in a commit message + template. This includes the conflicts comments added to + the commit message by `cherry-pick`, `merge`, `rebase` and + `revert`. +* It is incompatible with adding comments to the commit message + in the `prepare-commit-msg` hook. +* It is incompatible with the `fixup` and `squash` commands when + rebasing, +* It is not respected by `git notes` +-- ++ +endif::with-breaking-changes[] Note that these two variables are aliases of each other, and in modern versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with `commentChar`. Versions of Git prior to v2.45.0 will ignore diff --git a/builtin/commit.c b/builtin/commit.c index 757f51eac820a9..d25cc07a355aaa 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -683,6 +683,7 @@ static int author_date_is_interesting(void) return author_message || force_date; } +#ifndef WITH_BREAKING_CHANGES static void adjust_comment_line_char(const struct strbuf *sb) { char candidates[] = "#;@!$%^&|:"; @@ -720,6 +721,7 @@ static void adjust_comment_line_char(const struct strbuf *sb) free(comment_line_str_to_free); comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p); } +#endif /* !WITH_BREAKING_CHANGES */ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb, struct pretty_print_context *ctx) @@ -916,8 +918,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) die_errno(_("could not write commit template")); +#ifndef WITH_BREAKING_CHANGES if (auto_comment_line_char) adjust_comment_line_char(&sb); +#endif /* !WITH_BREAKING_CHANGES */ strbuf_release(&sb); /* This checks if committer ident is explicitly given */ diff --git a/environment.c b/environment.c index a0ac5934b37b30..4c87876d483143 100644 --- a/environment.c +++ b/environment.c @@ -122,7 +122,9 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT; */ const char *comment_line_str = "#"; char *comment_line_str_to_free; +#ifndef WITH_BREAKING_CHANGES int auto_comment_line_char; +#endif /* !WITH_BREAKING_CHANGES */ /* This is set by setup_git_directory_gently() and/or git_default_config() */ char *git_work_tree_cfg; @@ -459,18 +461,22 @@ static int git_default_core_config(const char *var, const char *value, if (!strcmp(var, "core.commentchar") || !strcmp(var, "core.commentstring")) { - if (!value) + if (!value) { return config_error_nonbool(var); - else if (!strcasecmp(value, "auto")) { +#ifndef WITH_BREAKING_CHANGES + } else if (!strcasecmp(value, "auto")) { auto_comment_line_char = 1; FREE_AND_NULL(comment_line_str_to_free); comment_line_str = "#"; +#endif /* !WITH_BREAKING_CHANGES */ } else if (value[0]) { if (strchr(value, '\n')) return error(_("%s cannot contain newline"), var); comment_line_str = value; FREE_AND_NULL(comment_line_str_to_free); +#ifndef WITH_BREAKING_CHANGES auto_comment_line_char = 0; +#endif /* !WITH_BREAKING_CHANGES */ } else return error(_("%s must have at least one character"), var); return 0; diff --git a/environment.h b/environment.h index 8cfce41015b3c8..e75c4abb388670 100644 --- a/environment.h +++ b/environment.h @@ -208,7 +208,9 @@ extern char *excludes_file; */ extern const char *comment_line_str; extern char *comment_line_str_to_free; +#ifndef WITH_BREAKING_CHANGES extern int auto_comment_line_char; +#endif /* !WITH_BREAKING_CHANGES */ # endif /* USE_THE_REPOSITORY_VARIABLE */ #endif /* ENVIRONMENT_H */ diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 6bac217ed3555e..ce0aebb9a7ec7d 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1176,7 +1176,7 @@ test_expect_success 'rebase -i respects core.commentchar' ' test B = $(git cat-file commit HEAD^ | sed -ne \$p) ' -test_expect_success 'rebase -i respects core.commentchar=auto' ' +test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar=auto' ' test_config core.commentchar auto && write_script copy-edit-script.sh <<-\EOF && cp "$1" edit-script diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index b8a8dd77e74408..f9b8999db50f1b 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -328,7 +328,7 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec ' -test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' ' +test_expect_success !WITH_BREAKING_CHANGES 'no change in comment character due to conflicts markers with core.commentChar=auto' ' git checkout -b branch-a && test_commit A F1 && git checkout -b branch-b HEAD^ && diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index b37e2018a74a7b..65b4519a715094 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -956,13 +956,13 @@ test_expect_success 'commit --status with custom comment character' ' test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG ' -test_expect_success 'switch core.commentchar' ' +test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' ' test_commit "#foo" foo && GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend && test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG ' -test_expect_success 'switch core.commentchar but out of options' ' +test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar but out of options' ' cat >text <<\EOF && # 1 ; 2 From a0e6aaea7da5134bdc784c6d68d4cc2125865330 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 26 Aug 2025 14:35:27 +0100 Subject: [PATCH 08/16] config: warn on core.commentString=auto As support for this setting was deprecated in the last commit print a warning (or die when WITH_BREAKING_CHANGES is enabled) if it is set. Avoid bombarding the user with warnings by only printing it (a) when running commands that call "git commit" and (b) only once per command. Some scaffolding is added to repo_read_config() to allow it to detect deprecated config settings and warn about them. As both "core.commentChar" and "core.commentString" set the comment character we record which one of them is used and tailor the warning message appropriately. Note the odd combination of die_message() followed by die(NULL) is to allow the next commit to insert a call to advise() in the middle. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 3 + builtin/merge.c | 3 + builtin/rebase.c | 3 + builtin/revert.c | 7 +++ config.c | 115 +++++++++++++++++++++++++++++++++- environment.c | 1 + environment.h | 1 + repository.c | 1 + repository.h | 3 + t/t3404-rebase-interactive.sh | 7 ++- t/t7502-commit-porcelain.sh | 17 ++++- 11 files changed, 157 insertions(+), 4 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index d25cc07a355aaa..f821fdcfcc3560 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1783,6 +1783,9 @@ int cmd_commit(int argc, show_usage_with_options_if_asked(argc, argv, builtin_commit_usage, builtin_commit_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0; diff --git a/builtin/merge.c b/builtin/merge.c index dc4cb8fb14dbf3..794cb7bb269eb1 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1378,6 +1378,9 @@ int cmd_merge(int argc, show_usage_with_options_if_asked(argc, argv, builtin_merge_usage, builtin_merge_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0; diff --git a/builtin/rebase.c b/builtin/rebase.c index 72a52bdfb9872e..962917ec48097a 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1242,6 +1242,9 @@ int cmd_rebase(int argc, builtin_rebase_usage, builtin_rebase_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0; diff --git a/builtin/revert.c b/builtin/revert.c index e07c2217fe846b..b197848bb0a475 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -4,6 +4,7 @@ #include "builtin.h" #include "parse-options.h" #include "diff.h" +#include "environment.h" #include "gettext.h" #include "revision.h" #include "rerere.h" @@ -285,6 +286,9 @@ int cmd_revert(int argc, struct replay_opts opts = REPLAY_OPTS_INIT; int res; +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ opts.action = REPLAY_REVERT; sequencer_init_config(&opts); res = run_sequencer(argc, argv, prefix, &opts); @@ -302,6 +306,9 @@ struct repository *repo UNUSED) struct replay_opts opts = REPLAY_OPTS_INIT; int res; +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ opts.action = REPLAY_PICK; sequencer_init_config(&opts); res = run_sequencer(argc, argv, prefix, &opts); diff --git a/config.c b/config.c index 97ffef42700111..18b42197095a00 100644 --- a/config.c +++ b/config.c @@ -11,6 +11,7 @@ #include "date.h" #include "branch.h" #include "config.h" +#include "dir.h" #include "parse.h" #include "convert.h" #include "environment.h" @@ -1951,10 +1952,110 @@ int git_configset_get_pathname(struct config_set *set, const char *key, char **d return 1; } +struct comment_char_config { + unsigned last_key_id; + bool auto_set; +}; + +#define COMMENT_CHAR_CFG_INIT { 0 } + +static const char *comment_key_name(unsigned id) +{ + static const char *name[] = { + "core.commentChar", + "core.commentString", + }; + + if (id >= ARRAY_SIZE(name)) + BUG("invalid comment key id"); + + return name[id]; +} + +static void comment_char_callback(const char *key, const char *value, + const struct config_context *ctx UNUSED, + void *data) +{ + struct comment_char_config *config = data; + unsigned key_id; + + if (!strcmp(key, "core.commentchar")) + key_id = 0; + else if (!strcmp(key, "core.commentstring")) + key_id = 1; + else + return; + + config->last_key_id = key_id; + config->auto_set = value && !strcmp(value, "auto"); +} + +struct repo_config { + struct repository *repo; + struct comment_char_config comment_char_config; +}; + +#define REPO_CONFIG_INIT(repo_) { \ + .comment_char_config = COMMENT_CHAR_CFG_INIT, \ + .repo = repo_, \ + }; + +#ifdef WITH_BREAKING_CHANGES +static void check_auto_comment_char_config(struct comment_char_config *config) +{ + if (!config->auto_set) + return; + + die_message(_("Support for '%s=auto' has been removed in Git 3.0"), + comment_key_name(config->last_key_id)); + die(NULL); +} +#else +static void check_auto_comment_char_config(struct comment_char_config *config) +{ + extern bool warn_on_auto_comment_char; + const char *DEPRECATED_CONFIG_ENV = + "GIT_AUTO_COMMENT_CHAR_CONFIG_WARNING_GIVEN"; + + if (!config->auto_set || !warn_on_auto_comment_char) + return; + + /* + * Use an environment variable to ensure that subprocesses do not repeat + * the warning. + */ + if (git_env_bool(DEPRECATED_CONFIG_ENV, false)) + return; + + setenv(DEPRECATED_CONFIG_ENV, "true", true); + + warning(_("Support for '%s=auto' is deprecated and will be removed in " + "Git 3.0"), comment_key_name(config->last_key_id)); +} +#endif /* WITH_BREAKING_CHANGES */ + +static void check_deprecated_config(struct repo_config *config) +{ + if (!config->repo->check_deprecated_config) + return; + + check_auto_comment_char_config(&config->comment_char_config); +} + +static int repo_config_callback(const char *key, const char *value, + const struct config_context *ctx, void *data) +{ + struct repo_config *config = data; + + comment_char_callback(key, value, ctx, &config->comment_char_config); + return config_set_callback(key, value, ctx, config->repo->config); +} + /* Functions use to read configuration from a repository */ static void repo_read_config(struct repository *repo) { struct config_options opts = { 0 }; + struct repo_config config = REPO_CONFIG_INIT(repo); opts.respect_includes = 1; opts.commondir = repo->commondir; @@ -1966,8 +2067,8 @@ static void repo_read_config(struct repository *repo) git_configset_clear(repo->config); git_configset_init(repo->config); - if (config_with_options(config_set_callback, repo->config, NULL, - repo, &opts) < 0) + if (config_with_options(repo_config_callback, &config, NULL, repo, + &opts) < 0) /* * config_with_options() normally returns only * zero, as most errors are fatal, and @@ -1980,6 +2081,7 @@ static void repo_read_config(struct repository *repo) * immediately. */ die(_("unknown error occurred while reading the configuration files")); + check_deprecated_config(&config); } static void git_config_check_init(struct repository *repo) @@ -2667,6 +2769,14 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, char *contents = NULL; size_t contents_sz; struct config_store_data store = CONFIG_STORE_INIT; + bool saved_check_deprecated_config = r->check_deprecated_config; + + /* + * Do not warn or die if there are deprecated config settings as + * we want the user to be able to change those settings by running + * "git config". + */ + r->check_deprecated_config = false; validate_comment_string(comment); @@ -2898,6 +3008,7 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, if (in_fd >= 0) close(in_fd); config_store_data_clear(&store); + r->check_deprecated_config = saved_check_deprecated_config; return ret; write_err_out: diff --git a/environment.c b/environment.c index 4c87876d483143..1ffa2ff30b2345 100644 --- a/environment.c +++ b/environment.c @@ -124,6 +124,7 @@ const char *comment_line_str = "#"; char *comment_line_str_to_free; #ifndef WITH_BREAKING_CHANGES int auto_comment_line_char; +bool warn_on_auto_comment_char; #endif /* !WITH_BREAKING_CHANGES */ /* This is set by setup_git_directory_gently() and/or git_default_config() */ diff --git a/environment.h b/environment.h index e75c4abb388670..51898c99cd1e45 100644 --- a/environment.h +++ b/environment.h @@ -210,6 +210,7 @@ extern const char *comment_line_str; extern char *comment_line_str_to_free; #ifndef WITH_BREAKING_CHANGES extern int auto_comment_line_char; +extern bool warn_on_auto_comment_char; #endif /* !WITH_BREAKING_CHANGES */ # endif /* USE_THE_REPOSITORY_VARIABLE */ diff --git a/repository.c b/repository.c index ecd691181fc97d..8af73923d344b9 100644 --- a/repository.c +++ b/repository.c @@ -57,6 +57,7 @@ void initialize_repository(struct repository *repo) repo->parsed_objects = parsed_object_pool_new(repo); ALLOC_ARRAY(repo->index, 1); index_state_init(repo->index, repo); + repo->check_deprecated_config = true; /* * When a command runs inside a repository, it learns what diff --git a/repository.h b/repository.h index 042dc93f0f2f49..5808a5d610846a 100644 --- a/repository.h +++ b/repository.h @@ -161,6 +161,9 @@ struct repository { /* Indicate if a repository has a different 'commondir' from 'gitdir' */ unsigned different_commondir:1; + + /* Should repo_config() check for deprecated settings */ + bool check_deprecated_config; }; #ifdef USE_THE_REPOSITORY_VARIABLE diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ce0aebb9a7ec7d..3b2a46c25ce69f 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1184,8 +1184,13 @@ test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar= test_when_finished "git rebase --abort || :" && ( test_set_editor "$(pwd)/copy-edit-script.sh" && - git rebase -i HEAD^ + git rebase -i HEAD^ 2>err ) && + sed -n "s/^warning: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + EOF + test_cmp expect actual && test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)" ' diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index 65b4519a715094..a9dc1e416d1947 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -958,7 +958,12 @@ test_expect_success 'commit --status with custom comment character' ' test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' ' test_commit "#foo" foo && - GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend && + GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend 2>err && + sed -n "s/^warning: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + EOF + test_cmp expect actual && test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG ' @@ -982,4 +987,14 @@ EOF ) ' +test_expect_success WITH_BREAKING_CHANGES 'core.commentChar=auto is rejected' ' + test_config core.commentChar auto && + test_must_fail git rev-parse --git-dir 2>err && + sed -n "s/^fatal: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} has been removed in Git 3.0 + EOF + test_cmp expect actual +' + test_done From ace1bb71503bc53b42ddfd68435c3af0adaf390f Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 26 Aug 2025 14:35:28 +0100 Subject: [PATCH 09/16] commit: print advice when core.commentString=auto Add some advice on how to change the config settings when "core.commentString=auto" or "core.commentChar=auto". The advice includes instructions for clearing the config setting or setting a fixed comment string. To try and be as specific as possible, the advice is customized based on the user's config. If "core.commentString=auto" is set in the system config and the user does not have write access then the advice omits the instructions to clear the config and recommends changing the global config instead. An alternative approach would be to advise the user to run "git config --show-origin" and leave them to figure out how to fix it themselves but that seems rather unfriendly. As we're forcing them to update their config we should try and make that as easy as possible. In order to generate this advice we need to record each file where either of the config keys is set and whether a key occurs more that once in a given file. This lets us generate the list of commands to remove all the keys and also tells us which key the "auto" setting comes from. As we want the user to update their config we do not provide a way for this advice to be disabled other than changing the value of "core.commentChar" or "core.commentString". Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- config.c | 194 ++++++++++++++++++++++++++++++++-- t/t3404-rebase-interactive.sh | 12 ++- t/t7502-commit-porcelain.sh | 37 ++++++- 3 files changed, 233 insertions(+), 10 deletions(-) diff --git a/config.c b/config.c index 18b42197095a00..18dcf341d588b2 100644 --- a/config.c +++ b/config.c @@ -8,6 +8,7 @@ #include "git-compat-util.h" #include "abspath.h" +#include "advice.h" #include "date.h" #include "branch.h" #include "config.h" @@ -1955,9 +1956,51 @@ int git_configset_get_pathname(struct config_set *set, const char *key, char **d struct comment_char_config { unsigned last_key_id; bool auto_set; + bool auto_set_in_file; + struct strintmap key_flags; + size_t alloc, nr; + struct comment_char_config_item { + unsigned key_id; + char *path; + enum config_scope scope; + } *item; }; -#define COMMENT_CHAR_CFG_INIT { 0 } +#define COMMENT_CHAR_CFG_INIT { \ + .key_flags = STRINTMAP_INIT, \ + } + +static void comment_char_config_release(struct comment_char_config *config) +{ + strintmap_clear(&config->key_flags); + for (size_t i = 0; i < config->nr; i++) + free(config->item[i].path); + free(config->item); +} + +/* Used to track whether the key occurs more than once in a given file */ +#define KEY_SEEN_ONCE 1u +#define KEY_SEEN_TWICE 2u +#define COMMENT_KEY_SHIFT(id) (2 * (id)) +#define COMMENT_KEY_MASK(id) (3u << COMMENT_KEY_SHIFT(id)) + +static void set_comment_key_flags(struct comment_char_config *config, + const char *path, unsigned id, unsigned value) +{ + unsigned old = strintmap_get(&config->key_flags, path); + unsigned new = (old & ~COMMENT_KEY_MASK(id)) | + value << COMMENT_KEY_SHIFT(id); + + strintmap_set(&config->key_flags, path, new); +} + +static unsigned get_comment_key_flags(struct comment_char_config *config, + const char *path, unsigned id) +{ + unsigned value = strintmap_get(&config->key_flags, path); + + return (value & COMMENT_KEY_MASK(id)) >> COMMENT_KEY_SHIFT(id); +} static const char *comment_key_name(unsigned id) { @@ -1973,10 +2016,10 @@ static const char *comment_key_name(unsigned id) } static void comment_char_callback(const char *key, const char *value, - const struct config_context *ctx UNUSED, - void *data) + const struct config_context *ctx, void *data) { struct comment_char_config *config = data; + const struct key_value_info *kvi = ctx->kvi; unsigned key_id; if (!strcmp(key, "core.commentchar")) @@ -1988,8 +2031,136 @@ static void comment_char_callback(const char *key, const char *value, config->last_key_id = key_id; config->auto_set = value && !strcmp(value, "auto"); + if (kvi->origin_type != CONFIG_ORIGIN_FILE) { + return; + } else if (get_comment_key_flags(config, kvi->filename, key_id)) { + set_comment_key_flags(config, kvi->filename, key_id, + KEY_SEEN_TWICE); + } else { + struct comment_char_config_item *item; + + ALLOC_GROW_BY(config->item, config->nr, 1, config->alloc); + item = &config->item[config->nr - 1]; + item->key_id = key_id; + item->scope = kvi->scope; + item->path = xstrdup(kvi->filename); + set_comment_key_flags(config, kvi->filename, key_id, + KEY_SEEN_ONCE); + } + config->auto_set_in_file = config->auto_set; } +static void add_config_scope_arg(struct repository *repo, struct strbuf *buf, + struct comment_char_config_item *item) +{ + char *global_config = git_global_config(); + char *system_config = git_system_config(); + + if (item->scope == CONFIG_SCOPE_SYSTEM && access(item->path, W_OK)) { + /* + * If the user cannot write to the system config recommend + * setting the global config instead. + */ + strbuf_addstr(buf, "--global "); + } else if (fspatheq(item->path, system_config)) { + strbuf_addstr(buf, "--system "); + } else if (fspatheq(item->path, global_config)) { + strbuf_addstr(buf, "--global "); + } else if (fspatheq(item->path, + mkpath("%s/config", + repo_get_git_dir(repo)))) { + ; /* --local is the default */ + } else if (fspatheq(item->path, + mkpath("%s/config.worktree", + repo_get_common_dir(repo)))) { + strbuf_addstr(buf, "--worktree "); + } else { + const char *path = item->path; + const char *home = getenv("HOME"); + + strbuf_addstr(buf, "--file "); + if (home && !fspathncmp(path, home, strlen(home))) { + path += strlen(home); + if (!fspathncmp(path, "/", 1)) + path++; + strbuf_addstr(buf, "~/"); + } + sq_quote_buf_pretty(buf, path); + strbuf_addch(buf, ' '); + } + + free(global_config); + free(system_config); +} + +static bool can_unset_comment_char_config(struct comment_char_config *config) +{ + for (size_t i = 0; i < config->nr; i++) { + struct comment_char_config_item *item = &config->item[i]; + + if (item->scope == CONFIG_SCOPE_SYSTEM && + access(item->path, W_OK)) + return false; + } + + return true; +} + +static void add_unset_auto_comment_char_advice(struct repository *repo, + struct comment_char_config *config) +{ + struct strbuf buf = STRBUF_INIT; + + if (!can_unset_comment_char_config(config)) + return; + + for (size_t i = 0; i < config->nr; i++) { + struct comment_char_config_item *item = &config->item[i]; + + strbuf_addstr(&buf, " git config unset "); + add_config_scope_arg(repo, &buf, item); + if (get_comment_key_flags(config, item->path, item->key_id) == KEY_SEEN_TWICE) + strbuf_addstr(&buf, "--all "); + strbuf_addf(&buf, "%s\n", comment_key_name(item->key_id)); + } + advise(_("\nTo use the default comment string (#) please run\n\n%s"), + buf.buf); + strbuf_release(&buf); +} + +static void add_comment_char_advice(struct repository *repo, + struct comment_char_config *config) +{ + struct strbuf buf = STRBUF_INIT; + struct comment_char_config_item *item; + /* TRANSLATORS this is a place holder for the value of core.commentString */ + const char *placeholder = _(""); + + /* + * If auto is set in the last file that we saw advise the user how to + * update their config. + */ + if (!config->auto_set_in_file) + return; + + add_unset_auto_comment_char_advice(repo, config); + item = &config->item[config->nr - 1]; + strbuf_reset(&buf); + strbuf_addstr(&buf, " git config set "); + add_config_scope_arg(repo, &buf, item); + strbuf_addf(&buf, "%s %s\n", comment_key_name(item->key_id), + placeholder); + advise(_("\nTo set a custom comment string please run\n\n" + "%s\nwhere '%s' is the string you wish to use.\n"), + buf.buf, placeholder); + strbuf_release(&buf); +} + +#undef KEY_SEEN_ONCE +#undef KEY_SEEN_TWICE +#undef COMMENT_KEY_SHIFT +#undef COMMENT_KEY_MASK + struct repo_config { struct repository *repo; struct comment_char_config comment_char_config; @@ -2000,18 +2171,26 @@ struct repo_config { .repo = repo_, \ }; +static void repo_config_release(struct repo_config *config) +{ + comment_char_config_release(&config->comment_char_config); +} + #ifdef WITH_BREAKING_CHANGES -static void check_auto_comment_char_config(struct comment_char_config *config) +static void check_auto_comment_char_config(struct repository *repo, + struct comment_char_config *config) { if (!config->auto_set) return; die_message(_("Support for '%s=auto' has been removed in Git 3.0"), comment_key_name(config->last_key_id)); + add_comment_char_advice(repo, config); die(NULL); } #else -static void check_auto_comment_char_config(struct comment_char_config *config) +static void check_auto_comment_char_config(struct repository *repo, + struct comment_char_config *config) { extern bool warn_on_auto_comment_char; const char *DEPRECATED_CONFIG_ENV = @@ -2031,6 +2210,7 @@ static void check_auto_comment_char_config(struct comment_char_config *config) warning(_("Support for '%s=auto' is deprecated and will be removed in " "Git 3.0"), comment_key_name(config->last_key_id)); + add_comment_char_advice(repo, config); } #endif /* WITH_BREAKING_CHANGES */ @@ -2039,7 +2219,8 @@ static void check_deprecated_config(struct repo_config *config) if (!config->repo->check_deprecated_config) return; - check_auto_comment_char_config(&config->comment_char_config); + check_auto_comment_char_config(config->repo, + &config->comment_char_config); } static int repo_config_callback(const char *key, const char *value, @@ -2082,6 +2263,7 @@ static void repo_read_config(struct repository *repo) */ die(_("unknown error occurred while reading the configuration files")); check_deprecated_config(&config); + repo_config_release(&config); } static void git_config_check_init(struct repository *repo) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 3b2a46c25ce69f..cc97628d81012e 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1186,9 +1186,19 @@ test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar= test_set_editor "$(pwd)/copy-edit-script.sh" && git rebase -i HEAD^ 2>err ) && - sed -n "s/^warning: //p" err >actual && + sed -n "s/^hint: *\$//p; s/^hint: //p; s/^warning: //p" err >actual && cat >expect <<-EOF && Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + + To use the default comment string (#) please run + + git config unset core.commentChar + + To set a custom comment string please run + + git config set core.commentChar + + where ${SQ}${SQ} is the string you wish to use. EOF test_cmp expect actual && test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)" diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index a9dc1e416d1947..05f6da4ad98448 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -958,10 +958,31 @@ test_expect_success 'commit --status with custom comment character' ' test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' ' test_commit "#foo" foo && - GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend 2>err && - sed -n "s/^warning: //p" err >actual && + cat >config-include <<-\EOF && + [core] + commentString=: + commentString=% + commentChar=auto + EOF + test_when_finished "rm config-include" && + test_config include.path "$(pwd)/config-include" && + test_config core.commentChar ! && + GIT_EDITOR=.git/FAKE_EDITOR git commit --amend 2>err && + sed -n "s/^hint: *\$//p; s/^hint: //p; s/^warning: //p" err >actual && cat >expect <<-EOF && Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + + To use the default comment string (#) please run + + git config unset core.commentChar + git config unset --file ~/config-include --all core.commentString + git config unset --file ~/config-include core.commentChar + + To set a custom comment string please run + + git config set --file ~/config-include core.commentChar + + where ${SQ}${SQ} is the string you wish to use. EOF test_cmp expect actual && test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG @@ -990,9 +1011,19 @@ EOF test_expect_success WITH_BREAKING_CHANGES 'core.commentChar=auto is rejected' ' test_config core.commentChar auto && test_must_fail git rev-parse --git-dir 2>err && - sed -n "s/^fatal: //p" err >actual && + sed -n "s/^hint: *\$//p; s/^hint: //p; s/^fatal: //p" err >actual && cat >expect <<-EOF && Support for ${SQ}core.commentChar=auto${SQ} has been removed in Git 3.0 + + To use the default comment string (#) please run + + git config unset core.commentChar + + To set a custom comment string please run + + git config set core.commentChar + + where ${SQ}${SQ} is the string you wish to use. EOF test_cmp expect actual ' From fafc9b08b83fb4510aad7226739d87ac810362d2 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Tue, 26 Aug 2025 15:09:22 +0000 Subject: [PATCH 10/16] docs: update sendmail docs to use more secure SMTP server for Gmail Earlier recommendation by IETF with RFC 2595 was to deprecate implicit TLS in preference for upgrade an initially unencrypted connection with STARTTLS command. These days, however, IETF recommends that connections be made using "Implicit TLS", in preference to STARTTLS and the like, completely reversing their earlier position, in RFC8314. Update the GMail example to use the implicit TLS to match the current recommendation at port 465. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.adoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index 5335502d68fc7b..c610909a92cc3e 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -521,10 +521,10 @@ edit `~/.gitconfig` to specify your account settings: ---- [sendemail] - smtpEncryption = tls + smtpEncryption = ssl smtpServer = smtp.gmail.com smtpUser = yourname@gmail.com - smtpServerPort = 587 + smtpServerPort = 465 ---- Gmail does not allow using your regular password for `git send-email`. @@ -542,10 +542,10 @@ if you want to use `OAUTHBEARER`, edit your `~/.gitconfig` file and add ---- [sendemail] - smtpEncryption = tls + smtpEncryption = ssl smtpServer = smtp.gmail.com smtpUser = yourname@gmail.com - smtpServerPort = 587 + smtpServerPort = 465 smtpAuth = OAUTHBEARER ---- From 929b1d08f790938e147301a61c2dee4253cc3fa5 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 26 Aug 2025 14:19:28 +0200 Subject: [PATCH 11/16] Documentation: note styling for bit fields Our codebase uses a lot of bit field variables, generally to mark boolean type variables. While there is a formatting rule in the '.clang-format', there is no guideline specified in the 'CodingGuidelines'. Since the '.clang-format' is not yet enforced, let's also add a guideline with the same rule as mentioned in the '.clang-format', which is to not use any spaces around the colon, like so: unsigned my_field:1; unsigned other_field:1; unsigned field_with_longer_name:1; This would allow us not to modify the clang-format file, and more importantly, discourage people from doing ugly alignment with spaces, i.e. unsigned my_field : 1; unsigned other_field : 1; unsigned field_with_longer_name : 1; Helped-by: Junio C Hamano Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 224f0978a86116..df72fe01772a18 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -650,6 +650,12 @@ For C programs: cases. However, it is recommended to find a more descriptive name wherever possible to improve the readability and maintainability of the code. + - Bit fields should be defined without a space around the colon. E.g. + + unsigned my_field:1; + unsigned other_field:1; + unsigned field_with_longer_name:1; + For Perl programs: - Most of the C guidelines above apply. From 1b5a6bfff323297bef85bb36fe65e8c18cf6bc73 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 27 Aug 2025 04:07:02 -0400 Subject: [PATCH 12/16] curl: add support for curl_global_trace() components In addition to the regular trace information produced by CURLOPT_VERBOSE, recent curl versions can enable or disable tracing of specific subsystems using a call to curl_global_trace(). This level of detail may or may not be useful for us in Git as mere users of libcurl, but there's one case where we need it for a test. In t5564, we set up a socks proxy, access it with GIT_TRACE_CURL set, and expect to find socks-related messages in the output. This test is broken in the release candidates for libcurl 8.16, as those socks messages are no longer produced in the trace. The problem bisects to curl's commit ab5e0bfddc (pytest: add SOCKS tests and scoring, 2025-07-21). There the socks messages were moved from generic infof() messages to the component-specific CURL_TRC_CF() system. And so we do not see them by default, but only if "socks" is enabled as a logging component. Teach Git's http code to accept a component list from the environment and pass it into curl_global_trace(). We can then use that in the test to enable the correct component. It should be safe to do so unconditionally. In older versions of curl which don't support this call, setting the environment variable is a noop. Likewise, any versions of curl which don't recognize the "socks" component should silently ignore it. The manpage for curl_global_trace() says this: The config string is a list of comma-separated component names. Names are case-insensitive and unknown names are ignored. The special name "all" applies to all components. Names may be prefixed with '+' or '-' to enable or disable detailed logging for a component. The list of component names is not part of curl's public API. Names may be added or disappear in future versions of libcurl. Since unknown names are silently ignored, outdated log configurations does not cause errors when upgrading libcurl. Given that, some names can be expected to be fairly stable and are listed below for easy reference. So this should let us make the test work on all versions without worrying about confusing older (or newer) versions. For the same reason, I've opted not to document this interface. This is deep internal voodoo for which we can make no promises to users. In fact, I was tempted to simply hard-code "socks" to let our test pass and not expose anything. But I suspect a little run-time flexibility may come in handy in the future when debugging or dealing with similar logging issues. I also considered just putting "all" into such a hard-coded default. But if you try it, you will see that many of the components are quite verbose and likely not interesting. They would clutter up our trace output if we enabled them by default. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-curl-compat.h | 7 +++++++ http.c | 8 ++++++++ t/t5564-http-proxy.sh | 4 +++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/git-curl-compat.h b/git-curl-compat.h index aa8eed7ed2b5e3..659e5a3875e3d6 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -45,6 +45,13 @@ #define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1 #endif +/** + * curl_global_trace() was added in 8.3.0, released September 2023. + */ +#if LIBCURL_VERSION_NUM >= 0x080300 +#define GIT_CURL_HAVE_GLOBAL_TRACE 1 +#endif + /** * CURLOPT_TCP_KEEPCNT was added in 8.9.0, released in July, 2024. */ diff --git a/http.c b/http.c index d88e79fbde9c4c..86254a3ebab23f 100644 --- a/http.c +++ b/http.c @@ -1347,6 +1347,14 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) die("curl_global_init failed"); +#ifdef GIT_CURL_HAVE_GLOBAL_TRACE + { + const char *comp = getenv("GIT_TRACE_CURL_COMPONENTS"); + if (comp) + curl_global_trace(comp); + } +#endif + if (proactive_auth && http_proactive_auth == PROACTIVE_AUTH_NONE) http_proactive_auth = PROACTIVE_AUTH_IF_CREDENTIALS; diff --git a/t/t5564-http-proxy.sh b/t/t5564-http-proxy.sh index b27e481f95bc4d..c3903faf2d3e6f 100755 --- a/t/t5564-http-proxy.sh +++ b/t/t5564-http-proxy.sh @@ -72,7 +72,9 @@ test_expect_success SOCKS_PROXY 'clone via Unix socket' ' test_when_finished "rm -rf clone" && test_config_global http.proxy "socks4://localhost$PWD/%2530.sock" && { { - GIT_TRACE_CURL=$PWD/trace git clone "$HTTPD_URL/smart/repo.git" clone 2>err && + GIT_TRACE_CURL=$PWD/trace \ + GIT_TRACE_CURL_COMPONENTS=socks \ + git clone "$HTTPD_URL/smart/repo.git" clone 2>err && grep -i "SOCKS4 request granted" trace } || old_libcurl_error err From 00727249ec8404c68391ec58e9c9f0d8a88d5ca0 Mon Sep 17 00:00:00 2001 From: Paulo Casaretto Date: Fri, 29 Aug 2025 16:02:54 +0000 Subject: [PATCH 13/16] range-diff: add configurable memory limit for cost matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When comparing large commit ranges (e.g., 250,000+ commits), range-diff attempts to allocate an n×n cost matrix that can exhaust available memory. For example, with 256,784 commits (n = 513,568), the matrix would require approximately 256GB of memory (513,568² × 4 bytes), causing either immediate segmentation faults due to integer overflow or system hangs. Add a memory limit check in get_correspondences() before allocating the cost matrix. This check uses the total size in bytes (n² × sizeof(int)) and compares it against a configurable maximum, preventing both excessive memory usage and integer overflow issues. The limit is configurable via a new --max-memory option that accepts human-readable sizes (e.g., "1G", "500M"). The default is 4GB for 64 bit systems and 2GB for 32 bit systems. This allows comparing ranges of approximately 32,000 (16,000) commits - generous for real-world use cases while preventing impractical operations. When the limit is exceeded, range-diff now displays a clear error message showing both the requested memory size and the maximum allowed, formatted in human-readable units for better user experience. Example usage: git range-diff --max-memory=1G branch1...branch2 git range-diff --max-memory=500M base..topic1 base..topic2 This approach was chosen over alternatives: - Pre-counting commits: Would require spawning additional git processes and reading all commits twice - Limiting by commit count: Less precise than actual memory usage - Streaming approach: Would require significant refactoring of the current algorithm This issue was previously discussed in: https://lore.kernel.org/git/RFC-cover-v2-0.5-00000000000-20211210T122901Z-avarab@gmail.com/ Acked-by: Johannes Schindelin Signed-off-by: Paulo Casaretto Signed-off-by: Junio C Hamano --- builtin/log.c | 1 + builtin/range-diff.c | 21 +++++++++++++++++++++ log-tree.c | 1 + range-diff.c | 20 ++++++++++++++++---- range-diff.h | 5 +++++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index c2f8bbf86301a9..5f552d14c0fe83 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1404,6 +1404,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, struct range_diff_options range_diff_opts = { .creation_factor = rev->creation_factor, .dual_color = 1, + .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &opts, .other_arg = &other_arg }; diff --git a/builtin/range-diff.c b/builtin/range-diff.c index a563abff5fee9b..aafcc99b96240f 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -6,6 +6,7 @@ #include "parse-options.h" #include "range-diff.h" #include "config.h" +#include "parse.h" static const char * const builtin_range_diff_usage[] = { @@ -15,6 +16,21 @@ N_("git range-diff [] "), NULL }; +static int parse_max_memory(const struct option *opt, const char *arg, int unset) +{ + size_t *max_memory = opt->value; + uintmax_t val; + + if (unset) + return 0; + + if (!git_parse_unsigned(arg, &val, SIZE_MAX)) + return error(_("invalid max-memory value: %s"), arg); + + *max_memory = (size_t)val; + return 0; +} + int cmd_range_diff(int argc, const char **argv, const char *prefix, @@ -25,6 +41,7 @@ int cmd_range_diff(int argc, struct strvec diff_merges_arg = STRVEC_INIT; struct range_diff_options range_diff_opts = { .creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT, + .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &diffopt, .other_arg = &other_arg }; @@ -40,6 +57,10 @@ int cmd_range_diff(int argc, PARSE_OPT_OPTARG), OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg, N_("style"), N_("passed to 'git log'"), 0), + OPT_CALLBACK(0, "max-memory", &range_diff_opts.max_memory, + N_("size"), + N_("maximum memory for cost matrix (default 4G)"), + parse_max_memory), OPT_PASSTHRU_ARGV(0, "remerge-diff", &diff_merges_arg, NULL, N_("passed to 'git log'"), PARSE_OPT_NOARG), OPT_BOOL(0, "left-only", &left_only, diff --git a/log-tree.c b/log-tree.c index 233bf9f227c61b..73d21f71764e94 100644 --- a/log-tree.c +++ b/log-tree.c @@ -717,6 +717,7 @@ static void show_diff_of_diff(struct rev_info *opt) struct range_diff_options range_diff_opts = { .creation_factor = opt->creation_factor, .dual_color = 1, + .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &opts }; diff --git a/range-diff.c b/range-diff.c index 8a2dcbee322e72..ca449a07693e85 100644 --- a/range-diff.c +++ b/range-diff.c @@ -325,13 +325,24 @@ static int diffsize(const char *a, const char *b) } static void get_correspondences(struct string_list *a, struct string_list *b, - int creation_factor) + int creation_factor, size_t max_memory) { int n = a->nr + b->nr; int *cost, c, *a2b, *b2a; int i, j; - - ALLOC_ARRAY(cost, st_mult(n, n)); + size_t cost_size = st_mult(n, n); + size_t cost_bytes = st_mult(sizeof(int), cost_size); + if (cost_bytes >= max_memory) { + struct strbuf cost_str = STRBUF_INIT; + struct strbuf max_str = STRBUF_INIT; + strbuf_humanise_bytes(&cost_str, cost_bytes); + strbuf_humanise_bytes(&max_str, max_memory); + die(_("range-diff: unable to compute the range-diff, since it " + "exceeds the maximum memory for the cost matrix: %s " + "(%"PRIuMAX" bytes) needed, limited to %s (%"PRIuMAX" bytes)"), + cost_str.buf, (uintmax_t)cost_bytes, max_str.buf, (uintmax_t)max_memory); + } + ALLOC_ARRAY(cost, cost_size); ALLOC_ARRAY(a2b, n); ALLOC_ARRAY(b2a, n); @@ -591,7 +602,8 @@ int show_range_diff(const char *range1, const char *range2, if (!res) { find_exact_matches(&branch1, &branch2); get_correspondences(&branch1, &branch2, - range_diff_opts->creation_factor); + range_diff_opts->creation_factor, + range_diff_opts->max_memory); output(&branch1, &branch2, range_diff_opts); } diff --git a/range-diff.h b/range-diff.h index cd85000b5a0da0..9d39818e349c91 100644 --- a/range-diff.h +++ b/range-diff.h @@ -5,6 +5,10 @@ #include "strvec.h" #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 +#define RANGE_DIFF_MAX_MEMORY_DEFAULT \ + (sizeof(void*) >= 8 ? \ + ((size_t)(1024L * 1024L) * (size_t)(4L * 1024L)) : /* 4GB on 64-bit */ \ + ((size_t)(1024L * 1024L) * (size_t)(2L * 1024L))) /* 2GB on 32-bit */ /* * A much higher value than the default, when we KNOW we are comparing @@ -17,6 +21,7 @@ struct range_diff_options { unsigned dual_color:1; unsigned left_only:1, right_only:1; unsigned include_merges:1; + size_t max_memory; const struct diff_options *diffopt; /* may be NULL */ const struct strvec *other_arg; /* may be NULL */ }; From 5e2feb5ca692c5c4d39b11e1ffa056911dd7dfd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=8E=E3=82=A6=E3=83=A9=20=7C=20Flare?= Date: Thu, 4 Sep 2025 17:44:16 +0000 Subject: [PATCH 14/16] alloc: fix dangling pointer in alloc_state cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All callers of clear_alloc_state() immediately free what they cleared, so currently it does not hurt anybody that the alloc_state is left in an unreusable state, but it is an error-prone API. Replace it with a new function that clears but in addition frees the structure, as well as NULLing the pointer that points at it and adjust existing callers. As it is a moral equivalent of FREE_AND_NULL(), except that what it frees has internal structure that needs to be cleaned, allow the helper to be called twice in a row, by making a call with a pointer to a pointer variable that already is NULLed. While at it, rename allocate_alloc_state() and name the new function alloc_state_free_and_null(), to follow more closely the function naming convention specified in the CodingGuidelines (namely, functions about S are named with S_ prefix and then verb). Signed-off-by: ノウラ | Flare Helped-by: Jeff King Signed-off-by: Junio C Hamano --- alloc.c | 10 ++++++++-- alloc.h | 4 ++-- object.c | 26 ++++++++++---------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/alloc.c b/alloc.c index 377e80f5dda2f8..533a045c2a8bdf 100644 --- a/alloc.c +++ b/alloc.c @@ -36,19 +36,25 @@ struct alloc_state { int slab_nr, slab_alloc; }; -struct alloc_state *allocate_alloc_state(void) +struct alloc_state *alloc_state_alloc(void) { return xcalloc(1, sizeof(struct alloc_state)); } -void clear_alloc_state(struct alloc_state *s) +void alloc_state_free_and_null(struct alloc_state **s_) { + struct alloc_state *s = *s_; + + if (!s) + return; + while (s->slab_nr > 0) { s->slab_nr--; free(s->slabs[s->slab_nr]); } FREE_AND_NULL(s->slabs); + FREE_AND_NULL(*s_); } static inline void *alloc_node(struct alloc_state *s, size_t node_size) diff --git a/alloc.h b/alloc.h index 3f4a0ad310a94b..87a47a970954c1 100644 --- a/alloc.h +++ b/alloc.h @@ -14,7 +14,7 @@ void *alloc_commit_node(struct repository *r); void *alloc_tag_node(struct repository *r); void *alloc_object_node(struct repository *r); -struct alloc_state *allocate_alloc_state(void); -void clear_alloc_state(struct alloc_state *s); +struct alloc_state *alloc_state_alloc(void); +void alloc_state_free_and_null(struct alloc_state **s_); #endif diff --git a/object.c b/object.c index c1553ee4330c89..986114a6dba843 100644 --- a/object.c +++ b/object.c @@ -517,12 +517,11 @@ struct parsed_object_pool *parsed_object_pool_new(struct repository *repo) memset(o, 0, sizeof(*o)); o->repo = repo; - o->blob_state = allocate_alloc_state(); - o->tree_state = allocate_alloc_state(); - o->commit_state = allocate_alloc_state(); - o->tag_state = allocate_alloc_state(); - o->object_state = allocate_alloc_state(); - + o->blob_state = alloc_state_alloc(); + o->tree_state = alloc_state_alloc(); + o->commit_state = alloc_state_alloc(); + o->tag_state = alloc_state_alloc(); + o->object_state = alloc_state_alloc(); o->is_shallow = -1; CALLOC_ARRAY(o->shallow_stat, 1); @@ -573,16 +572,11 @@ void parsed_object_pool_clear(struct parsed_object_pool *o) o->buffer_slab = NULL; parsed_object_pool_reset_commit_grafts(o); - clear_alloc_state(o->blob_state); - clear_alloc_state(o->tree_state); - clear_alloc_state(o->commit_state); - clear_alloc_state(o->tag_state); - clear_alloc_state(o->object_state); + alloc_state_free_and_null(&o->blob_state); + alloc_state_free_and_null(&o->tree_state); + alloc_state_free_and_null(&o->commit_state); + alloc_state_free_and_null(&o->tag_state); + alloc_state_free_and_null(&o->object_state); stat_validity_clear(o->shallow_stat); - FREE_AND_NULL(o->blob_state); - FREE_AND_NULL(o->tree_state); - FREE_AND_NULL(o->commit_state); - FREE_AND_NULL(o->tag_state); - FREE_AND_NULL(o->object_state); FREE_AND_NULL(o->shallow_stat); } From 31397bc4f7fd16dbafcc6d6d1e99808282689ba7 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Mon, 8 Sep 2025 22:28:45 +0200 Subject: [PATCH 15/16] doc: fast-import: replace literal block with paragraph 68061e34702 (fast-import: disallow "feature export-marks" by default, 2019-08-29) added the documentation for this option. The second paragraph is a literal block but it looks like it should just be a regular paragraph. Signed-off-by: Kristoffer Haugsbakk Acked-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-fast-import.adoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc index 58a2eaa51a8034..08b93f6f430339 100644 --- a/Documentation/git-fast-import.adoc +++ b/Documentation/git-fast-import.adoc @@ -61,10 +61,10 @@ OPTIONS currently impacts only the `export-marks`, `import-marks`, and `import-marks-if-exists` feature commands. + - Only enable this option if you trust the program generating the - fast-import stream! This option is enabled automatically for - remote-helpers that use the `import` capability, as they are - already trusted to run their own code. +Only enable this option if you trust the program generating the +fast-import stream! This option is enabled automatically for +remote-helpers that use the `import` capability, as they are +already trusted to run their own code. Options for Frontends ~~~~~~~~~~~~~~~~~~~~~ From ca2559c1d630eb4f04cdee2328aaf1c768907a9e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 18 Sep 2025 10:06:32 -0700 Subject: [PATCH 16/16] The tenth batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.52.0.adoc | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/Documentation/RelNotes/2.52.0.adoc b/Documentation/RelNotes/2.52.0.adoc index 959c8afe15f8a8..eae371f239a1fd 100644 --- a/Documentation/RelNotes/2.52.0.adoc +++ b/Documentation/RelNotes/2.52.0.adoc @@ -24,6 +24,13 @@ UI, Workflows & Features "--format=nul", and learns to report the objects format used in the repository. + * "core.commentChar=auto" that attempts to dynamically pick a + suitable comment character is non-workable, as it is too much + trouble to support for little benefit, and is marked as deprecated. + + * "git send-email" learned to drive "git imap-send" to store already + sent e-mails in an IMAP folder. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -63,6 +70,19 @@ Performance, Internal Implementation, Development Support etc. singleton variable, which has been updated to pass an instance throughout the callchain. + * CodingGuidelines now spells out how bitfields are to be written. + + * Adjust to the way newer versions of cURL selectivel enables tracing + options, so that our tests can continue to work. + (merge 1b5a6bfff3 jk/curl-global-trace-components later to maint). + + * The clear_alloc_state() API function was not fully clearing the + structure for reuse, but since nobody reuses it, replace it with a + variant that frees the structure as well, making the callers simpler. + + * "git range-diff" learned a way to limit the memory consumed by + O(N*N) cost matrix. + Fixes since v2.51 ----------------- @@ -177,6 +197,14 @@ including security updates, are included in this release. exhaust memory storing them redundantly, which has been corrected. (merge 88a2dc68c8 ps/upload-pack-oom-protection later to maint). + * A corner case bug in "git log -L..." has been corrected. + (merge e3106998ff sg/line-log-boundary-fixes later to maint). + + * "git rev-parse --short" and friends failed to disambiguate two + objects with object names that share common prefix longer than 32 + characters, which has been fixed. + (merge 8655908b9e jc/longer-disambiguation-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 823d537fa7 kh/doc-git-log-markup-fix later to maint). (merge cf7efa4f33 rj/t6137-cygwin-fix later to maint). @@ -198,3 +226,6 @@ including security updates, are included in this release. (merge c25651aefd ds/midx-write-fixes later to maint). (merge 069c15d256 rs/object-name-extend-abbrev-len-update later to maint). (merge bf5c224537 mm/worktree-doc-typofix later to maint). + (merge 31397bc4f7 kh/doc-fast-import-markup-fix later to maint). + (merge ac7096723b jc/doc-includeif-hasconfig-remote-url-fix later to maint). + (merge fafc9b08b8 ag/doc-sendmail-gmail-example-update later to maint).