is travis not running on this ? because its not a source code file change ?

What do you mean not "very robust" ? Just that it might not find the brew installed version (due to quirky homebrew installs etc.) ?

Any downsides to discuss ?

thanks

Yes, it would not find non standard homebrew, or any other libsndfile install, except for that single location in /usr/local/lib|include. Original findModules try to cover several places where libraries might be found. This just looks in 1 place and if it doesn't find it, it falls back to the bundled binary. I am not aware of another downside, but maybe somebody else is...

Well, there is also this static/shared question: the old libsndfile is linked statically... Sam suggested dynamic and I also used to use that. I am not sure why back then static was chosen. I think an advantage of dynamic is that they seem to bring in libvorbis and libflac automatically. But to which degree SC can make use of it, I don't know (but it looks good to have four additional sndfile related dylibs in the MacOS folder: libFLAC, libOgg, libvorbis and libvorbisenc ;))

someone should adapt https://github.com/supercollider/supercollider/blob/master/common/SC_SndFileHelpers.hpp#L59

as for static/dynamic linking: dynamic linking is recommended, as libsndfile links to other libs. cmake's fixup_bundle should copy the dylibs to the bundle

there is one point to consider: are homebrew builds relocatable? especially: is a binary installed via homebrew on 10.11 guaranteed to work on 10.8?

@tim: what would an adaption include? Add Ogg as format just the way the others are added and remove that version check, that is not necessary any more? Btw. raw is missing, that's on purpose, right?
As to portability to 10.8: how to check/to know other than test? I can't access a 10.8 machine.

Btw, the release builds are the travis builds, aren't they? They're built with 10.9, if I am not mistaken. Wouldn't that reduce the risk dramatically?

as to relocation, this seems to be the way to go:

brew install --build-bottle

Homebrew/legacy-homebrew#31483

(copied useful comment from #1783):

It seems homebrew supplies libraries that to some degree are optimized for the OSX version and the system architecture of the host system. As long as you build for yourself, that's good. But if you build for a release, it's a different story. It's planned to use the Travis build for release, right? One has to make sure libsndfile is built in a way that works on all OSs and architectures targeted by the release.
Using --build-bottle with brew install libsndfile causes libsndfile to be compiled locally. But I'm not sure what other flags to set, to make the build is as generic as possible. Otoh that question doesn't really need to be addressed in the pull request, I think, but in the Travis build (if it's going to be used for the release)

yes, we use travis build for release.

so I'm not 100% sure, but I think this is ready for merge.

except for that "you can use:" issue of the docs I just noticed and commented about.

I'll correct that typo. Give me a few hours. I am currently getting a 10.8 build system on a virtual machine ready and I'll try to confirm the open questions empirically.
As it just popped up in that diff: I don't think building for 32-bit is possible any more. Maybe remove that CMAKE_OSX_ARCHITECTURES at the same time?

Haha, conflicting with my own commit. Will fix that when doing the typo, in an hour or so.

So, the reservation turned out as valid:

1. I couldn't build on 10.8, it breaks in Qt (with Qt 5.5.1, which had to be compiled by homebrew, which doesn't support/maintain 10.8 any more. Maybe it would work with an older Qt).
2. Went on to build in 10.9 pretty much like the Travis build. Built successfully.
3. Ran sc and booted server, looks normal
4. Copied sc-folder to 10.8 Vanilla with latest updates. Start SC okay, on server boot:

*** ERROR: dlopen '/Users/rainer/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens.scx' err 'dlopen(/Users/rainer/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens.scx, 2): Library not loaded: /usr/local/opt/libsndfile/lib/libsndfile.1.dylib
  Referenced from: /Users/rainer/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens.scx
  Reason: image not found'
...
SC_AudioDriver: sample rate = 44100.000000, driver's block size = 512
SuperCollider 3 server ready.
Receiving notification messages from server localhost
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
exception in GraphDef_Recv: UGen 'DiskOut' not installed.
Shared memory server interface initialized
exception in GraphDef_Recv: UGen 'DiskOut' not installed.

But! Let's have clear that this not really about this pull request, but about deployment of homebrew libs in a release build. If libsndfile is not available on Travis, things will be as before. If you build locally with homebrew libsndfile, this problem will not occur. As I see it, there are two alternatives:

• release SC with the old libsndfile (will make sonic-pi people unhappy and isn't really cool anyways)
• find proper compile flags for the libsndfile build to compile in in a way that works on all targeted architectures/OSes

Best
.r.

Sorry, it still conflicts, but the merge is simple. If required I can do it if the PR is to be committed. Have to go now for a while...

fixup_bundle should be used to copy all dylibs into the app bundle https://cmake.org/cmake/help/v3.4/module/BundleUtilities.html (or dll next to the dlls/exes)

It does so already. So it could actually change the libs to be more generic? Is there a way to tell it that it should do some special treatment to the libsndfile libs? Would be nice if a custom libsndfile compile could be avoided.

When trying out @crucialfelix' move of scsynth to the Resources folder, I realized something that is relevant for this PR, although I consider it more a bug that has been there before, and that should be solved regardless of whether this PR is commited or not:

integrating a the homebrew libsndfile by adding the sndfile locations via the sndfile findModule, or by passing them in via the cmake command-line arguments -DSNDFILE_LIBRARY/INCLUDE_DIR causes the libs (libsndfile, libogg and a few more) to be copied to the bundle/MacOS folder. It "looks" ;) as if they were used there (and maybe they are by sclang?). And yet, if I uninstall the homebrew install of libsndfile on my system, the server cannot be booted any more. So it looks as if the server uses the library in the homebrew install location rather than the one bundled with SC. Note that this is the case both when scsynth is located in bundle/Resources, and when it is located in bundle/MacOS. So it's not the question whether scsynth and libsndfile are in the same folder...

This also means that the portability test I did by running an 10.9 built SC using homebrew libsndfile on a 10.8 machine is meaningless. The server can't boot on any machine, where libsndfile is not installed in the system...

I think I found the reason why scsynth doesn't used the "installed" sndfile dylib:

SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)

