[pull] master from git:master#70
Merged
pull[bot] merged 25 commits intoturkdevops:masterfrom Jun 25, 2025
Merged
Conversation
Whenever we send a thread of emails using send-email, a message number
is internally assigned to each email. This number is used to track the
order of the emails in the thread. Whenever a new message is processed
in a thread, the current script logic increments the message number by
one, which is intended.
But, if a message is edited and then resent, its message number again
gets incremented. This is because the script uses the same logic to
process the edited message, which it uses to send the next message.
This minor bug is usually harmless, unless a special situations arises.
That situation is when the first message in a thread is edited and
resent, and an `--in-reply-to` argument is also passed to send-email.
In this case, if the user has chosen shallow threading, the threading
does not work as expected, and all messages become replies to the
Message-ID specified in the `--in-reply-to` argument.
The reason for this bug is hidden in the code for threading itself.
if ($thread) {
if ($message_was_sent &&
($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
$message_num == 1)) {
$in_reply_to = $message_id;
if (length $references > 0) {
$references .= "\n $message_id";
} else {
$references = "$message_id";
}
}
}
Here `$message_num` is the current message number, and `$in_reply_to` is
the Message-ID of the message to which the current message is a reply.
In case `--in-reply-to` is specified, the `$in_reply_to` variable
is set to the value of the `--in-reply-to` argument.
Whenever this whole set of conditions is true, the script sets the
`$in_reply_to` variable to the current message's ID. This is done to
ensure that the next message in the thread is a reply to this message.
In case we specify an `--in-reply-to` argument, and have shallow
threading, the only condition that can make this true is
`$message_num == 1`, which is true for the first message in a thread.
Thus, the `$in_reply_to` variable gets set to the first message's ID.
For subsequent messages, the `$message_num` variable is always
greater than 1, and the whole set of conditions is false. Therefore, the
`$in_reply_to` variable remains as the first message's ID. This is what
we expect in shallow threading. But if the user edits the first message
and resends it, the `$message_num` variable gets incremented by 1, and
thus the condition `$message_num == 1` becomes false. This means that
the `$in_reply_to` variable is not set to the first message's ID. As a
result the next message in the thread is not a reply to the first
message, but to the `--in-reply-to` argument, effectively breaking the
threading.
In case the user does not specify an `--in-reply-to` argument, the
`!defined $in_reply_to` condition is true, and thus the `$in_reply_to`
variable is set to the first message's ID, and the threading works
as expected, regardless of the message number.
To fix this bug, we need to ensure that the `$message_num` variable is
not incremented by 1 when a message is edited and resent. We do this by
decreasing the `$message_num` variable by 1 whenever the request to edit
a message is received. This way, the next message in the thread will
have the same message number as the edited message. Therefore the
threading will work as expected.
The same logic has also been applied in case the user drops a single
message from the thread by choosing the "[n]o" option during
confirmation. By doing this, the next message in the thread is assigned
the message number of the dropped message, and thus the threading
works as expected.
Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Whenever an email is sent, send-email shows a log at last, which contains all the headers of the email that were received by the receipients. In case outlook changes the Message-ID, a log for the same is shown to the user, but that change is not reflected when the log containing all the headers is displayed. Here is an example of the log that is shown when outlook changes the Message-ID: Outlook reassigned Message-ID to: <PN3PR01MB95973E5ACD7CCFADCB4E298CB865A@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM> OK. Log says: Server: smtp.office365.com MAIL FROM:<gargaditya08@live.com> RCPT TO:<negahe7142@nomrista.com> From: Aditya Garg <gargaditya08@live.com> To: negahe7142@nomrista.com Subject: [PATCH] send-email: show the new message id assigned by outlook in the logs Date: Mon, 26 May 2025 20:28:36 +0530 Message-ID: <20250526145836.4825-1-gargaditya08@live.com> X-Mailer: git-send-email @GIT_VERSION@ MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 Fix this by updating the $header variable, which has the message ID we internally assigned on the "Message-ID:" header, with the message ID the Outlook server assigned. It should look like this after this patch: OK. Log says: Server: smtp.office365.com MAIL FROM:<gargaditya08@live.com> RCPT TO:<negahe7142@nomrista.com> From: Aditya Garg <gargaditya08@live.com> To: negahe7142@nomrista.com Subject: [PATCH] send-email: show the new message id assigned by outlook in the logs Date: Mon, 26 May 2025 20:29:22 +0530 Message-ID: <PN3PR01MB95977486061BD2542BD09B67B865A@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM> X-Mailer: git-send-email @GIT_VERSION@ MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 Signed-off-by: Aditya Garg <gargaditya08@live.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Optional parameter handling only works unambiguous with git rev-parse --parseopt when using the --stuck-long option. To prepare for future commits which add flags with optional parameters, parse with --stuck-long. Signed-off-by: Patrik Weiskircher <patrik@pspdfkit.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allows optionally signing the commits that git subtree creates. This can be necessary when working in a repository that requires gpg signed commits. Signed-off-by: Patrik Weiskircher <patrik@pspdfkit.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Historically "git stash [<options>]" was assumed to mean "git stash save [<options>]". Since 1ada502 (stash: use stash_push for no verb form, 2017-02-28) it is assumed to mean "git stash push [<options>]". As the push subcommand supports pathspecs, 9e14090 (stash: allow pathspecs in the no verb form, 2017-02-28) allowed "git stash -p <pathspec>" to mean "git stash push -p <pathspec>". This was broken in 8c3713c (stash: eliminate crude option parsing, 2020-02-17) which failed to account for "push" being added to the start of argv in cmd_stash() before it calls push_stash() and kept looking in argv[0] for "-p" after moving the code to push_stash(). Fix this by regression by checking argv[1] instead of argv[0] and add a couple of tests to prevent future regressions. Helped-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The support for assuming "push" when "-p" is given introduced in 9e14090 (stash: allow pathspecs in the no verb form, 2017-02-28) is very narrow, neither "git stash -m <message> -p <pathspec>" nor "git stash --patch <pathspec>" imply "push" and die instead. Relax this by passing PARSE_OPT_STOP_AT_NON_OPTION when push is being assumed and then setting "force_assume" if "--patch" was present. This means "git stash <pathspec> -p" still dies so that it does not assume the user meant "push" if they mistype a subcommand name but "git stash -m <message> -p <pathspec>" will now succeed. The test added in the last commit is adjusted to check that push is still assumed when "--patch" comes after other options on the command-line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In revision.c:prepare_show_merge(), we allocated an array in prune but forget to free it. Since parse_pathspec is not responsible to free prune, we should add `free(prune)` in the end of prepare_show_merge(). Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'core_preload_index' is used in a single function named 'preload_index()' in "preload-index.c". Move its declaration inside that function, removing unnecessary global state. This change is part of an ongoing effort to eliminate global variables, improve modularity and help libify the codebase. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor "preload-index.c" to remove the dependency on the global 'the_repository'. Replace the occurrences of 'the_repository' with 'index->repo' and thus remove the definition '#define USE_THE_REPOSITORY_VARIABLE'. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a stash, Git uses the current branch name of the superproject to construct the stash commit message. However, in repositories with submodules, the message may mistakenly display the submodule branch name instead. This is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer. Subsequent calls to the same function overwrite the buffer, corrupting the originally fetched `branch_name` used for the stash message. Use `xstrdup()` to duplicate the branch name immediately after resolving it, so that later buffer overwrites do not affect the stash message. Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have mentioned this in various reviews, but I didn't see it mentioned in the CodingGuildelines document. Let's add it. Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git cat-file command with --mailmap option fails to apply mailmap transformations to the committer field when the author and committer identities are different. This occurs due to a missing newline handling in apply_mailmap_to_header() after processing each identity line. When rewrite_ident_line() processes an identity, it stops at the end of the identity data (e.g., "Author Name <email> timestamp"), but doesn't account for the trailing newline. The current code adds the identity length to buf_offset but fails to advance past the newline character. This causes the next iteration to start parsing from the newline instead of the beginning of the next header line, making it impossible to match subsequent headers like "committer". Additionally, rewrite_ident_line() may reallocate the buffer during its operation. Any code using pointers into the old buffer would be using invalid memory after such a reallocation. This bug was introduced in e9c1b0e3 (revision: improve commit_rewrite_person(), 2022-07-19) when the much simpler version of commit_rewrite_person() that worked on one "person header" at a time was rewritten to use the current apply_mailmap_to_header() function. The original implementation processed author and committer separately, but the rewrite introduced this loop-based approach that failed to properly handle the transition between identity lines. Let's fix this by addressing both issues: 1. After processing an identity line, we now check if we're at a newline and advance past it, ensuring the next header line is parsed correctly. 2. We recompute the buffer position after rewrite_ident_line() to handle potential buffer reallocation. This ensures that all identity headers in commit and tag objects are consistently processed regardless of whether the author and committer are the same person. Reported-by: Vasilii Iakliushin <viakliushin@gitlab.com> Reviewed-by: Christian Couder <christian.couder@gmail.com> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run_builtin() takes a repo parameter, so the use of the_repository is no longer necessary. Removed the usage of the_repository. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
'test_path_is_file', 'test_path_is_dir' and 'test_file_is_missing' are test helpers used in Git's development, that emit useful diagnostic information when they detect a failing condition, while test -[efd] does not. Replace the basic shell commands 'test -f', 'test -d' and 'test -e', with these test helpers. Co-authored-by: Isabella Caselli <icaselli@usp.br> Signed-off-by: Isabella Caselli <icaselli@usp.br> Signed-off-by: Rodrigo Michelassi <rodmichelassi@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git subtree" (in contrib/) learns to grok GPG signing its commits. * pw/subtree-gpg-sign: contrib/subtree: add -S/--gpg-sign contrib/subtree: parse using --stuck-long
"git stash -p <pathspec>" improvements. * pw/stash-p-pathspec-fixes: stash: allow "git stash [<options>] --patch <pathspec>" to assume push stash: allow "git stash -p <pathspec>" to assume push again
"git send-email" incremented its internal message counter when a message was edited, which made logic that treats the first message specially misbehave, which has been corrected. * ag/send-email-edit-threading-fix: send-email: show the new message id assigned by outlook in the logs send-email: fix bug resulting in broken threads if a message is edited
"git stash" recorded a wrong branch name when submodules are present in the current checkout, which has been corrected. * kj/stash-onbranch-submodule-fix: stash: fix incorrect branch name in stash message
Leakfix. * ly/prepare-show-merge-leakfix: revision: fix memory leak in prepare_show_merge()
Code clean-up. * ac/preload-index-wo-the-repository: preload-index: stop depending on 'the_repository' environment: remove the global variable 'core_preload_index'
Clarify "do not explicitly initialize to zero" rule in the CodingGuidelines document. * jc/cg-let-bss-do-its-job: CodingGuidelines: let BSS do its job
When asking to apply mailmap to both author and committer field while showing a commit object, the field that appears later was not correctly parsed and replaced, which has been corrected. * sa/multi-mailmap-fix: cat-file: fix mailmap application for different author and committer
Test clean-up. * rm/t2400-modernize: t2400: replace 'test -[efd]' with 'test_path_is_*'
Code clean-up. * ly/run-builtin-use-passed-in-repo: git.c: remove the_repository dependence in run_builtin()
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.1)
Can you help keep this open source service alive? 💖 Please sponsor : )