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

Fix linking errors with LIBSCSYNTH=ON #5014

Merged

Conversation

mossheim
Copy link
Contributor

Purpose and Motivation

This fixes #5013, as well as another linking error with LIBSCSYNTH

On linux, building with LIBSCSYNTH=ON and SYSTEM_BOOST=OFF fails with linking errors because
a static library is being linked into a shared library. solved by compiling the offending library with -fPIC.

On macOS, building with LIBSCSYNTH=ON fails with linking errors because SC_EventLoop.hpp functions used in
scsynth's main() are hidden, similar to what we encountered in #4992. This PR solves the issue the same way as
#5012, by including the file in scsynth rather than libscsynth (and doing the equivalent change for supernova).
This required separating out the event loop-specific functions into a new header and implementation file since
disableAppNap is not used in quite the same way.

Types of changes

  • Bug fix

To-do list

@sonoro1234
Copy link
Contributor

@brianlheim Sorry, I cant review this PR because my macos knowledge is really bad, I dont even have one.

@mossheim mossheim requested review from Spacechild1 and removed request for sonoro1234 June 13, 2020 19:38
@mossheim
Copy link
Contributor Author

@sonoro1234 -- ok, no worries. @Spacechild1 or @joshpar , perhaps you could give this a quick look?

@@ -0,0 +1,34 @@
/************************************************************************
*
* Copyright 2013 Seth Nickell <snickell@gmail.com>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who is Seth Nickell? ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure! this was in the file before, and you kept it when you added your functions. do you want the copyright assignment then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see! well, I would be flattered :-)

@@ -0,0 +1,83 @@
/************************************************************************
*
* Copyright 2013 Seth Nickell <snickell@gmail.com>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@Spacechild1
Copy link
Contributor

Apart from that it looks fine! The event loops certainly belongs to the scsynth/supernova apps.

Note that people can still use the event loop with libscsynth, they just have to include common/SC_AppleEventLoop.cpp in their project and start the event loop manually (once).

@joshpar
Copy link
Member

joshpar commented Jun 14, 2020 via email

@mossheim
Copy link
Contributor Author

I’m guessing event loop code came from another source.

@Spacechild1 is the one who added the event loop code, that's why i asked if they wanted attribution. :) removing Seth Nickell's name here would not be removing attribution, because they did not contribute any of that code.

@joshpar
Copy link
Member

joshpar commented Jun 14, 2020 via email

@Spacechild1
Copy link
Contributor

Spacechild1 commented Jun 14, 2020

I’m wondering if that is the author of some code (or even sample code) that was used for this bit of code

Seth Nickell was not involved in the event loop code. He probably wrote the void disableAppNap(); which was the only function in the SC_Apple.hpp before I added the event loop. The sample code I've referenced is by another author.

otherwise, we get linking errors on macOS

see also #5012, #5013
@mossheim mossheim force-pushed the topic/fix-libscsynth-link-errors branch from a865795 to bdd624b Compare June 14, 2020 17:24
@mossheim
Copy link
Contributor Author

@joshpar @Spacechild1 updated!

@bagong
Copy link
Contributor

bagong commented Jun 14, 2020

Seth Nickell was not involved in the event loop code. He probably wrote the void disableAppNap(); which was the only function in the SC_Apple.hpp before I added the event loop. The sample code I've referenced is by another author.

Yes, that's true, I remember Seth turning up out of nowhere when AppNap was giving people a little scare ;-)

@mossheim mossheim mentioned this pull request Oct 3, 2020
4 tasks
@mossheim mossheim moved this from TODO to in progress in Patch release cherry-picks Oct 24, 2020
@mossheim mossheim moved this from in progress to DONE in Patch release cherry-picks Oct 25, 2020
dvzrv added a commit to dvzrv/supercollider that referenced this pull request Nov 16, 2020
Backport the linker error patches
(supercollider#5014) for 3.11.2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[scsynth] linking error with LIBSCSYNTH
5 participants