[vlc-devel] [PATCH 5/5] win32: package: install generated .pdb files with package-win-common

[Martin Storsjö]

On Thu, 16 May 2019, Martin Storsjö wrote:

> On Thu, 16 May 2019, Martin Storsjö wrote:
>
>> On Thu, 16 May 2019, Steve Lhomme wrote:
>> 
>>> ---
>>> extras/package/win32/package.mak | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>> 
>>> diff --git a/extras/package/win32/package.mak 
>> b/extras/package/win32/package.mak
>>> index 0d7e57bde6..54eabae64d 100644
>>> --- a/extras/package/win32/package.mak
>>> +++ b/extras/package/win32/package.mak
>>> @@ -40,6 +40,11 @@ package-win-sdk: package-win-install
>>> package-win-common: package-win-install package-win-sdk
>>> # Executables, major libs
>>> 	find $(prefix) -maxdepth 4 \( -name "*$(LIBEXT)" -o -name 
>> "*$(EXEEXT)" \) -exec cp {} "$(win32_destdir)/" \;
>>> +if HAVE_PDB
>>> +	for dir in bin lib src; do \
>>> +	  cp $$dir/.libs/*.pdb $(win32_destdir); \
>>> +	done
>>> +endif
>>> 
>>> # Text files, clean them from mail addresses
>>> 	for file in AUTHORS THANKS ; \
>>> @@ -55,6 +60,12 @@ package-win-common: package-win-install package-win-sdk
>>> 		mkdir -p "$$plugin_destdir"; \
>>> 		find "$$plugindir" -type f \( -not -name '*.la' -and -not 
>> -name '*.a' \) -exec cp -v "{}" "$$plugin_destdir" \; ;\
>>> 	done
>>> +if HAVE_PDB
>>> +	for plugindir in $(prefix)/lib/vlc/plugins/*/; do \
>>> +		plugin_destdir="$(win32_destdir)/plugins/`basename 
>> $$plugindir`"; \
>>> +		for plugin in $$(find "$$plugindir" -type f \( -not -name 
>> '*.la' -and -not -name '*.a' \)); do cp modules/.libs/$$(basename 
>> "$$plugin" | sed s/dll/pdb/) "$$plugin_destdir"; done; \
>>> +	done
>>> +endif
>> 
>> This patch seems confused. The produced pdb files are not in the .libs 
>> subdir, but directly in the normal directory (next to the .la files).
>> 
>> When building with these patches, building fails at the install stage as 
>> the "cp $$dir/.libs/*.pdb" command fails as there is no such file.
>> 
>> If you want them to be placed in the .libs subdir (to make running in the 
>> build tree easier), then you'd need to change patch 4/5 to place the output 
>> pdb in the .libs subdir instead.
>
> Sorry, now I see, your libtool patch does this (I didn't try that one as I 
> thought it was only necessary for the --generate-pdb option).

Actually, wouldn't it be simpler to just extend lld's -pdb option, to make 
it possible to set the pdb output name implicitly? (It's a custom, lld 
specific option as GNU ld doesn't have anything similar, so it's easy to 
extend. And so far, I think only Firefox has taken this option into use, 
so if we'd need to break it, we'd only need to synchronize with them.)

If we could pass a generic -Wl,--default-named-pdb in LDFLAGS everywhere, 
we could get rid of the libtool patch and reduce most of the other 
patches, right? (The only thing missing for libtool is that it wouldn't 
take care of installing the pdb along with exes/dlls though.)

The only open question wrt this is how to name it. The current syntax is 
"-pdb outputname", and then it's a bit hard to leave out the filename to 
indicate default.

Either we'd add something like --default-named-pdb, or add a new syntax 
for the option, so it'd be -pdb=outputfile, which allows you to do -pdb= 
which would imply the default. I tried discussing this with Firefox devs 
at https://bugzilla.mozilla.org/show_bug.cgi?id=1475562#c48 as well, where 
-pdb= and -debug (similar to link.exe's flag) were suggested.

Would that be worth pursuing, to reduce the amount of clutter that these 
patches produce?

// Martin