Message-ID: <dfc84b5c-3ff0-446b-84b8-7cbd1a74f566@gmail.com>
Date: Thu, 30 Oct 2025 08:38:03 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
To: Jakub Wartak <jakub.wartak@enterprisedb.com>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
 Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <202510300932.isdvgkzqzshk@alvherre.pgsql>
 <5b4c6ce0-ed17-4abb-9127-859cc95cbdf3@gmail.com>
 <CAKZiRmzuqPmgd_A+M+RiuT-4UtDt_TASqQJoqCdvCUrVY3w7cg@mail.gmail.com>
Content-Language: en-US
From: Bryan Green <dbryan.green@gmail.com>
In-Reply-To: 
 <CAKZiRmzuqPmgd_A+M+RiuT-4UtDt_TASqQJoqCdvCUrVY3w7cg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/dfc84b5c-3ff0-446b-84b8-7cbd1a74f566%40gmail.com>
Precedence: bulk

On 10/30/2025 3:52 AM, Jakub Wartak wrote:
> On Thu, Oct 30, 2025 at 10:40 AM Bryan Green <dbryan.green@gmail.com> wrote:
>>
>> On 10/30/2025 3:37 AM, Álvaro Herrera wrote:
>>> On 2025-Oct-30, Jakub Wartak wrote:
>>>
>>>> Hi Bryan, cfbot is red. I'm was fan of having those tests for this
>>>> (bring complexity and we didn't have tests for Linux backtrace
>>>> anyway), but now MINGW win32 is failing on those tests where the
>>>> feature is not present:
>>>
>>> I hate to say this after the code is written, but I think we should not
>>> put any tests in the first step.  I predict that these are going to be
>>> enormously brittle and that we'll waste a lot of time making them
>>> stable.  I think we should commit the Windows support for backtraces
>>> first, then consider whether we actually want TAP tests for the overall
>>> feature.  We've gone several years with glibc backtrace support without
>>> any tests -- why do we think the Windows implementation thereof _must_
>>> necessarily have them?
>>>
>> It will not bother me to remove them.  It was my first effort at writing
>> TAP tests, so it was a nice learning experience.
> 
> Well, that was a typo on my part (stupid me), I wanted to write: I was
> NOT a fan of having those tests for this (in first place) - sorry for
> confusion!
> 
> Anyway we have test because I think Michael and Euler triggered this
> but earlier i've tried to persuade NOT to do this (see: `Also is it
> worth it to test that setting backtrace_funciton=FOO really emits
> .*FOO.* in log message cross-platform way?`), anyway Bryan implemented
> this and it looks like v3 has just turned [gG]reen ;)
> (https://cirrus-ci.com/build/6001832838823936)
> 
> -J.
The tests are easy enough to get rid of. I think Alvaro has a good idea 
of committing the windows support for backtraces and then consider 
whether we want TAP tests or not.  I will make a v4 patch without the 
TAP tests unless someone strongly objects.

The tests don't really test whether this code would be the cause of 
cause of a problem, they mainly test whether you are getting the correct 
output in your backtrace.  If you have a pdb file, you should get 
filenames and linenumbers in addition to addresses and symbols.  If you 
don't have a pdb file you will only get export function symbols and 
addresses.  So, even if you should have a pdb and don't...you still get 
something useful.

Bryan Green