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

Create and install shared library with version on MinGW #663

Merged
merged 1 commit into from Dec 15, 2022

Conversation

rhabacker
Copy link
Contributor

Description

In package-oriented distributions like openSUSE, shared libraries are usually installed with a version to better handle dependencies between packages.

The main rationale is to allow for two or more (incompatible) versions of a shared library, such as libfoo.so.1 and libfoo.so.2 to be independently selectively installable (and trackable by rpm) at the same time without causing package conflicts.

To achieve this also for MinGW based packages installed on a distribution, the shared library 'vsg' for this compiler is also created and installed with version. Since the name of the generated import library is not affected, there are no changes for dependent packages.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Test Configuration:

  • Host: opensuse cloud build system
  • Toolchain: mingw gcc 12
  • SDK: mingw-header/runtime 10

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code

In package-oriented distributions like openSUSE, shared libraries are
usually installed with a version to better handle dependencies between
packages.

The main rationale is to allow for two or more (incompatible)
versions of a shared library, such as libfoo.so.1 and libfoo.so.2 to be
independently selectively installable (and trackable by rpm) at the same
time without causing package conflicts.

To achieve this also for MinGW based packages installed on a distribution,
the shared library 'vsg' for this compiler is also created and installed
with version.
@robertosfield
Copy link
Collaborator

I'm reluctant to add platform specific special cases, I would like to explore exactly what the VSG installs right now on MingW without these changes, and what it does with them.

For reference on my Kubuntu system when I compile as shared library and install I see the libs:

$ ls ~/Install/lib/libvsg*.so* -1
/home/robert/Install/lib/libvsg.so
/home/robert/Install/lib/libvsg.so.1.0.3
/home/robert/Install/lib/libvsg.so.11

From the sound of it you are trying to replicate this in MinGW, but I'm not clear what parts you've replicated, what parts you haven't. Having things work different on different platforms is not idea, but fitting in with local contents is also needs to be born in mind.

I presume other CMake users have this same issue under MinGW/Windows so perhaps CMake itself could provide the behavior rather than having to replicate it in our local CMake scripts.

@rhabacker
Copy link
Contributor Author

rhabacker commented Dec 15, 2022

An installation of the mentioned files on an openSUSE 64bit distribution would lead to the following results

usr/lib64/libvsg.so
/usr/lib64/libvsg.so.1.0.3
/usr/lib64/libvsg.so.11

These files would be put into two packages:

  • Development package (named libvsg-devel)

    /usr/lib64/libvsg.so

  • associated runtime package (libvsg11)

    /usr/lib64/libvsg.so.1.0.3
    /usr/lib64/libvsg.so.11

On the same distribution, an installation of mingw32-libvsg creates

/usr/i686-w64-mingw32/sys-root/mingw/lib/libvsg.dll.a 
/usr/i686-w64-mingw32/sys-root/mingw/bin/libvsg.dll

These files would also be placed in two packages:

  • Development package (named mingw32-libvsg-devel).

    /usr/i686-w64-mingw32/sys-root/mingw/lib/libvsg.dll.a

  • Shared library in the runtime package (mingw32-libvsg)

    /usr/i686-w64-mingw32/sys-root/mingw/bin/libvsg.dll

Only the shared library should be changed, which should get a suffix -<SOVERSION>, as for example happened with the existing curl library.

/usr/i686-w64-mingw32/sys-root/mingw/bin/libcurl-4.dll

For vsg this should be done by changing from

/usr/i686-w64-mingw32/sys-root/mingw/bin/libvsg.dll

to

/usr/i686-w64-mingw32/sys-root/mingw/bin/libvsg-11.dll

using the current SOVERSION.

I assume other CMake users have the same problem on MinGW/Windows, so maybe CMake itself could provide the behavior instead of replicating it in our local CMake scripts.

In the current cmake version 3.25, according to current knowledge, such support is not included, so it has always been done in the local CMake scripts (see osg for example).

I will open a corresponding request and submit a matching patch. The patch, if accepted, would probably not be included before cmake version 3.27, i.e. until this version is widely distributed, one would have to make do with the current patch.

If it is known with which version the patch will be released, one could change to the use the new cmake build-in support with this

if(MINGW AND CMAKE_VERSION VERSION_GREATER_EQUAL 3.27.0)
   set(CMAKE_DLL_NAME_WITH_SOVERSION 1)
elseif(MINGW)
    set_property(TARGET vsg PROPERTY RUNTIME_OUTPUT_NAME vsg-${VSG_SOVERSION})
endif()

Update: see https://gitlab.kitware.com/cmake/cmake/-/merge_requests/8021 for an associated request.

@robertosfield
Copy link
Collaborator

Thanks for the explanation.

Is there any reason why we shouldn't apply the -${SO_VERSION} to all Windows shared library builds?

My preference would be for Windows have libvsg-${SO_VERSION}.dll and libvsg.a that references it, regardless of the build tool you use.