is both in sc-ide and lang CMakeLists.txt, but not in the server file. Likely because everything is (used to be) statically linked in scsynth.

So provided I am right, what is the better fix? Add something corresponding for the server (and supernova?) or link the latest libsndfile statically too?

We are still undecided, right ? You need to look at this more ? No stress right now, we have time.

Yes, this absolutely needs more work (I just translated Sam's PR here) I'd like to concentrate on Windows right now, but I'll try to come back here in a week or so if nobody else does.

I think this is okay now, but if people are worried, I can switch it to the master branch.

Turned out what was needed, is that DiskIO_UGens get's told where to look for libsndfile, so I added all sc-plugins to the plugins slot in fixup_bundle. It works without libsndfile being installed now.

I also found that supernova wants a pointer to libsndfile in /usr/local/opt/libsndfile rather than just /usr/local, not sure why, but I fulfilled its wish, alongside with raising the priority of the path from PATHS to HINTS to avoid finding libsndfile if it resides in the environment PATH

As to portability: first of all all this has no effect, if libsndfile isn't installed, then the old static library will be used. Second: it dawned (denglish) on me that libsndfile is no different from Qt and readline, so I guess no special treatment should be needed.

I actually have a bit of doubts as to the effect of this. In theory also the old 1.0.18 version should already support ogg and flac. But I guess Sam made the original pull request for a reason. I verified that ogg and flac files can be opened and played. And flac files can also be written in a recording. Writing ogg doesn't work for me, not sure why and if it should...

What could still be done is remove all those references to the libsndfile version like add_definitions("-DLIBSNDFILE_1018"), which do appear obsolete in 2016.

On a sidenote: I wonder how sc3-plugins handle the fftw dependency. Likely so far only people who have fftw installed used it. What if that was provided as an independent package, wouldn't fftw have to be bundled and fixup_bundle be run there as well?

Confirmed working nicely.

-- Found homebrew install of libsndfile

-- 15/108: copying '/usr/local/lib/libFLAC.8.dylib'
-- 16/108: copying '/usr/local/lib/libogg.0.dylib'
-- 17/108: copying '/usr/local/lib/libsndfile.1.dylib'
-- 18/108: copying '/usr/local/lib/libvorbisenc.2.dylib'

-- 68/108: fixing up '/Users/crucial/code/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/MacOS/libvorbis.0.dylib'
-- 69/108: fixing up '/Users/crucial/code/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/MacOS/libFLAC.8.dylib'
-- 70/108: fixing up '/Users/crucial/code/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/MacOS/libogg.0.dylib'
-- 71/108: fixing up '/Users/crucial/code/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/MacOS/libsndfile.1.dylib'
-- 72/108: fixing up '/Users/crucial/code/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/MacOS/libvorbisenc.2.dylib'

@crucialfelix, note that the release will not contain the libsndfile/flac/vorbis dylib. This would require the travis build to install libsndfile. I am fine with any choice, let me know. Adjusting travis should be trivial.

That's just the libsndfile that I installed with homebrew. I'm posting proof for other interested people that it does indeed build with the homebrew - as advertised.

Also we can see "fixing up" which I think is correcting the dynamic linking paths (after having copied them). Am I right about that part ?

Though I get what you mean now—we could make the release build support FLAC and ogg which would be nice.

Your scrennshot from above (thanks for testing!) doesn't show whether it's the executables fixup or the plugins fixup. But that doesn't matter really, that's not the risky part of the commit, it should reproduce nicely on all systems.
What we don't have conclusive proof of is that the homebrew dylib is portable to all mac systems sc is supposed to run on. Although I think the fact that just the same is true for the Qt libs and readline should be a good indicator.
I have OSX 10.7 on a virtual machine. But I'd need a travis build first to see if it runs there... (Building it on my own machine isn't reliable as it's el-capitan)

Actually that should be easy by creating a pull request with the travis change and have it build. I'd just have to find out what the download link for that build would be... I'll try...

Ooops, while observing the travis build I just discovered that current build target is set to 10.9. Probably an accident that happened to Scott C. when he froze the Qt version? I guess I should change that?

"deployment_target", sorry

The travis build does the fixup with the plugins too, but it's a bit different from what I expected:

'/Users/travis/build/supercollider/supercollider/BUILD/Install/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens.scx'
-- 13/108: copying '/usr/local/Cellar/libvorbis/1.3.5/lib/libvorbis.0.dylib'
-- 14/108: copying '/usr/local/lib/libogg.0.dylib'
-- 15/108: copying '/usr/local/opt/flac/lib/libFLAC.8.dylib'
-- 16/108: copying '/usr/local/opt/libsndfile/lib/libsndfile.1.dylib'
-- 17/108: copying '/usr/local/opt/libvorbis/lib/libvorbisenc.2.dylib'

It shouldn't be a problem, as all libs are from the same homebrew install, but it's a bit untidy. I'd like to inspect the bundle. I'll rebuild with deployment target set to 10.7. We want that anyways.