[vlc-devel] [PATCH 4/5] configure: add an option to generate PDB files when building

[Steve Lhomme]

On 2019-05-17 10:17, Martin Storsjö wrote:
> On Fri, 17 May 2019, Steve Lhomme wrote:
> 
>> On 2019-05-17 8:18, Martin Storsjö wrote:
>>> On Fri, 17 May 2019, Steve Lhomme wrote:
>>>
>>>> On 2019-05-16 21:35, Martin Storsjö wrote:
>>>>> On Thu, 16 May 2019, Steve Lhomme wrote:
>>>>>
>>>>>> This will only be enabled when building windows targets and will 
>>>>>> only work when
>>>>>> compiling with Clang.
>>>>>> ---
>>>>>> bin/Makefile.am   |  3 +++
>>>>>> configure.ac      | 13 +++++++++++++
>>>>>> lib/Makefile.am   |  3 +++
>>>>>> modules/common.am |  3 +++
>>>>>> src/Makefile.am   |  3 +++
>>>>>> 5 files changed, 25 insertions(+)
>>>>>
>>>>> Thanks; this patchset is much clearer.
>>>>>
>>>>> However, if we'd go with tweaking the lld option (which is 
>>>>> progressing fine upstream at https://reviews.llvm.org/D62004), we 
>>>>> could get rid of patch 1 (the libtool patch) altogether, and get 
>>>>> rid of all the changes to Makefile.am here, by just adding 
>>>>> -Wl,-pdb= to the global LDFLAGS.
>>>>
>>>> One of the use of the libtool feature is that it handles the .pdb 
>>>> cleaning with "make clean".
>>>
>>> That's a valid point, but I don't see that handling in the libtool 
>>> patch in 1/5 in the latest series? The same would be to have it go 
>>> along with "make install" as well.
>>
>> I left "make install" out on purpose because a build might be created 
>> with PDB for debugging but not "installed" as such. In the case of VLC 
>> it doesn't matter since we repackage the exectuables for distribution 
>> but other projects may not do that. If we want a chance to upstream 
>> the libtool patch it's better that way.
>>
>> "make clean" actually deletes the .libs folder, so the .pdb is 
>> included inside.
>> libtool also handles removing the previous .pdb before creating a new 
>> one as shown with "V=1 make":
>>
>> libtool: link: rm -fr  .libs/libwdummy_plugin.dll 
>> .libs/libwdummy_plugin.dll.a .libs/libwdummy_plugin.dll.def 
>> .libs/libwdummy_plugin.exp .libs/libwdummy_plugin.la 
>> .libs/libwdummy_plugin.lai .libs/libwdummy_plugin.pdb
> 
> Yes, but this is not achieved by your patch. Libtool will do the same 
> even without the patch, if the linker writes a file into 
> .libs/<outputname>.whatever, it seems. So that's not an argument for 
> carrying yet another libtool patch.

Indeed

>>>> I'm also not sure about pathes to find the .PDB when built with WSL 
>>>> (I haven't tried it yet). So I prefer to put the PDB next to the 
>>>> DLL, in the .libs folder. And that's a libtool folder, it's unknown 
>>>> to the makefiles.
>>>
>>> Yes, with the -Wl,-pdb= feature for implicitly naming the pdb file, 
>>> it will go there. Libtool calls the real linker with "-o 
>>> .libs/myplugin.dll", and the linker would in this case make the 
>>> implicit pdb output name .libs/myplugin.pdb.
>>
>> Indeed, in the future we can get rid of this patch, hopefully. For now 
>> I think it's better to keep it.
> 
> Well I guess it's up to others if they are ok with the extra makefile 
> clutter with the -Wl,-pdb parameters (and the extra libtool patch), or 
> if we should wait for the new lld option.
> 
> If you want to try it out, you can try 
> https://martin.st/temp/llvm-mingw-pdb-x86_64.zip (or just unzip it and 
> copy the updated ld.lld.exe from it); this one supports "-Wl,-pdb=" 
> which will implicitly pick the pdb output name.

I confirm it works. I'll prepare a new patchset without the libtool 
tweak and -Wl,-pdb= now that your patch has been merged.