From 96e5b72d1a7ce8851cf8d1bdfc61f717c0a39407 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Mon, 30 Jun 2025 18:06:28 +0000 Subject: [PATCH 01/19] docs: link OpenSSL's verify(1) manual page to know about -CAfile and -CApath options The description of `--smtp-ssl-cert-path` in the git-send-email documentation mentions consulting OpenSSL's verify(1) manual page for details about the `-CAfile` and `-CApath` options. However, the way it was written was quite confusing, and it didn't mention that OpenSSL's verify(1) is the manual page to refer to. Fix this by slightly rewording the description and also add a link to the OpenSSL verify(1) manual page. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.adoc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index 7bd09c254b271e..4208bac44ceeb5 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -280,12 +280,14 @@ must be used for each option. Path to a store of trusted CA certificates for SMTP SSL/TLS certificate validation (either a directory that has been processed by `c_rehash`, or a single file containing one or more PEM format - certificates concatenated together: see verify(1) -CAfile and - -CApath for more information on these). Set it to an empty string - to disable certificate verification. Defaults to the value of the - `sendemail.smtpSSLCertPath` configuration variable, if set, or the - backing SSL library's compiled-in default otherwise (which should - be the best choice on most platforms). + certificates concatenated together: see the description of the + `-CAfile` __ and the `-CApath` __ options of + https://docs.openssl.org/master/man1/openssl-verify/ + [OpenSSL's verify(1) manual page] for more information on these). + Set it to an empty string to disable certificate verification. + Defaults to the value of the `sendemail.smtpSSLCertPath` configuration + variable, if set, or the backing SSL library's compiled-in default + otherwise (which should be the best choice on most platforms). --smtp-user=:: Username for SMTP-AUTH. Default is the value of `sendemail.smtpUser`; From a717ef18f2fa7c91e801869ec5aab00d99d4ceb5 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Mon, 30 Jun 2025 18:06:34 +0000 Subject: [PATCH 02/19] docs: add outlookidfix config option to sendemail documentation The documentation for command line option `--outlook-id-fix` is there in the sendemail documentation, but the config option `sendemail.outlookidfix` was missing. Add the same to the documentation. White at it, also enclose the values `true` and `false` in backticks in the documentation for `sendemail.mailmap`. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/config/sendemail.adoc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/config/sendemail.adoc b/Documentation/config/sendemail.adoc index 54f1248e647273..47223346579727 100644 --- a/Documentation/config/sendemail.adoc +++ b/Documentation/config/sendemail.adoc @@ -31,8 +31,8 @@ sendemail.confirm:: values. sendemail.mailmap:: - If true, makes linkgit:git-send-email[1] assume `--mailmap`, - otherwise assume `--no-mailmap`. False by default. + If `true`, makes linkgit:git-send-email[1] assume `--mailmap`, + otherwise assume `--no-mailmap`. `False` by default. sendemail.mailmap.file:: The location of a linkgit:git-send-email[1] specific augmenting @@ -96,6 +96,11 @@ sendemail.xmailer:: linkgit:git-send-email[1] command-line options. See its documentation for details. +sendemail.outlookidfix:: + If `true`, makes linkgit:git-send-email[1] assume `--outlook-id-fix`, + and if `false` assume `--no-outlook-id-fix`. If not specified, it will + behave the same way as if `--outlook-id-fix` is not specified. + sendemail.signedOffCc (deprecated):: Deprecated alias for `sendemail.signedOffByCc`. From 18617b2afd74db9912ae80b55a2c813b90231629 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Mon, 30 Jun 2025 18:06:40 +0000 Subject: [PATCH 03/19] docs: add an OAuth2.0 credential helper for AOL accounts Yahoo and AOL, both advertise that they support app passwords for third-party applications. But generating app passwords for them is broken and unreliable for quite some time now. Yahoo already had an OAuth2.0 credential helper added in the documentation, so I thought it would be a good idea to add one for AOL accounts as well, which is more reliable and secure. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index 4208bac44ceeb5..b31145901c7539 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -600,6 +600,9 @@ available online. Community maintained credential helpers are also available: - https://github.com/AdityaGarg8/git-credential-email[git-credential-yahoo] (cross platform, dedicated helper for authenticating Yahoo accounts) + - https://github.com/AdityaGarg8/git-credential-email[git-credential-aol] + (cross platform, dedicated helper for authenticating AOL accounts) + You can also see linkgit:gitcredentials[7] for more OAuth based authentication helpers. From 95ce81f68d519339fc57c0f321845527792cade9 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Mon, 30 Jun 2025 18:06:46 +0000 Subject: [PATCH 04/19] docs: add a paragraph explaining the `sendmailCmd` option of sendemail `sendmailCmd` is a configuration option in `git-send-email` that allows users to send emails using an external application that supports sendmail-like commands. This ability has been very useful to support proprietary email APIs without modifying the `git-send-email` codebase. It is also useful for users who prefer to use another SMTP client instead of the SMTP perl library used by `git-send-email`. This commit adds a paragraph to the documentation explaining this option. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.adoc | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index b31145901c7539..6556f949a1034b 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -619,6 +619,35 @@ These additional Perl modules are also required: https://metacpan.org/pod/Authen::SASL[Authen::SASL] and https://metacpan.org/pod/Mail::Address[Mail::Address]. +Exploiting the `sendmailCmd` option of `git send-email` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Apart from sending emails via an SMTP server, `git send-email` can also send +emails through any application that supports sendmail-like commands. You can +read documentation of `--sendmail-cmd=` above for more information. +This ability can be very useful if you want to use another application as an +SMTP client for `git send-email`, or if your email provider uses proprietary +APIs instead of SMTP to send emails. + +As an example, lets see how to configure https://marlam.de/msmtp/[msmtp], a +popular SMTP client found in many Linux distributions. Edit `~/.gitconfig` +to instruct `git-send-email` to use it for sending emails. + +---- +[sendemail] + sendmailCmd = /usr/bin/msmtp # Change this to the path where msmtp is installed +---- + +Links of a few such community maintained helpers are: + + - https://marlam.de/msmtp/[msmtp] + (popular SMTP client with many features, available for Linux and macOS) + + - https://github.com/AdityaGarg8/git-credential-email[git-protonmail] + (cross platform client that can send emails using the ProtonMail API) + + - https://github.com/AdityaGarg8/git-credential-email[git-msgraph] + (cross platform client that can send emails using the Microsoft Graph API) SEE ALSO -------- From ac1a32ea52532fadba18128d67b9dc211002059e Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Mon, 30 Jun 2025 18:06:56 +0000 Subject: [PATCH 05/19] docs: mention possible options for Proton Mail users Proton Mail is an privacy-focused email service gaining popularity. Unfortunately, it does not provide an SMTP server to send emails. Proton Mail Bridge is an official solution for paid users, and for free users, a client named git-protonmail is available. Mention the same in the docs. Signed-off-by: Aditya Garg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.adoc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc index 6556f949a1034b..5335502d68fc7b 100644 --- a/Documentation/git-send-email.adoc +++ b/Documentation/git-send-email.adoc @@ -606,6 +606,14 @@ available online. Community maintained credential helpers are also available: You can also see linkgit:gitcredentials[7] for more OAuth based authentication helpers. +Proton Mail does not provide an SMTP server to send emails. If you are a paid +customer of Proton Mail, you can use +https://proton.me/mail/bridge[Proton Mail Bridge] +officially provided by Proton Mail to create a local SMTP server for sending +emails. For both free and paid users, community maintained projects like +https://github.com/AdityaGarg8/git-credential-email[git-protonmail] can be +used. + Note: the following core Perl modules that may be installed with your distribution of Perl are required: From cc7dc407fe4c153195e5ce38d0109f3c2c35ceaf Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:13 -0700 Subject: [PATCH 06/19] fetch-prune: optimize dangling-ref reporting When pruning during `git fetch` we check each pruned ref against the ref_store one at a time to decide whether to report it as dangling. This causes every local ref to be scanned for each ref being pruned. If there are N refs in the repo and M refs being pruned, this code is O(M*N). However, `git remote prune` uses a very similar function that is only O(N*log(M)). Remove the wasteful ref scanning for each pruned ref and use the faster version already available in refs_warn_dangling_symrefs. Change the message to include the original refname since the message is no longer printed immediately after the line that did just print the refname. In a repo with 126,000 refs, where I was pruning 28,000 refs, this code made about 3.6 billion calls to strcmp and consumed 410 seconds of CPU. (Invariably in that time, my remote would timeout and the fetch would fail anyway.) After this change, the same operation completes in under a second. Signed-off-by: Phil Hord Reviewed-by: Jacob Keller Signed-off-by: Junio C Hamano --- builtin/fetch.c | 20 ++++++++++---------- builtin/remote.c | 4 ++-- refs.c | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index fe2b26c74aecab..f05530b62ee415 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1384,9 +1384,13 @@ static int prune_refs(struct display_state *display_state, int result = 0; struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; + struct string_list refnames = STRING_LIST_INIT_NODUP; const char *dangling_msg = dry_run - ? _(" (%s will become dangling)") - : _(" (%s has become dangling)"); + ? _(" %s will become dangling after %s is deleted") + : _(" %s has become dangling after %s was deleted"); + + for (ref = stale_refs; ref; ref = ref->next) + string_list_append(&refnames, ref->name); if (!dry_run) { if (transaction) { @@ -1397,15 +1401,9 @@ static int prune_refs(struct display_state *display_state, goto cleanup; } } else { - struct string_list refnames = STRING_LIST_INIT_NODUP; - - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); - result = refs_delete_refs(get_main_ref_store(the_repository), "fetch: prune", &refnames, 0); - string_list_clear(&refnames, 0); } } @@ -1417,12 +1415,14 @@ static int prune_refs(struct display_state *display_state, _("(none)"), ref->name, &ref->new_oid, &ref->old_oid, summary_width); - refs_warn_dangling_symref(get_main_ref_store(the_repository), - stderr, dangling_msg, ref->name); } + string_list_sort(&refnames); + refs_warn_dangling_symrefs(get_main_ref_store(the_repository), + stderr, dangling_msg, &refnames); } cleanup: + string_list_clear(&refnames, 0); strbuf_release(&err); free_refs(stale_refs); return result; diff --git a/builtin/remote.c b/builtin/remote.c index 043596328638af..30cf4100d44951 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1516,8 +1516,8 @@ static int prune_remote(const char *remote, int dry_run) struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; struct string_list_item *item; const char *dangling_msg = dry_run - ? _(" %s will become dangling!") - : _(" %s has become dangling!"); + ? _(" %s will become dangling after %s is deleted!") + : _(" %s has become dangling after %s was deleted!"); get_remote_ref_states(remote, &states, GET_REF_STATES); diff --git a/refs.c b/refs.c index 0f41b2fd4a6b67..def147b685203d 100644 --- a/refs.c +++ b/refs.c @@ -461,7 +461,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } - fprintf(d->fp, d->msg_fmt, refname); + fprintf(d->fp, d->msg_fmt, refname, resolves_to); fputc('\n', d->fp); return 0; } From 0f846954999a332735c52dc26451be674ea617ba Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:14 -0700 Subject: [PATCH 07/19] refs: remove old refs_warn_dangling_symref The dangling warning function that takes a single ref to search for is no longer used. Remove it. Signed-off-by: Phil Hord Signed-off-by: Junio C Hamano --- refs.c | 17 +---------------- refs.h | 2 -- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/refs.c b/refs.c index def147b685203d..b0e67077a6c994 100644 --- a/refs.c +++ b/refs.c @@ -438,7 +438,6 @@ static int for_each_filter_refs(const char *refname, const char *referent, struct warn_if_dangling_data { struct ref_store *refs; FILE *fp; - const char *refname; const struct string_list *refnames; const char *msg_fmt; }; @@ -455,9 +454,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU resolves_to = refs_resolve_ref_unsafe(d->refs, refname, 0, NULL, NULL); if (!resolves_to - || (d->refname - ? strcmp(resolves_to, d->refname) - : !string_list_has_string(d->refnames, resolves_to))) { + || !string_list_has_string(d->refnames, resolves_to)) { return 0; } @@ -466,18 +463,6 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } -void refs_warn_dangling_symref(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const char *refname) -{ - struct warn_if_dangling_data data = { - .refs = refs, - .fp = fp, - .refname = refname, - .msg_fmt = msg_fmt, - }; - refs_for_each_rawref(refs, warn_if_dangling_symref, &data); -} - void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const char *msg_fmt, const struct string_list *refnames) { diff --git a/refs.h b/refs.h index a0cdd99250e828..60642bc24ac90a 100644 --- a/refs.h +++ b/refs.h @@ -435,8 +435,6 @@ static inline const char *has_glob_specials(const char *pattern) return strpbrk(pattern, "?*["); } -void refs_warn_dangling_symref(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const char *refname); void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const char *msg_fmt, const struct string_list *refnames); From 87d8d8c5d09b1ee52cdf472b53b370020a7cb41c Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:15 -0700 Subject: [PATCH 08/19] clean up interface for refs_warn_dangling_symrefs The refs_warn_dangling_symrefs interface is a bit fragile as it passes in printf-formatting strings with expectations about the number of arguments. This patch series made it worse by adding a 2nd positional argument. But there are only two call sites, and they both use almost identical display options. Make this safer by moving the format strings into the function that uses them to make it easier to see when the arguments don't match. Pass a prefix string and a dry_run flag so the decision logic can be handled where needed. Signed-off-by: Phil Hord Signed-off-by: Junio C Hamano --- builtin/fetch.c | 5 +---- builtin/remote.c | 5 +---- refs.c | 17 +++++++++++------ refs.h | 3 ++- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index f05530b62ee415..98d89d6f65214b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1385,9 +1385,6 @@ static int prune_refs(struct display_state *display_state, struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; struct string_list refnames = STRING_LIST_INIT_NODUP; - const char *dangling_msg = dry_run - ? _(" %s will become dangling after %s is deleted") - : _(" %s has become dangling after %s was deleted"); for (ref = stale_refs; ref; ref = ref->next) string_list_append(&refnames, ref->name); @@ -1418,7 +1415,7 @@ static int prune_refs(struct display_state *display_state, } string_list_sort(&refnames); refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stderr, dangling_msg, &refnames); + stderr, " ", dry_run, &refnames); } cleanup: diff --git a/builtin/remote.c b/builtin/remote.c index 30cf4100d44951..81912fed55ddcb 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1515,9 +1515,6 @@ static int prune_remote(const char *remote, int dry_run) struct ref_states states = REF_STATES_INIT; struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; struct string_list_item *item; - const char *dangling_msg = dry_run - ? _(" %s will become dangling after %s is deleted!") - : _(" %s has become dangling after %s was deleted!"); get_remote_ref_states(remote, &states, GET_REF_STATES); @@ -1549,7 +1546,7 @@ static int prune_remote(const char *remote, int dry_run) } refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stdout, dangling_msg, &refs_to_prune); + stdout, " ", dry_run, &refs_to_prune); string_list_clear(&refs_to_prune, 0); free_remote_ref_states(&states); diff --git a/refs.c b/refs.c index b0e67077a6c994..8573310a06f863 100644 --- a/refs.c +++ b/refs.c @@ -439,7 +439,8 @@ struct warn_if_dangling_data { struct ref_store *refs; FILE *fp; const struct string_list *refnames; - const char *msg_fmt; + const char *indent; + int dry_run; }; static int warn_if_dangling_symref(const char *refname, const char *referent UNUSED, @@ -447,7 +448,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU int flags, void *cb_data) { struct warn_if_dangling_data *d = cb_data; - const char *resolves_to; + const char *resolves_to, *msg; if (!(flags & REF_ISSYMREF)) return 0; @@ -458,19 +459,23 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } - fprintf(d->fp, d->msg_fmt, refname, resolves_to); - fputc('\n', d->fp); + msg = d->dry_run + ? _("%s%s will become dangling after %s is deleted\n") + : _("%s%s has become dangling after %s was deleted\n"); + fprintf(d->fp, msg, d->indent, refname, resolves_to); return 0; } void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const struct string_list *refnames) + const char *indent, int dry_run, + const struct string_list *refnames) { struct warn_if_dangling_data data = { .refs = refs, .fp = fp, .refnames = refnames, - .msg_fmt = msg_fmt, + .indent = indent, + .dry_run = dry_run, }; refs_for_each_rawref(refs, warn_if_dangling_symref, &data); } diff --git a/refs.h b/refs.h index 60642bc24ac90a..6b9c6d5764d91d 100644 --- a/refs.h +++ b/refs.h @@ -436,7 +436,8 @@ static inline const char *has_glob_specials(const char *pattern) } void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const struct string_list *refnames); + const char *indent, int dry_run, + const struct string_list *refnames); /* * Flags for controlling behaviour of pack_refs() From ad7780b38fae2fb915404a59b0c904da8ed154a6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 8 Jul 2025 13:23:56 +0200 Subject: [PATCH 09/19] docs/git-pack-refs: document heuristic used for packing loose refs The `git pack-refs --auto` flag asks the ref backend to decide for itself whether or not references need to be repacked. This is done to ensure that we don't repack in cases where the backend is already in a good-enough state, which is typically the case for the "reftable" backend that performs auto-compaction on writes. As such, we initially only had heuristics in place for the "reftable" backend. The "files" backend didn't have any heuristics, so we'd repack loose references every time `git pack-refs --auto` was executed. This caused excessive repacking with that backend though, which is why we eventually implemented a heuristic via c3459ae9ef2 (refs/files: use heuristic to decide whether to repack with `--auto`, 2024-09-04). The documentation for the `--auto` flag hasn't been updated accordingly and still claims that we don't have any metrics for the "files" backend. Update it to reflect the new reality. Reported-by: Karthik Nayak Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/git-pack-refs.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt index 2dcabaf74cefb4..a6cd43c9beedac 100644 --- a/Documentation/git-pack-refs.txt +++ b/Documentation/git-pack-refs.txt @@ -66,7 +66,10 @@ Pack refs as needed depending on the current state of the ref database. The behavior depends on the ref format used by the repository and may change in the future. + - - "files": No special handling for `--auto` has been implemented. + - "files": Loose references are packed into the `packed-refs` file + based on the ratio of loose references to the size of the + `packed-refs` file. The bigger the `packed-refs` file, the more loose + references need to exist before we repack. + - "reftable": Tables are compacted such that they form a geometric sequence. For two tables N and N+1, where N+1 is newer, this From a3a7f2051686e087cba80f3af1557107406205c9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 8 Jul 2025 12:19:54 +0200 Subject: [PATCH 10/19] refs/files: remove empty parent dirs when ref creation fails When creating a new reference in the "files" backend we first create the directory hierarchy for that reference, then create the lockfile for that reference, and finally rename the lockfile into place. When the transaction gets aborted we prune the lockfile, but we don't clean up the directory hierarchy that we may have created for the lockfile. In some egde cases this can lead to lots of empty directories being cluttered in the ".git/refs" directory that really serve no purpose at all. We know to prune such empty directories when packing refs, but that only patches over the issue. Improve this by removing empty parents when cleaning up still-locked references in `files_transaction_cleanup()`. This function is also called when preparing or committing the transaction, so this change also helps when not explicitly aborting the transaction. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/files-backend.c | 2 ++ t/t1400-update-ref.sh | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/refs/files-backend.c b/refs/files-backend.c index bf6f89b1d1938b..00128f2183251e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2760,6 +2760,8 @@ static void files_transaction_cleanup(struct files_ref_store *refs, if (lock) { unlock_ref(lock); + try_remove_empty_parents(refs, update->refname, + REMOVE_EMPTY_PARENTS_REF); update->backend_data = NULL; } } diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index d29d23cb8905f8..42bbe22c850d26 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -2310,4 +2310,23 @@ test_expect_success 'update-ref should also create reflog for HEAD' ' test_cmp expect actual ' +test_expect_success REFFILES 'empty directories are pruned when aborting a transaction' ' + test_path_is_missing .git/refs/heads/nested && + git update-ref --stdin <<-EOF && + create refs/heads/nested/something HEAD + prepare + abort + EOF + test_path_is_missing .git/refs/heads/nested +' + +test_expect_success REFFILES 'empty directories are pruned when not committing' ' + test_path_is_missing .git/refs/heads/nested && + git update-ref --stdin <<-EOF && + create refs/heads/nested/something HEAD + prepare + EOF + test_path_is_missing .git/refs/heads/nested +' + test_done From 52d0c32b9f48122766d3effc07abb9a4eaa89bc0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 8 Jul 2025 12:18:58 +0200 Subject: [PATCH 11/19] t1006: fix broken TAP format When running t1006 via Meson we receive an error about invalid TAP format: $ meson test t1006-cat-file 1/1 t1006-cat-file OK 3.86s 420 subtests passed stdout: 147: UNKNOWN: c308ae01840d8e620ad554ee5d77fe114dc2d912:path with spaces stdout: 159: UNKNOWN: 3625298bf5e7c464a7d0e38ea80c2a5b5904d9a3e5b2b025b67f360e09b68dc7:path with spaces ERROR: Unknown TAP output lines for a supported TAP version. This is probably a bug in the test; if they are not TAP syntax, prefix them with a # Ok: 1 Fail: 0 While Meson copes with it alright, it's still annoying to see these errors on every test run. The root cause of the broken format is a call to grep(1) that gets executed outside of a test case, which has been added recently via 9fd38038b9c (t1006: update 'run_tests' to test generic object specifiers, 2025-06-02). This call is done to determine whether a subsequent test case is expected to succeed or fail, so it makes sense to have it execute outside of a test case. But whenever we do that, we must be extra careful to not generate any output that breaks the TAP format. Fix the issue by adding '-q' to the command so that it doesn't print any matching lines. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1006-cat-file.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index f123ef1e360aca..1f61b666a7d382 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -197,7 +197,7 @@ $content" # FIXME: %(rest) is incompatible with object names that include whitespace, # e.g. HEAD:path/to/a/file with spaces. Use the resolved OID as input to # test this instead of the raw object name. - if echo "$object_name" | grep " "; then + if echo "$object_name" | grep -q " "; then test_rest=test_expect_failure else test_rest=test_expect_success From 369e6d94b21d238a203ffb702605f97aca5704c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:44:09 +0200 Subject: [PATCH 12/19] parse-options: require PARSE_OPT_NOARG for OPTION_BITOP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OPTION_BITOP options don't take arguments. Make sure they are declared that way using the flag PARSE_OPT_NOARG. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- parse-options.c | 1 + 1 file changed, 1 insertion(+) diff --git a/parse-options.c b/parse-options.c index a9a39ecaef6c36..68ff4944925c61 100644 --- a/parse-options.c +++ b/parse-options.c @@ -591,6 +591,7 @@ static void parse_options_check(const struct option *opts) case OPTION_NEGBIT: case OPTION_SET_INT: case OPTION_NUMBER: + case OPTION_BITOP: if ((opts->flags & PARSE_OPT_OPTARG) || !(opts->flags & PARSE_OPT_NOARG)) optbug(opts, "should not accept an argument"); From 0d3e045b34f38d23e6160ce8aae363f358bd5cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:45:14 +0200 Subject: [PATCH 13/19] parse-options: add precision handling for PARSE_OPT_CMDMODE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build on 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) to support value variables of different sizes for PARSE_OPT_CMDMODE options. Do that by requiring their "precision" to be set and casting their "value" pointer accordingly. Call the function that does the raw casting do_get_int_value() to reserve the name get_int_value() for a more friendly wrapper we're going to introduce in one of the next patches. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/am.c | 1 + parse-options.c | 41 ++++++++++++++++++++++++++++++----- parse-options.h | 1 + t/helper/test-parse-options.c | 13 ++++++++--- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index e32a3b4c973a42..ce58abf2a2e8ba 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2406,6 +2406,7 @@ int cmd_am(int argc, .type = OPTION_CALLBACK, .long_name = "show-current-patch", .value = &resume_mode, + .precision = sizeof(resume_mode), .argh = "(diff|raw)", .help = N_("show the patch being applied"), .flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, diff --git a/parse-options.c b/parse-options.c index 68ff4944925c61..ddac008a5ea7a7 100644 --- a/parse-options.c +++ b/parse-options.c @@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file) return prefix_filename_except_for_dash(prefix, file); } +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret) +{ + switch (precision) { + case sizeof(int8_t): + *ret = *(int8_t *)value; + return 0; + case sizeof(int16_t): + *ret = *(int16_t *)value; + return 0; + case sizeof(int32_t): + *ret = *(int32_t *)value; + return 0; + case sizeof(int64_t): + *ret = *(int64_t *)value; + return 0; + default: + return -1; + } +} + static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, const struct option *opt, enum opt_parsed flags, @@ -266,7 +286,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } struct parse_opt_cmdmode_list { - int value, *value_ptr; + intmax_t value; + void *value_ptr; + size_t precision; const struct option *opt; const char *arg; enum opt_parsed flags; @@ -280,7 +302,7 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx, for (; opts->type != OPTION_END; opts++) { struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list; - int *value_ptr = opts->value; + void *value_ptr = opts->value; if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr) continue; @@ -292,10 +314,13 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx, CALLOC_ARRAY(elem, 1); elem->value_ptr = value_ptr; - elem->value = *value_ptr; + elem->precision = opts->precision; + if (do_get_int_value(value_ptr, opts->precision, &elem->value)) + optbug(opts, "has invalid precision"); elem->next = ctx->cmdmode_list; ctx->cmdmode_list = elem; } + BUG_if_bug("invalid 'struct option'"); } static char *optnamearg(const struct option *opt, const char *arg, @@ -317,7 +342,13 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, char *opt_name, *other_opt_name; for (; elem; elem = elem->next) { - if (*elem->value_ptr == elem->value) + intmax_t new_value; + + if (do_get_int_value(elem->value_ptr, elem->precision, + &new_value)) + BUG("impossible: invalid precision"); + + if (new_value == elem->value) continue; if (elem->opt && @@ -327,7 +358,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, elem->opt = opt; elem->arg = arg; elem->flags = flags; - elem->value = *elem->value_ptr; + elem->value = new_value; } if (result || !elem) diff --git a/parse-options.h b/parse-options.h index 91c3e3c29b3dda..c75a473c9ecdfc 100644 --- a/parse-options.h +++ b/parse-options.h @@ -269,6 +269,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \ .defval = (i), \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index f2663dd0c07279..1e03ff88f6ff89 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -148,9 +148,16 @@ int cmd__parse_options(int argc, const char **argv) OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1), OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2), - OPT_CALLBACK_F(0, "mode34", &integer, "(3|4)", - "set integer to 3 or 4 (cmdmode option)", - PARSE_OPT_CMDMODE, mode34_callback), + { + .type = OPTION_CALLBACK, + .long_name = "mode34", + .value = &integer, + .precision = sizeof(integer), + .argh = "(3|4)", + .help = "set integer to 3 or 4 (cmdmode option)", + .flags = PARSE_OPT_CMDMODE, + .callback = mode34_callback, + }, OPT_CALLBACK('L', "length", &integer, "str", "get length of ", length_callback), OPT_FILENAME('F', "file", &file, "set file to "), From c898bbc5e4b582c28379bc64b7f9c9ec96106993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:45:24 +0200 Subject: [PATCH 14/19] parse-options: add precision handling for OPTION_SET_INT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_SET_INT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits. Factor out the casting code from the part of do_get_value() that handles OPTION_INTEGER to avoid code duplication. We're going to use it in the next patches as well. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/update-index.c | 6 ++++ parse-options.c | 56 ++++++++++++++++++++++------------- parse-options.h | 2 ++ t/helper/test-parse-options.c | 1 + 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 538b619ba4f3bb..0c1d4ed55beb08 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -981,6 +981,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "assume-unchanged", .value = &mark_valid_only, + .precision = sizeof(mark_valid_only), .help = N_("mark files as \"not changing\""), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -989,6 +990,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-assume-unchanged", .value = &mark_valid_only, + .precision = sizeof(mark_valid_only), .help = N_("clear assumed-unchanged bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, @@ -997,6 +999,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "skip-worktree", .value = &mark_skip_worktree_only, + .precision = sizeof(mark_skip_worktree_only), .help = N_("mark files as \"index-only\""), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -1005,6 +1008,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-skip-worktree", .value = &mark_skip_worktree_only, + .precision = sizeof(mark_skip_worktree_only), .help = N_("clear skip-worktree bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, @@ -1079,6 +1083,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "fsmonitor-valid", .value = &mark_fsmonitor_only, + .precision = sizeof(mark_fsmonitor_only), .help = N_("mark files as fsmonitor valid"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -1087,6 +1092,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-fsmonitor-valid", .value = &mark_fsmonitor_only, + .precision = sizeof(mark_fsmonitor_only), .help = N_("clear fsmonitor valid bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, diff --git a/parse-options.c b/parse-options.c index ddac008a5ea7a7..639f41b83b697b 100644 --- a/parse-options.c +++ b/parse-options.c @@ -88,6 +88,36 @@ static int do_get_int_value(const void *value, size_t precision, intmax_t *ret) } } +static enum parse_opt_result set_int_value(const struct option *opt, + enum opt_parsed flags, + intmax_t value) +{ + switch (opt->precision) { + case sizeof(int8_t): + *(int8_t *)opt->value = value; + return 0; + case sizeof(int16_t): + *(int16_t *)opt->value = value; + return 0; + case sizeof(int32_t): + *(int32_t *)opt->value = value; + return 0; + case sizeof(int64_t): + *(int64_t *)opt->value = value; + return 0; + default: + BUG("invalid precision for option %s", optname(opt, flags)); + } +} + +static int signed_int_fits(intmax_t value, size_t precision) +{ + size_t bits = precision * CHAR_BIT; + intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits); + intmax_t lower_bound = -upper_bound - 1; + return lower_bound <= value && value <= upper_bound; +} + static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, const struct option *opt, enum opt_parsed flags, @@ -136,8 +166,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return 0; case OPTION_SET_INT: - *(int *)opt->value = unset ? 0 : opt->defval; - return 0; + return set_int_value(opt, flags, unset ? 0 : opt->defval); case OPTION_STRING: if (unset) @@ -219,23 +248,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound); - switch (opt->precision) { - case 1: - *(int8_t *)opt->value = value; - return 0; - case 2: - *(int16_t *)opt->value = value; - return 0; - case 4: - *(int32_t *)opt->value = value; - return 0; - case 8: - *(int64_t *)opt->value = value; - return 0; - default: - BUG("invalid precision for option %s", - optname(opt, flags)); - } + return set_int_value(opt, flags, value); } case OPTION_UNSIGNED: { @@ -617,10 +630,13 @@ static void parse_options_check(const struct option *opts) opts->long_name && !(opts->flags & PARSE_OPT_NONEG)) optbug(opts, "OPTION_SET_INT 0 should not be negatable"); switch (opts->type) { + case OPTION_SET_INT: + if (!signed_int_fits(opts->defval, opts->precision)) + optbug(opts, "has invalid defval"); + /* fallthru */ case OPTION_COUNTUP: case OPTION_BIT: case OPTION_NEGBIT: - case OPTION_SET_INT: case OPTION_NUMBER: case OPTION_BITOP: if ((opts->flags & PARSE_OPT_OPTARG) || diff --git a/parse-options.h b/parse-options.h index c75a473c9ecdfc..71516e4b5b9fed 100644 --- a/parse-options.h +++ b/parse-options.h @@ -190,6 +190,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG | (f), \ .defval = (i), \ @@ -260,6 +261,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \ .defval = 1, \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 1e03ff88f6ff89..2ba2546d70a0e9 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -131,6 +131,7 @@ int cmd__parse_options(int argc, const char **argv) .short_name = 'B', .long_name = "no-fear", .value = &boolean, + .precision = sizeof(boolean), .help = "be brave", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = 1, From 5228211c4b92052c0a38f2ab67cd0b87a7baec30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:45:33 +0200 Subject: [PATCH 15/19] parse-options: add precision handling for OPTION_BIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/write-tree.c | 1 + parse-options.c | 19 +++++++++++++++---- parse-options.h | 1 + 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/builtin/write-tree.c b/builtin/write-tree.c index 5a8dc377ec07b0..cfec044710a56a 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -35,6 +35,7 @@ int cmd_write_tree(int argc, .type = OPTION_BIT, .long_name = "ignore-cache-tree", .value = &flags, + .precision = sizeof(flags), .help = N_("only useful for debugging"), .flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, .defval = WRITE_TREE_IGNORE_CACHE_TREE, diff --git a/parse-options.c b/parse-options.c index 639f41b83b697b..b5c877d5e1ea88 100644 --- a/parse-options.c +++ b/parse-options.c @@ -88,6 +88,14 @@ static int do_get_int_value(const void *value, size_t precision, intmax_t *ret) } } +static intmax_t get_int_value(const struct option *opt, enum opt_parsed flags) +{ + intmax_t ret; + if (do_get_int_value(opt->value, opt->precision, &ret)) + BUG("invalid precision for option %s", optname(opt, flags)); + return ret; +} + static enum parse_opt_result set_int_value(const struct option *opt, enum opt_parsed flags, intmax_t value) @@ -139,11 +147,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return opt->ll_callback(p, opt, NULL, unset); case OPTION_BIT: + { + intmax_t value = get_int_value(opt, flags); if (unset) - *(int *)opt->value &= ~opt->defval; + value &= ~opt->defval; else - *(int *)opt->value |= opt->defval; - return 0; + value |= opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_NEGBIT: if (unset) @@ -631,11 +642,11 @@ static void parse_options_check(const struct option *opts) optbug(opts, "OPTION_SET_INT 0 should not be negatable"); switch (opts->type) { case OPTION_SET_INT: + case OPTION_BIT: if (!signed_int_fits(opts->defval, opts->precision)) optbug(opts, "has invalid defval"); /* fallthru */ case OPTION_COUNTUP: - case OPTION_BIT: case OPTION_NEGBIT: case OPTION_NUMBER: case OPTION_BITOP: diff --git a/parse-options.h b/parse-options.h index 71516e4b5b9fed..6501ca3c2793c9 100644 --- a/parse-options.h +++ b/parse-options.h @@ -172,6 +172,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|(f), \ .callback = NULL, \ From feeebbf1b7d5ed8761355d354e9529c791b77e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:45:53 +0200 Subject: [PATCH 16/19] parse-options: add precision handling for OPTION_NEGBIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_NEGBIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/rebase.c | 1 + parse-options.c | 11 +++++++---- parse-options.h | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 2e8c4ee6784d84..e90562a3b8a84e 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1128,6 +1128,7 @@ int cmd_rebase(int argc, .short_name = 'n', .long_name = "no-stat", .value = &options.flags, + .precision = sizeof(options.flags), .help = N_("do not show diffstat of what changed upstream"), .flags = PARSE_OPT_NOARG, .defval = REBASE_DIFFSTAT, diff --git a/parse-options.c b/parse-options.c index b5c877d5e1ea88..ba89dc4d098d60 100644 --- a/parse-options.c +++ b/parse-options.c @@ -157,11 +157,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } case OPTION_NEGBIT: + { + intmax_t value = get_int_value(opt, flags); if (unset) - *(int *)opt->value |= opt->defval; + value |= opt->defval; else - *(int *)opt->value &= ~opt->defval; - return 0; + value &= ~opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_BITOP: if (unset) @@ -643,11 +646,11 @@ static void parse_options_check(const struct option *opts) switch (opts->type) { case OPTION_SET_INT: case OPTION_BIT: + case OPTION_NEGBIT: if (!signed_int_fits(opts->defval, opts->precision)) optbug(opts, "has invalid defval"); /* fallthru */ case OPTION_COUNTUP: - case OPTION_NEGBIT: case OPTION_NUMBER: case OPTION_BITOP: if ((opts->flags & PARSE_OPT_OPTARG) || diff --git a/parse-options.h b/parse-options.h index 6501ca3c2793c9..076f88b3841da3 100644 --- a/parse-options.h +++ b/parse-options.h @@ -250,6 +250,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG, \ .defval = (b), \ From 1d918bf2a5eb9d860df1dd115ef2641d7b5870e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:46:20 +0200 Subject: [PATCH 17/19] parse-options: add precision handling for OPTION_BITOP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BITOP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits. Check if "devfal" fits into an integer variable with the given "precision", but don't check "extra", as its value is only used to clear bits, so cannot lead to an overflow. Not checking continues to allow e.g., using -1 to clear all bits even if the value variable has a narrower type than intptr_t. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- parse-options.c | 11 +++++++---- parse-options.h | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/parse-options.c b/parse-options.c index ba89dc4d098d60..a813511b1bcb68 100644 --- a/parse-options.c +++ b/parse-options.c @@ -167,11 +167,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } case OPTION_BITOP: + { + intmax_t value = get_int_value(opt, flags); if (unset) BUG("BITOP can't have unset form"); - *(int *)opt->value &= ~opt->extra; - *(int *)opt->value |= opt->defval; - return 0; + value &= ~opt->extra; + value |= opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_COUNTUP: if (*(int *)opt->value < 0) @@ -647,12 +650,12 @@ static void parse_options_check(const struct option *opts) case OPTION_SET_INT: case OPTION_BIT: case OPTION_NEGBIT: + case OPTION_BITOP: if (!signed_int_fits(opts->defval, opts->precision)) optbug(opts, "has invalid defval"); /* fallthru */ case OPTION_COUNTUP: case OPTION_NUMBER: - case OPTION_BITOP: if ((opts->flags & PARSE_OPT_OPTARG) || !(opts->flags & PARSE_OPT_NOARG)) optbug(opts, "should not accept an argument"); diff --git a/parse-options.h b/parse-options.h index 076f88b3841da3..8bdf469ae9b29a 100644 --- a/parse-options.h +++ b/parse-options.h @@ -240,6 +240,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|PARSE_OPT_NONEG, \ .defval = (set), \ From c1e616c39b31e78acc595790bf3a9553a022a19d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 9 Jul 2025 11:46:28 +0200 Subject: [PATCH 18/19] parse-options: add precision handling for OPTION_COUNTUP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_COUNTUP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- parse-options.c | 22 +++++++++++++++++----- parse-options.h | 1 + t/helper/test-parse-options.c | 3 +++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/parse-options.c b/parse-options.c index a813511b1bcb68..5224203ffe7bf8 100644 --- a/parse-options.c +++ b/parse-options.c @@ -177,10 +177,22 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } case OPTION_COUNTUP: - if (*(int *)opt->value < 0) - *(int *)opt->value = 0; - *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1; - return 0; + { + size_t bits = CHAR_BIT * opt->precision; + intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits); + intmax_t value = get_int_value(opt, flags); + + if (value < 0) + value = 0; + if (unset) + value = 0; + else if (value < upper_bound) + value++; + else + return error(_("value for %s exceeds %"PRIdMAX), + optname(opt, flags), upper_bound); + return set_int_value(opt, flags, value); + } case OPTION_SET_INT: return set_int_value(opt, flags, unset ? 0 : opt->defval); @@ -651,10 +663,10 @@ static void parse_options_check(const struct option *opts) case OPTION_BIT: case OPTION_NEGBIT: case OPTION_BITOP: + case OPTION_COUNTUP: if (!signed_int_fits(opts->defval, opts->precision)) optbug(opts, "has invalid defval"); /* fallthru */ - case OPTION_COUNTUP: case OPTION_NUMBER: if ((opts->flags & PARSE_OPT_OPTARG) || !(opts->flags & PARSE_OPT_NOARG)) diff --git a/parse-options.h b/parse-options.h index 8bdf469ae9b29a..312045604d98d1 100644 --- a/parse-options.h +++ b/parse-options.h @@ -183,6 +183,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|(f), \ } diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 2ba2546d70a0e9..68579d83f3939e 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -178,6 +178,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .short_name = '+', .value = &boolean, + .precision = sizeof(boolean), .help = "same as -b", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH, }, @@ -185,6 +186,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .long_name = "ambiguous", .value = &ambiguous, + .precision = sizeof(ambiguous), .help = "positive ambiguity", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, }, @@ -192,6 +194,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .long_name = "no-ambiguous", .value = &ambiguous, + .precision = sizeof(ambiguous), .help = "negative ambiguity", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, }, From 90c0775e972847832ac8dfa6a14bc4c3abacd914 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 16 Jul 2025 09:42:12 -0700 Subject: [PATCH 19/19] The eleventh batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.51.0.adoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Documentation/RelNotes/2.51.0.adoc b/Documentation/RelNotes/2.51.0.adoc index ca0e39e1e82964..8ff921809a9107 100644 --- a/Documentation/RelNotes/2.51.0.adoc +++ b/Documentation/RelNotes/2.51.0.adoc @@ -84,6 +84,10 @@ Performance, Internal Implementation, Development Support etc. * Code clean-up around object access API. + * Define .precision to more canned parse-options type to avoid bugs + coming from using a variable with a wrong type to capture the + parsed values. + Fixes since v2.50 ----------------- @@ -166,6 +170,14 @@ including security updates, are included in this release. * Leakfix with a new and a bit invasive test on pack-bitmap files. (merge bfd5522e98 ly/load-bitmap-leakfix later to maint). + * "git fetch --prune" used to be O(n^2) expensive when there are many + refs, which has been corrected. + (merge 87d8d8c5d0 ph/fetch-prune-optim later to maint). + + * When a ref creation at refs/heads/foo/bar fails, the files backend + now removes refs/heads/foo/ if the directory is otherwise not used. + (merge a3a7f20516 ps/refs-files-remove-empty-parent later to maint). + * Other code cleanup, docfix, build fix, etc. (merge b257adb571 lo/my-first-ow-doc-update later to maint). (merge 8b34b6a220 ly/sequencer-update-squash-is-fixup-only later to maint). @@ -188,3 +200,4 @@ including security updates, are included in this release. (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). + (merge ad7780b38f ps/doc-pack-refs-auto-with-files-backend-fix later to maint).