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

[Martin Storsjö]

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.

>>> 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.

// Martin