[sndfile] Add new port sndfile #4514

evpobr · 2018-10-20T06:43:24Z

This alternative to libsndfile port. As CMake developer for libsndfile I've made huge amout of changes to libsndfile CMake build system, so i think we need to recreate port with new name because:

CMake package search name is now SndFile:
(find_package(SndFile)
Library target is now SndFile::sndfile:
target_link_libraries(Foo PRIVATE SndFile::sndfile)
Library name is now generated by build system, it is sndfile.dll under Windows

Old libsndfile is untouched and still works.

ras0219-msft · 2018-11-08T08:01:44Z

Thanks for the PR and sorry for the long delay getting back to you!

Generally, the reasons to keep a library as a separate ports is:

Is the old version still maintained (there's a bug tracker+community that is fixing issues and publishing new versions for that old release branch)?
Can they both be used simultaneously (i.e. no conflicting headers or symbols)?

If either of these is true, then it would make sense to keep it separate. However, if it is just for CMake compatibility, it would be a much better strategy to just deploy a CMake compatibility file that allows users to find the new package with the old name (in addition to the current name).

Edit: And we can create+deploy this shim inside the portfile without needing upstream changes.

evpobr · 2018-11-08T08:14:06Z

@bagong , what do you think?

bagong · 2018-12-02T14:51:20Z

@evpobr sure! Sorry, I am not following any more these days...

evpobr · 2018-12-29T13:58:31Z

@ras0219-msft , so author of old libsndfile port is agree with me. Old port will be developed no more.

Is the old version still maintained (there's a bug tracker+community that is fixing issues and publishing new versions for that old release branch)?

No, see libsndfile author remark above.

2. Can they both be used simultaneously (i.e. no conflicting headers or symbols)?

No.

However, if it is just for CMake compatibility, it would be a much better strategy to just deploy a CMake compatibility file that allows users to find the new package with the old name (in addition to the current name).

Hmm, probably it is possible to edit libsndfile port to export SndFile::sndfile target. But it is confusing, because official new library target name is SndFile without lib prefix. It is already in upstream. CMake project of libsndfile was documented as experimental when libsndfile port was created, so i don't think we can call it compatibility issue.

frabert · 2019-04-03T11:49:25Z

Has there been any progress on this? The current libsndfile port is not very CMake-friendly and doesn't build under linux

EDIT: Just tried this, it still doesn't build under linux:

The following packages will be built and installed:
    sndfile[core,external-libs]:x64-linux
Starting package 1/1: sndfile:x64-linux
Building package sndfile[core,external-libs]:x64-linux...
-- Downloading https://github.com/erikd/libsndfile/archive/cebfdf275e6173259bee6bfd40de22c8c102cf23.tar.gz...
-- Extracting source /home/frabert/vcpkg/downloads/erikd-libsndfile-cebfdf275e6173259bee6bfd40de22c8c102cf23.tar.gz
-- Applying patch /home/frabert/vcpkg/ports/sndfile/uwp-createfile-getfilesize.patch
-- Applying patch /home/frabert/vcpkg/ports/sndfile/uwp-createfile-getfilesize-addendum.patch
-- Using source at /home/frabert/vcpkg/buildtrees/sndfile/src/c8c102cf23-c25072e23f
-- Configuring x64-linux-dbg
-- Configuring x64-linux-rel
-- Building x64-linux-dbg
-- Building x64-linux-rel
CMake Error at scripts/cmake/vcpkg_fixup_cmake_targets.cmake:45 (message):
  '/home/frabert/vcpkg/packages/sndfile_x64-linux/debug/cmake' does not
  exist.
Call Stack (most recent call first):
  ports/sndfile/portfile.cmake:36 (vcpkg_fixup_cmake_targets)
  scripts/ports.cmake:71 (include)


Error: Building package sndfile:x64-linux failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `.\vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: sndfile:x64-linux
  Vcpkg version: 2018.11.23-unknownhash

Additionally, attach any relevant sections from the log files above.

frabert · 2019-04-05T12:05:22Z

@evpobr I found a workaround for the linux issue:

diff --git a/ports/sndfile/portfile.cmake b/ports/sndfile/portfile.cmake
index cd6ad459..905329ef 100644
--- a/ports/sndfile/portfile.cmake
+++ b/ports/sndfile/portfile.cmake
@@ -33,13 +33,17 @@ vcpkg_configure_cmake(
 
 vcpkg_install_cmake()
 
-vcpkg_fixup_cmake_targets(CONFIG_PATH cmake)
+if(WIN32)
+    vcpkg_fixup_cmake_targets(CONFIG_PATH cmake)
+else()
+    vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/SndFile)
+endif()
 
 vcpkg_copy_pdbs()
 
 file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/share)
 file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include)
-file(RENAME ${CURRENT_PACKAGES_DIR}/share/doc/libsndfile ${CURRENT_PACKAGES_DIR}/share/${PORT}/doc)
+#file(RENAME ${CURRENT_PACKAGES_DIR}/share/doc/libsndfile ${CURRENT_PACKAGES_DIR}/share/${PORT}/doc)
 file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/share/doc)
 
 if(BUILD_EXECUTABLES)

frabert · 2019-04-05T16:59:57Z

ports/sndfile/portfile.cmake

@@ -33,7 +33,11 @@ vcpkg_configure_cmake(

 vcpkg_install_cmake()

-vcpkg_fixup_cmake_targets(CONFIG_PATH cmake)
+if(WIN32)


@evpobr It seems like a + slipped through! :)

frabert · 2019-04-07T15:57:34Z

@Rastaban Can we have a summary of what's failing?

frabert · 2019-04-07T20:40:51Z

@evpobr I've discovered that linking to SndFile::sndfile when sndfile is built with external libraries enabled fails, because the SndFile::sndfile target references Vorbis::VorbisEnc, FLAC::FLAC and others. You will probably need to install the modules you use internally in order for the users to be able to find them too.

evpobr · 2019-04-09T07:03:09Z

@frabert, static build or both?

frabert · 2019-04-09T07:50:52Z

I have only tested with x64-windows-static, but I suspect similar issues would exist with shared build as well

evpobr · 2019-04-09T07:52:39Z

Not sure. Linked libraries are private for shared build.

frabert · 2019-04-09T08:23:13Z

In my experience, private dependencies are still needed to be found on the system because of the way Windows handles symbols. Basically, dlls are not sufficient for the linker to resolve the symbols and still need the .lib files even for shared libraries.

evpobr · 2019-04-09T08:27:49Z

I'm using MSYS2 + MinGW toolchain, shared build target has no external libraries dependencies.

Can you check x64-windows build?

frabert · 2019-04-09T08:35:17Z

I can confirm that x64-windows works fine (on MSVC btw)

evpobr · 2019-04-09T08:57:18Z

So it is Libsndfile issue. But the problem is compicated. The simple solution is to search dependencies in SndFileConfig.cmake. To make it work i need package config modules installed with Vcpkg for all externall libraries.

The status is:

Ogg port installs module with unofficial Ogg::ogg target ([libogg] Add package config module #5384).
Opus port will install it with official Opus::opus target (vcpkg install opus arm-uwp #4875)
Vorbis - ??? Port needs to be fixed
VorbisEnc - ??? Port needs to be fixed
Speex - ??? Port needs to be fixed

Export library names are also problem. Because CMake developers din't explain it, we have many variants: (Ogg::Ogg, LibOgg::ogg, Ogg::ogg and so on). But i think it should be:

Namespace in camel case, all uppercase if abbreviation, no 'lib' prefix (Ogg:: (not abbreviation), Vorbis::,: Opus::, FLAC::, Speex::)
exported library name as it is (Ogg::ogg, Vorbis::vorbis, Vorbis::vorbisenc. Opus::opus, FLAC::FLAC, Speex::speex).

frabert · 2019-04-09T09:01:58Z

If I can suggest a course of action: it seems that most of the libraries you depend on don't natively offer CMake packages, thus you rely on vcpkg. What I would do is create FindXXX.cmake modules that first do a find_package(XXX CONFIG) to check if there is a vcpkg package installed, if nothing is found you resume to the standard find_library/find_path course, but you also create an UNKNOWN IMPORTED library with the same name as the one that the vcpkg packages use. This way, you have a uniform interface for both systems that use vcpkg and those that use their standard package manager.

evpobr · 2019-04-09T09:12:37Z

Hmm. Maybe just copy Libsndfile FindXXX modules to package config directory? And add find_package(XXX CONFIG) to SndFileConfig.cmake... It could work, Can you test it as Vcpkg patch now? I have no Vcpkg here.

frabert · 2019-04-09T09:28:00Z

Copying the FindXXX modules to config directory and adding

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
find_package(Ogg REQUIRED)
find_package(FLAC REQUIRED)
find_package(Vorbis REQUIRED)
find_package(VorbisEnc REQUIRED)

towards the end of SndFileConfig.cmake seems to do the trick

evpobr · 2019-04-09T09:36:00Z

find_dependency is recommended in package config modules.

frabert · 2019-04-09T09:42:58Z

I fear that still doesn't solve the fact that two different SndFileConfig.cmake files are needed depending on whether the external libraries are used or not

evpobr · 2019-04-09T09:54:42Z

The solution is configure_package_config_file.

It is possible to generate SndFileConfig.cmake from SndFileConfig.cmake.in which includes ${CMAKE_CURRENT_LIST_DIR}/SndFileTargets.cmake

find_dependency(Ogg)
...
@PACKAGE_INIT@

set(HAVE_EXTERNAL_LIBS @HAVE_EXTERNAL_LIBS@)

include("${CMAKE_CURRENT_LIST_DIR}/SndFileTargets.cmake")

check_required_components(SndFile)

Then we need to install both SndFileTargets.cmakeand SndFileConfig.cmake

frabert · 2019-04-09T10:10:56Z

I think this is best solved upstream though

otherwise the vcpkg_configure_cmake can fail.

[port] Add sndfile port

3d9c3ff

Rastaban self-assigned this Mar 14, 2019

evpobr and others added 2 commits April 5, 2019 17:40

[sndfile] Fix configuration on non-Win32 platforms

a2dcd51

Fix stray '+'

35a361c

frabert approved these changes Apr 7, 2019

View reviewed changes

[sndfile] add python to path

db5fe1d

otherwise the vcpkg_configure_cmake can fail.

Rastaban changed the title ~~[port] Add sndfile port~~ [sndfile] Add new port sndfile May 2, 2019

Rastaban merged commit bba224b into microsoft:master May 2, 2019

evpobr deleted the port/sndfile branch May 27, 2019 03:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sndfile] Add new port sndfile #4514

[sndfile] Add new port sndfile #4514

evpobr commented Oct 20, 2018

ras0219-msft commented Nov 8, 2018 •

edited

Loading

evpobr commented Nov 8, 2018

bagong commented Dec 2, 2018

evpobr commented Dec 29, 2018 •

edited

Loading

frabert commented Apr 3, 2019 •

edited

Loading

frabert commented Apr 5, 2019

frabert Apr 5, 2019

frabert commented Apr 7, 2019

frabert commented Apr 7, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019 •

edited

Loading

frabert commented Apr 9, 2019

[sndfile] Add new port sndfile #4514

[sndfile] Add new port sndfile #4514

Conversation

evpobr commented Oct 20, 2018

ras0219-msft commented Nov 8, 2018 • edited Loading

evpobr commented Nov 8, 2018

bagong commented Dec 2, 2018

evpobr commented Dec 29, 2018 • edited Loading

frabert commented Apr 3, 2019 • edited Loading

frabert commented Apr 5, 2019

frabert Apr 5, 2019

Choose a reason for hiding this comment

frabert commented Apr 7, 2019

frabert commented Apr 7, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019

frabert commented Apr 9, 2019

evpobr commented Apr 9, 2019 • edited Loading

frabert commented Apr 9, 2019

ras0219-msft commented Nov 8, 2018 •

edited

Loading

evpobr commented Dec 29, 2018 •

edited

Loading

frabert commented Apr 3, 2019 •

edited

Loading

evpobr commented Apr 9, 2019 •

edited

Loading