Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: for Windows builds, defaults PROJ DLL to be just 'proj_${PROJ_MAJOR_VERSION}.dll' #4167

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Jun 5, 2024

Fixes #4151 . Aligns with GDAL PR OSGeo/gdal#5701

@hobu Any opinion regarding that ? Current behaviour of having 'proj_X_Y.dll' comes from the initial CMake commit of yours 532a0f5 from 10 years ago.

@rouault rouault added this to the 9.5.0 milestone Jun 5, 2024
@rouault
Copy link
Member Author

rouault commented Jul 1, 2024

@kbevers any opinion w.r.t this change?

@kbevers
Copy link
Member

kbevers commented Jul 2, 2024

I'm not sure I've understood the change. Currently on Windows we produce a DLL called proj.dll except when using MingW where you instead get proj_9_4.dll, correct? And this changes the MingW-dll's name to be based on the soname, making it proj_25.dll (or whatever the current soversion is), yes?

Assuming I've understood that correctly, I think this looks like a change that will break peoples workflows. Probably not too many people are relying on MingW but some will for sure and I don't think we can just change things willy-nilly. Would it be possible to create to versions of the DLL, one with the current name and one with the proposed name? That would keep backwards compatibility, at least.

@rouault
Copy link
Member Author

rouault commented Jul 2, 2024

currently on Windows we produce a DLL called proj.dll except when using MingW where you instead get proj_9_4.dll, correct?

no, we produce proj_9_4.dll for both MSVC and MinGW currently.
This PR changes to "proj.dll" for MSVC and "proj-{soname}.dll" for MinGW.

I think this looks like a change that will break peoples workflows

yes, possibly

Would it be possible to create to versions of the DLL, one with the current name and one with the proposed name?

Hum, this would complicate things a bit more than what I'd like, and would probably cause some confusion. If we are not ready for that change in the 9.x series, it would probably be better to just drop it for now. Or maybe had a CMake variable PROJ_NEW_DLL_NAME=ON/OFF ? PROJ_10_COMPATIBLE_DLL_NAME=ON/OFF ?

@kbevers
Copy link
Member

kbevers commented Jul 2, 2024

I think a CMake variable that switches to a future PROJ 10 behaviour is a good solution. If you don't do anything you'll get the same as always and if you ask for it you get the improved naming suggested in this PR.

In PROJ 10 we can then get rid of the current naming scheme for Windows builds.

@hobu
Copy link
Contributor

hobu commented Jul 4, 2024

@hobu Any opinion regarding that ? Current behaviour of having 'proj_X_Y.dll' comes from the initial CMake commit of yours 532a0f5 from 10 years ago.

IIRC the reason why we did this was because the APIs of proj.dll had changed quite significantly and we wanted to break anyone who was doing makefile.vc-based builds such that they would need to update things to switch over their linkage for CMake. I don't know that it is so relevant anymore, but I would say that proj.dll is a quite terrible DLL name for being super generic and more likely to DLLHell with some other random DLL from a lazy developer who shorted "project" to "proj" and let their generate emit defaults.

@kbevers
Copy link
Member

kbevers commented Jul 5, 2024

I don't know that it is so relevant anymore, but I would say that proj.dll is a quite terrible DLL name for being super generic and more likely to DLLHell with some other random DLL from a lazy developer who shorted "project" to "proj" and let their generate emit defaults.

Perhaps a compromise could be to include the current major version number in the name? This way, updates to PROJ builds within the major version shouldn't cause any problems. In case there's a major version update users will quickly realise since the name doesn't match any more.

@hobu
Copy link
Contributor

hobu commented Jul 5, 2024

This way, updates to PROJ builds within the major version shouldn't cause any problems. In case there's a major version update users will quickly realise since the name doesn't match any more.

I don't mind this but it probably doesn't satisfy the original complaint of having version numbers in the DLL name. Maybe we should just rename it every major release to a random projection family name 😄

@rouault
Copy link
Member Author

rouault commented Jul 6, 2024

Perhaps a compromise could be to include the current major version number in the name? This way, updates to PROJ builds within the major version shouldn't cause any problems

@visr / @joa-quim opinion?

@visr
Copy link
Contributor

visr commented Jul 6, 2024

