From 819d3a55fc5a530f90309c2d208101765e8fe78d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 23 Jun 2025 09:14:11 -0700 Subject: [PATCH 01/40] coccicheck: fail "make" when it fails With "make coccicheck", we generate contrib/coccinelle/*.cocci.patch files that contain changes suggested by semantic patches, but "make" succeeds. Admittedly, not many developers may run "make coccicheck" in the first place, but it makes it harder to notice when they do run it after they introduced an iffy piece of code. Check that the resulting cocci.patch files are all empty. Signed-off-by: Junio C Hamano --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 97e8385b6643b9..8f1e9424a73faa 100644 --- a/Makefile +++ b/Makefile @@ -3422,11 +3422,14 @@ endif coccicheck-test: $(COCCI_TEST_RES_GEN) coccicheck: coccicheck-test + ifdef SPATCH_CONCAT_COCCI -coccicheck: contrib/coccinelle/ALL.cocci.patch +COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = contrib/coccinelle/ALL.cocci.patch else -coccicheck: $(COCCICHECK_PATCHES_INTREE) +COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = $(COCCICHECK_PATCHES_INTREE) endif +coccicheck: $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) + ! grep -q ^ $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) /dev/null # See contrib/coccinelle/README coccicheck-pending: coccicheck-test From 3570fba943e36b7c4c6bbbc60454a01dde73ef5e Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Wed, 25 Jun 2025 23:25:09 +0900 Subject: [PATCH 02/40] contrib: use a more portable shebang for git-credential-netrc While the installed scripts have their Perl shebang set to PERL_PATH, it is nevertheless useful to be able to run the uninstalled script for manual tests while developing. This change makes the shebang more portable by having the perl command looked from PATH instead of from a fixed location. Signed-off-by: Maxim Cournoyer Signed-off-by: Junio C Hamano --- contrib/credential/netrc/git-credential-netrc.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl index 9fb998ae090325..514f68d00be72a 100755 --- a/contrib/credential/netrc/git-credential-netrc.perl +++ b/contrib/credential/netrc/git-credential-netrc.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; From 53ca38298d69529e59e33a21e63040aef509bbf8 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Wed, 25 Jun 2025 23:25:10 +0900 Subject: [PATCH 03/40] contrib: warn for invalid netrc file ports in git-credential-netrc Invalid ports were previously silently dropped; now a warning message is produced. Signed-off-by: Maxim Cournoyer Signed-off-by: Junio C Hamano --- contrib/credential/netrc/git-credential-netrc.perl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl index 514f68d00be72a..09d77b4f69380b 100755 --- a/contrib/credential/netrc/git-credential-netrc.perl +++ b/contrib/credential/netrc/git-credential-netrc.perl @@ -267,9 +267,14 @@ sub load_netrc { if (!defined $nentry->{machine}) { next; } - if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) { - $num_port = $nentry->{port}; - delete $nentry->{port}; + if (defined $nentry->{port}) { + if ($nentry->{port} =~ m/^\d+$/) { + $num_port = $nentry->{port}; + delete $nentry->{port}; + } else { + printf(STDERR "ignoring invalid port `%s' " . + "from netrc file\n", $nentry->{port}); + } } # create the new entry for the credential helper protocol From 1926d9b6dac2cc7584362fb9275b55c2904891d3 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Wed, 25 Jun 2025 23:25:11 +0900 Subject: [PATCH 04/40] contrib: better support symbolic port names in git-credential-netrc To improve support for symbolic port names in netrc files, this changes does the following: - Treat symbolic port names as ports, not protocols in git-credential-netrc - Validate the SMTP server port provided to send-email - Convert the above symbolic port names to their numerical values. Before this change, it was not possible to have a SMTP server port set to "smtps" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would be registered as a protocol and break the match for a "smtp" protocol host, as queried for by git-send-email. Signed-off-by: Maxim Cournoyer Signed-off-by: Junio C Hamano --- contrib/credential/netrc/git-credential-netrc.perl | 11 +++++++---- contrib/credential/netrc/test.pl | 8 ++++---- git-send-email.perl | 11 +++++++++++ perl/Git.pm | 13 +++++++++++++ t/t9001-send-email.sh | 7 +++++++ 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl index 09d77b4f69380b..3c0a532d0e2409 100755 --- a/contrib/credential/netrc/git-credential-netrc.perl +++ b/contrib/credential/netrc/git-credential-netrc.perl @@ -268,13 +268,16 @@ sub load_netrc { next; } if (defined $nentry->{port}) { - if ($nentry->{port} =~ m/^\d+$/) { - $num_port = $nentry->{port}; - delete $nentry->{port}; - } else { + $num_port = Git::port_num($nentry->{port}); + unless ($num_port) { printf(STDERR "ignoring invalid port `%s' " . "from netrc file\n", $nentry->{port}); } + # Since we've already validated and converted + # the port to its numerical value, do not + # capture it as the `protocol' value, as used + # to be the case for symbolic port names. + delete $nentry->{port}; } # create the new entry for the credential helper protocol diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl index 67a0ede5644dd2..8a7fc2588a37f3 100755 --- a/contrib/credential/netrc/test.pl +++ b/contrib/credential/netrc/test.pl @@ -45,7 +45,7 @@ BEGIN diag "Testing with invalid data\n"; $cred = run_credential(['-f', $netrc, 'get'], "bad data"); -ok(scalar keys %$cred == 4, "Got first found keys with bad data"); +ok(scalar keys %$cred == 3, "Got first found keys with bad data"); diag "Testing netrc file for a missing corovamilkbar entry\n"; $cred = run_credential(['-f', $netrc, 'get'], @@ -64,12 +64,12 @@ BEGIN diag "Testing netrc file for a username-specific entry\n"; $cred = run_credential(['-f', $netrc, 'get'], - { host => 'imap', username => 'bob' }); + { host => 'imap:993', username => 'bob' }); -ok(scalar keys %$cred == 2, "Got 2 username-specific keys"); +# Only the password field gets returned. +ok(scalar keys %$cred == 1, "Got 1 username-specific keys"); is($cred->{password}, 'bobwillknow', "Got correct user-specific password"); -is($cred->{protocol}, 'imaps', "Got correct user-specific protocol"); diag "Testing netrc file for a host:port-specific entry\n"; $cred = run_credential(['-f', $netrc, 'get'], diff --git a/git-send-email.perl b/git-send-email.perl index 659e6c588bcf87..d2cf9b717af047 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -2101,6 +2101,17 @@ sub initialize_modified_loop_vars { } } + # Validate the SMTP server port, if provided. + if (defined $smtp_server_port) { + my $port = Git::port_num($smtp_server_port); + if ($port) { + $smtp_server_port = $port; + } else { + die sprintf(__("error: invalid SMTP port '%s'\n"), + $smtp_server_port); + } + } + # Run the loop once again to avoid gaps in the counter due to FIFO # arguments provided by the user. my $num = 1; diff --git a/perl/Git.pm b/perl/Git.pm index 6f47d653abe522..090cf77daba323 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -1061,6 +1061,19 @@ sub _close_cat_blob { delete @$self{@vars}; } +# Given PORT, a port number or service name, return its numerical +# value else undef. +sub port_num { + my ($port) = @_; + + # Port can be either a positive integer within the 16-bit range... + if ($port =~ /^\d+$/ && $port > 0 && $port <= (2**16 - 1)) { + return $port; + } + + # ... or a symbolic port (service name). + return scalar getservbyname($port, ''); +} =item credential_read( FILEHANDLE ) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 0c1af43f6fbb41..e56e0c8d7770b0 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' ' test_cmp expected-cc commandline1 ' +test_expect_failure $PREREQ 'invalid smtp server port value' ' + clean_fake_sendmail && + git send-email -1 --to=recipient@example.com \ + --smtp-server-port=bogus-symbolic-name \ + --smtp-server="$(pwd)/fake.sendmail" +' + test_expect_success $PREREQ 'setup expect' " cat >expected-show-all-headers <<\EOF 0001-Second.patch From 88a4ed40c0056eb6563f50ac41f5c09e7aedc418 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Tue, 1 Jul 2025 17:14:28 +0200 Subject: [PATCH 05/40] config: document --[no-]show-names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These options were introduced in 4e513890008 (builtin/config: introduce "get" subcommand, 2024-05-06) but not documented here. Use the description from the source code. Document this option and the negated form according to the current convention.[1] `--show-names` is also the default when `--get-regexp` is given. But don’t mention it here since all the deprecated modes are quarantined in the “Deprecated Modes” section. [1]: https://lore.kernel.org/git/xmqqcyct1mtq.fsf@gitster.g/ Acked-by: Patrick Steinhardt Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-config.adoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc index 936e0c5130fe7d..e05bf813d46420 100644 --- a/Documentation/git-config.adoc +++ b/Documentation/git-config.adoc @@ -259,6 +259,12 @@ Valid ``'s include: Output only the names of config variables for `list` or `get`. +`--show-names`:: +`--no-show-names`:: + With `get`, show config keys in addition to their values. The + default is `--no-show-names` unless `--url` is given and there + are no subsections in __. + --show-origin:: Augment the output of all queried config options with the origin type (file, standard input, blob, command line) and From f322f86e30b7667ef0ae519ff87796b2f0a0632d Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Tue, 1 Jul 2025 17:14:29 +0200 Subject: [PATCH 06/40] config: use --value= consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This option was introduced in a series of commits from fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15). But two styles were used for the value provided to the option: 1. Synopsis: `--value=` 2. Deprecated Modes: `--value=` (2) is also used in the synopsis on the command. Use (2) consistently throughout since it’s a pattern in the general case (`value` sounds more generic). Acked-by: Patrick Steinhardt Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-config.adoc | 6 +++--- builtin/config.c | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc index e05bf813d46420..d3ddc53855538b 100644 --- a/Documentation/git-config.adoc +++ b/Documentation/git-config.adoc @@ -10,9 +10,9 @@ SYNOPSIS -------- [verse] 'git config list' [] [] [--includes] -'git config get' [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] -'git config set' [] [--type=] [--all] [--value=] [--fixed-value] -'git config unset' [] [--all] [--value=] [--fixed-value] +'git config get' [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] +'git config set' [] [--type=] [--all] [--value=] [--fixed-value] +'git config unset' [] [--all] [--value=] [--fixed-value] 'git config rename-section' [] 'git config remove-section' [] 'git config edit' [] diff --git a/builtin/config.c b/builtin/config.c index f70d6354772259..706269647e595b 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -17,9 +17,9 @@ static const char *const builtin_config_usage[] = { N_("git config list [] [] [--includes]"), - N_("git config get [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] "), - N_("git config set [] [--type=] [--all] [--value=] [--fixed-value] "), - N_("git config unset [] [--all] [--value=] [--fixed-value] "), + N_("git config get [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] "), + N_("git config set [] [--type=] [--all] [--value=] [--fixed-value] "), + N_("git config unset [] [--all] [--value=] [--fixed-value] "), N_("git config rename-section [] "), N_("git config remove-section [] "), N_("git config edit []"), @@ -33,17 +33,17 @@ static const char *const builtin_config_list_usage[] = { }; static const char *const builtin_config_get_usage[] = { - N_("git config get [] [] [--includes] [--all] [--regexp=] [--value=] [--fixed-value] [--default=] "), + N_("git config get [] [] [--includes] [--all] [--regexp=] [--value=] [--fixed-value] [--default=] "), NULL }; static const char *const builtin_config_set_usage[] = { - N_("git config set [] [--type=] [--comment=] [--all] [--value=] [--fixed-value] "), + N_("git config set [] [--type=] [--comment=] [--all] [--value=] [--fixed-value] "), NULL }; static const char *const builtin_config_unset_usage[] = { - N_("git config unset [] [--all] [--value=] [--fixed-value] "), + N_("git config unset [] [--all] [--value=] [--fixed-value] "), NULL }; From 5ba6e6cfe3c6279019d8a1d915bd0e9f35b177c4 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Tue, 1 Jul 2025 17:14:30 +0200 Subject: [PATCH 07/40] config: document --[no-]value These options were introduced in a series of commits from fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15).[1] But they were not documented here. Document this option and the negated form according to the current convention.[2] [1]: `--value` is a replacement for the `value-pattern` positional argument [2]: https://lore.kernel.org/git/xmqqcyct1mtq.fsf@gitster.g/ Acked-by: Patrick Steinhardt Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-config.adoc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc index d3ddc53855538b..03808b18d3e713 100644 --- a/Documentation/git-config.adoc +++ b/Documentation/git-config.adoc @@ -200,6 +200,14 @@ See also <>. section in linkgit:gitrevisions[7] for a more complete list of ways to spell blob names. +`--value=`:: +`--no-value`:: + With `get`, `set`, and `unset`, match only against + __. The pattern is an extended regular expression unless + `--fixed-value` is given. ++ +Use `--no-value` to unset __. + --fixed-value:: When used with the `value-pattern` argument, treat `value-pattern` as an exact string instead of a regular expression. This will restrict From d46f69862645e31cbf25c8bb53f0761f03860533 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Tue, 1 Jul 2025 17:14:31 +0200 Subject: [PATCH 08/40] config: use --value instead of value-pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This option was introduced in a series of commits from fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15) and deprecated `value-pattern`. But `value-pattern` is still used throughout the doc. The deprecated modes have been quarantined in the “Deprecated Modes” section. So let’s only use `--value=` in the rest of the doc. Acked-by: Patrick Steinhardt Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-config.adoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc index 03808b18d3e713..9d8f9bb04e5318 100644 --- a/Documentation/git-config.adoc +++ b/Documentation/git-config.adoc @@ -26,7 +26,7 @@ escaped. Multiple lines can be added to an option by using the `--append` option. If you want to update or unset an option which can occur on multiple -lines, a `value-pattern` (which is an extended regular expression, +lines, `--value=` (which is an extended regular expression, unless the `--fixed-value` option is given) needs to be given. Only the existing values that match the pattern are updated or unset. If you want to handle the lines that do *not* match the pattern, just @@ -109,7 +109,7 @@ OPTIONS --replace-all:: Default behavior is to replace at most one line. This replaces - all lines matching the key (and optionally the `value-pattern`). + all lines matching the key (and optionally `--value=`). --append:: Adds a new line to the option without altering any existing @@ -209,10 +209,10 @@ See also <>. Use `--no-value` to unset __. --fixed-value:: - When used with the `value-pattern` argument, treat `value-pattern` as + When used with `--value=`, treat __ as an exact string instead of a regular expression. This will restrict the name/value pairs that are matched to only those where the value - is exactly equal to the `value-pattern`. + is exactly equal to __. --type :: 'git config' will ensure that any input or output is valid under the given From c4e9775c60b880f06054c22f98520bdb4f1ba38e Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Tue, 1 Jul 2025 17:14:32 +0200 Subject: [PATCH 09/40] config: mention --url in the synopsis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4e513890008 (builtin/config: introduce "get" subcommand, 2024-05-06) introduced `get` and `--url` but didn’t add `--url` to the synopsis. Acked-by: Patrick Steinhardt Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-config.adoc | 2 +- builtin/config.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc index 9d8f9bb04e5318..511b2e26bfb00f 100644 --- a/Documentation/git-config.adoc +++ b/Documentation/git-config.adoc @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] 'git config list' [] [] [--includes] -'git config get' [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] +'git config get' [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] [--url=] 'git config set' [] [--type=] [--all] [--value=] [--fixed-value] 'git config unset' [] [--all] [--value=] [--fixed-value] 'git config rename-section' [] diff --git a/builtin/config.c b/builtin/config.c index 706269647e595b..5efe2730106c99 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -17,7 +17,7 @@ static const char *const builtin_config_usage[] = { N_("git config list [] [] [--includes]"), - N_("git config get [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] "), + N_("git config get [] [] [--includes] [--all] [--regexp] [--value=] [--fixed-value] [--default=] [--url=] "), N_("git config set [] [--type=] [--all] [--value=] [--fixed-value] "), N_("git config unset [] [--all] [--value=] [--fixed-value] "), N_("git config rename-section [] "), From 9e45fc6ce54171bec645917c05b79e3455266a61 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:18 +0200 Subject: [PATCH 10/40] clang-format: set 'ColumnLimit' to 0 When clang-format was introduced to the Git project in 6134de6ac1 (clang-format: outline the git project's coding style, 2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our recommendation in 'Documentation/CodingGuidelines', which states: We try to keep to at most 80 characters per line. However while this is recommended limit, this is not the enforced limit. In some cases in we do overflow this limit to prioritize readability. Setting the 'ColumnLimit' also means that shorter lines are concatenated to simply as the result would still be below 80 characters, which is undesirable. In the past, we tried to adjust the penalties around line wrapping, once in 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29) and another time in 5e9fa0f9fa (clang-format: re-adjust line break penalties, 2024-10-18). While these settings help tweak the line break penalties to be more in-line with the requirements of the Git project, using 'clang-format' still produces a lot of false positives. So to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This means that line-wrapping is no-longer a concern of the formatter and something that the user needs to take care of. The previous commit also added a more flexible guideline to the '.editorconfig' setting a 'max_line_length' of 120 characters. This should provide some guidance to users. In the future, it would be nice to re-instate this limit with adequate penalties which would follow our guidelines, but currently, it makes more sense to have a working formatter which we can rely on and which doesn't create too many false positives. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- .clang-format | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/.clang-format b/.clang-format index 9547fe1b77cac0..19d6cf4200f5a9 100644 --- a/.clang-format +++ b/.clang-format @@ -12,7 +12,15 @@ UseTab: Always TabWidth: 8 IndentWidth: 8 ContinuationIndentWidth: 8 -ColumnLimit: 80 + +# While we do want to enforce a character limit of 80 characters, we often +# allow lines to overflow that limit to prioritize readability. Setting a +# character limit here with penalties has been finicky and creates too many +# false positives. +# +# NEEDSWORK: It would be nice if we can find optimal settings to ensure we +# can re-enable the limit here. +ColumnLimit: 0 # C Language specifics Language: Cpp @@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1 # No empty line at the start of a block. KeepEmptyLinesAtTheStartOfBlocks: false -# Penalties -# This decides what order things should be done if a line is too long -PenaltyBreakAssignment: 5 -PenaltyBreakBeforeFirstCallParameter: 5 -PenaltyBreakComment: 5 -PenaltyBreakFirstLessLess: 0 -PenaltyBreakOpenParenthesis: 300 -PenaltyBreakString: 5 -PenaltyExcessCharacter: 10 -PenaltyReturnTypeOnItsOwnLine: 300 - # Don't sort #include's SortIncludes: false From 73d8380e56087ac0a4285596e74444e26d01fb89 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:19 +0200 Subject: [PATCH 11/40] clang-format: add 'RemoveBracesLLVM' to the main config In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job, 2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the clang formatter. This rule checks and warns against using braces on simple single-statement bodies of statements. Since we haven't had any issues regarding this rule, we can now move it into the main clang-format config and remove it from being CI exclusive. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- .clang-format | 6 ++++++ ci/run-style-check.sh | 18 +----------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/.clang-format b/.clang-format index 19d6cf4200f5a9..dcfd0aad60c31e 100644 --- a/.clang-format +++ b/.clang-format @@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false # Don't sort #include's SortIncludes: false + +# Remove optional braces of control statements (if, else, for, and while) +# according to the LLVM coding style. This avoids braces on simple +# single-statement bodies of statements but keeps braces if one side of +# if/else if/.../else cascade has multi-statement body. +RemoveBracesLLVM: true diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh index 6cd4b1d934a76a..0832c19df076f4 100755 --- a/ci/run-style-check.sh +++ b/ci/run-style-check.sh @@ -5,21 +5,5 @@ baseCommit=$1 -# Remove optional braces of control statements (if, else, for, and while) -# according to the LLVM coding style. This avoids braces on simple -# single-statement bodies of statements but keeps braces if one side of -# if/else if/.../else cascade has multi-statement body. -# -# As this rule comes with a warning [1], we want to experiment with it -# before adding it in-tree. since the CI job for the style check is allowed -# to fail, appending the rule here allows us to validate its efficacy. -# While also ensuring that end-users are not affected directly. -# -# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm -{ - cat .clang-format - echo "RemoveBracesLLVM: true" -} >/tmp/clang-format-rules - -git clang-format --style=file:/tmp/clang-format-rules \ +git clang-format --style=file:.clang-format \ --diff --extensions c,h "$baseCommit" From 3f7e447aaf40724e54fe81c99bee104829e3d23d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:20 +0200 Subject: [PATCH 12/40] meson: add rule to run 'git clang-format' The Makefile has a 'style' rule to run 'git clang-format'. While Meson intrinsically supports a 'clang-format' target, which can be run when using the ninja backend by running 'ninja clang-format', this runs the formatting on all existing files. Our Meson build doesn't yet support a way to run 'git clang-format', which runs the formatter between the working directory and commit provided. Add a new 'style' target to Meson to mimic the target in the Makefile. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- meson.build | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/meson.build b/meson.build index 596f5ac7110ebf..7f2d7b5f8c8765 100644 --- a/meson.build +++ b/meson.build @@ -2132,6 +2132,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc' alias_target('check-headers', hdr_check) endif +git_clang_format = find_program('git-clang-format', required: false, native: true) +if git_clang_format.found() + run_target('style', + command: [ + git_clang_format, + '--style', 'file', + '--diff', + '--extensions', 'c,h' + ] + ) +endif + foreach key, value : { 'DIFF': diff.full_path(), 'GIT_SOURCE_DIR': meson.project_source_root(), From 46a3ab744b9d83e9c21a5b9f6ede29e840f658b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= Date: Wed, 2 Jul 2025 02:37:35 -0700 Subject: [PATCH 13/40] config.mak.uname: set NO_MEMMEM only for functional version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FreeBSD 6 introduced memmem(), but the implementation diverged from what was standard everywhere else (including our "compat" fallback). FreeBSD 10.4 (went EOL in 2018) corrected the functionality bugs but kept a suboptimal implementation until FreeBSD 11.4 (the last version of FreeBSD 11, that went EOL in September 2021). Let's draw the line to require FreeBSD 12 or newer, which allows us to drop the special casing of FreeBSD 4.x and rely on the platform implementation of memmem() unconditionally for all versions that are still being supported. Suggested-by: Brad Smith Helped-by: brian m. carlson Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- config.mak.uname | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index b12d4e168ae119..2b434df9e53d2c 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -273,16 +273,13 @@ ifeq ($(uname_S),FreeBSD) ifeq ($(firstword $(subst -, ,$(uname_R))),10.1) OLD_ICONV = YesPlease endif - NO_MEMMEM = YesPlease + ifeq ($(shell v=$(uname_R) && test $${v%%.*} -lt 12 && echo 1),1) + NO_MEMMEM = UnfortunatelyYes + endif BASIC_CFLAGS += -I/usr/local/include BASIC_LDFLAGS += -L/usr/local/lib DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease USE_ST_TIMESPEC = YesPlease - ifeq ($(shell expr "$(uname_R)" : '4\.'),2) - PTHREAD_LIBS = -pthread - NO_UINTMAX_T = YesPlease - NO_STRTOUMAX = YesPlease - endif PYTHON_PATH = /usr/local/bin/python PERL_PATH = /usr/local/bin/perl HAVE_PATHS_H = YesPlease From 0392f976a78867a1f8cfb6c937188f92c8206f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= Date: Wed, 2 Jul 2025 02:37:36 -0700 Subject: [PATCH 14/40] build: retire NO_UINTMAX_T MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A previous commit removed the last user of it, and it is no longer useful with the codebase moving towards C99, which specifies its definition. Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- Makefile | 5 ----- configure.ac | 8 -------- meson.build | 11 ----------- 3 files changed, 24 deletions(-) diff --git a/Makefile b/Makefile index 3868edd349eac2..ba111f191fb804 100644 --- a/Makefile +++ b/Makefile @@ -114,8 +114,6 @@ include shared.mak # # Define NO_INTPTR_T if you don't have intptr_t or uintptr_t. # -# Define NO_UINTMAX_T if you don't have uintmax_t. -# # Define NEEDS_SOCKET if linking with libc is not enough (SunOS, # Patrick Mauritz). # @@ -1915,9 +1913,6 @@ endif ifdef NO_INTPTR_T COMPAT_CFLAGS += -DNO_INTPTR_T endif -ifdef NO_UINTMAX_T - BASIC_CFLAGS += -Duintmax_t=uint32_t -endif ifdef NO_SOCKADDR_STORAGE ifdef NO_IPV6 BASIC_CFLAGS += -Dsockaddr_storage=sockaddr_in diff --git a/configure.ac b/configure.ac index 5923edc44aa7b6..d8c3af161b0416 100644 --- a/configure.ac +++ b/configure.ac @@ -1121,14 +1121,6 @@ GIT_CHECK_FUNC(strlcpy, [NO_STRLCPY=YesPlease]) GIT_CONF_SUBST([NO_STRLCPY]) # -# Define NO_UINTMAX_T if your platform does not have uintmax_t -AC_CHECK_TYPE(uintmax_t, -[NO_UINTMAX_T=], -[NO_UINTMAX_T=YesPlease],[ -#include -]) -GIT_CONF_SUBST([NO_UINTMAX_T]) -# # Define NO_STRTOUMAX if you don't have strtoumax in the C library. GIT_CHECK_FUNC(strtoumax, [NO_STRTOUMAX=], diff --git a/meson.build b/meson.build index efe2871c9dba13..27d5f407414223 100644 --- a/meson.build +++ b/meson.build @@ -1331,17 +1331,6 @@ if compiler.compiles(''' libgit_c_args += '-DHAVE_CLOCK_MONOTONIC' endif -if not compiler.compiles(''' - #include - - void func(void) - { - uintmax_t x = 0; - } -''', name: 'uintmax_t') - libgit_c_args += '-DNO_UINTMAX_T' -endif - has_bsd_sysctl = false if compiler.has_header('sys/sysctl.h') if compiler.compiles(''' From f3a9558c8c0630e3ffb85b58e79a72a131f50dc7 Mon Sep 17 00:00:00 2001 From: Brett A C Sheffield Date: Wed, 2 Jul 2025 18:19:52 +0200 Subject: [PATCH 15/40] gitremote-helpers.adoc: fix formatting Add missing colon to fix formatting. Signed-off-by: Brett A C Sheffield Signed-off-by: Junio C Hamano --- Documentation/gitremote-helpers.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index d0be008e5e3576..39cdece16e93a4 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -498,7 +498,7 @@ set by Git if the remote helper has the 'option' capability. ask for the tag specifically. Some helpers may be able to use this option to avoid a second network connection. -'option dry-run' {'true'|'false'}: +'option dry-run' {'true'|'false'}:: If true, pretend the operation completed successfully, but don't actually change any repository data. For most helpers this only applies to the 'push', if supported. From d0b94577dda3a50c1833626a70ebefd478bfcbf9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 4 Jul 2025 11:42:56 +0200 Subject: [PATCH 16/40] BreakingChanges: announce switch to "reftable" format The "reftable" format has come a long way and has matured nicely since it has been merged into git via 57db2a094d5 (refs: introduce reftable backend, 2024-02-07). It fixes longstanding issues that cannot be fixed with the "files" format in a backwards-compatible way and performs significantly better in many use cases. Announce that we will switch to the "reftable" format in Git 3.0 for newly created repositories and wire up the change, hidden behind the WITH_BREAKING_CHANGES preprocessor define. This switch is dependent on support in the larger Git ecosystem. Most importantly, libraries like JGit, libgit2 and Gitoxide should support the reftable backend so that we don't break all applications and tools built on top of those libraries. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/BreakingChanges.adoc | 47 ++++++++++++++++++++++++++++++ help.c | 2 ++ repository.h | 6 ++++ setup.c | 2 ++ t/t0001-init.sh | 11 +++++++ 5 files changed, 68 insertions(+) diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc index c6bd94986c5fcc..f8d2eba061c82a 100644 --- a/Documentation/BreakingChanges.adoc +++ b/Documentation/BreakingChanges.adoc @@ -118,6 +118,53 @@ Cf. <2f5de416-04ba-c23d-1e0b-83bb655829a7@zombino.com>, <20170223155046.e7nxivfwqqoprsqj@LykOS.localdomain>, . +* The default storage format for references in newly created repositories will + be changed from "files" to "reftable". The "reftable" format provides + multiple advantages over the "files" format: ++ + ** It is impossible to store two references that only differ in casing on + case-insensitive filesystems with the "files" format. This issue is common + on Windows and macOS platforms. As the "reftable" backend does not use + filesystem paths to encode reference names this problem goes away. + ** Similarly, macOS normalizes path names that contain unicode characters, + which has the consequence that you cannot store two names with unicode + characters that are encoded differently with the "files" backend. Again, + this is not an issue with the "reftable" backend. + ** Deleting references with the "files" backend requires Git to rewrite the + complete "packed-refs" file. In large repositories with many references + this file can easily be dozens of megabytes in size, in extreme cases it + may be gigabytes. The "reftable" backend uses tombstone markers for + deleted references and thus does not have to rewrite all of its data. + ** Repository housekeeping with the "files" backend typically performs + all-into-one repacks of references. This can be quite expensive, and + consequently housekeeping is a tradeoff between the number of loose + references that accumulate and slow down operations that read references, + and compressing those loose references into the "packed-refs" file. The + "reftable" backend uses geometric compaction after every write, which + amortizes costs and ensures that the backend is always in a + well-maintained state. + ** Operations that write multiple references at once are not atomic with the + "files" backend. Consequently, Git may see in-between states when it reads + references while a reference transaction is in the process of being + committed to disk. + ** Writing many references at once is slow with the "files" backend because + every reference is created as a separate file. The "reftable" backend + significantly outperforms the "files" backend by multiple orders of + magnitude. + ** The reftable backend uses a binary format with prefix compression for + reference names. As a result, the format uses less space compared to the + "packed-refs" file. ++ +Users that get immediate benefit from the "reftable" backend could continue to +opt-in to the "reftable" format manually by setting the "init.defaultRefFormat" +config. But defaults matter, and we think that overall users will have a better +experience with less platform-specific quirks when they use the new backend by +default. ++ +A prerequisite for this change is that the ecosystem is ready to support the +"reftable" format. Most importantly, alternative implementations of Git like +JGit, libgit2 and Gitoxide need to support it. + === Removals * Support for grafting commits has long been superseded by git-replace(1). diff --git a/help.c b/help.c index 21b778707a6a65..89cd47e3b867f7 100644 --- a/help.c +++ b/help.c @@ -810,6 +810,8 @@ void get_version_info(struct strbuf *buf, int show_build_options) SHA1_UNSAFE_BACKEND); #endif strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND); + strbuf_addf(buf, "default-ref-format: %s\n", + ref_storage_format_to_name(REF_STORAGE_FORMAT_DEFAULT)); } } diff --git a/repository.h b/repository.h index c4c92b2ab9c9e3..77c4189d5dcb52 100644 --- a/repository.h +++ b/repository.h @@ -20,6 +20,12 @@ enum ref_storage_format { REF_STORAGE_FORMAT_REFTABLE, }; +#ifdef WITH_BREAKING_CHANGES /* Git 3.0 */ +# define REF_STORAGE_FORMAT_DEFAULT REF_STORAGE_FORMAT_REFTABLE +#else +# define REF_STORAGE_FORMAT_DEFAULT REF_STORAGE_FORMAT_FILES +#endif + struct repo_path_cache { char *squash_msg; char *merge_msg; diff --git a/setup.c b/setup.c index f93bd6a24a5d9c..f0c06c655a929a 100644 --- a/setup.c +++ b/setup.c @@ -2541,6 +2541,8 @@ static void repository_format_configure(struct repository_format *repo_fmt, repo_fmt->ref_storage_format = ref_format; } else if (cfg.ref_format != REF_STORAGE_FORMAT_UNKNOWN) { repo_fmt->ref_storage_format = cfg.ref_format; + } else { + repo_fmt->ref_storage_format = REF_STORAGE_FORMAT_DEFAULT; } repo_set_ref_storage_format(the_repository, repo_fmt->ref_storage_format); } diff --git a/t/t0001-init.sh b/t/t0001-init.sh index f11a40811f243a..186664162fcb85 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -658,6 +658,17 @@ test_expect_success 'init warns about invalid init.defaultRefFormat' ' test_cmp expected actual ' +test_expect_success 'default ref format' ' + test_when_finished "rm -rf refformat" && + ( + sane_unset GIT_DEFAULT_REF_FORMAT && + git init refformat + ) && + git version --build-options | sed -ne "s/^default-ref-format: //p" >expect && + git -C refformat rev-parse --show-ref-format >actual && + test_cmp expect actual +' + backends="files reftable" for format in $backends do From 793b14e1c833dd4ea0d85cdef53cc5ab38f7915e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 4 Jul 2025 11:42:57 +0200 Subject: [PATCH 17/40] setup: use "reftable" format when experimental features are enabled With the preceding commit we have announced the switch to the "reftable" format in Git 3.0 for newly created repositories. The format is being battle tested by GitLab and a couple of other developers, and except for a small handful of issues exposed early after it has been merged it has been rock solid. Regardless of that though the test user base is still comparatively small, which increases the risk that we miss critical bugs. Address this by enabling the reftable format when experimental features are enabled. This should increase the test user base by some margin and thus give us more input before making the format the default. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/config/feature.adoc | 6 ++++++ setup.c | 12 +++++++++++ t/t0001-init.sh | 34 +++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/Documentation/config/feature.adoc b/Documentation/config/feature.adoc index cb49ff2604a632..924f5ff4e3caaa 100644 --- a/Documentation/config/feature.adoc +++ b/Documentation/config/feature.adoc @@ -24,6 +24,12 @@ reusing objects from multiple packs instead of just one. * `pack.usePathWalk` may speed up packfile creation and make the packfiles be significantly smaller in the presence of certain filename collisions with Git's default name-hash. ++ +* `init.defaultRefFormat=reftable` causes newly initialized repositories to use +the reftable format for storing references. This new format solves issues with +case-insensitive filesystems, compresses better and performs significantly +better with many use cases. Refer to Documentation/technical/reftable.adoc for +more information on this new storage format. feature.manyFiles:: Enable config options that optimize for repos with many files in the diff --git a/setup.c b/setup.c index f0c06c655a929a..97d7824d07a117 100644 --- a/setup.c +++ b/setup.c @@ -2481,6 +2481,18 @@ static int read_default_format_config(const char *key, const char *value, goto out; } + /* + * Enable the reftable format when "features.experimental" is enabled. + * "init.defaultRefFormat" takes precedence over this setting. + */ + if (!strcmp(key, "feature.experimental") && + cfg->ref_format == REF_STORAGE_FORMAT_UNKNOWN && + git_config_bool(key, value)) { + cfg->ref_format = REF_STORAGE_FORMAT_REFTABLE; + ret = 0; + goto out; + } + ret = 0; out: free(str); diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 186664162fcb85..f593c5368746fa 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -749,6 +749,40 @@ test_expect_success "GIT_DEFAULT_REF_FORMAT= overrides init.defaultRefFormat" ' test_cmp expect actual ' +test_expect_success "init with feature.experimental=true" ' + test_when_finished "rm -rf refformat" && + test_config_global feature.experimental true && + ( + sane_unset GIT_DEFAULT_REF_FORMAT && + git init refformat + ) && + echo reftable >expect && + git -C refformat rev-parse --show-ref-format >actual && + test_cmp expect actual +' + +test_expect_success "init.defaultRefFormat overrides feature.experimental=true" ' + test_when_finished "rm -rf refformat" && + test_config_global feature.experimental true && + test_config_global init.defaultRefFormat files && + ( + sane_unset GIT_DEFAULT_REF_FORMAT && + git init refformat + ) && + echo files >expect && + git -C refformat rev-parse --show-ref-format >actual && + test_cmp expect actual +' + +test_expect_success "GIT_DEFAULT_REF_FORMAT= overrides feature.experimental=true" ' + test_when_finished "rm -rf refformat" && + test_config_global feature.experimental true && + GIT_DEFAULT_REF_FORMAT=files git init refformat && + echo files >expect && + git -C refformat rev-parse --show-ref-format >actual && + test_cmp expect actual +' + for from_format in $backends do test_expect_success "re-init with same format ($from_format)" ' From 375ac087c5c5d17f941ed235c0bf434870eba8e7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 3 Jul 2025 18:44:28 -0400 Subject: [PATCH 18/40] setup_revisions(): turn on diffs for all-negative diff filter When the user gives us a diff filter like --diff-filter=D, we need to do a tree diff even if we're not planning to show the diff result itself, in order to decide whether to show the commit at all. So there's an explicit check of revs->diffopt.filter in setup_revisions(), and we set revs->diff if any bits are set. Originally that "filter" field covered both positive capital-letter filters (like "D") and also negative lowercase filters (like "d"), so it was sufficient for both cases. But later, 75408ca949 (diff-filter: be more careful when looking for negative bits, 2022-01-28) split the negative bits out into a "filter_not" field. We eventually fold those into "filter", but not until diff_setup_done() is called, which happens after our explicit check. As a result, a purely negative filter like: git log --diff-filter=d failed to turn on diffs at all. But rather than fail to filter by diff, because the filter variable is eventually set, we mistakenly show no commits at all, thinking that the empty diffs were cases where nothing passed through the filter. The smallest fix here is to just have our check look for any bits in either "filter" or "filter_not". I suspect it would also be OK to reorder the function a bit to call diff_setup_done() earlier, but that risks violating some other subtle ordering dependency. So I went with the simple and safe solution here. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 2 +- t/t4202-log.sh | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index 474fa1e767c8d8..0f9da40d8cb8c3 100644 --- a/revision.c +++ b/revision.c @@ -3112,7 +3112,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s /* Pickaxe, diff-filter and rename following need diffs */ if ((revs->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK) || - revs->diffopt.filter || + revs->diffopt.filter || revs->diffopt.filter_not || revs->diffopt.flags.follow_renames) revs->diff = 1; diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 51f7beb59f88c8..fed69717a0a91c 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -134,6 +134,12 @@ test_expect_success 'diff-filter=D' ' ' +test_expect_success 'all-negative filter' ' + git log --no-renames --format=%s --diff-filter=d HEAD >actual && + printf "%s\n" fifth fourth third second initial >expect && + test_cmp expect actual +' + test_expect_success 'diff-filter=R' ' git log -M --pretty="format:%s" --diff-filter=R HEAD >actual && From 57391a96fb3dea31d0f89861b66d59b063e9eb7a Mon Sep 17 00:00:00 2001 From: "Raymond E. Pasco" Date: Mon, 7 Jul 2025 08:12:30 -0400 Subject: [PATCH 19/40] apply: read in the index in --intent-to-add mode There are three main modes of operation for apply: applying only to the worktree, applying to the worktree and index (--index), and applying only to the index (--cached). The --intent-to-add flag modifies the first of these modes, applying only to the worktree, in a way which touches the index, because intents to add are special index entries. However, since its introduction in cff5dc09ed (apply: add --intent-to-add, 2018-05-26), it has not worked correctly in any but the most trivial (empty repository) cases, because the index is never read in (in apply, this is done in read_apply_cache()) before writing to it. This causes the operation to clobber the old, correct index with a new empty-tree index before writing intent-to-add entries to this empty index; the final result is that the index now records every existing file in the repository as deleted, which is incorrect. This error can be corrected by first reading the index. The update_index flag is correctly set if ita_only is true, because this flag causes the index to be updated. However, if we merely gate the call to read_apply_cache() behind update_index, then it will not be read when state->apply is false, even if it must be checked due to being in --index or --cached mode. Therefore, we instead read the index if it will be either checked or updated, because reading the index is a prerequisite to either. Reported-by: Ryan Hodges Original-patch-by: Johannes Altmanninger Signed-off-by: Raymond E. Pasco Signed-off-by: Junio C Hamano --- apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apply.c b/apply.c index 8bbe6ed224032e..c8d4517c0a299f 100644 --- a/apply.c +++ b/apply.c @@ -4833,7 +4833,7 @@ static int apply_patch(struct apply_state *state, LOCK_DIE_ON_ERROR); } - if (state->check_index && read_apply_cache(state) < 0) { + if ((state->check_index || state->update_index) && read_apply_cache(state) < 0) { error(_("unable to read index file")); res = -128; goto end; From 7c6e61f877fc8eee250f1fa3537434c0ff62ac1f Mon Sep 17 00:00:00 2001 From: "Raymond E. Pasco" Date: Mon, 7 Jul 2025 08:12:31 -0400 Subject: [PATCH 20/40] apply: only write intents to add for new files In the "apply only to files" mode (i.e., neither --index nor --cached mode), the index should not be touched except to record intents to add when --intent-to-add is on. Because having --intent-to-add on sets update_index, to indicate that we may touch the index, we can't rely only on that flag in create_file() (which is called to write both new files and updated files) to decide whether to write an index entry; if we did, we would write an index entry for every file being patched (which would moreover be an intent-to-add entry despite not being a new file, because we are going to turn on the CE_INTENT_TO_ADD flag in add_index_entry() if we enter it here and ita_only is true). To decide whether to touch the index, we need to check the specific reason the index would be updated, rather than merely their aggregate in the update_index flag. Because we have already entered write_out_results() and are performing writes, we know that state->apply is true. If state->check_index is additionally true, we are in --index or --cached mode, which updates the index and should always write, whereas if we are merely in ita_only mode we must only write if the patch is a new file creation patch. Signed-off-by: Raymond E. Pasco Signed-off-by: Junio C Hamano --- apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apply.c b/apply.c index c8d4517c0a299f..8637ad4c9fc9f0 100644 --- a/apply.c +++ b/apply.c @@ -4565,7 +4565,7 @@ static int create_file(struct apply_state *state, struct patch *patch) if (patch->conflicted_threeway) return add_conflicted_stages_file(state, patch); - else if (state->update_index) + else if (state->check_index || (state->ita_only && patch->is_new > 0)) return add_index_file(state, path, mode, buf, size); return 0; } From a4c969aa0d9c98ed29ea495fa0bae61bfbc713fe Mon Sep 17 00:00:00 2001 From: "Raymond E. Pasco" Date: Mon, 7 Jul 2025 08:12:32 -0400 Subject: [PATCH 21/40] t4140: test apply --intent-to-add interactions Test that applying a new file creation patch with --intent-to-add to an existing index does not modify the index outside adding the correct intents-to-add, and that applying a patch with both modifications and new file creations with --intent-to-add correctly only adds intents-to-add to the index. Signed-off-by: Raymond E. Pasco Signed-off-by: Junio C Hamano --- t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh index c614eaf04cca93..0b11a8aef4fa12 100755 --- a/t/t4140-apply-ita.sh +++ b/t/t4140-apply-ita.sh @@ -7,6 +7,10 @@ test_description='git apply of i-t-a file' test_expect_success setup ' test_write_lines 1 2 3 4 5 >blueprint && + cat blueprint >committed-file && + git add committed-file && + git commit -m "commit" && + cat blueprint >test-file && git add -N test-file && git diff >creation-patch && @@ -14,7 +18,14 @@ test_expect_success setup ' rm -f test-file && git diff >deletion-patch && - grep "deleted file mode 100644" deletion-patch + grep "deleted file mode 100644" deletion-patch && + + git rm -f test-file && + test_write_lines 6 >>committed-file && + cat blueprint >test-file && + git add -N test-file && + git diff >complex-patch && + git restore committed-file ' test_expect_success 'apply creation patch to ita path (--cached)' ' @@ -53,4 +64,22 @@ test_expect_success 'apply deletion patch to ita path (--index)' ' git ls-files --stage --error-unmatch test-file ' +test_expect_success 'apply creation patch to existing index with -N' ' + git rm -f test-file && + cat blueprint >index-file && + git add index-file && + git apply -N creation-patch && + + git ls-files --stage --error-unmatch index-file && + git ls-files --stage --error-unmatch test-file +' + +test_expect_success 'apply complex patch with -N' ' + git rm -f test-file index-file && + git apply -N complex-patch && + + git ls-files --stage --error-unmatch test-file && + git diff | grep "a/committed-file" +' + test_done From 2b49d97fcb8fb2f39ded5b00f0f1a8824267c89e Mon Sep 17 00:00:00 2001 From: "Raymond E. Pasco" Date: Mon, 7 Jul 2025 08:12:33 -0400 Subject: [PATCH 22/40] apply docs: clarify wording for --intent-to-add Avoid using a double negative, and keep in mind that --index and --cached are distinct modes of operation. Signed-off-by: Raymond E. Pasco Signed-off-by: Junio C Hamano --- Documentation/git-apply.adoc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/git-apply.adoc b/Documentation/git-apply.adoc index 952518b8af6fc0..6c71ee69da977d 100644 --- a/Documentation/git-apply.adoc +++ b/Documentation/git-apply.adoc @@ -75,13 +75,14 @@ OPTIONS tree. If `--check` is in effect, merely check that it would apply cleanly to the index entry. +-N:: --intent-to-add:: When applying the patch only to the working tree, mark new files to be added to the index later (see `--intent-to-add` - option in linkgit:git-add[1]). This option is ignored unless - running in a Git repository and `--index` is not specified. - Note that `--index` could be implied by other options such - as `--cached` or `--3way`. + option in linkgit:git-add[1]). This option is ignored if + `--index` or `--cached` are used, and has no effect outside a Git + repository. Note that `--index` could be implied by other options + such as `--3way`. -3:: --3way:: From 9455397a5cfcef186fab3321b3fbe29fd3ef78c7 Mon Sep 17 00:00:00 2001 From: Han Young Date: Thu, 3 Jul 2025 15:45:02 +0800 Subject: [PATCH 23/40] read-cache: report lock error when refreshing index In the repo_refresh_and_write_index of read-cache.c, we return -1 to indicate that writing the index to disk failed. However, callers do not use this information. Commands such as stash print "could not write index" and then exit, which does not help to discover the exact problem. We can let repo_hold_locked_index print the error message if the locking failed. Signed-off-by: Han Young Signed-off-by: Junio C Hamano --- read-cache.c | 3 ++- t/t3903-stash.sh | 18 ++++++------------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/read-cache.c b/read-cache.c index c0bb760ad473ef..c44efc89324aa7 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1456,7 +1456,8 @@ int repo_refresh_and_write_index(struct repository *repo, struct lock_file lock_file = LOCK_INIT; int fd, ret = 0; - fd = repo_hold_locked_index(repo, &lock_file, 0); + fd = repo_hold_locked_index(repo, &lock_file, + gentle ? 0 : LOCK_REPORT_ON_ERROR); if (!gentle && fd < 0) return -1; if (refresh_index(repo->index, refresh_flags, pathspec, seen, header_msg)) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 74666ff3e4b2b8..72ab9da0d55222 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1549,11 +1549,9 @@ test_expect_success 'stash create reports a locked index' ' echo change >A.file && touch .git/index.lock && - cat >expect <<-EOF && - error: could not write index - EOF test_must_fail git stash create 2>err && - test_cmp expect err + test_grep "error: could not write index" err && + test_grep "error: Unable to create '.*index.lock'" err ) ' @@ -1566,11 +1564,9 @@ test_expect_success 'stash push reports a locked index' ' echo change >A.file && touch .git/index.lock && - cat >expect <<-EOF && - error: could not write index - EOF test_must_fail git stash push 2>err && - test_cmp expect err + test_grep "error: could not write index" err && + test_grep "error: Unable to create '.*index.lock'" err ) ' @@ -1584,11 +1580,9 @@ test_expect_success 'stash apply reports a locked index' ' git stash push && touch .git/index.lock && - cat >expect <<-EOF && - error: could not write index - EOF test_must_fail git stash apply 2>err && - test_cmp expect err + test_grep "error: could not write index" err && + test_grep "error: Unable to create '.*index.lock'" err ) ' From ba472ab2f1b1af8ccb9768859fa56e2d136a759e Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:27:41 +0800 Subject: [PATCH 24/40] string-list: fix sign compare warnings for loop iterator There are a couple of "-Wsign-compare" warnings in "string-list.c". Fix trivial ones that result from a mismatched loop iterator type. There is a single warning left after these fixes. This warning needs a bit more care and is thus handled in subsequent commits. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- string-list.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/string-list.c b/string-list.c index bf061fec563d74..801ece0cbad39e 100644 --- a/string-list.c +++ b/string-list.c @@ -116,9 +116,9 @@ struct string_list_item *string_list_lookup(struct string_list *list, const char void string_list_remove_duplicates(struct string_list *list, int free_util) { if (list->nr > 1) { - int src, dst; + size_t dst = 1; compare_strings_fn cmp = list->cmp ? list->cmp : strcmp; - for (src = dst = 1; src < list->nr; src++) { + for (size_t src = 1; src < list->nr; src++) { if (!cmp(list->items[dst - 1].string, list->items[src].string)) { if (list->strdup_strings) free(list->items[src].string); @@ -134,8 +134,8 @@ void string_list_remove_duplicates(struct string_list *list, int free_util) int for_each_string_list(struct string_list *list, string_list_each_func_t fn, void *cb_data) { - int i, ret = 0; - for (i = 0; i < list->nr; i++) + int ret = 0; + for (size_t i = 0; i < list->nr; i++) if ((ret = fn(&list->items[i], cb_data))) break; return ret; @@ -144,8 +144,8 @@ int for_each_string_list(struct string_list *list, void filter_string_list(struct string_list *list, int free_util, string_list_each_func_t want, void *cb_data) { - int src, dst = 0; - for (src = 0; src < list->nr; src++) { + size_t dst = 0; + for (size_t src = 0; src < list->nr; src++) { if (want(&list->items[src], cb_data)) { list->items[dst++] = list->items[src]; } else { @@ -171,13 +171,12 @@ void string_list_remove_empty_items(struct string_list *list, int free_util) void string_list_clear(struct string_list *list, int free_util) { if (list->items) { - int i; if (list->strdup_strings) { - for (i = 0; i < list->nr; i++) + for (size_t i = 0; i < list->nr; i++) free(list->items[i].string); } if (free_util) { - for (i = 0; i < list->nr; i++) + for (size_t i = 0; i < list->nr; i++) free(list->items[i].util); } free(list->items); @@ -189,13 +188,12 @@ void string_list_clear(struct string_list *list, int free_util) void string_list_clear_func(struct string_list *list, string_list_clear_func_t clearfunc) { if (list->items) { - int i; if (clearfunc) { - for (i = 0; i < list->nr; i++) + for (size_t i = 0; i < list->nr; i++) clearfunc(list->items[i].util, list->items[i].string); } if (list->strdup_strings) { - for (i = 0; i < list->nr; i++) + for (size_t i = 0; i < list->nr; i++) free(list->items[i].string); } free(list->items); From 394e063bf9ab88f2117fe8bb8115809420ecfeda Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:27:49 +0800 Subject: [PATCH 25/40] string-list: remove unused "insert_at" parameter from add_entry In "add_entry", we accept "insert_at" parameter which must be either -1 (auto) or between 0 and `list->nr` inclusive. Any other value is invalid. When caller specify any invalid "insert_at" value, we won't check the range and move the element, which would definitely cause the trouble. However, we only use "add_entry" in "string_list_insert" function and we always pass the "-1" for "insert_at" parameter. So, we never use this parameter to insert element in a user specified position. And we should know why there is such code path in the first place. We used to have another function "string_list_insert_at_index()", which uses the extra "insert_at" parameter. And in f8c4ab611a (string_list: remove string_list_insert_at_index() from its API, 2014-11-24), we remove this function but we don't clean all the code path. Let's simply delete this parameter as we'd better use "strmap" for such functionality. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- string-list.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/string-list.c b/string-list.c index 801ece0cbad39e..8540c29bc9e255 100644 --- a/string-list.c +++ b/string-list.c @@ -41,10 +41,10 @@ static int get_entry_index(const struct string_list *list, const char *string, } /* returns -1-index if already exists */ -static int add_entry(int insert_at, struct string_list *list, const char *string) +static int add_entry(struct string_list *list, const char *string) { int exact_match = 0; - int index = insert_at != -1 ? insert_at : get_entry_index(list, string, &exact_match); + int index = get_entry_index(list, string, &exact_match); if (exact_match) return -1 - index; @@ -63,7 +63,7 @@ static int add_entry(int insert_at, struct string_list *list, const char *string struct string_list_item *string_list_insert(struct string_list *list, const char *string) { - int index = add_entry(-1, list, string); + int index = add_entry(list, string); if (index < 0) index = -1 - index; From 885becd9c4aec6c3764b4c701baf41853b49899e Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:27:57 +0800 Subject: [PATCH 26/40] string-list: return index directly when inserting an existing element When inserting an existing element, "add_entry" would convert "index" value to "-1-index" to indicate the caller that this element is in the list already. However, in "string_list_insert", we would simply convert this to the original positive index without any further action. In 8fd2cb4069 (Extract helper bits from c-merge-recursive work, 2006-07-25), we create "path-list.c" and then introduce above code path. Let's directly return the index as we don't care about whether the element is in the list by using "add_entry". In the future, if we want to let "add_entry" tell the caller, we may add "int *exact_match" parameter to "add_entry" instead of converting the index to negative to indicate. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- string-list.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/string-list.c b/string-list.c index 8540c29bc9e255..171cef5dbbd9ba 100644 --- a/string-list.c +++ b/string-list.c @@ -40,14 +40,13 @@ static int get_entry_index(const struct string_list *list, const char *string, return right; } -/* returns -1-index if already exists */ static int add_entry(struct string_list *list, const char *string) { int exact_match = 0; int index = get_entry_index(list, string, &exact_match); if (exact_match) - return -1 - index; + return index; ALLOC_GROW(list->items, list->nr+1, list->alloc); if (index < list->nr) @@ -65,9 +64,6 @@ struct string_list_item *string_list_insert(struct string_list *list, const char { int index = add_entry(list, string); - if (index < 0) - index = -1 - index; - return list->items + index; } From 67cfd2924d099b3316ddf579ba7c11fdc29ad2b6 Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:28:06 +0800 Subject: [PATCH 27/40] string-list: enable sign compare warnings check In "add_entry", we call "get_entry_index" function to get the inserted position. However, as the return type of "get_entry_index" function is `int`, there is a sign compare warning when comparing the `index` with the `list-nr` of unsigned type. "get_entry_index" would always return unsigned index. However, the current binary search algorithm initializes "left" to be "-1", which necessitates the use of signed `int` return type. The reason why we need to assign "left" to be "-1" is that in the `while` loop, we increment "left" by 1 to determine whether the loop should end. This design choice, while functional, forces us to use signed arithmetic throughout the function. To resolve this sign comparison issue, let's modify the binary search algorithm with the following approach: 1. Initialize "left" to 0 instead of -1 2. Use `left < right` as the loop termination condition instead of `left + 1 < right` 3. When searching the right part, set `left = middle + 1` instead of `middle` Then, we could delete "#define DISABLE_SIGN_COMPARE_WARNING" to enable sign warnings check for "string-list". Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- string-list.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/string-list.c b/string-list.c index 171cef5dbbd9ba..53faaa84207bf9 100644 --- a/string-list.c +++ b/string-list.c @@ -1,5 +1,3 @@ -#define DISABLE_SIGN_COMPARE_WARNINGS - #include "git-compat-util.h" #include "string-list.h" @@ -17,19 +15,19 @@ void string_list_init_dup(struct string_list *list) /* if there is no exact match, point to the index where the entry could be * inserted */ -static int get_entry_index(const struct string_list *list, const char *string, - int *exact_match) +static size_t get_entry_index(const struct string_list *list, const char *string, + int *exact_match) { - int left = -1, right = list->nr; + size_t left = 0, right = list->nr; compare_strings_fn cmp = list->cmp ? list->cmp : strcmp; - while (left + 1 < right) { - int middle = left + (right - left) / 2; + while (left < right) { + size_t middle = left + (right - left) / 2; int compare = cmp(string, list->items[middle].string); if (compare < 0) right = middle; else if (compare > 0) - left = middle; + left = middle + 1; else { *exact_match = 1; return middle; @@ -40,10 +38,10 @@ static int get_entry_index(const struct string_list *list, const char *string, return right; } -static int add_entry(struct string_list *list, const char *string) +static size_t add_entry(struct string_list *list, const char *string) { int exact_match = 0; - int index = get_entry_index(list, string, &exact_match); + size_t index = get_entry_index(list, string, &exact_match); if (exact_match) return index; @@ -62,7 +60,7 @@ static int add_entry(struct string_list *list, const char *string) struct string_list_item *string_list_insert(struct string_list *list, const char *string) { - int index = add_entry(list, string); + size_t index = add_entry(list, string); return list->items + index; } From 07d90fda58c79af661016137cbbafab169eeb329 Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:28:14 +0800 Subject: [PATCH 28/40] u-string-list: move "test_split" into "u-string-list.c" We rely on "test-tool string-list" command to test the functionality of the "string-list". However, as we have introduced clar test framework, we'd better move the shell script into C program to improve speed and readability. Create a new file "u-string-list.c" under "t/unit-tests", then update the Makefile and "meson.build" to build the file. And let's first move "test_split" into unit test and gradually convert the shell script into C program. In order to create `string_list` easily by simply specifying strings in the function call, create "t_vcreate_string_list_dup" function to do this. Then port the shell script tests to C program and remove unused "test-tool" code and tests. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- Makefile | 1 + t/helper/test-string-list.c | 14 --------- t/meson.build | 1 + t/t0063-string-list.sh | 53 ---------------------------------- t/unit-tests/u-string-list.c | 55 ++++++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 67 deletions(-) create mode 100644 t/unit-tests/u-string-list.c diff --git a/Makefile b/Makefile index 70d1543b6b8688..744f060e5394ba 100644 --- a/Makefile +++ b/Makefile @@ -1367,6 +1367,7 @@ CLAR_TEST_SUITES += u-prio-queue CLAR_TEST_SUITES += u-reftable-tree CLAR_TEST_SUITES += u-strbuf CLAR_TEST_SUITES += u-strcmp-offset +CLAR_TEST_SUITES += u-string-list CLAR_TEST_SUITES += u-strvec CLAR_TEST_SUITES += u-trailer CLAR_TEST_SUITES += u-urlmatch-normalization diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c index 6f10c5a43540b8..17c18c30f6f5bc 100644 --- a/t/helper/test-string-list.c +++ b/t/helper/test-string-list.c @@ -46,20 +46,6 @@ static int prefix_cb(struct string_list_item *item, void *cb_data) int cmd__string_list(int argc, const char **argv) { - if (argc == 5 && !strcmp(argv[1], "split")) { - struct string_list list = STRING_LIST_INIT_DUP; - int i; - const char *s = argv[2]; - int delim = *argv[3]; - int maxsplit = atoi(argv[4]); - - i = string_list_split(&list, s, delim, maxsplit); - printf("%d\n", i); - write_list(&list); - string_list_clear(&list, 0); - return 0; - } - if (argc == 5 && !strcmp(argv[1], "split_in_place")) { struct string_list list = STRING_LIST_INIT_NODUP; int i; diff --git a/t/meson.build b/t/meson.build index d052fc3e23d2ec..ae65445f7192f3 100644 --- a/t/meson.build +++ b/t/meson.build @@ -11,6 +11,7 @@ clar_test_suites = [ 'unit-tests/u-reftable-tree.c', 'unit-tests/u-strbuf.c', 'unit-tests/u-strcmp-offset.c', + 'unit-tests/u-string-list.c', 'unit-tests/u-strvec.c', 'unit-tests/u-trailer.c', 'unit-tests/u-urlmatch-normalization.c', diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh index aac63ba5064b78..6b20ffd206c769 100755 --- a/t/t0063-string-list.sh +++ b/t/t0063-string-list.sh @@ -7,16 +7,6 @@ test_description='Test string list functionality' . ./test-lib.sh -test_split () { - cat >expected && - test_expect_success "split $1 at $2, max $3" " - test-tool string-list split '$1' '$2' '$3' >actual && - test_cmp expected actual && - test-tool string-list split_in_place '$1' '$2' '$3' >actual && - test_cmp expected actual - " -} - test_split_in_place() { cat >expected && test_expect_success "split (in place) $1 at $2, max $3" " @@ -25,49 +15,6 @@ test_split_in_place() { " } -test_split "foo:bar:baz" ":" "-1" <strdup_strings); + + string_list_clear(list, free_util); + while ((arg = va_arg(ap, const char *))) + string_list_append(list, arg); +} + +static void t_string_list_equal(struct string_list *list, + struct string_list *expected_strings) +{ + cl_assert_equal_i(list->nr, expected_strings->nr); + cl_assert(list->nr <= list->alloc); + for (size_t i = 0; i < expected_strings->nr; i++) + cl_assert_equal_s(list->items[i].string, + expected_strings->items[i].string); +} + +static void t_string_list_split(const char *data, int delim, int maxsplit, ...) +{ + struct string_list expected_strings = STRING_LIST_INIT_DUP; + struct string_list list = STRING_LIST_INIT_DUP; + va_list ap; + int len; + + va_start(ap, maxsplit); + t_vcreate_string_list_dup(&expected_strings, 0, ap); + va_end(ap); + + string_list_clear(&list, 0); + len = string_list_split(&list, data, delim, maxsplit); + cl_assert_equal_i(len, expected_strings.nr); + t_string_list_equal(&list, &expected_strings); + + string_list_clear(&expected_strings, 0); + string_list_clear(&list, 0); +} + +void test_string_list__split(void) +{ + t_string_list_split("foo:bar:baz", ':', -1, "foo", "bar", "baz", NULL); + t_string_list_split("foo:bar:baz", ':', 0, "foo:bar:baz", NULL); + t_string_list_split("foo:bar:baz", ':', 1, "foo", "bar:baz", NULL); + t_string_list_split("foo:bar:baz", ':', 2, "foo", "bar", "baz", NULL); + t_string_list_split("foo:bar:", ':', -1, "foo", "bar", "", NULL); + t_string_list_split("", ':', -1, "", NULL); + t_string_list_split(":", ':', -1, "", "", NULL); +} From 62c514a9efd2206e081509ca3abc9cd5645eff0b Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:28:23 +0800 Subject: [PATCH 29/40] u-string-list: move "test_split_in_place" to "u-string-list.c" We use "test-tool string-list split_in_place" to test the "string_list_split_in_place" function. As we have introduced the unit test, we'd better remove the logic from shell script to C program to improve test speed and readability. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- t/helper/test-string-list.c | 22 ---------------- t/t0063-string-list.sh | 51 ------------------------------------ t/unit-tests/u-string-list.c | 37 ++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 73 deletions(-) diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c index 17c18c30f6f5bc..8a344347ada196 100644 --- a/t/helper/test-string-list.c +++ b/t/helper/test-string-list.c @@ -18,13 +18,6 @@ static void parse_string_list(struct string_list *list, const char *arg) (void)string_list_split(list, arg, ':', -1); } -static void write_list(const struct string_list *list) -{ - int i; - for (i = 0; i < list->nr; i++) - printf("[%d]: \"%s\"\n", i, list->items[i].string); -} - static void write_list_compact(const struct string_list *list) { int i; @@ -46,21 +39,6 @@ static int prefix_cb(struct string_list_item *item, void *cb_data) int cmd__string_list(int argc, const char **argv) { - if (argc == 5 && !strcmp(argv[1], "split_in_place")) { - struct string_list list = STRING_LIST_INIT_NODUP; - int i; - char *s = xstrdup(argv[2]); - const char *delim = argv[3]; - int maxsplit = atoi(argv[4]); - - i = string_list_split_in_place(&list, s, delim, maxsplit); - printf("%d\n", i); - write_list(&list); - string_list_clear(&list, 0); - free(s); - return 0; - } - if (argc == 4 && !strcmp(argv[1], "filter")) { /* * Retain only the items that have the specified prefix. diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh index 6b20ffd206c769..1a9cf8bfcf35e9 100755 --- a/t/t0063-string-list.sh +++ b/t/t0063-string-list.sh @@ -7,57 +7,6 @@ test_description='Test string list functionality' . ./test-lib.sh -test_split_in_place() { - cat >expected && - test_expect_success "split (in place) $1 at $2, max $3" " - test-tool string-list split_in_place '$1' '$2' '$3' >actual && - test_cmp expected actual - " -} - -test_split_in_place "foo:;:bar:;:baz:;:" ":;" "-1" < Date: Sun, 29 Jun 2025 12:28:32 +0800 Subject: [PATCH 30/40] u-string-list: move "filter string" test to "u-string-list.c" We use "test-tool string-list filter" to test the "filter_string_list" function. As we have introduced the unit test, we'd better remove the logic from shell script to C program to improve test speed and readability. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- t/helper/test-string-list.c | 21 ----------- t/t0063-string-list.sh | 11 ------ t/unit-tests/u-string-list.c | 73 ++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c index 8a344347ada196..262b28c599310a 100644 --- a/t/helper/test-string-list.c +++ b/t/helper/test-string-list.c @@ -31,29 +31,8 @@ static void write_list_compact(const struct string_list *list) } } -static int prefix_cb(struct string_list_item *item, void *cb_data) -{ - const char *prefix = (const char *)cb_data; - return starts_with(item->string, prefix); -} - int cmd__string_list(int argc, const char **argv) { - if (argc == 4 && !strcmp(argv[1], "filter")) { - /* - * Retain only the items that have the specified prefix. - * Arguments: list|- prefix - */ - struct string_list list = STRING_LIST_INIT_DUP; - const char *prefix = argv[3]; - - parse_string_list(&list, argv[2]); - filter_string_list(&list, 0, prefix_cb, (void *)prefix); - write_list_compact(&list); - string_list_clear(&list, 0); - return 0; - } - if (argc == 3 && !strcmp(argv[1], "remove_duplicates")) { struct string_list list = STRING_LIST_INIT_DUP; diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh index 1a9cf8bfcf35e9..31fd62bba87954 100755 --- a/t/t0063-string-list.sh +++ b/t/t0063-string-list.sh @@ -7,17 +7,6 @@ test_description='Test string list functionality' . ./test-lib.sh -test_expect_success "test filter_string_list" ' - test "x-" = "x$(test-tool string-list filter - y)" && - test "x-" = "x$(test-tool string-list filter no y)" && - test yes = "$(test-tool string-list filter yes y)" && - test yes = "$(test-tool string-list filter no:yes y)" && - test yes = "$(test-tool string-list filter yes:no y)" && - test y1:y2 = "$(test-tool string-list filter y1:y2 y)" && - test y2:y1 = "$(test-tool string-list filter y2:y1 y)" && - test "x-" = "x$(test-tool string-list filter x1:x2 y)" -' - test_expect_success "test remove_duplicates" ' test "x-" = "x$(test-tool string-list remove_duplicates -)" && test "x" = "x$(test-tool string-list remove_duplicates "")" && diff --git a/t/unit-tests/u-string-list.c b/t/unit-tests/u-string-list.c index d2761e1f2fb21e..f061a3694b5e5f 100644 --- a/t/unit-tests/u-string-list.c +++ b/t/unit-tests/u-string-list.c @@ -13,6 +13,26 @@ static void t_vcreate_string_list_dup(struct string_list *list, string_list_append(list, arg); } +static void t_create_string_list_dup(struct string_list *list, int free_util, ...) +{ + va_list ap; + + cl_assert(list->strdup_strings); + + string_list_clear(list, free_util); + va_start(ap, free_util); + t_vcreate_string_list_dup(list, free_util, ap); + va_end(ap); +} + +static void t_string_list_clear(struct string_list *list, int free_util) +{ + string_list_clear(list, free_util); + cl_assert_equal_p(list->items, NULL); + cl_assert_equal_i(list->nr, 0); + cl_assert_equal_i(list->alloc, 0); +} + static void t_string_list_equal(struct string_list *list, struct string_list *expected_strings) { @@ -90,3 +110,56 @@ void test_string_list__split_in_place(void) t_string_list_split_in_place("foo:;:bar:;:", ":;", -1, "foo", "", "", "bar", "", "", "", NULL); } + +static int prefix_cb(struct string_list_item *item, void *cb_data) +{ + const char *prefix = (const char *)cb_data; + return starts_with(item->string, prefix); +} + +static void t_string_list_filter(struct string_list *list, ...) +{ + struct string_list expected_strings = STRING_LIST_INIT_DUP; + const char *prefix = "y"; + va_list ap; + + va_start(ap, list); + t_vcreate_string_list_dup(&expected_strings, 0, ap); + va_end(ap); + + filter_string_list(list, 0, prefix_cb, (void *)prefix); + t_string_list_equal(list, &expected_strings); + + string_list_clear(&expected_strings, 0); +} + +void test_string_list__filter(void) +{ + struct string_list list = STRING_LIST_INIT_DUP; + + t_create_string_list_dup(&list, 0, NULL); + t_string_list_filter(&list, NULL); + + t_create_string_list_dup(&list, 0, "no", NULL); + t_string_list_filter(&list, NULL); + + t_create_string_list_dup(&list, 0, "yes", NULL); + t_string_list_filter(&list, "yes", NULL); + + t_create_string_list_dup(&list, 0, "no", "yes", NULL); + t_string_list_filter(&list, "yes", NULL); + + t_create_string_list_dup(&list, 0, "yes", "no", NULL); + t_string_list_filter(&list, "yes", NULL); + + t_create_string_list_dup(&list, 0, "y1", "y2", NULL); + t_string_list_filter(&list, "y1", "y2", NULL); + + t_create_string_list_dup(&list, 0, "y2", "y1", NULL); + t_string_list_filter(&list, "y2", "y1", NULL); + + t_create_string_list_dup(&list, 0, "x1", "x2", NULL); + t_string_list_filter(&list, NULL); + + t_string_list_clear(&list, 0); +} From 6e5b26c3ff639211147ccb2b1ca681c768b8db11 Mon Sep 17 00:00:00 2001 From: shejialuo Date: Sun, 29 Jun 2025 12:28:41 +0800 Subject: [PATCH 31/40] u-string-list: move "remove duplicates" test to "u-string-list.c" We use "test-tool string-list remove_duplicates" to test the "string_list_remove_duplicates" function. As we have introduced the unit test, we'd better remove the logic from shell script to C program to improve test speed and readability. As all the tests in shell script are removed, let's just delete the "t0063-string-list.sh" and update the "meson.build" file to align with this change. Also we could simply remove "DISABLE_SIGN_COMPARE_WARNINGS" due to we have already deleted related code. Unfortunately, we cannot totally remove "test-string-list.c" due to that we would test the performance of sorting about string list by executing "test-tool string-list sort" in "p0071-sort.sh". Signed-off-by: shejialuo Signed-off-by: Junio C Hamano --- t/helper/test-string-list.c | 39 ----------------------- t/meson.build | 1 - t/t0063-string-list.sh | 27 ---------------- t/unit-tests/u-string-list.c | 62 ++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 67 deletions(-) delete mode 100755 t/t0063-string-list.sh diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c index 262b28c599310a..6be0cdb8e2006a 100644 --- a/t/helper/test-string-list.c +++ b/t/helper/test-string-list.c @@ -1,48 +1,9 @@ -#define DISABLE_SIGN_COMPARE_WARNINGS - #include "test-tool.h" #include "strbuf.h" #include "string-list.h" -/* - * Parse an argument into a string list. arg should either be a - * ':'-separated list of strings, or "-" to indicate an empty string - * list (as opposed to "", which indicates a string list containing a - * single empty string). list->strdup_strings must be set. - */ -static void parse_string_list(struct string_list *list, const char *arg) -{ - if (!strcmp(arg, "-")) - return; - - (void)string_list_split(list, arg, ':', -1); -} - -static void write_list_compact(const struct string_list *list) -{ - int i; - if (!list->nr) - printf("-\n"); - else { - printf("%s", list->items[0].string); - for (i = 1; i < list->nr; i++) - printf(":%s", list->items[i].string); - printf("\n"); - } -} - int cmd__string_list(int argc, const char **argv) { - if (argc == 3 && !strcmp(argv[1], "remove_duplicates")) { - struct string_list list = STRING_LIST_INIT_DUP; - - parse_string_list(&list, argv[2]); - string_list_remove_duplicates(&list, 0); - write_list_compact(&list); - string_list_clear(&list, 0); - return 0; - } - if (argc == 2 && !strcmp(argv[1], "sort")) { struct string_list list = STRING_LIST_INIT_NODUP; struct strbuf sb = STRBUF_INIT; diff --git a/t/meson.build b/t/meson.build index ae65445f7192f3..586084fdf24640 100644 --- a/t/meson.build +++ b/t/meson.build @@ -124,7 +124,6 @@ integration_tests = [ 't0060-path-utils.sh', 't0061-run-command.sh', 't0062-revision-walking.sh', - 't0063-string-list.sh', 't0066-dir-iterator.sh', 't0067-parse_pathspec_file.sh', 't0068-for-each-repo.sh', diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh deleted file mode 100755 index 31fd62bba87954..00000000000000 --- a/t/t0063-string-list.sh +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2012 Michael Haggerty -# - -test_description='Test string list functionality' - -. ./test-lib.sh - -test_expect_success "test remove_duplicates" ' - test "x-" = "x$(test-tool string-list remove_duplicates -)" && - test "x" = "x$(test-tool string-list remove_duplicates "")" && - test a = "$(test-tool string-list remove_duplicates a)" && - test a = "$(test-tool string-list remove_duplicates a:a)" && - test a = "$(test-tool string-list remove_duplicates a:a:a:a:a)" && - test a:b = "$(test-tool string-list remove_duplicates a:b)" && - test a:b = "$(test-tool string-list remove_duplicates a:a:b)" && - test a:b = "$(test-tool string-list remove_duplicates a:b:b)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:b:c)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:a:b:c)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:b:b:c)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:b:c:c)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:a:b:b:c:c)" && - test a:b:c = "$(test-tool string-list remove_duplicates a:a:a:b:b:b:c:c:c)" -' - -test_done diff --git a/t/unit-tests/u-string-list.c b/t/unit-tests/u-string-list.c index f061a3694b5e5f..d4ba5f9fa52aa2 100644 --- a/t/unit-tests/u-string-list.c +++ b/t/unit-tests/u-string-list.c @@ -163,3 +163,65 @@ void test_string_list__filter(void) t_string_list_clear(&list, 0); } + +static void t_string_list_remove_duplicates(struct string_list *list, ...) +{ + struct string_list expected_strings = STRING_LIST_INIT_DUP; + va_list ap; + + va_start(ap, list); + t_vcreate_string_list_dup(&expected_strings, 0, ap); + va_end(ap); + + string_list_remove_duplicates(list, 0); + t_string_list_equal(list, &expected_strings); + + string_list_clear(&expected_strings, 0); +} + +void test_string_list__remove_duplicates(void) +{ + struct string_list list = STRING_LIST_INIT_DUP; + + t_create_string_list_dup(&list, 0, NULL); + t_string_list_remove_duplicates(&list, NULL); + + t_create_string_list_dup(&list, 0, "", NULL); + t_string_list_remove_duplicates(&list, "", NULL); + + t_create_string_list_dup(&list, 0, "a", NULL); + t_string_list_remove_duplicates(&list, "a", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", NULL); + t_string_list_remove_duplicates(&list, "a", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", "a", NULL); + t_string_list_remove_duplicates(&list, "a", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", "b", NULL); + t_string_list_remove_duplicates(&list, "a", "b", NULL); + + t_create_string_list_dup(&list, 0, "a", "b", "b", NULL); + t_string_list_remove_duplicates(&list, "a", "b", NULL); + + t_create_string_list_dup(&list, 0, "a", "b", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", "b", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_create_string_list_dup(&list, 0, "a", "b", "b", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_create_string_list_dup(&list, 0, "a", "b", "c", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", "b", "b", "c", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_create_string_list_dup(&list, 0, "a", "a", "a", "b", "b", "b", + "c", "c", "c", NULL); + t_string_list_remove_duplicates(&list, "a", "b", "c", NULL); + + t_string_list_clear(&list, 0); +} From 44e300a97480ef272a596e02b912b72528043193 Mon Sep 17 00:00:00 2001 From: Ayush Chandekar Date: Fri, 4 Jul 2025 19:42:34 +0530 Subject: [PATCH 32/40] repository: move 'repository_format_precious_objects' to repo scope The 'extensions.preciousObjects' setting when set true, prevents operations that might drop objects from the object storage. This setting is populated in the global variable 'repository_format_precious_objects'. Move this global variable to repo scope by adding it to 'struct repository and also refactor all the occurences accordingly. This change is part of an ongoing effort to eliminate global variables, improve modularity and help libify the codebase. Mentored-by: Christian Couder Mentored-by: Ghanshyam Thakkar Signed-off-by: Ayush Chandekar Signed-off-by: Junio C Hamano --- builtin/gc.c | 2 +- builtin/prune.c | 2 +- builtin/repack.c | 2 +- environment.c | 1 - environment.h | 2 -- repository.c | 1 + repository.h | 1 + setup.c | 5 ++++- 8 files changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 845876ff02869b..ec10b81dcc36f6 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -998,7 +998,7 @@ int cmd_gc(int argc, if (opts.detach <= 0 && !skip_foreground_tasks) gc_foreground_tasks(&opts, &cfg); - if (!repository_format_precious_objects) { + if (!the_repository->repository_format_precious_objects) { struct child_process repack_cmd = CHILD_PROCESS_INIT; repack_cmd.git_cmd = 1; diff --git a/builtin/prune.c b/builtin/prune.c index e930caa0c0af0e..dab3c19b6fdd32 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -177,7 +177,7 @@ int cmd_prune(int argc, argc = parse_options(argc, argv, prefix, options, prune_usage, 0); - if (repository_format_precious_objects) + if (the_repository->repository_format_precious_objects) die(_("cannot prune in a precious-objects repo")); while (argc--) { diff --git a/builtin/repack.c b/builtin/repack.c index 5ddc6e7f9573d4..d0e4fa6bed16e3 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -1240,7 +1240,7 @@ int cmd_repack(int argc, po_args.depth = xstrdup_or_null(opt_depth); po_args.threads = xstrdup_or_null(opt_threads); - if (delete_redundant && repository_format_precious_objects) + if (delete_redundant && the_repository->repository_format_precious_objects) die(_("cannot delete packs in a precious-objects repo")); die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A", diff --git a/environment.c b/environment.c index 7bf0390a3355d3..7c2480b22e5991 100644 --- a/environment.c +++ b/environment.c @@ -37,7 +37,6 @@ int ignore_case; int assume_unchanged; int is_bare_repository_cfg = -1; /* unspecified */ int warn_on_object_refname_ambiguity = 1; -int repository_format_precious_objects; char *git_commit_encoding; char *git_log_output_encoding; char *apply_default_whitespace; diff --git a/environment.h b/environment.h index 9a3d05d414a7ad..3d806ced6e4552 100644 --- a/environment.h +++ b/environment.h @@ -189,8 +189,6 @@ extern enum object_creation_mode object_creation_mode; extern int grafts_keep_true_parents; -extern int repository_format_precious_objects; - const char *get_log_output_encoding(void); const char *get_commit_output_encoding(void); diff --git a/repository.c b/repository.c index 9b3d6665fc6d4f..62709d1c916e1b 100644 --- a/repository.c +++ b/repository.c @@ -284,6 +284,7 @@ int repo_init(struct repository *repo, repo_set_ref_storage_format(repo, format.ref_storage_format); repo->repository_format_worktree_config = format.worktree_config; repo->repository_format_relative_worktrees = format.relative_worktrees; + repo->repository_format_precious_objects = format.precious_objects; /* take ownership of format.partial_clone */ repo->repository_format_partial_clone = format.partial_clone; diff --git a/repository.h b/repository.h index c4c92b2ab9c9e3..ad23a243c6ab92 100644 --- a/repository.h +++ b/repository.h @@ -151,6 +151,7 @@ struct repository { /* Configurations */ int repository_format_worktree_config; int repository_format_relative_worktrees; + int repository_format_precious_objects; /* Indicate if a repository has a different 'commondir' from 'gitdir' */ unsigned different_commondir:1; diff --git a/setup.c b/setup.c index f93bd6a24a5d9c..3ea01e9331149c 100644 --- a/setup.c +++ b/setup.c @@ -753,7 +753,8 @@ static int check_repository_format_gently(const char *gitdir, struct repository_ die("%s", err.buf); } - repository_format_precious_objects = candidate->precious_objects; + the_repository->repository_format_precious_objects = candidate->precious_objects; + string_list_clear(&candidate->unknown_extensions, 0); string_list_clear(&candidate->v1_only_extensions, 0); @@ -1864,6 +1865,8 @@ const char *setup_git_directory_gently(int *nongit_ok) the_repository->repository_format_partial_clone = repo_fmt.partial_clone; repo_fmt.partial_clone = NULL; + the_repository->repository_format_precious_objects = + repo_fmt.precious_objects; } } /* From 7cd03a555a0d951759a5bce201ad0686c0fc8b12 Mon Sep 17 00:00:00 2001 From: Ayush Chandekar Date: Fri, 4 Jul 2025 19:42:35 +0530 Subject: [PATCH 33/40] builtin/prune: stop depending on 'the_repository' Refactor builtin/prune.c to remove the dependency on the global 'the_repository'. Replace all the occurrences of 'the_repository' with repo and thus remove the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that 'git prune -h' can be called when the repository is `NULL`. Mentored-by: Christian Couder Mentored-by: Ghanshyam Thakkar Signed-off-by: Ayush Chandekar Signed-off-by: Junio C Hamano --- builtin/prune.c | 27 ++++++++++++--------------- t/t1517-outside-repo.sh | 7 +++++++ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/builtin/prune.c b/builtin/prune.c index dab3c19b6fdd32..320e9c234161a9 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "builtin.h" @@ -64,7 +63,7 @@ static void perform_reachability_traversal(struct rev_info *revs) return; if (show_progress) - progress = start_delayed_progress(the_repository, + progress = start_delayed_progress(revs->repo, _("Checking connectivity"), 0); mark_reachable_objects(revs, 1, expire, progress); stop_progress(&progress); @@ -78,7 +77,7 @@ static int is_object_reachable(const struct object_id *oid, perform_reachability_traversal(revs); - obj = lookup_object(the_repository, oid); + obj = lookup_object(revs->repo, oid); return obj && (obj->flags & SEEN); } @@ -99,8 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath, if (st.st_mtime > expire) return 0; if (show_only || verbose) { - enum object_type type = oid_object_info(the_repository, oid, - NULL); + enum object_type type = oid_object_info(revs->repo, oid, NULL); printf("%s %s\n", oid_to_hex(oid), (type > 0) ? type_name(type) : "unknown"); } @@ -154,7 +152,7 @@ static void remove_temporary_files(const char *path) int cmd_prune(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { struct rev_info revs; int exclude_promisor_objects = 0; @@ -173,20 +171,19 @@ int cmd_prune(int argc, expire = TIME_MAX; save_commit_buffer = 0; disable_replace_refs(); - repo_init_revisions(the_repository, &revs, prefix); argc = parse_options(argc, argv, prefix, options, prune_usage, 0); - if (the_repository->repository_format_precious_objects) + repo_init_revisions(repo, &revs, prefix); + if (repo->repository_format_precious_objects) die(_("cannot prune in a precious-objects repo")); while (argc--) { struct object_id oid; const char *name = *argv++; - if (!repo_get_oid(the_repository, name, &oid)) { - struct object *object = parse_object_or_die(the_repository, &oid, - name); + if (!repo_get_oid(repo, name, &oid)) { + struct object *object = parse_object_or_die(repo, &oid, name); add_pending_object(&revs, object, ""); } else @@ -200,16 +197,16 @@ int cmd_prune(int argc, revs.exclude_promisor_objects = 1; } - for_each_loose_file_in_objdir(repo_get_object_directory(the_repository), + for_each_loose_file_in_objdir(repo_get_object_directory(repo), prune_object, prune_cruft, prune_subdir, &revs); prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0); - remove_temporary_files(repo_get_object_directory(the_repository)); - s = mkpathdup("%s/pack", repo_get_object_directory(the_repository)); + remove_temporary_files(repo_get_object_directory(repo)); + s = mkpathdup("%s/pack", repo_get_object_directory(repo)); remove_temporary_files(s); free(s); - if (is_repository_shallow(the_repository)) { + if (is_repository_shallow(repo)) { perform_reachability_traversal(&revs); prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0); } diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh index 6824581317411a..8f59b867f2701f 100755 --- a/t/t1517-outside-repo.sh +++ b/t/t1517-outside-repo.sh @@ -114,4 +114,11 @@ test_expect_success 'update-server-info does not crash with -h' ' test_grep "[Uu]sage: git update-server-info " usage ' +test_expect_success 'prune does not crash with -h' ' + test_expect_code 129 git prune -h >usage && + test_grep "[Uu]sage: git prune " usage && + test_expect_code 129 nongit git prune -h >usage && + test_grep "[Uu]sage: git prune " usage +' + test_done From 0c83bbc70412d96953b3b648fcea1a8fa15a5ca1 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Fri, 4 Jul 2025 23:23:11 +0100 Subject: [PATCH 34/40] build: fix FreeBSD build when sysinfo compat library installed Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for cygwin", 2025-04-17) and later commit 187ce0222f ("configure.ac: upgrade to a compilation check for sysinfo", 2025-05-19) added a 'sysinfo()' check to the autoconf build. The FreeBSD system has an optional sysinfo compatibility library, used to assist in porting software, which causes the build to fail when it is installed. The reason for the failure is the lack of '-lsysinfo' during the linking step. Several solutions were considered: - add a 'linking' check to configure.ac in order to determine the need to link a separate library (-lsysinfo). (This would require a similar change to meson.build). - change the order of the preprocessor conditionals in the total_ram() function in 'builtin/gc.c', so that the *BSD sysctl() function (in the HAVE_BSD_SYSCTL block) takes priority over the sysinfo() function (in the HAVE_SYSINFO block). - suppress the setting of HAVE_SYSINFO when HAVE_BSD_SYSCTL has been defined (in both configure.ac and meson.build). The first solution above, while simple, adds unnecessary code (the sysinfo compat function is likely implemented using sysctl() anyway) when git is happy to use sysctl() on *BSD systems. The second solution would only be required by the autoconf and meson build systems, the Makefile already sets the build variables to the required values (since they are not 'auto-detected'). Here we opt for the final solution above, since it only requires that we prioritise the 'auto-detected' build variables in the autoconf and meson builds. In order to fix the FreeBSD build, move the sysinfo() check after the determination of the HAVE_BSD_SYSCTL build variable, suppressing the setting of HAVE_SYSINFO if HAVE_BSD_SYSCTL is defined. Apply this logic to both the configure.ac and meson.build file. [Thanks go to Renato Botelho for testing this patch on FreeBSD.] Tested-by: Renato Botelho Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- configure.ac | 61 ++++++++++++++++++++++++++++++---------------------- meson.build | 10 +++++---- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/configure.ac b/configure.ac index f6caab919a3e0e..bf710ac91a450d 100644 --- a/configure.ac +++ b/configure.ac @@ -1067,32 +1067,6 @@ AC_CHECK_LIB([iconv], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) -# -# Define HAVE_SYSINFO=YesPlease if sysinfo is available. -# -AC_DEFUN([HAVE_SYSINFO_SRC], [ -AC_LANG_PROGRAM([[ -#include -#include -]], [[ -struct sysinfo si; -uint64_t t = 0; -if (!sysinfo(&si)) { - t = si.totalram; - if (si.mem_unit > 1) - t *= (uint64_t)si.mem_unit; -} -return t; -]])]) - -AC_MSG_CHECKING([for sysinfo]) -AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC], - [AC_MSG_RESULT([yes]) - HAVE_SYSINFO=YesPlease], - [AC_MSG_RESULT([no]) - HAVE_SYSINFO=]) -GIT_CONF_SUBST([HAVE_SYSINFO]) - # # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available. GIT_CHECK_FUNC(clock_gettime, @@ -1221,6 +1195,41 @@ AC_COMPILE_IFELSE([BSD_SYSCTL_SRC], HAVE_BSD_SYSCTL=]) GIT_CONF_SUBST([HAVE_BSD_SYSCTL]) +# +# Define HAVE_SYSINFO=YesPlease if sysinfo is available. +# + +HAVE_SYSINFO= +# on a *BSD system, sysctl() takes precedence over the +# sysinfo() compatibility library (if installed). + +if test -z "$HAVE_BSD_SYSCTL"; then + + AC_DEFUN([HAVE_SYSINFO_SRC], [ + AC_LANG_PROGRAM([[ + #include + #include + ]], [[ + struct sysinfo si; + uint64_t t = 0; + if (!sysinfo(&si)) { + t = si.totalram; + if (si.mem_unit > 1) + t *= (uint64_t)si.mem_unit; + } + return t; + ]])]) + + AC_MSG_CHECKING([for sysinfo]) + AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC], + [AC_MSG_RESULT([yes]) + HAVE_SYSINFO=YesPlease], + [AC_MSG_RESULT([no]) + HAVE_SYSINFO=]) + GIT_CONF_SUBST([HAVE_SYSINFO]) + +fi + ## Other checks. # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link. # Enable it on Windows. By default, symrefs are still used. diff --git a/meson.build b/meson.build index 596f5ac7110ebf..70d2f86f4b0fa9 100644 --- a/meson.build +++ b/meson.build @@ -1331,10 +1331,6 @@ if host_machine.system() != 'windows' endif endif -if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ') - libgit_c_args += '-DHAVE_SYSINFO' -endif - if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include ') libgit_c_args += '-DUSE_ST_TIMESPEC' elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include ') @@ -1449,6 +1445,12 @@ if compiler.has_header('sys/sysctl.h') endif endif +if not has_bsd_sysctl + if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ') + libgit_c_args += '-DHAVE_SYSINFO' + endif +endif + if not meson.is_cross_build() and compiler.run(''' #include From 953049eed81b5da8602da97717fa225ab5bc1287 Mon Sep 17 00:00:00 2001 From: Timur Sultanaev Date: Sat, 5 Jul 2025 17:39:18 +0000 Subject: [PATCH 35/40] docs: correct ORIG_HEAD example in "git merge" documentation Documentation for git-merge incorrectly notes that tip of the current branch on ascii diagram is C, while it is actually G (current branch is master, HEAD on diagram is G). Additionally diagrams on the page are adjusted to use spaces instead of tabs, so that they align regardless of tab size. This is in line with diagrams on other git documentation pages. Signed-off-by: Timur Sultanaev Signed-off-by: Junio C Hamano --- Documentation/git-merge.adoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/git-merge.adoc b/Documentation/git-merge.adoc index 12aa859d16def9..500120421cacfd 100644 --- a/Documentation/git-merge.adoc +++ b/Documentation/git-merge.adoc @@ -28,8 +28,8 @@ Assume the following history exists and the current branch is `master`: ------------ - A---B---C topic - / + A---B---C topic + / D---E---F---G master ------------ @@ -38,11 +38,11 @@ Then `git merge topic` will replay the changes made on the its current commit (`C`) on top of `master`, and record the result in a new commit along with the names of the two parent commits and a log message from the user describing the changes. Before the operation, -`ORIG_HEAD` is set to the tip of the current branch (`C`). +`ORIG_HEAD` is set to the tip of the current branch (`G`). ------------ - A---B---C topic - / \ + A---B---C topic + / \ D---E---F---G---H master ------------ From 385e175cb596e086e89c4cbc06ece733c5515a39 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 7 Jul 2025 13:08:33 +0200 Subject: [PATCH 36/40] t4150: fix warning printed by awk due to escaped '\@' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 6aec8d38fdd (t: refactor tests depending on Perl to print data, 2025-04-03) we have changed one of the tests in t4150 to use awk(1) instead of Perl. The test works, but at least gawk(1) prints a warning now: awk: cmd. line:3: warning: escape sequence `\@' treated as plain `@' Fix this by removing the backslash. Reported-by: SZEDER Gábor Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4150-am.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 2ae93d3c967aad..699a81ab5cc6d2 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -1086,7 +1086,7 @@ test_expect_success 'am works with multi-line in-body headers' ' # bump from, date, and subject down to in-body header awk " /^From:/{ - print \"From: x \"; + print \"From: x \"; print \"Date: Sat, 1 Jan 2000 00:00:00 +0000\"; print \"Subject: x\n\"; }; 1 From de404249abc2524dfd0a6b09bbe7723d83cb32a3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 7 Jul 2025 13:08:34 +0200 Subject: [PATCH 37/40] t5333: fix missing terminator for sed(1) 's' command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 6aec8d38fdd (t: refactor tests depending on Perl to print data, 2025-04-03) we have changed some of the tests in t4150 to use sed(1) instead of Perl. One of the conversions is broken though: sed: -e expression #1, char 41: unterminated `s' command Curiously enough, the test itself still passes. This is caused by a sequence of failures: 1. The output of sed(1) is piped into git-update-ref(1), and because sed(1) is the upstream command we don't notice that it fails. 2. git-update-ref(1) does not receive any input and thus won't create any references. 3. We then repack the repository with the configured pseudo merges pattern, but as we didn't create any references the pattern doesn't match anything. 4. We use `test_pseudo_merges()` to compute the list of pseudo-merges and write it into a file. This file is empty as there are none. 5. The loop over the pseudo-merges becomes a no-op. 6. The final test succeeds as well because the number of lines in an empty file is obviously the same as the number of unique lines, namely zero. Fix the issue by adding the terminating '|' to the sed(1) command. Furthermore, make the test a tiny bit more robust by not using it as part of a pipe. Reported-by: SZEDER Gábor Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t5333-pseudo-merge-bitmaps.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh index 56674db562f948..a21a3114d35ed1 100755 --- a/t/t5333-pseudo-merge-bitmaps.sh +++ b/t/t5333-pseudo-merge-bitmaps.sh @@ -234,8 +234,8 @@ test_expect_success 'pseudo-merge pattern with capture groups' ' test_commit_bulk 16 && git rev-list HEAD~16.. >in && - sed "s|\(.*\)|create refs/remotes/$r/tags/\1 \1" in | - git update-ref --stdin || return 1 + sed "s|\(.*\)|create refs/remotes/$r/tags/\1 \1|" in >refs && + git update-ref --stdin Date: Mon, 7 Jul 2025 09:45:18 -0700 Subject: [PATCH 38/40] builtin/gc: correct total_ram calculation with HAVE_BSD_SYSCTL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The calls to sysctl() assume a 64-bit memory size for the variable holding the value, but the actual size depends on the key name and platform, at least for HW_PHYSMEM. Detect any mismatched reads, and retry with a shorter variable when needed. Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- builtin/gc.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 7dc94f243d7dc6..6880f5b13ddc80 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -517,7 +517,7 @@ static uint64_t total_ram(void) return total; } #elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64)) - int64_t physical_memory; + uint64_t physical_memory; int mib[2]; size_t length; @@ -529,9 +529,16 @@ static uint64_t total_ram(void) # else mib[1] = HW_PHYSMEM; # endif - length = sizeof(int64_t); - if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) + length = sizeof(physical_memory); + if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) { + if (length == 4) { + uint32_t mem; + + if (!sysctl(mib, 2, &mem, &length, NULL, 0)) + physical_memory = mem; + } return physical_memory; + } #elif defined(GIT_WINDOWS_NATIVE) MEMORYSTATUSEX memInfo; From 4498127b04372cff39cf34c5559d1cf547c643e6 Mon Sep 17 00:00:00 2001 From: redoste Date: Mon, 7 Jul 2025 20:48:51 +0200 Subject: [PATCH 39/40] ssh signing: don't detach the filename strbuf from key_file tempfile Detaching the filename string from the tempfile structure used to cause delete_tempfile() to fail and the temporary file was not cleaned up. While it's possible to get rid of the allocation and copy from xstrdup(), it keeps the code symetric with the other branch since interpolate_path() also allocates and ssh_signing_key_file is freed in both cases. The exisiting test was updated to check if the temporary files are properly deleted. To prevent TMPDIR from leaking into the other tests, a new subshell is created, however this prevents test_config from working. The cleanup of the config changed in the subshell is done by test_unconfig in a call to test_when_finished outside of it. Helped-by: brian m. carlson Helped-by: Patrick Steinhardt Helped-by: Phillip Wood Signed-off-by: redoste Signed-off-by: Junio C Hamano --- gpg-interface.c | 2 +- t/t7528-signed-commit-ssh.sh | 32 ++++++++++++++++++++------------ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 0896458de5a988..bdcc8c2a2ed683 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -1048,7 +1048,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, key_file->filename.buf); goto out; } - ssh_signing_key_file = strbuf_detach(&key_file->filename, NULL); + ssh_signing_key_file = xstrdup(key_file->filename.buf); } else { /* We assume a file */ ssh_signing_key_file = interpolate_path(signing_key, 1); diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh index 065f78063629cb..0f887a3ebee19b 100755 --- a/t/t7528-signed-commit-ssh.sh +++ b/t/t7528-signed-commit-ssh.sh @@ -84,18 +84,26 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen test_config gpg.format ssh && eval $(ssh-agent) && test_when_finished "kill ${SSH_AGENT_PID}" && - ssh-add "${GPGSSH_KEY_PRIMARY}" && - echo 1 >file && git add file && - git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" && - echo 2 >file && - test_config user.signingkey "$(cat "${GPGSSH_KEY_PRIMARY}.pub")" && - git commit -a -m rsa-config -S && - ssh-add "${GPGSSH_KEY_ECDSA}" && - echo 3 >file && - git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" && - echo 4 >file && - test_config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" && - git commit -a -m ecdsa-config -S + test_when_finished "test_unconfig user.signingkey" && + mkdir tmpdir && + TMPDIR="$(pwd)/tmpdir" && + ( + export TMPDIR && + ssh-add "${GPGSSH_KEY_PRIMARY}" && + echo 1 >file && git add file && + git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" && + echo 2 >file && + git config user.signingkey "$(cat "${GPGSSH_KEY_PRIMARY}.pub")" && + git commit -a -m rsa-config -S && + ssh-add "${GPGSSH_KEY_ECDSA}" && + echo 3 >file && + git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" && + echo 4 >file && + git config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" && + git commit -a -m ecdsa-config -S + ) && + find tmpdir -type f >tmpfiles && + test_must_be_empty tmpfiles ' test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' ' From d30e120486c5e0632d97f3cba79c03efb6dbb3cb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 14 Jul 2025 11:18:42 -0700 Subject: [PATCH 40/40] The ninth batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.51.0.adoc | 51 ++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/Documentation/RelNotes/2.51.0.adoc b/Documentation/RelNotes/2.51.0.adoc index 40920e649525ff..4eb30d26a11201 100644 --- a/Documentation/RelNotes/2.51.0.adoc +++ b/Documentation/RelNotes/2.51.0.adoc @@ -40,6 +40,13 @@ UI, Workflows & Features client, instead of retrying, it skipped it by mistake, which has been corrected. + * The reftable ref backend has matured enough; Git 3.0 will make it + the default format in a newly created repositories by default. + + * "netrc" credential helper has been improved to understand textual + service names (like smtp) in addition to the numeric port numbers + (like 25). + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -65,6 +72,16 @@ Performance, Internal Implementation, Development Support etc. * "git push" and "git fetch" are taught to update refs in batches to gain performance. + * Some code paths in the "git prune" used to ignore passed in + repository object and used the_repository singleton instance + instead, which has been corrected. + + * Update ".clang-format" and ".editorconfig" to match our style guide + a bit better. + + * "make coccicheck" succeeds even when spatch made suggestions, which + has been updated to fail in such a case. + Fixes since v2.50 ----------------- @@ -117,8 +134,32 @@ including security updates, are included in this release. * Remove unnecessary check from "git daemon" code. (merge 0c856224d2 cb/daemon-fd-check-fix later to maint). - * Leakfix. - (merge b0e9d25865 jk/fix-leak-send-pack later to maint). + * Use of sysctl() system call to learn the total RAM size used on + BSDs has been corrected. + (merge 781c1cf571 cb/total-ram-bsd-fix later to maint). + + * Drop FreeBSD 4 support and declare that we support only FreeBSD 12 + or later, which has memmem() supported. + (merge 0392f976a7 bs/config-mak-freebsd later to maint). + + * A diff-filter with negative-only specification like "git log + --diff-filter=d" did not trigger correctly, which has been fixed. + (merge 375ac087c5 jk/all-negative-diff-filter-fix later to maint). + + * A failure to open the index file for writing due to conflicting + access did not state what went wrong, which has been corrected. + (merge 9455397a5c hy/read-cache-lock-error-fix later to maint). + + * Tempfile removal fix in the codepath to sign commits with SSH keys. + (merge 4498127b04 re/ssh-sign-buffer-fix later to maint). + + * Code and test clean-up around string-list API. + (merge 6e5b26c3ff sj/string-list later to maint). + + * "git apply -N" should start from the current index and register + only new files, but it instead started from an empty index, which + has been corrected. + (merge 2b49d97fcb rp/apply-intent-to-add-fix later to maint). * Other code cleanup, docfix, build fix, etc. (merge b257adb571 lo/my-first-ow-doc-update later to maint). @@ -136,3 +177,9 @@ including security updates, are included in this release. (merge ff73f375bb jg/mailinfo-leakfix later to maint). (merge 996f14c02b jj/doc-branch-markup-fix later to maint). (merge 1e77de1864 cb/ci-freebsd-update-to-14.3 later to maint). + (merge b0e9d25865 jk/fix-leak-send-pack later to maint). + (merge f3a9558c8c bs/remote-helpers-doc-markup-fix later to maint). + (merge c4e9775c60 kh/doc-config-subcommands later to maint). + (merge de404249ab ps/perlless-test-fixes later to maint). + (merge 953049eed8 ts/merge-orig-head-doc-fix later to maint). + (merge 0c83bbc704 rj/freebsd-sysinfo-build-fix later to maint).