The .dll.a seems a bit awkward but I can see that it would allow one to build a static and shader library version of the VSG and install it on one system, so can see value in using that. Are other projects using the .dll.a convention?

@rhabacker
Copy link
Contributor Author

Thanks for the explanation.

Is there any reason why we should not apply the -${SO_VERSION} to all Windows shared library builds?

For the local patch, it can be easily extended by using WIN32 instead of MINGW.
Currently I only have a working patch for mingw. To have appropriate support in cmake, this would need to be investigated, implemented and tested. May be it is easy too, but I haven't looked yet.

I would prefer Windows to have libvsg-${SO_VERSION}.dll and libvsg.a pointing to it, regardless of the build tool used.

Unfortunately, the compilers and build systems see it differently.

The .dll.a seems a bit cumbersome.

There are fixed conventions built into the buildsystems and compilers/linkers. e.g. for the filename suffix cmake:set(MAKE_SHARED_LIBRARY_SUFFIX and prefix: msvc uses none, MinGW 'lib', cygwin 'cyg' etc. cmake:set(MAKE_SHARED_LIBRARY_PREFIX)

but I can see that it would allow one to build a static and a shared library version of the VSG and install them on a system, so I can see the value in using that.

Yes, that's the advantage, you install both if you want to and decide later which variant you want to use. With msvc you don't have this possibility to distinguish, so often a workaround is used and _static is appended to the filename.

Do other projects use the .dll.a convention?

According to cmake git repo, cygwin gcc, clang and mingw gcc understand this extension .dll.a and so do other common build systems like meson, qmake.

@robertosfield
Copy link
Collaborator

Thanks for the links. I've had a read through and understand the topic a little better now. My current thought is that the Visual Studio Windows .dll should have the SOVERSION as well, so using WIN32 seems appropriate.

I will merge as is then we can create a branch to test using this for all of WIN32.

@robertosfield robertosfield merged commit 1ea9c86 into vsg-dev:master Dec 15, 2022
@robertosfield
Copy link
Collaborator

I have done the merge and then changed the CMake check to use WIN32 rather than MINGW, the branch is:

https://github.com/vsg-dev/VulkanSceneGraph/tree/Windows_SOVERSION_dll

I'll watch the automated builds now to see is things still compile OK, if it does we'll need Windows users using VS, MinGW and Cygwin to test it out before we merge with VSG master.

I'll make a 1.0.3 release for this and other changes before Christmas.

@rhabacker
Copy link
Contributor Author

rhabacker commented Dec 16, 2022

I have done the merge and then changed the CMake check to use WIN32 rather than MINGW, the branch is:
https://github.com/vsg-dev/VulkanSceneGraph/tree/Windows_SOVERSION_dll

I see

if it does we'll need Windows users using VS

The VC build shows that the installed file names are as expected:
-- Installing: C:/Program Files (x86)/vsg/lib/vsg.lib
-- Installing: C:/Program Files (x86)/vsg/bin/vsg-11.dll

MinGW

The vsg-dev related CI build jobs for mingw was not triggered by this branch and only generates static libraries
On my personal mingw32 builds it works as expected
[ 426s] -- Installing: /home/abuild/rpmbuild/BUILDROOT/mingw32-libvsg-1.0.0+20221215+git.1ea9c86-lp153.439.1.x86_64/usr/i686-w64-mingw32/sys-root/mingw/lib/libvsg.dll.a
[ 426s] -- Installing: /home/abuild/rpmbuild/BUILDROOT/mingw32-libvsg-1.0.0+20221215+git.1ea9c86-lp153.439.1.x86_64/usr/i686-w64-mingw32/sys-root/mingw/bin/libvsg-11.dll

Cygwin

The vsg-dev related CI build jobs for cygwin was not triggered by this branch and only generates static libraries

@rhabacker rhabacker deleted the VersionedMinGWLib branch December 16, 2022 08:16
@robertosfield
Copy link
Collaborator

I think it's safe enough to merge with master so I've gone ahead and merged and have started the windows-mingw-cygwin action:

https://github.com/vsg-dev/VulkanSceneGraph/actions/runs/3712142913

@rhabacker
Copy link
Contributor Author

I think it's safe enough to merge with master so I've gone ahead and merged and have started the windows-mingw-cygwin action:

https://github.com/vsg-dev/VulkanSceneGraph/actions/runs/3712142913

Please note that these jobs are not building a shared library.

@rhabacker
Copy link
Contributor Author

I think it's safe enough to merge with master so I've gone ahead and merged and have started the windows-mingw-cygwin action:
https://github.com/vsg-dev/VulkanSceneGraph/actions/runs/3712142913

Please note that these jobs are not building a shared library.

It is required to add -DBUILD_SHARED_LIBS=1 to https://github.com/vsg-dev/VulkanSceneGraph/blob/master/.github/workflows/win-cygwin-mingw.yml#L27 and https://github.com/vsg-dev/VulkanSceneGraph/blob/master/.github/workflows/win-cygwin-mingw.yml#L65 to see this change.

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.

None yet

2 participants