Sorry I missed this PR, thanks for putting it up. The issue from #4151 is fixed by either proj.dll, proj-{major}.dll, or proj-{soversion}.dll. The only reason I argued for the latter is for consistency with GDAL, but all would be an improvement. I guess proj-{major}.dll still could have some of the original issue if the breaking changes all relate to unrelated parts of the PROJ project, and the SOVERSION can stay the same.

If DLL names change from including the minor release I think it could be done in a minor release, since due to this issue it already broke every minor release. Though any build scripts that assume the current naming scheme need updating, so an opt-in until the next major release could also make sense.

@joa-quim
Copy link
Contributor

joa-quim commented Jul 6, 2024

I favor the idea having a variable to let choose the DLL name. In GDAL we have GDAL_LIB_OUTPUT_NAME for that purpose and I don't see why PROJ should not do the same. For my builds I have patched some files to allow me choosing the dll name so, this change would be wellcome. A different matter is the default name. For me it should be a name with no version numbers in it. And, is there really a reason why MSVC and MINGW builds have different names?

@rouault
Copy link
Member Author

rouault commented Jul 9, 2024

not easy to find a middle ground for all opinions expressed. I've revised this PR as:

    CMake: for Windows builds, defaults PROJ DLL to be just 'proj_${PROJ_MAJOR_VERSION}.dll'
    
    instead of "proj_${PROJ_MAJOR_VERSION}_${PROJ_MINOR_VERSION}.dll"
    
    This is configurable (for all platforms) by setting the new
    PROJ_OUTPUT_NAME Make Variable
    
    On MinGW, the APPEND_SOVERSION option can be set to ON, to add a "-${PROJ_SOVERSION}"
    suffix to the PROJ shared library name.
    
    Fixes #4151

This should hopeully be flexible enough for people to give the name they wish, and have relatively stable .dll name

@rouault rouault changed the title CMake: for Windows builds, name proj DLL just 'proj.dll', except on mingw where it will be 'proj-{soname}.dll' CMake: for Windows builds, defaults PROJ DLL to be just 'proj_${PROJ_MAJOR_VERSION}.dll' Jul 9, 2024
…MAJOR_VERSION}.dll'

instead of "proj_${PROJ_MAJOR_VERSION}_${PROJ_MINOR_VERSION}.dll"

This is configurable (for all platforms) by setting the new
PROJ_OUTPUT_NAME Make Variable

On MinGW, the APPEND_SOVERSION option can be set to ON, to add a "-${PROJ_SOVERSION}"
suffix to the PROJ shared library name.

Fixes OSGeo#4151
@jmckenna
Copy link
Contributor

jmckenna commented Jul 9, 2024

thanks @rouault, PROJ_OUTPUT_NAME is very helpful for packagers, thanks so much!

@rouault rouault merged commit d6311ab into OSGeo:master Jul 10, 2024
23 checks passed
visr added a commit to visr/Yggdrasil that referenced this pull request Sep 29, 2024
Fixes JuliaPackaging#8780. This is hopefully the last minor release that is breaking for us as they the devs changed their DLL naming after our issue. Now by default only the major version is included in the name, though other naming options are also configurable.

OSGeo/PROJ#4167
https://proj.org/en/9.5/news.html
giordano pushed a commit to JuliaPackaging/Yggdrasil that referenced this pull request Oct 1, 2024
* [PROJ] update to 9.5

Fixes #8780. This is hopefully the last minor release that is breaking for us as they the devs changed their DLL naming after our issue. Now by default only the major version is included in the name, though other naming options are also configurable.

OSGeo/PROJ#4167
https://proj.org/en/9.5/news.html

* Disable aarch64 for freebsd for now
avik-pal pushed a commit to avik-pal/Yggdrasil that referenced this pull request Oct 25, 2024
* [PROJ] update to 9.5

Fixes JuliaPackaging#8780. This is hopefully the last minor release that is breaking for us as they the devs changed their DLL naming after our issue. Now by default only the major version is included in the name, though other naming options are also configurable.

OSGeo/PROJ#4167
https://proj.org/en/9.5/news.html

* Disable aarch64 for freebsd for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor version in libproj DLL suffix for MinGW
6 participants