Skip to content

Enable building on Fedora linux #3239

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

Merged
merged 7 commits into from
Jan 6, 2023
Merged

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Nov 28, 2022

Indicate dependencies and modify build scripts creating errors.

Also addressing #3238

@lilyinstarlight
Copy link
Contributor

With IMGUI, it can use either gl3w or GLEW depending on which is available and our other Linux builds all use gl3w. We also pull in gl3w from vcpkg to ensure it exists and therefore GLEW should never be needed with a vcpkg build

Were you encountering issues with linking against GLEW when USE_SYSTEM_LIBS was on or using a build with vcpkg dependencies?

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 28, 2022

With a vcpkg build, cmake complained about not finding libGLEW. vcpkg did download gl3w, and cmake did not bring up errors about not finding gl3w.

@lilyinstarlight
Copy link
Contributor

Interesting, thank you for the info!

I'll see if I can replicate in a Fedora VM myself, because something else may be wrong if it's looking for GLEW

Also as a quick note: the imgui interface can be disabled via -n to the linux-build-all.sh script or any of the others to avoid the issue

@lilyinstarlight
Copy link
Contributor

lilyinstarlight commented Nov 29, 2022

I was able to replicate the issue on a fresh Fedora 37 install and interestingly the result of CMake configure records that it should link against gl3w and not GLEW, but it doesn't add the gl3w include directory to the flags and so the compiler only finds the GLEW headers (if it's installed) and not gl3w -- Edit: Ignore me I missed that it adds vcpkg/installed/x64-linux/include to flags which includes the gl3w headers

I'll investigate a bit further later today because something is definitely up here

@lilyinstarlight
Copy link
Contributor

lilyinstarlight commented Nov 29, 2022

It looks like imgui searches for GLEW before it searches for gl3w, which means it does actually build correctly if glew-devel is not installed

This patch should force imgui to always select gl3w when built from the CMakeLists file that requires it:

diff --git a/app/gui/imgui/CMakeLists.txt b/app/gui/imgui/CMakeLists.txt
index 0cc68866d..a8924c517 100644
--- a/app/gui/imgui/CMakeLists.txt
+++ b/app/gui/imgui/CMakeLists.txt
@@ -93,6 +93,9 @@ target_link_directories(${APP_NAME}
 	PRIVATE
 	 /opt/vc/lib
 	 )
