Created attachment 134762 [details] [review]
Install pkgconfig files

I don't see a reason why the suffix should be added.

i pushed the pkgconfig change

suffix: well I suppose it's the same reason why there is an so version under linux, it is by no means uncommon, and indeed the default behaviour of i.e. autotools. In any event, as you please, I have no problems just carrying the patch downstream.

(In reply to Sandro Mani from comment #4)
> suffix: well I suppose it's the same reason why there is an so version under
> linux, it is by no means uncommon, and indeed the default behaviour of i.e.
> autotools. In any event, as you please, I have no problems just carrying the
> patch downstream.

But is it the default of windows? i've never seen a dll in windows having a suffix number

It depends on the toolchain I suppose, with mingw it is certainly not uncommon. How about just adding them for MINGW?

I wouldn't disagree with that if it seems that is actually what mingw does.

Is there a guideline or something you could share?

I've looked around a bit and can't really find much, except that besides autotools also qmake appears to do it by default, as I gather from [1].

As an experiment, I looked at all dlls of all Fedora mingw packages:

$ dnf provides '/usr/x86_64-w64-mingw32/sys-root/mingw/bin/*.dll' | grep Filename | awk '{print $3}' | sort | uniq > dlls

$ cat dlls | wc -l
714

$ cat dlls | grep -Ev '[0-9].dll' | grep -v "^Qt5" | wc -l
343

So out of 714 dlls, 343 have no versioning.

A strong argument for versioning IMO is that from a packaging point of view it enforces versioned dependencies between packages, i.e.:

$ rpm -q --requires mingw64-gdal
mingw64(iconv.dll)
mingw64(kernel32.dll)
mingw64(libcfitsio-3.dll)
mingw64(libcurl-4.dll)
mingw64(libexpat-1.dll)
mingw64(libfreexl-1.dll)
mingw64(libgcc_s_seh-1.dll)
mingw64(libgeos_c-1.dll)
mingw64(libgeotiff-2.dll)
mingw64(libgif-7.dll)
mingw64(libgrib2c.dll)
mingw64(libgta-0.dll)
mingw64(libjasper-4.dll)
mingw64(libjpeg-62.dll)
mingw64(libkmlbase.dll)
mingw64(libkmldom.dll)
mingw64(libkmlengine.dll)
mingw64(libpcre-1.dll)
mingw64(libpng16-16.dll)
mingw64(libpoppler-71.dll)
mingw64(libpq.dll)
mingw64(libproj-12.dll)
mingw64(libspatialite-4.dll)
mingw64(libsqlite3-0.dll)
mingw64(libstdc++-6.dll)
mingw64(libtiff-5.dll)
mingw64(libwebp-7.dll)
mingw64(libxerces-c-3-1.dll)
mingw64(libxml2-2.dll)
mingw64(msvcrt.dll)
mingw64(odbc32.dll)
mingw64(odbccp32.dll)
mingw64(psapi.dll)
mingw64(user32.dll)
mingw64(ws2_32.dll)
mingw64(zlib1.dll)

[1] https://stackoverflow.com/questions/404774/why-library-name-gets-an-additional-0-in-its-name

ok, if you can provice a better patch than the one you proposed, i.e. one where you don't duplicate the numbers like the one that you proposed, i guess i'd accept it.

Created attachment 135216 [details] [review]
Add soversion suffix to dlls (V2)

Here you go

can you please try to see if there's another way of doing that doesn't involve new variables? this kind of breaks my release workflow

Hmm I think the only alternative is

set_target_properties(poppler PROPERTIES VERSION 71.0.0 SOVERSION 71)
if(MINGW)
    get_target_property(LIBPOPPLER_SOVERSION poppler SOVERSION)
    set_target_properties(poppler PROPERTIES SUFFIX "-${LIBPOPPLER_SOVERSION}${CMAKE_SHARED_LIBRARY_SUFFIX}")
endif(MINGW)

would that work?

if that works, yes, that is fine, i just want the patch to be

if (mingw)
things i should never touch
endif()

wihtout touching the rest of the code as much as possible :)

Created attachment 135217 [details] [review]
Add soversion suffix to dlls (V3)

pushed