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

Use new carla-native-plugin library #4593

Closed
wants to merge 1 commit into from
Closed

Conversation

falkTX
Copy link
Contributor

@falkTX falkTX commented Sep 13, 2018

Also fixes build with latest carla (which will be 2.0 soon)

Should be pushed to stable-1.2 branch too, but there will be conflicts to resolve for that.
@tresf can I let you handle that?

Also fixes build with latest carla (which will be 2.0 soon)
@PhysSong
Copy link
Member

stable-1.1 is no longer used, so you should open a PR against stable-1.2.
I think @tresf can handle conflicts. If not, I can help you resolve them.

@PhysSong PhysSong closed this Sep 13, 2018
@falkTX
Copy link
Contributor Author

falkTX commented Sep 13, 2018

stable-1.1 is still the latest stable release of LMMS available, and it is simply not true that is no longer used - I am using it myself and everyone with kxstudio repos.
Without the patch, the build fails against latest carla.
This is a bugfix, not a new feature. Please merge.

@PhysSong
Copy link
Member

BTW, won't it make the build fail with old version of Carla?

@falkTX
Copy link
Contributor Author

falkTX commented Sep 13, 2018

no, as the library it uses was changed. it no longer links against the full carla standalone lib, but only against the exported plugin version (libcarla_native-plugin.so).
it makes a lot more sense to link against this new object file.

as it is now, stable-1.1 finds carla-standalone and then fails to build.
the patch fixes this, among other things.

@PhysSong
Copy link
Member

I meant there's no fallback logic for old versions. It looks like a regression to me. Can we try carla-standalone if the old version of Carla is detected?

stable-1.1 finds carla-standalone and then fails to build.

I can't reproduce it. Do you have a build log with the failure?

@falkTX
Copy link
Contributor Author

falkTX commented Sep 13, 2018

treat it as a regression if you wish, I feel like now it is doing things proper.
the symbols it uses are all defined in a single header file, which is the one also used when exporting the library.
the old behaviour was hacky, experimenting with getting it to work.
now with carla going stable (2.0), it was time to clear up the hacks and make things work properly.

I dont have the logs with the failure, but the error only happens due to -Werror.
You need latest git of carla (from maximum 2 days ago) to get the new header file and library.

@tresf
Copy link
Member

tresf commented Sep 13, 2018

I have mixed feelings about this landing on the stable-1.1 branch. First of all, I really don't care because the branch is stale, unmaintained and doesn't provide any binaries to our website, so the impact is only downstream and I can't speak on behalf of downstream + Carla. @falkTX perhaps you can shed some insight on this?

So the impact of merging this to stable-1.1 is:

What I think would be more responsible of a change -- albeit leaving cruft into the Carla build system -- is for @falkTX to leave the old library around and deprecate it off sanely instead of putting this decision on us at LMMS. The justification for this is we've worked with package maintainers that require a version bump to receive a change so I'd like to be cognizant of that. Otherwise, they'd potentially get the new Carla and the old LMMS and then it would break.

So a single, small commit to 1.1.3 to provide compatibility with the latest Carla seems rather benign but I want to know how this can impact other distros and that's something I simply don't know. Bumping 1.1.3 to 1.1.4 for this is possible as well so as long a package maintainers are careful to update LMMS and Carla simultaneously (which I wouldn't expect them to know to do).

@falkTX thoughts welcome. :)

Edit: And I just remembered that PKG_CHECK_MODULES(CARLA carla-native-plugin) is going to make sure the build system is prepared, so the worst that will happen is Carla will just be missing from LMMS downstream if the versions aren't matched up.

@falkTX
Copy link
Contributor Author

falkTX commented Sep 13, 2018

thanks for the reply.

I understand the situation, I was hoping 1.1 would still be maintained in a way.
it is the current stable branch, and the one I am still using. but it is fine that it is not supported anymore.
In such cases, then I really recommend taking out -Werror, and that is it.
Without that flag, carla will keep building and working just fine with the current code.

Most distros don't have Carla afaik.
I know Arch does, I know the maintainer for the packager and we meet in Berlin from time to time, so I can request updates in person :P
Some Fedora and Gentoo audio overlay/repos/external-sources contain it too, but I dont know how those work.
Debian does not have it, neither does Ubuntu.

@tresf could you please merge this patch into your lmms-carla-osx PR?
I think pushing to 1.2 branch right now is a bit pointless, if you will have conflicts with it right away, so you will have to resolve them anyway.

@tresf
Copy link
Member

tresf commented Sep 13, 2018

@tresf could you please merge this patch into your lmms-carla-osx PR?
I think pushing to 1.2 branch right now is a bit pointless, if you will have conflicts with it right away, so you will have to resolve them anyway.

Yes, I'll make it part of the stable-1.2 macOS Carla PR (#4558)

@tresf tresf mentioned this pull request Sep 13, 2018
3 tasks
@tresf tresf mentioned this pull request Sep 15, 2018
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.

3 participants