threads / patch / 63521

v3, 2 partsRe: [PATCH v3 1/2] send-email: fix bug resulting in increased message number if a message is edited

Subject: Re: [PATCH v3 1/2] send-email: fix bug resulting in increased message number if a message is edited

## tl;dr

2 messages between May 25, 2025 and May 25, 2025. Diffs are folded; open one to read it.

replies: 1people: 1as markdown or json

Aditya Garg· May 25, 2025, 19:05 UTC · lore

On 26/05/25 12:19 am, Kristoffer Haugsbakk wrote:
Show 64 quoted lines
> Hi
> 
>> send-email: fix bug resulting in increased message number if a message is edited
> 
> I don’t understand what the bug is from the title.  “Message number”
> sounds harmless.  It breaks the threading?  The summary/subject could
> say that instead.  Fix threading bug.
> 
> On Sun, May 25, 2025, at 19:12, Aditya Garg wrote:
>> In case a message is edited before it is sent, its message number gets
>> increased by 1, and so does its order in the message id.
> 
> It feels like this part about increasing by one and if-editing gets
> repeated at least two times in this paragraph.
> 
>> The cause of this bug was that when a person attempts to edit the
>> message, the whole sub process_file gets terminated, and the user is
>> asked to edit the message.
> 
> Here’s the repetition.
> 
> Also I am not familiar with the code.  Just testing it I get this `6` here:
> 
>    Message-ID: <20250525181003.40129-6-kristofferhaugsbakk@fastmail.com>
> 
> Which was incremented every time I did an edit with:
> 
>    send-email --suppress-cc=all --to=<me> \
>        --confirm=always one two
> 
> But that turned out to be benign in my simple case since the next email
> used the correct In-Reply-To.
> 
> So at this point (reading the paragraph) I don’t know what the bug is.
> 
>> After necessary edits are done, the whole sub process_file is executed again.
>> The way sub process_file is designed, every time is runs, it increases the
>> $message_num variable by 1. The reason for this was that the function ran
>> again everytime a next message was sent in a thread, and thus we need to
>> increase the message number for that message. In case a user edits the message,
>> there is no check for the same and the new message gets treated as a subsequent
>> message of a thread, therefore increasing its message number by one.
> 
> This feels like repetition again.  You say that a variable is
> incremented because the message is edited.
> 
>> This breaks the shallow thread logic which relies on $message_num
>> being 1 for the first message, and it gets changed in case the user
>> edits the first message.
> 
> If I’m right in my assumption that this number is the `4` here:
> 
>    Message-ID: <20250525182426.41076-4-kristofferhaugsbakk@fastmail.com>
> 
> This was the first proposed email I got with “shallow thread” (all in
> reply to first):
> 
>    git send-email --suppress-cc=all --to=<me> \
>        --thread --no-chain-reply-to --confirm=always one two three
> 
> Then I edit all the messages.  They still all manage to refer to the
> first message id in the thread.
> 
> I still don’t understand what the bug is.
Steps to reproduce:
1. Run `git send-email --to=someone@example.com HEAD~3 --in-reply-to=some_message_id`
2. Edit the first patch (pressing e and enter) when send-email asks for confirmation.
(You may have to set confirm = always)
3. Do the edits. The message id of the first patch should have 2 instead of 1. If yes,
send all the messages and watch the threads break.
Show 19 quoted lines
> 
> ❦
> 
>    $ git diagnose
>    Collecting diagnostic info
> 
>    git version 2.49.0.780.g892193c3f50
>    cpu: x86_64
>    built from commit: 892193c3f509fb8a9e4e7a5a19a2e24137befda8
>    sizeof-long: 8
>    sizeof-size_t: 8
>    shell-path: /bin/sh
>    libcurl: 7.81.0
>    OpenSSL: OpenSSL 3.0.2 15 Mar 2022
>    zlib: 1.2.11
>    SHA-1: SHA1_DC
>    SHA-256: SHA256_BLK
>    Repository root: /home/kristoffer/programming/git
>    Available space on '/home/kristoffer/programming/git': 200.56 GiB (mount flags 0x1000)
Aditya Garg· May 25, 2025, 19:07 UTC · re: Aditya Garg · lore
Show 73 quoted lines
> On 26 May 2025, at 12:35 AM, Aditya Garg <adityagarg1208@gmail.com> wrote:
> 
> 
> 
>> On 26/05/25 12:19 am, Kristoffer Haugsbakk wrote:
>> Hi
>> 
>>> send-email: fix bug resulting in increased message number if a message is edited
>> 
>> I don’t understand what the bug is from the title.  “Message number”
>> sounds harmless.  It breaks the threading?  The summary/subject could
>> say that instead.  Fix threading bug.
>> 
>>> On Sun, May 25, 2025, at 19:12, Aditya Garg wrote:
>>> In case a message is edited before it is sent, its message number gets
>>> increased by 1, and so does its order in the message id.
>> 
>> It feels like this part about increasing by one and if-editing gets
>> repeated at least two times in this paragraph.
>> 
>>> The cause of this bug was that when a person attempts to edit the
>>> message, the whole sub process_file gets terminated, and the user is
>>> asked to edit the message.
>> 
>> Here’s the repetition.
>> 
>> Also I am not familiar with the code.  Just testing it I get this `6` here:
>> 
>>   Message-ID: <20250525181003.40129-6-kristofferhaugsbakk@fastmail.com>
>> 
>> Which was incremented every time I did an edit with:
>> 
>>   send-email --suppress-cc=all --to=<me> \
>>       --confirm=always one two
>> 
>> But that turned out to be benign in my simple case since the next email
>> used the correct In-Reply-To.
>> 
>> So at this point (reading the paragraph) I don’t know what the bug is.
>> 
>>> After necessary edits are done, the whole sub process_file is executed again.
>>> The way sub process_file is designed, every time is runs, it increases the
>>> $message_num variable by 1. The reason for this was that the function ran
>>> again everytime a next message was sent in a thread, and thus we need to
>>> increase the message number for that message. In case a user edits the message,
>>> there is no check for the same and the new message gets treated as a subsequent
>>> message of a thread, therefore increasing its message number by one.
>> 
>> This feels like repetition again.  You say that a variable is
>> incremented because the message is edited.
>> 
>>> This breaks the shallow thread logic which relies on $message_num
>>> being 1 for the first message, and it gets changed in case the user
>>> edits the first message.
>> 
>> If I’m right in my assumption that this number is the `4` here:
>> 
>>   Message-ID: <20250525182426.41076-4-kristofferhaugsbakk@fastmail.com>
>> 
>> This was the first proposed email I got with “shallow thread” (all in
>> reply to first):
>> 
>>   git send-email --suppress-cc=all --to=<me> \
>>       --thread --no-chain-reply-to --confirm=always one two three
>> 
>> Then I edit all the messages.  They still all manage to refer to the
>> first message id in the thread.
>> 
>> I still don’t understand what the bug is.
> 
> Steps to reproduce:
> 
> 1. Run `git send-email --to=someone@example.com HEAD~3 --in-reply-to=some_message_id`
You didn't specify --in-reply-to in your command
Show 6 quoted lines
> 
> 2. Edit the first patch (pressing e and enter) when send-email asks for confirmation.
> (You may have to set confirm = always)
> 
> 3. Do the edits. The message id of the first patch should have 2 instead of 1. If yes,
> send all the messages and watch the threads break.
You also didn't edit the first patch, and rather edited some other one?

← back to recent threads