+else()
+    target_compile_definitions(${APP_NAME} PRIVATE
+        IMGUI_IMPL_OPENGL_LOADER_GL3W)
 endif()
 
 target_link_libraries (${APP_NAME}

Could you adjust your patch to add that instead of adding GLEW to the CMakeLists and replace glew-devel with mesa-libGLU-devel in the BUILD-LINUX dependency list?

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 29, 2022

Thanks. Will check this tomorrow.

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 30, 2022

It works with your changes. Thanks.

### 1.2 Fedora
```bash
sudo dnf group install "Development Tools"
sudo dnf install alsa-utils aubio-devel catch-devel cmake crossguid-devel elixir erlang erlang-xmerl fmt-devel glew-devel gsl-lite-devel jack-audio-connection-kit-devel jack-audio-connection-kit-example-clients libsndfile-devel ninja-build openssl-devel qt5-qtconfiguration-devel qt5-qttools-devel qt5-qtsvg-devel reproc-devel ruby-devel SDL2-devel supercollider-devel vcpkg
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm wondering if all of this is necessary since vcpkg bootstraps itself and some of these dependencies (like SDL2 or reproc or ninja) should be coming from there rather than the system. Also I'm not sure jack-audio-connection-kit-devel and supercollider-devel would even be necessary (but supercollider would be)

Also is sc3-plugins included in the supercollider package? That is needed for some synths

I'll do some testing again later today to see about this, but this PR is looking pretty good now!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

supercollider-devel will bring in supercollider. sc3-plugins does not seem to be packaged in the main repositories:
https://src.fedoraproject.org/rpms/supercollider/blob/rawhide/f/supercollider.spec
There is no separate sc3-plugins in Fedora. sc3-plugins are available from Audinux:
https://github.com/audinux/fedora-spec/blob/master/supercollider/supercollider-sc3-plugins.spec
It would be helpful to know what features are not available without this.

Using packaged dependencies minimizes bloat. It seems many changes are needed in the CMakeLists files to check if an installed library can be used, at present options for only either a set of all installed or all downloaded dependencies it would be great to use what is packaged. For stable dependencies, it would be great to use what is packaged in many distributions. For example catch2, crossguid, FLAC, fmt, reproc, ogg, opus, SDL2, sndfile, uuid, vorbis should be used if possible - using these would also enable easier packaging in the main repositories. KissFFT and gl3w are not in Fedora main repositories, though could be added. vcpkg is not bootstrapped, packaged version seems adequate.

One also needs to start jack audio in a separate terminal, $jackd -d alsa.

Copy link
Contributor

@lilyinstarlight lilyinstarlight Nov 30, 2022

Choose a reason for hiding this comment

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

It's a couple of the synthdefs that need sc3-plugins. The only synth off the top of my head that needs it is the :piano synth, but @samaaron can probably elaborate on others

The vcpkg builds (and BUILD-LINUX.md instructions) are specifically for users (who tend to be on Ubuntu or Raspbian and often older versions) that want it working with tested dep versions and it is used for the macOS/Windows releases. I made it possible to build without any vendored dependencies specifically to support the Linux packaging use-case

Would it be possible to package kissfft and gl3w then for Fedora? I'm happy to assist with that and then assist with creating an rpmspec that builds entirely from system dependencies

Also you can use pipewire with Sonic Pi too instead of JACK if desired (I know desktop Fedora defaults to that now), and Sonic Pi should be able to start a basic JACK server itself if it needs -- Let us know if that isn't working right

Copy link
Contributor

@lilyinstarlight lilyinstarlight Nov 30, 2022

Choose a reason for hiding this comment

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

Oh yeah and only supercollider but not supercollider-devel should be necessary since only the scsynth server binary and base ugens/plugins are actually needed. Not any of the development headers or anything like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kiss FFT is in Fedora https://src.fedoraproject.org/rpms/kiss-fft so only gl3w and sc3-plugins need to be added to the main repositories.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the interest of getting the gl3w fix and basic Fedora instructions merged, can you try just the following and see if this is all that is needed with the default build?

It seems to be all that I need in a freshly installed VM

Suggested change
sudo dnf install alsa-utils aubio-devel catch-devel cmake crossguid-devel elixir erlang erlang-xmerl fmt-devel glew-devel gsl-lite-devel jack-audio-connection-kit-devel jack-audio-connection-kit-example-clients libsndfile-devel ninja-build openssl-devel qt5-qtconfiguration-devel qt5-qttools-devel qt5-qtsvg-devel reproc-devel ruby-devel SDL2-devel supercollider-devel vcpkg
sudo dnf install alsa-utils cmake elixir erlang erlang-xmerl mesa-libGLU-devel jack-audio-connection-kit-devel jack-audio-connection-kit-example-clients ninja-build openssl-devel alsa-lib-devel qt5-qtconfiguration-devel qt5-qttools-devel qt5-qtsvg-devel rubygem-rexml ruby-devel supercollider

We can work on/track the proper Fedora packaging in a new issue

Thank you again so much for your work on this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks will check. Would suggest using vcpkg from Fedora repositories since the setup for that matches typical user preferences which a direct download may not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks will check. Would suggest using vcpkg from Fedora repositories since the setup for that matches typical user preferences which a direct download may not.

Yeah, I can imagine a number of users may prefer it to use system vcpkg if available, especially since then vcpkg won't try to manage its own prebuilt ninja/cmake for bootstrapping. So we do appreciate the suggestion!

I just suppose that would be up to @samaaron though because then we wouldn't be managing vcpkg repo version (for packages) ourselves for those users, and I'm not sure it gets us much benefit over the current choice between all system dependencies or all non-Qt/non-supercollider dependencies managed by us (once all of the system deps are available in Fedora that is)

Vcpkg won't be necessary for users at all if they choose to build using all system deps (-s to the build scripts)

Also as far as I know, we tried to manage dependencies using our own bootstrapped vcpkg (with repo pinned to a particular version) to begin with because Ubuntu was missing a lot of deps or the deps were too old, and users with less coding experience ran into too many difficulties getting it all built themselves if a distro package wasn't available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Packaged version of vcpkg in Fedora works very well and is much preferred to bootstrapping. The version is quite recent.

At present one can either use all system dependencies or none. Updating this to check for suitable versions of system dependencies and using them if available is another ticket.

It is possible to integrate CI testing using podman to check builds work on a variety of distributions. Can add this for Fedora.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need the following for it to build in GitHub actions see #3247:

dnf -y install alsa-lib-devel alsa-utils cmake elixir erlang erlang-xmerl \
  jack-audio-connection-kit-example-clients libX11-devel \
  libXft-devel libXext-devel mesa-libGLU-devel \
  ninja-build openssl-devel \
  pipewire-jack-audio-connection-kit \
  qt5-qtconfiguration-devel qt5-qttools-devel \
  qt5-qtsvg-devel rubygem-rexml ruby-devel supercollider

Would have preferred to use vcpkg from Fedora repos, but it seems the current configuration
forces downloads of most packages including bootstrapping vcpkg. In Fedora telemetry data is
disabled, but default vcpkg install has telemetry
enabled.

To minimize bloat, it is better to determine minimum and possibly maximum release versions of
dependencies and use those, rather than specific commits. This also enables taking advantage
of security updates. This does of course also require more effort in maintenance and testing,
though automation can help.

@samaaron
Copy link
Collaborator

Hiya, just wanted to drop by and say thanks so much @bkmgit for all your efforts with getting Sonic Pi to play nicely with Fedora. Thanks also, as always, to @lilyinstarlight for the amazing support and help with the Linux packaging efforts.

I really hope that we can continue to improve our support for the various distros and if there's any modification to the source structure / dependency linking / etc. that would make this pursuit easier I'd be delighted to discuss them.

@samaaron samaaron merged commit e57fd84 into sonic-pi-net:dev Jan 6, 2023
@samaaron
Copy link
Collaborator

samaaron commented Jan 6, 2023

Thanks so much!

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